Pausable contracts #311
Merged
Pausable contracts #311
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implement an emergency stop mechanism that can be triggered by an authorized account. In stBTC contract we should pause deposits and withdrawals.
Implement an emergency stop mechanism that can be triggered by an authorized account. In `AcreBitcoinDepositor` contract we should pause staking finalization and unstaking in the future.
✅ Deploy Preview for acre-dapp-testnet canceled.
|
This error could be useful in other contracts so we extract it to a separate file `Errors.sol` so we can reuse it across other contracts. To avoid duplicating code, other common errors should be included here.
This abstract contract extracts a common part of the emergency stop mechanism. The emergency stop mechanism can be triggered by an authorized account. Only owner of the contract can update the emergency stop account. The child contract must override the `_checkOwner` internal function that checks if the caller is an owner of contract and throws an error if the sender is not the owner.
Inherit the emergency stop mechanism from `AbstractPausable` contract and override correctly the `_checkOwner` function.
Inherit the emergency stop mechanism from `AbstractPausable` contract and override correctly the `_checkOwner` function.
nkuba
reviewed
Mar 15, 2024
The staking flow can be paused by `stBTC.deposit` function pausing.
`emergencyStopAccount` -> `pauseAdmin`
dimpar
reviewed
Mar 18, 2024
Fix the test scenario when the owner calls `unpause` function - the caller should be an owner (in our case it's a `governance` account).
nkuba
reviewed
Mar 18, 2024
Variable name should be camel cased.
`NotAuthorizedAccount` -> `PausableUnauthorizedAccount`. To align with `OwnableUnauthorizedAccount` thrown from the `Ownable` contract.
Remove the `isOwner` function and the `_onlyOwner` modifier as they were only used in one place. There may also be a name collision with the `Ownable` contract from the OZ library because it also defines the `onlyOwner` modifier, so it may be confusing when and which modifier should be used in the child contract.
The init functions are here to replace the constructor, so we should place them in the place we define the constructor.
Inherit the `Ownable2StepUpgradeable` contract because we can assume the `Pausable` contract will always by `Ownable` as well.
nkuba
reviewed
Apr 2, 2024
Move the pasue admin setter to execute before ownership transfer.
nkuba
previously approved these changes
Apr 4, 2024
Upgradable contracts should define a gap as per https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
|
@r-czajkowski I let myself update this PR. Please take a look, and if you're fine with the proposed changes I'll merge the PR. |
nkuba
approved these changes
Apr 4, 2024
Thanks! Everything looks good! |
dimpar
approved these changes
Apr 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on: #308Closes: #170
This PR adds support for the emergency stop mechanism for
stBTCcontract by implementing thePausableUpgradeablecontract from Open Zeppelin library.What has been done
PausableOwnablecontractExtract common logic for the emergency stop mechanism by creating the
PausableOwnablecontract. It inherits thePausableUpgradeableandOwnable2StepUpgradeablecontracts from Open Zeppelin library. We assume that thePausablecontract will always beOwnableas well. The emergency stop mechanism can be triggered by an authorized account and it can be different than theowneraccount. Only owner of the contract can update the emergency stop account.Make the
stBTCcontract pausableMake the
stBTCcontract pausable by inheriting thePausableOwnablecontract that provides the emergency stop mechanism. Functions that move funds should be pausble:depositandmint,withdrawandredeem.Other
ZeroAddress) to a separate fileErrors.solso we can reuse errors in the other contracts.