addContender by signature only

.gas-snapshot

@@ -1,4 +1,5 @@
 AIP1Point2ActionTest:testAction() (gas: 629328)
+AIPXActionTest:testAction() (gas: 8153)
 ArbitrumDAOConstitutionTest:testConstructor() (gas: 259383)
 ArbitrumDAOConstitutionTest:testMonOwnerCannotSetHash() (gas: 262836)
 ArbitrumDAOConstitutionTest:testOwnerCanSetHash() (gas: 261148)
@@ -25,7 +26,7 @@ ArbitrumVestingWalletTest:testDelegateFailsForNonBeneficiary() (gas: 16008435)
 ArbitrumVestingWalletTest:testDoesDeploy() (gas: 15971342)
 ArbitrumVestingWalletTest:testReleaseAffordance() (gas: 16008649)
 ArbitrumVestingWalletTest:testVestedAmountStart() (gas: 16074917)
-E2E:testE2E() (gas: 83632522)
+E2E:testE2E() (gas: 84736756)
 FixedDelegateErc20WalletTest:testInit() (gas: 5822575)
 FixedDelegateErc20WalletTest:testInitZeroToken() (gas: 5816805)
 FixedDelegateErc20WalletTest:testTransfer() (gas: 5932218)
@@ -93,11 +94,11 @@ L2GovernanceFactoryTest:testSanityCheckValues() (gas: 28415658)
 L2GovernanceFactoryTest:testSetMinDelay() (gas: 28364371)
 L2GovernanceFactoryTest:testSetMinDelayRevertsForCoreAddress() (gas: 28417242)
 L2GovernanceFactoryTest:testUpgraderCanCancel() (gas: 28657360)
-L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 30363122)
-L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 30367353)
-L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 25498765)
-L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 30365353)
-L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 30384446)
+L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 30534838)
+L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 30539025)
+L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 25670482)
+L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 30537069)
+L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 30556162)
 OutboxActionsTest:testAddOutbxesAction() (gas: 651591)
 OutboxActionsTest:testCantAddEOA() (gas: 969161)
 OutboxActionsTest:testCantReAddOutbox() (gas: 974559)
@@ -142,7 +143,7 @@ SecurityCouncilMemberElectionGovernorTest:testOnlyNomineeElectionGovernorCanProp
 SecurityCouncilMemberElectionGovernorTest:testProperInitialization() (gas: 49388)
 SecurityCouncilMemberElectionGovernorTest:testProposeReverts() (gas: 32916)
 SecurityCouncilMemberElectionGovernorTest:testRelay() (gas: 42229)
-SecurityCouncilMemberElectionGovernorTest:testSelectTopNominees(uint256) (runs: 256, μ: 340004, ~: 339432)
+SecurityCouncilMemberElectionGovernorTest:testSelectTopNominees(uint256) (runs: 256, μ: 339963, ~: 339326)
 SecurityCouncilMemberElectionGovernorTest:testSelectTopNomineesFails() (gas: 273335)
 SecurityCouncilMemberElectionGovernorTest:testSetFullWeightDuration() (gas: 34951)
 SecurityCouncilMemberElectionGovernorTest:testVotesToWeight() (gas: 152898)
@@ -176,21 +177,21 @@ SecurityCouncilMemberSyncActionTest:testRemoveOne() (gas: 7930546)
 SecurityCouncilMemberSyncActionTest:testUpdateCohort() (gas: 8171934)
 SecurityCouncilMemberSyncActionTest:testUpdateCohort() (gas: 8172795)
 SecurityCouncilMgmtUtilsTests:testIsInArray() (gas: 2102)
