Member self rotation

.gas-snapshot

@@ -26,7 +26,7 @@ ArbitrumVestingWalletTest:testDelegateFailsForNonBeneficiary() (gas: 16008435)
 ArbitrumVestingWalletTest:testDoesDeploy() (gas: 15971342)
 ArbitrumVestingWalletTest:testReleaseAffordance() (gas: 16008649)
 ArbitrumVestingWalletTest:testVestedAmountStart() (gas: 16074917)
-E2E:testE2E() (gas: 84680100)
+E2E:testE2E() (gas: 85785140)
 FixedDelegateErc20WalletTest:testInit() (gas: 5822575)
 FixedDelegateErc20WalletTest:testInitZeroToken() (gas: 5816805)
 FixedDelegateErc20WalletTest:testTransfer() (gas: 5932218)
@@ -94,14 +94,14 @@ L2GovernanceFactoryTest:testSanityCheckValues() (gas: 28415658)
 L2GovernanceFactoryTest:testSetMinDelay() (gas: 28364371)
 L2GovernanceFactoryTest:testSetMinDelayRevertsForCoreAddress() (gas: 28417242)
 L2GovernanceFactoryTest:testUpgraderCanCancel() (gas: 28657360)
-L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 30524001)
-L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 30528232)
-L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 25659644)
-L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 30526232)
-L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 30545325)
+L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 31628990)
+L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 31633221)
+L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 26672005)
+L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 31631221)
+L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 31652101)
 NomineeGovernorV2UpgradeActionTest:testAction() (gas: 8153)
 OfficeHoursActionTest:testConstructor() (gas: 9050)
-OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 258, μ: 317071, ~: 317184)
+OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 256, μ: 317069, ~: 317184)
 OfficeHoursActionTest:testInvalidConstructorParameters() (gas: 235740)
 OfficeHoursActionTest:testPerformBeforeMinimumTimestamp() (gas: 8646)
 OfficeHoursActionTest:testPerformDuringOfficeHours() (gas: 9140)
@@ -116,28 +116,31 @@ OutboxActionsTest:testRemoveAllOutboxes() (gas: 693007)
 OutboxActionsTest:testRemoveOutboxes() (gas: 853882)
 ProxyUpgradeAndCallActionTest:testUpgrade() (gas: 137095)
 ProxyUpgradeAndCallActionTest:testUpgradeAndCall() (gas: 143042)
