OpenZeppelin / OpenZeppelin/openzeppelin-contracts

coded `pauseFor(uint time)` functionality for `Pausable.sol`

Open
#5,378 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

idea
Dominant language
Solidity
Stars
27.2k
Forks
12.4k
Avg merge
2d 19h
Merged PRs (30d)
33

Description

Summary 📓

I've re-factored, in a backwards compatible way, the Pausable.sol contract to allow developers to optionally support a pauseFor(uint time) functionality.

All tested and coded in this fork: my fork

Motivation 🧐

It is common to use the Pausable.sol contract in a project with some special permissions to 1 or more addresses.

However, as of now it exists the danger of pausing forever. The simplest example is if the only permissioned address losses its private key while the contract was paused.

Another more complex real-example I came across while auditing and which motivated me to propose this change is the following:

A project had 1 owner and X number of "guardians" which could pause the contracts.

The owner could be renounced to `address(0)` and once that was done, no-one, including guardians, 
would be able to pause the contracts again.

A malicious guardian could out of malice, or simply by accident, front-run the owner's renouncement 
call with a pause function call, effectively locking the contract forever.
Conclusion 🔚

As protocols evolve in complexity and more complex role structures and permissions are created, the risk of pausing something forever won't be lower but higher. Thus, I consider useful this re-factored Pausable.sol contract to allow developers to optionally support a pauseFor() functionality.

Technical Details 💻
  • Backwards compatible:

The amount of state slots used remains unchanged. Only the first one. And new values do not override previous values as:

Previous values: 0 or 1 on the least significant bit.
New values: 0 or 1 on the least significant bit, and the remaining 248 bytes are used for duration amounts when using pauseFor().

This makes proxies who desire to implement this functionality easily updateable.

  • Optional functionality:

As per the usual Pasuable.sol, all new functionalities have been created using internal functions, thus if the project using Pausable.sol thinks they do not need the extra pauseFor() functionality they can just never call it and the compiler won't include it in the final bytecode.

How it works ⚙️
  • _pauseFor(uint time): Allows the addresses capable of pausing to pause for "at most" time seconds if the contract is not paused already.

This action can be unpaused anytime by _unpause() or ONLY WHEN TIME HAS ELAPSED by _unpauseAfterPausedFor().

It is up to the project to decide if to allow earlier unpausing of _pauseFor() through _unapuse() or not. If not done the code will be paused until the time has elapsed.

Ideally the projects should make _unpauseAfterPausedFor() callable by anyone, this function allows unpausing actions started with pauseFor(uint time) after time seconds have passed.

Code changes 📜

0️⃣ The only state bool private _paused; now is uint256 private _pausedInfo; with the back-wards compatible structure explained earlier.

1️⃣ 3 functions, 1 event and 1 error added are:

  • function _pauseFor(uint256 time) internal virtual whenNotPaused

  • function _unpauseAfterPausedFor() internal virtual whenPaused

  • function _unpauseDeadline() internal view virtual returns (uint256)

  • event PausedFor(address account, uint256 duration);

  • error PauseDurationNotElapsed();

2️⃣ SafeCast.sol library is now used to safely toUint248() when manipulating slot data-structure.

3️⃣ For readability when manipulating the slot, 2 constants have been added to the file:

  • uint8 constant PAUSED = 1;
  • uint8 constant PAUSE_DURATION_OFFSET = 8;

4️⃣ None already existing functions were deleted, they have only been modified to adapt to the new data structure in the slot.

5️⃣ New test cases have been added to account for new pauseFor() execution flows:

  • pasuedFor and then unpaused with classic _unpause() interactions.
  • pasuedFor and then unpaused with _unpauseAfterPausedFor() interactions.
  • pausedFor and then classic _pause interactions.
  • Classic _pause and then pauseFor interactions.
  • Classic _pause and then _unpauseAfterPausedFor interactions.
  • Classic _unpause() and then pauseFor interactions.
  • Classic _unpause() and then _unpauseAfterPausedFor interactions.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with contracts/utils/Pausable.sol and compare it with the linked fork, then inspect SafeCast.sol and the existing Pausable tests. Verify the proposed pauseFor, unpauseAfterPausedFor, and unpauseDeadline flows, including interactions with classic pause and unpause behavior, while preserving the stated storage-slot compatibility.

Written by the indexing model from the issue text.

Assessment

Tech stack
solidity
Domain
blockchain, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.