increase nonemergency security council threshold action

.gas-snapshot

@@ -1,4 +1,7 @@
 AIP1Point2ActionTest:testAction() (gas: 629328)
+AIPIncreaseNonEmergencySCThresholdAction:testUnfoundConstitutionHash() (gas: 4919281)
+AIPIncreaseNonEmergencySCThresholdAction:testUpdateInitialHashIsOldHash1() (gas: 4937307)
+AIPIncreaseNonEmergencySCThresholdAction:testUpdateInitialHashIsOldHash2() (gas: 4937515)
 ArbitrumDAOConstitutionTest:testConstructor() (gas: 259383)
 ArbitrumDAOConstitutionTest:testMonOwnerCannotSetHash() (gas: 262836)
 ArbitrumDAOConstitutionTest:testOwnerCanSetHash() (gas: 261148)

src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol

@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity 0.8.16;
+
+import "../../governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol";
+import "../../../interfaces/IArbitrumDAOConstitution.sol";
+
+///@notice increase the non-emergency Security Council Threshold from 7 to 9 and update constitution accordingly.
+/// For discussion / rationale, see https://forum.arbitrum.foundation/t/rfc-constitutional-aip-security-council-improvement-proposal/20541
+/// Constitution hash updates depends on whether election change AIP passes; see https://forum.arbitrum.foundation/t/aip-changes-to-the-constitution-and-the-security-council-election-process/20856/13
+contract AIPIncreaseNonEmergencySCThresholdAction is
+    SetSCThresholdAndConditionallyUpdateConstitutionAction
+{
+    constructor()
+        SetSCThresholdAndConditionallyUpdateConstitutionAction(
+            IGnosisSafe(0xADd68bCb0f66878aB9D37a447C7b9067C5dfa941), // non emergency security council
+            7, // old threshold
+            9, // new threshold
+            IArbitrumDAOConstitution(address(0x1D62fFeB72e4c360CcBbacf7c965153b00260417)), // DAO constitution
+            bytes32(0x60acde40ad14f4ecdb1bea0704d1e3889264fb029231c9016352c670703b35d6), // 1. constitution hash: no election change, no threshold increase. https://github.com/ArbitrumFoundation/docs/tree/8071e3468cc0122e33c88ab7510c7c4320d35929
+            bytes32(""), // 2. constitution hash: no election change, yes threshold increase.  TODO link
+            bytes32(0xe794b7d0466ffd4a33321ea14c307b2de987c3229cf858727052a6f4b8a19cc1), // 3. constitution hash: yes election change, no threshold increase. https://github.com/ArbitrumFoundation/docs/tree/0837520dccc12e56a25f62de90ff9e3869196d05
+            bytes32("")
+        ) // 4. constitution hash: yes election change, yes threshold.  TODO link
+    // if 1, that means election change AIP didn't pass; apply threshold increase changes (2) on top of 1.
+    // if 3, that means election change AIP did pass; apply threshold increase changes (4) on top of 3.
+    {}
+}

src/gov-action-contracts/governance/ConstitutionActionLib.sol

@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity 0.8.16;
+
+import "../../interfaces/IArbitrumDAOConstitution.sol";
+
+library ConstitutionActionLib {
+    error ConstitutionHashNotSet();
+    error UnhandledConstitutionHash();
+    error ConstitutionHashLengthMismatch();
+
+    /// @notice Update dao constitution hash
+    /// @param constitution DAO constitution contract
+    /// @param _newConstitutionHash new constitution hash
+    function updateConstitutionHash(
+        IArbitrumDAOConstitution constitution,
+        bytes32 _newConstitutionHash
+    ) internal {
+        constitution.setConstitutionHash(_newConstitutionHash);
+        if (constitution.constitutionHash() != _newConstitutionHash) {
+            revert ConstitutionHashNotSet();
+        }
+    }
+
+    /// @notice checks actual constitution hash for presence in _oldConstitutionHashes and sets constitution hash to the hash in the corresponding index in _newConstitutionHashes if found
+    /// @param _constitution DAO constitution contract
+    /// @param _oldConstitutionHashes hashes to check against the current constitution
+    /// @param _newConstitutionHashes  hashes to set at corresponding index if hash in oldConstitutionHashes is found
+    function conditonallyUpdateConstitutionHash(
+        IArbitrumDAOConstitution _constitution,
+        bytes32[] memory _oldConstitutionHashes,
+        bytes32[] memory _newConstitutionHashes
+    ) internal returns (bytes32) {
+        bytes32 constitutionHash = _constitution.constitutionHash();
+        if (_oldConstitutionHashes.length != _newConstitutionHashes.length) {
+            revert ConstitutionHashLengthMismatch();
+        }
+
+        for (uint256 i = 0; i < _oldConstitutionHashes.length; i++) {
+            if (_oldConstitutionHashes[i] == constitutionHash) {
+                bytes32 newConstitutionHash = _newConstitutionHashes[i];
+                updateConstitutionHash(_constitution, newConstitutionHash);
+                return newConstitutionHash;
+            }
+        }
+        revert UnhandledConstitutionHash();
+    }
+}