-SecurityCouncilNomineeElectionGovernorTest:testAddContender() (gas: 215127)
-SecurityCouncilNomineeElectionGovernorTest:testCastBySig() (gas: 318348)
-SecurityCouncilNomineeElectionGovernorTest:testCastBySigTwice() (gas: 281704)
-SecurityCouncilNomineeElectionGovernorTest:testCastVoteReverts() (gas: 35255)
-SecurityCouncilNomineeElectionGovernorTest:testCountVote() (gas: 542695)
-SecurityCouncilNomineeElectionGovernorTest:testCreateElection() (gas: 253049)
-SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 431856)
-SecurityCouncilNomineeElectionGovernorTest:testExecute() (gas: 671552)
-SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 176798)
-SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 644396)
-SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7095881)
+SecurityCouncilNomineeElectionGovernorTest:testAddContender() (gas: 314166)
+SecurityCouncilNomineeElectionGovernorTest:testCastBySig() (gas: 334321)
+SecurityCouncilNomineeElectionGovernorTest:testCastBySigTwice() (gas: 297176)
+SecurityCouncilNomineeElectionGovernorTest:testCastVoteReverts() (gas: 35256)
+SecurityCouncilNomineeElectionGovernorTest:testCountVote() (gas: 584101)
+SecurityCouncilNomineeElectionGovernorTest:testCreateElection() (gas: 253095)
+SecurityCouncilNomineeElectionGovernorTest:testExcludeNominee() (gas: 457070)
+SecurityCouncilNomineeElectionGovernorTest:testExecute() (gas: 677169)
+SecurityCouncilNomineeElectionGovernorTest:testForceSupport() (gas: 195298)
+SecurityCouncilNomineeElectionGovernorTest:testIncludeNominee() (gas: 674563)
+SecurityCouncilNomineeElectionGovernorTest:testInvalidInit() (gas: 7267578)
 SecurityCouncilNomineeElectionGovernorTest:testProperInitialization() (gas: 78091)
-SecurityCouncilNomineeElectionGovernorTest:testProposeFails() (gas: 19744)
-SecurityCouncilNomineeElectionGovernorTest:testRelay() (gas: 42365)
-SecurityCouncilNomineeElectionGovernorTest:testSetNomineeVetter() (gas: 40019)
+SecurityCouncilNomineeElectionGovernorTest:testProposeFails() (gas: 19811)
+SecurityCouncilNomineeElectionGovernorTest:testRelay() (gas: 42431)
+SecurityCouncilNomineeElectionGovernorTest:testSetNomineeVetter() (gas: 39909)
 SequencerActionsTest:testAddAndRemoveSequencer() (gas: 483356)
 SequencerActionsTest:testCantAddZeroAddress() (gas: 235614)
 SetInitialGovParamsActionTest:testL1() (gas: 259904)
@@ -231,7 +232,7 @@ TokenDistributorTest:testZeroDelegateTo() (gas: 4132733)
 TokenDistributorTest:testZeroOwner() (gas: 4132646)
 TokenDistributorTest:testZeroReceiver() (gas: 4132675)
 TokenDistributorTest:testZeroToken() (gas: 71889)
-TopNomineesGasTest:testTopNomineesGas() (gas: 4502786)
+TopNomineesGasTest:testTopNomineesGas() (gas: 4502996)
 UpgradeExecRouteBuilderTest:testAIP1Point2() (gas: 1322645)
 UpgradeExecRouteBuilderTest:testRouteBuilderErrors() (gas: 1127374)
 UpgradeExecutorTest:testAdminCanChangeExecutor() (gas: 2583801)

foundry.toml

@@ -9,7 +9,7 @@ via_ir = false
 solc_version = '0.8.16'
 
 [profile.sec_council_mgmt]
-optimizer_runs = 1500
+optimizer_runs = 750
 
 [fmt]
 number_underscore = 'thousands'

hardhat.config.ts

@@ -18,7 +18,7 @@ const solidityProfiles = {
     settings: {
       optimizer: {
         enabled: true,
-        runs: 1500
+        runs: 750
       },
     }
   }

src/gov-action-contracts/AIPs/AIPXAction.sol → src/gov-action-contracts/AIPs/NomineeGovernorV2UpgradeAction.sol

@@ -11,7 +11,7 @@ import {
 } from "../../security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol";
 import {IArbitrumDAOConstitution} from "../../interfaces/IArbitrumDAOConstitution.sol";
 