-SecurityCouncilManagerTest:testAddMemberAffordances() (gas: 249651)
-SecurityCouncilManagerTest:testAddMemberSpecialAddresses() (gas: 20795)
-SecurityCouncilManagerTest:testAddMemberToFirstCohort() (gas: 339764)
-SecurityCouncilManagerTest:testAddMemberToSecondCohort() (gas: 343060)
-SecurityCouncilManagerTest:testAddSC() (gas: 118567)
-SecurityCouncilManagerTest:testAddSCAffordances() (gas: 112083)
-SecurityCouncilManagerTest:testCantUpdateCohortWithADup() (gas: 123116)
-SecurityCouncilManagerTest:testCohortMethods() (gas: 136185)
-SecurityCouncilManagerTest:testInitialization() (gas: 193074)
-SecurityCouncilManagerTest:testRemoveMember() (gas: 213029)
-SecurityCouncilManagerTest:testRemoveMemberAffordances() (gas: 99074)
-SecurityCouncilManagerTest:testRemoveSCAffordances() (gas: 81253)
-SecurityCouncilManagerTest:testRemoveSeC() (gas: 38309)
-SecurityCouncilManagerTest:testReplaceMemberAffordances() (gas: 208560)
-SecurityCouncilManagerTest:testReplaceMemberInFirstCohort() (gas: 258788)
-SecurityCouncilManagerTest:testReplaceMemberInSecondCohort() (gas: 262305)
-SecurityCouncilManagerTest:testRotateMember() (gas: 258792)
-SecurityCouncilManagerTest:testUpdateCohortAffordances() (gas: 83026)
-SecurityCouncilManagerTest:testUpdateFirstCohort() (gas: 295311)
-SecurityCouncilManagerTest:testUpdateRouter() (gas: 76296)
-SecurityCouncilManagerTest:testUpdateRouterAffordacnes() (gas: 112379)
-SecurityCouncilManagerTest:testUpdateSecondCohort() (gas: 295316)
+SecurityCouncilManagerTest:testAddMemberAffordances() (gas: 249787)
+SecurityCouncilManagerTest:testAddMemberSpecialAddresses() (gas: 20800)
+SecurityCouncilManagerTest:testAddMemberToFirstCohort() (gas: 340022)
+SecurityCouncilManagerTest:testAddMemberToSecondCohort() (gas: 343319)
+SecurityCouncilManagerTest:testAddSC() (gas: 118677)
+SecurityCouncilManagerTest:testAddSCAffordances() (gas: 112133)
+SecurityCouncilManagerTest:testCantUpdateCohortWithADup() (gas: 123130)
+SecurityCouncilManagerTest:testCohortMethods() (gas: 136182)
+SecurityCouncilManagerTest:testInitialization() (gas: 201641)
+SecurityCouncilManagerTest:testPostUpgradeInit() (gas: 5074706)
+SecurityCouncilManagerTest:testRemoveMember() (gas: 213142)
+SecurityCouncilManagerTest:testRemoveMemberAffordances() (gas: 99080)
+SecurityCouncilManagerTest:testRemoveSCAffordances() (gas: 81331)
+SecurityCouncilManagerTest:testRemoveSeC() (gas: 38350)
+SecurityCouncilManagerTest:testReplaceMemberAffordances() (gas: 208648)
+SecurityCouncilManagerTest:testReplaceMemberInFirstCohort() (gas: 258948)
+SecurityCouncilManagerTest:testReplaceMemberInSecondCohort() (gas: 262487)
+SecurityCouncilManagerTest:testRotateMember() (gas: 557872)
+SecurityCouncilManagerTest:testRotateMemberNotContender() (gas: 3587319)
+SecurityCouncilManagerTest:testSetMinRotationPeriod() (gas: 65822)
+SecurityCouncilManagerTest:testUpdateCohortAffordances() (gas: 83057)
+SecurityCouncilManagerTest:testUpdateFirstCohort() (gas: 295419)
+SecurityCouncilManagerTest:testUpdateRouter() (gas: 76302)
+SecurityCouncilManagerTest:testUpdateRouterAffordances() (gas: 112336)
+SecurityCouncilManagerTest:testUpdateSecondCohort() (gas: 295468)
 SecurityCouncilMemberElectionGovernorTest:testCannotUseMoreVotesThanAvailable() (gas: 246997)
 SecurityCouncilMemberElectionGovernorTest:testCastBySig() (gas: 302852)
 SecurityCouncilMemberElectionGovernorTest:testCastBySigTwice() (gas: 266244)
@@ -153,7 +156,7 @@ SecurityCouncilMemberElectionGovernorTest:testOnlyNomineeElectionGovernorCanProp
 SecurityCouncilMemberElectionGovernorTest:testProperInitialization() (gas: 49388)
 SecurityCouncilMemberElectionGovernorTest:testProposeReverts() (gas: 32916)
 SecurityCouncilMemberElectionGovernorTest:testRelay() (gas: 42229)
-SecurityCouncilMemberElectionGovernorTest:testSelectTopNominees(uint256) (runs: 258, μ: 339983, ~: 339822)
+SecurityCouncilMemberElectionGovernorTest:testSelectTopNominees(uint256) (runs: 256, μ: 340178, ~: 340008)
 SecurityCouncilMemberElectionGovernorTest:testSelectTopNomineesFails() (gas: 273335)
 SecurityCouncilMemberElectionGovernorTest:testSetFullWeightDuration() (gas: 34951)
 SecurityCouncilMemberElectionGovernorTest:testVotesToWeight() (gas: 152898)

src/security-council-mgmt/SecurityCouncilManager.sol

@@ -11,7 +11,22 @@ import "../UpgradeExecRouteBuilder.sol";
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "@openzeppelin/contracts/utils/Address.sol";
 import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
