From 29a4bab90be6d1840300a231f77911845d7709f4 Mon Sep 17 00:00:00 2001 From: Aleksandr Tarelkin Date: Fri, 2 Aug 2024 00:22:04 +0300 Subject: [PATCH 1/6] HashConsensus unit tests --- test/unit/HashConsensus.t.sol | 375 ++++++++++++++++++++++++++++++++++ 1 file changed, 375 insertions(+) create mode 100644 test/unit/HashConsensus.t.sol diff --git a/test/unit/HashConsensus.t.sol b/test/unit/HashConsensus.t.sol new file mode 100644 index 00000000..be2fb1e5 --- /dev/null +++ b/test/unit/HashConsensus.t.sol @@ -0,0 +1,375 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import {UnitTest} from "test/utils/unit-test.sol"; + +import {Vm} from "forge-std/Test.sol"; + +import {HashConsensus} from "../../contracts/committees/HashConsensus.sol"; +import {Duration} from "../../contracts/types/Duration.sol"; + +abstract contract HashConsensusUnitTest is UnitTest { + HashConsensus internal _hashConsensus; + + address internal _owner = makeAddr("COMMITTEE_OWNER"); + + address internal _stranger = makeAddr("STRANGER"); + + uint256 internal _membersCount = 13; + uint256 internal _quorum = 7; + address[] internal _committeeMembers = new address[](_membersCount); + + constructor() { + for (uint256 i = 0; i < _membersCount; ++i) { + _committeeMembers[i] = makeAddr(string(abi.encode(0xFE + i * _membersCount + 65))); + } + } + + function test_isMember() public { + for (uint256 i = 0; i < _membersCount; ++i) { + assertEq(_hashConsensus.isMember(_committeeMembers[i]), true); + } + + assertEq(_hashConsensus.isMember(_owner), false); + assertEq(_hashConsensus.isMember(_stranger), false); + } + + function test_getMembers() public { + address[] memory committeeMembers = _hashConsensus.getMembers(); + + assertEq(committeeMembers.length, _committeeMembers.length); + + for (uint256 i = 0; i < _membersCount; ++i) { + assertEq(committeeMembers[i], _committeeMembers[i]); + } + } + + function test_addMember_stranger_call() public { + address newMember = makeAddr("NEW_MEMBER"); + assertEq(_hashConsensus.isMember(newMember), false); + + vm.prank(_stranger); + vm.expectRevert(abi.encodeWithSignature("OwnableUnauthorizedAccount(address)", _stranger)); + _hashConsensus.addMember(newMember, _quorum); + + for (uint256 i = 0; i < _membersCount; ++i) { + vm.prank(_committeeMembers[i]); + vm.expectRevert(abi.encodeWithSignature("OwnableUnauthorizedAccount(address)", _committeeMembers[i])); + _hashConsensus.addMember(newMember, _quorum); + } + } + + function test_addMember_reverts_on_duplicate() public { + address existedMember = _committeeMembers[0]; + assertEq(_hashConsensus.isMember(existedMember), true); + + vm.prank(_owner); + vm.expectRevert(abi.encodeWithSignature("DuplicatedMember(address)", existedMember)); + _hashConsensus.addMember(existedMember, _quorum); + } + + function test_addMember_reverts_on_invalid_quorum() public { + address newMember = makeAddr("NEW_MEMBER"); + assertEq(_hashConsensus.isMember(newMember), false); + + vm.prank(_owner); + vm.expectRevert(abi.encodeWithSignature("InvalidQuorum()")); + _hashConsensus.addMember(newMember, 0); + + vm.prank(_owner); + vm.expectRevert(abi.encodeWithSignature("InvalidQuorum()")); + _hashConsensus.addMember(newMember, _membersCount + 2); + } + + function test_addMember() public { + address newMember = makeAddr("NEW_MEMBER"); + uint256 newQuorum = _quorum + 1; + + assertEq(_hashConsensus.isMember(newMember), false); + + vm.prank(_owner); + vm.expectEmit(address(_hashConsensus)); + emit HashConsensus.MemberAdded(newMember); + vm.expectEmit(address(_hashConsensus)); + emit HashConsensus.QuorumSet(newQuorum); + _hashConsensus.addMember(newMember, newQuorum); + + assertEq(_hashConsensus.isMember(newMember), true); + + address[] memory committeeMembers = _hashConsensus.getMembers(); + + assertEq(committeeMembers.length, _membersCount + 1); + assertEq(committeeMembers[committeeMembers.length - 1], newMember); + } + + function test_removeMember_stranger_call() public { + address memberToRemove = _committeeMembers[0]; + assertEq(_hashConsensus.isMember(memberToRemove), true); + + vm.prank(_stranger); + vm.expectRevert(abi.encodeWithSignature("OwnableUnauthorizedAccount(address)", _stranger)); + _hashConsensus.removeMember(memberToRemove, _quorum); + + for (uint256 i = 0; i < _membersCount; ++i) { + vm.prank(_committeeMembers[i]); + vm.expectRevert(abi.encodeWithSignature("OwnableUnauthorizedAccount(address)", _committeeMembers[i])); + _hashConsensus.removeMember(memberToRemove, _quorum); + } + } + + function test_removeMember_reverts_on_member_is_not_exist() public { + assertEq(_hashConsensus.isMember(_stranger), false); + + vm.prank(_owner); + vm.expectRevert(abi.encodeWithSignature("IsNotMember()")); + _hashConsensus.removeMember(_stranger, _quorum); + } + + function test_removeMember_reverts_on_invalid_quorum() public { + address memberToRemove = _committeeMembers[0]; + assertEq(_hashConsensus.isMember(memberToRemove), true); + + vm.prank(_owner); + vm.expectRevert(abi.encodeWithSignature("InvalidQuorum()")); + _hashConsensus.removeMember(memberToRemove, 0); + + vm.prank(_owner); + vm.expectRevert(abi.encodeWithSignature("InvalidQuorum()")); + _hashConsensus.removeMember(memberToRemove, _membersCount); + } + + function test_removeMember() public { + address memberToRemove = _committeeMembers[0]; + uint256 newQuorum = _quorum - 1; + + assertEq(_hashConsensus.isMember(memberToRemove), true); + + vm.prank(_owner); + vm.expectEmit(address(_hashConsensus)); + emit HashConsensus.MemberRemoved(memberToRemove); + vm.expectEmit(address(_hashConsensus)); + emit HashConsensus.QuorumSet(newQuorum); + _hashConsensus.removeMember(memberToRemove, newQuorum); + + assertEq(_hashConsensus.isMember(memberToRemove), false); + + address[] memory committeeMembers = _hashConsensus.getMembers(); + + assertEq(committeeMembers.length, _membersCount - 1); + for (uint256 i = 0; i < committeeMembers.length; ++i) { + assertNotEq(committeeMembers[i], memberToRemove); + } + } +} + +contract Target { + event Executed(); + + function trigger() public { + emit Executed(); + } +} + +contract HashConsensusWrapper is HashConsensus { + event OnlyMemberModifierPassed(); + + Target internal _target; + + constructor( + address owner, + address[] memory newMembers, + uint256 executionQuorum, + uint256 timelock, + Target target + ) HashConsensus(owner, newMembers, executionQuorum, timelock) { + _target = target; + } + + function vote(bytes32 hash, bool support) public { + _vote(hash, support); + } + + function execute(bytes32 hash) public { + _markUsed(hash); + _target.trigger(); + } + + function getHashState(bytes32 hash) + public + view + returns (uint256 support, uint256 execuitionQuorum, bool isExecuted) + { + return _getHashState(hash); + } + + function getSupport(bytes32 hash) public view returns (uint256 support) { + return _getSupport(hash); + } + + function onlyMemberProtected() public onlyMember { + emit OnlyMemberModifierPassed(); + } +} + +contract HashConsensusInternalUnitTest is HashConsensusUnitTest { + HashConsensusWrapper internal _hashConsensusWrapper; + Target internal _target; + Duration internal _timelock = Duration.wrap(3600); + + bytes internal data; + bytes32 internal dataHash; + + function setUp() public { + _target = new Target(); + _hashConsensusWrapper = + new HashConsensusWrapper(_owner, _committeeMembers, _quorum, _timelock.toSeconds(), _target); + _hashConsensus = HashConsensus(_hashConsensusWrapper); + data = abi.encode(address(_target)); + dataHash = keccak256(data); + } + + function test_getSupport() public { + assertEq(_hashConsensusWrapper.getSupport(dataHash), 0); + + for (uint256 i = 0; i < _membersCount; ++i) { + assertEq(_hashConsensusWrapper.getSupport(dataHash), i); + vm.prank(_committeeMembers[i]); + _hashConsensusWrapper.vote(dataHash, true); + assertEq(_hashConsensusWrapper.getSupport(dataHash), i + 1); + } + + assertEq(_hashConsensusWrapper.getSupport(dataHash), _membersCount); + + for (uint256 i = 0; i < _membersCount; ++i) { + assertEq(_hashConsensusWrapper.getSupport(dataHash), _membersCount - i); + vm.prank(_committeeMembers[i]); + _hashConsensusWrapper.vote(dataHash, false); + assertEq(_hashConsensusWrapper.getSupport(dataHash), _membersCount - i - 1); + } + + assertEq(_hashConsensusWrapper.getSupport(dataHash), 0); + } + + function test_getHashState() public { + uint256 support; + uint256 execuitionQuorum; + bool isExecuted; + + (support, execuitionQuorum, isExecuted) = _hashConsensusWrapper.getHashState(dataHash); + assertEq(support, 0); + assertEq(execuitionQuorum, _quorum); + assertEq(isExecuted, false); + + for (uint256 i = 0; i < _membersCount; ++i) { + (support, execuitionQuorum, isExecuted) = _hashConsensusWrapper.getHashState(dataHash); + assertEq(support, i); + assertEq(execuitionQuorum, _quorum); + assertEq(isExecuted, false); + + vm.prank(_committeeMembers[i]); + _hashConsensusWrapper.vote(dataHash, true); + + (support, execuitionQuorum, isExecuted) = _hashConsensusWrapper.getHashState(dataHash); + assertEq(support, i + 1); + assertEq(execuitionQuorum, _quorum); + assertEq(isExecuted, false); + } + + (support, execuitionQuorum, isExecuted) = _hashConsensusWrapper.getHashState(dataHash); + assertEq(support, _membersCount); + assertEq(execuitionQuorum, _quorum); + assertEq(isExecuted, false); + + _wait(_timelock); + + _hashConsensusWrapper.execute(dataHash); + + (support, execuitionQuorum, isExecuted) = _hashConsensusWrapper.getHashState(dataHash); + assertEq(support, _membersCount); + assertEq(execuitionQuorum, _quorum); + assertEq(isExecuted, true); + } + + function test_vote() public { + assertEq(_hashConsensusWrapper.approves(_committeeMembers[0], dataHash), false); + + vm.prank(_committeeMembers[0]); + vm.expectEmit(address(_hashConsensusWrapper)); + emit HashConsensus.Voted(_committeeMembers[0], dataHash, true); + _hashConsensusWrapper.vote(dataHash, true); + assertEq(_hashConsensusWrapper.approves(_committeeMembers[0], dataHash), true); + + vm.prank(_committeeMembers[0]); + vm.recordLogs(); + _hashConsensusWrapper.vote(dataHash, true); + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(logs.length, 0); + assertEq(_hashConsensusWrapper.approves(_committeeMembers[0], dataHash), true); + + vm.prank(_committeeMembers[0]); + vm.expectEmit(address(_hashConsensusWrapper)); + emit HashConsensus.Voted(_committeeMembers[0], dataHash, false); + _hashConsensusWrapper.vote(dataHash, false); + assertEq(_hashConsensusWrapper.approves(_committeeMembers[0], dataHash), false); + + vm.prank(_committeeMembers[0]); + vm.recordLogs(); + _hashConsensusWrapper.vote(dataHash, false); + logs = vm.getRecordedLogs(); + assertEq(logs.length, 0); + assertEq(_hashConsensusWrapper.approves(_committeeMembers[0], dataHash), false); + } + + function test_vote_reverts_on_executed() public { + for (uint256 i = 0; i < _quorum; ++i) { + vm.prank(_committeeMembers[i]); + _hashConsensusWrapper.vote(dataHash, true); + } + + _wait(_timelock); + + _hashConsensusWrapper.execute(dataHash); + + vm.prank(_committeeMembers[0]); + vm.expectRevert(abi.encodeWithSignature("HashAlreadyUsed()")); + _hashConsensusWrapper.vote(dataHash, true); + } + + function test_execute_events() public { + vm.prank(_stranger); + vm.expectRevert(abi.encodeWithSignature("QuorumIsNotReached()")); + _hashConsensusWrapper.execute(dataHash); + + for (uint256 i = 0; i < _quorum; ++i) { + vm.prank(_committeeMembers[i]); + _hashConsensusWrapper.vote(dataHash, true); + } + + vm.prank(_stranger); + vm.expectRevert(abi.encodeWithSignature("TimelockNotPassed()")); + _hashConsensusWrapper.execute(dataHash); + + _wait(_timelock); + vm.prank(_stranger); + vm.expectEmit(address(_hashConsensusWrapper)); + emit HashConsensus.HashUsed(dataHash); + vm.expectEmit(address(_target)); + emit Target.Executed(); + _hashConsensusWrapper.execute(dataHash); + + vm.prank(_stranger); + vm.expectRevert(abi.encodeWithSignature("HashAlreadyUsed()")); + _hashConsensusWrapper.execute(dataHash); + } + + function test_onlyMemberModifier() public { + vm.prank(_stranger); + vm.expectRevert(abi.encodeWithSignature("SenderIsNotMember()")); + _hashConsensusWrapper.onlyMemberProtected(); + + vm.prank(_committeeMembers[0]); + vm.expectEmit(address(_hashConsensus)); + emit HashConsensusWrapper.OnlyMemberModifierPassed(); + _hashConsensusWrapper.onlyMemberProtected(); + } +} From 0b1b3de8d7f463992dc25c7e8e6fddc2c61eb225 Mon Sep 17 00:00:00 2001 From: Aleksandr Tarelkin Date: Fri, 2 Aug 2024 10:19:03 +0300 Subject: [PATCH 2/6] update naming changing --- .../EmergencyActivationCommittee.sol | 11 +-- docs/plan-b.md | 12 ++-- docs/specification.md | 12 ++-- test/scenario/emergency-committee.t.sol | 72 +++++++++++++++++++ test/utils/scenario-test-blueprint.sol | 6 +- 5 files changed, 93 insertions(+), 20 deletions(-) create mode 100644 test/scenario/emergency-committee.t.sol diff --git a/contracts/committees/EmergencyActivationCommittee.sol b/contracts/committees/EmergencyActivationCommittee.sol index d71b6e33..dff2b05d 100644 --- a/contracts/committees/EmergencyActivationCommittee.sol +++ b/contracts/committees/EmergencyActivationCommittee.sol @@ -5,7 +5,7 @@ import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {HashConsensus} from "./HashConsensus.sol"; interface IEmergencyProtectedTimelock { - function emergencyActivate() external; + function activateEmergencyMode() external; } /// @title Emergency Activation Committee Contract @@ -27,7 +27,7 @@ contract EmergencyActivationCommittee is HashConsensus { /// @notice Approves the emergency activation by casting a vote /// @dev Only callable by committee members - function approveEmergencyActivate() public onlyMember { + function approveexecuteActivateEmergencyMode() public onlyMember { _vote(EMERGENCY_ACTIVATION_HASH, true); } @@ -35,7 +35,7 @@ contract EmergencyActivationCommittee is HashConsensus { /// @return support The number of votes in support of the activation /// @return execuitionQuorum The required number of votes for execution /// @return isExecuted Whether the activation has been executed - function getEmergencyActivateState() + function getActivateEmergencyModeState() public view returns (uint256 support, uint256 execuitionQuorum, bool isExecuted) @@ -45,10 +45,11 @@ contract EmergencyActivationCommittee is HashConsensus { /// @notice Executes the emergency activation if the quorum is reached /// @dev Calls the emergencyActivate function on the Emergency Protected Timelock contract - function executeEmergencyActivate() external { + function executeActivateEmergencyMode() external { _markUsed(EMERGENCY_ACTIVATION_HASH); Address.functionCall( - EMERGENCY_PROTECTED_TIMELOCK, abi.encodeWithSelector(IEmergencyProtectedTimelock.emergencyActivate.selector) + EMERGENCY_PROTECTED_TIMELOCK, + abi.encodeWithSelector(IEmergencyProtectedTimelock.activateEmergencyMode.selector) ); } } diff --git a/docs/plan-b.md b/docs/plan-b.md index 1674a677..c17f45dc 100644 --- a/docs/plan-b.md +++ b/docs/plan-b.md @@ -333,27 +333,27 @@ Initializes the contract with an owner, committee members, a quorum, and the add #### Preconditions - `executionQuorum` MUST be greater than 0. -### Function: `EmergencyActivationCommittee.approveEmergencyActivate` +### Function: `EmergencyActivationCommittee.approveActivateEmergencyMode` ```solidity -function approveEmergencyActivate() public onlyMember +function approveActivateEmergencyMode() public onlyMember ``` Approves the emergency activation by voting on the `EMERGENCY_ACTIVATION_HASH`. #### Preconditions - MUST be called by a committee member. -### Function: `EmergencyActivationCommittee.getEmergencyActivateState` +### Function: `EmergencyActivationCommittee.getActivateEmergencyModeState` ```solidity -function getEmergencyActivateState() +function getActivateEmergencyModeState() public view returns (uint256 support, uint256 executionQuorum, bool isExecuted) ``` Returns the state of the emergency activation proposal, including the support count, quorum, and execution status. -### Function: `EmergencyActivationCommittee.executeEmergencyActivate` +### Function: `EmergencyActivationCommittee.executeActivateEmergencyMode` ```solidity -function executeEmergencyActivate() external +function executeActivateEmergencyMode() external ``` Executes the emergency activation by calling the `emergencyActivate` function on the `EmergencyProtectedTimelock` contract. diff --git a/docs/specification.md b/docs/specification.md index 8ff2f00f..2b9fb419 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -1326,10 +1326,10 @@ executionQuorum MUST be greater than 0. emergencyProtectedTimelock MUST be a valid address. -### Function: EmergencyActivationCommittee.approveEmergencyActivate +### Function: EmergencyActivationCommittee.approveActivateEmergencyMode ```solidity -function approveEmergencyActivate() public onlyMember +function approveActivateEmergencyMode() public onlyMember ``` Approves the emergency activation by voting on the EMERGENCY_ACTIVATION_HASH. @@ -1338,10 +1338,10 @@ Approves the emergency activation by voting on the EMERGENCY_ACTIVATION_HASH. * MUST be called by a member. -### Function: EmergencyActivationCommittee.getEmergencyActivateState +### Function: EmergencyActivationCommittee.getActivateEmergencyModeState ```solidity -function getEmergencyActivateState() +function getActivateEmergencyModeState() public view returns (uint256 support, uint256 executionQuorum, bool isExecuted) @@ -1349,10 +1349,10 @@ function getEmergencyActivateState() Returns the state of the emergency activation proposal including support count, quorum, and execution status. -### Function: EmergencyActivationCommittee.executeEmergencyActivate +### Function: EmergencyActivationCommittee.executeActivateEmergencyMode ```solidity -function executeEmergencyActivate() external +function executeActivateEmergencyMode() external ``` Executes the emergency activation by calling the emergencyActivate function on the EmergencyProtectedTimelock contract. diff --git a/test/scenario/emergency-committee.t.sol b/test/scenario/emergency-committee.t.sol new file mode 100644 index 00000000..671a1afb --- /dev/null +++ b/test/scenario/emergency-committee.t.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import { + ScenarioTestBlueprint, percents, ExternalCall, ExternalCallHelpers +} from "../utils/scenario-test-blueprint.sol"; + +import {EmergencyProtectedTimelock} from "contracts/EmergencyProtectedTimelock.sol"; + +import {DAO_AGENT} from "../utils/mainnet-addresses.sol"; + +contract EmergencyCommitteeTest is ScenarioTestBlueprint { + address internal immutable _VETOER = makeAddr("VETOER"); + uint256 public constant PAUSE_INFINITELY = type(uint256).max; + + function setUp() external { + _selectFork(); + _deployDualGovernanceSetup( /* isEmergencyProtectionEnabled */ false); + _depositStETH(_VETOER, 1 ether); + } + + function test_proposal_approval() external { + uint256 quorum; + uint256 support; + bool isExecuted; + + address[] memory members; + + ExternalCall[] memory proposalCalls = ExternalCallHelpers.create(address(0), new bytes(0)); + uint256 proposalIdToExecute = _submitProposal(_dualGovernance, "Proposal for execution", proposalCalls); + + // Emergency Activation + members = _emergencyActivationCommittee.getMembers(); + for (uint256 i = 0; i < _emergencyActivationCommittee.quorum() - 1; i++) { + vm.prank(members[i]); + _emergencyActivationCommittee.approveActivateEmergencyMode(); + (support, quorum, isExecuted) = _emergencyActivationCommittee.getActivateEmergencyModeState(); + assert(support < quorum); + assert(isExecuted == false); + } + + vm.prank(members[members.length - 1]); + _emergencyActivationCommittee.approveActivateEmergencyMode(); + (support, quorum, isExecuted) = _emergencyActivationCommittee.getActivateEmergencyModeState(); + assert(support == quorum); + assert(isExecuted == false); + + _emergencyActivationCommittee.executeActivateEmergencyMode(); + (support, quorum, isExecuted) = _emergencyActivationCommittee.getActivateEmergencyModeState(); + assert(support < quorum); + + // Emergency Execute + members = _emergencyExecutionCommittee.getMembers(); + for (uint256 i = 0; i < _emergencyExecutionCommittee.quorum() - 1; i++) { + vm.prank(members[i]); + _emergencyExecutionCommittee.voteEmergencyExecute(proposalIdToExecute, true); + (support, quorum, isExecuted) = _emergencyExecutionCommittee.getEmergencyExecuteState(proposalIdToExecute); + assert(support < quorum); + assert(isExecuted == false); + } + + vm.prank(members[members.length - 1]); + _emergencyExecutionCommittee.voteEmergencyExecute(proposalIdToExecute, true); + (support, quorum, isExecuted) = _emergencyExecutionCommittee.getEmergencyExecuteState(proposalIdToExecute); + assert(support == quorum); + assert(isExecuted == false); + + _emergencyExecutionCommittee.executeEmergencyExecute(proposalIdToExecute); + (support, quorum, isExecuted) = _emergencyExecutionCommittee.getEmergencyExecuteState(proposalIdToExecute); + assert(support < quorum); + } +} diff --git a/test/utils/scenario-test-blueprint.sol b/test/utils/scenario-test-blueprint.sol index fd1eb261..b8f2bc8d 100644 --- a/test/utils/scenario-test-blueprint.sol +++ b/test/utils/scenario-test-blueprint.sol @@ -691,13 +691,13 @@ contract ScenarioTestBlueprint is Test { _wait(_config.AFTER_SCHEDULE_DELAY() + ONE_SECOND); } - function _executeEmergencyActivate() internal { + function _executeActivateEmergencyMode() internal { address[] memory members = _emergencyActivationCommittee.getMembers(); for (uint256 i = 0; i < _emergencyActivationCommittee.quorum(); ++i) { vm.prank(members[i]); - _emergencyActivationCommittee.approveEmergencyActivate(); + _emergencyActivationCommittee.approveActivateEmergencyMode(); } - _emergencyActivationCommittee.executeEmergencyActivate(); + _emergencyActivationCommittee.executeActivateEmergencyMode(); } function _executeEmergencyExecute(uint256 proposalId) internal { From 2add98277553593991eb8a0ae8ea9bc6b97c276f Mon Sep 17 00:00:00 2001 From: Aleksandr Tarelkin Date: Fri, 2 Aug 2024 12:03:41 +0300 Subject: [PATCH 3/6] restore tests --- .../EmergencyActivationCommittee.sol | 2 +- contracts/committees/ResealCommittee.sol | 6 +- test/scenario/emergency-committee.t.sol | 15 +++-- test/scenario/reseal-committee.t.sol | 67 +++++++++++++++++++ test/utils/scenario-test-blueprint.sol | 22 +++++- 5 files changed, 103 insertions(+), 9 deletions(-) create mode 100644 test/scenario/reseal-committee.t.sol diff --git a/contracts/committees/EmergencyActivationCommittee.sol b/contracts/committees/EmergencyActivationCommittee.sol index dff2b05d..652b1999 100644 --- a/contracts/committees/EmergencyActivationCommittee.sol +++ b/contracts/committees/EmergencyActivationCommittee.sol @@ -27,7 +27,7 @@ contract EmergencyActivationCommittee is HashConsensus { /// @notice Approves the emergency activation by casting a vote /// @dev Only callable by committee members - function approveexecuteActivateEmergencyMode() public onlyMember { + function approveActivateEmergencyMode() public onlyMember { _vote(EMERGENCY_ACTIVATION_HASH, true); } diff --git a/contracts/committees/ResealCommittee.sol b/contracts/committees/ResealCommittee.sol index 7b7d27d4..3e98cbf4 100644 --- a/contracts/committees/ResealCommittee.sol +++ b/contracts/committees/ResealCommittee.sol @@ -6,7 +6,7 @@ import {HashConsensus} from "./HashConsensus.sol"; import {ProposalsList} from "./ProposalsList.sol"; interface IDualGovernance { - function reseal(address[] memory sealables) external; + function resealSealables(address[] memory sealables) external; } /// @title Reseal Committee Contract @@ -59,7 +59,9 @@ contract ResealCommittee is HashConsensus, ProposalsList { (, bytes32 key) = _encodeResealProposal(sealables); _markUsed(key); - Address.functionCall(DUAL_GOVERNANCE, abi.encodeWithSelector(IDualGovernance.reseal.selector, sealables)); + Address.functionCall( + DUAL_GOVERNANCE, abi.encodeWithSelector(IDualGovernance.resealSealables.selector, sealables) + ); bytes32 resealNonceHash = keccak256(abi.encode(sealables)); _resealNonces[resealNonceHash]++; diff --git a/test/scenario/emergency-committee.t.sol b/test/scenario/emergency-committee.t.sol index 671a1afb..e261dd47 100644 --- a/test/scenario/emergency-committee.t.sol +++ b/test/scenario/emergency-committee.t.sol @@ -15,20 +15,25 @@ contract EmergencyCommitteeTest is ScenarioTestBlueprint { function setUp() external { _selectFork(); - _deployDualGovernanceSetup( /* isEmergencyProtectionEnabled */ false); + _deployTarget(); + _deployDualGovernanceSetup( /* isEmergencyProtectionEnabled */ true); _depositStETH(_VETOER, 1 ether); } - function test_proposal_approval() external { + function test_emergency_committees_happy_path() external { uint256 quorum; uint256 support; bool isExecuted; address[] memory members; - ExternalCall[] memory proposalCalls = ExternalCallHelpers.create(address(0), new bytes(0)); + ExternalCall[] memory proposalCalls = _getTargetRegularStaffCalls(); uint256 proposalIdToExecute = _submitProposal(_dualGovernance, "Proposal for execution", proposalCalls); + _wait(_config.AFTER_SUBMIT_DELAY().plusSeconds(1)); + _assertCanSchedule(_dualGovernance, proposalIdToExecute, true); + _scheduleProposal(_dualGovernance, proposalIdToExecute); + // Emergency Activation members = _emergencyActivationCommittee.getMembers(); for (uint256 i = 0; i < _emergencyActivationCommittee.quorum() - 1; i++) { @@ -47,7 +52,7 @@ contract EmergencyCommitteeTest is ScenarioTestBlueprint { _emergencyActivationCommittee.executeActivateEmergencyMode(); (support, quorum, isExecuted) = _emergencyActivationCommittee.getActivateEmergencyModeState(); - assert(support < quorum); + assert(isExecuted == true); // Emergency Execute members = _emergencyExecutionCommittee.getMembers(); @@ -67,6 +72,6 @@ contract EmergencyCommitteeTest is ScenarioTestBlueprint { _emergencyExecutionCommittee.executeEmergencyExecute(proposalIdToExecute); (support, quorum, isExecuted) = _emergencyExecutionCommittee.getEmergencyExecuteState(proposalIdToExecute); - assert(support < quorum); + assert(isExecuted == true); } } diff --git a/test/scenario/reseal-committee.t.sol b/test/scenario/reseal-committee.t.sol new file mode 100644 index 00000000..d8aa13bd --- /dev/null +++ b/test/scenario/reseal-committee.t.sol @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import {ScenarioTestBlueprint, ExternalCall, percents} from "../utils/scenario-test-blueprint.sol"; +import {DualGovernance} from "../../contracts/DualGovernance.sol"; +import {ResealManager} from "../../contracts/ResealManager.sol"; +import {DAO_AGENT} from "../utils/mainnet-addresses.sol"; + +contract ResealCommitteeTest is ScenarioTestBlueprint { + address internal immutable _VETOER = makeAddr("VETOER"); + uint256 public constant PAUSE_INFINITELY = type(uint256).max; + + function setUp() external { + _selectFork(); + _deployTarget(); + _deployDualGovernanceSetup( /* isEmergencyProtectionEnabled */ true); + _depositStETH(_VETOER, 1 ether); + } + + function test_reseal_committees_happy_path() external { + uint256 quorum; + uint256 support; + bool isExecuted; + + address[] memory members; + + address[] memory sealables = new address[](1); + sealables[0] = address(_WITHDRAWAL_QUEUE); + + vm.prank(DAO_AGENT); + _WITHDRAWAL_QUEUE.grantRole(0x139c2898040ef16910dc9f44dc697df79363da767d8bc92f2e310312b816e46d, address(this)); + + // Reseal + members = _resealCommittee.getMembers(); + for (uint256 i = 0; i < _resealCommittee.quorum() - 1; i++) { + vm.prank(members[i]); + _resealCommittee.voteReseal(sealables, true); + (support, quorum, isExecuted) = _resealCommittee.getResealState(sealables); + assert(support < quorum); + assert(isExecuted == false); + } + + vm.prank(members[members.length - 1]); + _resealCommittee.voteReseal(sealables, true); + (support, quorum, isExecuted) = _resealCommittee.getResealState(sealables); + assert(support == quorum); + assert(isExecuted == false); + + _assertNormalState(); + + vm.expectRevert(abi.encodeWithSelector(DualGovernance.ResealIsNotAllowedInNormalState.selector)); + _resealCommittee.executeReseal(sealables); + + _lockStETH(_VETOER, percents(_config.FIRST_SEAL_RAGE_QUIT_SUPPORT())); + _lockStETH(_VETOER, 1 gwei); + _assertVetoSignalingState(); + + assertEq(_WITHDRAWAL_QUEUE.isPaused(), false); + vm.expectRevert(abi.encodeWithSelector(ResealManager.SealableWrongPauseState.selector)); + _resealCommittee.executeReseal(sealables); + + _WITHDRAWAL_QUEUE.pauseFor(3600 * 24 * 6); + assertEq(_WITHDRAWAL_QUEUE.isPaused(), true); + + _resealCommittee.executeReseal(sealables); + } +} diff --git a/test/utils/scenario-test-blueprint.sol b/test/utils/scenario-test-blueprint.sol index b8f2bc8d..6976f0ef 100644 --- a/test/utils/scenario-test-blueprint.sol +++ b/test/utils/scenario-test-blueprint.sol @@ -17,6 +17,7 @@ import {Executor} from "contracts/Executor.sol"; import {EmergencyActivationCommittee} from "contracts/committees/EmergencyActivationCommittee.sol"; import {EmergencyExecutionCommittee} from "contracts/committees/EmergencyExecutionCommittee.sol"; +import {ResealCommittee} from "contracts/committees/ResealCommittee.sol"; import {TiebreakerCore} from "contracts/committees/TiebreakerCore.sol"; import {TiebreakerSubCommittee} from "contracts/committees/TiebreakerSubCommittee.sol"; @@ -82,6 +83,7 @@ contract ScenarioTestBlueprint is Test { EmergencyActivationCommittee internal _emergencyActivationCommittee; EmergencyExecutionCommittee internal _emergencyExecutionCommittee; + ResealCommittee internal _resealCommittee; TiebreakerCore internal _tiebreakerCommittee; TiebreakerSubCommittee[] internal _tiebreakerSubCommittees; @@ -529,6 +531,7 @@ contract ScenarioTestBlueprint is Test { _deployDualGovernance(); _deployEmergencyActivationCommittee(); _deployEmergencyExecutionCommittee(); + _deployResealCommittee(); _deployTiebreaker(); _finishTimelockSetup(address(_dualGovernance), isEmergencyProtectionEnabled); } @@ -542,6 +545,7 @@ contract ScenarioTestBlueprint is Test { _deployTimelockedGovernance(); _deployEmergencyActivationCommittee(); _deployEmergencyExecutionCommittee(); + _deployResealCommittee(); _deployTiebreaker(); _finishTimelockSetup(address(_timelockedGovernance), isEmergencyProtectionEnabled); } @@ -627,6 +631,17 @@ contract ScenarioTestBlueprint is Test { new EmergencyExecutionCommittee(address(_adminExecutor), committeeMembers, quorum, address(_timelock)); } + function _deployResealCommittee() internal { + uint256 quorum = 3; + uint256 membersCount = 5; + address[] memory committeeMembers = new address[](membersCount); + for (uint256 i = 0; i < membersCount; ++i) { + committeeMembers[i] = makeAddr(string(abi.encode(0xFA + i * membersCount + 65))); + } + _resealCommittee = + new ResealCommittee(address(_adminExecutor), committeeMembers, quorum, address(_dualGovernance), 0); + } + function _finishTimelockSetup(address governance, bool isEmergencyProtectionEnabled) internal { if (isEmergencyProtectionEnabled) { _adminExecutor.execute( @@ -656,7 +671,6 @@ contract ScenarioTestBlueprint is Test { _WITHDRAWAL_QUEUE.grantRole( 0x2fc10cc8ae19568712f7a176fb4978616a610650813c9d05326c34abb62749c7, address(_resealManager) ); - if (governance == address(_dualGovernance)) { _adminExecutor.execute( address(_dualGovernance), @@ -665,6 +679,12 @@ contract ScenarioTestBlueprint is Test { _dualGovernance.setTiebreakerProtection, (address(_tiebreakerCommittee), address(_resealManager)) ) ); + + _adminExecutor.execute( + address(_dualGovernance), + 0, + abi.encodeCall(_dualGovernance.setReseal, (address(_resealManager), address(_resealCommittee))) + ); } _adminExecutor.execute(address(_timelock), 0, abi.encodeCall(_timelock.setGovernance, (governance))); _adminExecutor.transferOwnership(address(_timelock)); From 692383d1029d9217af7901605de2bd67cdda2c31 Mon Sep 17 00:00:00 2001 From: Roman Kolpakov Date: Mon, 5 Aug 2024 16:21:55 +0300 Subject: [PATCH 4/6] feat: make reseal function working with only one sealable --- contracts/DualGovernance.sol | 4 +-- contracts/ResealManager.sol | 14 ++++----- contracts/committees/ResealCommittee.sol | 36 ++++++++++++------------ contracts/interfaces/IResealManager.sol | 2 +- docs/specification.md | 6 ++-- test/scenario/reseal-committee.t.sol | 17 ++++++----- 6 files changed, 38 insertions(+), 41 deletions(-) diff --git a/contracts/DualGovernance.sol b/contracts/DualGovernance.sol index 25df297c..3d1d6d57 100644 --- a/contracts/DualGovernance.sol +++ b/contracts/DualGovernance.sol @@ -172,14 +172,14 @@ contract DualGovernance is IGovernance, ConfigurationProvider { // Reseal executor // --- - function resealSealables(address[] memory sealables) external { + function resealSealable(address sealable) external { if (msg.sender != _resealCommittee) { revert NotResealCommitttee(msg.sender); } if (_stateMachine.getCurrentState() == State.Normal) { revert ResealIsNotAllowedInNormalState(); } - _resealManager.reseal(sealables); + _resealManager.reseal(sealable); } function setReseal(address resealManager, address resealCommittee) external { diff --git a/contracts/ResealManager.sol b/contracts/ResealManager.sol index 1c5593ff..82bc8271 100644 --- a/contracts/ResealManager.sol +++ b/contracts/ResealManager.sol @@ -20,15 +20,13 @@ contract ResealManager { EMERGENCY_PROTECTED_TIMELOCK = emergencyProtectedTimelock; } - function reseal(address[] memory sealables) public onlyGovernance { - for (uint256 i = 0; i < sealables.length; ++i) { - uint256 sealableResumeSinceTimestamp = ISealable(sealables[i]).getResumeSinceTimestamp(); - if (sealableResumeSinceTimestamp < block.timestamp || sealableResumeSinceTimestamp == PAUSE_INFINITELY) { - revert SealableWrongPauseState(); - } - Address.functionCall(sealables[i], abi.encodeWithSelector(ISealable.resume.selector)); - Address.functionCall(sealables[i], abi.encodeWithSelector(ISealable.pauseFor.selector, PAUSE_INFINITELY)); + function reseal(address sealable) public onlyGovernance { + uint256 sealableResumeSinceTimestamp = ISealable(sealable).getResumeSinceTimestamp(); + if (sealableResumeSinceTimestamp < block.timestamp || sealableResumeSinceTimestamp == PAUSE_INFINITELY) { + revert SealableWrongPauseState(); } + Address.functionCall(sealable, abi.encodeWithSelector(ISealable.resume.selector)); + Address.functionCall(sealable, abi.encodeWithSelector(ISealable.pauseFor.selector, PAUSE_INFINITELY)); } function resume(address sealable) public onlyGovernance { diff --git a/contracts/committees/ResealCommittee.sol b/contracts/committees/ResealCommittee.sol index 3e98cbf4..9e94402e 100644 --- a/contracts/committees/ResealCommittee.sol +++ b/contracts/committees/ResealCommittee.sol @@ -6,7 +6,7 @@ import {HashConsensus} from "./HashConsensus.sol"; import {ProposalsList} from "./ProposalsList.sol"; interface IDualGovernance { - function resealSealables(address[] memory sealables) external; + function resealSealable(address sealables) external; } /// @title Reseal Committee Contract @@ -28,53 +28,53 @@ contract ResealCommittee is HashConsensus, ProposalsList { } /// @notice Votes on a reseal proposal - /// @dev Allows committee members to vote on resealing a set of addresses - /// @param sealables The addresses to reseal + /// @dev Allows committee members to vote on resealing a sealed address + /// @param sealable The address to reseal /// @param support Indicates whether the member supports the proposal - function voteReseal(address[] memory sealables, bool support) public onlyMember { - (bytes memory proposalData, bytes32 key) = _encodeResealProposal(sealables); + function voteReseal(address sealable, bool support) public onlyMember { + (bytes memory proposalData, bytes32 key) = _encodeResealProposal(sealable); _vote(key, support); _pushProposal(key, 0, proposalData); } /// @notice Gets the current state of a reseal proposal - /// @dev Retrieves the state of the reseal proposal for a set of addresses - /// @param sealables The addresses for the reseal proposal + /// @dev Retrieves the state of the reseal proposal for a sealed address + /// @param sealable The addresses for the reseal proposal /// @return support The number of votes in support of the proposal /// @return execuitionQuorum The required number of votes for execution /// @return isExecuted Whether the proposal has been executed - function getResealState(address[] memory sealables) + function getResealState(address sealable) public view returns (uint256 support, uint256 execuitionQuorum, bool isExecuted) { - (, bytes32 key) = _encodeResealProposal(sealables); + (, bytes32 key) = _encodeResealProposal(sealable); return _getHashState(key); } /// @notice Executes an approved reseal proposal /// @dev Executes the reseal proposal by calling the reseal function on the Dual Governance contract - /// @param sealables The addresses to reseal - function executeReseal(address[] memory sealables) external { - (, bytes32 key) = _encodeResealProposal(sealables); + /// @param sealable The address to reseal + function executeReseal(address sealable) external { + (, bytes32 key) = _encodeResealProposal(sealable); _markUsed(key); Address.functionCall( - DUAL_GOVERNANCE, abi.encodeWithSelector(IDualGovernance.resealSealables.selector, sealables) + DUAL_GOVERNANCE, abi.encodeWithSelector(IDualGovernance.resealSealable.selector, sealable) ); - bytes32 resealNonceHash = keccak256(abi.encode(sealables)); + bytes32 resealNonceHash = keccak256(abi.encode(sealable)); _resealNonces[resealNonceHash]++; } /// @notice Encodes a reseal proposal /// @dev Internal function to encode the proposal data and generate the proposal key - /// @param sealables The addresses to reseal + /// @param sealable The address to reseal /// @return data The encoded proposal data /// @return key The generated proposal key - function _encodeResealProposal(address[] memory sealables) internal view returns (bytes memory data, bytes32 key) { - bytes32 resealNonceHash = keccak256(abi.encode(sealables)); - data = abi.encode(sealables, _resealNonces[resealNonceHash]); + function _encodeResealProposal(address sealable) internal view returns (bytes memory data, bytes32 key) { + bytes32 resealNonceHash = keccak256(abi.encode(sealable)); + data = abi.encode(sealable, _resealNonces[resealNonceHash]); key = keccak256(data); } } diff --git a/contracts/interfaces/IResealManager.sol b/contracts/interfaces/IResealManager.sol index bea725ef..844bd8e1 100644 --- a/contracts/interfaces/IResealManager.sol +++ b/contracts/interfaces/IResealManager.sol @@ -3,5 +3,5 @@ pragma solidity 0.8.26; interface IResealManager { function resume(address sealable) external; - function reseal(address[] memory sealables) external; + function reseal(address sealable) external; } diff --git a/docs/specification.md b/docs/specification.md index 2b9fb419..4cdfbb52 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -417,15 +417,15 @@ Initializes the contract with the address of the EmergencyProtectedTimelock cont ### Function ResealManager.reseal ```solidity -function reseal(address[] memory sealables) public onlyGovernance +function reseal(address sealable) public onlyGovernance ``` -Extends the pause of the specified `sealables` contracts. This function can be called by the governance address defined in the `EmergencyProtectedTimelock`. +Extends the pause of the specified `sealable` contract. This function can be called by the governance address defined in the `EmergencyProtectedTimelock`. #### Preconditions - The `ResealManager` MUST have `PAUSE_ROLE` and `RESUME_ROLE` for the target contracts. -- The target contracts MUST be paused until a future timestamp and not indefinitely. +- The target contract MUST be paused until a future timestamp and not indefinitely. - The function MUST be called by the governance address defined in `EmergencyProtectedTimelock`. #### Errors diff --git a/test/scenario/reseal-committee.t.sol b/test/scenario/reseal-committee.t.sol index d8aa13bd..2b2dfcf9 100644 --- a/test/scenario/reseal-committee.t.sol +++ b/test/scenario/reseal-committee.t.sol @@ -24,8 +24,7 @@ contract ResealCommitteeTest is ScenarioTestBlueprint { address[] memory members; - address[] memory sealables = new address[](1); - sealables[0] = address(_WITHDRAWAL_QUEUE); + address sealable = address(_WITHDRAWAL_QUEUE); vm.prank(DAO_AGENT); _WITHDRAWAL_QUEUE.grantRole(0x139c2898040ef16910dc9f44dc697df79363da767d8bc92f2e310312b816e46d, address(this)); @@ -34,22 +33,22 @@ contract ResealCommitteeTest is ScenarioTestBlueprint { members = _resealCommittee.getMembers(); for (uint256 i = 0; i < _resealCommittee.quorum() - 1; i++) { vm.prank(members[i]); - _resealCommittee.voteReseal(sealables, true); - (support, quorum, isExecuted) = _resealCommittee.getResealState(sealables); + _resealCommittee.voteReseal(sealable, true); + (support, quorum, isExecuted) = _resealCommittee.getResealState(sealable); assert(support < quorum); assert(isExecuted == false); } vm.prank(members[members.length - 1]); - _resealCommittee.voteReseal(sealables, true); - (support, quorum, isExecuted) = _resealCommittee.getResealState(sealables); + _resealCommittee.voteReseal(sealable, true); + (support, quorum, isExecuted) = _resealCommittee.getResealState(sealable); assert(support == quorum); assert(isExecuted == false); _assertNormalState(); vm.expectRevert(abi.encodeWithSelector(DualGovernance.ResealIsNotAllowedInNormalState.selector)); - _resealCommittee.executeReseal(sealables); + _resealCommittee.executeReseal(sealable); _lockStETH(_VETOER, percents(_config.FIRST_SEAL_RAGE_QUIT_SUPPORT())); _lockStETH(_VETOER, 1 gwei); @@ -57,11 +56,11 @@ contract ResealCommitteeTest is ScenarioTestBlueprint { assertEq(_WITHDRAWAL_QUEUE.isPaused(), false); vm.expectRevert(abi.encodeWithSelector(ResealManager.SealableWrongPauseState.selector)); - _resealCommittee.executeReseal(sealables); + _resealCommittee.executeReseal(sealable); _WITHDRAWAL_QUEUE.pauseFor(3600 * 24 * 6); assertEq(_WITHDRAWAL_QUEUE.isPaused(), true); - _resealCommittee.executeReseal(sealables); + _resealCommittee.executeReseal(sealable); } } From d7a4d8d6e319021b44cf20f5ba812028235a005c Mon Sep 17 00:00:00 2001 From: Roman Kolpakov Date: Tue, 6 Aug 2024 14:03:15 +0300 Subject: [PATCH 5/6] feat: add natspec for reseal manager --- contracts/ResealManager.sol | 38 +++++++++++++++++++++++------- contracts/interfaces/ITimelock.sol | 2 ++ docs/specification.md | 2 +- test/unit/mocks/TimelockMock.sol | 4 ++++ 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/contracts/ResealManager.sol b/contracts/ResealManager.sol index 82bc8271..9b3b43b6 100644 --- a/contracts/ResealManager.sol +++ b/contracts/ResealManager.sol @@ -2,12 +2,12 @@ pragma solidity 0.8.26; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; -import {ISealable} from "./interfaces/ISealable.sol"; -interface IEmergencyProtectedTimelock { - function getGovernance() external view returns (address); -} +import {ITimelock} from "./interfaces/ITimelock.sol"; +import {ISealable} from "./interfaces/ISealable.sol"; +/// @title ResealManager +/// @dev Allows to extend pause of temporarily paused contracts to permanent pause or resume it. contract ResealManager { error SealableWrongPauseState(); error SenderIsNotGovernance(); @@ -16,11 +16,22 @@ contract ResealManager { uint256 public constant PAUSE_INFINITELY = type(uint256).max; address public immutable EMERGENCY_PROTECTED_TIMELOCK; + /// @notice Initializes the ResealManager contract. + /// @param emergencyProtectedTimelock The address of the EmergencyProtectedTimelock contract. constructor(address emergencyProtectedTimelock) { EMERGENCY_PROTECTED_TIMELOCK = emergencyProtectedTimelock; } - function reseal(address sealable) public onlyGovernance { + /// @notice Extends the pause of the specified sealable contract. + /// @dev Works only if conditions are met: + /// - ResealManager has PAUSE_ROLE for target contract; + /// - Contract are paused until timestamp after current timestamp and not for infinite time; + /// - The DAO governance is blocked by DualGovernance; + /// - Function is called by the governance contract. + /// @param sealable The address of the sealable contract. + function reseal(address sealable) public { + _checkSenderIsGovernance(); + uint256 sealableResumeSinceTimestamp = ISealable(sealable).getResumeSinceTimestamp(); if (sealableResumeSinceTimestamp < block.timestamp || sealableResumeSinceTimestamp == PAUSE_INFINITELY) { revert SealableWrongPauseState(); @@ -29,7 +40,15 @@ contract ResealManager { Address.functionCall(sealable, abi.encodeWithSelector(ISealable.pauseFor.selector, PAUSE_INFINITELY)); } - function resume(address sealable) public onlyGovernance { + /// @notice Resumes the specified sealable contract if it is scheduled to resume in the future. + /// @dev Works only if conditions are met: + /// - ResealManager has RESUME_ROLE for target contract; + /// - Contract are paused until timestamp after current timestamp; + /// - Function is called by the governance contract. + /// @param sealable The address of the sealable contract. + function resume(address sealable) public { + _checkSenderIsGovernance(); + uint256 sealableResumeSinceTimestamp = ISealable(sealable).getResumeSinceTimestamp(); if (sealableResumeSinceTimestamp < block.timestamp) { revert SealableWrongPauseState(); @@ -37,11 +56,12 @@ contract ResealManager { Address.functionCall(sealable, abi.encodeWithSelector(ISealable.resume.selector)); } - modifier onlyGovernance() { - address governance = IEmergencyProtectedTimelock(EMERGENCY_PROTECTED_TIMELOCK).getGovernance(); + /// @notice Ensures that the function can only be called by the governance address. + /// @dev Reverts if the sender is not the governance address. + function _checkSenderIsGovernance() internal view { + address governance = ITimelock(EMERGENCY_PROTECTED_TIMELOCK).getGovernance(); if (msg.sender != governance) { revert SenderIsNotGovernance(); } - _; } } diff --git a/contracts/interfaces/ITimelock.sol b/contracts/interfaces/ITimelock.sol index f3128ec6..1e30eb45 100644 --- a/contracts/interfaces/ITimelock.sol +++ b/contracts/interfaces/ITimelock.sol @@ -37,4 +37,6 @@ interface ITimelock { external view returns (uint256 id, ProposalStatus status, address executor, Timestamp submittedAt, Timestamp scheduledAt); + + function getGovernance() external view returns (address); } diff --git a/docs/specification.md b/docs/specification.md index 4cdfbb52..eb527628 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -424,7 +424,7 @@ Extends the pause of the specified `sealable` contract. This function can be cal #### Preconditions -- The `ResealManager` MUST have `PAUSE_ROLE` and `RESUME_ROLE` for the target contracts. +- The `ResealManager` MUST have `PAUSE_ROLE` and `RESUME_ROLE` for the target contract. - The target contract MUST be paused until a future timestamp and not indefinitely. - The function MUST be called by the governance address defined in `EmergencyProtectedTimelock`. diff --git a/test/unit/mocks/TimelockMock.sol b/test/unit/mocks/TimelockMock.sol index b9bcbc43..5c0d4b50 100644 --- a/test/unit/mocks/TimelockMock.sol +++ b/test/unit/mocks/TimelockMock.sol @@ -71,6 +71,10 @@ contract TimelockMock is ITimelock { revert("Not Implemented"); } + function getGovernance() external view returns (address) { + revert("Not Implemented"); + } + function getProposalInfo(uint256 proposalId) external view From df71747d04dfae8f8e3c500563b65333190e6438 Mon Sep 17 00:00:00 2001 From: Roman Kolpakov Date: Tue, 6 Aug 2024 14:32:48 +0300 Subject: [PATCH 6/6] feat: add unit tests for reseal manager --- contracts/interfaces/ITimelock.sol | 1 + test/{unit => }/mocks/TimelockMock.sol | 8 +- test/unit/ResealManager.t.sol | 106 ++++++++++++++++++ ...ernance.sol => TimelockedGovernance.t.sol} | 2 +- 4 files changed, 115 insertions(+), 2 deletions(-) rename test/{unit => }/mocks/TimelockMock.sol (94%) create mode 100644 test/unit/ResealManager.t.sol rename test/unit/{TimelockedGovernance.sol => TimelockedGovernance.t.sol} (98%) diff --git a/contracts/interfaces/ITimelock.sol b/contracts/interfaces/ITimelock.sol index 1e30eb45..90d8bca6 100644 --- a/contracts/interfaces/ITimelock.sol +++ b/contracts/interfaces/ITimelock.sol @@ -39,4 +39,5 @@ interface ITimelock { returns (uint256 id, ProposalStatus status, address executor, Timestamp submittedAt, Timestamp scheduledAt); function getGovernance() external view returns (address); + function setGovernance(address governance) external; } diff --git a/test/unit/mocks/TimelockMock.sol b/test/mocks/TimelockMock.sol similarity index 94% rename from test/unit/mocks/TimelockMock.sol rename to test/mocks/TimelockMock.sol index 5c0d4b50..eb28e564 100644 --- a/test/unit/mocks/TimelockMock.sol +++ b/test/mocks/TimelockMock.sol @@ -16,6 +16,8 @@ contract TimelockMock is ITimelock { uint256 public lastCancelledProposalId; + address internal governance; + function submit(address, ExternalCall[] calldata) external returns (uint256 newProposalId) { newProposalId = submittedProposals.length + OFFSET; submittedProposals.push(newProposalId); @@ -71,8 +73,12 @@ contract TimelockMock is ITimelock { revert("Not Implemented"); } + function setGovernance(address newGovernance) external { + governance = newGovernance; + } + function getGovernance() external view returns (address) { - revert("Not Implemented"); + return governance; } function getProposalInfo(uint256 proposalId) diff --git a/test/unit/ResealManager.t.sol b/test/unit/ResealManager.t.sol new file mode 100644 index 00000000..029fcbca --- /dev/null +++ b/test/unit/ResealManager.t.sol @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import {ResealManager} from "contracts/ResealManager.sol"; +import {ITimelock} from "contracts/interfaces/ITimelock.sol"; +import {ISealable} from "contracts/interfaces/ISealable.sol"; +import {Durations} from "contracts/types/Duration.sol"; + +import {UnitTest} from "test/utils/unit-test.sol"; + +contract ResealManagerUnitTests is UnitTest { + ResealManager internal resealManager; + + address timelock = makeAddr("timelock"); + address sealable = makeAddr("sealable"); + address private governance = makeAddr("governance"); + + function setUp() external { + vm.mockCall(timelock, abi.encodeWithSelector(ITimelock.getGovernance.selector), abi.encode(governance)); + + resealManager = new ResealManager(timelock); + } + + function test_resealSuccess() public { + uint256 futureTimestamp = block.timestamp + 1000; + vm.mockCall( + sealable, abi.encodeWithSelector(ISealable.getResumeSinceTimestamp.selector), abi.encode(futureTimestamp) + ); + + vm.expectCall(sealable, abi.encodeWithSelector(ISealable.resume.selector)); + vm.expectCall(sealable, abi.encodeWithSelector(ISealable.pauseFor.selector, type(uint256).max)); + + vm.prank(governance); + resealManager.reseal(sealable); + } + + function test_resealFailsForPastTimestamp() public { + uint256 pastTimestamp = block.timestamp; + vm.mockCall( + sealable, abi.encodeWithSelector(ISealable.getResumeSinceTimestamp.selector), abi.encode(pastTimestamp) + ); + + _wait(Durations.from(1)); + + vm.prank(governance); + vm.expectRevert(ResealManager.SealableWrongPauseState.selector); + resealManager.reseal(sealable); + } + + function test_resealFailsForInfinitePause() public { + vm.mockCall( + sealable, abi.encodeWithSelector(ISealable.getResumeSinceTimestamp.selector), abi.encode(type(uint256).max) + ); + + vm.prank(governance); + vm.expectRevert(ResealManager.SealableWrongPauseState.selector); + resealManager.reseal(sealable); + } + + function test_resumeSuccess() public { + uint256 futureTimestamp = block.timestamp + 1000; + vm.mockCall( + sealable, abi.encodeWithSelector(ISealable.getResumeSinceTimestamp.selector), abi.encode(futureTimestamp) + ); + + vm.expectCall(sealable, abi.encodeWithSelector(ISealable.resume.selector)); + + vm.prank(governance); + resealManager.resume(sealable); + } + + function test_resumeFailsForPastTimestamp() public { + uint256 pastTimestamp = block.timestamp; + vm.mockCall( + sealable, abi.encodeWithSelector(ISealable.getResumeSinceTimestamp.selector), abi.encode(pastTimestamp) + ); + + _wait(Durations.from(1)); + + vm.prank(governance); + vm.expectRevert(ResealManager.SealableWrongPauseState.selector); + resealManager.resume(sealable); + } + + function test_revertWhenSenderIsNotGovernanceOnReseal() public { + uint256 futureTimestamp = block.timestamp + 1000; + vm.mockCall( + sealable, abi.encodeWithSelector(ISealable.getResumeSinceTimestamp.selector), abi.encode(futureTimestamp) + ); + + vm.prank(address(0x123)); + vm.expectRevert(ResealManager.SenderIsNotGovernance.selector); + resealManager.reseal(sealable); + } + + function test_revertWhenSenderIsNotGovernanceOnResume() public { + uint256 futureTimestamp = block.timestamp + 1000; + vm.mockCall( + sealable, abi.encodeWithSelector(ISealable.getResumeSinceTimestamp.selector), abi.encode(futureTimestamp) + ); + + vm.prank(address(0x123)); + vm.expectRevert(ResealManager.SenderIsNotGovernance.selector); + resealManager.resume(sealable); + } +} diff --git a/test/unit/TimelockedGovernance.sol b/test/unit/TimelockedGovernance.t.sol similarity index 98% rename from test/unit/TimelockedGovernance.sol rename to test/unit/TimelockedGovernance.t.sol index 33de1225..e343e8da 100644 --- a/test/unit/TimelockedGovernance.sol +++ b/test/unit/TimelockedGovernance.t.sol @@ -10,7 +10,7 @@ import {IConfiguration, Configuration} from "contracts/Configuration.sol"; import {UnitTest} from "test/utils/unit-test.sol"; import {TargetMock} from "test/utils/utils.sol"; -import {TimelockMock} from "./mocks/TimelockMock.sol"; +import {TimelockMock} from "../mocks/TimelockMock.sol"; contract SingleGovernanceUnitTests is UnitTest { TimelockMock private _timelock;