src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol

@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity 0.8.16;
+
+import "../../security-council-mgmt/interfaces/IGnosisSafe.sol";
+import "../../interfaces/IArbitrumDAOConstitution.sol";
+import "./ConstitutionActionLib.sol";
+
+interface _IGnosisSafe {
+    function changeThreshold(uint256 _threshold) external;
+}
+
+///@notice Set the minimum signing threshold for a security council gnosis safe. Assumes that the safe has the UpgradeExecutor added as a module.
+/// Also conditionally updates constitution dependent on its current hash.
+contract SetSCThresholdAndConditionallyUpdateConstitutionAction {
+    IGnosisSafe public immutable gnosisSafe;
+    uint256 public immutable oldThreshold;
+    uint256 public immutable newThreshold;
+    IArbitrumDAOConstitution public immutable constitution;
+    bytes32 public immutable oldConstitutionHash1;
+    bytes32 public immutable newConstitutionHash1;
+    bytes32 public immutable oldConstitutionHash2;
+    bytes32 public immutable newConstitutionHash2;
+
+    event ActionPerformed(uint256 newThreshold, bytes32 newConstitutionHash);
+
+    constructor(
+        IGnosisSafe _gnosisSafe,
+        uint256 _oldThreshold,
+        uint256 _newThreshold,
+        IArbitrumDAOConstitution _constitution,
+        bytes32 _oldConstitutionHash1,
+        bytes32 _newConstitutionHash1,
+        bytes32 _oldConstitutionHash2,
+        bytes32 _newConstitutionHash2
+    ) {
+        gnosisSafe = _gnosisSafe;
+        oldThreshold = _oldThreshold;
+        newThreshold = _newThreshold;
+        constitution = _constitution;
+        oldConstitutionHash1 = _oldConstitutionHash1;
+        newConstitutionHash1 = _newConstitutionHash1;
+        oldConstitutionHash2 = _oldConstitutionHash2;
+        newConstitutionHash2 = _newConstitutionHash2;
+    }
+
+    function perform() external {
+        bytes32[] memory oldConstitutionHashes = new bytes32[](2);
+        oldConstitutionHashes[0] = oldConstitutionHash1;
+        oldConstitutionHashes[1] = oldConstitutionHash2;
+
+        bytes32[] memory newConstitutionHashes = new bytes32[](2);
+        newConstitutionHashes[0] = newConstitutionHash1;
+        newConstitutionHashes[1] = newConstitutionHash2;
+
+        ConstitutionActionLib.conditonallyUpdateConstitutionHash({
+            _constitution: constitution,
+            _oldConstitutionHashes: oldConstitutionHashes,
+            _newConstitutionHashes: newConstitutionHashes
+        });
+
+        // sanity check old threshold
+        require(
+            gnosisSafe.getThreshold() == oldThreshold, "SecSCThresholdAction: WRONG_OLD_THRESHOLD"
+        );
+
+        gnosisSafe.execTransactionFromModule({
+            to: address(gnosisSafe),
+            value: 0,
+            data: abi.encodeWithSelector(_IGnosisSafe.changeThreshold.selector, newThreshold),
+            operation: OpEnum.Operation.Call
+        });
+        // sanity check new threshold was set
+        require(
+            gnosisSafe.getThreshold() == newThreshold, "SecSCThresholdAction: NEW_THRESHOLD_NOT_SET"
+        );
+        emit ActionPerformed(newThreshold, constitution.constitutionHash());
+    }
+}

test/ArbitrumDAOConstitution.t.sol