+import "@openzeppelin/contracts-upgradeable/governance/IGovernorUpgradeable.sol";
+import "@openzeppelin/contracts-upgradeable/utils/cryptography/draft-EIP712Upgradeable.sol";
 import "./Common.sol";
+import "./interfaces/ISecurityCouncilMemberElectionGovernor.sol";
+
+library ProxyUtil {
+    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)
+        }
+    }
+}
 
 /// @title  The Security Council Manager
 /// @notice The source of truth for an array of Security Councils that are under management.
@@ -28,7 +43,8 @@ import "./Common.sol";
 contract SecurityCouncilManager is
     Initializable,
     AccessControlUpgradeable,
-    ISecurityCouncilManager
+    ISecurityCouncilManager,
+    EIP712Upgradeable
 {
     event CohortReplaced(address[] newCohort, Cohort indexed cohort);
     event MemberAdded(address indexed newMember, Cohort indexed cohort);
@@ -46,6 +62,7 @@ contract SecurityCouncilManager is
         uint256 securityCouncilsLength
     );
     event UpgradeExecRouteBuilderSet(address indexed UpgradeExecRouteBuilder);
+    event MinRotationPeriodSet(uint256 minRotationPeriod);
 
     // The Security Council members are separated into two cohorts, allowing a whole cohort to be replaced, as
     // specified by the Arbitrum Constitution.
@@ -76,6 +93,13 @@ contract SecurityCouncilManager is
     /// @notice Size of cohort under ordinary circumstances
     uint256 public cohortSize;
 
+    /// @notice The timestamp at which the address was last rotated
+    mapping(address => uint256) public lastRotated;
+
+    /// @notice There is a minimum period between when an address can be rotated
+    ///         This is to ensure a single member cannot do many rotations in a row
+    uint256 public minRotationPeriod;
+
     /// @notice Magic value used by the L1 timelock to indicate that a retryable ticket should be created
     ///         Value is defined in L1ArbitrumTimelock contract https://etherscan.io/address/0xE6841D92B0C345144506576eC13ECf5103aC7f49#readProxyContract#F5
     address public constant RETRYABLE_TICKET_MAGIC = 0xa723C008e76E379c55599D2E4d93879BeaFDa79C;
@@ -85,6 +109,8 @@ contract SecurityCouncilManager is
     bytes32 public constant MEMBER_REPLACER_ROLE = keccak256("MEMBER_REPLACER");
     bytes32 public constant MEMBER_ROTATOR_ROLE = keccak256("MEMBER_ROTATOR");
     bytes32 public constant MEMBER_REMOVER_ROLE = keccak256("MEMBER_REMOVER");
