diff --git a/package.json b/package.json index 54acd0e9..940763f1 100644 --- a/package.json +++ b/package.json @@ -79,6 +79,7 @@ "@arbitrum/nitro-contracts": "1.1.1", "@arbitrum/token-bridge-contracts": "1.0.0-beta.0", "@gnosis.pm/safe-contracts": "1.3.0", + "@offchainlabs/upgrade-executor": "1.1.1", "@openzeppelin/contracts": "4.7.3", "@openzeppelin/contracts-upgradeable": "4.7.3", "@types/yargs": "^17.0.17", diff --git a/remappings.txt b/remappings.txt index aa38660f..7a98cb9d 100644 --- a/remappings.txt +++ b/remappings.txt @@ -1,5 +1,6 @@ forge-std/=lib/forge-std/src/ solady/=lib/solady/src/ +@offchainlabs/upgrade-executor/=node_modules/@offchainlabs/upgrade-executor/ @arbitrum/token-bridge-contracts/=node_modules/@arbitrum/token-bridge-contracts/ @arbitrum/nitro-contracts/=node_modules/@arbitrum/nitro-contracts/ @openzeppelin/contracts-upgradeable/=node_modules/@openzeppelin/contracts-upgradeable/ diff --git a/src/L1GovernanceFactory.sol b/src/L1GovernanceFactory.sol index c6292aad..ebc99b84 100644 --- a/src/L1GovernanceFactory.sol +++ b/src/L1GovernanceFactory.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.16; import "./L1ArbitrumTimelock.sol"; -import "./UpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; diff --git a/src/L2GovernanceFactory.sol b/src/L2GovernanceFactory.sol index 5a4ad6b9..5528eb63 100644 --- a/src/L2GovernanceFactory.sol +++ b/src/L2GovernanceFactory.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.16; import "./L2ArbitrumToken.sol"; import "./L2ArbitrumGovernor.sol"; import "./ArbitrumTimelock.sol"; -import "./UpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; import "./FixedDelegateErc20Wallet.sol"; import "./ArbitrumDAOConstitution.sol"; import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; diff --git a/src/UpgradeExecRouteBuilder.sol b/src/UpgradeExecRouteBuilder.sol index b1f7f6ee..679111f8 100644 --- a/src/UpgradeExecRouteBuilder.sol +++ b/src/UpgradeExecRouteBuilder.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.16; import "@arbitrum/nitro-contracts/src/precompiles/ArbSys.sol"; -import "./UpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; import "./L1ArbitrumTimelock.sol"; import "./security-council-mgmt/Common.sol"; diff --git a/src/UpgradeExecutor.sol b/src/UpgradeExecutor.sol deleted file mode 100644 index adaa6d83..00000000 --- a/src/UpgradeExecutor.sol +++ /dev/null @@ -1,60 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -pragma solidity 0.8.16; - -import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; -import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; -import "@openzeppelin/contracts/utils/Address.sol"; - -/// @title A root contract from which it execute upgrades -/// @notice Does not contain upgrade logic itself, only the means to call upgrade contracts and execute them -/// @dev We use these upgrade contracts as they allow multiple actions to take place in an upgrade -/// and for these actions to interact. However because we are delegatecalling into these upgrade -/// contracts, it's important that these upgrade contract do not touch or modify contract state. -contract UpgradeExecutor is Initializable, AccessControlUpgradeable, ReentrancyGuard { - using Address for address; - - bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE"); - bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE"); - - /// @notice Emitted when an upgrade execution occurs - event UpgradeExecuted(address indexed upgrade, uint256 value, bytes data); - - constructor() { - _disableInitializers(); - } - - /// @notice Initialise the upgrade executor - /// @param admin The admin who can update other roles, and itself - ADMIN_ROLE - /// @param executors Can call the execute function - EXECUTOR_ROLE - function initialize(address admin, address[] memory executors) public initializer { - require(admin != address(0), "UpgradeExecutor: zero admin"); - - __AccessControl_init(); - - _setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE); - _setRoleAdmin(EXECUTOR_ROLE, ADMIN_ROLE); - - _setupRole(ADMIN_ROLE, admin); - for (uint256 i = 0; i < executors.length; ++i) { - _setupRole(EXECUTOR_ROLE, executors[i]); - } - } - - /// @notice Execute an upgrade by delegate calling an upgrade contract - /// @dev Only executor can call this. Since we're using a delegatecall here the Upgrade contract - /// will have access to the state of this contract - including the roles. Only upgrade contracts - /// that do not touch local state should be used. - function execute(address upgrade, bytes memory upgradeCallData) - public - payable - onlyRole(EXECUTOR_ROLE) - nonReentrant - { - // OZ Address library check if the address is a contract and bubble up inner revert reason - address(upgrade).functionDelegateCall( - upgradeCallData, "UpgradeExecutor: inner delegate call failed without reason" - ); - - emit UpgradeExecuted(upgrade, msg.value, upgradeCallData); - } -} diff --git a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/GovernanceChainSCMgmtActivationAction.sol b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/GovernanceChainSCMgmtActivationAction.sol index 956ce437..4c064780 100644 --- a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/GovernanceChainSCMgmtActivationAction.sol +++ b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/GovernanceChainSCMgmtActivationAction.sol @@ -5,9 +5,10 @@ import "../../../security-council-mgmt/interfaces/IGnosisSafe.sol"; import "../../address-registries/L2AddressRegistryInterfaces.sol"; import "./SecurityCouncilMgmtUpgradeLib.sol"; import "../../../interfaces/IArbitrumDAOConstitution.sol"; -import "../../../interfaces/IUpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/IUpgradeExecutor.sol"; import "../../../interfaces/ICoreTimelock.sol"; import "@openzeppelin/contracts/utils/Address.sol"; +import "@openzeppelin/contracts/access/IAccessControl.sol"; contract GovernanceChainSCMgmtActivationAction { IGnosisSafe public immutable newEmergencySecurityCouncil; @@ -116,11 +117,11 @@ contract GovernanceChainSCMgmtActivationAction { // confirm updates bytes32 EXECUTOR_ROLE = upgradeExecutor.EXECUTOR_ROLE(); require( - upgradeExecutor.hasRole(EXECUTOR_ROLE, address(newEmergencySecurityCouncil)), + IAccessControl(address(upgradeExecutor)).hasRole(EXECUTOR_ROLE, address(newEmergencySecurityCouncil)), "NonGovernanceChainSCMgmtActivationAction: new emergency security council not set" ); require( - !upgradeExecutor.hasRole(EXECUTOR_ROLE, address(prevEmergencySecurityCouncil)), + !IAccessControl(address(upgradeExecutor)).hasRole(EXECUTOR_ROLE, address(prevEmergencySecurityCouncil)), "NonGovernanceChainSCMgmtActivationAction: prev emergency security council still set" ); diff --git a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/L1SCMgmtActivationAction.sol b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/L1SCMgmtActivationAction.sol index 602e149c..e1cebc91 100644 --- a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/L1SCMgmtActivationAction.sol +++ b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/L1SCMgmtActivationAction.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.16; import "../../../security-council-mgmt/interfaces/IGnosisSafe.sol"; -import "../../../interfaces/IUpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/IUpgradeExecutor.sol"; import "../../../interfaces/ICoreTimelock.sol"; import "./SecurityCouncilMgmtUpgradeLib.sol"; diff --git a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/NonGovernanceChainSCMgmtActivationAction.sol b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/NonGovernanceChainSCMgmtActivationAction.sol index d3c48f05..9004ebbb 100644 --- a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/NonGovernanceChainSCMgmtActivationAction.sol +++ b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/NonGovernanceChainSCMgmtActivationAction.sol @@ -34,11 +34,11 @@ contract NonGovernanceChainSCMgmtActivationAction { // confirm updates bytes32 EXECUTOR_ROLE = upgradeExecutor.EXECUTOR_ROLE(); require( - upgradeExecutor.hasRole(EXECUTOR_ROLE, address(newEmergencySecurityCouncil)), + IAccessControl(address(upgradeExecutor)).hasRole(EXECUTOR_ROLE, address(newEmergencySecurityCouncil)), "NonGovernanceChainSCMgmtActivationAction: new emergency security council not set" ); require( - !upgradeExecutor.hasRole(EXECUTOR_ROLE, address(prevEmergencySecurityCouncil)), + !IAccessControl(address(upgradeExecutor)).hasRole(EXECUTOR_ROLE, address(prevEmergencySecurityCouncil)), "NonGovernanceChainSCMgmtActivationAction: prev emergency security council still set" ); } diff --git a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/SecurityCouncilMgmtUpgradeLib.sol b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/SecurityCouncilMgmtUpgradeLib.sol index d12c9212..95ab0db9 100644 --- a/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/SecurityCouncilMgmtUpgradeLib.sol +++ b/src/gov-action-contracts/AIPs/SecurityCouncilMgmt/SecurityCouncilMgmtUpgradeLib.sol @@ -2,7 +2,8 @@ pragma solidity 0.8.16; import "../../../security-council-mgmt/interfaces/IGnosisSafe.sol"; -import "../../../interfaces/IUpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/IUpgradeExecutor.sol"; +import "@openzeppelin/contracts/access/IAccessControl.sol"; library SecurityCouncilMgmtUpgradeLib { function replaceEmergencySecurityCouncil( @@ -14,16 +15,16 @@ library SecurityCouncilMgmtUpgradeLib { requireSafesEquivalent(_prevSecurityCouncil, _newSecurityCouncil, _threshold); bytes32 EXECUTOR_ROLE = _upgradeExecutor.EXECUTOR_ROLE(); require( - _upgradeExecutor.hasRole(EXECUTOR_ROLE, address(_prevSecurityCouncil)), + IAccessControl(address(_upgradeExecutor)).hasRole(EXECUTOR_ROLE, address(_prevSecurityCouncil)), "SecurityCouncilMgmtUpgradeLib: prev council not executor" ); require( - !_upgradeExecutor.hasRole(EXECUTOR_ROLE, address(_newSecurityCouncil)), + !IAccessControl(address(_upgradeExecutor)).hasRole(EXECUTOR_ROLE, address(_newSecurityCouncil)), "SecurityCouncilMgmtUpgradeLib: new council already executor" ); - _upgradeExecutor.revokeRole(EXECUTOR_ROLE, address(_prevSecurityCouncil)); - _upgradeExecutor.grantRole(EXECUTOR_ROLE, address(_newSecurityCouncil)); + IAccessControl(address(_upgradeExecutor)).revokeRole(EXECUTOR_ROLE, address(_prevSecurityCouncil)); + IAccessControl(address(_upgradeExecutor)).grantRole(EXECUTOR_ROLE, address(_newSecurityCouncil)); } function requireSafesEquivalent( diff --git a/src/gov-action-contracts/AIPs/upgrade-executor-upgrade/UpgradeExecutorUpgradeAction.sol b/src/gov-action-contracts/AIPs/upgrade-executor-upgrade/UpgradeExecutorUpgradeAction.sol new file mode 100644 index 00000000..aa5f5236 --- /dev/null +++ b/src/gov-action-contracts/AIPs/upgrade-executor-upgrade/UpgradeExecutorUpgradeAction.sol @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.16; + +import {UpgradeExecutor} from "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; // todo: deploy UpgradeExecutor separately +import { + ProxyAdmin, + TransparentUpgradeableProxy +} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; + +contract UpgradeExecutorUpgradeAction { + ProxyAdmin public immutable proxyAdmin; + address public immutable newUpgradeExecutorImplementation; + + constructor(address _proxyAdmin, address _newUpgradeExecutorImplementation) { + proxyAdmin = ProxyAdmin(_proxyAdmin); + newUpgradeExecutorImplementation = _newUpgradeExecutorImplementation; + } + + function perform() external { + TransparentUpgradeableProxy proxy = TransparentUpgradeableProxy(payable(address(this))); + + proxyAdmin.upgrade(proxy, newUpgradeExecutorImplementation); + + require( + proxyAdmin.getProxyImplementation(proxy) == newUpgradeExecutorImplementation, + "UpgradeExecutorUpgradeAction: upgrade failed" + ); + } +} + +// Proxy Admins: +// Arb1: 0xdb216562328215E010F819B5aBe947bad4ca961e +// Nova: 0xf58eA15B20983116c21b05c876cc8e6CDAe5C2b9 +// L1: 0x5613AF0474EB9c528A34701A5b1662E3C8FA0678 + +// Upgrade Executor Impls: +// Arb1: 0x12B1389Fbf261E781bdc3094d28636Abfb03C5b3 +// Nova: 0xebb11Bbd7d72165FaC86bb5AB1B07A602540b286 +// L1: 0xDE505e42D50abd07c8D39Dcf692920d56cBA35Da + +contract ArbOneUpgradeExecutorUpgradeAction is UpgradeExecutorUpgradeAction { + constructor() + UpgradeExecutorUpgradeAction( + 0xdb216562328215E010F819B5aBe947bad4ca961e, + 0x12B1389Fbf261E781bdc3094d28636Abfb03C5b3 + ) + {} +} + +contract NovaUpgradeExecutorUpgradeAction is UpgradeExecutorUpgradeAction { + constructor() + UpgradeExecutorUpgradeAction( + 0xf58eA15B20983116c21b05c876cc8e6CDAe5C2b9, + 0xebb11Bbd7d72165FaC86bb5AB1B07A602540b286 + ) + {} +} + +contract L1UpgradeExecutorUpgradeAction is UpgradeExecutorUpgradeAction { + constructor() + UpgradeExecutorUpgradeAction( + 0x5613AF0474EB9c528A34701A5b1662E3C8FA0678, + 0xDE505e42D50abd07c8D39Dcf692920d56cBA35Da + ) + {} +} diff --git a/src/interfaces/IUpgradeExecutor.sol b/src/interfaces/IUpgradeExecutor.sol deleted file mode 100644 index e9f02416..00000000 --- a/src/interfaces/IUpgradeExecutor.sol +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -pragma solidity 0.8.16; - -import "@openzeppelin/contracts-upgradeable/access/IAccessControlUpgradeable.sol"; - -interface IUpgradeExecutor is IAccessControlUpgradeable { - function execute(address upgrade, bytes memory upgradeCallData) external; - function ADMIN_ROLE() external returns (bytes32); - function EXECUTOR_ROLE() external returns (bytes32); -} diff --git a/src/security-council-mgmt/SecurityCouncilManager.sol b/src/security-council-mgmt/SecurityCouncilManager.sol index fb03bf74..c657401a 100644 --- a/src/security-council-mgmt/SecurityCouncilManager.sol +++ b/src/security-council-mgmt/SecurityCouncilManager.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.16; import "../ArbitrumTimelock.sol"; -import "../UpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; import "../L1ArbitrumTimelock.sol"; import "./SecurityCouncilMgmtUtils.sol"; import "./interfaces/ISecurityCouncilManager.sol"; diff --git a/src/security-council-mgmt/governors/SecurityCouncilMemberElectionGovernor.sol b/src/security-council-mgmt/governors/SecurityCouncilMemberElectionGovernor.sol index 768792d3..88c9abe0 100644 --- a/src/security-council-mgmt/governors/SecurityCouncilMemberElectionGovernor.sol +++ b/src/security-council-mgmt/governors/SecurityCouncilMemberElectionGovernor.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorVotesUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorSettingsUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import "@openzeppelin/contracts/utils/Address.sol"; import "./modules/SecurityCouncilMemberElectionGovernorCountingUpgradeable.sol"; import "../interfaces/ISecurityCouncilMemberElectionGovernor.sol"; import "../interfaces/ISecurityCouncilNomineeElectionGovernor.sol"; diff --git a/src/security-council-mgmt/governors/SecurityCouncilMemberRemovalGovernor.sol b/src/security-council-mgmt/governors/SecurityCouncilMemberRemovalGovernor.sol index 3c14d744..b4a60056 100644 --- a/src/security-council-mgmt/governors/SecurityCouncilMemberRemovalGovernor.sol +++ b/src/security-council-mgmt/governors/SecurityCouncilMemberRemovalGovernor.sol @@ -7,6 +7,7 @@ import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorCountingSimpleUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorSettingsUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import "@openzeppelin/contracts/utils/Address.sol"; import "./../interfaces/ISecurityCouncilManager.sol"; import "../Common.sol"; import "./modules/ArbitrumGovernorVotesQuorumFractionUpgradeable.sol"; diff --git a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol index e1eba1a9..1cc67318 100644 --- a/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol +++ b/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorSettingsUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import "@openzeppelin/contracts/utils/Address.sol"; import "../interfaces/ISecurityCouncilMemberElectionGovernor.sol"; import "../interfaces/ISecurityCouncilNomineeElectionGovernor.sol"; import "./modules/SecurityCouncilNomineeElectionGovernorCountingUpgradeable.sol"; diff --git a/test/L2GovernanceFactory.t.sol b/test/L2GovernanceFactory.t.sol index 13ad9405..6cbf6468 100644 --- a/test/L2GovernanceFactory.t.sol +++ b/test/L2GovernanceFactory.t.sol @@ -3,7 +3,7 @@ import "../src/L2GovernanceFactory.sol"; import "../src/L2ArbitrumGovernor.sol"; -import "../src/UpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; import "../src/ArbitrumTimelock.sol"; import "../src/ArbitrumDAOConstitution.sol"; diff --git a/test/UpgradeExecutor.t.sol b/test/UpgradeExecutor.t.sol deleted file mode 100644 index f27e8047..00000000 --- a/test/UpgradeExecutor.t.sol +++ /dev/null @@ -1,145 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -pragma solidity 0.8.16; - -import "../src/UpgradeExecutor.sol"; -import "./util/TestUtil.sol"; -import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; - -import "forge-std/Test.sol"; - -contract Setter { - uint256 public val = 0; - address public lastSender; - - function setVal(uint256 _val) public { - val = _val; - lastSender = msg.sender; - } -} - -contract SetterUpgrade { - function upgrade(address setter, uint256 val) public { - Setter(setter).setVal(val); - } -} - -contract AccessControlUpgrader { - function grantRole(address target, bytes32 role, address account) public { - AccessControlUpgradeable(target).grantRole(role, account); - } -} - -contract UpgradeExecutorTest is Test { - address executor0 = address(138); - address executor1 = address(139); - address nobody = address(140); - address executor2 = address(141); - - function deployAndInit() internal returns (UpgradeExecutor) { - UpgradeExecutor ue = UpgradeExecutor(TestUtil.deployProxy(address(new UpgradeExecutor()))); - address[] memory executors = new address[](2); - executors[0] = executor0; - executors[1] = executor1; - ue.initialize(address(ue), executors); - return ue; - } - - function testInit() external { - UpgradeExecutor ue = deployAndInit(); - - assertEq(ue.hasRole(ue.EXECUTOR_ROLE(), executor0), true, "Executor 0"); - assertEq(ue.hasRole(ue.EXECUTOR_ROLE(), executor1), true, "Executor 1"); - assertEq(ue.hasRole(ue.ADMIN_ROLE(), address(ue)), true, "Executor 1"); - assertEq(ue.getRoleAdmin(ue.ADMIN_ROLE()), ue.ADMIN_ROLE(), "admin admin"); - assertEq(ue.getRoleAdmin(ue.EXECUTOR_ROLE()), ue.ADMIN_ROLE(), "executor admin"); - } - - function testInitFailsZeroAdmin() external { - UpgradeExecutor ue = UpgradeExecutor(TestUtil.deployProxy(address(new UpgradeExecutor()))); - address[] memory executors = new address[](2); - executors[0] = executor0; - executors[1] = executor1; - - vm.expectRevert("UpgradeExecutor: zero admin"); - ue.initialize(address(0), executors); - } - - function testExecute() external { - UpgradeExecutor ue = deployAndInit(); - Setter setter = new Setter(); - SetterUpgrade se = new SetterUpgrade(); - - uint256 val = 25; - bytes memory data = abi.encodeWithSelector(se.upgrade.selector, address(setter), val); - - assertEq(setter.val(), 0, "Val before"); - assertEq(setter.lastSender(), address(0), "Sender before"); - - vm.prank(executor0); - ue.execute(address(se), data); - - assertEq(setter.val(), val, "Val after"); - assertEq(setter.lastSender(), address(ue), "Sender after"); - } - - function testCantExecuteEOA() external { - UpgradeExecutor ue = deployAndInit(); - bytes memory data; - - vm.prank(executor0); - vm.expectRevert("Address: delegate call to non-contract"); - ue.execute(address(111), data); - } - - function roleError(address account, bytes32 role) internal pure returns (string memory) { - return string( - abi.encodePacked( - "AccessControl: account ", - StringsUpgradeable.toHexString(uint160(account), 20), - " is missing role ", - StringsUpgradeable.toHexString(uint256(role), 32) - ) - ); - } - - function testExecuteFailsForAdmin() external { - UpgradeExecutor ue = deployAndInit(); - Setter setter = new Setter(); - SetterUpgrade se = new SetterUpgrade(); - - uint256 val = 25; - bytes memory data = abi.encodeWithSelector(se.upgrade.selector, address(setter), val); - - vm.expectRevert(bytes(roleError(address(ue), ue.EXECUTOR_ROLE()))); - vm.prank(address(ue)); - ue.execute(address(se), data); - } - - function testExecuteFailsForNobody() external { - UpgradeExecutor ue = deployAndInit(); - Setter setter = new Setter(); - SetterUpgrade se = new SetterUpgrade(); - - uint256 val = 25; - bytes memory data = abi.encodeWithSelector(se.upgrade.selector, address(setter), val); - - vm.expectRevert(bytes(roleError(nobody, ue.EXECUTOR_ROLE()))); - vm.prank(nobody); - ue.execute(address(se), data); - } - - function testAdminCanChangeExecutor() external { - UpgradeExecutor ue = deployAndInit(); - AccessControlUpgrader ae = new AccessControlUpgrader(); - - bytes memory data = abi.encodeWithSelector( - ae.grantRole.selector, address(ue), ue.EXECUTOR_ROLE(), executor2 - ); - - assertEq(ue.hasRole(ue.EXECUTOR_ROLE(), executor2), false, "executor 2 before"); - vm.prank(executor1); - ue.execute(address(ae), data); - - assertEq(ue.hasRole(ue.EXECUTOR_ROLE(), executor2), true, "executor 2 before"); - } -} diff --git a/test/gov-actions/AIPNovaFeeRoutingAction.t.sol b/test/gov-actions/AIPNovaFeeRoutingAction.t.sol index 93b79753..a1b09d66 100644 --- a/test/gov-actions/AIPNovaFeeRoutingAction.t.sol +++ b/test/gov-actions/AIPNovaFeeRoutingAction.t.sol @@ -6,7 +6,7 @@ pragma solidity 0.8.16; import "forge-std/Test.sol"; import "../../src/gov-action-contracts/AIPs/AIPNovaFeeRoutingAction.sol"; -import "../../src/UpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; contract AIPNovaFeeRoutingActionTest is Test { UpgradeExecutor constant upExec = UpgradeExecutor(0x86a02dD71363c440b21F4c0E5B2Ad01Ffe1A7482); diff --git a/test/gov-actions/ProxyAdminUpgrader.t.sol b/test/gov-actions/ProxyAdminUpgrader.t.sol index 39b3f424..605ef39b 100644 --- a/test/gov-actions/ProxyAdminUpgrader.t.sol +++ b/test/gov-actions/ProxyAdminUpgrader.t.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.16; import "forge-std/Test.sol"; import "../util/TestUtil.sol"; -import "../../src/UpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; import "../../src/gov-action-contracts/gov-upgrade-contracts/upgrade-proxy/ProxyUpgradeAndCallAction.sol"; import "../../src/gov-action-contracts/gov-upgrade-contracts/upgrade-proxy/ProxyUpgradeAction.sol"; diff --git a/test/gov-actions/UpgradeExecutorUpgradeAction.t.sol b/test/gov-actions/UpgradeExecutorUpgradeAction.t.sol new file mode 100644 index 00000000..fba2ff14 --- /dev/null +++ b/test/gov-actions/UpgradeExecutorUpgradeAction.t.sol @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.16; + +import "forge-std/Test.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; + +import "src/gov-action-contracts/AIPs/upgrade-executor-upgrade/UpgradeExecutorUpgradeAction.sol"; + +contract UpgradeExecutorUpgradeActionTest is Test { + function testArbOne() external { + vm.createSelectFork(vm.envString("ARB_URL"), 265_159_958); + _testUpgrade( + new ArbOneUpgradeExecutorUpgradeAction(), + 0xCF57572261c7c2BCF21ffD220ea7d1a27D40A827, + 0xf7951D92B0C345144506576eC13Ecf5103aC905a + ); + } + + function testNova() external { + vm.createSelectFork(vm.envString("NOVA_URL"), 78_263_024); + _testUpgrade( + new NovaUpgradeExecutorUpgradeAction(), + 0x86a02dD71363c440b21F4c0E5B2Ad01Ffe1A7482, + 0xf7951D92B0C345144506576eC13Ecf5103aC905a + ); + } + + function testL1() external { + vm.createSelectFork(vm.envString("ETH_URL"), 20_993_735); + _testUpgrade( + new L1UpgradeExecutorUpgradeAction(), + 0x3ffFbAdAF827559da092217e474760E2b2c3CeDd, + 0xE6841D92B0C345144506576eC13ECf5103aC7f49 + ); + } + + function _testUpgrade(UpgradeExecutorUpgradeAction action, address ue, address executor) + internal + { + vm.prank(executor); + UpgradeExecutor(ue).execute(address(action), abi.encodeWithSignature("perform()")); + + assertTrue( + ProxyAdmin(action.proxyAdmin()).getProxyImplementation( + TransparentUpgradeableProxy(payable(address(ue))) + ) == action.newUpgradeExecutorImplementation() + ); + } +} diff --git a/test/security-council-mgmt/E2E.t.sol b/test/security-council-mgmt/E2E.t.sol index d5380943..4933cecc 100644 --- a/test/security-council-mgmt/E2E.t.sol +++ b/test/security-council-mgmt/E2E.t.sol @@ -3,7 +3,6 @@ pragma solidity 0.8.16; import "forge-std/Test.sol"; -import "../../src/UpgradeExecutor.sol"; import "../../src/ArbitrumTimelock.sol"; import "../../src/L2ArbitrumGovernor.sol"; import "../../src/FixedDelegateErc20Wallet.sol"; diff --git a/test/security-council-mgmt/SecurityCouncilMemberSyncAction.t.sol b/test/security-council-mgmt/SecurityCouncilMemberSyncAction.t.sol index 1f941474..710ceffb 100644 --- a/test/security-council-mgmt/SecurityCouncilMemberSyncAction.t.sol +++ b/test/security-council-mgmt/SecurityCouncilMemberSyncAction.t.sol @@ -5,7 +5,7 @@ pragma solidity 0.8.16; import "@gnosis.pm/safe-contracts/contracts/GnosisSafeL2.sol"; import "../util/TestUtil.sol"; import "../util/DeployGnosisWithModule.sol"; -import "../../src/UpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; import "../../src/security-council-mgmt/SecurityCouncilMemberSyncAction.sol"; import "../../src/security-council-mgmt/interfaces/IGnosisSafe.sol"; diff --git a/test/security-council-mgmt/SecurityCouncilUpgradeAction.t.sol b/test/security-council-mgmt/SecurityCouncilUpgradeAction.t.sol index aa67cd96..cdd39770 100644 --- a/test/security-council-mgmt/SecurityCouncilUpgradeAction.t.sol +++ b/test/security-council-mgmt/SecurityCouncilUpgradeAction.t.sol @@ -5,7 +5,7 @@ pragma solidity 0.8.16; import "@gnosis.pm/safe-contracts/contracts/GnosisSafeL2.sol"; import "../util/TestUtil.sol"; import "../util/DeployGnosisWithModule.sol"; -import "../../src/UpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; import "../../src/security-council-mgmt/SecurityCouncilMemberSyncAction.sol"; import "../../src/security-council-mgmt/interfaces/IGnosisSafe.sol"; diff --git a/test/storage/UpgradeExecutor b/test/storage/UpgradeExecutor index 17b41d91..d8c53559 100644 --- a/test/storage/UpgradeExecutor +++ b/test/storage/UpgradeExecutor @@ -1,19 +1,19 @@ -╭---------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------╮ -| Name | Type | Slot | Offset | Bytes | Contract | -+================================================================================================================================================+ -| _initialized | uint8 | 0 | 0 | 1 | src/UpgradeExecutor.sol:UpgradeExecutor | -|---------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------| -| _initializing | bool | 0 | 1 | 1 | src/UpgradeExecutor.sol:UpgradeExecutor | -|---------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------| -| __gap | uint256[50] | 1 | 0 | 1600 | src/UpgradeExecutor.sol:UpgradeExecutor | -|---------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------| -| __gap | uint256[50] | 51 | 0 | 1600 | src/UpgradeExecutor.sol:UpgradeExecutor | -|---------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------| -| _roles | mapping(bytes32 => struct AccessControlUpgradeable.RoleData) | 101 | 0 | 32 | src/UpgradeExecutor.sol:UpgradeExecutor | -|---------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------| -| __gap | uint256[49] | 102 | 0 | 1568 | src/UpgradeExecutor.sol:UpgradeExecutor | -|---------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------| -| _status | uint256 | 151 | 0 | 32 | src/UpgradeExecutor.sol:UpgradeExecutor | -╰---------------+--------------------------------------------------------------+------+--------+-------+-----------------------------------------╯ +╭---------------+--------------------------------------------------------------+------+--------+-------+-------------------------------------------------------------------------------------╮ +| Name | Type | Slot | Offset | Bytes | Contract | ++============================================================================================================================================================================================+ +| _initialized | uint8 | 0 | 0 | 1 | node_modules/@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol:UpgradeExecutor | +|---------------+--------------------------------------------------------------+------+--------+-------+-------------------------------------------------------------------------------------| +| _initializing | bool | 0 | 1 | 1 | node_modules/@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol:UpgradeExecutor | +|---------------+--------------------------------------------------------------+------+--------+-------+-------------------------------------------------------------------------------------| +| __gap | uint256[50] | 1 | 0 | 1600 | node_modules/@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol:UpgradeExecutor | +|---------------+--------------------------------------------------------------+------+--------+-------+-------------------------------------------------------------------------------------| +| __gap | uint256[50] | 51 | 0 | 1600 | node_modules/@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol:UpgradeExecutor | +|---------------+--------------------------------------------------------------+------+--------+-------+-------------------------------------------------------------------------------------| +| _roles | mapping(bytes32 => struct AccessControlUpgradeable.RoleData) | 101 | 0 | 32 | node_modules/@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol:UpgradeExecutor | +|---------------+--------------------------------------------------------------+------+--------+-------+-------------------------------------------------------------------------------------| +| __gap | uint256[49] | 102 | 0 | 1568 | node_modules/@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol:UpgradeExecutor | +|---------------+--------------------------------------------------------------+------+--------+-------+-------------------------------------------------------------------------------------| +| _status | uint256 | 151 | 0 | 32 | node_modules/@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol:UpgradeExecutor | +╰---------------+--------------------------------------------------------------+------+--------+-------+-------------------------------------------------------------------------------------╯ diff --git a/test/util/ActionTestBase.sol b/test/util/ActionTestBase.sol index d82c7aa8..5be3c183 100644 --- a/test/util/ActionTestBase.sol +++ b/test/util/ActionTestBase.sol @@ -15,7 +15,7 @@ import "../../src/ArbitrumTimelock.sol"; import "../../src/L1ArbitrumTimelock.sol"; import "../../src/FixedDelegateErc20Wallet.sol"; import "../util/TestUtil.sol"; -import "../../src/UpgradeExecutor.sol"; +import "@offchainlabs/upgrade-executor/src/UpgradeExecutor.sol"; import "../../src/ArbitrumDAOConstitution.sol"; import "../../src/gov-action-contracts/address-registries/L1AddressRegistry.sol" as _ar; import "../../src/gov-action-contracts/address-registries/L2AddressRegistry.sol" as _ar1; diff --git a/yarn.lock b/yarn.lock index d4ee3322..0a5463e6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -939,6 +939,14 @@ "@openzeppelin/contracts" "4.7.3" "@openzeppelin/contracts-upgradeable" "4.7.3" +"@offchainlabs/upgrade-executor@1.1.1": + version "1.1.1" + resolved "https://registry.yarnpkg.com/@offchainlabs/upgrade-executor/-/upgrade-executor-1.1.1.tgz#ae3dfe4a183d88c0c3fd3b39ab6dd6508b371b53" + integrity sha512-/hnUblMbzXS4asPdTSWjFUn/N5+E71dVfDW1314j/r/847OIy1VHZDFhw+fOlUGL5/qrZu2UV34oZAwwrRV4tg== + dependencies: + "@openzeppelin/contracts" "4.7.3" + "@openzeppelin/contracts-upgradeable" "4.7.3" + "@openzeppelin/contracts-upgradeable@3.4.2": version "3.4.2" resolved "https://registry.npmjs.org/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-3.4.2.tgz"