From 0fb34edca2ad2f92bbdac498c7adf77cdb6ad9ee Mon Sep 17 00:00:00 2001 From: dzgoldman Date: Thu, 11 Jan 2024 12:32:31 -0500 Subject: [PATCH 01/20] sc improvement action contracts --- .../AIPIncreaseCoreTimelockDelayAction.sol | 16 +++++++++ ...PIncreaseNonEmergencySCThresholdAction.sol | 12 +++++++ .../governance/SetSCThresholdAction.sol | 33 +++++++++++++++++++ .../UpdateCoreTimelockDelayAction.sol | 21 ++++++++++++ 4 files changed, 82 insertions(+) create mode 100644 src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseCoreTimelockDelayAction.sol create mode 100644 src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol create mode 100644 src/gov-action-contracts/governance/SetSCThresholdAction.sol create mode 100644 src/gov-action-contracts/governance/UpdateCoreTimelockDelayAction.sol diff --git a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseCoreTimelockDelayAction.sol b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseCoreTimelockDelayAction.sol new file mode 100644 index 000000000..aec11c0b0 --- /dev/null +++ b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseCoreTimelockDelayAction.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.16; + +import "../../governance/UpdateCoreTimelockDelayAction.sol"; +import "../../address-registries/L2AddressRegistry.sol"; + +///@notice Increase core timelock day to eight days. +/// For discussion / rationale, see https://forum.arbitrum.foundation/t/rfc-constitutional-aip-security-council-improvement-proposal/20541 +contract AIPIncreaseCoreTimelockDelayAction is UpdateCoreTimelockDelayAction { + constructor() + UpdateCoreTimelockDelayAction( + ICoreGovTimelockGetter(0x56C4E9Eb6c63aCDD19AeC2b1a00e4f0d7aBda9d3), + 8 days + ) + {} +} diff --git a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol new file mode 100644 index 000000000..71cee407d --- /dev/null +++ b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.16; + +import "../../governance/SetSCThresholdAction.sol"; + +///@notice increase the non-emergency Security Council Threshold from 7 to 9. +/// For discussion / rationale, see https://forum.arbitrum.foundation/t/rfc-constitutional-aip-security-council-improvement-proposal/20541 +contract AIPIncreaseNonEmergencySCThresholdAction is SetSCThresholdAction { + constructor() + SetSCThresholdAction(IGnosisSafe(0xADd68bCb0f66878aB9D37a447C7b9067C5dfa941), 7, 9) + {} +} diff --git a/src/gov-action-contracts/governance/SetSCThresholdAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAction.sol new file mode 100644 index 000000000..83a97cdd0 --- /dev/null +++ b/src/gov-action-contracts/governance/SetSCThresholdAction.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.16; + +interface IGnosisSafe { + function getThreshold() external view returns (uint256); + 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. +contract SetSCThresholdAction { + IGnosisSafe public immutable gnosisSafe; + uint256 public immutable oldThreshold; + uint256 public immutable newThreshold; + + constructor(IGnosisSafe _gnosisSafe, uint256 _oldThreshold, uint256 _newThreshold) { + gnosisSafe = _gnosisSafe; + oldThreshold = _oldThreshold; + newThreshold = _newThreshold; + } + + function perform() external { + // sanity check old threshold + require( + gnosisSafe.getThreshold() == oldThreshold, "SecSCThresholdAction: WRONG_OLD_THRESHOLD" + ); + + gnosisSafe.changeThreshold(newThreshold); + // sanity check new threshold was set + require( + gnosisSafe.getThreshold() == newThreshold, "SecSCThresholdAction: NEW_THRESHOLD_NOT_SET" + ); + } +} diff --git a/src/gov-action-contracts/governance/UpdateCoreTimelockDelayAction.sol b/src/gov-action-contracts/governance/UpdateCoreTimelockDelayAction.sol new file mode 100644 index 000000000..cb2f913e9 --- /dev/null +++ b/src/gov-action-contracts/governance/UpdateCoreTimelockDelayAction.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.16; + +import "../address-registries/L2AddressRegistry.sol"; + +///@notice Update core timelock delay — the minimum amount of time after a passed-proposal is queued before it can be executed. +contract UpdateCoreTimelockDelayAction { + IArbitrumTimelock public immutable timelock; + uint256 public immutable newDelay; + + constructor(ICoreGovTimelockGetter _l2AddressRegistry, uint256 _newDelay) { + timelock = _l2AddressRegistry.coreGovTimelock(); + newDelay = _newDelay; + } + + function perform() external { + timelock.updateDelay(newDelay); + // sanity check: + require(timelock.getMinDelay() == newDelay, "UpdateTimelockDelayAction: DELAY_NOT_SET"); + } +} From 86410f81194163afe82768595462ec632706219b Mon Sep 17 00:00:00 2001 From: dzgoldman Date: Thu, 11 Jan 2024 13:39:39 -0500 Subject: [PATCH 02/20] add non-emergency council removal action --- .../AIPRemoveNonEmergencySCAction.sol | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol diff --git a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol new file mode 100644 index 000000000..29a820cd8 --- /dev/null +++ b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.16; + +import "../../../security-council-mgmt/interfaces/ISecurityCouncilManager.sol"; +import "../../../interfaces/ICoreTimelock.sol"; + +///@notice Effectively "remove" the non emergency security council; prevent it from proposing in the timelock and don't update it in security council elections +contract AIPRemoveNonEmergencySCAction { + ISecurityCouncilManager public constant securityCouncilManager = + ISecurityCouncilManager(0xD509E5f5aEe2A205F554f36E8a7d56094494eDFC); + ICoreTimelock public constant timelock = + ICoreTimelock(0x34d45e99f7D8c45ed05B5cA72D54bbD1fb3F98f0); + address nonEmergecySC = 0xADd68bCb0f66878aB9D37a447C7b9067C5dfa941; + + function perform() external { + // revoke SC's role on timelock + timelock.revokeRole(timelock.PROPOSER_ROLE(), nonEmergecySC); + + // remove SC from elections + securityCouncilManager.removeSecurityCouncil( + SecurityCouncilData({ + securityCouncil: nonEmergecySC, + updateAction: 0x9BF7b8884Fa381a45f8CB2525905fb36C996297a, + chainId: 42_161 + }) + ); + } +} From cae969f822862d3bc0e25fc32ad3f2bff47d2a4c Mon Sep 17 00:00:00 2001 From: dzgoldman Date: Thu, 11 Jan 2024 15:01:11 -0500 Subject: [PATCH 03/20] fix SetSCThresholdAction; use module --- .../governance/SetSCThresholdAction.sol | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/gov-action-contracts/governance/SetSCThresholdAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAction.sol index 83a97cdd0..c13287f92 100644 --- a/src/gov-action-contracts/governance/SetSCThresholdAction.sol +++ b/src/gov-action-contracts/governance/SetSCThresholdAction.sol @@ -1,8 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity 0.8.16; -interface IGnosisSafe { - function getThreshold() external view returns (uint256); +import "../../security-council-mgmt/interfaces/IGnosisSafe.sol"; + +interface _IGnosisSafe { function changeThreshold(uint256 _threshold) external; } @@ -24,7 +25,12 @@ contract SetSCThresholdAction { gnosisSafe.getThreshold() == oldThreshold, "SecSCThresholdAction: WRONG_OLD_THRESHOLD" ); - gnosisSafe.changeThreshold(newThreshold); + 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" From ce429867cc8fb6ec9fbbe3057400ccde3f4fdefd Mon Sep 17 00:00:00 2001 From: dzgoldman Date: Thu, 11 Jan 2024 15:02:40 -0500 Subject: [PATCH 04/20] add comment --- .../AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol index 29a820cd8..42a785bcb 100644 --- a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol +++ b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol @@ -5,6 +5,7 @@ import "../../../security-council-mgmt/interfaces/ISecurityCouncilManager.sol"; import "../../../interfaces/ICoreTimelock.sol"; ///@notice Effectively "remove" the non emergency security council; prevent it from proposing in the timelock and don't update it in security council elections +/// For discussion / rationale, see https://forum.arbitrum.foundation/t/rfc-constitutional-aip-security-council-improvement-proposal/20541 contract AIPRemoveNonEmergencySCAction { ISecurityCouncilManager public constant securityCouncilManager = ISecurityCouncilManager(0xD509E5f5aEe2A205F554f36E8a7d56094494eDFC); From bb0a56dbd3b35e62f7a1c02bbdaa5cf0e7d801b3 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 5 Feb 2024 11:44:24 -0500 Subject: [PATCH 05/20] AIPIncreaseNonEmergencySCThresholdAction --- .../AIPIncreaseCoreTimelockDelayAction.sol | 16 ----- ...PIncreaseNonEmergencySCThresholdAction.sol | 21 ++++-- .../AIPRemoveNonEmergencySCAction.sol | 29 -------- .../governance/ConstitutionActionLib.sol | 45 ++++++++++++ .../governance/SetSCThresholdAction.sol | 39 ---------- ...dConditionallyUpdateConstitutionAction.sol | 72 +++++++++++++++++++ 6 files changed, 134 insertions(+), 88 deletions(-) delete mode 100644 src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseCoreTimelockDelayAction.sol delete mode 100644 src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol create mode 100644 src/gov-action-contracts/governance/ConstitutionActionLib.sol delete mode 100644 src/gov-action-contracts/governance/SetSCThresholdAction.sol create mode 100644 src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol diff --git a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseCoreTimelockDelayAction.sol b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseCoreTimelockDelayAction.sol deleted file mode 100644 index aec11c0b0..000000000 --- a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseCoreTimelockDelayAction.sol +++ /dev/null @@ -1,16 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -pragma solidity 0.8.16; - -import "../../governance/UpdateCoreTimelockDelayAction.sol"; -import "../../address-registries/L2AddressRegistry.sol"; - -///@notice Increase core timelock day to eight days. -/// For discussion / rationale, see https://forum.arbitrum.foundation/t/rfc-constitutional-aip-security-council-improvement-proposal/20541 -contract AIPIncreaseCoreTimelockDelayAction is UpdateCoreTimelockDelayAction { - constructor() - UpdateCoreTimelockDelayAction( - ICoreGovTimelockGetter(0x56C4E9Eb6c63aCDD19AeC2b1a00e4f0d7aBda9d3), - 8 days - ) - {} -} diff --git a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol index 71cee407d..2f2373625 100644 --- a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol +++ b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol @@ -1,12 +1,25 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity 0.8.16; -import "../../governance/SetSCThresholdAction.sol"; +import "../../governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol"; +import "../../../interfaces/IArbitrumDAOConstitution.sol"; -///@notice increase the non-emergency Security Council Threshold from 7 to 9. + +///@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 -contract AIPIncreaseNonEmergencySCThresholdAction is SetSCThresholdAction { +/// 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() - SetSCThresholdAction(IGnosisSafe(0xADd68bCb0f66878aB9D37a447C7b9067C5dfa941), 7, 9) + 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. {} } diff --git a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol deleted file mode 100644 index 42a785bcb..000000000 --- a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPRemoveNonEmergencySCAction.sol +++ /dev/null @@ -1,29 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -pragma solidity 0.8.16; - -import "../../../security-council-mgmt/interfaces/ISecurityCouncilManager.sol"; -import "../../../interfaces/ICoreTimelock.sol"; - -///@notice Effectively "remove" the non emergency security council; prevent it from proposing in the timelock and don't update it in security council elections -/// For discussion / rationale, see https://forum.arbitrum.foundation/t/rfc-constitutional-aip-security-council-improvement-proposal/20541 -contract AIPRemoveNonEmergencySCAction { - ISecurityCouncilManager public constant securityCouncilManager = - ISecurityCouncilManager(0xD509E5f5aEe2A205F554f36E8a7d56094494eDFC); - ICoreTimelock public constant timelock = - ICoreTimelock(0x34d45e99f7D8c45ed05B5cA72D54bbD1fb3F98f0); - address nonEmergecySC = 0xADd68bCb0f66878aB9D37a447C7b9067C5dfa941; - - function perform() external { - // revoke SC's role on timelock - timelock.revokeRole(timelock.PROPOSER_ROLE(), nonEmergecySC); - - // remove SC from elections - securityCouncilManager.removeSecurityCouncil( - SecurityCouncilData({ - securityCouncil: nonEmergecySC, - updateAction: 0x9BF7b8884Fa381a45f8CB2525905fb36C996297a, - chainId: 42_161 - }) - ); - } -} diff --git a/src/gov-action-contracts/governance/ConstitutionActionLib.sol b/src/gov-action-contracts/governance/ConstitutionActionLib.sol new file mode 100644 index 000000000..d59c3b9a3 --- /dev/null +++ b/src/gov-action-contracts/governance/ConstitutionActionLib.sol @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.16; + +import "../../interfaces/IArbitrumDAOConstitution.sol"; + +library ConstitutionActionLib { + error ConstitutionHashNotSet(); + error UnhandledConstitutionHash(); + + /// @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 sets the consitution hash to _newConstitutionHash1 if it's currently _oldConstitutionHash1 and sets it to _newConstitutionHash2 if it's currently _oldConstitutionHash2 + /// @param _constitution DAO constitution contract + /// @param _oldConstitutionHash1 potential constitution hash to be changed + /// @param _newConstitutionHash1 potential new constitution hash + /// @param _oldConstitutionHash2 potential constitution hash to be changed + /// @param _newConstitutionHash2 potential new constitution hash + function conditonallyUpdateConstitutionHash( + IArbitrumDAOConstitution _constitution, + bytes32 _oldConstitutionHash1, + bytes32 _newConstitutionHash1, + bytes32 _oldConstitutionHash2, + bytes32 _newConstitutionHash2 + ) internal { + bytes32 constitutionHash = _constitution.constitutionHash(); + if (constitutionHash == _oldConstitutionHash1) { + updateConstitutionHash(_constitution, _newConstitutionHash1); + } else if (constitutionHash == _oldConstitutionHash2) { + updateConstitutionHash(_constitution, _newConstitutionHash2); + } else { + revert UnhandledConstitutionHash(); + } + } +} diff --git a/src/gov-action-contracts/governance/SetSCThresholdAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAction.sol deleted file mode 100644 index c13287f92..000000000 --- a/src/gov-action-contracts/governance/SetSCThresholdAction.sol +++ /dev/null @@ -1,39 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -pragma solidity 0.8.16; - -import "../../security-council-mgmt/interfaces/IGnosisSafe.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. -contract SetSCThresholdAction { - IGnosisSafe public immutable gnosisSafe; - uint256 public immutable oldThreshold; - uint256 public immutable newThreshold; - - constructor(IGnosisSafe _gnosisSafe, uint256 _oldThreshold, uint256 _newThreshold) { - gnosisSafe = _gnosisSafe; - oldThreshold = _oldThreshold; - newThreshold = _newThreshold; - } - - function perform() external { - // 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" - ); - } -} diff --git a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol new file mode 100644 index 000000000..620b35aa5 --- /dev/null +++ b/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol @@ -0,0 +1,72 @@ +// 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 constitution; + bytes32 oldConstitutionHash1; + bytes32 newConstitutionHash1; + bytes32 oldConstitutionHash2; + bytes32 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 = _oldConstitutionHash; + newConstitutionHash1 = _newConstitutionHash; + oldConstitutionHash2 = oldConstitutionHash2; + newConstitutionHash2 = newConstitutionHash2; + } + + function perform() external { + ConstitutionActionLib.conditonallyUpdateConstitutionHash({ + _constitution: constitution, + _oldConstitutionHash1: oldConstitutionHash1, + _newConstitutionHash1: newConstitutionHash1, + _oldConstitutionHash2: oldConstitutionHash2, + _newConstitutionHash2: newConstitutionHash2 + }); + + // 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()); + } +} From f0af9f65c40d53e79b38cd7434a161d48664aba4 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 5 Feb 2024 11:45:48 -0500 Subject: [PATCH 06/20] fix vars --- ...SetSCThresholdAndConditionallyUpdateConstitutionAction.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol index 620b35aa5..23e2863f1 100644 --- a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol +++ b/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol @@ -37,8 +37,8 @@ contract SetSCThresholdAndConditionallyUpdateConstitutionAction { oldThreshold = _oldThreshold; newThreshold = _newThreshold; constitution = _constitution; - oldConstitutionHash1 = _oldConstitutionHash; - newConstitutionHash1 = _newConstitutionHash; + oldConstitutionHash1 = _oldConstitutionHash1; + newConstitutionHash1 = _newConstitutionHash2; oldConstitutionHash2 = oldConstitutionHash2; newConstitutionHash2 = newConstitutionHash2; } From 2b62774a3ec535633ed4004decea18044eb8c7ac Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 5 Feb 2024 11:47:05 -0500 Subject: [PATCH 07/20] fix vars --- ...tSCThresholdAndConditionallyUpdateConstitutionAction.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol index 23e2863f1..137004cc4 100644 --- a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol +++ b/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol @@ -38,9 +38,9 @@ contract SetSCThresholdAndConditionallyUpdateConstitutionAction { newThreshold = _newThreshold; constitution = _constitution; oldConstitutionHash1 = _oldConstitutionHash1; - newConstitutionHash1 = _newConstitutionHash2; - oldConstitutionHash2 = oldConstitutionHash2; - newConstitutionHash2 = newConstitutionHash2; + newConstitutionHash1 = _newConstitutionHash1; + oldConstitutionHash2 = _oldConstitutionHash2; + newConstitutionHash2 = _newConstitutionHash2; } function perform() external { From fce75bfcd58c76909b25c8a77377d7725b104de9 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 5 Feb 2024 11:48:08 -0500 Subject: [PATCH 08/20] remove UpdateCoreTimelockDelayAction --- .../UpdateCoreTimelockDelayAction.sol | 21 ------------------- 1 file changed, 21 deletions(-) delete mode 100644 src/gov-action-contracts/governance/UpdateCoreTimelockDelayAction.sol diff --git a/src/gov-action-contracts/governance/UpdateCoreTimelockDelayAction.sol b/src/gov-action-contracts/governance/UpdateCoreTimelockDelayAction.sol deleted file mode 100644 index cb2f913e9..000000000 --- a/src/gov-action-contracts/governance/UpdateCoreTimelockDelayAction.sol +++ /dev/null @@ -1,21 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -pragma solidity 0.8.16; - -import "../address-registries/L2AddressRegistry.sol"; - -///@notice Update core timelock delay — the minimum amount of time after a passed-proposal is queued before it can be executed. -contract UpdateCoreTimelockDelayAction { - IArbitrumTimelock public immutable timelock; - uint256 public immutable newDelay; - - constructor(ICoreGovTimelockGetter _l2AddressRegistry, uint256 _newDelay) { - timelock = _l2AddressRegistry.coreGovTimelock(); - newDelay = _newDelay; - } - - function perform() external { - timelock.updateDelay(newDelay); - // sanity check: - require(timelock.getMinDelay() == newDelay, "UpdateTimelockDelayAction: DELAY_NOT_SET"); - } -} From 6a502e2c5927d119e51504beb03e011577991312 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 5 Feb 2024 11:50:06 -0500 Subject: [PATCH 09/20] lint --- ...PIncreaseNonEmergencySCThresholdAction.sol | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol index 2f2373625..32ae00cbd 100644 --- a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol +++ b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol @@ -4,22 +4,24 @@ 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. +///@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 { +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. + 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. {} } From 1d1478bf88216b77e2b0d0814b7069540b43177f Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 5 Feb 2024 13:20:16 -0500 Subject: [PATCH 10/20] make all vars immutable; add unit tests --- ...dConditionallyUpdateConstitutionAction.sol | 10 +- ...ncreaseNonEmergencySCThresholdAction.t.sol | 125 ++++++++++++++++++ 2 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 test/gov-actions/AIPIncreaseNonEmergencySCThresholdAction.t.sol diff --git a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol index 137004cc4..f547eaaeb 100644 --- a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol +++ b/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol @@ -15,11 +15,11 @@ contract SetSCThresholdAndConditionallyUpdateConstitutionAction { IGnosisSafe public immutable gnosisSafe; uint256 public immutable oldThreshold; uint256 public immutable newThreshold; - IArbitrumDAOConstitution constitution; - bytes32 oldConstitutionHash1; - bytes32 newConstitutionHash1; - bytes32 oldConstitutionHash2; - bytes32 newConstitutionHash2; + 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); diff --git a/test/gov-actions/AIPIncreaseNonEmergencySCThresholdAction.t.sol b/test/gov-actions/AIPIncreaseNonEmergencySCThresholdAction.t.sol new file mode 100644 index 000000000..755ef92db --- /dev/null +++ b/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 + ) + ); + } +} From 90ec167ba7bba9fadff9cd3875222ebfccd12f34 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 5 Feb 2024 13:20:23 -0500 Subject: [PATCH 11/20] fix typo --- test/ArbitrumDAOConstitution.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ArbitrumDAOConstitution.t.sol b/test/ArbitrumDAOConstitution.t.sol index 41fdaee13..01152ac2f 100644 --- a/test/ArbitrumDAOConstitution.t.sol +++ b/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 From 5fd270e3264ac7540de0e717e5b020655806beb5 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 5 Feb 2024 13:21:05 -0500 Subject: [PATCH 12/20] update snapshot --- .gas-snapshot | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gas-snapshot b/.gas-snapshot index fbffee028..342523dd2 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,4 +1,7 @@ AIP1Point2ActionTest:testAction() (gas: 629328) +AIPIncreaseNonEmergencySCThresholdAction:testUnfoundConstitutionHash() (gas: 4825091) +AIPIncreaseNonEmergencySCThresholdAction:testUpdateInitialHashIsOldHash1() (gas: 4843318) +AIPIncreaseNonEmergencySCThresholdAction:testUpdateInitialHashIsOldHash2() (gas: 4843363) ArbitrumDAOConstitutionTest:testConstructor() (gas: 259383) ArbitrumDAOConstitutionTest:testMonOwnerCannotSetHash() (gas: 262836) ArbitrumDAOConstitutionTest:testOwnerCanSetHash() (gas: 261148) From 1f0455b23e34913ac4cb90bee89fbd003082aa27 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 5 Feb 2024 16:09:44 -0500 Subject: [PATCH 13/20] generalize conditonallyUpdateConstitutionHash to array --- .../governance/ConstitutionActionLib.sol | 34 ++++++++++--------- ...dConditionallyUpdateConstitutionAction.sol | 14 +++++--- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/gov-action-contracts/governance/ConstitutionActionLib.sol b/src/gov-action-contracts/governance/ConstitutionActionLib.sol index d59c3b9a3..afd8cdf53 100644 --- a/src/gov-action-contracts/governance/ConstitutionActionLib.sol +++ b/src/gov-action-contracts/governance/ConstitutionActionLib.sol @@ -6,6 +6,7 @@ import "../../interfaces/IArbitrumDAOConstitution.sol"; library ConstitutionActionLib { error ConstitutionHashNotSet(); error UnhandledConstitutionHash(); + error ConstitutionHashLengthMismatch(); /// @notice Update dao constitution hash /// @param constitution DAO constitution contract @@ -20,26 +21,27 @@ library ConstitutionActionLib { } } - /// @notice sets the consitution hash to _newConstitutionHash1 if it's currently _oldConstitutionHash1 and sets it to _newConstitutionHash2 if it's currently _oldConstitutionHash2 + /// @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 _oldConstitutionHash1 potential constitution hash to be changed - /// @param _newConstitutionHash1 potential new constitution hash - /// @param _oldConstitutionHash2 potential constitution hash to be changed - /// @param _newConstitutionHash2 potential new constitution hash + /// @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 _oldConstitutionHash1, - bytes32 _newConstitutionHash1, - bytes32 _oldConstitutionHash2, - bytes32 _newConstitutionHash2 - ) internal { + bytes32[] memory _oldConstitutionHashes, + bytes32[] memory _newConstitutionHashes + ) internal returns (bytes32) { bytes32 constitutionHash = _constitution.constitutionHash(); - if (constitutionHash == _oldConstitutionHash1) { - updateConstitutionHash(_constitution, _newConstitutionHash1); - } else if (constitutionHash == _oldConstitutionHash2) { - updateConstitutionHash(_constitution, _newConstitutionHash2); - } else { - revert UnhandledConstitutionHash(); + 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(); } } diff --git a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol index f547eaaeb..7f476e35d 100644 --- a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol +++ b/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol @@ -44,12 +44,18 @@ contract SetSCThresholdAndConditionallyUpdateConstitutionAction { } 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, - _oldConstitutionHash1: oldConstitutionHash1, - _newConstitutionHash1: newConstitutionHash1, - _oldConstitutionHash2: oldConstitutionHash2, - _newConstitutionHash2: newConstitutionHash2 + _oldConstitutionHashes: oldConstitutionHashes, + _newConstitutionHashes: newConstitutionHashes }); // sanity check old threshold From 6a7178ead38d068cc879bf47c934289a00a3a087 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 5 Feb 2024 16:10:16 -0500 Subject: [PATCH 14/20] update snapshot --- .gas-snapshot | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 342523dd2..42aeaca81 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,7 +1,7 @@ AIP1Point2ActionTest:testAction() (gas: 629328) -AIPIncreaseNonEmergencySCThresholdAction:testUnfoundConstitutionHash() (gas: 4825091) -AIPIncreaseNonEmergencySCThresholdAction:testUpdateInitialHashIsOldHash1() (gas: 4843318) -AIPIncreaseNonEmergencySCThresholdAction:testUpdateInitialHashIsOldHash2() (gas: 4843363) +AIPIncreaseNonEmergencySCThresholdAction:testUnfoundConstitutionHash() (gas: 4919281) +AIPIncreaseNonEmergencySCThresholdAction:testUpdateInitialHashIsOldHash1() (gas: 4937307) +AIPIncreaseNonEmergencySCThresholdAction:testUpdateInitialHashIsOldHash2() (gas: 4937515) ArbitrumDAOConstitutionTest:testConstructor() (gas: 259383) ArbitrumDAOConstitutionTest:testMonOwnerCannotSetHash() (gas: 262836) ArbitrumDAOConstitutionTest:testOwnerCanSetHash() (gas: 261148) From 34c26e13eea681499bb09c36205ab06b66ad9c4f Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Wed, 7 Feb 2024 11:59:38 -0500 Subject: [PATCH 15/20] Update src/gov-action-contracts/governance/ConstitutionActionLib.sol Co-authored-by: gzeon --- src/gov-action-contracts/governance/ConstitutionActionLib.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gov-action-contracts/governance/ConstitutionActionLib.sol b/src/gov-action-contracts/governance/ConstitutionActionLib.sol index afd8cdf53..b4a0b79e6 100644 --- a/src/gov-action-contracts/governance/ConstitutionActionLib.sol +++ b/src/gov-action-contracts/governance/ConstitutionActionLib.sol @@ -24,7 +24,7 @@ library ConstitutionActionLib { /// @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 + /// @param _newConstitutionHashes hashes to set at corresponding index if hash in oldConstitutionHashes is found (on the first match) function conditonallyUpdateConstitutionHash( IArbitrumDAOConstitution _constitution, bytes32[] memory _oldConstitutionHashes, From b4578e117e98093d599747e3ca879fe38c86b249 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 26 Feb 2024 15:08:49 -0500 Subject: [PATCH 16/20] remove conditional constitution logic --- ...PIncreaseNonEmergencySCThresholdAction.sol | 20 +- ...CThresholdAndUpdateConstitutionAction.sol} | 39 ++-- ...ncreaseNonEmergencySCThresholdAction.t.sol | 187 +++++++++--------- 3 files changed, 112 insertions(+), 134 deletions(-) rename src/gov-action-contracts/governance/{SetSCThresholdAndConditionallyUpdateConstitutionAction.sol => SetSCThresholdAndUpdateConstitutionAction.sol} (59%) diff --git a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol index 32ae00cbd..1ccc61893 100644 --- a/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol +++ b/src/gov-action-contracts/AIPs/SCImprovementAIP/AIPIncreaseNonEmergencySCThresholdAction.sol @@ -1,27 +1,21 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity 0.8.16; -import "../../governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol"; +import "../../governance/SetSCThresholdAndUpdateConstitutionAction.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 -{ +/// Old constitution hash comes from election propoosal, see https://forum.arbitrum.foundation/t/aip-changes-to-the-constitution-and-the-security-council-election-process/20856/13 +contract AIPIncreaseNonEmergencySCThresholdAction is SetSCThresholdAndUpdateConstitutionAction { constructor() - SetSCThresholdAndConditionallyUpdateConstitutionAction( + SetSCThresholdAndUpdateConstitutionAction( 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. + bytes32(0xe794b7d0466ffd4a33321ea14c307b2de987c3229cf858727052a6f4b8a19cc1), // constitution hash: election change, no threshold increase. https://github.com/ArbitrumFoundation/docs/tree/0837520dccc12e56a25f62de90ff9e3869196d05 + bytes32(0x7cc34e90dde73cfe0b4a041e79b5638e99f0d9547001e42b466c32a18ed6789d) // constitution hash: election change abd threshold increase. https://github.com/ArbitrumFoundation/docs/pull/762/commits/88a6d38e15f1691c2ce7d31fe7c21e8fd52ac126 + ) {} } diff --git a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol similarity index 59% rename from src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol rename to src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol index 7f476e35d..c5dbcc509 100644 --- a/src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol +++ b/src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol @@ -11,15 +11,13 @@ interface _IGnosisSafe { ///@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 { +contract SetSCThresholdAndUpdateConstitutionAction { 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; + bytes32 public immutable oldConstitutionHash; + bytes32 public immutable newConstitutionHash; event ActionPerformed(uint256 newThreshold, bytes32 newConstitutionHash); @@ -28,36 +26,23 @@ contract SetSCThresholdAndConditionallyUpdateConstitutionAction { uint256 _oldThreshold, uint256 _newThreshold, IArbitrumDAOConstitution _constitution, - bytes32 _oldConstitutionHash1, - bytes32 _newConstitutionHash1, - bytes32 _oldConstitutionHash2, - bytes32 _newConstitutionHash2 + bytes32 _oldConstitutionHash, + bytes32 _newConstitutionHash ) { gnosisSafe = _gnosisSafe; oldThreshold = _oldThreshold; newThreshold = _newThreshold; constitution = _constitution; - oldConstitutionHash1 = _oldConstitutionHash1; - newConstitutionHash1 = _newConstitutionHash1; - oldConstitutionHash2 = _oldConstitutionHash2; - newConstitutionHash2 = _newConstitutionHash2; + oldConstitutionHash = _oldConstitutionHash; + newConstitutionHash = _newConstitutionHash; } 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 - }); - + require( + constitution.constitutionHash() == oldConstitutionHash, "WRONG_OLD_CONSTITUTION_HASH" + ); + constitution.setConstitutionHash(newConstitutionHash); + require(constitution.constitutionHash() == newConstitutionHash, "NEW_CONSTUTION_HASH_SET"); // sanity check old threshold require( gnosisSafe.getThreshold() == oldThreshold, "SecSCThresholdAction: WRONG_OLD_THRESHOLD" diff --git a/test/gov-actions/AIPIncreaseNonEmergencySCThresholdAction.t.sol b/test/gov-actions/AIPIncreaseNonEmergencySCThresholdAction.t.sol index 755ef92db..96310fef7 100644 --- a/test/gov-actions/AIPIncreaseNonEmergencySCThresholdAction.t.sol +++ b/test/gov-actions/AIPIncreaseNonEmergencySCThresholdAction.t.sol @@ -2,8 +2,7 @@ pragma solidity 0.8.16; import "forge-std/Test.sol"; -import - "../../src/gov-action-contracts/governance/SetSCThresholdAndConditionallyUpdateConstitutionAction.sol"; +import "../../src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol"; import "../../src/gov-action-contracts/governance/ConstitutionActionLib.sol"; import "../util/ActionTestBase.sol"; import "../util/DeployGnosisWithModule.sol"; @@ -24,102 +23,102 @@ contract AIPIncreaseNonEmergencySCThresholdAction is bytes32 constHash5 = bytes32("0x5"); address safeAddress; + // TODO: outdated tests + // function runUpdate( + // bytes32 _initialConstitutionHash, + // bytes32 _oldConstitutionHash1, + // bytes32 _newConstitutionHash1, + // bytes32 _oldConstitutionHash2, + // bytes32 _newConstitutionHash2 + // ) public { + // safeAddress = deploySafe(owners, oldThreshold, address(arbOneUe)); - 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" + // ); - 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 + // }) + // ); - 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 + // ) + // ); + // } - 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 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 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 - ) - ); - } + // 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 + // ) + // ); + // } } From 1bc50c554192620a4e8b6cb741345ef478d8fc67 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 26 Feb 2024 15:09:59 -0500 Subject: [PATCH 17/20] snapshot --- .gas-snapshot | 3 --- 1 file changed, 3 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 42aeaca81..fbffee028 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,7 +1,4 @@ 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) From f669c295469c5cc950f29460ea7322a656520270 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Fri, 15 Mar 2024 15:16:49 -0400 Subject: [PATCH 18/20] fix typo --- .../governance/SetSCThresholdAndUpdateConstitutionAction.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol index c5dbcc509..2b7f21656 100644 --- a/src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol +++ b/src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol @@ -42,7 +42,7 @@ contract SetSCThresholdAndUpdateConstitutionAction { constitution.constitutionHash() == oldConstitutionHash, "WRONG_OLD_CONSTITUTION_HASH" ); constitution.setConstitutionHash(newConstitutionHash); - require(constitution.constitutionHash() == newConstitutionHash, "NEW_CONSTUTION_HASH_SET"); + require(constitution.constitutionHash() == newConstitutionHash, "NEW_CONSTITUTION_HASH_SET"); // sanity check old threshold require( gnosisSafe.getThreshold() == oldThreshold, "SecSCThresholdAction: WRONG_OLD_THRESHOLD" From d90249166f22be53b4b808007c5a704c87102dac Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Mon, 18 Mar 2024 14:08:12 -0400 Subject: [PATCH 19/20] fix typos --- .../governance/SetSCThresholdAndUpdateConstitutionAction.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol b/src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol index 2b7f21656..52e38b5b2 100644 --- a/src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol +++ b/src/gov-action-contracts/governance/SetSCThresholdAndUpdateConstitutionAction.sol @@ -45,7 +45,7 @@ contract SetSCThresholdAndUpdateConstitutionAction { require(constitution.constitutionHash() == newConstitutionHash, "NEW_CONSTITUTION_HASH_SET"); // sanity check old threshold require( - gnosisSafe.getThreshold() == oldThreshold, "SecSCThresholdAction: WRONG_OLD_THRESHOLD" + gnosisSafe.getThreshold() == oldThreshold, "SetSCThresholdAction: WRONG_OLD_THRESHOLD" ); gnosisSafe.execTransactionFromModule({ @@ -56,7 +56,7 @@ contract SetSCThresholdAndUpdateConstitutionAction { }); // sanity check new threshold was set require( - gnosisSafe.getThreshold() == newThreshold, "SecSCThresholdAction: NEW_THRESHOLD_NOT_SET" + gnosisSafe.getThreshold() == newThreshold, "SetSCThresholdAction: NEW_THRESHOLD_NOT_SET" ); emit ActionPerformed(newThreshold, constitution.constitutionHash()); } From 5de454a6d29c3ca467999a99ff03f6ad420e45d9 Mon Sep 17 00:00:00 2001 From: fred Date: Fri, 12 Apr 2024 10:18:49 -0300 Subject: [PATCH 20/20] add proposal calldata for sc threshold change --- .../data/AIPSCThreshold-data.json | 13 ++++ .../proposals/AIPSCThreshold/description.txt | 67 ++++++++++++++++ .../AIPSCThreshold/generateProposalData.ts | 76 +++++++++++++++++++ 3 files changed, 156 insertions(+) create mode 100644 scripts/proposals/AIPSCThreshold/data/AIPSCThreshold-data.json create mode 100644 scripts/proposals/AIPSCThreshold/description.txt create mode 100644 scripts/proposals/AIPSCThreshold/generateProposalData.ts diff --git a/scripts/proposals/AIPSCThreshold/data/AIPSCThreshold-data.json b/scripts/proposals/AIPSCThreshold/data/AIPSCThreshold-data.json new file mode 100644 index 000000000..6cc8fe51e --- /dev/null +++ b/scripts/proposals/AIPSCThreshold/data/AIPSCThreshold-data.json @@ -0,0 +1,13 @@ +{ + "actionChainID": [ + 42161 + ], + "actionAddress": [ + "0x25afB879bb5364cB3f7e0b607AD280C0F52B0D82" + ], + "description": "\n### **Abstract**\n\nThis AIP seeks to propose changes to the structure of the security council so Arbitrum can maintain the “Stage 1” designation as per L2BEAT and not fall back to “Stage 0” designation.\n\n### **Motivation**\n\nOn December 7, [L2BEAT published an update](https://medium.com/l2beat/stages-update-security-council-requirements-4c79cea8ef52) to the security council requirements for the [Stages Framework](https://medium.com/l2beat/introducing-stages-a-framework-to-evaluate-rollups-maturity-d290bb22befe). The requirements were updated after a lot of research and feedback to make Stages more formal and precise. \n\n### **Rationale**\n\nUpgrading the security council as per the Stage 1 requirements set by L2BEAT, will help ensure Arbitrum remains decentralized, but properly secured. See ‘Specifications’ for more details.\n\n### **Key Terms**\n\n**Stages:** A framework, inspired by [Vitalik’s proposed milestones](https://ethereum-magicians.org/t/proposed-milestones-for-rollups-taking-off-training-wheels/11571), that categorises rollups into three distinct stages based on their reliance on these training wheels. You can learn [more about the Stages framework here](https://medium.com/l2beat/introducing-stages-a-framework-to-evaluate-rollups-maturity-d290bb22befe).\n\n**Security Council:** A group of 12 individuals who are responsible for addressing risks to the Arbitrum ecosystem through the selective application of **e**mergency actions and non-emergency actions. Learn more in [the ArbitrumDAO Docs](https://docs.arbitrum.foundation/concepts/security-council).\n\n**Timelock:** Smart contracts which implement a delay between an upgrade confirmation and execution.\n\n**Exit Window:** The actual time users have to exit the system in case of an unwanted upgrade.\n\n### **Specifications**\n\nArbitrum currently has two multisigs and they both contain the same set of members:\n\na) A 9/12 multisig with instant upgrade power \n\nb) A 7/12 multisig that can upgrade with a 3+7+3 days delay \n\nWhile the higher threshold multisig can be classified as a Security Council, the lower one is below the minimum threshold and it’s considered a simple multisig according to the Stages framework introduced above.\n\nFor normal multisigs, L2BEAT requires at least a 7 days exit window for users. The current exit window for Arbitrum is 2 days (see [this thread](https://x.com/stonecoldpat0/status/1737840485967032739?s=20) for a quick explanation).\n\nMoreover, the higher threshold multisig is supposed to stop malicious upgrades attempted by the lower threshold multisig. However, since the member set is the same, if the lower threshold agrees on something there are not enough members in the higher threshold to stop them, which means that the actual security of the upgradeability mechanism boils down to the 7/12 threshold.\n\nFor the above reason, technically, with the updated requirements for Stages, Arbitrum falls back to the Stage 0 designation. Since we know that it takes time to upgrade Arbitrum, we decided to leave the Stage 1 designation with the promise of addressing the above issues in a timely manner. This proposal is about addressing the issues and moving them to be voted on by the DAO.\n\n**Proposed Solutions**\n\n1) The **first solution** would be to remove the lower threshold (7/12) multisig entirely. This can be done in two ways:\n* The contract is removed which requires an on-chain upgrade, or,\n* The lower threshold multisig increases its threshold from 7/12 to 9/12 which requires no upgrade.\n \nIncreasing the threshold gives us the flexibility to restore a lower threshold in the future should the need arise, and it’s also a very quick and easy fix since it doesn’t require an on-chain upgrade.\n \nOn the other hand, removing the dependency on the lower threshold mutlisig for all the contracts in Arbitrum is a broad and potentially risky change. Therefore we suggest raising the threshold for the time being and revisiting the removal of all the dependencies at a later date if needed.\n\n2) The **second solution** would be to leave the lower threshold multisig as it is, but to increase the exit window to 7 days. In practice, this involves increasing the L2 timelock delay from 3 days to 8 days, since there is a 1 day max delay to force transactions on Arbitrum via L1 using the ‘DelayedInbox’. Increasing the L1 Timelock would not be very beneficial due to delay attacks on the fraud proof systems, since, even with BoLD, the challenge period would end up being up to [16 days](https://x.com/DZack23/status/1737864854059335905?s=20).\n\n3) The ****************************third solution****************************, which is not strictly required by the Stages Framework for the Stage 1 designation, is to both remove the lower threshold multisig entirely and increase the L2 Timelock delay so users have more time to exit in case of unwanted upgrades, increasing the security of the system even more.\n\n### Steps to Implement\n\nFollowing a week of discussion of this RFC, the proposal will go for a vote on Snapshot with the following 4 options (as they are or slightly adjusted), and/or any additional ones, should they arise from the discussion during the RFC phase:\n\n1. Increase the threshold from 7/12 to 9/12.\n2. Increase the L2 timelock delay from 3 days to 8 days.\n3. Increase the threshold and the L2 timelock delay.\n4. Make no changes.\n\nFollowing the temp-check, if any of the aforementioned options apart from No.4 is the most popular, the proposal will move to on-chain vote to execute the proposal.\n\n### **Overall Cost**\n\nThere’s no overhead to the DAO for the implementation of this proposal.\n", + "arbSysSendTxToL1Args": { + "l1Timelock": "0xE6841D92B0C345144506576eC13ECf5103aC7f49", + "calldata": "0x8f2a0bb000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001400000000000000000000000000000000000000000000000000000000000000000f081971a6bbfc9246db73cab798cd1344571aba50e5b99e47c9830f4bd015080000000000000000000000000000000000000000000000000000000000003f4800000000000000000000000000000000000000000000000000000000000000001000000000000000000000000a723c008e76e379c55599d2e4d93879beafda79c000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000001800000000000000000000000004dbd4fc535ac27206064b68ffcf827b0a60bab3f000000000000000000000000cf57572261c7c2bcf21ffd220ea7d1a27d40a82700000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000841cff79cd00000000000000000000000025afb879bb5364cb3f7e0b607ad280c0f52b0d8200000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000004b147f40c0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" + } +} \ No newline at end of file diff --git a/scripts/proposals/AIPSCThreshold/description.txt b/scripts/proposals/AIPSCThreshold/description.txt new file mode 100644 index 000000000..a4c44b7fd --- /dev/null +++ b/scripts/proposals/AIPSCThreshold/description.txt @@ -0,0 +1,67 @@ + +### **Abstract** + +This AIP seeks to propose changes to the structure of the security council so Arbitrum can maintain the “Stage 1” designation as per L2BEAT and not fall back to “Stage 0” designation. + +### **Motivation** + +On December 7, [L2BEAT published an update](https://medium.com/l2beat/stages-update-security-council-requirements-4c79cea8ef52) to the security council requirements for the [Stages Framework](https://medium.com/l2beat/introducing-stages-a-framework-to-evaluate-rollups-maturity-d290bb22befe). The requirements were updated after a lot of research and feedback to make Stages more formal and precise. + +### **Rationale** + +Upgrading the security council as per the Stage 1 requirements set by L2BEAT, will help ensure Arbitrum remains decentralized, but properly secured. See ‘Specifications’ for more details. + +### **Key Terms** + +**Stages:** A framework, inspired by [Vitalik’s proposed milestones](https://ethereum-magicians.org/t/proposed-milestones-for-rollups-taking-off-training-wheels/11571), that categorises rollups into three distinct stages based on their reliance on these training wheels. You can learn [more about the Stages framework here](https://medium.com/l2beat/introducing-stages-a-framework-to-evaluate-rollups-maturity-d290bb22befe). + +**Security Council:** A group of 12 individuals who are responsible for addressing risks to the Arbitrum ecosystem through the selective application of **e**mergency actions and non-emergency actions. Learn more in [the ArbitrumDAO Docs](https://docs.arbitrum.foundation/concepts/security-council). + +**Timelock:** Smart contracts which implement a delay between an upgrade confirmation and execution. + +**Exit Window:** The actual time users have to exit the system in case of an unwanted upgrade. + +### **Specifications** + +Arbitrum currently has two multisigs and they both contain the same set of members: + +a) A 9/12 multisig with instant upgrade power + +b) A 7/12 multisig that can upgrade with a 3+7+3 days delay + +While the higher threshold multisig can be classified as a Security Council, the lower one is below the minimum threshold and it’s considered a simple multisig according to the Stages framework introduced above. + +For normal multisigs, L2BEAT requires at least a 7 days exit window for users. The current exit window for Arbitrum is 2 days (see [this thread](https://x.com/stonecoldpat0/status/1737840485967032739?s=20) for a quick explanation). + +Moreover, the higher threshold multisig is supposed to stop malicious upgrades attempted by the lower threshold multisig. However, since the member set is the same, if the lower threshold agrees on something there are not enough members in the higher threshold to stop them, which means that the actual security of the upgradeability mechanism boils down to the 7/12 threshold. + +For the above reason, technically, with the updated requirements for Stages, Arbitrum falls back to the Stage 0 designation. Since we know that it takes time to upgrade Arbitrum, we decided to leave the Stage 1 designation with the promise of addressing the above issues in a timely manner. This proposal is about addressing the issues and moving them to be voted on by the DAO. + +**Proposed Solutions** + +1) The **first solution** would be to remove the lower threshold (7/12) multisig entirely. This can be done in two ways: +* The contract is removed which requires an on-chain upgrade, or, +* The lower threshold multisig increases its threshold from 7/12 to 9/12 which requires no upgrade. +  +Increasing the threshold gives us the flexibility to restore a lower threshold in the future should the need arise, and it’s also a very quick and easy fix since it doesn’t require an on-chain upgrade. +  +On the other hand, removing the dependency on the lower threshold mutlisig for all the contracts in Arbitrum is a broad and potentially risky change. Therefore we suggest raising the threshold for the time being and revisiting the removal of all the dependencies at a later date if needed. + +2) The **second solution** would be to leave the lower threshold multisig as it is, but to increase the exit window to 7 days. In practice, this involves increasing the L2 timelock delay from 3 days to 8 days, since there is a 1 day max delay to force transactions on Arbitrum via L1 using the ‘DelayedInbox’. Increasing the L1 Timelock would not be very beneficial due to delay attacks on the fraud proof systems, since, even with BoLD, the challenge period would end up being up to [16 days](https://x.com/DZack23/status/1737864854059335905?s=20). + +3) The ****************************third solution****************************, which is not strictly required by the Stages Framework for the Stage 1 designation, is to both remove the lower threshold multisig entirely and increase the L2 Timelock delay so users have more time to exit in case of unwanted upgrades, increasing the security of the system even more. + +### Steps to Implement + +Following a week of discussion of this RFC, the proposal will go for a vote on Snapshot with the following 4 options (as they are or slightly adjusted), and/or any additional ones, should they arise from the discussion during the RFC phase: + +1. Increase the threshold from 7/12 to 9/12. +2. Increase the L2 timelock delay from 3 days to 8 days. +3. Increase the threshold and the L2 timelock delay. +4. Make no changes. + +Following the temp-check, if any of the aforementioned options apart from No.4 is the most popular, the proposal will move to on-chain vote to execute the proposal. + +### **Overall Cost** + +There’s no overhead to the DAO for the implementation of this proposal. diff --git a/scripts/proposals/AIPSCThreshold/generateProposalData.ts b/scripts/proposals/AIPSCThreshold/generateProposalData.ts new file mode 100644 index 000000000..da2b719e9 --- /dev/null +++ b/scripts/proposals/AIPSCThreshold/generateProposalData.ts @@ -0,0 +1,76 @@ +import { RoundTripProposalCreator } from "../../../src-ts/proposalCreator"; +import { JsonRpcProvider } from "@ethersproject/providers"; +import { constants, utils } from "ethers"; +import { CoreGovPropposal } from "../coreGovProposalInterface"; +import dotenv from "dotenv"; +import { importDeployedContracts } from "../../../src-ts/utils"; +import fs from "fs"; +const zero = constants.Zero; +dotenv.config(); + +const mainnetDeployedContracts = importDeployedContracts("./files/mainnet/deployedContracts.json"); + +dotenv.config(); + +const description = fs.readFileSync("./scripts/proposals/AIPSCThreshold/description.txt").toString() + +if(!process.env.ETH_URL) throw new Error("no eth rpc") +if(!process.env.ARB_URL) throw new Error("no arb1 rpc") + +const l1Provider = new JsonRpcProvider(process.env.ETH_URL); +const govChainProvider = new JsonRpcProvider(process.env.ARB_URL); + +const l1GovConfig = { + timelockAddr: mainnetDeployedContracts.l1Timelock, + provider: l1Provider, +}; + +if (!mainnetDeployedContracts.novaUpgradeExecutorProxy) + throw new Error("novaUpgradeExecutorProxy not found"); +const upgradeExecs = [ + { + upgradeExecutorAddr: mainnetDeployedContracts.l2Executor, + provider: govChainProvider, + }, +]; + +const actionAddresses = [ + "0x25afB879bb5364cB3f7e0b607AD280C0F52B0D82", +]; + +const performEncoded = new utils.Interface(["function perform() external"]).encodeFunctionData( + "perform", + [] +); + +const values = actionAddresses.map(() => zero); +const datas = actionAddresses.map(() => performEncoded); + +const main = async () => { + const propCreator = new RoundTripProposalCreator(l1GovConfig, upgradeExecs); + + const res = await propCreator.createRoundTripCallDataForArbSysCall( + actionAddresses, + values, + datas, + description + ); + + const proposal: CoreGovPropposal = { + actionChainID: [42161], + actionAddress: actionAddresses, + description, + arbSysSendTxToL1Args: { + l1Timelock: mainnetDeployedContracts.l1Timelock, + calldata: res.l1TimelockScheduleCallData, + }, + }; + + const path = `${__dirname}/data/AIPSCThreshold-data.json`; + fs.writeFileSync(path, JSON.stringify(proposal, null, 2)); + console.log("Wrote proposal data to", path); +}; + +main().then(() => { + console.log("done"); +});