From 2cd220c43d681e42469a567dc6817cc54363e0dc Mon Sep 17 00:00:00 2001 From: gzeon Date: Thu, 4 Sep 2025 21:51:57 +0900 Subject: [PATCH 1/7] fix: prevent duplicate members --- ...SecurityCouncilNomineeElectionGovernor.sol | 51 ++++++------------- ...neeElectionGovernorCountingUpgradeable.sol | 3 ++ 2 files changed, 19 insertions(+), 35 deletions(-) diff --git a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol index e611f3bf9..e289383f2 100644 --- a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol +++ b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol @@ -62,7 +62,7 @@ contract SecurityCouncilNomineeElectionGovernor is /// 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; + uint256 public constant ROTATION_CUT_OFF_BLOCKS = 21_600; /// @notice Address responsible for blocking non compliant nominees address public nomineeVetter; @@ -261,22 +261,13 @@ contract SecurityCouncilNomineeElectionGovernor is revert ProposalNotPending(state_); } - // 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 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. - if (securityCouncilManager.cohortIncludes(otherCohort(), signer)) { - revert AccountInOtherCohort(otherCohort(), signer); - } - + _validateNotInOtherCohort(signer); 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)) { + // no need to check for duplicate nominees as we already checked _addNominee(proposalId, signer); } } @@ -342,26 +333,13 @@ contract SecurityCouncilNomineeElectionGovernor is revert ProposalNotSucceededState(state_); } - if (isNominee(proposalId, account)) { - revert NomineeAlreadyAdded(account); - } - uint256 cnCount = compliantNomineeCount(proposalId); uint256 cohortSize = securityCouncilManager.cohortSize(); if (cnCount >= cohortSize) { revert CompliantNomineeTargetHit(cnCount, cohortSize); } - // can't include nominees from 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 `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); - } - + _validateNotInOtherCohort(account); _addNominee(proposalId, account); } @@ -388,22 +366,25 @@ contract SecurityCouncilNomineeElectionGovernor is revert InvalidSignature(); } + // rotation by first excluding the nominee and then adding the new nominee + election.isExcluded[msg.sender] = true; + election.excludedNomineeCount++; + _validateNotInOtherCohort(newNomineeAddress); + _addNominee(proposalId, newNomineeAddress); + emit NomineeExcluded(proposalId, msg.sender); + emit NomineeRotated(proposalId, msg.sender, newNomineeAddress); + } + + function _validateNotInOtherCohort(address account) internal { // 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); + if (securityCouncilManager.cohortIncludes(otherCohort(), account)) { + revert AccountInOtherCohort(otherCohort(), account); } - - // 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. diff --git a/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorCountingUpgradeable.sol b/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorCountingUpgradeable.sol index 9cc6d7e3c..efbe43206 100644 --- a/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorCountingUpgradeable.sol +++ b/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorCountingUpgradeable.sol @@ -121,6 +121,9 @@ abstract contract SecurityCouncilNomineeElectionGovernorCountingUpgradeable is /// @dev Transitions an account to being a nominee function _addNominee(uint256 proposalId, address account) internal { + if (isNominee(proposalId, account)) { + revert NomineeAlreadyAdded(account); + } _elections[proposalId].nominees.push(account); _elections[proposalId].isNominee[account] = true; emit NewNominee(proposalId, account); From 56d9fa23f77795b76c0600b6a9f44edf80557030 Mon Sep 17 00:00:00 2001 From: gzeon Date: Thu, 4 Sep 2025 21:53:59 +0900 Subject: [PATCH 2/7] feat: _requireNotInOtherCohort --- ...SecurityCouncilNomineeElectionGovernor.sol | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol index e289383f2..8fc1f8431 100644 --- a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol +++ b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol @@ -211,6 +211,18 @@ contract SecurityCouncilNomineeElectionGovernor is electionCount++; } + function _requireNotInOtherCohort(address account) internal { + // 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(), account)) { + revert AccountInOtherCohort(otherCohort(), account); + } + } + /// @dev Revert if the previous member election has not executed. /// Ensures that there are no unexpected behaviors from multiple elections running at the same time. /// If, for some reason, the previous member election is blocked, @@ -261,7 +273,7 @@ contract SecurityCouncilNomineeElectionGovernor is revert ProposalNotPending(state_); } - _validateNotInOtherCohort(signer); + _requireNotInOtherCohort(signer); election.isContender[signer] = true; emit ContenderAdded(proposalId, signer); @@ -339,7 +351,7 @@ contract SecurityCouncilNomineeElectionGovernor is revert CompliantNomineeTargetHit(cnCount, cohortSize); } - _validateNotInOtherCohort(account); + _requireNotInOtherCohort(account); _addNominee(proposalId, account); } @@ -369,24 +381,12 @@ contract SecurityCouncilNomineeElectionGovernor is // rotation by first excluding the nominee and then adding the new nominee election.isExcluded[msg.sender] = true; election.excludedNomineeCount++; - _validateNotInOtherCohort(newNomineeAddress); + _requireNotInOtherCohort(newNomineeAddress); _addNominee(proposalId, newNomineeAddress); emit NomineeExcluded(proposalId, msg.sender); emit NomineeRotated(proposalId, msg.sender, newNomineeAddress); } - function _validateNotInOtherCohort(address account) internal { - // 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(), account)) { - revert AccountInOtherCohort(otherCohort(), account); - } - } - /// @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, From 5a315ba92f6d12491625b86fd1deab3cdc9bcda5 Mon Sep 17 00:00:00 2001 From: gzeon Date: Thu, 4 Sep 2025 21:58:55 +0900 Subject: [PATCH 3/7] test: cannot rotate to existing nominee --- ...ecurityCouncilNomineeElectionGovernor.t.sol | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol b/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol index 8ed5af784..672d23b02 100644 --- a/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol +++ b/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol @@ -1231,6 +1231,24 @@ contract SecurityCouncilNomineeElectionGovernorTest is Test { ); governor.rotateNominee(proposalId, _contender(1), sig); + // cannot rotate to existing nominee + bytes memory sig2 = + sigUtils.signRotateNomineeMessage(proposalId, _contenderPrivKey(2), _contender(0)); + vm.prank(initParams.nomineeVetter); + _mockCohortIncludes(Cohort.SECOND, _contender(2), false); + governor.includeNominee(proposalId, _contender(2)); + _mockCohortIncludes(Cohort.SECOND, _contender(2), false); + vm.prank(_contender(0)); + vm.expectRevert( + abi.encodeWithSelector( + SecurityCouncilNomineeElectionGovernorCountingUpgradeable + .NomineeAlreadyAdded + .selector, + _contender(2) + ) + ); + governor.rotateNominee(proposalId, _contender(2), sig2); + // rotate the nominee _mockCohortIncludes(Cohort.SECOND, _contender(1), false); vm.prank(_contender(0)); From f1902f5ac11a8b6e2264673e0d85828456f018eb Mon Sep 17 00:00:00 2001 From: gzeon Date: Thu, 4 Sep 2025 22:03:34 +0900 Subject: [PATCH 4/7] chore: update gas snapshot --- .gas-snapshot | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 59be04bce..a0b21b9ea 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: 86859495) +E2E:testE2E() (gas: 86786575) 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: 32322314) -L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 32326589) -L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 27265404) -L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32324545) -L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32345900) +L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 32247486) +L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 32251761) +L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 27190576) +L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32249717) +L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32271072) NomineeGovernorV2UpgradeActionTest:testAction() (gas: 8153) OfficeHoursActionTest:testConstructor() (gas: 9050) OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 256, μ: 317059, ~: 317184) @@ -138,7 +138,7 @@ SecurityCouncilManagerTest:testReplaceMemberInFirstCohortAfterRotation() (gas: 4 SecurityCouncilManagerTest:testReplaceMemberInSecondCohort() (gas: 479028) SecurityCouncilManagerTest:testReplaceMemberInSecondCohortAfterRotation() (gas: 270210) SecurityCouncilManagerTest:testRotateMember() (gas: 1015787) -SecurityCouncilManagerTest:testRotateMemberNotContender() (gas: 4078679) +SecurityCouncilManagerTest:testRotateMemberNotContender() (gas: 4080567) SecurityCouncilManagerTest:testSetMinRotationPeriod() (gas: 65814) SecurityCouncilManagerTest:testUpdateCohortAffordances() (gas: 83252) SecurityCouncilManagerTest:testUpdateFirstCohort() (gas: 313830) @@ -194,25 +194,25 @@ SecurityCouncilMemberSyncActionTest:testRemoveOne() (gas: 8086867) SecurityCouncilMemberSyncActionTest:testUpdateCohort() (gas: 8328313) SecurityCouncilMemberSyncActionTest:testUpdateCohort() (gas: 8329174) SecurityCouncilMgmtUtilsTests:testIsInArray() (gas: 2102) -SecurityCouncilNomineeElectionGovernorTest:testAddContender() (gas: 418329) +SecurityCouncilNomineeElectionGovernorTest:testAddContender() (gas: 418681) SecurityCouncilNomineeElectionGovernorTest:testCadenceWithLargeValues() (gas: 52898) -SecurityCouncilNomineeElectionGovernorTest:testCastBySig() (gas: 338828) -SecurityCouncilNomineeElectionGovernorTest:testCastBySigTwice() (gas: 301643) +SecurityCouncilNomineeElectionGovernorTest:testCastBySig() (gas: 338857) +SecurityCouncilNomineeElectionGovernorTest:testCastBySigTwice() (gas: 301672) SecurityCouncilNomineeElectionGovernorTest:testCastVoteReverts() (gas: 35303) -SecurityCouncilNomineeElectionGovernorTest:testCountVote() (gas: 593196) +SecurityCouncilNomineeElectionGovernorTest:testCountVote() (gas: 593835) SecurityCouncilNomineeElectionGovernorTest:testCreateElection() (gas: 257942) SecurityCouncilNomineeElectionGovernorTest:testDefaultCadence() (gas: 14950) SecurityCouncilNomineeElectionGovernorTest:testElectionTimestampsWithDefaultCadence() (gas: 37625) -SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 461501) -SecurityCouncilNomineeElectionGovernorTest:testExecute() (gas: 679315) -SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 199863) -SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 679144) -SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7833321) +SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 461806) +SecurityCouncilNomineeElectionGovernorTest:testExecute() (gas: 679489) +SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 199892) +SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 678878) +SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7758495) SecurityCouncilNomineeElectionGovernorTest:testMultipleCadenceChanges() (gas: 238915) SecurityCouncilNomineeElectionGovernorTest:testProperInitialization() (gas: 78159) SecurityCouncilNomineeElectionGovernorTest:testProposeFails() (gas: 19786) SecurityCouncilNomineeElectionGovernorTest:testRelay() (gas: 42411) -SecurityCouncilNomineeElectionGovernorTest:testRotateNominee() (gas: 510312) +SecurityCouncilNomineeElectionGovernorTest:testRotateNominee() (gas: 673577) SecurityCouncilNomineeElectionGovernorTest:testSetCadenceAfterElections() (gas: 227636) SecurityCouncilNomineeElectionGovernorTest:testSetCadenceBeforeFirstElection() (gas: 42502) SecurityCouncilNomineeElectionGovernorTest:testSetCadenceInvalidValue() (gas: 26056) From 36d17e2ceb5dbcdb6c71aa2ee33681623189eb27 Mon Sep 17 00:00:00 2001 From: gzeon Date: Thu, 4 Sep 2025 23:02:24 +0900 Subject: [PATCH 5/7] fix: check new nominee address excluded --- .gas-snapshot | 16 ++++++++-------- .../SecurityCouncilNomineeElectionGovernor.sol | 4 ++++ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index a0b21b9ea..c2c029d57 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: 86786575) +E2E:testE2E() (gas: 86811023) 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: 32247486) -L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 32251761) -L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 27190576) -L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32249717) -L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32271072) +L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 32271955) +L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 32276230) +L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 27215045) +L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32274186) +L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32295541) NomineeGovernorV2UpgradeActionTest:testAction() (gas: 8153) OfficeHoursActionTest:testConstructor() (gas: 9050) OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 256, μ: 317059, ~: 317184) @@ -207,12 +207,12 @@ SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 461806) SecurityCouncilNomineeElectionGovernorTest:testExecute() (gas: 679489) SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 199892) SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 678878) -SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7758495) +SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7782964) SecurityCouncilNomineeElectionGovernorTest:testMultipleCadenceChanges() (gas: 238915) SecurityCouncilNomineeElectionGovernorTest:testProperInitialization() (gas: 78159) SecurityCouncilNomineeElectionGovernorTest:testProposeFails() (gas: 19786) SecurityCouncilNomineeElectionGovernorTest:testRelay() (gas: 42411) -SecurityCouncilNomineeElectionGovernorTest:testRotateNominee() (gas: 673577) +SecurityCouncilNomineeElectionGovernorTest:testRotateNominee() (gas: 680405) SecurityCouncilNomineeElectionGovernorTest:testSetCadenceAfterElections() (gas: 227636) SecurityCouncilNomineeElectionGovernorTest:testSetCadenceBeforeFirstElection() (gas: 42502) SecurityCouncilNomineeElectionGovernorTest:testSetCadenceInvalidValue() (gas: 26056) diff --git a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol index 8fc1f8431..92f03a0e9 100644 --- a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol +++ b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol @@ -373,6 +373,10 @@ contract SecurityCouncilNomineeElectionGovernor is revert ProposalNotInRotationPeriod(block.number, rotationDeadline); } + if (election.isExcluded[newNomineeAddress]) { + revert NomineeAlreadyExcluded(newNomineeAddress); + } + address signer = recoverRotateNomineeMessage(proposalId, signature, msg.sender); if (signer != newNomineeAddress) { revert InvalidSignature(); From 38c9efd8021aa2d0c38006c34d4ad4db77d3a740 Mon Sep 17 00:00:00 2001 From: gzeon Date: Fri, 5 Sep 2025 00:02:50 +0900 Subject: [PATCH 6/7] chore: update gas snapshot for new foundry version --- .gas-snapshot | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index c2c029d57..b610a96a1 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -14,7 +14,7 @@ ArbitrumFoundationVestingWalletTest:testOnlyOwnerCanMigrate() (gas: 16329777) ArbitrumFoundationVestingWalletTest:testOwnerCanSetBeneficiary() (gas: 16332196) ArbitrumFoundationVestingWalletTest:testProperlyInits() (gas: 16337566) ArbitrumFoundationVestingWalletTest:testRandomAddressCantSetBeneficiary() (gas: 16329676) -ArbitrumFoundationVestingWalletTest:testRelease() (gas: 16451151) +ArbitrumFoundationVestingWalletTest:testRelease() (gas: 16448651) ArbitrumVestingWalletFactoryTest:testDeploy() (gas: 4589688) ArbitrumVestingWalletFactoryTest:testOnlyOwnerCanCreateWallets() (gas: 1504286) ArbitrumVestingWalletTest:testCastVote() (gas: 16201599) @@ -27,7 +27,7 @@ ArbitrumVestingWalletTest:testDoesDeploy() (gas: 15971357) ArbitrumVestingWalletTest:testReleaseAffordance() (gas: 16008664) ArbitrumVestingWalletTest:testVestedAmountStart() (gas: 16074932) CancelTimelockAndRemoveMemberActionTest:testAction() (gas: 8159) -E2E:testE2E() (gas: 86811023) +E2E:testE2E() (gas: 86806023) FixedDelegateErc20WalletTest:testInit() (gas: 5822585) FixedDelegateErc20WalletTest:testInitZeroToken() (gas: 5816815) FixedDelegateErc20WalletTest:testTransfer() (gas: 5932228) @@ -102,7 +102,7 @@ L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 32274186) L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 32295541) NomineeGovernorV2UpgradeActionTest:testAction() (gas: 8153) OfficeHoursActionTest:testConstructor() (gas: 9050) -OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 256, μ: 317059, ~: 317184) +OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 256, μ: 317090, ~: 317184) OfficeHoursActionTest:testInvalidConstructorParameters() (gas: 235740) OfficeHoursActionTest:testPerformBeforeMinimumTimestamp() (gas: 8646) OfficeHoursActionTest:testPerformDuringOfficeHours() (gas: 9140) @@ -143,7 +143,7 @@ SecurityCouncilManagerTest:testSetMinRotationPeriod() (gas: 65814) SecurityCouncilManagerTest:testUpdateCohortAffordances() (gas: 83252) SecurityCouncilManagerTest:testUpdateFirstCohort() (gas: 313830) SecurityCouncilManagerTest:testUpdateRouter() (gas: 76407) -SecurityCouncilManagerTest:testUpdateRouterAffordances() (gas: 112474) +SecurityCouncilManagerTest:testUpdateRouterAffordances() (gas: 109974) SecurityCouncilManagerTest:testUpdateSecondCohort() (gas: 313924) SecurityCouncilMemberElectionGovernorTest:testCannotUseMoreVotesThanAvailable() (gas: 247018) SecurityCouncilMemberElectionGovernorTest:testCastBySig() (gas: 302873) @@ -160,7 +160,7 @@ SecurityCouncilMemberElectionGovernorTest:testOnlyNomineeElectionGovernorCanProp SecurityCouncilMemberElectionGovernorTest:testProperInitialization() (gas: 49388) SecurityCouncilMemberElectionGovernorTest:testProposeReverts() (gas: 32916) SecurityCouncilMemberElectionGovernorTest:testRelay() (gas: 42229) -SecurityCouncilMemberElectionGovernorTest:testSelectTopNominees(uint256) (runs: 256, μ: 340137, ~: 339953) +SecurityCouncilMemberElectionGovernorTest:testSelectTopNominees(uint256) (runs: 256, μ: 339752, ~: 339539) SecurityCouncilMemberElectionGovernorTest:testSelectTopNomineesFails() (gas: 273467) SecurityCouncilMemberElectionGovernorTest:testSetFullWeightDuration() (gas: 34951) SecurityCouncilMemberElectionGovernorTest:testVotesToWeight() (gas: 152898) From d2c153395f5a073199e65c0d1b97a5c1f8f95591 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Thu, 4 Sep 2025 10:45:48 -0700 Subject: [PATCH 7/7] mark _requireNotInOtherCohort as view and fix comment --- .../SecurityCouncilNomineeElectionGovernor.sol | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol index 92f03a0e9..f805ff0e6 100644 --- a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol +++ b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol @@ -211,12 +211,12 @@ contract SecurityCouncilNomineeElectionGovernor is electionCount++; } - function _requireNotInOtherCohort(address account) internal { - // 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. + function _requireNotInOtherCohort(address account) internal view { + // 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 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 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); @@ -275,6 +275,7 @@ contract SecurityCouncilNomineeElectionGovernor is _requireNotInOtherCohort(signer); election.isContender[signer] = true; + emit ContenderAdded(proposalId, signer); // if the signer is part of the outgoing cohort, we automatically add them as a nominee