+    bytes32 public constant MIN_ROTATION_PERIOD_SETTER_ROLE =
+        keccak256("MIN_ROATATION_PERIOD_SETTER");
 
     constructor() {
         _disableInitializers();
@@ -96,7 +122,8 @@ contract SecurityCouncilManager is
         SecurityCouncilData[] memory _securityCouncils,
         SecurityCouncilManagerRoles memory _roles,
         address payable _l2CoreGovTimelock,
-        UpgradeExecRouteBuilder _router
+        UpgradeExecRouteBuilder _router,
+        uint256 _minRotationPeriod
     ) external initializer {
         if (_firstCohort.length != _secondCohort.length) {
             revert CohortLengthMismatch(_firstCohort, _secondCohort);
@@ -112,6 +139,7 @@ contract SecurityCouncilManager is
         }
         _grantRole(MEMBER_ROTATOR_ROLE, _roles.memberRotator);
         _grantRole(MEMBER_REPLACER_ROLE, _roles.memberReplacer);
+        _grantRole(MIN_ROTATION_PERIOD_SETTER_ROLE, _roles.minRotationPeriodSetter);
 
         if (!Address.isContract(_l2CoreGovTimelock)) {
             revert NotAContract({account: _l2CoreGovTimelock});
@@ -122,6 +150,33 @@ contract SecurityCouncilManager is
         for (uint256 i = 0; i < _securityCouncils.length; i++) {
             _addSecurityCouncil(_securityCouncils[i]);
         }
+
+        setMinRotationPeriodImpl(_minRotationPeriod);
+
+        __EIP712_init_unchained("SecurityCouncilManager", "1");
+    }
+
+    function postUpgradeInit(uint256 _minRotationPeriod, address minRotationPeriodSetter)
+        external
+    {
+        address proxyAdmin = ProxyUtil.getProxyAdmin();
+        require(msg.sender == proxyAdmin, "NOT_FROM_ADMIN");
+
+        _grantRole(MIN_ROTATION_PERIOD_SETTER_ROLE, minRotationPeriodSetter);
+        setMinRotationPeriodImpl(_minRotationPeriod);
+    }
+
+    /// @inheritdoc ISecurityCouncilManager
+    function setMinRotationPeriod(uint256 _minRotationPeriod)
+        external
+        onlyRole(MIN_ROTATION_PERIOD_SETTER_ROLE)
+    {
+        setMinRotationPeriodImpl(_minRotationPeriod);
+    }
+
+    function setMinRotationPeriodImpl(uint256 _minRotationPeriod) internal {
+        minRotationPeriod = _minRotationPeriod;
+        emit MinRotationPeriodSet(_minRotationPeriod);
     }
 
     /// @inheritdoc ISecurityCouncilManager
@@ -207,16 +262,90 @@ contract SecurityCouncilManager is
     }
 
     /// @inheritdoc ISecurityCouncilManager
