From 695130f8961482c7f7cd6ea07a9eb73c2accc261 Mon Sep 17 00:00:00 2001 From: Roman Kolpakov Date: Mon, 14 Oct 2024 17:17:27 +0300 Subject: [PATCH 1/3] change reseal manager variable in dual governance --- contracts/DualGovernance.sol | 28 +++++++--- test/unit/DualGovernance.t.sol | 98 ++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 7 deletions(-) diff --git a/contracts/DualGovernance.sol b/contracts/DualGovernance.sol index 66175aaf..a7601bbf 100644 --- a/contracts/DualGovernance.sol +++ b/contracts/DualGovernance.sol @@ -43,6 +43,7 @@ contract DualGovernance is IDualGovernance { error ProposalSubmissionBlocked(); error ProposalSchedulingBlocked(uint256 proposalId); error ResealIsNotAllowedInNormalState(); + error InvalidResealManager(address resealManager); // --- // Events @@ -52,6 +53,7 @@ contract DualGovernance is IDualGovernance { event CancelAllPendingProposalsExecuted(); event EscrowMasterCopyDeployed(IEscrow escrowMasterCopy); event ResealCommitteeSet(address resealCommittee); + event ResealManagerSet(address resealManager); // --- // Sanity Check Parameters & Immutables @@ -110,9 +112,6 @@ contract DualGovernance is IDualGovernance { /// @notice The address of the Timelock contract. ITimelock public immutable TIMELOCK; - /// @notice The address of the Reseal Manager. - IResealManager public immutable RESEAL_MANAGER; - /// @notice The address of the Escrow contract used as the implementation for the Signalling and Rage Quit /// instances of the Escrows managed by the DualGovernance contract. IEscrow public immutable ESCROW_MASTER_COPY; @@ -138,13 +137,15 @@ contract DualGovernance is IDualGovernance { /// period of time when the Dual Governance proposal adoption is blocked. address internal _resealCommittee; + /// @dev The address of the Reseal Manager. + IResealManager internal _resealManager; + // --- // Constructor // --- constructor(ExternalDependencies memory dependencies, SanityCheckParams memory sanityCheckParams) { TIMELOCK = dependencies.timelock; - RESEAL_MANAGER = dependencies.resealManager; MIN_TIEBREAKER_ACTIVATION_TIMEOUT = sanityCheckParams.minTiebreakerActivationTimeout; MAX_TIEBREAKER_ACTIVATION_TIMEOUT = sanityCheckParams.maxTiebreakerActivationTimeout; @@ -157,10 +158,12 @@ contract DualGovernance is IDualGovernance { withdrawalQueue: dependencies.withdrawalQueue, minWithdrawalsBatchSize: sanityCheckParams.minWithdrawalsBatchSize }); - emit EscrowMasterCopyDeployed(ESCROW_MASTER_COPY); _stateMachine.initialize(dependencies.configProvider, ESCROW_MASTER_COPY); + + _resealManager = dependencies.resealManager; + emit ResealManagerSet(address(_resealManager)); } // --- @@ -444,7 +447,7 @@ contract DualGovernance is IDualGovernance { _tiebreaker.checkCallerIsTiebreakerCommittee(); _stateMachine.activateNextState(ESCROW_MASTER_COPY); _tiebreaker.checkTie(_stateMachine.getPersistedState(), _stateMachine.normalOrVetoCooldownExitedAt); - RESEAL_MANAGER.resume(sealable); + _resealManager.resume(sealable); } /// @notice Allows the tiebreaker committee to schedule for execution a submitted proposal when @@ -484,7 +487,7 @@ contract DualGovernance is IDualGovernance { if (_stateMachine.getPersistedState() == State.Normal) { revert ResealIsNotAllowedInNormalState(); } - RESEAL_MANAGER.reseal(sealable); + _resealManager.reseal(sealable); } /// @notice Sets the address of the reseal committee. @@ -496,6 +499,17 @@ contract DualGovernance is IDualGovernance { emit ResealCommitteeSet(resealCommittee); } + /// @notice Sets the address of the Reseal Manager. + /// @param resealManager The address of the new Reseal Manager. + function setResealManager(address resealManager) external { + _checkCallerIsAdminExecutor(); + if (resealManager == address(0) || resealManager == address(_resealManager)) { + revert InvalidResealManager(resealManager); + } + _resealManager = IResealManager(resealManager); + emit ResealManagerSet(resealManager); + } + // --- // Internal methods // --- diff --git a/test/unit/DualGovernance.t.sol b/test/unit/DualGovernance.t.sol index 36a6601c..ad67661a 100644 --- a/test/unit/DualGovernance.t.sol +++ b/test/unit/DualGovernance.t.sol @@ -26,6 +26,7 @@ import {IWithdrawalQueue} from "contracts/interfaces/IWithdrawalQueue.sol"; import {ITimelock} from "contracts/interfaces/ITimelock.sol"; import {ISealable} from "contracts/interfaces/ISealable.sol"; import {ITiebreaker} from "contracts/interfaces/ITiebreaker.sol"; +import {IEscrow} from "contracts/interfaces/IEscrow.sol"; import {UnitTest} from "test/utils/unit-test.sol"; import {StETHMock} from "test/mocks/StETHMock.sol"; @@ -99,6 +100,50 @@ contract DualGovernanceUnitTests is UnitTest { _STETH_MOCK.approve(address(_escrow), 10 ether); } + // --- + // constructor() + // --- + + function test_constructor_HappyPath() external { + address testDeployerAddress = address(this); + uint256 testDeployerNonce = vm.getNonce(testDeployerAddress); + address predictedDualGovernanceAddress = computeAddress(testDeployerAddress, testDeployerNonce); + + address predictedEscrowCopyAddress = computeAddress(predictedDualGovernanceAddress, 1); + + vm.expectEmit(); + emit DualGovernance.EscrowMasterCopyDeployed(IEscrow(predictedEscrowCopyAddress)); + vm.expectEmit(); + emit DualGovernance.ResealManagerSet(address(_RESEAL_MANAGER_STUB)); + + Duration minTiebreakerActivationTimeout = Durations.from(30 days); + Duration maxTiebreakerActivationTimeout = Durations.from(180 days); + uint256 maxSealableWithdrawalBlockersCount = 128; + + DualGovernance dualGovernanceLocal = new DualGovernance({ + dependencies: DualGovernance.ExternalDependencies({ + stETH: _STETH_MOCK, + wstETH: _WSTETH_STUB, + withdrawalQueue: _WITHDRAWAL_QUEUE_MOCK, + timelock: _timelock, + resealManager: _RESEAL_MANAGER_STUB, + configProvider: _configProvider + }), + sanityCheckParams: DualGovernance.SanityCheckParams({ + minWithdrawalsBatchSize: 4, + minTiebreakerActivationTimeout: minTiebreakerActivationTimeout, + maxTiebreakerActivationTimeout: maxTiebreakerActivationTimeout, + maxSealableWithdrawalBlockersCount: maxSealableWithdrawalBlockersCount + }) + }); + + assertEq(address(dualGovernanceLocal.TIMELOCK()), address(_timelock)); + assertEq(dualGovernanceLocal.MIN_TIEBREAKER_ACTIVATION_TIMEOUT(), minTiebreakerActivationTimeout); + assertEq(dualGovernanceLocal.MAX_TIEBREAKER_ACTIVATION_TIMEOUT(), maxTiebreakerActivationTimeout); + assertEq(dualGovernanceLocal.MAX_SEALABLE_WITHDRAWAL_BLOCKERS_COUNT(), maxSealableWithdrawalBlockersCount); + assertEq(address(dualGovernanceLocal.ESCROW_MASTER_COPY()), predictedEscrowCopyAddress); + } + // --- // submitProposal() // --- @@ -2062,6 +2107,39 @@ contract DualGovernanceUnitTests is UnitTest { _dualGovernance.setResealCommittee(makeAddr("NEW_RESEAL_COMMITTEE")); } + // --- + // setResealManager() + // --- + + function testFuzz_setResealManager_HappyPath(address newResealManager) external { + vm.assume(newResealManager != address(0) && newResealManager != address(_RESEAL_MANAGER_STUB)); + + vm.expectEmit(); + emit DualGovernance.ResealManagerSet(newResealManager); + + _executor.execute( + address(_dualGovernance), + 0, + abi.encodeWithSelector(DualGovernance.setResealManager.selector, newResealManager) + ); + } + + function test_setResealManager_RevertOn_InvalidAddress() external { + vm.expectRevert(abi.encodeWithSelector(DualGovernance.InvalidResealManager.selector, address(0))); + _executor.execute( + address(_dualGovernance), 0, abi.encodeWithSelector(DualGovernance.setResealManager.selector, address(0)) + ); + + vm.expectRevert( + abi.encodeWithSelector(DualGovernance.InvalidResealManager.selector, address(_RESEAL_MANAGER_STUB)) + ); + _executor.execute( + address(_dualGovernance), + 0, + abi.encodeWithSelector(DualGovernance.setResealManager.selector, address(_RESEAL_MANAGER_STUB)) + ); + } + // --- // Helper methods // --- @@ -2075,4 +2153,24 @@ contract DualGovernanceUnitTests is UnitTest { calls = new ExternalCall[](1); calls[0] = ExternalCall({target: address(0x123), value: 0, payload: abi.encodeWithSignature("someFunction()")}); } + + function computeAddress(address deployer, uint256 nonce) public pure returns (address) { + bytes memory data; + + if (nonce == 0x00) { + data = abi.encodePacked(hex"94", deployer, hex"80"); + } else if (nonce <= 0x7f) { + data = abi.encodePacked(hex"d6", hex"94", deployer, uint8(nonce)); + } else if (nonce <= 0xff) { + data = abi.encodePacked(hex"d7", hex"94", deployer, hex"81", uint8(nonce)); + } else if (nonce <= 0xffff) { + data = abi.encodePacked(hex"d8", hex"94", deployer, hex"82", uint16(nonce)); + } else if (nonce <= 0xffffff) { + data = abi.encodePacked(hex"d9", hex"94", deployer, hex"83", uint24(nonce)); + } else { + data = abi.encodePacked(hex"da", hex"94", deployer, hex"84", uint32(nonce)); + } + + return address(uint160(uint256(keccak256(data)))); + } } From 7b7369789bae32d2214d664a9387e669625bd30f Mon Sep 17 00:00:00 2001 From: Roman Kolpakov Date: Mon, 14 Oct 2024 17:17:42 +0300 Subject: [PATCH 2/3] change admin executor variable in timelock --- contracts/EmergencyProtectedTimelock.sol | 45 ++++++++++++++++------ test/unit/EmergencyProtectedTimelock.t.sol | 25 ++++++++++++ 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/contracts/EmergencyProtectedTimelock.sol b/contracts/EmergencyProtectedTimelock.sol index 645eba5d..bf8d85b6 100644 --- a/contracts/EmergencyProtectedTimelock.sol +++ b/contracts/EmergencyProtectedTimelock.sol @@ -21,11 +21,18 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { using ExecutableProposals for ExecutableProposals.Context; using EmergencyProtection for EmergencyProtection.Context; + // --- + // Events + // --- + + event AdminExecutorSet(address newAdminExecutor); + // --- // Errors // --- error CallerIsNotAdminExecutor(address value); + error InvalidAdminExecutor(address adminExecutor); // --- // Sanity Check Parameters & Immutables @@ -55,13 +62,6 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @notice The upper bound for the time the emergency protection mechanism can be activated. Duration public immutable MAX_EMERGENCY_PROTECTION_DURATION; - // --- - // Admin Executor Immutables - // --- - - /// @dev The address of the admin executor, authorized to manage the EmergencyProtectedTimelock instance. - address private immutable _ADMIN_EXECUTOR; - // --- // Aspects // --- @@ -75,17 +75,25 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @dev The functionality for managing the emergency protection mechanism. EmergencyProtection.Context internal _emergencyProtection; + // --- + // Admin Executor + // --- + + /// @dev The address of the admin executor, authorized to manage the EmergencyProtectedTimelock instance. + address private _adminExecutor; + // --- // Constructor // --- constructor(SanityCheckParams memory sanityCheckParams, address adminExecutor) { - _ADMIN_EXECUTOR = adminExecutor; - MAX_AFTER_SUBMIT_DELAY = sanityCheckParams.maxAfterSubmitDelay; MAX_AFTER_SCHEDULE_DELAY = sanityCheckParams.maxAfterScheduleDelay; MAX_EMERGENCY_MODE_DURATION = sanityCheckParams.maxEmergencyModeDuration; MAX_EMERGENCY_PROTECTION_DURATION = sanityCheckParams.maxEmergencyProtectionDuration; + + _adminExecutor = adminExecutor; + emit AdminExecutorSet(adminExecutor); } // --- @@ -279,7 +287,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @notice Returns the address of the admin executor. /// @return adminExecutor The address of the admin executor. function getAdminExecutor() external view returns (address) { - return _ADMIN_EXECUTOR; + return _adminExecutor; } /// @notice Returns the configured delay duration required before a submitted proposal can be scheduled. @@ -354,12 +362,27 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { return _proposals.canSchedule(proposalId, _timelockState.getAfterSubmitDelay()); } + // --- + // Admin Executor Methods + // --- + + /// @notice Sets the address of the admin executor. + /// @param newAdminExecutor The address of the new admin executor. + function setAdminExecutor(address newAdminExecutor) external { + _checkCallerIsAdminExecutor(); + if (newAdminExecutor == address(0) || newAdminExecutor == _adminExecutor) { + revert InvalidAdminExecutor(newAdminExecutor); + } + _adminExecutor = newAdminExecutor; + emit AdminExecutorSet(newAdminExecutor); + } + // --- // Internal Methods // --- function _checkCallerIsAdminExecutor() internal view { - if (msg.sender != _ADMIN_EXECUTOR) { + if (msg.sender != _adminExecutor) { revert CallerIsNotAdminExecutor(msg.sender); } } diff --git a/test/unit/EmergencyProtectedTimelock.t.sol b/test/unit/EmergencyProtectedTimelock.t.sol index d5827462..0c4387b5 100644 --- a/test/unit/EmergencyProtectedTimelock.t.sol +++ b/test/unit/EmergencyProtectedTimelock.t.sol @@ -60,6 +60,8 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { EmergencyProtectedTimelock.SanityCheckParams memory sanityCheckParams, address adminExecutor ) external { + vm.expectEmit(); + emit EmergencyProtectedTimelock.AdminExecutorSet(adminExecutor); EmergencyProtectedTimelock timelock = new EmergencyProtectedTimelock(sanityCheckParams, adminExecutor); assertEq(timelock.getAdminExecutor(), adminExecutor); @@ -990,6 +992,29 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { assertEq(timelock.getAdminExecutor(), executor); } + function testFuzz_setAdminExecutor_HappyPath(address adminExecutor) external { + vm.assume(adminExecutor != _adminExecutor && adminExecutor != address(0)); + + vm.expectEmit(); + emit EmergencyProtectedTimelock.AdminExecutorSet(adminExecutor); + vm.prank(_adminExecutor); + _timelock.setAdminExecutor(adminExecutor); + + assertEq(_timelock.getAdminExecutor(), adminExecutor); + } + + function test_setAdminExecutor_RevertOn_InvalidAddress() external { + vm.startPrank(_adminExecutor); + + vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.InvalidAdminExecutor.selector, address(0))); + _timelock.setAdminExecutor(address(0)); + + vm.expectRevert( + abi.encodeWithSelector(EmergencyProtectedTimelock.InvalidAdminExecutor.selector, _adminExecutor) + ); + _timelock.setAdminExecutor(_adminExecutor); + } + // Utils function _submitProposal() internal { From ec6814525ff5a3b33640b6b9f20b95d15b12f7c9 Mon Sep 17 00:00:00 2001 From: Roman Kolpakov Date: Tue, 15 Oct 2024 16:17:16 +0300 Subject: [PATCH 3/3] review fixes --- contracts/DualGovernance.sol | 29 ++++++--- contracts/EmergencyProtectedTimelock.sol | 27 ++------- contracts/libraries/TimelockState.sol | 16 +++++ test/unit/DualGovernance.t.sol | 68 ++++++++++++++-------- test/unit/EmergencyProtectedTimelock.t.sol | 21 ++----- test/unit/libraries/TimelockState.t.sol | 21 +++++++ test/utils/addresses.sol | 22 +++++++ 7 files changed, 133 insertions(+), 71 deletions(-) create mode 100644 test/utils/addresses.sol diff --git a/contracts/DualGovernance.sol b/contracts/DualGovernance.sol index a7601bbf..8227161f 100644 --- a/contracts/DualGovernance.sol +++ b/contracts/DualGovernance.sol @@ -44,6 +44,7 @@ contract DualGovernance is IDualGovernance { error ProposalSchedulingBlocked(uint256 proposalId); error ResealIsNotAllowedInNormalState(); error InvalidResealManager(address resealManager); + error InvalidResealCommittee(address resealCommittee); // --- // Events @@ -161,9 +162,7 @@ contract DualGovernance is IDualGovernance { emit EscrowMasterCopyDeployed(ESCROW_MASTER_COPY); _stateMachine.initialize(dependencies.configProvider, ESCROW_MASTER_COPY); - - _resealManager = dependencies.resealManager; - emit ResealManagerSet(address(_resealManager)); + _setResealManager(address(dependencies.resealManager)); } // --- @@ -494,8 +493,10 @@ contract DualGovernance is IDualGovernance { /// @param resealCommittee The address of the new reseal committee. function setResealCommittee(address resealCommittee) external { _checkCallerIsAdminExecutor(); + if (resealCommittee == _resealCommittee) { + revert InvalidResealCommittee(resealCommittee); + } _resealCommittee = resealCommittee; - emit ResealCommitteeSet(resealCommittee); } @@ -503,17 +504,27 @@ contract DualGovernance is IDualGovernance { /// @param resealManager The address of the new Reseal Manager. function setResealManager(address resealManager) external { _checkCallerIsAdminExecutor(); - if (resealManager == address(0) || resealManager == address(_resealManager)) { - revert InvalidResealManager(resealManager); - } - _resealManager = IResealManager(resealManager); - emit ResealManagerSet(resealManager); + _setResealManager(resealManager); + } + + /// @notice Gets the address of the Reseal Manager. + /// @return resealManager The address of the Reseal Manager. + function getResealManager() external view returns (IResealManager) { + return _resealManager; } // --- // Internal methods // --- + function _setResealManager(address resealManager) internal { + if (resealManager == address(_resealManager) || resealManager == address(0)) { + revert InvalidResealManager(resealManager); + } + _resealManager = IResealManager(resealManager); + emit ResealManagerSet(resealManager); + } + function _checkCallerIsAdminExecutor() internal view { if (TIMELOCK.getAdminExecutor() != msg.sender) { revert CallerIsNotAdminExecutor(msg.sender); diff --git a/contracts/EmergencyProtectedTimelock.sol b/contracts/EmergencyProtectedTimelock.sol index bf8d85b6..206555b6 100644 --- a/contracts/EmergencyProtectedTimelock.sol +++ b/contracts/EmergencyProtectedTimelock.sol @@ -21,18 +21,11 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { using ExecutableProposals for ExecutableProposals.Context; using EmergencyProtection for EmergencyProtection.Context; - // --- - // Events - // --- - - event AdminExecutorSet(address newAdminExecutor); - // --- // Errors // --- error CallerIsNotAdminExecutor(address value); - error InvalidAdminExecutor(address adminExecutor); // --- // Sanity Check Parameters & Immutables @@ -75,13 +68,6 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @dev The functionality for managing the emergency protection mechanism. EmergencyProtection.Context internal _emergencyProtection; - // --- - // Admin Executor - // --- - - /// @dev The address of the admin executor, authorized to manage the EmergencyProtectedTimelock instance. - address private _adminExecutor; - // --- // Constructor // --- @@ -92,8 +78,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { MAX_EMERGENCY_MODE_DURATION = sanityCheckParams.maxEmergencyModeDuration; MAX_EMERGENCY_PROTECTION_DURATION = sanityCheckParams.maxEmergencyProtectionDuration; - _adminExecutor = adminExecutor; - emit AdminExecutorSet(adminExecutor); + _timelockState.setAdminExecutor(adminExecutor); } // --- @@ -287,7 +272,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @notice Returns the address of the admin executor. /// @return adminExecutor The address of the admin executor. function getAdminExecutor() external view returns (address) { - return _adminExecutor; + return _timelockState.adminExecutor; } /// @notice Returns the configured delay duration required before a submitted proposal can be scheduled. @@ -370,11 +355,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @param newAdminExecutor The address of the new admin executor. function setAdminExecutor(address newAdminExecutor) external { _checkCallerIsAdminExecutor(); - if (newAdminExecutor == address(0) || newAdminExecutor == _adminExecutor) { - revert InvalidAdminExecutor(newAdminExecutor); - } - _adminExecutor = newAdminExecutor; - emit AdminExecutorSet(newAdminExecutor); + _timelockState.setAdminExecutor(newAdminExecutor); } // --- @@ -382,7 +363,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { // --- function _checkCallerIsAdminExecutor() internal view { - if (msg.sender != _adminExecutor) { + if (msg.sender != _timelockState.adminExecutor) { revert CallerIsNotAdminExecutor(msg.sender); } } diff --git a/contracts/libraries/TimelockState.sol b/contracts/libraries/TimelockState.sol index 05976793..67453c9f 100644 --- a/contracts/libraries/TimelockState.sol +++ b/contracts/libraries/TimelockState.sol @@ -10,10 +10,12 @@ library TimelockState { error InvalidGovernance(address value); error InvalidAfterSubmitDelay(Duration value); error InvalidAfterScheduleDelay(Duration value); + error InvalidAdminExecutor(address adminExecutor); event GovernanceSet(address newGovernance); event AfterSubmitDelaySet(Duration newAfterSubmitDelay); event AfterScheduleDelaySet(Duration newAfterScheduleDelay); + event AdminExecutorSet(address newAdminExecutor); struct Context { /// @dev slot0 [0..159] @@ -22,6 +24,8 @@ library TimelockState { Duration afterSubmitDelay; /// @dev slot0 [192..224] Duration afterScheduleDelay; + /// @dev slot1 [0..159] + address adminExecutor; } /// @notice Sets the governance address. @@ -70,6 +74,18 @@ library TimelockState { emit AfterScheduleDelaySet(newAfterScheduleDelay); } + /// @notice Sets the admin executor address. + /// @dev Reverts if the new admin executor address is zero or the same as the current one. + /// @param self The context of the timelock state. + /// @param newAdminExecutor The new admin executor address. + function setAdminExecutor(Context storage self, address newAdminExecutor) internal { + if (newAdminExecutor == address(0) || newAdminExecutor == self.adminExecutor) { + revert InvalidAdminExecutor(newAdminExecutor); + } + self.adminExecutor = newAdminExecutor; + emit AdminExecutorSet(newAdminExecutor); + } + /// @notice Gets the after submit delay. /// @param self The context of the timelock state. /// @return The current after submit delay. diff --git a/test/unit/DualGovernance.t.sol b/test/unit/DualGovernance.t.sol index ad67661a..5adab9b8 100644 --- a/test/unit/DualGovernance.t.sol +++ b/test/unit/DualGovernance.t.sol @@ -33,6 +33,7 @@ import {StETHMock} from "test/mocks/StETHMock.sol"; import {TimelockMock} from "test/mocks/TimelockMock.sol"; import {WithdrawalQueueMock} from "test/mocks/WithdrawalQueueMock.sol"; import {SealableMock} from "test/mocks/SealableMock.sol"; +import {computeAddress} from "test/utils/addresses.sol"; contract DualGovernanceUnitTests is UnitTest { Executor private _executor = new Executor(address(this)); @@ -2107,6 +2108,21 @@ contract DualGovernanceUnitTests is UnitTest { _dualGovernance.setResealCommittee(makeAddr("NEW_RESEAL_COMMITTEE")); } + function testFuzz_setResealCommittee_RevertOn_InvalidResealCommittee(address newResealCommittee) external { + _executor.execute( + address(_dualGovernance), + 0, + abi.encodeWithSelector(DualGovernance.setResealCommittee.selector, newResealCommittee) + ); + + vm.expectRevert(abi.encodeWithSelector(DualGovernance.InvalidResealCommittee.selector, newResealCommittee)); + _executor.execute( + address(_dualGovernance), + 0, + abi.encodeWithSelector(DualGovernance.setResealCommittee.selector, newResealCommittee) + ); + } + // --- // setResealManager() // --- @@ -2125,19 +2141,43 @@ contract DualGovernanceUnitTests is UnitTest { } function test_setResealManager_RevertOn_InvalidAddress() external { + vm.expectRevert( + abi.encodeWithSelector(DualGovernance.InvalidResealManager.selector, address(_RESEAL_MANAGER_STUB)) + ); + _executor.execute( + address(_dualGovernance), + 0, + abi.encodeWithSelector(DualGovernance.setResealManager.selector, address(_RESEAL_MANAGER_STUB)) + ); + vm.expectRevert(abi.encodeWithSelector(DualGovernance.InvalidResealManager.selector, address(0))); _executor.execute( address(_dualGovernance), 0, abi.encodeWithSelector(DualGovernance.setResealManager.selector, address(0)) ); + } + + function test_setResealManger_RevertOn_CallerIsNotAdminExecutor(address stranger) external { + vm.assume(stranger != address(_executor)); + + vm.prank(stranger); + vm.expectRevert(abi.encodeWithSelector(DualGovernance.CallerIsNotAdminExecutor.selector, stranger)); + _dualGovernance.setResealManager(address(0x123)); + } + + // --- + // getResealManager() + // --- + + function testFuzz_getResealManager_HappyPath(address newResealManager) external { + vm.assume(newResealManager != address(_RESEAL_MANAGER_STUB)); - vm.expectRevert( - abi.encodeWithSelector(DualGovernance.InvalidResealManager.selector, address(_RESEAL_MANAGER_STUB)) - ); _executor.execute( address(_dualGovernance), 0, - abi.encodeWithSelector(DualGovernance.setResealManager.selector, address(_RESEAL_MANAGER_STUB)) + abi.encodeWithSelector(DualGovernance.setResealManager.selector, newResealManager) ); + + assertEq(newResealManager, address(_dualGovernance.getResealManager())); } // --- @@ -2153,24 +2193,4 @@ contract DualGovernanceUnitTests is UnitTest { calls = new ExternalCall[](1); calls[0] = ExternalCall({target: address(0x123), value: 0, payload: abi.encodeWithSignature("someFunction()")}); } - - function computeAddress(address deployer, uint256 nonce) public pure returns (address) { - bytes memory data; - - if (nonce == 0x00) { - data = abi.encodePacked(hex"94", deployer, hex"80"); - } else if (nonce <= 0x7f) { - data = abi.encodePacked(hex"d6", hex"94", deployer, uint8(nonce)); - } else if (nonce <= 0xff) { - data = abi.encodePacked(hex"d7", hex"94", deployer, hex"81", uint8(nonce)); - } else if (nonce <= 0xffff) { - data = abi.encodePacked(hex"d8", hex"94", deployer, hex"82", uint16(nonce)); - } else if (nonce <= 0xffffff) { - data = abi.encodePacked(hex"d9", hex"94", deployer, hex"83", uint24(nonce)); - } else { - data = abi.encodePacked(hex"da", hex"94", deployer, hex"84", uint32(nonce)); - } - - return address(uint160(uint256(keccak256(data)))); - } } diff --git a/test/unit/EmergencyProtectedTimelock.t.sol b/test/unit/EmergencyProtectedTimelock.t.sol index 0c4387b5..729b2b7f 100644 --- a/test/unit/EmergencyProtectedTimelock.t.sol +++ b/test/unit/EmergencyProtectedTimelock.t.sol @@ -60,12 +60,10 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { EmergencyProtectedTimelock.SanityCheckParams memory sanityCheckParams, address adminExecutor ) external { - vm.expectEmit(); - emit EmergencyProtectedTimelock.AdminExecutorSet(adminExecutor); + vm.assume(adminExecutor != address(0)); EmergencyProtectedTimelock timelock = new EmergencyProtectedTimelock(sanityCheckParams, adminExecutor); assertEq(timelock.getAdminExecutor(), adminExecutor); - assertEq(timelock.MAX_AFTER_SUBMIT_DELAY(), sanityCheckParams.maxAfterSubmitDelay); assertEq(timelock.MAX_AFTER_SCHEDULE_DELAY(), sanityCheckParams.maxAfterScheduleDelay); assertEq(timelock.MAX_EMERGENCY_MODE_DURATION(), sanityCheckParams.maxEmergencyModeDuration); @@ -994,25 +992,18 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { function testFuzz_setAdminExecutor_HappyPath(address adminExecutor) external { vm.assume(adminExecutor != _adminExecutor && adminExecutor != address(0)); - - vm.expectEmit(); - emit EmergencyProtectedTimelock.AdminExecutorSet(adminExecutor); vm.prank(_adminExecutor); _timelock.setAdminExecutor(adminExecutor); assertEq(_timelock.getAdminExecutor(), adminExecutor); } - function test_setAdminExecutor_RevertOn_InvalidAddress() external { - vm.startPrank(_adminExecutor); - - vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.InvalidAdminExecutor.selector, address(0))); - _timelock.setAdminExecutor(address(0)); + function test_setAdminExecutor_RevertOn_NotAdminExecutor(address stranger) external { + vm.assume(stranger != _adminExecutor); - vm.expectRevert( - abi.encodeWithSelector(EmergencyProtectedTimelock.InvalidAdminExecutor.selector, _adminExecutor) - ); - _timelock.setAdminExecutor(_adminExecutor); + vm.prank(stranger); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, stranger)); + _timelock.setAdminExecutor(address(0x123)); } // Utils diff --git a/test/unit/libraries/TimelockState.t.sol b/test/unit/libraries/TimelockState.t.sol index 68758e86..aa78fc36 100644 --- a/test/unit/libraries/TimelockState.t.sol +++ b/test/unit/libraries/TimelockState.t.sol @@ -12,6 +12,7 @@ contract TimelockStateUnitTests is UnitTest { TimelockState.Context internal _timelockState; address internal governance = makeAddr("governance"); + address internal adminExecutor = makeAddr("adminExecutor"); Duration internal afterSubmitDelay = Durations.from(1 days); Duration internal afterScheduleDelay = Durations.from(2 days); @@ -20,6 +21,7 @@ contract TimelockStateUnitTests is UnitTest { function setUp() external { TimelockState.setGovernance(_timelockState, governance); + TimelockState.setAdminExecutor(_timelockState, adminExecutor); TimelockState.setAfterSubmitDelay(_timelockState, afterSubmitDelay, maxAfterSubmitDelay); TimelockState.setAfterScheduleDelay(_timelockState, afterScheduleDelay, maxAfterScheduleDelay); } @@ -87,6 +89,25 @@ contract TimelockStateUnitTests is UnitTest { TimelockState.setAfterScheduleDelay(_timelockState, afterScheduleDelay, maxAfterScheduleDelay); } + function testFuzz_setAdminExecutor_HappyPath(address newAdminExecutor) external { + vm.assume(newAdminExecutor != address(0) && newAdminExecutor != adminExecutor); + + vm.expectEmit(); + emit TimelockState.AdminExecutorSet(newAdminExecutor); + + TimelockState.setAdminExecutor(_timelockState, newAdminExecutor); + } + + function test_setAdminExecutor_RevertOn_ZeroAddress() external { + vm.expectRevert(abi.encodeWithSelector(TimelockState.InvalidAdminExecutor.selector, address(0))); + TimelockState.setAdminExecutor(_timelockState, address(0)); + } + + function test_setAdminExecutor_RevertOn_SameAddress() external { + vm.expectRevert(abi.encodeWithSelector(TimelockState.InvalidAdminExecutor.selector, adminExecutor)); + TimelockState.setAdminExecutor(_timelockState, adminExecutor); + } + function testFuzz_getAfterSubmitDelay_HappyPath(Duration newAfterSubmitDelay) external { TimelockState.setAfterSubmitDelay(_timelockState, newAfterSubmitDelay, newAfterSubmitDelay); assertEq(TimelockState.getAfterSubmitDelay(_timelockState), newAfterSubmitDelay); diff --git a/test/utils/addresses.sol b/test/utils/addresses.sol new file mode 100644 index 00000000..7a3c4ba1 --- /dev/null +++ b/test/utils/addresses.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +function computeAddress(address deployer, uint256 nonce) returns (address) { + bytes memory data; + + if (nonce == 0x00) { + data = abi.encodePacked(hex"94", deployer, hex"80"); + } else if (nonce <= 0x7f) { + data = abi.encodePacked(hex"d6", hex"94", deployer, uint8(nonce)); + } else if (nonce <= 0xff) { + data = abi.encodePacked(hex"d7", hex"94", deployer, hex"81", uint8(nonce)); + } else if (nonce <= 0xffff) { + data = abi.encodePacked(hex"d8", hex"94", deployer, hex"82", uint16(nonce)); + } else if (nonce <= 0xffffff) { + data = abi.encodePacked(hex"d9", hex"94", deployer, hex"83", uint24(nonce)); + } else { + data = abi.encodePacked(hex"da", hex"94", deployer, hex"84", uint32(nonce)); + } + + return address(uint160(uint256(keccak256(data)))); +}