@@ -10,7 +10,7 @@ contract ArbitrumDAOConstitutionTest is Test {
     bytes32 initialHash = bytes32("0x123");
     address owner = address(12_345);
 
-    function deployConstition() internal returns (ArbitrumDAOConstitution) {
+    function deployConstitution() internal returns (ArbitrumDAOConstitution) {
         vm.prank(owner);
         ArbitrumDAOConstitution arbitrumDAOConstitution = new ArbitrumDAOConstitution(
                 initialHash

test/gov-actions/AIPIncreaseNonEmergencySCThresholdAction.t.sol

@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity 0.8.16;
+
+import "forge-std/Test.sol";
+import
+    "../../src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol";
+import "../../src/gov-action-contracts/governance/ConstitutionActionLib.sol";
+import "../util/ActionTestBase.sol";
+import "../util/DeployGnosisWithModule.sol";
+
+contract AIPIncreaseNonEmergencySCThresholdAction is
+    Test,
+    ActionTestBase,
+    DeployGnosisWithModule
+{
+    uint256 oldThreshold = 1;
+    uint256 newThreshold = 2;
+    address[] owners = [address(123), address(456)];
+
+    bytes32 constHash1 = bytes32("0x1");
+    bytes32 constHash2 = bytes32("0x2");
+    bytes32 constHash3 = bytes32("0x3");
+    bytes32 constHash4 = bytes32("0x4");
+    bytes32 constHash5 = bytes32("0x5");
+
+    address safeAddress;
+
+    function runUpdate(
+        bytes32 _initialConstitutionHash,
+        bytes32 _oldConstitutionHash1,
+        bytes32 _newConstitutionHash1,
+        bytes32 _oldConstitutionHash2,
+        bytes32 _newConstitutionHash2
+    ) public {
+        safeAddress = deploySafe(owners, oldThreshold, address(arbOneUe));
+
+        vm.prank(address(arbOneUe));
+        arbitrumDAOConstitution.setConstitutionHash(_initialConstitutionHash);
+        assertEq(
+            arbitrumDAOConstitution.constitutionHash(),
+            _initialConstitutionHash,
+            "initial constitution hash set"
+        );
+
+        address action = address(
+            new SetSCThresholdAndConditionallyUpdateConstitutionAction({
+                _gnosisSafe: IGnosisSafe(safeAddress),
+                _oldThreshold: oldThreshold,
+                _newThreshold: newThreshold,
+                _constitution: IArbitrumDAOConstitution(address(arbitrumDAOConstitution)),
+                _oldConstitutionHash1: _oldConstitutionHash1,
+                _newConstitutionHash1: _newConstitutionHash1,
+                _oldConstitutionHash2: _oldConstitutionHash2,
+                _newConstitutionHash2: _newConstitutionHash2
+            })
+        );
+
+        vm.prank(executor2);
+        arbOneUe.execute(
+            action,
+            abi.encodeWithSelector(
+                SetSCThresholdAndConditionallyUpdateConstitutionAction.perform.selector
+            )
+        );
+    }
+
+    function testUpdateInitialHashIsOldHash1() public {
+        runUpdate({
+            _initialConstitutionHash: constHash1,
+            _oldConstitutionHash1: constHash1,
+            _newConstitutionHash1: constHash2,
+            _oldConstitutionHash2: constHash3,
+            _newConstitutionHash2: constHash4
+        });
+        assertEq(
+            arbitrumDAOConstitution.constitutionHash(), constHash2, "proper constitution hash set"
+        );
+        assertEq(IGnosisSafe(safeAddress).getThreshold(), newThreshold, "new threshold set");
+    }
+
+    function testUpdateInitialHashIsOldHash2() public {
+        runUpdate({
+            _initialConstitutionHash: constHash3,
+            _oldConstitutionHash1: constHash1,
+            _newConstitutionHash1: constHash2,
+            _oldConstitutionHash2: constHash3,
+            _newConstitutionHash2: constHash4
+        });
+        assertEq(
+            arbitrumDAOConstitution.constitutionHash(), constHash4, "proper constitution hash set"
+        );
+        assertEq(IGnosisSafe(safeAddress).getThreshold(), newThreshold, "new threshold set");
+    }
+
+    function testUnfoundConstitutionHash() public {
+        safeAddress = deploySafe(owners, oldThreshold, address(arbOneUe));
+        vm.prank(address(arbOneUe));
+        arbitrumDAOConstitution.setConstitutionHash(constHash1);
+        assertEq(
+            arbitrumDAOConstitution.constitutionHash(), constHash1, "initial constitution hash set"
+        );
+        address action = address(
+            new SetSCThresholdAndConditionallyUpdateConstitutionAction({
+                _gnosisSafe: IGnosisSafe(safeAddress),
+                _oldThreshold: oldThreshold,
+                _newThreshold: newThreshold,
+                _constitution: IArbitrumDAOConstitution(address(arbitrumDAOConstitution)),
+                _oldConstitutionHash1: constHash2,
+                _newConstitutionHash1: constHash3,
+                _oldConstitutionHash2: constHash4,
+                _newConstitutionHash2: constHash5
+            })
+        );
+        vm.expectRevert(
+            abi.encodeWithSelector(ConstitutionActionLib.UnhandledConstitutionHash.selector)
+        );
+        vm.prank(executor2);
+        arbOneUe.execute(
+            action,
+            abi.encodeWithSelector(
+                SetSCThresholdAndConditionallyUpdateConstitutionAction.perform.selector
+            )
+        );
+    }
+}