From 00aa9b5f96d01d7818a38347aba05a3465b0d23e Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Thu, 31 Jul 2025 10:50:12 -0500 Subject: [PATCH 1/4] feat: outgoing members automatically become nominees (#350) * automatically make members nominees * member must add themself as contender before nomination * test auto nomination * fix tests * gas snapshot * test: reelection * chore: update snapshot --------- Co-authored-by: gzeon --- .gas-snapshot | 30 ++++++++--------- ...SecurityCouncilNomineeElectionGovernor.sol | 5 +++ ...curityCouncilNomineeElectionGovernor.t.sol | 33 +++++++++++++++++++ .../governors/TopNomineesGas.t.sol | 1 + 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 6cdb11e5a..1e9c6ec38 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -27,7 +27,7 @@ ArbitrumVestingWalletTest:testDoesDeploy() (gas: 15971357) ArbitrumVestingWalletTest:testReleaseAffordance() (gas: 16008664) ArbitrumVestingWalletTest:testVestedAmountStart() (gas: 16074932) CancelTimelockAndRemoveMemberActionTest:testAction() (gas: 8159) -E2E:testE2E() (gas: 86427645) +E2E:testE2E() (gas: 86487452) FixedDelegateErc20WalletTest:testInit() (gas: 5822585) FixedDelegateErc20WalletTest:testInitZeroToken() (gas: 5816815) FixedDelegateErc20WalletTest:testTransfer() (gas: 5932228) @@ -95,11 +95,11 @@ L2GovernanceFactoryTest:testSanityCheckValues() (gas: 28571182) L2GovernanceFactoryTest:testSetMinDelay() (gas: 28519939) L2GovernanceFactoryTest:testSetMinDelayRevertsForCoreAddress() (gas: 28572810) L2GovernanceFactoryTest:testUpgraderCanCancel() (gas: 28812928) -L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 32000018) -L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 32004249) -L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 26965169) -L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32002249) -L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32023714) +L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 32028304) +L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 32032535) +L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 26993455) +L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32030535) +L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32052000) NomineeGovernorV2UpgradeActionTest:testAction() (gas: 8153) OfficeHoursActionTest:testConstructor() (gas: 9050) OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 256, μ: 317059, ~: 317184) @@ -140,7 +140,7 @@ SecurityCouncilManagerTest:testReplaceMemberInFirstCohortAfterRotation() (gas: 4 SecurityCouncilManagerTest:testReplaceMemberInSecondCohort() (gas: 479079) SecurityCouncilManagerTest:testReplaceMemberInSecondCohortAfterRotation() (gas: 270188) SecurityCouncilManagerTest:testRotateMember() (gas: 1016355) -SecurityCouncilManagerTest:testRotateMemberNotContender() (gas: 3869126) +SecurityCouncilManagerTest:testRotateMemberNotContender() (gas: 3891932) SecurityCouncilManagerTest:testSetMinRotationPeriod() (gas: 65924) SecurityCouncilManagerTest:testSetRotatingTo() (gas: 113048) SecurityCouncilManagerTest:testUpdateCohortAffordances() (gas: 83230) @@ -197,17 +197,17 @@ SecurityCouncilMemberSyncActionTest:testRemoveOne() (gas: 8086867) SecurityCouncilMemberSyncActionTest:testUpdateCohort() (gas: 8328313) SecurityCouncilMemberSyncActionTest:testUpdateCohort() (gas: 8329174) SecurityCouncilMgmtUtilsTests:testIsInArray() (gas: 2102) -SecurityCouncilNomineeElectionGovernorTest:testAddContender() (gas: 270750) -SecurityCouncilNomineeElectionGovernorTest:testCastBySig() (gas: 333730) -SecurityCouncilNomineeElectionGovernorTest:testCastBySigTwice() (gas: 296589) +SecurityCouncilNomineeElectionGovernorTest:testAddContender() (gas: 415952) +SecurityCouncilNomineeElectionGovernorTest:testCastBySig() (gas: 336423) +SecurityCouncilNomineeElectionGovernorTest:testCastBySigTwice() (gas: 299282) SecurityCouncilNomineeElectionGovernorTest:testCastVoteReverts() (gas: 35278) -SecurityCouncilNomineeElectionGovernorTest:testCountVote() (gas: 582574) +SecurityCouncilNomineeElectionGovernorTest:testCountVote() (gas: 590700) SecurityCouncilNomineeElectionGovernorTest:testCreateElection() (gas: 253153) -SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 456505) +SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 459197) SecurityCouncilNomineeElectionGovernorTest:testExecute() (gas: 677159) -SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 194733) -SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 674020) -SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7256741) +SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 197425) +SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 676717) +SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7285027) SecurityCouncilNomineeElectionGovernorTest:testProperInitialization() (gas: 78113) SecurityCouncilNomineeElectionGovernorTest:testProposeFails() (gas: 19740) SecurityCouncilNomineeElectionGovernorTest:testRelay() (gas: 42427) diff --git a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol index 0d8e498e1..688eaac3e 100644 --- a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol +++ b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol @@ -245,6 +245,11 @@ contract SecurityCouncilNomineeElectionGovernor is election.isContender[signer] = true; emit ContenderAdded(proposalId, signer); + + // if the signer is part of the outgoing cohort, we automatically add them as a nominee + if (securityCouncilManager.cohortIncludes(currentCohort(), signer)) { + _addNominee(proposalId, signer); + } } /// @notice Allows the owner to change the nomineeVetter diff --git a/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol b/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol index 7cb019024..c8a086195 100644 --- a/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol +++ b/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol @@ -248,6 +248,7 @@ contract SecurityCouncilNomineeElectionGovernorTest is Test { sig = sigUtils.signAddContenderMessage(proposalId, _contenderPrivKey(0)); // test in other cohort + _mockCohortIncludes(Cohort.FIRST, _contender(0), false); _mockCohortIncludes(Cohort.SECOND, _contender(0), true); vm.expectRevert( abi.encodeWithSelector( @@ -259,6 +260,7 @@ contract SecurityCouncilNomineeElectionGovernorTest is Test { governor.addContender(proposalId, sig); // should fail if the proposal is not pending + _mockCohortIncludes(Cohort.FIRST, _contender(0), false); _mockCohortIncludes(Cohort.SECOND, _contender(0), false); vm.roll(governor.proposalSnapshot(proposalId) + 1); assertTrue(governor.state(proposalId) == IGovernorUpgradeable.ProposalState.Active); @@ -277,6 +279,7 @@ contract SecurityCouncilNomineeElectionGovernorTest is Test { // check that it correctly mutated the state assertTrue(governor.isContender(proposalId, _contender(0))); + assertFalse(governor.isNominee(proposalId, _contender(0))); // adding again should fail vm.expectRevert( @@ -285,6 +288,35 @@ contract SecurityCouncilNomineeElectionGovernorTest is Test { ) ); governor.addContender(proposalId, sig); + + // adding a member up for reelection should succeed and automatically add them as a nominee + _mockCohortIncludes(Cohort.FIRST, _contender(1), true); + _mockCohortIncludes(Cohort.SECOND, _contender(1), false); + sig = sigUtils.signAddContenderMessage(proposalId, _contenderPrivKey(1)); + governor.addContender(proposalId, sig); + + // check that it correctly mutated the state + assertTrue(governor.isContender(proposalId, _contender(1))); + assertTrue(governor.isNominee(proposalId, _contender(1))); + + // reelection member should not be able to receive votes + vm.roll(governor.proposalSnapshot(proposalId) + 1); + _mockGetPastVotes(_voter(0), governor.quorum(proposalId)); + vm.prank(_voter(0)); + vm.expectRevert( + abi.encodeWithSelector( + SecurityCouncilNomineeElectionGovernorCountingUpgradeable + .NomineeAlreadyAdded + .selector, + _contender(1) + ) + ); + governor.castVoteWithReasonAndParams({ + proposalId: proposalId, + support: 1, + reason: "", + params: abi.encode(_contender(1), 1) + }); } function testSetNomineeVetter() public { @@ -917,6 +949,7 @@ contract SecurityCouncilNomineeElectionGovernorTest is Test { function _addContender(uint256 proposalId, uint8 contender) internal { uint256 privKey = _contenderPrivKey(contender); address addr = _contender(contender); + _mockCohortIncludes(Cohort.FIRST, addr, false); _mockCohortIncludes(Cohort.SECOND, addr, false); bytes memory sig = sigUtils.signAddContenderMessage(proposalId, privKey); governor.addContender(proposalId, sig); diff --git a/test/security-council-mgmt/governors/TopNomineesGas.t.sol b/test/security-council-mgmt/governors/TopNomineesGas.t.sol index 7e2873e1c..9637283e9 100644 --- a/test/security-council-mgmt/governors/TopNomineesGas.t.sol +++ b/test/security-council-mgmt/governors/TopNomineesGas.t.sol @@ -91,6 +91,7 @@ contract TopNomineesGasTest is Test { // vote for N nominees uint256 quorum = nomineeGov.quorum(proposalId); for (uint16 i = 0; i < N; i++) { + _mockCohortIncludes(Cohort.FIRST, _nominee(i), false); _mockCohortIncludes(Cohort.SECOND, _nominee(i), false); vm.roll(nomineeGov.proposalSnapshot(proposalId)); From 8737f43861a1a8533dfd3afb40213e8d856983c1 Mon Sep 17 00:00:00 2001 From: gzeon Date: Wed, 13 Aug 2025 00:14:35 +0800 Subject: [PATCH 2/4] feat: setCadence (#349) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: setCadence * test: setCadence * chore: update misc * test: 36 months * feat: CadenceChanged event * refactor: move CadenceChanged to Timing abstract contract * update optimizer runs * refactor: cut some size * refactor: equal 0 * fix: return * docs: currentElectionCount * refactor: use solady add and subMonths * chore: reduce sec_council_mgmt opt-run to 500 due to contract size * chore: cleanup lint warning * Simple cadence minor fixes (#359) --------- Co-authored-by: Henry <11198460+godzillaba@users.noreply.github.com> Co-authored-by: José FP <105675159+TucksonDev@users.noreply.github.com> --- .gas-snapshot | 61 ++++--- foundry.toml | 2 +- hardhat.config.ts | 2 +- ...SecurityCouncilNomineeElectionGovernor.sol | 31 +++- ...tyCouncilNomineeElectionGovernorTiming.sol | 78 ++++++++- ...curityCouncilNomineeElectionGovernor.t.sol | 161 +++++++++++++++++- .../SecurityCouncilNomineeElectionGovernor | 6 + .../SecurityCouncilNomineeElectionGovernor | 4 +- 8 files changed, 308 insertions(+), 37 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 108f0b682..9e401fed3 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -27,7 +27,7 @@ ArbitrumVestingWalletTest:testDoesDeploy() (gas: 15971357) ArbitrumVestingWalletTest:testReleaseAffordance() (gas: 16008664) ArbitrumVestingWalletTest:testVestedAmountStart() (gas: 16074932) CancelTimelockAndRemoveMemberActionTest:testAction() (gas: 8159) -E2E:testE2E() (gas: 86487452) +E2E:testE2E() (gas: 86859267) FixedDelegateErc20WalletTest:testInit() (gas: 5822585) FixedDelegateErc20WalletTest:testInitZeroToken() (gas: 5816815) FixedDelegateErc20WalletTest:testTransfer() (gas: 5932228) @@ -95,11 +95,11 @@ L2GovernanceFactoryTest:testSanityCheckValues() (gas: 28571182) L2GovernanceFactoryTest:testSetMinDelay() (gas: 28519939) L2GovernanceFactoryTest:testSetMinDelayRevertsForCoreAddress() (gas: 28572810) L2GovernanceFactoryTest:testUpgraderCanCancel() (gas: 28812928) -L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 32028301) -L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 32032532) -L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 26993452) -L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32030532) -L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32051997) +L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 32400694) +L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 32404948) +L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 27343716) +L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32402925) +L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32424390) NomineeGovernorV2UpgradeActionTest:testAction() (gas: 8153) OfficeHoursActionTest:testConstructor() (gas: 9050) OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 256, μ: 317059, ~: 317184) @@ -130,17 +130,17 @@ SecurityCouncilManagerTest:testInitialization() (gas: 206820) SecurityCouncilManagerTest:testPostUpgradeInit() (gas: 5255512) SecurityCouncilManagerTest:testRemoveMember() (gas: 217503) SecurityCouncilManagerTest:testRemoveMemberAffordances() (gas: 101612) -SecurityCouncilManagerTest:testRemoveMemberRotated() (gas: 423630) +SecurityCouncilManagerTest:testRemoveMemberRotated() (gas: 423607) SecurityCouncilManagerTest:testRemoveSCAffordances() (gas: 81486) SecurityCouncilManagerTest:testRemoveSeC() (gas: 38435) SecurityCouncilManagerTest:testReplaceCohortRotatingTo() (gas: 963734) SecurityCouncilManagerTest:testReplaceMemberAffordances() (gas: 216337) SecurityCouncilManagerTest:testReplaceMemberInFirstCohort() (gas: 266597) -SecurityCouncilManagerTest:testReplaceMemberInFirstCohortAfterRotation() (gas: 471835) -SecurityCouncilManagerTest:testReplaceMemberInSecondCohort() (gas: 479079) +SecurityCouncilManagerTest:testReplaceMemberInFirstCohortAfterRotation() (gas: 471812) +SecurityCouncilManagerTest:testReplaceMemberInSecondCohort() (gas: 479056) SecurityCouncilManagerTest:testReplaceMemberInSecondCohortAfterRotation() (gas: 270188) -SecurityCouncilManagerTest:testRotateMember() (gas: 1016355) -SecurityCouncilManagerTest:testRotateMemberNotContender() (gas: 3891932) +SecurityCouncilManagerTest:testRotateMember() (gas: 1016263) +SecurityCouncilManagerTest:testRotateMemberNotContender() (gas: 3893898) SecurityCouncilManagerTest:testSetMinRotationPeriod() (gas: 65924) SecurityCouncilManagerTest:testSetRotatingTo() (gas: 113048) SecurityCouncilManagerTest:testUpdateCohortAffordances() (gas: 83230) @@ -197,21 +197,30 @@ SecurityCouncilMemberSyncActionTest:testRemoveOne() (gas: 8086867) SecurityCouncilMemberSyncActionTest:testUpdateCohort() (gas: 8328313) SecurityCouncilMemberSyncActionTest:testUpdateCohort() (gas: 8329174) SecurityCouncilMgmtUtilsTests:testIsInArray() (gas: 2102) -SecurityCouncilNomineeElectionGovernorTest:testAddContender() (gas: 415952) -SecurityCouncilNomineeElectionGovernorTest:testCastBySig() (gas: 336423) -SecurityCouncilNomineeElectionGovernorTest:testCastBySigTwice() (gas: 299282) -SecurityCouncilNomineeElectionGovernorTest:testCastVoteReverts() (gas: 35278) -SecurityCouncilNomineeElectionGovernorTest:testCountVote() (gas: 590700) -SecurityCouncilNomineeElectionGovernorTest:testCreateElection() (gas: 253153) -SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 459197) -SecurityCouncilNomineeElectionGovernorTest:testExecute() (gas: 677159) -SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 197425) -SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 676717) -SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7285027) -SecurityCouncilNomineeElectionGovernorTest:testProperInitialization() (gas: 78113) -SecurityCouncilNomineeElectionGovernorTest:testProposeFails() (gas: 19740) -SecurityCouncilNomineeElectionGovernorTest:testRelay() (gas: 42427) -SecurityCouncilNomineeElectionGovernorTest:testSetNomineeVetter() (gas: 39905) +SecurityCouncilNomineeElectionGovernorTest:testAddContender() (gas: 417954) +SecurityCouncilNomineeElectionGovernorTest:testCadenceWithLargeValues() (gas: 52875) +SecurityCouncilNomineeElectionGovernorTest:testCastBySig() (gas: 338629) +SecurityCouncilNomineeElectionGovernorTest:testCastBySigTwice() (gas: 301488) +SecurityCouncilNomineeElectionGovernorTest:testCastVoteReverts() (gas: 35323) +SecurityCouncilNomineeElectionGovernorTest:testCountVote() (gas: 593018) +SecurityCouncilNomineeElectionGovernorTest:testCreateElection() (gas: 257849) +SecurityCouncilNomineeElectionGovernorTest:testDefaultCadence() (gas: 14927) +SecurityCouncilNomineeElectionGovernorTest:testElectionTimestampsWithDefaultCadence() (gas: 37625) +SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 461456) +SecurityCouncilNomineeElectionGovernorTest:testExecute() (gas: 679418) +SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 199664) +SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 678802) +SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7657451) +SecurityCouncilNomineeElectionGovernorTest:testMultipleCadenceChanges() (gas: 238823) +SecurityCouncilNomineeElectionGovernorTest:testProperInitialization() (gas: 78160) +SecurityCouncilNomineeElectionGovernorTest:testProposeFails() (gas: 19741) +SecurityCouncilNomineeElectionGovernorTest:testRelay() (gas: 42433) +SecurityCouncilNomineeElectionGovernorTest:testSetCadenceAfterElections() (gas: 227567) +SecurityCouncilNomineeElectionGovernorTest:testSetCadenceBeforeFirstElection() (gas: 42479) +SecurityCouncilNomineeElectionGovernorTest:testSetCadenceInvalidValue() (gas: 26010) +SecurityCouncilNomineeElectionGovernorTest:testSetCadenceOnlyOwner() (gas: 16089) +SecurityCouncilNomineeElectionGovernorTest:testSetCadenceTooSoonReverts() (gas: 148066) +SecurityCouncilNomineeElectionGovernorTest:testSetNomineeVetter() (gas: 40001) SequencerActionsTest:testAddAndRemoveSequencer() (gas: 486652) SequencerActionsTest:testCantAddZeroAddress() (gas: 235659) SetInitialGovParamsActionTest:testL1() (gas: 259949) diff --git a/foundry.toml b/foundry.toml index 26097a46e..60ddf0d41 100644 --- a/foundry.toml +++ b/foundry.toml @@ -9,7 +9,7 @@ via_ir = false solc_version = '0.8.16' [profile.sec_council_mgmt] -optimizer_runs = 750 +optimizer_runs = 500 [fmt] number_underscore = 'thousands' diff --git a/hardhat.config.ts b/hardhat.config.ts index 7b6985b3a..85851ddda 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -19,7 +19,7 @@ const solidityProfiles = { settings: { optimizer: { enabled: true, - runs: 750 + runs: 500 }, } } diff --git a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol index 688eaac3e..193dd76f3 100644 --- a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol +++ b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol @@ -35,7 +35,7 @@ contract SecurityCouncilNomineeElectionGovernor is /// @param owner Owner of the governor (the Arbitrum DAO) /// @param quorumNumeratorValue Numerator of the quorum fraction (0.2% = 20) /// @param votingPeriod Duration of the voting period (expressed in blocks) - /// Note that the voting period + nominee vetting duration must be << than 6 months to ensure elections dont overlap + /// Note that the voting period + nominee vetting duration must be << than the set cadence (`cadenceInMonths`) to ensure elections dont overlap struct InitParams { Date firstNominationStartDate; uint256 nomineeVettingDuration; @@ -101,6 +101,23 @@ contract SecurityCouncilNomineeElectionGovernor is _disableInitializers(); } + function getProxyAdmin() internal view returns (address admin) { + // https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v3.4.0/contracts/proxy/TransparentUpgradeableProxy.sol#L48 + // Storage slot with the admin of the proxy contract. + // This is the keccak-256 hash of "eip1967.proxy.admin" subtracted by 1, and is + bytes32 slot = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103; + assembly { + admin := sload(slot) + } + } + + function postUpgradeInit() external { + require(msg.sender == getProxyAdmin(), "NOT_FROM_ADMIN"); + if (cadenceInMonths == 0) { + cadenceInMonths = 6; + } + } + /// @notice Initializes the governor function initialize(InitParams memory params) public initializer { __Governor_init("SecurityCouncilNomineeElectionGovernor"); @@ -158,7 +175,7 @@ contract SecurityCouncilNomineeElectionGovernor is } /// @notice Creates a new nominee election proposal. - /// Can be called by anyone every 6 months. + /// Can be called by anyone every `cadenceInMonths` months. /// @return proposalId The id of the proposal function createElection() external returns (uint256 proposalId) { // require that the last member election has executed @@ -236,7 +253,7 @@ contract SecurityCouncilNomineeElectionGovernor is // this only checks against the current the current other cohort, and against the current cohort membership // in the security council, so changes to those will mean this check will be inconsistent. // this check then is only a relevant check when the elections are running as expected - one at a time, - // every 6 months. Updates to the sec council manager using methods other than replaceCohort can effect this check + // every `cadenceInMonths` months. Updates to the sec council manager using methods other than replaceCohort can effect this check // and it's expected that the entity making those updates understands this. if (securityCouncilManager.cohortIncludes(otherCohort(), signer)) { revert AccountInOtherCohort(otherCohort(), signer); @@ -270,6 +287,12 @@ contract SecurityCouncilNomineeElectionGovernor is AddressUpgradeable.functionCallWithValue(target, data, value); } + /// @notice Set the cadence for future elections + /// @param numberOfMonths The new cadence in months (must be >= 1) + function setCadence(uint256 numberOfMonths) external onlyGovernance { + _setCadence(numberOfMonths, electionCount); + } + /// @notice Allows the nomineeVetter to exclude a noncompliant nominee. /// @dev Can be called only after a nominee election proposal has "succeeded" (voting has ended) and before the nominee vetting period has ended. /// Will revert if the provided account is not a nominee (had less than the required votes). @@ -321,7 +344,7 @@ contract SecurityCouncilNomineeElectionGovernor is // this only checks against the current the current other cohort, and against the current cohort membership // in the security council, so changes to those will mean this check will be inconsistent. // this check then is only a relevant check when the elections are running as expected - one at a time, - // every 6 months. Updates to the sec council manager using methods other than replaceCohort can effect this check + // every `cadenceInMonths` months. Updates to the sec council manager using methods other than replaceCohort can effect this check // and it's expected that the entity making those updates understands this. if (securityCouncilManager.cohortIncludes(otherCohort(), account)) { revert AccountInOtherCohort(otherCohort(), account); diff --git a/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol b/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol index c8fd05662..e17de3fbd 100644 --- a/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol +++ b/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol @@ -13,15 +13,28 @@ abstract contract SecurityCouncilNomineeElectionGovernorTiming is Initializable, GovernorUpgradeable { - /// @notice First election start date + /// @notice This is the first election start date only if the first election is yet to be created Date public firstNominationStartDate; /// @notice Duration of the nominee vetting period (expressed in blocks) /// @dev This is the amount of time after voting ends that the nomineeVetter can exclude noncompliant nominees uint256 public nomineeVettingDuration; + /// @notice The cadence of elections in months + uint256 public cadenceInMonths; + + event CadenceChanged( + uint256 newCadence, + uint256 nextElectionYear, + uint256 nextElectionMonth, + uint256 nextElectionDay, + uint256 nextElectionHour + ); + error InvalidStartDate(uint256 year, uint256 month, uint256 day, uint256 hour); error StartDateTooEarly(uint256 startTime, uint256 currentTime); + error InvalidCadence(uint256 cadence); + error NextElectionTooSoon(uint256 nextElectionTimestamp, uint256 currentTimestamp); /// @notice Initialize the timing module /// @dev Checks to make sure the start date is in the future and is valid @@ -63,6 +76,7 @@ abstract contract SecurityCouncilNomineeElectionGovernorTiming is firstNominationStartDate = _firstNominationStartDate; nomineeVettingDuration = _nomineeVettingDuration; + cadenceInMonths = 6; // Default to 6 months } /// @notice Deadline for the nominee vetting period for a given `proposalId` @@ -70,13 +84,71 @@ abstract contract SecurityCouncilNomineeElectionGovernorTiming is return proposalDeadline(proposalId) + nomineeVettingDuration; } + /// @notice Set the cadence for future elections + /// @param numberOfMonths The new cadence in months (must be >= 1) + /// @param currentElectionCount The current number of elections + /// @dev Internal function to be called by the main governor contract + function _setCadence(uint256 numberOfMonths, uint256 currentElectionCount) internal { + if (numberOfMonths == 0) { + revert InvalidCadence(numberOfMonths); + } + + // If no elections have been created yet, just update the cadence + if (currentElectionCount == 0) { + cadenceInMonths = numberOfMonths; + emit CadenceChanged( + numberOfMonths, + firstNominationStartDate.year, + firstNominationStartDate.month, + firstNominationStartDate.day, + firstNominationStartDate.hour + ); + return; + } + + // Calculate what the next election timestamp should be (last + new cadence) + uint256 nextElectionTimestamp; + { + // Calculate the timestamp of the last election + uint256 lastElectionTimestamp = electionToTimestamp(currentElectionCount - 1); + + nextElectionTimestamp = DateTimeLib.addMonths(lastElectionTimestamp, numberOfMonths); + (uint256 _year, uint256 _month, uint256 _day, uint256 _hour,,) = + DateTimeLib.timestampToDateTime(nextElectionTimestamp); + + // we emit the event here to save some stack space + emit CadenceChanged(numberOfMonths, _year, _month, _day, _hour); + } + + // Ensure the next election won't be moved to the past + if (nextElectionTimestamp < block.timestamp) { + revert NextElectionTooSoon(nextElectionTimestamp, block.timestamp); + } + + // Calculate the new firstNominationStartDate that would make election at currentElectionCount + // occur at nextElectionTimestamp with the new cadence + // nextElectionTimestamp = newFirstDate + (currentElectionCount * numberOfMonths) + // So: newFirstDate = nextElectionTimestamp - (currentElectionCount * numberOfMonths) + + // Work backwards from the next election timestamp + uint256 monthsToSubtract = numberOfMonths * currentElectionCount; + uint256 offsetTimestamp = DateTimeLib.subMonths(nextElectionTimestamp, monthsToSubtract); + (uint256 year, uint256 month, uint256 day, uint256 hour,,) = + DateTimeLib.timestampToDateTime(offsetTimestamp); + + // Update the firstNominationStartDate and cadence + firstNominationStartDate = Date({year: year, month: month, day: day, hour: hour}); + cadenceInMonths = numberOfMonths; + } + /// @notice Start timestamp of an election + /// Only returns accurate timestamps for the last and upcoming elections after cadence changes /// @param electionIndex The index of the election function electionToTimestamp(uint256 electionIndex) public view returns (uint256) { // subtract one to make month 0 indexed uint256 month = firstNominationStartDate.month - 1; - month += 6 * electionIndex; + month += cadenceInMonths * electionIndex; uint256 year = firstNominationStartDate.year + month / 12; month = month % 12; @@ -98,5 +170,5 @@ abstract contract SecurityCouncilNomineeElectionGovernorTiming is * variables without shifting down storage in the inheritance chain. * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps */ - uint256[45] private __gap; + uint256[44] private __gap; } diff --git a/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol b/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol index c8a086195..2691c0cd5 100644 --- a/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol +++ b/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol @@ -870,8 +870,11 @@ contract SecurityCouncilNomineeElectionGovernorTest is Test { pure returns (uint256) { + uint256 year = months / 12; + months = months % 12; + return DateTimeLib.dateTimeToTimestamp({ - year: date.year, + year: date.year + year, month: date.month + months, day: date.day, hour: date.hour, @@ -996,4 +999,160 @@ contract SecurityCouncilNomineeElectionGovernorTest is Test { ) ); } + + function testDefaultCadence() public { + assertEq(governor.cadenceInMonths(), 6, "Default cadence should be 6 months"); + } + + function testSetCadenceBeforeFirstElection() public { + vm.prank(initParams.owner); + governor.relay( + address(governor), 0, abi.encodeWithSelector(governor.setCadence.selector, 3) + ); + + assertEq(governor.cadenceInMonths(), 3, "Cadence should be updated to 3 months"); + } + + function testSetCadenceInvalidValue() public { + vm.prank(initParams.owner); + vm.expectRevert( + abi.encodeWithSelector( + SecurityCouncilNomineeElectionGovernorTiming.InvalidCadence.selector, 0 + ) + ); + governor.relay( + address(governor), 0, abi.encodeWithSelector(governor.setCadence.selector, 0) + ); + } + + function testSetCadenceOnlyOwner() public { + address nonOwner = address(0x1234); + vm.prank(nonOwner); + vm.expectRevert("Governor: onlyGovernance"); + governor.setCadence(3); + } + + function testElectionTimestampsWithDefaultCadence() public { + uint256 secondElectionTime = governor.electionToTimestamp(1); + uint256 thirdElectionTime = governor.electionToTimestamp(2); + + // Check that elections are properly spaced + // First election: Jan 1, 2030 + // Second election: Jul 1, 2030 (6 months later) + // Third election: Jan 1, 2031 (6 months later) + + // The actual timestamps depend on the exact calendar calculation + uint256 expectedSecondTime = + _datePlusMonthsToTimestamp(initParams.firstNominationStartDate, 6); + uint256 expectedThirdTime = + _datePlusMonthsToTimestamp(initParams.firstNominationStartDate, 12); + + assertEq( + secondElectionTime, expectedSecondTime, "Second election should be 6 months after first" + ); + assertEq( + thirdElectionTime, expectedThirdTime, "Third election should be 12 months after first" + ); + } + + function testSetCadenceAfterElections() public { + // Create first election + _propose(); + + // Fast forward and create second election + vm.warp(_datePlusMonthsToTimestamp(initParams.firstNominationStartDate, 6)); + vm.prank(proposer); + governor.createElection(); + + // Now change cadence to 3 months + vm.prank(initParams.owner); + governor.relay( + address(governor), 0, abi.encodeWithSelector(governor.setCadence.selector, 3) + ); + + assertEq(governor.cadenceInMonths(), 3, "Cadence should be updated to 3 months"); + + // The next election (index 2) should be 3 months after the last one (index 1) + uint256 nextElectionTime = governor.electionToTimestamp(2); + + // Should be approximately 3 months + uint256 expectedTime = _datePlusMonthsToTimestamp( + Date({ + year: 2030, + month: 7, // January + 6 months + day: 1, + hour: 0 + }), + 3 + ); + assertEq(nextElectionTime, expectedTime, "Next election should follow new cadence"); + } + + function testSetCadenceTooSoonReverts() public { + // Create first election + _propose(); + + // Fast forward to near the end of the 6-month period + vm.warp(_datePlusMonthsToTimestamp(initParams.firstNominationStartDate, 6) - 1 days); + + // Try to set cadence to 1 month - this would make next election in the past + vm.prank(initParams.owner); + vm.expectRevert(); + governor.relay( + address(governor), 0, abi.encodeWithSelector(governor.setCadence.selector, 1) + ); + } + + function testMultipleCadenceChanges() public { + // Create first election with default 6-month cadence + _propose(); + + // Change to 4 months + vm.prank(initParams.owner); + governor.relay( + address(governor), 0, abi.encodeWithSelector(governor.setCadence.selector, 4) + ); + + // Fast forward and create second election + vm.warp(_datePlusMonthsToTimestamp(initParams.firstNominationStartDate, 4)); + vm.prank(proposer); + governor.createElection(); + + // Change to 2 months + vm.prank(initParams.owner); + governor.relay( + address(governor), 0, abi.encodeWithSelector(governor.setCadence.selector, 2) + ); + + // Verify the third election timing + uint256 thirdElectionTime = governor.electionToTimestamp(2); + + // Should be 2 months after the second election + uint256 expectedTime = _datePlusMonthsToTimestamp( + Date({ + year: 2030, + month: 5, // January + 4 months + day: 1, + hour: 0 + }), + 2 + ); + assertEq(thirdElectionTime, expectedTime, "Third election should follow newest cadence"); + } + + function testCadenceWithLargeValues() public { + vm.prank(initParams.owner); + governor.relay( + address(governor), 0, abi.encodeWithSelector(governor.setCadence.selector, 36) + ); + + uint256 secondElection = governor.electionToTimestamp(1); + + // First election: Jan 1, 2030 + // Second election: Jan 1, 2033 (36 months later) + uint256 expectedSecondTime = + _datePlusMonthsToTimestamp(initParams.firstNominationStartDate, 36); + + assertEq(secondElection, expectedSecondTime, "Elections should be 36 months apart"); + } } diff --git a/test/signatures/SecurityCouncilNomineeElectionGovernor b/test/signatures/SecurityCouncilNomineeElectionGovernor index 4e192dc98..2f3f77cab 100644 --- a/test/signatures/SecurityCouncilNomineeElectionGovernor +++ b/test/signatures/SecurityCouncilNomineeElectionGovernor @@ -14,6 +14,8 @@ |-----------------------------------------------------------------------------------------------------------------+------------| | addContender(uint256,bytes) | a8f38759 | |-----------------------------------------------------------------------------------------------------------------+------------| +| cadenceInMonths() | e182a4cd | +|-----------------------------------------------------------------------------------------------------------------+------------| | castVote(uint256,uint8) | 56781388 | |-----------------------------------------------------------------------------------------------------------------+------------| | castVoteBySig(uint256,uint8,uint8,bytes32,bytes32) | 3bccf4fd | @@ -92,6 +94,8 @@ |-----------------------------------------------------------------------------------------------------------------+------------| | owner() | 8da5cb5b | |-----------------------------------------------------------------------------------------------------------------+------------| +| postUpgradeInit() | 95fcea78 | +|-----------------------------------------------------------------------------------------------------------------+------------| | proposalDeadline(uint256) | c01f9e37 | |-----------------------------------------------------------------------------------------------------------------+------------| | proposalSnapshot(uint256) | 2d63f693 | @@ -120,6 +124,8 @@ |-----------------------------------------------------------------------------------------------------------------+------------| | securityCouncilMemberElectionGovernor() | 1b6a7673 | |-----------------------------------------------------------------------------------------------------------------+------------| +| setCadence(uint256) | 5ad525b1 | +|-----------------------------------------------------------------------------------------------------------------+------------| | setNomineeVetter(address) | ae8acb5e | |-----------------------------------------------------------------------------------------------------------------+------------| | setProposalThreshold(uint256) | ece40cc1 | diff --git a/test/storage/SecurityCouncilNomineeElectionGovernor b/test/storage/SecurityCouncilNomineeElectionGovernor index 235fb7592..95df9dcec 100644 --- a/test/storage/SecurityCouncilNomineeElectionGovernor +++ b/test/storage/SecurityCouncilNomineeElectionGovernor @@ -58,7 +58,9 @@ |---------------------------------------+------------------------------------------------------------------------------------------------------------------+------+--------+-------+-----------------------------------------------------------------------------------------------------------------------| | nomineeVettingDuration | uint256 | 558 | 0 | 32 | src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol:SecurityCouncilNomineeElectionGovernor | |---------------------------------------+------------------------------------------------------------------------------------------------------------------+------+--------+-------+-----------------------------------------------------------------------------------------------------------------------| -| __gap | uint256[45] | 559 | 0 | 1440 | src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol:SecurityCouncilNomineeElectionGovernor | +| cadenceInMonths | uint256 | 559 | 0 | 32 | src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol:SecurityCouncilNomineeElectionGovernor | +|---------------------------------------+------------------------------------------------------------------------------------------------------------------+------+--------+-------+-----------------------------------------------------------------------------------------------------------------------| +| __gap | uint256[44] | 560 | 0 | 1408 | src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol:SecurityCouncilNomineeElectionGovernor | |---------------------------------------+------------------------------------------------------------------------------------------------------------------+------+--------+-------+-----------------------------------------------------------------------------------------------------------------------| | usedNonces | mapping(bytes32 => bool) | 604 | 0 | 32 | src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol:SecurityCouncilNomineeElectionGovernor | |---------------------------------------+------------------------------------------------------------------------------------------------------------------+------+--------+-------+-----------------------------------------------------------------------------------------------------------------------| From 049d47b966b0544078103efc946e834bb293afab Mon Sep 17 00:00:00 2001 From: gzeon Date: Wed, 13 Aug 2025 01:05:20 +0800 Subject: [PATCH 3/4] feat: SecurityCouncilUpgradeAction (#357) * feat: setCadence * test: setCadence * chore: update misc * test: 36 months * feat: CadenceChanged event * refactor: move CadenceChanged to Timing abstract contract * update optimizer runs * refactor: cut some size * refactor: equal 0 * fix: return * docs: currentElectionCount * refactor: use solady add and subMonths * chore: reduce sec_council_mgmt opt-run to 500 due to contract size * refactor: rename to SecurityCouncilUpgradeAction * feat: SecurityCouncilUpgradeAction * chore: cleanup lint warning * chore: storage * chore: 4bytes * feat: more validations --------- Co-authored-by: Henry <11198460+godzillaba@users.noreply.github.com> --- .gas-snapshot | 2 +- .../RotateMembersUpgradeAction.sol | 49 -------- .../SecurityCouncilUpgradeAction.sol | 113 ++++++++++++++++++ ...celTimelockAndRemoveMemberActionTest.t.sol | 23 +++- ...sol => SecurityCouncilUpgradeAction.t.sol} | 30 ++++- test/signatures/RotateMembersUpgradeAction | 15 --- test/signatures/SecurityCouncilUpgradeAction | 21 ++++ ...adeAction => SecurityCouncilUpgradeAction} | 0 8 files changed, 180 insertions(+), 73 deletions(-) delete mode 100644 src/gov-action-contracts/AIPs/SecurityCouncilMgmt/RotateMembersUpgradeAction.sol create mode 100644 src/gov-action-contracts/AIPs/SecurityCouncilMgmt/SecurityCouncilUpgradeAction.sol rename test/gov-actions/{RotateMembersUpgradeAction.t.sol => SecurityCouncilUpgradeAction.t.sol} (71%) delete mode 100644 test/signatures/RotateMembersUpgradeAction create mode 100644 test/signatures/SecurityCouncilUpgradeAction rename test/storage/{RotateMembersUpgradeAction => SecurityCouncilUpgradeAction} (100%) diff --git a/.gas-snapshot b/.gas-snapshot index 9e401fed3..73e400efb 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -117,7 +117,6 @@ OutboxActionsTest:testRemoveAllOutboxes() (gas: 693079) OutboxActionsTest:testRemoveOutboxes() (gas: 853972) ProxyUpgradeAndCallActionTest:testUpgrade() (gas: 137140) ProxyUpgradeAndCallActionTest:testUpgradeAndCall() (gas: 143087) -RotateMembersUpgradeActionTest:testAction() (gas: 8153) SecurityCouncilManagerTest:testAddMemberAffordances() (gas: 253923) SecurityCouncilManagerTest:testAddMemberSpecialAddresses() (gas: 20770) SecurityCouncilManagerTest:testAddMemberToFirstCohort() (gas: 349222) @@ -221,6 +220,7 @@ SecurityCouncilNomineeElectionGovernorTest:testSetCadenceInvalidValue() (gas: 26 SecurityCouncilNomineeElectionGovernorTest:testSetCadenceOnlyOwner() (gas: 16089) SecurityCouncilNomineeElectionGovernorTest:testSetCadenceTooSoonReverts() (gas: 148066) SecurityCouncilNomineeElectionGovernorTest:testSetNomineeVetter() (gas: 40001) +SecurityCouncilUpgradeActionTest:testAction() (gas: 8153) SequencerActionsTest:testAddAndRemoveSequencer() (gas: 486652) SequencerActionsTest:testCantAddZeroAddress() (gas: 235659) SetInitialGovParamsActionTest:testL1() (gas: 259949) diff --git a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/RotateMembersUpgradeAction.sol b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/RotateMembersUpgradeAction.sol deleted file mode 100644 index 9bb1df902..000000000 --- a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/RotateMembersUpgradeAction.sol +++ /dev/null @@ -1,49 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -pragma solidity 0.8.16; - -import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; -import "../../address-registries/L2AddressRegistryInterfaces.sol"; -import "@openzeppelin/contracts-upgradeable/access/IAccessControlUpgradeable.sol"; - -/// @notice Upgrades the sec council manager to allow member rotation and sets min rotation vars -contract RotateMembersUpgradeAction { - IL2AddressRegistry public immutable l2AddressRegistry; - address public immutable secCouncilManagerImpl; - uint256 public immutable minRotationPeriod; - address public immutable minRotationPeriodSetter; - - constructor( - IL2AddressRegistry _l2AddressRegistry, - address _secCouncilManagerImpl, - uint256 _minRotationPeriod, - address _minRotationPeriodSetter - ) { - l2AddressRegistry = _l2AddressRegistry; - secCouncilManagerImpl = _secCouncilManagerImpl; - minRotationPeriod = _minRotationPeriod; - minRotationPeriodSetter = _minRotationPeriodSetter; - } - - function perform() external { - ISecurityCouncilManager secCouncilManager = l2AddressRegistry.securityCouncilManager(); - l2AddressRegistry.govProxyAdmin().upgradeAndCall( - TransparentUpgradeableProxy(payable(address(secCouncilManager))), - secCouncilManagerImpl, - abi.encodeCall( - ISecurityCouncilManager(secCouncilManagerImpl).postUpgradeInit, - (minRotationPeriod, minRotationPeriodSetter) - ) - ); - - require( - minRotationPeriod == secCouncilManager.minRotationPeriod(), - "RotateMembersUpgradeAction: Min rotation period not set" - ); - require( - IAccessControlUpgradeable(address(secCouncilManager)).hasRole( - secCouncilManager.MIN_ROTATION_PERIOD_SETTER_ROLE(), minRotationPeriodSetter - ), - "RotateMembersUpgradeAction: Min rotation period setter not set" - ); - } -} diff --git a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/SecurityCouncilUpgradeAction.sol b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/SecurityCouncilUpgradeAction.sol new file mode 100644 index 000000000..e289dffde --- /dev/null +++ b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/SecurityCouncilUpgradeAction.sol @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.16; + +import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; +import "../../address-registries/L2AddressRegistryInterfaces.sol"; +import "@openzeppelin/contracts-upgradeable/access/IAccessControlUpgradeable.sol"; +import + "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorVotesQuorumFractionUpgradeable.sol"; +import "../../../security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol"; + +/// @notice Perform the following upgrade proposed by the Arbitrum Foundation: +/// - Upgrade the sec council manager to allow member rotation and sets min rotation vars +/// - Upgrade the sec council nominee election governor to allow modifying the cadence of election +/// - Adjusting the qualification threshold of the Member Election phase from 0.2% to 0.1% +/// - Allowing existing sec council members to automatically progress from the Nominee Selection phase +/// - Updating the ArbitrumDAO Constitution to reflect these changes +contract SecurityCouncilUpgradeAction { + IL2AddressRegistry public immutable l2AddressRegistry; + address public immutable secCouncilManagerImpl; + address public immutable scNomineeElectionGovernorImpl; + uint256 public immutable minRotationPeriod; + address public immutable minRotationPeriodSetter; + uint256 public immutable cadenceInMonths; + bytes32 public immutable newConstitutionHash; + + constructor( + IL2AddressRegistry _l2AddressRegistry, + address _secCouncilManagerImpl, + address _scNomineeElectionGovernorImpl, + uint256 _minRotationPeriod, + address _minRotationPeriodSetter, + uint256 _cadenceInMonths, + bytes32 _newConstitutionHash + ) { + l2AddressRegistry = _l2AddressRegistry; + secCouncilManagerImpl = _secCouncilManagerImpl; + scNomineeElectionGovernorImpl = _scNomineeElectionGovernorImpl; + minRotationPeriod = _minRotationPeriod; + minRotationPeriodSetter = _minRotationPeriodSetter; + cadenceInMonths = _cadenceInMonths; + newConstitutionHash = _newConstitutionHash; + } + + function perform() external { + SecurityCouncilNomineeElectionGovernor scNomineeElectionGovernor = + SecurityCouncilNomineeElectionGovernor( + payable(address(l2AddressRegistry.scNomineeElectionGovernor())) + ); + require( + scNomineeElectionGovernor.electionCount() == 5, + "SecurityCouncilUpgradeAction: not expected timing" + ); + + // Upgrade the sec council manager to allow member rotation and sets min rotation vars + ISecurityCouncilManager secCouncilManager = l2AddressRegistry.securityCouncilManager(); + l2AddressRegistry.govProxyAdmin().upgradeAndCall( + TransparentUpgradeableProxy(payable(address(secCouncilManager))), + secCouncilManagerImpl, + abi.encodeCall( + ISecurityCouncilManager(secCouncilManagerImpl).postUpgradeInit, + (minRotationPeriod, minRotationPeriodSetter) + ) + ); + require( + minRotationPeriod == secCouncilManager.minRotationPeriod(), + "SecurityCouncilUpgradeAction: Min rotation period not set" + ); + require( + IAccessControlUpgradeable(address(secCouncilManager)).hasRole( + secCouncilManager.MIN_ROTATION_PERIOD_SETTER_ROLE(), minRotationPeriodSetter + ), + "SecurityCouncilUpgradeAction: Min rotation period setter not set" + ); + + // Upgrade the sec council nominee election governor to allow modifying the cadence of election + // Allowing existing sec council members to automatically progress from the Nominee Selection phase + l2AddressRegistry.govProxyAdmin().upgradeAndCall( + TransparentUpgradeableProxy(payable(address(scNomineeElectionGovernor))), + scNomineeElectionGovernorImpl, + abi.encodeCall(scNomineeElectionGovernor.postUpgradeInit, ()) + ); + + scNomineeElectionGovernor.relay( + address(scNomineeElectionGovernor), + 0, + abi.encodeCall(scNomineeElectionGovernor.setCadence, (cadenceInMonths)) + ); + require( + scNomineeElectionGovernor.cadenceInMonths() == cadenceInMonths, + "SecurityCouncilUpgradeAction: Cadence not set" + ); + + // Adjusting the qualification threshold of the Member Election phase from 0.2% to 0.1% + scNomineeElectionGovernor.relay( + address(scNomineeElectionGovernor), + 0, + abi.encodeCall(scNomineeElectionGovernor.updateQuorumNumerator, (10)) + ); + require( + scNomineeElectionGovernor.quorumNumerator() == 10, + "SecurityCouncilUpgradeAction: Quorum numerator not set" + ); + + // Updating the ArbitrumDAO Constitution to reflect these changes + IArbitrumDAOConstitution arbitrumDaoConstitution = + l2AddressRegistry.arbitrumDAOConstitution(); + arbitrumDaoConstitution.setConstitutionHash(newConstitutionHash); + require( + arbitrumDaoConstitution.constitutionHash() == newConstitutionHash, + "SecurityCouncilUpgradeAction: new constitution hash not set" + ); + } +} diff --git a/test/gov-actions/CancelTimelockAndRemoveMemberActionTest.t.sol b/test/gov-actions/CancelTimelockAndRemoveMemberActionTest.t.sol index 3bc65b930..11de8e816 100644 --- a/test/gov-actions/CancelTimelockAndRemoveMemberActionTest.t.sol +++ b/test/gov-actions/CancelTimelockAndRemoveMemberActionTest.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.16; import "forge-std/Test.sol"; -import "../../src/gov-action-contracts/AIPs/SecurityCouncilMgmt/RotateMembersUpgradeAction.sol"; +import "../../src/gov-action-contracts/AIPs/SecurityCouncilMgmt/SecurityCouncilUpgradeAction.sol"; import "../../src/gov-action-contracts/governance/CancelTimelockAndRemoveMemberAction.sol"; import "../../src/security-council-mgmt/SecurityCouncilManager.sol"; import "../../src/gov-action-contracts/address-registries/L2AddressRegistry.sol"; @@ -25,7 +25,7 @@ contract CancelTimelockAndRemoveMemberActionTest is Test { function testAction() external { if (!_isForkTest()) { - console.log("not fork test, skipping RotateMembersUpgradeActionTest"); + console.log("not fork test, skipping SecurityCouncilUpgradeActionTest"); return; } @@ -118,10 +118,25 @@ contract CancelTimelockAndRemoveMemberActionTest is Test { function ensureLatestScm(L2AddressRegistry reg) internal { address newImplementation = address(new SecurityCouncilManager()); + address newNomineeElectionGovernorImplementation = + address(new SecurityCouncilNomineeElectionGovernor()); address rotationSetter = address(1337); uint256 minRotationPeriod = 1 weeks; - RotateMembersUpgradeAction action = new RotateMembersUpgradeAction( - reg, newImplementation, minRotationPeriod, rotationSetter + uint256 cadenceInMonths = 12; + + SecurityCouncilNomineeElectionGovernor scNomineeElectionGovernor = + SecurityCouncilNomineeElectionGovernor(payable(address(reg.scNomineeElectionGovernor()))); + vm.warp(1_757_937_601); // After the 2025 Sep election + scNomineeElectionGovernor.createElection(); + + SecurityCouncilUpgradeAction action = new SecurityCouncilUpgradeAction( + reg, + newImplementation, + newNomineeElectionGovernorImplementation, + minRotationPeriod, + rotationSetter, + cadenceInMonths, + bytes32(0) ); vm.prank(council); arbOneUe.execute(address(action), abi.encodeWithSelector(action.perform.selector)); diff --git a/test/gov-actions/RotateMembersUpgradeAction.t.sol b/test/gov-actions/SecurityCouncilUpgradeAction.t.sol similarity index 71% rename from test/gov-actions/RotateMembersUpgradeAction.t.sol rename to test/gov-actions/SecurityCouncilUpgradeAction.t.sol index 6e20a26e1..56136357f 100644 --- a/test/gov-actions/RotateMembersUpgradeAction.t.sol +++ b/test/gov-actions/SecurityCouncilUpgradeAction.t.sol @@ -3,11 +3,12 @@ pragma solidity 0.8.16; import "forge-std/Test.sol"; -import "../../src/gov-action-contracts/AIPs/SecurityCouncilMgmt/RotateMembersUpgradeAction.sol"; +import "../../src/gov-action-contracts/AIPs/SecurityCouncilMgmt/SecurityCouncilUpgradeAction.sol"; import "../../src/security-council-mgmt/SecurityCouncilManager.sol"; +import "../../src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol"; import "../../src/gov-action-contracts/address-registries/L2AddressRegistry.sol"; -contract RotateMembersUpgradeActionTest is Test { +contract SecurityCouncilUpgradeActionTest is Test { SecurityCouncilManager scm = SecurityCouncilManager(0xD509E5f5aEe2A205F554f36E8a7d56094494eDFC); address oldImplementation = 0x468dA0eE5570Bdb1Dd81bFd925BAf028A93Dce64; ProxyAdmin proxyAdmin = ProxyAdmin(0xdb216562328215E010F819B5aBe947bad4ca961e); @@ -46,11 +47,25 @@ contract RotateMembersUpgradeActionTest is Test { ISecurityCouncilNomineeElectionGovernor(0x8a1cDA8dee421cD06023470608605934c16A05a0) ); + SecurityCouncilNomineeElectionGovernor scNomineeElectionGovernor = + SecurityCouncilNomineeElectionGovernor(payable(address(reg.scNomineeElectionGovernor()))); + vm.warp(1_757_937_601); // After the 2025 Sep election + scNomineeElectionGovernor.createElection(); + address newImplementation = address(new SecurityCouncilManager()); + address newNomineeElectionGovernorImplementation = + address(new SecurityCouncilNomineeElectionGovernor()); address rotationSetter = address(137); uint256 minRotationPeriod = 1 weeks; - RotateMembersUpgradeAction action = new RotateMembersUpgradeAction( - reg, newImplementation, minRotationPeriod, rotationSetter + uint256 cadenceInMonths = 12; + SecurityCouncilUpgradeAction action = new SecurityCouncilUpgradeAction( + reg, + newImplementation, + newNomineeElectionGovernorImplementation, + minRotationPeriod, + rotationSetter, + cadenceInMonths, + bytes32(0) ); vm.prank(council); arbOneUe.execute(address(action), abi.encodeWithSelector(action.perform.selector)); @@ -63,6 +78,13 @@ contract RotateMembersUpgradeActionTest is Test { "Min rotation period setter not set" ); assertEq(_getImplementation(), newImplementation, "implementation not set"); + + uint256 electionCount = scNomineeElectionGovernor.electionCount(); + assertEq( + scNomineeElectionGovernor.electionToTimestamp(electionCount), + 1_789_473_600, + "not September 15, 2026 12:00:00 PM" + ); } function _getImplementation() internal view returns (address) { diff --git a/test/signatures/RotateMembersUpgradeAction b/test/signatures/RotateMembersUpgradeAction deleted file mode 100644 index d465c78f0..000000000 --- a/test/signatures/RotateMembersUpgradeAction +++ /dev/null @@ -1,15 +0,0 @@ - -╭---------------------------+------------╮ -| Method | Identifier | -+========================================+ -| l2AddressRegistry() | 9b491216 | -|---------------------------+------------| -| minRotationPeriod() | cfc02946 | -|---------------------------+------------| -| minRotationPeriodSetter() | 7a5c8992 | -|---------------------------+------------| -| perform() | b147f40c | -|---------------------------+------------| -| secCouncilManagerImpl() | a03700cc | -╰---------------------------+------------╯ - diff --git a/test/signatures/SecurityCouncilUpgradeAction b/test/signatures/SecurityCouncilUpgradeAction new file mode 100644 index 000000000..33891359d --- /dev/null +++ b/test/signatures/SecurityCouncilUpgradeAction @@ -0,0 +1,21 @@ + +╭---------------------------------+------------╮ +| Method | Identifier | ++==============================================+ +| cadenceInMonths() | e182a4cd | +|---------------------------------+------------| +| l2AddressRegistry() | 9b491216 | +|---------------------------------+------------| +| minRotationPeriod() | cfc02946 | +|---------------------------------+------------| +| minRotationPeriodSetter() | 7a5c8992 | +|---------------------------------+------------| +| newConstitutionHash() | 8035cce0 | +|---------------------------------+------------| +| perform() | b147f40c | +|---------------------------------+------------| +| scNomineeElectionGovernorImpl() | c91461be | +|---------------------------------+------------| +| secCouncilManagerImpl() | a03700cc | +╰---------------------------------+------------╯ + diff --git a/test/storage/RotateMembersUpgradeAction b/test/storage/SecurityCouncilUpgradeAction similarity index 100% rename from test/storage/RotateMembersUpgradeAction rename to test/storage/SecurityCouncilUpgradeAction From 7c8369586bf6086352afabd11438b36c2909f0af Mon Sep 17 00:00:00 2001 From: gzeon Date: Thu, 14 Aug 2025 16:34:37 +0800 Subject: [PATCH 4/4] feat: rotateNominee (#358) * feat: setCadence * test: setCadence * chore: update misc * test: 36 months * feat: CadenceChanged event * refactor: move CadenceChanged to Timing abstract contract * update optimizer runs * refactor: cut some size * refactor: equal 0 * fix: return * docs: currentElectionCount * refactor: use solady add and subMonths * chore: reduce sec_council_mgmt opt-run to 500 due to contract size * refactor: rename to SecurityCouncilUpgradeAction * feat: SecurityCouncilUpgradeAction * chore: cleanup lint warning * chore: storage * chore: 4bytes * feat: more validations * revert: rotatingTo * feat: rotateNominee * chore: contract size reduction * refactor: use isCompliantNominee * fix: wrong sign * fix: rotationDeadline * test: rotateNominee * review fixes for "feat: rotateNominee" (#360) * misc review fixes * fix gas and sigs --------- Co-authored-by: Henry <11198460+godzillaba@users.noreply.github.com> --- .gas-snapshot | 113 ++++++++-------- foundry.toml | 2 +- hardhat.config.ts | 2 +- .../SecurityCouncilManager.sol | 50 +------- ...SecurityCouncilNomineeElectionGovernor.sol | 85 ++++++++++-- .../interfaces/ISecurityCouncilManager.sol | 19 --- .../SecurityCouncilManager.t.sol | 121 ------------------ ...curityCouncilNomineeElectionGovernor.t.sol | 94 ++++++++++++++ test/signatures/SecurityCouncilManager | 8 -- .../SecurityCouncilNomineeElectionGovernor | 6 +- test/storage/SecurityCouncilManager | 6 +- 11 files changed, 234 insertions(+), 272 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 73e400efb..59be04bce 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -27,7 +27,7 @@ ArbitrumVestingWalletTest:testDoesDeploy() (gas: 15971357) ArbitrumVestingWalletTest:testReleaseAffordance() (gas: 16008664) ArbitrumVestingWalletTest:testVestedAmountStart() (gas: 16074932) CancelTimelockAndRemoveMemberActionTest:testAction() (gas: 8159) -E2E:testE2E() (gas: 86859267) +E2E:testE2E() (gas: 86859495) FixedDelegateErc20WalletTest:testInit() (gas: 5822585) FixedDelegateErc20WalletTest:testInitZeroToken() (gas: 5816815) FixedDelegateErc20WalletTest:testTransfer() (gas: 5932228) @@ -95,11 +95,11 @@ L2GovernanceFactoryTest:testSanityCheckValues() (gas: 28571182) L2GovernanceFactoryTest:testSetMinDelay() (gas: 28519939) L2GovernanceFactoryTest:testSetMinDelayRevertsForCoreAddress() (gas: 28572810) L2GovernanceFactoryTest:testUpgraderCanCancel() (gas: 28812928) -L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 32400694) -L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 32404948) -L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 27343716) -L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32402925) -L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32424390) +L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 32322314) +L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 32326589) +L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 27265404) +L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32324545) +L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32345900) NomineeGovernorV2UpgradeActionTest:testAction() (gas: 8153) OfficeHoursActionTest:testConstructor() (gas: 9050) OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 256, μ: 317059, ~: 317184) @@ -117,36 +117,34 @@ OutboxActionsTest:testRemoveAllOutboxes() (gas: 693079) OutboxActionsTest:testRemoveOutboxes() (gas: 853972) ProxyUpgradeAndCallActionTest:testUpgrade() (gas: 137140) ProxyUpgradeAndCallActionTest:testUpgradeAndCall() (gas: 143087) -SecurityCouncilManagerTest:testAddMemberAffordances() (gas: 253923) +SecurityCouncilManagerTest:testAddMemberAffordances() (gas: 253879) SecurityCouncilManagerTest:testAddMemberSpecialAddresses() (gas: 20770) -SecurityCouncilManagerTest:testAddMemberToFirstCohort() (gas: 349222) -SecurityCouncilManagerTest:testAddMemberToSecondCohort() (gas: 352657) +SecurityCouncilManagerTest:testAddMemberToFirstCohort() (gas: 349200) +SecurityCouncilManagerTest:testAddMemberToSecondCohort() (gas: 352635) SecurityCouncilManagerTest:testAddSC() (gas: 118742) -SecurityCouncilManagerTest:testAddSCAffordances() (gas: 112296) -SecurityCouncilManagerTest:testCantUpdateCohortWithADup() (gas: 148550) -SecurityCouncilManagerTest:testCohortMethods() (gas: 137958) -SecurityCouncilManagerTest:testInitialization() (gas: 206820) -SecurityCouncilManagerTest:testPostUpgradeInit() (gas: 5255512) -SecurityCouncilManagerTest:testRemoveMember() (gas: 217503) -SecurityCouncilManagerTest:testRemoveMemberAffordances() (gas: 101612) -SecurityCouncilManagerTest:testRemoveMemberRotated() (gas: 423607) -SecurityCouncilManagerTest:testRemoveSCAffordances() (gas: 81486) -SecurityCouncilManagerTest:testRemoveSeC() (gas: 38435) -SecurityCouncilManagerTest:testReplaceCohortRotatingTo() (gas: 963734) -SecurityCouncilManagerTest:testReplaceMemberAffordances() (gas: 216337) -SecurityCouncilManagerTest:testReplaceMemberInFirstCohort() (gas: 266597) -SecurityCouncilManagerTest:testReplaceMemberInFirstCohortAfterRotation() (gas: 471812) -SecurityCouncilManagerTest:testReplaceMemberInSecondCohort() (gas: 479056) -SecurityCouncilManagerTest:testReplaceMemberInSecondCohortAfterRotation() (gas: 270188) -SecurityCouncilManagerTest:testRotateMember() (gas: 1016263) -SecurityCouncilManagerTest:testRotateMemberNotContender() (gas: 3893898) -SecurityCouncilManagerTest:testSetMinRotationPeriod() (gas: 65924) -SecurityCouncilManagerTest:testSetRotatingTo() (gas: 113048) -SecurityCouncilManagerTest:testUpdateCohortAffordances() (gas: 83230) -SecurityCouncilManagerTest:testUpdateFirstCohort() (gas: 327741) -SecurityCouncilManagerTest:testUpdateRouter() (gas: 76429) -SecurityCouncilManagerTest:testUpdateRouterAffordances() (gas: 112452) -SecurityCouncilManagerTest:testUpdateSecondCohort() (gas: 327845) +SecurityCouncilManagerTest:testAddSCAffordances() (gas: 112428) +SecurityCouncilManagerTest:testCantUpdateCohortWithADup() (gas: 136633) +SecurityCouncilManagerTest:testCohortMethods() (gas: 137890) +SecurityCouncilManagerTest:testInitialization() (gas: 206665) +SecurityCouncilManagerTest:testPostUpgradeInit() (gas: 5000887) +SecurityCouncilManagerTest:testRemoveMember() (gas: 217459) +SecurityCouncilManagerTest:testRemoveMemberAffordances() (gas: 101567) +SecurityCouncilManagerTest:testRemoveMemberRotated() (gas: 423573) +SecurityCouncilManagerTest:testRemoveSCAffordances() (gas: 81441) +SecurityCouncilManagerTest:testRemoveSeC() (gas: 38383) +SecurityCouncilManagerTest:testReplaceMemberAffordances() (gas: 216447) +SecurityCouncilManagerTest:testReplaceMemberInFirstCohort() (gas: 266641) +SecurityCouncilManagerTest:testReplaceMemberInFirstCohortAfterRotation() (gas: 471806) +SecurityCouncilManagerTest:testReplaceMemberInSecondCohort() (gas: 479028) +SecurityCouncilManagerTest:testReplaceMemberInSecondCohortAfterRotation() (gas: 270210) +SecurityCouncilManagerTest:testRotateMember() (gas: 1015787) +SecurityCouncilManagerTest:testRotateMemberNotContender() (gas: 4078679) +SecurityCouncilManagerTest:testSetMinRotationPeriod() (gas: 65814) +SecurityCouncilManagerTest:testUpdateCohortAffordances() (gas: 83252) +SecurityCouncilManagerTest:testUpdateFirstCohort() (gas: 313830) +SecurityCouncilManagerTest:testUpdateRouter() (gas: 76407) +SecurityCouncilManagerTest:testUpdateRouterAffordances() (gas: 112474) +SecurityCouncilManagerTest:testUpdateSecondCohort() (gas: 313924) SecurityCouncilMemberElectionGovernorTest:testCannotUseMoreVotesThanAvailable() (gas: 247018) SecurityCouncilMemberElectionGovernorTest:testCastBySig() (gas: 302873) SecurityCouncilMemberElectionGovernorTest:testCastBySigTwice() (gas: 266265) @@ -196,30 +194,31 @@ SecurityCouncilMemberSyncActionTest:testRemoveOne() (gas: 8086867) SecurityCouncilMemberSyncActionTest:testUpdateCohort() (gas: 8328313) SecurityCouncilMemberSyncActionTest:testUpdateCohort() (gas: 8329174) SecurityCouncilMgmtUtilsTests:testIsInArray() (gas: 2102) -SecurityCouncilNomineeElectionGovernorTest:testAddContender() (gas: 417954) -SecurityCouncilNomineeElectionGovernorTest:testCadenceWithLargeValues() (gas: 52875) -SecurityCouncilNomineeElectionGovernorTest:testCastBySig() (gas: 338629) -SecurityCouncilNomineeElectionGovernorTest:testCastBySigTwice() (gas: 301488) -SecurityCouncilNomineeElectionGovernorTest:testCastVoteReverts() (gas: 35323) -SecurityCouncilNomineeElectionGovernorTest:testCountVote() (gas: 593018) -SecurityCouncilNomineeElectionGovernorTest:testCreateElection() (gas: 257849) -SecurityCouncilNomineeElectionGovernorTest:testDefaultCadence() (gas: 14927) +SecurityCouncilNomineeElectionGovernorTest:testAddContender() (gas: 418329) +SecurityCouncilNomineeElectionGovernorTest:testCadenceWithLargeValues() (gas: 52898) +SecurityCouncilNomineeElectionGovernorTest:testCastBySig() (gas: 338828) +SecurityCouncilNomineeElectionGovernorTest:testCastBySigTwice() (gas: 301643) +SecurityCouncilNomineeElectionGovernorTest:testCastVoteReverts() (gas: 35303) +SecurityCouncilNomineeElectionGovernorTest:testCountVote() (gas: 593196) +SecurityCouncilNomineeElectionGovernorTest:testCreateElection() (gas: 257942) +SecurityCouncilNomineeElectionGovernorTest:testDefaultCadence() (gas: 14950) SecurityCouncilNomineeElectionGovernorTest:testElectionTimestampsWithDefaultCadence() (gas: 37625) -SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 461456) -SecurityCouncilNomineeElectionGovernorTest:testExecute() (gas: 679418) -SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 199664) -SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 678802) -SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7657451) -SecurityCouncilNomineeElectionGovernorTest:testMultipleCadenceChanges() (gas: 238823) -SecurityCouncilNomineeElectionGovernorTest:testProperInitialization() (gas: 78160) -SecurityCouncilNomineeElectionGovernorTest:testProposeFails() (gas: 19741) -SecurityCouncilNomineeElectionGovernorTest:testRelay() (gas: 42433) -SecurityCouncilNomineeElectionGovernorTest:testSetCadenceAfterElections() (gas: 227567) -SecurityCouncilNomineeElectionGovernorTest:testSetCadenceBeforeFirstElection() (gas: 42479) -SecurityCouncilNomineeElectionGovernorTest:testSetCadenceInvalidValue() (gas: 26010) -SecurityCouncilNomineeElectionGovernorTest:testSetCadenceOnlyOwner() (gas: 16089) -SecurityCouncilNomineeElectionGovernorTest:testSetCadenceTooSoonReverts() (gas: 148066) -SecurityCouncilNomineeElectionGovernorTest:testSetNomineeVetter() (gas: 40001) +SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 461501) +SecurityCouncilNomineeElectionGovernorTest:testExecute() (gas: 679315) +SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 199863) +SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 679144) +SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7833321) +SecurityCouncilNomineeElectionGovernorTest:testMultipleCadenceChanges() (gas: 238915) +SecurityCouncilNomineeElectionGovernorTest:testProperInitialization() (gas: 78159) +SecurityCouncilNomineeElectionGovernorTest:testProposeFails() (gas: 19786) +SecurityCouncilNomineeElectionGovernorTest:testRelay() (gas: 42411) +SecurityCouncilNomineeElectionGovernorTest:testRotateNominee() (gas: 510312) +SecurityCouncilNomineeElectionGovernorTest:testSetCadenceAfterElections() (gas: 227636) +SecurityCouncilNomineeElectionGovernorTest:testSetCadenceBeforeFirstElection() (gas: 42502) +SecurityCouncilNomineeElectionGovernorTest:testSetCadenceInvalidValue() (gas: 26056) +SecurityCouncilNomineeElectionGovernorTest:testSetCadenceOnlyOwner() (gas: 16090) +SecurityCouncilNomineeElectionGovernorTest:testSetCadenceTooSoonReverts() (gas: 148112) +SecurityCouncilNomineeElectionGovernorTest:testSetNomineeVetter() (gas: 40024) SecurityCouncilUpgradeActionTest:testAction() (gas: 8153) SequencerActionsTest:testAddAndRemoveSequencer() (gas: 486652) SequencerActionsTest:testCantAddZeroAddress() (gas: 235659) diff --git a/foundry.toml b/foundry.toml index 60ddf0d41..b55c7065e 100644 --- a/foundry.toml +++ b/foundry.toml @@ -9,7 +9,7 @@ via_ir = false solc_version = '0.8.16' [profile.sec_council_mgmt] -optimizer_runs = 500 +optimizer_runs = 200 [fmt] number_underscore = 'thousands' diff --git a/hardhat.config.ts b/hardhat.config.ts index 85851ddda..13ac7c2cb 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -19,7 +19,7 @@ const solidityProfiles = { settings: { optimizer: { enabled: true, - runs: 500 + runs: 200 }, } } diff --git a/src/security-council-mgmt/SecurityCouncilManager.sol b/src/security-council-mgmt/SecurityCouncilManager.sol index 46b5272bf..99388123a 100644 --- a/src/security-council-mgmt/SecurityCouncilManager.sol +++ b/src/security-council-mgmt/SecurityCouncilManager.sol @@ -38,7 +38,6 @@ contract SecurityCouncilManager is event MemberRemoved(address indexed member, Cohort indexed cohort); event MemberReplaced(address indexed replacedMember, address indexed newMember, Cohort cohort); event MemberRotated(address indexed replacedAddress, address indexed newAddress, Cohort cohort); - event RotatingToSet(address indexed replacedAddress, address indexed newAddress); event SecurityCouncilAdded( address indexed securityCouncil, address indexed updateAction, @@ -91,11 +90,7 @@ contract SecurityCouncilManager is /// @inheritdoc ISecurityCouncilManager uint256 public minRotationPeriod; - /// @notice Store the address to be rotated to for new members in the future - /// @dev `rotatingTo[X] = Y` means if X is installed as a new member, Y will be installed instead - mapping(address => address) public rotatingTo; - - /// @notice Nonce used when setting rotatingTo or rotatedTo + /// @notice Nonce used when setting rotatedTo mapping(address => uint256) public rotationNonce; /// @notice The 712 name hash @@ -119,8 +114,6 @@ contract SecurityCouncilManager is ); bytes32 public constant ROTATE_MEMBER_TYPE_HASH = keccak256(bytes("rotateMember(address from, uint256 nonce)")); - bytes32 public constant SET_ROTATING_TO_TYPE_HASH = - keccak256(bytes("setRotatingTo(address from, uint256 nonce)")); constructor() { _disableInitializers(); @@ -215,20 +208,8 @@ contract SecurityCouncilManager is // delete the old cohort _cohort == Cohort.FIRST ? delete firstCohort : delete secondCohort; - address[] storage otherCohort = _cohort == Cohort.FIRST ? secondCohort : firstCohort; for (uint256 i = 0; i < _newCohort.length; i++) { - // we have to change the array so correct _newCohort can be emitted - address rotatingAddress = rotatingTo[_newCohort[i]]; - if (rotatingAddress != address(0)) { - // only replace if there is no clash - if ( - !SecurityCouncilMgmtUtils.isInArray(rotatingAddress, _newCohort) - && !SecurityCouncilMgmtUtils.isInArray(rotatingAddress, otherCohort) - ) { - _newCohort[i] = rotatingAddress; - } - } _addMemberToCohortArray(_newCohort[i], _cohort); } @@ -316,13 +297,6 @@ contract SecurityCouncilManager is ); } - /// @inheritdoc ISecurityCouncilManager - function getSetRotatingToHash(address from, uint256 nonce) public view returns (bytes32) { - return ECDSAUpgradeable.toTypedDataHash( - _domainSeparatorV4(), keccak256(abi.encode(SET_ROTATING_TO_TYPE_HASH, from, nonce)) - ); - } - /// @inheritdoc ISecurityCouncilManager function rotateMember( address newMemberAddress, @@ -407,26 +381,6 @@ contract SecurityCouncilManager is emit MemberRotated({replacedAddress: msg.sender, newAddress: newAddress, cohort: cohort}); } - /// @inheritdoc ISecurityCouncilManager - function setRotatingTo(address newMemberAddress, bytes calldata signature) external { - uint256 currentRotationNonce = rotationNonce[msg.sender]; - // we enforce that a the new address is an eoa in the same way do - // in NomineeGovernor.addContender by requiring a signature - address newAddress = ECDSAUpgradeable.recover( - getSetRotatingToHash(msg.sender, currentRotationNonce), signature - ); - // we safety check the new member address is the one that we expect to replace here - // this isn't strictly necessary but it guards against the case where the wrong sig is accidentally used - if (newAddress != newMemberAddress) { - revert InvalidNewAddress(newAddress); - } - - rotatingTo[msg.sender] = newAddress; - rotationNonce[msg.sender] = currentRotationNonce + 1; - - emit RotatingToSet({replacedAddress: msg.sender, newAddress: newAddress}); - } - function _swapMembers(address _addressToRemove, address _addressToAdd) internal returns (Cohort) @@ -659,5 +613,5 @@ contract SecurityCouncilManager is * variables without shifting down storage in the inheritance chain. * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps */ - uint256[38] private __gap; + uint256[39] private __gap; } diff --git a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol index 193dd76f3..e611f3bf9 100644 --- a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol +++ b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol @@ -58,6 +58,12 @@ contract SecurityCouncilNomineeElectionGovernor is uint256 excludedNomineeCount; } + /// @notice Nominees can rotate their position to a new address. They are allowed to do this during the vetting period, but no later than `ROTATION_CUT_OFF_BLOCKS` L1 blocks before the vetting deadline. + /// Currently this is set to 3 days, assuming 12 blocks per second. + /// @dev It is known that a malicious nominee can abuse rotation to avoid vetting, + /// but the nominee vetter would always have 3 extra days after any rotation to exclude the nominee if needed. + uint256 public constant ROTATION_CUT_OFF_BLOCKS = 21600; + /// @notice Address responsible for blocking non compliant nominees address public nomineeVetter; @@ -76,6 +82,7 @@ contract SecurityCouncilNomineeElectionGovernor is event NomineeVetterChanged(address indexed oldNomineeVetter, address indexed newNomineeVetter); event ContenderAdded(uint256 indexed proposalId, address indexed contender); event NomineeExcluded(uint256 indexed proposalId, address indexed nominee); + event NomineeRotated(uint256 indexed proposalId, address indexed from, address indexed to); error OnlyNomineeVetter(); error CreateTooEarly(uint256 blockTimestamp, uint256 startTime); @@ -84,18 +91,21 @@ contract SecurityCouncilNomineeElectionGovernor is error AccountInOtherCohort(Cohort cohort, address account); error ProposalNotSucceededState(ProposalState state); error ProposalNotInVettingPeriod(uint256 blockNumber, uint256 vettingDeadline); + error ProposalNotInRotationPeriod(uint256 blockNumber, uint256 rotationDeadline); error NomineeAlreadyExcluded(address nominee); error CompliantNomineeTargetHit(uint256 nomineeCount, uint256 expectedCount); error ProposalInVettingPeriod(uint256 blockNumber, uint256 vettingDeadline); error InsufficientCompliantNomineeCount(uint256 compliantNomineeCount, uint256 expectedCount); error ProposeDisabled(); error NotNominee(address nominee); + error NotCompliantNominee(address nominee); error ProposalIdMismatch(uint256 nomineeProposalId, uint256 memberProposalId); error QuorumNumeratorTooLow(uint256 quorumNumeratorValue); error CastVoteDisabled(); error LastMemberElectionNotExecuted(uint256 prevProposalId); error InvalidSignature(); error Deprecated(string message); + error NotFromProxyAdmin(); constructor() { _disableInitializers(); @@ -112,7 +122,9 @@ contract SecurityCouncilNomineeElectionGovernor is } function postUpgradeInit() external { - require(msg.sender == getProxyAdmin(), "NOT_FROM_ADMIN"); + if (msg.sender != getProxyAdmin()) { + revert NotFromProxyAdmin(); + } if (cadenceInMonths == 0) { cadenceInMonths = 6; } @@ -250,8 +262,8 @@ contract SecurityCouncilNomineeElectionGovernor is } // check to make sure the contender is not part of the other cohort (the cohort not currently up for election) - // this only checks against the current the current other cohort, and against the current cohort membership - // in the security council, so changes to those will mean this check will be inconsistent. + // this only checks against the current cohort membership of the security council, + // so changes to those will mean this check will be inconsistent. // this check then is only a relevant check when the elections are running as expected - one at a time, // every `cadenceInMonths` months. Updates to the sec council manager using methods other than replaceCohort can effect this check // and it's expected that the entity making those updates understands this. @@ -353,6 +365,47 @@ contract SecurityCouncilNomineeElectionGovernor is _addNominee(proposalId, account); } + /// @notice Allows a nominee to rotate their position to a new address + /// @param proposalId The id of the proposal + /// @param newNomineeAddress The new address to rotate to + /// @param signature A signature from the new member address over the 712 rotateNominee hash + function rotateNominee(uint256 proposalId, address newNomineeAddress, bytes calldata signature) + external + { + ElectionInfo storage election = _elections[proposalId]; + + if (!isCompliantNominee(proposalId, msg.sender)) { + revert NotCompliantNominee(msg.sender); + } + + uint256 rotationDeadline = proposalVettingDeadline(proposalId) - ROTATION_CUT_OFF_BLOCKS; + if (block.number > rotationDeadline) { + revert ProposalNotInRotationPeriod(block.number, rotationDeadline); + } + + address signer = recoverRotateNomineeMessage(proposalId, signature, msg.sender); + if (signer != newNomineeAddress) { + revert InvalidSignature(); + } + + // check to make sure the new nominee is not part of the other cohort (the cohort not currently up for election) + // this only checks against the current the current other cohort, and against the current cohort membership + // in the security council, so changes to those will mean this check will be inconsistent. + // this check then is only a relevant check when the elections are running as expected - one at a time, + // every 6 months. Updates to the sec council manager using methods other than replaceCohort can effect this check + // and it's expected that the entity making those updates understands this. + if (securityCouncilManager.cohortIncludes(otherCohort(), newNomineeAddress)) { + revert AccountInOtherCohort(otherCohort(), newNomineeAddress); + } + + // rotation by first excluding the nominee and then adding the new nominee + election.isExcluded[msg.sender] = true; + election.excludedNomineeCount++; + _addNominee(proposalId, newNomineeAddress); + emit NomineeExcluded(proposalId, msg.sender); + emit NomineeRotated(proposalId, msg.sender, newNomineeAddress); + } + /// @dev `GovernorUpgradeable` function to execute a proposal overridden to handle nominee elections. /// Can be called by anyone via `execute` after voting and nominee vetting periods have ended. /// If the number of compliant nominees is > the target number of nominees, @@ -471,6 +524,23 @@ contract SecurityCouncilNomineeElectionGovernor is return ECDSAUpgradeable.recover(digest, signature); } + function recoverRotateNomineeMessage(uint256 proposalId, bytes calldata signature, address from) + public + view + returns (address) + { + bytes32 digest = _hashTypedDataV4( + keccak256( + abi.encode( + keccak256("RotateNomineeMessage(uint256 proposalId, address from)"), + proposalId, + from + ) + ) + ); + return ECDSAUpgradeable.recover(digest, signature); + } + /// @notice Always reverts. /// @dev `GovernorUpgradeable` function to create a proposal overridden to just revert. /// We only want proposals to be created via `createElection`. @@ -523,15 +593,6 @@ contract SecurityCouncilNomineeElectionGovernor is ); } - /// @notice Deprecated, use `addContender(uint256 proposalId, bytes calldata signature)` instead - /// @dev This function is deprecated because contenders should only be EOA's that can produce signatures. - /// If a security council member's address is not an EOA, then they may be unable to sign on all relevant chains. - function addContender(uint256) external pure { - revert Deprecated( - "addContender(uint256 proposalId) has been deprecated. Use addContender(uint256 proposalId, bytes calldata signature) instead" - ); - } - /** * @dev This empty reserved space is put in place to allow future versions to add new * variables without shifting down storage in the inheritance chain. diff --git a/src/security-council-mgmt/interfaces/ISecurityCouncilManager.sol b/src/security-council-mgmt/interfaces/ISecurityCouncilManager.sol index 8376b8844..c5ed75f96 100644 --- a/src/security-council-mgmt/interfaces/ISecurityCouncilManager.sol +++ b/src/security-council-mgmt/interfaces/ISecurityCouncilManager.sol @@ -46,12 +46,9 @@ interface ISecurityCouncilManager { error GovernorNotReplacer(); error NewMemberIsContender(uint256 proposalId, address newMember); error NewMemberIsNominee(uint256 proposalId, address newMember); - error NewMemberIsRotating(address newMember); - error NewMemberIsRotatingTarget(address newMember); error InvalidNewAddress(address newAddress); function rotatedTo(address) external view returns (address); - function rotatingTo(address) external view returns (address); function rotationNonce(address) external view returns (uint256); /// @notice There is a minimum period between when an address can be rotated @@ -107,9 +104,6 @@ interface ISecurityCouncilManager { /// @param _member Member to remove function removeMember(address _member) external; /// @notice Replace a member in a council - equivalent to removing a member, then adding another in its place. - /// Identities of members should be different. - /// Functionality is equivalent to replaceMember, - /// though emits a different event to distinguish the security council's intent (different identities). /// @dev Initiates cross chain messages to update the individual Security Councils. /// When replacing a member, make sure that the key does not conflict with any contenders/nominees of ongoing electoins. /// @param _memberToReplace Security Council member to remove @@ -130,19 +124,6 @@ interface ISecurityCouncilManager { address memberElectionGovernor, bytes calldata signature ) external; - /// @notice Get the hash to be signed for future member rotation - /// @param from The address that will be rotated out. This is included in the hash so that other members cant use this message to rotate their address - /// @param nonce The message nonce. Must be the from address's current rotationNonce - function getSetRotatingToHash(address from, uint256 nonce) external view returns (bytes32); - /// @notice Set an address to be rotated to if the sender is ever elected as a member - /// This enables unelected members to decide where their election address will update to. When a member is elected to the council they - /// are expected to have a high level of security on their member key. Election candidates may not have set up that high level of security before - /// registering their election key, so this method allows them to set up a new key that will be actually installed as the member upon election. - /// If this future rotation causes a clash, the rotation will not be executed and the original address will be installed - /// This rotation only applies to future replaceCohort, mainly used by the member election governor - /// @param newMemberAddress The new member address to be rotated to - /// @param signature A signature from the new member address over the 712 setRotatingTo hash - function setRotatingTo(address newMemberAddress, bytes calldata signature) external; /// @notice Is the account a member of the first cohort function firstCohortIncludes(address account) external view returns (bool); /// @notice Is the account a member of the second cohort diff --git a/test/security-council-mgmt/SecurityCouncilManager.t.sol b/test/security-council-mgmt/SecurityCouncilManager.t.sol index dc3f757a5..7ff34235e 100644 --- a/test/security-council-mgmt/SecurityCouncilManager.t.sol +++ b/test/security-council-mgmt/SecurityCouncilManager.t.sol @@ -921,127 +921,6 @@ contract SecurityCouncilManagerTest is Test { scm.replaceCohort(newCohortWithADup, Cohort.SECOND); } - function testReplaceCohortRotatingTo() public { - // set a rotatingTo for a member of the first cohort - address[] memory newCohortCopy = newCohort; - address rotatingFrom = newCohortCopy[1]; - bytes32 digest = scm.getSetRotatingToHash(rotatingFrom, scm.rotationNonce(rotatingFrom)); - bytes memory signature = sign(pk1, digest); - vm.prank(rotatingFrom); - scm.setRotatingTo(memberToRotate1, signature); - - vm.startPrank(roles.cohortUpdator); - vm.recordLogs(); - scm.replaceCohort(newCohortCopy, Cohort.FIRST); - checkScheduleWasCalled(); - vm.stopPrank(); - - newCohortCopy[1] = memberToRotate1; - - assertTrue( - TestUtil.areUniqueAddressArraysEqual(newCohortCopy, scm.getFirstCohort()), - "first cohort updated" - ); - - assertTrue( - TestUtil.areUniqueAddressArraysEqual(secondCohort, scm.getSecondCohort()), - "second cohort untouched" - ); - - rotatingFrom = newCohortCopy[2]; - digest = scm.getSetRotatingToHash(rotatingFrom, scm.rotationNonce(rotatingFrom)); - signature = sign(pknc1, digest); - vm.prank(rotatingFrom); - scm.setRotatingTo(pkncAddr1, signature); - - // set rotation to a member of the incoming cohort - vm.startPrank(roles.cohortUpdator); - vm.recordLogs(); - scm.replaceCohort(newCohortCopy, Cohort.FIRST); - checkScheduleWasCalled(); - vm.stopPrank(); - - // should still just equal the newcohort copy - assertTrue( - TestUtil.areUniqueAddressArraysEqual(newCohortCopy, scm.getFirstCohort()), - "first cohort updated" - ); - assertTrue( - TestUtil.areUniqueAddressArraysEqual(secondCohort, scm.getSecondCohort()), - "second cohort untouched" - ); - - // now try rotate to a member of the other cohort - rotatingFrom = newCohortCopy[2]; - digest = scm.getSetRotatingToHash(rotatingFrom, scm.rotationNonce(rotatingFrom)); - signature = sign(pknc2, digest); - vm.prank(rotatingFrom); - scm.setRotatingTo(pkncAddr2, signature); - - // set rotation to a member of the incoming cohort - vm.startPrank(roles.cohortUpdator); - vm.recordLogs(); - scm.replaceCohort(newCohortCopy, Cohort.FIRST); - checkScheduleWasCalled(); - vm.stopPrank(); - - // should still just equal the newcohort copy - assertTrue( - TestUtil.areUniqueAddressArraysEqual(newCohortCopy, scm.getFirstCohort()), - "first cohort updated" - ); - assertTrue( - TestUtil.areUniqueAddressArraysEqual(secondCohort, scm.getSecondCohort()), - "second cohort untouched" - ); - - // put it back to how we found it - vm.startPrank(roles.cohortUpdator); - vm.recordLogs(); - scm.replaceCohort(firstCohort, Cohort.FIRST); - checkScheduleWasCalled(); - vm.stopPrank(); - } - - event RotatingToSet(address indexed replacedAddress, address indexed newAddress); - - function testSetRotatingTo() public { - address testAddr = vm.addr(97_990); - - uint256 testPk1 = 45_678; - address addr1 = vm.addr(testPk1); - uint256 testPk2 = 45_679; - address addr2 = vm.addr(testPk2); - - assertEq(scm.rotatingTo(testAddr), address(0)); - assertEq(scm.rotationNonce(testAddr), 0); - - bytes32 digest = scm.getSetRotatingToHash(testAddr, scm.rotationNonce(testAddr)); - bytes memory signature = sign(testPk1, digest); - - vm.prank(testAddr); - vm.expectRevert( - abi.encodeWithSelector(ISecurityCouncilManager.InvalidNewAddress.selector, addr1) - ); - scm.setRotatingTo(address(1), signature); - - vm.prank(testAddr); - vm.expectEmit(true, true, true, true); - emit RotatingToSet({replacedAddress: testAddr, newAddress: addr1}); - scm.setRotatingTo(addr1, signature); - assertEq(scm.rotatingTo(testAddr), addr1); - assertEq(scm.rotationNonce(testAddr), 1); - - digest = scm.getSetRotatingToHash(testAddr, scm.rotationNonce(testAddr)); - signature = sign(testPk2, digest); - vm.prank(testAddr); - vm.expectEmit(true, true, true, true); - emit RotatingToSet({replacedAddress: testAddr, newAddress: addr2}); - scm.setRotatingTo(addr2, signature); - assertEq(scm.rotatingTo(testAddr), addr2); - assertEq(scm.rotationNonce(testAddr), 2); - } - function testUpdateRouterAffordances() public { UpgradeExecRouteBuilder newRouter = UpgradeExecRouteBuilder(TestUtil.deployStubContract()); vm.prank(rando); diff --git a/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol b/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol index 2691c0cd5..8ed5af784 100644 --- a/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol +++ b/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol @@ -36,6 +36,26 @@ contract SigUtils is Test { sig = abi.encodePacked(r, s, v); } + function signRotateNomineeMessage(uint256 proposalId, uint256 privKey, address from) + public + view + returns (bytes memory sig) + { + bytes32 digest = _hashTypedDataV4( + keccak256( + abi.encode( + keccak256("RotateNomineeMessage(uint256 proposalId, address from)"), + proposalId, + from + ) + ) + ); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(privKey, digest); + + sig = abi.encodePacked(r, s, v); + } + function _domainSeparatorV4() internal view returns (bytes32) { return _buildDomainSeparator(_TYPE_HASH, _EIP712NameHash(), _EIP712VersionHash()); } @@ -1155,4 +1175,78 @@ contract SecurityCouncilNomineeElectionGovernorTest is Test { assertEq(secondElection, expectedSecondTime, "Elections should be 36 months apart"); } + + function testRotateNominee() public { + uint256 proposalId = _propose(); + + // create a nominee + vm.roll(governor.proposalSnapshot(proposalId)); + _addContender(proposalId, 0); + vm.roll(governor.proposalDeadline(proposalId)); + _mockGetPastVotes(_voter(0), governor.quorum(proposalId)); + _castVoteForContender(proposalId, _voter(0), _contender(0), governor.quorum(proposalId)); + + bytes memory sig = + sigUtils.signRotateNomineeMessage(proposalId, _contenderPrivKey(1), _contender(0)); + uint256 rotationDeadline = governor.proposalVettingDeadline(proposalId) - 21_600; + + // cannot rotate after the deadline + vm.roll(rotationDeadline + 1); + vm.prank(_contender(0)); + vm.expectRevert( + abi.encodeWithSelector( + SecurityCouncilNomineeElectionGovernor.ProposalNotInRotationPeriod.selector, + block.number, + rotationDeadline + ) + ); + governor.rotateNominee(proposalId, _contender(1), sig); + vm.roll(rotationDeadline); + + // cannot rotate if not a compliant nominee + vm.prank(_contender(1)); + vm.expectRevert( + abi.encodeWithSelector( + SecurityCouncilNomineeElectionGovernor.NotCompliantNominee.selector, _contender(1) + ) + ); + governor.rotateNominee(proposalId, _contender(1), sig); + + // cannot rotate with invalid signature + vm.prank(_contender(0)); + vm.expectRevert( + abi.encodeWithSelector(SecurityCouncilNomineeElectionGovernor.InvalidSignature.selector) + ); + governor.rotateNominee(proposalId, _contender(2), sig); + + // cannot rotate if in other cohort + _mockCohortIncludes(Cohort.SECOND, _contender(1), true); + vm.prank(_contender(0)); + vm.expectRevert( + abi.encodeWithSelector( + SecurityCouncilNomineeElectionGovernor.AccountInOtherCohort.selector, + Cohort.SECOND, + _contender(1) + ) + ); + governor.rotateNominee(proposalId, _contender(1), sig); + + // rotate the nominee + _mockCohortIncludes(Cohort.SECOND, _contender(1), false); + vm.prank(_contender(0)); + governor.rotateNominee(proposalId, _contender(1), sig); + + // cannot rotate again + vm.prank(_contender(0)); + vm.expectRevert( + abi.encodeWithSelector( + SecurityCouncilNomineeElectionGovernor.NotCompliantNominee.selector, _contender(0) + ) + ); + governor.rotateNominee(proposalId, _contender(1), sig); + + // make sure state is correct + assertTrue(governor.isCompliantNominee(proposalId, _contender(1))); + assertFalse(governor.isCompliantNominee(proposalId, _contender(0))); + } } diff --git a/test/signatures/SecurityCouncilManager b/test/signatures/SecurityCouncilManager index 025a874f4..9c8385eae 100644 --- a/test/signatures/SecurityCouncilManager +++ b/test/signatures/SecurityCouncilManager @@ -26,8 +26,6 @@ |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | ROTATE_MEMBER_TYPE_HASH() | aea6b1e7 | |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| -| SET_ROTATING_TO_TYPE_HASH() | ff57aaed | -|-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | VERSION_HASH() | 9e4e7318 | |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | addMember(address,uint8) | 62d0d1c3 | @@ -54,8 +52,6 @@ |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | getSecondCohort() | bdc9f17c | |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| -| getSetRotatingToHash(address,uint256) | 2bf9dbfe | -|-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | grantRole(bytes32,address) | 2f2ff15d | |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | hasRole(bytes32,address) | 91d14854 | @@ -86,8 +82,6 @@ |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | rotatedTo(address) | 86bc77a3 | |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| -| rotatingTo(address) | cd6150a4 | -|-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | rotationNonce(address) | ac823694 | |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | router() | f887ea40 | @@ -100,8 +94,6 @@ |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | setMinRotationPeriod(uint256) | d4c271b2 | |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| -| setRotatingTo(address,bytes) | 62cd078d | -|-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | setUpgradeExecRouteBuilder(address) | 0e5e43d7 | |-------------------------------------------------------------------------------------------------------------------------------------------------+------------| | supportsInterface(bytes4) | 01ffc9a7 | diff --git a/test/signatures/SecurityCouncilNomineeElectionGovernor b/test/signatures/SecurityCouncilNomineeElectionGovernor index 2f3f77cab..91fbde2ab 100644 --- a/test/signatures/SecurityCouncilNomineeElectionGovernor +++ b/test/signatures/SecurityCouncilNomineeElectionGovernor @@ -10,7 +10,7 @@ |-----------------------------------------------------------------------------------------------------------------+------------| | EXTENDED_BALLOT_TYPEHASH() | 2fe3e261 | |-----------------------------------------------------------------------------------------------------------------+------------| -| addContender(uint256) | 140af012 | +| ROTATION_CUT_OFF_BLOCKS() | 6bc9d9ab | |-----------------------------------------------------------------------------------------------------------------+------------| | addContender(uint256,bytes) | a8f38759 | |-----------------------------------------------------------------------------------------------------------------+------------| @@ -116,10 +116,14 @@ |-----------------------------------------------------------------------------------------------------------------+------------| | recoverAddContenderMessage(uint256,bytes) | 5a756eaf | |-----------------------------------------------------------------------------------------------------------------+------------| +| recoverRotateNomineeMessage(uint256,bytes,address) | 307d1d14 | +|-----------------------------------------------------------------------------------------------------------------+------------| | relay(address,uint256,bytes) | c28bc2fa | |-----------------------------------------------------------------------------------------------------------------+------------| | renounceOwnership() | 715018a6 | |-----------------------------------------------------------------------------------------------------------------+------------| +| rotateNominee(uint256,address,bytes) | 5cd48043 | +|-----------------------------------------------------------------------------------------------------------------+------------| | securityCouncilManager() | 03d1ce8a | |-----------------------------------------------------------------------------------------------------------------+------------| | securityCouncilMemberElectionGovernor() | 1b6a7673 | diff --git a/test/storage/SecurityCouncilManager b/test/storage/SecurityCouncilManager index ffd9b0e5c..972049f05 100644 --- a/test/storage/SecurityCouncilManager +++ b/test/storage/SecurityCouncilManager @@ -34,10 +34,8 @@ |-------------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------------------------------------------| | minRotationPeriod | uint256 | 160 | 0 | 32 | src/security-council-mgmt/SecurityCouncilManager.sol:SecurityCouncilManager | |-------------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------------------------------------------| -| rotatingTo | mapping(address => address) | 161 | 0 | 32 | src/security-council-mgmt/SecurityCouncilManager.sol:SecurityCouncilManager | +| rotationNonce | mapping(address => uint256) | 161 | 0 | 32 | src/security-council-mgmt/SecurityCouncilManager.sol:SecurityCouncilManager | |-------------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------------------------------------------| -| rotationNonce | mapping(address => uint256) | 162 | 0 | 32 | src/security-council-mgmt/SecurityCouncilManager.sol:SecurityCouncilManager | -|-------------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------------------------------------------| -| __gap | uint256[38] | 163 | 0 | 1216 | src/security-council-mgmt/SecurityCouncilManager.sol:SecurityCouncilManager | +| __gap | uint256[39] | 162 | 0 | 1248 | src/security-council-mgmt/SecurityCouncilManager.sol:SecurityCouncilManager | ╰-------------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------------------------------------------╯