-contract AIPXAction {
+contract NomineeGovernorV2UpgradeActionTemplate {
     address public immutable proxyAdmin;
     address public immutable nomineeElectionGovernorProxy;
     address public immutable newNomineeElectionGovernorImplementation;
@@ -54,3 +54,14 @@ contract AIPXAction {
         IArbitrumDAOConstitution(constitution).setConstitutionHash(newConstitutionHash);
     }
 }
+
+contract NomineeGovernorV2UpgradeAction is NomineeGovernorV2UpgradeActionTemplate {
+    constructor() NomineeGovernorV2UpgradeActionTemplate(
+        0xdb216562328215E010F819B5aBe947bad4ca961e,
+        0x8a1cDA8dee421cD06023470608605934c16A05a0,
+        address(0), // todo: new implementation
+        50400,
+        0x1D62fFeB72e4c360CcBbacf7c965153b00260417,
+        0x0101010101010101010101010101010101010101010101010101010101010101 // todo: new constitution hash
+    ) {}
+}

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

@@ -94,6 +94,8 @@ contract SecurityCouncilNomineeElectionGovernor is
     error QuorumNumeratorTooLow(uint256 quorumNumeratorValue);
     error CastVoteDisabled();
     error LastMemberElectionNotExecuted(uint256 prevProposalId);
+    error InvalidSignature();
+    error Deprecated(string message);
 
     constructor() {
         _disableInitializers();
@@ -208,18 +210,22 @@ contract SecurityCouncilNomineeElectionGovernor is
         }
     }
 
-    /// @notice Put `msg.sender` up for nomination. Must be called before a contender can receive votes.
-    ///         Contenders are expected to control an address than can create a signature that would be a
-    ///         recognised by a Gnosis Safe. They need to be able to do this with this same address on each of the
-    ///         chains where the Security Council is active. It is expected that the nominee vetter will check this
-    ///         during the vetting phase and exclude any contenders which dont meet this criteria.
+    /// @notice Put `contender` up for nomination. Must be called before a contender can receive votes.
+    /// @param  proposalId The id of the proposal
+    /// @param  contender The address of the contender
+    /// @param  signature EIP712 `AddContenderMessage(uint256 proposalId,address contender)` signed by `contender`
     /// @dev    Can be called only while a proposal is pending (after proposal created but before voting phase)
     ///         A contender cannot be a member of the opposite cohort.