-    function rotateMember(address _currentAddress, address _newAddress)
-        external
-        onlyRole(MEMBER_ROTATOR_ROLE)
-    {
-        Cohort cohort = _swapMembers(_currentAddress, _newAddress);
-        emit MemberRotated({
-            replacedAddress: _currentAddress,
-            newAddress: _newAddress,
-            cohort: cohort
-        });
+    function getRotateMemberHash(address from, uint256 nonce) public view returns (bytes32) {
+        return _hashTypedDataV4(
+            keccak256(
+                abi.encode(keccak256("rotateMember(address from, uint256 nonce)"), from, nonce)
+            )
+        );
+    }
+
+    /// @inheritdoc ISecurityCouncilManager
+    function rotateMember(
+        address newMemberAddress,
+        address memberElectionGovernor,
+        bytes calldata signature
+    ) external {
+        uint256 lastRotatedTimestamp = lastRotated[msg.sender];
+        if (lastRotatedTimestamp != 0 && block.timestamp < lastRotatedTimestamp + minRotationPeriod)
+        {
+            revert RotationTooSoon(msg.sender, lastRotatedTimestamp + minRotationPeriod);
+        }
+
+        // we enforce that a the new address is an eoa in the same way do
+        // in NomineeGovernor.addContender by requiring a signature
+        bytes32 digest = getRotateMemberHash(msg.sender, updateNonce);
+        address newAddress = ECDSAUpgradeable.recover(digest, 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 agains the case where the wrong sig is accidentally used
+        if (newAddress != newMemberAddress) {
+            revert InvalidNewAddress(newAddress);
+        }
+
+        // the cohort replacer should be the member election governor
+        // we don't explicitly store the member election governor in this manager
+        // so we pass it in here and verify it as having the correct role
+        // since cohort replacing can change any member it's already a trusted entity
+        if (!hasRole(COHORT_REPLACER_ROLE, memberElectionGovernor)) {
+            revert GovernorNotReplacer();
+        }
+        // use the member election governor to get the nominee governor
+        // we we'll use that to check if there is a clash between the rotation and an ongoing election
+        ISecurityCouncilNomineeElectionGovernor nomineeGovernor =
+            ISecurityCouncilMemberElectionGovernor(memberElectionGovernor).nomineeElectionGovernor();
+        // election count is incremented after proposal, so the current election is electionCount - 1
+        // we use this to form the proposal id for that election, and then check isContender
+        uint256 electionCount = nomineeGovernor.electionCount();
+        // if the election count is still zero then no elections have started or taken place
+        // in that case it is always valid to rotate a member as there can be non clash with contenders
+        if (electionCount != 0) {
+            uint256 currentElectionIndex = electionCount - 1;
+            (
+                address[] memory targets,
+                uint256[] memory values,
+                bytes[] memory callDatas,
+                string memory description
+            ) = nomineeGovernor.getProposeArgs(currentElectionIndex);
+            uint256 proposalId = IGovernorUpgradeable(address(nomineeGovernor)).hashProposal(
+                targets, values, callDatas, keccak256(bytes(description))
+            );
+
+            // there can only be a clash with an incoming member if there is
+            // a. an ongoing election
+            // b. the election is for the other cohort than the member being rotated
+            // c. the address is a contender in that ongoing election
+            IGovernorUpgradeable.ProposalState nomineePropState =
+                IGovernorUpgradeable(address(nomineeGovernor)).state(proposalId);
+            if (
+                nomineePropState != IGovernorUpgradeable.ProposalState.Executed // the proposal is ongoing in nomination phase
+                    || (
+                        nomineePropState == IGovernorUpgradeable.ProposalState.Executed // the proposal has passed nomination phase but is still in member selection phase
+                            && IGovernorUpgradeable(memberElectionGovernor).state(proposalId)
+                                != IGovernorUpgradeable.ProposalState.Executed
+                    )
+            ) {
+                Cohort otherCohort = nomineeGovernor.otherCohort();
+                if (cohortIncludes(otherCohort, msg.sender)) {
+                    if (nomineeGovernor.isContender(proposalId, newAddress)) {
+                        revert NewMemberIsContender(proposalId, newAddress);
+                    }
+                }
+            }
+        }
+
+        lastRotated[newAddress] = block.timestamp;
+        Cohort cohort = _swapMembers(msg.sender, newAddress);
+        emit MemberRotated({replacedAddress: msg.sender, newAddress: newAddress, cohort: cohort});
     }
 
     function _swapMembers(address _addressToRemove, address _addressToAdd)

src/security-council-mgmt/factories/L2SecurityCouncilMgmtFactory.sol

@@ -41,6 +41,8 @@ struct DeployParams {
     uint256 nomineeVotingPeriod;
     uint256 memberVotingPeriod;
     uint256 fullWeightDuration;
+    uint256 minRotationPeriod;
+    address minRotationPeriodSetter;
 }
 
 struct ContractImplementations {
@@ -152,7 +154,8 @@ contract L2SecurityCouncilMgmtFactory is Ownable {
             memberAdder: dp.govChainEmergencySecurityCouncil,
             memberRemovers: memberRemovers,
             memberRotator: dp.govChainEmergencySecurityCouncil,
-            memberReplacer: dp.govChainEmergencySecurityCouncil
+            memberReplacer: dp.govChainEmergencySecurityCouncil,
+            minRotationPeriodSetter: dp.minRotationPeriodSetter
         });
 
         deployedContracts.upgradeExecRouteBuilder = new UpgradeExecRouteBuilder({
@@ -175,7 +178,8 @@ contract L2SecurityCouncilMgmtFactory is Ownable {
             _securityCouncils: dp.securityCouncils,
             _roles: roles,
             _l2CoreGovTimelock: payable(dp.l2CoreGovTimelock),
-            _router: deployedContracts.upgradeExecRouteBuilder
+            _router: deployedContracts.upgradeExecRouteBuilder,
+            _minRotationPeriod: dp.minRotationPeriod
         });
 
         _initRemovalGov(

src/security-council-mgmt/governors/SecurityCouncilMemberElectionGovernor.sol

@@ -24,7 +24,7 @@ contract SecurityCouncilMemberElectionGovernor is
     ElectionGovernor,
     ISecurityCouncilMemberElectionGovernor
 {
-    /// @notice The SecurityCouncilNomineeElectionGovernor that creates proposals for this governor and contains the list of compliant nominees
+    /// @inheritdoc ISecurityCouncilMemberElectionGovernor
     ISecurityCouncilNomineeElectionGovernor public nomineeElectionGovernor;
 
     /// @notice The SecurityCouncilManager that will execute the election result