-
Notifications
You must be signed in to change notification settings - Fork 12.3k
Add Governor module connecting with AccessManager #4523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
frangio
merged 36 commits into
OpenZeppelin:feat/access-manager
from
frangio:governor-timelock-access
Aug 16, 2023
Merged
Changes from 7 commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
5fd9a98
add back GovernorTimelock
frangio 1498539
WIP
frangio 36dc884
rename GovernorTimelock -> GovernorTimelockAccess
frangio 2d2b087
make base delay modifiable
frangio 221261f
remove available since
frangio 499a2a1
use relay to call the target contract
frangio d5ca8be
fix warning
frangio 7ed7c1c
Update contracts/governance/extensions/GovernorTimelockAccess.sol
frangio bf9fdf9
fix initial set of base delay
frangio 5ced769
rename maxDelay -> neededDelay
frangio 3820637
address guardian cancellation risk
frangio 31e20a1
use single manager per governor
frangio 56ed5a4
add nonces
frangio 6ca99ef
make _hashOperation private
frangio e34c093
add docs for nonce
frangio 40adbb7
typo
frangio 2c41d41
Update contracts/governance/extensions/GovernorTimelockAccess.sol
frangio 1446af0
Update contracts/governance/extensions/GovernorTimelockAccess.sol
frangio bc2bfa5
Update contracts/access/manager/AccessManager.sol
frangio af274b1
add proposalNeedsQueuing
frangio bac912e
remove timepoint from executed and canceled events
frangio a1cbc83
add tests for proposalNeedsQueuing
frangio ebd6dcd
Apply suggestions from code review
frangio 6902ad7
fix docs
frangio e96474c
remove unnecessary override
frangio 452c65e
remove _authorityOverride
frangio f7b3b93
add missing docs
frangio 3b47038
remove unused imports
frangio 8d5e734
typo
frangio 4f89411
lint
frangio 40eea48
add basic tests
frangio 0604f4d
add custom error
frangio 16519fe
lint
frangio 85148a9
make variable private
frangio a36618b
add changeset
frangio b37ed30
do not delete executionPlan
frangio File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
216 changes: 216 additions & 0 deletions
216
contracts/governance/extensions/GovernorTimelockAccess.sol
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,216 @@ | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| pragma solidity ^0.8.20; | ||
|
|
||
| import {IGovernor, Governor} from "../Governor.sol"; | ||
| import {IAuthority} from "../../access/manager/IAuthority.sol"; | ||
| import {AuthorityUtils} from "../../access/manager/AuthorityUtils.sol"; | ||
| import {IAccessManager} from "../../access/manager/IAccessManager.sol"; | ||
| import {IAccessManaged} from "../../access/manager/IAccessManaged.sol"; | ||
| import {Address} from "../../utils/Address.sol"; | ||
| import {Math} from "../../utils/math/Math.sol"; | ||
| import {SafeCast} from "../../utils/math/SafeCast.sol"; | ||
| import {Time} from "../../utils/types/Time.sol"; | ||
|
|
||
| /** | ||
| * @dev TODO | ||
| */ | ||
| abstract contract GovernorTimelockAccess is Governor { | ||
| struct ExecutionPlan { | ||
| uint32 delay; | ||
| IAccessManager[] managers; | ||
| } | ||
|
|
||
| mapping(uint256 => ExecutionPlan) private _executionPlan; | ||
| mapping(address target => address) private _authorityOverride; | ||
|
|
||
| uint32 _baseDelay; | ||
|
|
||
| error GovernorUnmetDelay(uint256 proposalId, uint256 neededTimestamp); | ||
|
|
||
| event BaseDelaySet(uint32 oldBaseDelaySeconds, uint32 newBaseDelaySeconds); | ||
|
|
||
| constructor(uint32 initialBaseDelay) { | ||
| _baseDelay = initialBaseDelay; | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| /** | ||
| * @dev Base delay that will be applied to all function calls. Some may be further delayed by their associated | ||
| * `AccessManager` authority; in this case the final delay will be the maximum of the base delay and the one | ||
| * demanded by the authority. | ||
| * | ||
| * NOTE: Execution delays are processed by the `AccessManager` contracts, and according to that contract are | ||
| * expressed in seconds. Therefore, the base delay is also in seconds, regardless of the governor's clock mode. | ||
| */ | ||
| function baseDelaySeconds() public view virtual returns (uint32) { | ||
| return _baseDelay; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Change the value of {baseDelaySeconds}. This operation can only be invoked through a governance proposal. | ||
| */ | ||
| function setBaseDelaySeconds(uint32 newBaseDelay) public virtual onlyGovernance { | ||
| _setBaseDelaySeconds(newBaseDelay); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Change the value of {baseDelaySeconds}. Internal function without access control. | ||
| */ | ||
| function _setBaseDelaySeconds(uint32 newBaseDelay) internal virtual { | ||
| emit BaseDelaySet(_baseDelay, newBaseDelay); | ||
ernestognw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| _baseDelay = newBaseDelay; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Public accessor to check the execution details. | ||
| */ | ||
| function proposalExecutionPlan(uint256 proposalId) public view returns (ExecutionPlan memory) { | ||
| return _executionPlan[proposalId]; | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {IGovernor-propose} | ||
| */ | ||
| function propose( | ||
| address[] memory targets, | ||
| uint256[] memory values, | ||
| bytes[] memory calldatas, | ||
| string memory description | ||
| ) public virtual override returns (uint256) { | ||
| uint256 proposalId = super.propose(targets, values, calldatas, description); | ||
|
|
||
| uint32 maxDelay = baseDelaySeconds(); | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| ExecutionPlan storage plan = _executionPlan[proposalId]; | ||
|
|
||
| for (uint256 i = 0; i < targets.length; ++i) { | ||
| (address manager, uint32 delay) = _detectExecutionRequirements(targets[i], bytes4(calldatas[i])); | ||
| plan.managers.push(IAccessManager(manager)); | ||
| // downcast is safe because both arguments are uint32 | ||
| maxDelay = uint32(Math.max(delay, maxDelay)); | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| plan.delay = maxDelay; | ||
|
|
||
| return proposalId; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Function to queue a proposal to the timelock. | ||
| * | ||
| * NOTE: execution delay is estimated based on the delay information retrieved in {proposal}. This value may be | ||
| * off if the delay were updated during the vote. | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| */ | ||
| function _queueOperations( | ||
| uint256 proposalId, | ||
| address[] memory targets, | ||
| uint256[] memory /* values */, | ||
| bytes[] memory calldatas, | ||
| bytes32 /* descriptionHash */ | ||
| ) internal virtual override returns (uint48) { | ||
| ExecutionPlan storage plan = _executionPlan[proposalId]; | ||
| uint48 eta = Time.timestamp() + plan.delay; | ||
|
|
||
| for (uint256 i = 0; i < targets.length; ++i) { | ||
| IAccessManager manager = plan.managers[i]; | ||
| if (address(manager) != address(0)) { | ||
| manager.schedule(targets[i], calldatas[i], eta); | ||
| } | ||
| } | ||
|
|
||
| return eta; | ||
| } | ||
|
|
||
| function _executeOperations( | ||
| uint256 proposalId, | ||
| address[] memory targets, | ||
| uint256[] memory values, | ||
| bytes[] memory calldatas, | ||
| bytes32 /* descriptionHash */ | ||
| ) internal virtual override { | ||
| uint256 eta = proposalEta(proposalId); | ||
ernestognw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (block.timestamp < eta) { | ||
| revert GovernorUnmetDelay(proposalId, eta); | ||
| } | ||
|
|
||
| ExecutionPlan storage plan = _executionPlan[proposalId]; | ||
|
|
||
| for (uint256 i = 0; i < targets.length; ++i) { | ||
| IAccessManager manager = plan.managers[i]; | ||
| if (address(manager) != address(0)) { | ||
| manager.relay{value: values[i]}(targets[i], calldatas[i]); | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } else { | ||
| (bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]); | ||
| Address.verifyCallResult(success, returndata); | ||
frangio marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| delete _executionPlan[proposalId]; | ||
| } | ||
|
|
||
| /** | ||
| * @dev See {IGovernor-_cancel} | ||
| */ | ||
| function _cancel( | ||
| address[] memory targets, | ||
| uint256[] memory values, | ||
| bytes[] memory calldatas, | ||
| bytes32 descriptionHash | ||
| ) internal virtual override returns (uint256) { | ||
| uint256 proposalId = super._cancel(targets, values, calldatas, descriptionHash); | ||
|
|
||
| ExecutionPlan storage plan = _executionPlan[proposalId]; | ||
|
|
||
| for (uint256 i = 0; i < targets.length; ++i) { | ||
| IAccessManager manager = plan.managers[i]; | ||
| if (address(manager) != address(0)) { | ||
| // Attempt to cancel, but allow to revert if a guardian cancelled part of the proposal | ||
| try manager.cancel(address(this), targets[i], calldatas[i]) {} catch {} | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| delete _executionPlan[proposalId]; | ||
|
|
||
| return proposalId; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Check if the execution of a call needs to be performed through an AccessManager and what delay should be | ||
| * applied to this call. | ||
| * | ||
| * Returns { manager: address(0), delay: _baseDelaySeconds() } if: | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| * - target does not have code | ||
| * - target does not implement IAccessManaged | ||
| * - calling canCall on the target's manager returns a 0 delay | ||
| * - calling canCall on the target's manager reverts | ||
| * Otherwise (calling canCall on the target's manager returns a non 0 delay), return the address of the | ||
| * AccessManager to use, and the delay for this call. | ||
| */ | ||
| function _detectExecutionRequirements(address target, bytes4 selector) private view returns (address, uint32) { | ||
| address authority = _authorityOverride[target]; | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // Get authority from target contract if there is no override | ||
| if (authority == address(0)) { | ||
| bool success; | ||
| bytes memory returndata; | ||
|
|
||
| (success, returndata) = target.staticcall(abi.encodeCall(IAccessManaged.authority, ())); | ||
| if (success && returndata.length >= 0x20) { | ||
| authority = abi.decode(returndata, (address)); | ||
| } | ||
ernestognw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| if (authority != address(0)) { | ||
| // Check if governor can call according to authority, and try to detect a delay | ||
| (bool authorized, uint32 delay) = AuthorityUtils.canCallWithDelay(authority, address(this), target, selector); | ||
|
|
||
| // If direct call is not authorized, and delayed call is possible | ||
| if (!authorized && delay > 0) { | ||
| return (authority, delay); | ||
| } | ||
ernestognw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // Use internal delay mechanism | ||
| return (address(0), 0); | ||
| } | ||
| } | ||
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.
Uh oh!
There was an error while loading. Please reload this page.