From 0bd250f387fe4ae3ae56192ef80966b595f52def Mon Sep 17 00:00:00 2001 From: Bogdan Kovtun Date: Fri, 6 Dec 2024 01:11:48 +0400 Subject: [PATCH 1/2] Use prefix new for arguments in the setters --- contracts/DualGovernance.sol | 30 +++++++++---------- contracts/EmergencyProtectedTimelock.sol | 30 +++++++++---------- contracts/committees/HashConsensus.sol | 12 ++++---- contracts/interfaces/IDualGovernance.sol | 4 +-- .../IEmergencyProtectedTimelock.sol | 10 +++---- contracts/interfaces/ITiebreaker.sol | 4 +-- contracts/libraries/Proposers.sol | 14 ++++----- docs/specification.md | 2 +- 8 files changed, 53 insertions(+), 53 deletions(-) diff --git a/contracts/DualGovernance.sol b/contracts/DualGovernance.sol index 19d4426d..f5ace612 100644 --- a/contracts/DualGovernance.sol +++ b/contracts/DualGovernance.sol @@ -376,10 +376,10 @@ contract DualGovernance is IDualGovernance { /// @dev Ensures that at least one proposer remains assigned to the `adminExecutor` following the update. /// Reverts if updating the proposer’s executor would leave the `adminExecutor` without any associated proposer. /// @param proposerAccount The address of the proposer whose executor is being updated. - /// @param executor The new executor address to assign to the proposer. - function setProposerExecutor(address proposerAccount, address executor) external { + /// @param newExecutor The new executor address to assign to the proposer. + function setProposerExecutor(address proposerAccount, address newExecutor) external { _checkCallerIsAdminExecutor(); - _proposers.setProposerExecutor(proposerAccount, executor); + _proposers.setProposerExecutor(proposerAccount, newExecutor); /// @dev after update of the proposer, check that admin executor still belongs to some proposer _proposers.checkRegisteredExecutor(TIMELOCK.getAdminExecutor()); @@ -450,21 +450,21 @@ contract DualGovernance is IDualGovernance { } /// @notice Sets the new address of the tiebreaker committee in the system. - /// @param tiebreakerCommittee The address of the new tiebreaker committee. - function setTiebreakerCommittee(address tiebreakerCommittee) external { + /// @param newTiebreakerCommittee The address of the new tiebreaker committee. + function setTiebreakerCommittee(address newTiebreakerCommittee) external { _checkCallerIsAdminExecutor(); - _tiebreaker.setTiebreakerCommittee(tiebreakerCommittee); + _tiebreaker.setTiebreakerCommittee(newTiebreakerCommittee); } /// @notice Sets the new value for the tiebreaker activation timeout. /// @dev If the Dual Governance system remains out of the `Normal` or `VetoCooldown` state for longer than /// the `tiebreakerActivationTimeout` duration, the tiebreaker committee is allowed to schedule /// submitted proposals. - /// @param tiebreakerActivationTimeout The new duration for the tiebreaker activation timeout. - function setTiebreakerActivationTimeout(Duration tiebreakerActivationTimeout) external { + /// @param newTiebreakerActivationTimeout The new duration for the tiebreaker activation timeout. + function setTiebreakerActivationTimeout(Duration newTiebreakerActivationTimeout) external { _checkCallerIsAdminExecutor(); _tiebreaker.setTiebreakerActivationTimeout( - MIN_TIEBREAKER_ACTIVATION_TIMEOUT, tiebreakerActivationTimeout, MAX_TIEBREAKER_ACTIVATION_TIMEOUT + MIN_TIEBREAKER_ACTIVATION_TIMEOUT, newTiebreakerActivationTimeout, MAX_TIEBREAKER_ACTIVATION_TIMEOUT ); } @@ -516,17 +516,17 @@ contract DualGovernance is IDualGovernance { } /// @notice Sets the address of the reseal committee. - /// @param resealCommittee The address of the new reseal committee. - function setResealCommittee(address resealCommittee) external { + /// @param newResealCommittee The address of the new reseal committee. + function setResealCommittee(address newResealCommittee) external { _checkCallerIsAdminExecutor(); - _resealer.setResealCommittee(resealCommittee); + _resealer.setResealCommittee(newResealCommittee); } /// @notice Sets the address of the Reseal Manager. - /// @param resealManager The address of the new Reseal Manager. - function setResealManager(address resealManager) external { + /// @param newResealManager The address of the new Reseal Manager. + function setResealManager(address newResealManager) external { _checkCallerIsAdminExecutor(); - _resealer.setResealManager(resealManager); + _resealer.setResealManager(newResealManager); } /// @notice Gets the address of the Reseal Manager. diff --git a/contracts/EmergencyProtectedTimelock.sol b/contracts/EmergencyProtectedTimelock.sol index 55e4b69b..331f4502 100644 --- a/contracts/EmergencyProtectedTimelock.sol +++ b/contracts/EmergencyProtectedTimelock.sol @@ -180,40 +180,40 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { // --- /// @notice Sets the emergency activation committee address. - /// @param emergencyActivationCommittee The address of the emergency activation committee. - function setEmergencyProtectionActivationCommittee(address emergencyActivationCommittee) external { + /// @param newEmergencyActivationCommittee The address of the emergency activation committee. + function setEmergencyProtectionActivationCommittee(address newEmergencyActivationCommittee) external { _timelockState.checkCallerIsAdminExecutor(); - _emergencyProtection.setEmergencyActivationCommittee(emergencyActivationCommittee); + _emergencyProtection.setEmergencyActivationCommittee(newEmergencyActivationCommittee); } /// @notice Sets the emergency execution committee address. - /// @param emergencyExecutionCommittee The address of the emergency execution committee. - function setEmergencyProtectionExecutionCommittee(address emergencyExecutionCommittee) external { + /// @param newEmergencyExecutionCommittee The address of the emergency execution committee. + function setEmergencyProtectionExecutionCommittee(address newEmergencyExecutionCommittee) external { _timelockState.checkCallerIsAdminExecutor(); - _emergencyProtection.setEmergencyExecutionCommittee(emergencyExecutionCommittee); + _emergencyProtection.setEmergencyExecutionCommittee(newEmergencyExecutionCommittee); } /// @notice Sets the emergency protection end date. - /// @param emergencyProtectionEndDate The timestamp of the emergency protection end date. - function setEmergencyProtectionEndDate(Timestamp emergencyProtectionEndDate) external { + /// @param newEmergencyProtectionEndDate The timestamp of the emergency protection end date. + function setEmergencyProtectionEndDate(Timestamp newEmergencyProtectionEndDate) external { _timelockState.checkCallerIsAdminExecutor(); _emergencyProtection.setEmergencyProtectionEndDate( - emergencyProtectionEndDate, MAX_EMERGENCY_PROTECTION_DURATION + newEmergencyProtectionEndDate, MAX_EMERGENCY_PROTECTION_DURATION ); } /// @notice Sets the emergency mode duration. - /// @param emergencyModeDuration The duration of the emergency mode. - function setEmergencyModeDuration(Duration emergencyModeDuration) external { + /// @param newEmergencyModeDuration The duration of the emergency mode. + function setEmergencyModeDuration(Duration newEmergencyModeDuration) external { _timelockState.checkCallerIsAdminExecutor(); - _emergencyProtection.setEmergencyModeDuration(emergencyModeDuration, MAX_EMERGENCY_MODE_DURATION); + _emergencyProtection.setEmergencyModeDuration(newEmergencyModeDuration, MAX_EMERGENCY_MODE_DURATION); } /// @notice Sets the emergency governance address. - /// @param emergencyGovernance The address of the emergency governance. - function setEmergencyGovernance(address emergencyGovernance) external { + /// @param newEmergencyGovernance The address of the emergency governance. + function setEmergencyGovernance(address newEmergencyGovernance) external { _timelockState.checkCallerIsAdminExecutor(); - _emergencyProtection.setEmergencyGovernance(emergencyGovernance); + _emergencyProtection.setEmergencyGovernance(newEmergencyGovernance); } /// @notice Activates the emergency mode. diff --git a/contracts/committees/HashConsensus.sol b/contracts/committees/HashConsensus.sol index 0e825f7d..2643c4f6 100644 --- a/contracts/committees/HashConsensus.sol +++ b/contracts/committees/HashConsensus.sol @@ -104,14 +104,14 @@ abstract contract HashConsensus is Ownable { /// @notice Sets the timelock duration /// @dev Only callable by the owner - /// @param timelock The new timelock duration in seconds - function setTimelockDuration(Duration timelock) external { + /// @param newTimelock The new timelock duration in seconds + function setTimelockDuration(Duration newTimelock) external { _checkOwner(); - if (timelock == _timelockDuration) { - revert InvalidTimelockDuration(timelock); + if (newTimelock == _timelockDuration) { + revert InvalidTimelockDuration(newTimelock); } - _timelockDuration = timelock; - emit TimelockDurationSet(timelock); + _timelockDuration = newTimelock; + emit TimelockDurationSet(newTimelock); } /// @notice Gets the quorum value diff --git a/contracts/interfaces/IDualGovernance.sol b/contracts/interfaces/IDualGovernance.sol index db7596b8..1c0989cf 100644 --- a/contracts/interfaces/IDualGovernance.sol +++ b/contracts/interfaces/IDualGovernance.sol @@ -47,8 +47,8 @@ interface IDualGovernance is IGovernance, ITiebreaker { function isRegisteredExecutor(address account) external view returns (bool); function resealSealable(address sealable) external; - function setResealCommittee(address resealCommittee) external; - function setResealManager(address resealManager) external; + function setResealCommittee(address newResealCommittee) external; + function setResealManager(address newResealManager) external; function getResealManager() external view returns (IResealManager); function getResealCommittee() external view returns (address); } diff --git a/contracts/interfaces/IEmergencyProtectedTimelock.sol b/contracts/interfaces/IEmergencyProtectedTimelock.sol index fa8ab86e..6b0b47fa 100644 --- a/contracts/interfaces/IEmergencyProtectedTimelock.sol +++ b/contracts/interfaces/IEmergencyProtectedTimelock.sol @@ -17,11 +17,11 @@ interface IEmergencyProtectedTimelock is ITimelock { function MAX_EMERGENCY_MODE_DURATION() external view returns (Duration); function MAX_EMERGENCY_PROTECTION_DURATION() external view returns (Duration); - function setEmergencyProtectionActivationCommittee(address emergencyActivationCommittee) external; - function setEmergencyProtectionExecutionCommittee(address emergencyExecutionCommittee) external; - function setEmergencyProtectionEndDate(Timestamp emergencyProtectionEndDate) external; - function setEmergencyModeDuration(Duration emergencyModeDuration) external; - function setEmergencyGovernance(address emergencyGovernance) external; + function setEmergencyProtectionActivationCommittee(address newEmergencyActivationCommittee) external; + function setEmergencyProtectionExecutionCommittee(address newEmergencyExecutionCommittee) external; + function setEmergencyProtectionEndDate(Timestamp newEmergencyProtectionEndDate) external; + function setEmergencyModeDuration(Duration newEmergencyModeDuration) external; + function setEmergencyGovernance(address newEmergencyGovernance) external; function activateEmergencyMode() external; function emergencyExecute(uint256 proposalId) external; diff --git a/contracts/interfaces/ITiebreaker.sol b/contracts/interfaces/ITiebreaker.sol index 6375fdf7..9d4e89fa 100644 --- a/contracts/interfaces/ITiebreaker.sol +++ b/contracts/interfaces/ITiebreaker.sol @@ -13,8 +13,8 @@ interface ITiebreaker { function addTiebreakerSealableWithdrawalBlocker(address sealableWithdrawalBlocker) external; function removeTiebreakerSealableWithdrawalBlocker(address sealableWithdrawalBlocker) external; - function setTiebreakerCommittee(address tiebreakerCommittee) external; - function setTiebreakerActivationTimeout(Duration tiebreakerActivationTimeout) external; + function setTiebreakerCommittee(address newTiebreakerCommittee) external; + function setTiebreakerActivationTimeout(Duration newTiebreakerActivationTimeout) external; function tiebreakerScheduleProposal(uint256 proposalId) external; function getTiebreakerDetails() external view returns (TiebreakerDetails memory tiebreakerState); function tiebreakerResumeSealable(address sealable) external; diff --git a/contracts/libraries/Proposers.sol b/contracts/libraries/Proposers.sol index 5bbb471c..058d7b6f 100644 --- a/contracts/libraries/Proposers.sol +++ b/contracts/libraries/Proposers.sol @@ -89,21 +89,21 @@ library Proposers { /// @notice Updates the executor for a registered proposer. /// @param self The context storage of the Proposers library. /// @param proposerAccount The address of the proposer to update. - /// @param executor The new executor address to assign to the proposer. - function setProposerExecutor(Context storage self, address proposerAccount, address executor) internal { + /// @param newExecutor The new executor address to assign to the proposer. + function setProposerExecutor(Context storage self, address proposerAccount, address newExecutor) internal { ExecutorData memory executorData = self.executors[proposerAccount]; _checkRegisteredProposer(proposerAccount, executorData); - if (executor == address(0) || executorData.executor == executor) { - revert InvalidExecutor(executor); + if (newExecutor == address(0) || executorData.executor == newExecutor) { + revert InvalidExecutor(newExecutor); } - self.executors[proposerAccount].executor = executor; + self.executors[proposerAccount].executor = newExecutor; - self.executorRefsCounts[executor] += 1; + self.executorRefsCounts[newExecutor] += 1; self.executorRefsCounts[executorData.executor] -= 1; - emit ProposerExecutorSet(proposerAccount, executor); + emit ProposerExecutorSet(proposerAccount, newExecutor); } /// @notice Unregisters a proposer, removing its association with an executor. diff --git a/docs/specification.md b/docs/specification.md index 4bd812b9..d62816f2 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -1139,7 +1139,7 @@ Returns if an address is a member. ### Function: HashConsensus.setTimelockDuration ```solidity -function setTimelockDuration(uint256 timelock) +function setTimelockDuration(uint256 newTimelock) ``` Sets the timelock duration. From ef218958d87a61d3e6dd404cb45243b52806cf21 Mon Sep 17 00:00:00 2001 From: Bogdan Kovtun Date: Fri, 6 Dec 2024 16:47:16 +0400 Subject: [PATCH 2/2] Add setProposerExecutor to IDualGovernance. Fix test assume --- contracts/interfaces/IDualGovernance.sol | 1 + test/unit/DualGovernance.t.sol | 1 + 2 files changed, 2 insertions(+) diff --git a/contracts/interfaces/IDualGovernance.sol b/contracts/interfaces/IDualGovernance.sol index 1c0989cf..ebd10686 100644 --- a/contracts/interfaces/IDualGovernance.sol +++ b/contracts/interfaces/IDualGovernance.sol @@ -40,6 +40,7 @@ interface IDualGovernance is IGovernance, ITiebreaker { function getStateDetails() external view returns (StateDetails memory stateDetails); function registerProposer(address proposer, address executor) external; + function setProposerExecutor(address proposerAccount, address newExecutor) external; function unregisterProposer(address proposer) external; function isRegisteredProposer(address account) external view returns (bool); function getProposer(address account) external view returns (Proposers.Proposer memory proposer); diff --git a/test/unit/DualGovernance.t.sol b/test/unit/DualGovernance.t.sol index add00fc9..1ef609c4 100644 --- a/test/unit/DualGovernance.t.sol +++ b/test/unit/DualGovernance.t.sol @@ -2312,6 +2312,7 @@ contract DualGovernanceUnitTests is UnitTest { } function testFuzz_setResealCommittee_RevertOn_InvalidResealCommittee(address newResealCommittee) external { + vm.assume(_dualGovernance.getResealCommittee() != newResealCommittee); _executor.execute( address(_dualGovernance), 0,