-    function addContender(uint256 proposalId) external {
+    function addContender(uint256 proposalId, address contender, bytes calldata signature) external {
+        address signer = recoverAddContenderMessage(proposalId, contender, signature);
+        if (signer == address(0) || signer != contender) {
+            revert InvalidSignature();
+        }
+
         ElectionInfo storage election = _elections[proposalId];
 
-        if (election.isContender[msg.sender]) {
-            revert AlreadyContender(msg.sender);
+        if (election.isContender[signer]) {
+            revert AlreadyContender(signer);
         }
 
         ProposalState state_ = state(proposalId);
@@ -233,13 +239,13 @@ contract SecurityCouncilNomineeElectionGovernor is
         // 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(), msg.sender)) {
-            revert AccountInOtherCohort(otherCohort(), msg.sender);
+        if (securityCouncilManager.cohortIncludes(otherCohort(), signer)) {
+            revert AccountInOtherCohort(otherCohort(), signer);
         }
 
-        election.isContender[msg.sender] = true;
+        election.isContender[signer] = true;
 
-        emit ContenderAdded(proposalId, msg.sender);
+        emit ContenderAdded(proposalId, signer);
     }
 
     /// @notice Allows the owner to change the nomineeVetter
@@ -423,6 +429,16 @@ contract SecurityCouncilNomineeElectionGovernor is
         return _elections[proposalId].isContender[possibleContender];
     }
 
+    /// @notice Recover EIP712 signature for `AddContenderMessage`
+    function recoverAddContenderMessage(uint256 proposalId, address contender, bytes calldata signature) public view returns (address) {
+        bytes32 digest = _hashTypedDataV4(keccak256(abi.encode(
+            keccak256("AddContenderMessage(uint256 proposalId,address contender)"),
+            proposalId,
+            contender
+        )));
+        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`.
@@ -475,6 +491,13 @@ 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.

test/gov-actions/AIPXAction.t.sol → test/gov-actions/NomineeGovernorV2UpgradeAction.sol

@@ -3,10 +3,10 @@ pragma solidity 0.8.16;
 
 import "forge-std/Test.sol";
 
-import "../../src/gov-action-contracts/AIPs/AIPXAction.sol";
+import "../../src/gov-action-contracts/AIPs/NomineeGovernorV2UpgradeAction.sol";
 import "../../src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol";
 
-contract AIPXActionTest is Test {
+contract NomineeGovernorV2UpgradeActionTest is Test {
     address oldImplementation = 0x8436A1bc9f9f9EB0cF1B51942C5657b60A40CCDD;
     SecurityCouncilNomineeElectionGovernor gov = SecurityCouncilNomineeElectionGovernor(payable(0x8a1cDA8dee421cD06023470608605934c16A05a0));
     ProxyAdmin proxyAdmin = ProxyAdmin(0xdb216562328215E010F819B5aBe947bad4ca961e);
@@ -17,7 +17,7 @@ contract AIPXActionTest is Test {
 
     address newImplementation = address(new SecurityCouncilNomineeElectionGovernor());
     uint256 votingDelay = 7 days;
-    AIPXAction action = new AIPXAction(
+    NomineeGovernorV2UpgradeActionTemplate action = new NomineeGovernorV2UpgradeActionTemplate(
         address(proxyAdmin),
         address(gov),
         newImplementation,
@@ -28,12 +28,12 @@ contract AIPXActionTest is Test {
 
     function testAction() external {
         if (!_isForkTest()) {
-            console.log("not fork test, skipping AIPXActionTest");
+            console.log("not fork test, skipping NomineeGovernorV2UpgradeActionTest");
             return;
         }
 
         if (_getImplementation() != oldImplementation) {
-            console.log("implementation not set to old implementation, skipping AIPXActionTest");
+            console.log("implementation not set to old implementation, skipping NomineeGovernorV2UpgradeActionTest");
             return;
         }
 

test/security-council-mgmt/E2E.t.sol

@@ -19,6 +19,7 @@ import "@gnosis.pm/safe-contracts/contracts/proxies/GnosisSafeProxyFactory.sol";
 import "../../src/gov-action-contracts/address-registries/L2AddressRegistry.sol";
 import "../util/DeployGnosisWithModule.sol";
 import "../../src/security-council-mgmt/Common.sol";
+import "./governors/SecurityCouncilNomineeElectionGovernor.t.sol";
 
 contract ArbSysMock {
     event ArbSysL2ToL1Tx(address from, address to, uint256 value, bytes data);
@@ -94,24 +95,26 @@ contract E2E is Test, DeployGnosisWithModule {
     L2ArbitrumToken l2TokenLogic = new L2ArbitrumToken();
 
     // owners
-    address member1 = address(637);
-    address member2 = address(638);
-    address member3 = address(639);
-    address member4 = address(640);
-    address member5 = address(641);
-    address member6 = address(642);
-    address member7 = address(643);
-    address member8 = address(644);
-    address member9 = address(645);
-    address member10 = address(646);
-    address member11 = address(647);
-    address member12 = address(648);
-    address member13 = address(649);
-    address member14 = address(650);
-    address member15 = address(651);
-    address member16 = address(652);
-    address member17 = address(653);
-    address member18 = address(654);
+    address member1 = vm.addr(637);
+    address member2 = vm.addr(638);
+    address member3 = vm.addr(639);
+    address member4 = vm.addr(640);
+    address member5 = vm.addr(641);
+    address member6 = vm.addr(642);
+    address member7 = vm.addr(643);
+    address member8 = vm.addr(644);
+    address member9 = vm.addr(645);
+    address member10 = vm.addr(646);
+    address member11 = vm.addr(647);
+    address member12 = vm.addr(648);
+    address member13 = vm.addr(649);
+    address member14 = vm.addr(650);
+    address member15 = vm.addr(651);
+    address member16 = vm.addr(652);
+    address member17 = vm.addr(653);
+    address member18 = vm.addr(654);
+
+
 
     address[] members = [
         member1,
@@ -449,9 +452,10 @@ contract E2E is Test, DeployGnosisWithModule {
         uint256 propId = vars.secDeployedContracts.nomineeElectionGovernor.createElection();
 
         // put contenders up for election
+        SigUtils sigUtils = new SigUtils(address(vars.secDeployedContracts.nomineeElectionGovernor));
         for (uint256 i = 0; i < newCohort1.length; i++) {
-            vm.prank(newCohort1[i]);
-            vars.secDeployedContracts.nomineeElectionGovernor.addContender(propId);
+            uint256 pk = 649 + i; // member 13 - 18 priv keys
+            vars.secDeployedContracts.nomineeElectionGovernor.addContender(propId, vm.addr(pk), sigUtils.signAddContenderMessage(propId, pk));
         }
 
         // vote for them