From bf3278f5c1372b140a972f25e9733a42231546e0 Mon Sep 17 00:00:00 2001 From: Disco <131301107+0xDiscotech@users.noreply.github.com> Date: Wed, 20 Nov 2024 18:24:41 -0300 Subject: [PATCH 01/13] feat: add shared lockbox (#126) * feat: create shared lockbox contract with its interface and unit tests * chore: polish tests and interfaces * chore: run pre-pr * chore: improve natspec * chore: run pre-pr * chore: update compiler version --- .../snapshots/abi/SharedLockbox.json | 147 ++++++++++++++++++ .../snapshots/semver-lock.json | 4 + .../storageLayout/SharedLockbox.json | 9 ++ .../src/L1/SharedLockbox.sol | 66 ++++++++ .../src/L1/interfaces/ISharedLockbox.sol | 28 ++++ .../test/L1/SharedLockbox.t.sol | 143 +++++++++++++++++ 6 files changed, 397 insertions(+) create mode 100644 packages/contracts-bedrock/snapshots/abi/SharedLockbox.json create mode 100644 packages/contracts-bedrock/snapshots/storageLayout/SharedLockbox.json create mode 100644 packages/contracts-bedrock/src/L1/SharedLockbox.sol create mode 100644 packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol create mode 100644 packages/contracts-bedrock/test/L1/SharedLockbox.t.sol diff --git a/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json b/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json new file mode 100644 index 000000000000..5b707cd7ceff --- /dev/null +++ b/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json @@ -0,0 +1,147 @@ +[ + { + "inputs": [ + { + "internalType": "address", + "name": "_superchainConfig", + "type": "address" + } + ], + "stateMutability": "nonpayable", + "type": "constructor" + }, + { + "inputs": [], + "name": "SUPERCHAIN_CONFIG", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "_portal", + "type": "address" + } + ], + "name": "authorizePortal", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "authorizedPortals", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "lockETH", + "outputs": [], + "stateMutability": "payable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "uint256", + "name": "_value", + "type": "uint256" + } + ], + "name": "unlockETH", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [], + "name": "version", + "outputs": [ + { + "internalType": "string", + "name": "", + "type": "string" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "portal", + "type": "address" + } + ], + "name": "AuthorizedPortal", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "portal", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "ETHLocked", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "portal", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "ETHUnlocked", + "type": "event" + }, + { + "inputs": [], + "name": "Unauthorized", + "type": "error" + } +] \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index fb7b6d824392..464c85d1a990 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -43,6 +43,10 @@ "initCodeHash": "0xefd4806e8737716d5d2022ca2e9e9fba0a0cb5714b026166b58e472222c7d15f", "sourceCodeHash": "0x15205131bf420aa6d03c558bb75dd49cd7439caed7ccdcbfd89c4170a48c94f5" }, + "src/L1/SharedLockbox.sol": { + "initCodeHash": "0x284eaf282bfbe4403ec5c692371ad2876ec872851b384e5779e6eae577f1dfbb", + "sourceCodeHash": "0x85f55d85d3fed2e5b7ddcd48c5ab8c166abc26e33f38c567baa5f7e31109366a" + }, "src/L1/SuperchainConfig.sol": { "initCodeHash": "0xfca12d9016c746e5c275b186e0ca40cfd65cf45a5665aab7589a669fea3abb47", "sourceCodeHash": "0x39489a85bc3a5c8560f82d41b31bf7fe22f5b648f4ed538f61695a73092ea9eb" diff --git a/packages/contracts-bedrock/snapshots/storageLayout/SharedLockbox.json b/packages/contracts-bedrock/snapshots/storageLayout/SharedLockbox.json new file mode 100644 index 000000000000..7c441f14dcc0 --- /dev/null +++ b/packages/contracts-bedrock/snapshots/storageLayout/SharedLockbox.json @@ -0,0 +1,9 @@ +[ + { + "bytes": "32", + "label": "authorizedPortals", + "offset": 0, + "slot": "0", + "type": "mapping(address => bool)" + } +] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L1/SharedLockbox.sol b/packages/contracts-bedrock/src/L1/SharedLockbox.sol new file mode 100644 index 000000000000..eb53b8fcffb4 --- /dev/null +++ b/packages/contracts-bedrock/src/L1/SharedLockbox.sol @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import { ISemver } from "src/universal/interfaces/ISemver.sol"; +import { IOptimismPortal } from "src/L1/interfaces/IOptimismPortal.sol"; +import { Unauthorized } from "src/libraries/errors/CommonErrors.sol"; + +contract SharedLockbox is ISemver { + /// @notice Emitted when ETH is locked in the lockbox by an authorized portal. + /// @param portal The address of the portal that locked the ETH. + /// @param amount The amount of ETH locked. + event ETHLocked(address indexed portal, uint256 amount); + + /// @notice Emitted when ETH is unlocked from the lockbox by an authorized portal. + /// @param portal The address of the portal that unlocked the ETH. + /// @param amount The amount of ETH unlocked. + event ETHUnlocked(address indexed portal, uint256 amount); + + /// @notice Emitted when a portal is set as authorized to interact with the lockbox. + /// @param portal The address of the authorized portal. + event AuthorizedPortal(address indexed portal); + + /// @notice The address of the SuperchainConfig contract. + address public immutable SUPERCHAIN_CONFIG; + + /// @notice OptimismPortals that are part of the dependency cluster authorized to interact with the SharedLockbox. + mapping(address => bool) public authorizedPortals; + + /// @notice Semantic version. + /// @custom:semver 1.0.0-beta.1 + function version() public view virtual returns (string memory) { + return "1.0.0-beta.1"; + } + + /// @notice Constructs the SharedLockbox contract. + /// @param _superchainConfig The address of the SuperchainConfig contract. + constructor(address _superchainConfig) { + SUPERCHAIN_CONFIG = _superchainConfig; + } + + /// @notice Locks ETH in the lockbox. + /// Called by an authorized portal when migrating its ETH liquidity or when depositing with some ETH value. + function lockETH() external payable { + if (!authorizedPortals[msg.sender]) revert Unauthorized(); + + emit ETHLocked(msg.sender, msg.value); + } + + /// @notice Unlocks ETH from the lockbox. + /// Called by an authorized portal when finalizing a withdrawal that requires ETH. + function unlockETH(uint256 _value) external { + if (!authorizedPortals[msg.sender]) revert Unauthorized(); + + // Using `donateETH` to avoid triggering a deposit + IOptimismPortal(payable(msg.sender)).donateETH{ value: _value }(); + emit ETHUnlocked(msg.sender, _value); + } + + /// @notice Authorizes a portal to interact with the lockbox. + function authorizePortal(address _portal) external { + if (msg.sender != SUPERCHAIN_CONFIG) revert Unauthorized(); + + authorizedPortals[_portal] = true; + emit AuthorizedPortal(_portal); + } +} diff --git a/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol b/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol new file mode 100644 index 000000000000..9aac053e2d42 --- /dev/null +++ b/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import { ISemver } from "src/universal/interfaces/ISemver.sol"; + +/// @title ISharedLockbox +/// @notice Interface for the SharedLockbox contract +interface ISharedLockbox is ISemver { + error Unauthorized(); + + event ETHLocked(address indexed portal, uint256 amount); + + event ETHUnlocked(address indexed portal, uint256 amount); + + event AuthorizedPortal(address indexed portal); + + function SUPERCHAIN_CONFIG() external view returns (address); + + function authorizedPortals(address) external view returns (bool); + + function __constructor__(address _superchainConfig) external; + + function unlockETH(uint256 _value) external; + + function lockETH() external payable; + + function authorizePortal(address _portal) external; +} diff --git a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol new file mode 100644 index 000000000000..c3a9e9466352 --- /dev/null +++ b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +// Testing utilities +import { Test } from "forge-std/Test.sol"; +import { Unauthorized } from "src/libraries/errors/CommonErrors.sol"; + +// Targent contract +import { SharedLockbox } from "src/L1/SharedLockbox.sol"; + +// Interfaces +import { IOptimismPortal } from "src/L1/interfaces/IOptimismPortal.sol"; + +contract SharedLockboxTest is Test { + event ETHLocked(address indexed portal, uint256 amount); + + event ETHUnlocked(address indexed portal, uint256 amount); + + event AuthorizedPortal(address indexed portal); + + address internal immutable SUPERCHAIN_CONFIG = makeAddr("SuperchainConfig"); + IOptimismPortal internal immutable PORTAL = IOptimismPortal(payable(makeAddr("OptimismPortal"))); + SharedLockbox public sharedLockbox; + + // TODO: Update setup to use CommonTest and simulate a real deployment environment + function setUp() public { + // Deploy the SharedLockbox contract + sharedLockbox = new SharedLockbox(SUPERCHAIN_CONFIG); + + // Etch the OptimismPortal contract code into the declared address + bytes memory code = vm.getDeployedCode("L1/OptimismPortal.sol:OptimismPortal"); + vm.etch(address(PORTAL), code); + } + + /// @notice Tests it reverts when the caller is not an authorized portal. + function test_lockETH_unauthorizedPortal_reverts(address _caller) public { + vm.assume(!sharedLockbox.authorizedPortals(_caller)); + + // Expect the revert with `Unauthorized` selector + vm.expectRevert(Unauthorized.selector); + + // Call the `lockETH` function with an unauthorized caller + vm.prank(_caller); + sharedLockbox.lockETH(); + } + + /// @notice Tests the ETH is correctly locked when the caller is an authorized portal. + function test_lockETH_succeeds(address _portal, uint256 _amount) public { + // Set the caller as an authorized portal + vm.prank(SUPERCHAIN_CONFIG); + sharedLockbox.authorizePortal(_portal); + + // Deal the ETH amount to the portal + vm.deal(_portal, _amount); + + // Get the balance of the portal and lockbox before the lock to compare later on the assertions + uint256 _portalBalanceBefore = address(_portal).balance; + uint256 _lockboxBalanceBefore = address(sharedLockbox).balance; + + // Look for the emit of the `ETHLocked` event + vm.expectEmit(address(sharedLockbox)); + emit ETHLocked(_portal, _amount); + + // Call the `lockETH` function with the portal + vm.prank(_portal); + sharedLockbox.lockETH{ value: _amount }(); + + // Assert the portal's balance decreased and the lockbox's balance increased by the amount locked + assertEq(address(_portal).balance, _portalBalanceBefore - _amount); + assertEq(address(sharedLockbox).balance, _lockboxBalanceBefore + _amount); + } + + /// @notice Tests it reverts when the caller is not an authorized portal. + function test_unlockETH_unauthorizedPortal_reverts(address _caller, uint256 _value) public { + vm.assume(!sharedLockbox.authorizedPortals(_caller)); + + // Expect the revert with `Unauthorized` selector + vm.expectRevert(Unauthorized.selector); + + // Call the `unlockETH` function with an unauthorized caller + vm.prank(_caller); + sharedLockbox.unlockETH(_value); + } + + /// @notice Tests the ETH is correctly unlocked when the caller is an authorized portal. + function test_unlockETH_succeeds(uint256 _value) public { + // Set the caller as an authorized portal + vm.prank(SUPERCHAIN_CONFIG); + sharedLockbox.authorizePortal(address(PORTAL)); + + // Deal the ETH amount to the lockbox + vm.deal(address(sharedLockbox), _value); + + // Get the balance of the portal and lockbox before the unlock to compare later on the assertions + uint256 _portalBalanceBefore = address(PORTAL).balance; + uint256 _lockboxBalanceBefore = address(sharedLockbox).balance; + + // Expect `donateETH` function to be called on Portal + vm.expectCall(address(PORTAL), abi.encodeWithSelector(IOptimismPortal.donateETH.selector)); + + // Look for the emit of the `ETHUnlocked` event + vm.expectEmit(address(sharedLockbox)); + emit ETHUnlocked(address(PORTAL), _value); + + // Call the `unlockETH` function with the portal + vm.prank(address(PORTAL)); + sharedLockbox.unlockETH(_value); + + // Assert the portal's balance increased and the lockbox's balance decreased by the amount unlocked + assertEq(address(PORTAL).balance, _portalBalanceBefore + _value); + assertEq(address(sharedLockbox).balance, _lockboxBalanceBefore - _value); + } + + /// @notice Tests it reverts when the caller is not the SuperchainConfig. + function test_authorizePortal_notSuperchainConfig_reverts(address _caller) public { + vm.assume(_caller != SUPERCHAIN_CONFIG); + + // Expect the revert with `Unauthorized` selector + vm.expectRevert(Unauthorized.selector); + + // Call the `authorizePortal` function with a non-SuperchainConfig caller + vm.prank(_caller); + sharedLockbox.authorizePortal(_caller); + } + + /// @notice Tests the portal is correctly authorized when the caller is the SuperchainConfig. + function test_authorizePortal_succeeds(address _portal) public { + // Check the portal's authorized status before the authorization to compare later on the assertions. + // Adding this check to make it more future proof in case something changes on the setup. + vm.assume(sharedLockbox.authorizedPortals(_portal) == false); + + // Look for the emit of the `AuthorizedPortal` event + vm.expectEmit(address(sharedLockbox)); + emit AuthorizedPortal(_portal); + + // Call the `authorizePortal` function with the SuperchainConfig + vm.prank(SUPERCHAIN_CONFIG); + sharedLockbox.authorizePortal(_portal); + + // Assert the portal's authorized status was updated correctly + assertEq(sharedLockbox.authorizedPortals(_portal), true); + } +} From 27a4515f37edb6391c23cf4fa0e71a74103188de Mon Sep 17 00:00:00 2001 From: AgusDuha <81362284+agusduha@users.noreply.github.com> Date: Tue, 26 Nov 2024 18:00:51 -0300 Subject: [PATCH 02/13] feat: integrate portal to lockbox (#139) * feat: integrate portal to lockbox * fix: pr fixes * test: refactor assert --- .../scripts/deploy/ChainAssertions.sol | 16 +++++ .../scripts/deploy/Deploy.s.sol | 26 +++++-- .../deploy/DeployImplementations.s.sol | 15 +++- .../scripts/deploy/DeploySuperchain.s.sol | 70 ++++++++++++++++++- .../scripts/libraries/Types.sol | 1 + .../contracts-bedrock/snapshots/.gas-snapshot | 14 ++-- .../snapshots/abi/OptimismPortal2.json | 18 +++++ .../snapshots/abi/OptimismPortalInterop.json | 18 +++++ .../snapshots/semver-lock.json | 8 +-- .../src/L1/OptimismPortal2.sol | 51 ++++++++++---- .../src/L1/OptimismPortalInterop.sol | 9 +-- .../src/L1/interfaces/IOptimismPortal2.sol | 8 ++- .../L1/interfaces/IOptimismPortalInterop.sol | 8 ++- .../test/L1/L1StandardBridge.t.sol | 32 ++++++--- .../test/L1/OptimismPortal2.t.sol | 60 +++++++++++++--- .../test/L1/SharedLockbox.t.sol | 38 ++++------ .../test/invariants/OptimismPortal2.t.sol | 4 +- .../test/opcm/DeployImplementations.t.sol | 10 +++ .../test/opcm/DeployOPChain.t.sol | 6 +- .../test/opcm/DeploySuperchain.t.sol | 26 +++++++ .../test/setup/CommonTest.sol | 6 ++ .../contracts-bedrock/test/setup/Setup.sol | 5 ++ .../test/universal/Specs.t.sol | 14 +++- 23 files changed, 379 insertions(+), 84 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol b/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol index 25d67be3828c..1cb2599c9bce 100644 --- a/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol +++ b/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol @@ -19,6 +19,7 @@ import { IResourceMetering } from "src/L1/interfaces/IResourceMetering.sol"; import { ISystemConfig } from "src/L1/interfaces/ISystemConfig.sol"; import { IL2OutputOracle } from "src/L1/interfaces/IL2OutputOracle.sol"; import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; +import { ISharedLockbox } from "src/L1/interfaces/ISharedLockbox.sol"; import { IL1CrossDomainMessenger } from "src/L1/interfaces/IL1CrossDomainMessenger.sol"; import { IOptimismPortal } from "src/L1/interfaces/IOptimismPortal.sol"; import { IOptimismPortal2 } from "src/L1/interfaces/IOptimismPortal2.sol"; @@ -577,6 +578,21 @@ library ChainAssertions { // TODO: Add assertions for blueprints and setters? } + /// @notice Asserts that the SharedLockbox is setup correctly + function checkSharedLockbox(Types.ContractSet memory _contracts, bool _isProxy) internal view { + ISharedLockbox sharedLockbox = ISharedLockbox(_contracts.SharedLockbox); + ISuperchainConfig superchainConfig = ISuperchainConfig(_contracts.SuperchainConfig); + + console.log( + "Running chain assertions on the SharedLockbox %s at %s", + _isProxy ? "proxy" : "implementation", + address(sharedLockbox) + ); + + require(address(sharedLockbox) != address(0), "CHECK-SLB-10"); + require(sharedLockbox.SUPERCHAIN_CONFIG() == address(superchainConfig), "CHECK-SLB-20"); + } + /// @dev Asserts that for a given contract the value of a storage slot at an offset is 1 or 0xff. /// A call to `initialize` will set it to 1 and a call to _disableInitializers will set it to 0xff. function assertInitializedSlotIsSet(address _contractAddress, uint256 _slot, uint256 _offset) internal view { diff --git a/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol b/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol index 6dc6a8bb90bf..15951c2d0980 100644 --- a/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol @@ -162,7 +162,8 @@ contract Deploy is Deployer { L1ERC721Bridge: getAddress("L1ERC721BridgeProxy"), ProtocolVersions: getAddress("ProtocolVersionsProxy"), SuperchainConfig: getAddress("SuperchainConfigProxy"), - OPContractsManager: getAddress("OPContractsManager") + OPContractsManager: getAddress("OPContractsManager"), + SharedLockbox: getAddress("SharedLockboxProxy") }); } @@ -183,7 +184,8 @@ contract Deploy is Deployer { L1ERC721Bridge: getAddress("L1ERC721Bridge"), ProtocolVersions: getAddress("ProtocolVersions"), SuperchainConfig: getAddress("SuperchainConfig"), - OPContractsManager: getAddress("OPContractsManager") + OPContractsManager: getAddress("OPContractsManager"), + SharedLockbox: getAddress("SharedLockbox") }); } @@ -222,16 +224,19 @@ contract Deploy is Deployer { /// @notice Deploy a new OP Chain using an existing SuperchainConfig and ProtocolVersions /// @param _superchainConfigProxy Address of the existing SuperchainConfig proxy /// @param _protocolVersionsProxy Address of the existing ProtocolVersions proxy + /// @param _sharedLockboxProxy Address of the existing SharedLockbox proxy /// @param _includeDump Whether to include a state dump after deployment function runWithSuperchain( address payable _superchainConfigProxy, address payable _protocolVersionsProxy, + address payable _sharedLockboxProxy, bool _includeDump ) public { require(_superchainConfigProxy != address(0), "Deploy: must specify address for superchain config proxy"); require(_protocolVersionsProxy != address(0), "Deploy: must specify address for protocol versions proxy"); + require(_sharedLockboxProxy != address(0), "Deploy: must specify address for shared lockbox proxy"); vm.chainId(cfg.l1ChainID()); @@ -245,6 +250,10 @@ contract Deploy is Deployer { save("ProtocolVersions", pvProxy.implementation()); save("ProtocolVersionsProxy", _protocolVersionsProxy); + IProxy slProxy = IProxy(_sharedLockboxProxy); + save("SharedLockbox", slProxy.implementation()); + save("SharedLockboxProxy", _sharedLockboxProxy); + _run(false); if (_includeDump) { @@ -324,9 +333,10 @@ contract Deploy is Deployer { //////////////////////////////////////////////////////////////// /// @notice Deploy a full system with a new SuperchainConfig - /// The Superchain system has 2 singleton contracts which lie outside of an OP Chain: + /// The Superchain system has 3 singleton contracts which lie outside of an OP Chain: /// 1. The SuperchainConfig contract /// 2. The ProtocolVersions contract + /// 3. The SharedLockbox contract function deploySuperchain() public { console.log("Setting up Superchain"); DeploySuperchain ds = new DeploySuperchain(); @@ -348,11 +358,18 @@ contract Deploy is Deployer { save("SuperchainConfig", address(dso.superchainConfigImpl())); save("ProtocolVersionsProxy", address(dso.protocolVersionsProxy())); save("ProtocolVersions", address(dso.protocolVersionsImpl())); + save("SharedLockboxProxy", address(dso.sharedLockboxProxy())); + save("SharedLockbox", address(dso.sharedLockboxImpl())); - // First run assertions for the ProtocolVersions and SuperchainConfig proxy contracts. + // First run assertions for the ProtocolVersions, SuperchainConfig and SharedLockbox proxy contracts. Types.ContractSet memory contracts = _proxies(); ChainAssertions.checkProtocolVersions({ _contracts: contracts, _cfg: cfg, _isProxy: true }); ChainAssertions.checkSuperchainConfig({ _contracts: contracts, _cfg: cfg, _isProxy: true, _isPaused: false }); + ChainAssertions.checkSharedLockbox({ _contracts: contracts, _isProxy: true }); + + // Then replace the SharedLockbox proxy with the implementation address and run assertions on it. + contracts.SharedLockbox = mustGetAddress("SharedLockbox"); + ChainAssertions.checkSharedLockbox({ _contracts: contracts, _isProxy: false }); // Then replace the ProtocolVersions proxy with the implementation address and run assertions on it. contracts.ProtocolVersions = mustGetAddress("ProtocolVersions"); @@ -384,6 +401,7 @@ contract Deploy is Deployer { ); dii.set(dii.superchainConfigProxy.selector, mustGetAddress("SuperchainConfigProxy")); dii.set(dii.protocolVersionsProxy.selector, mustGetAddress("ProtocolVersionsProxy")); + dii.set(dii.sharedLockboxProxy.selector, mustGetAddress("SharedLockboxProxy")); dii.set(dii.salt.selector, _implSalt()); if (_isInterop) { diff --git a/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol index 71ba435df4c3..bc21753e5371 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol @@ -8,6 +8,7 @@ import { LibString } from "@solady/utils/LibString.sol"; import { IResourceMetering } from "src/L1/interfaces/IResourceMetering.sol"; import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; import { IProtocolVersions } from "src/L1/interfaces/IProtocolVersions.sol"; +import { ISharedLockbox } from "src/L1/interfaces/ISharedLockbox.sol"; import { Constants } from "src/libraries/Constants.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; @@ -53,6 +54,7 @@ contract DeployImplementationsInput is BaseDeployIO { // Outputs from DeploySuperchain.s.sol. ISuperchainConfig internal _superchainConfigProxy; IProtocolVersions internal _protocolVersionsProxy; + ISharedLockbox internal _sharedLockboxProxy; string internal _standardVersionsToml; @@ -88,6 +90,7 @@ contract DeployImplementationsInput is BaseDeployIO { require(_addr != address(0), "DeployImplementationsInput: cannot set zero address"); if (_sel == this.superchainConfigProxy.selector) _superchainConfigProxy = ISuperchainConfig(_addr); else if (_sel == this.protocolVersionsProxy.selector) _protocolVersionsProxy = IProtocolVersions(_addr); + else if (_sel == this.sharedLockboxProxy.selector) _sharedLockboxProxy = ISharedLockbox(_addr); else revert("DeployImplementationsInput: unknown selector"); } @@ -153,6 +156,11 @@ contract DeployImplementationsInput is BaseDeployIO { require(address(_protocolVersionsProxy) != address(0), "DeployImplementationsInput: not set"); return _protocolVersionsProxy; } + + function sharedLockboxProxy() public view returns (ISharedLockbox) { + require(address(_sharedLockboxProxy) != address(0), "DeployImplementationsInput: not set"); + return _sharedLockboxProxy; + } } contract DeployImplementationsOutput is BaseDeployIO { @@ -711,13 +719,15 @@ contract DeployImplementations is Script { } else { uint256 proofMaturityDelaySeconds = _dii.proofMaturityDelaySeconds(); uint256 disputeGameFinalityDelaySeconds = _dii.disputeGameFinalityDelaySeconds(); + address sharedLockbox = address(_dii.sharedLockboxProxy()); vm.broadcast(msg.sender); impl = IOptimismPortal2( DeployUtils.create1({ _name: "OptimismPortal2", _args: DeployUtils.encodeConstructor( abi.encodeCall( - IOptimismPortal2.__constructor__, (proofMaturityDelaySeconds, disputeGameFinalityDelaySeconds) + IOptimismPortal2.__constructor__, + (proofMaturityDelaySeconds, disputeGameFinalityDelaySeconds, sharedLockbox) ) ) }) @@ -992,6 +1002,7 @@ contract DeployImplementationsInterop is DeployImplementations { } else { uint256 proofMaturityDelaySeconds = _dii.proofMaturityDelaySeconds(); uint256 disputeGameFinalityDelaySeconds = _dii.disputeGameFinalityDelaySeconds(); + address sharedLockbox = address(_dii.sharedLockboxProxy()); vm.broadcast(msg.sender); impl = IOptimismPortalInterop( DeployUtils.create1({ @@ -999,7 +1010,7 @@ contract DeployImplementationsInterop is DeployImplementations { _args: DeployUtils.encodeConstructor( abi.encodeCall( IOptimismPortalInterop.__constructor__, - (proofMaturityDelaySeconds, disputeGameFinalityDelaySeconds) + (proofMaturityDelaySeconds, disputeGameFinalityDelaySeconds, sharedLockbox) ) ) }) diff --git a/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol b/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol index 5e35e8848c8a..698e41c8c931 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol @@ -8,6 +8,7 @@ import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; import { IProtocolVersions, ProtocolVersion } from "src/L1/interfaces/IProtocolVersions.sol"; import { IProxyAdmin } from "src/universal/interfaces/IProxyAdmin.sol"; import { IProxy } from "src/universal/interfaces/IProxy.sol"; +import { ISharedLockbox } from "src/L1/interfaces/ISharedLockbox.sol"; import { DeployUtils } from "scripts/libraries/DeployUtils.sol"; import { Solarray } from "scripts/libraries/Solarray.sol"; @@ -161,6 +162,8 @@ contract DeploySuperchainOutput is BaseDeployIO { ISuperchainConfig internal _superchainConfigImpl; ISuperchainConfig internal _superchainConfigProxy; IProxyAdmin internal _superchainProxyAdmin; + ISharedLockbox internal _sharedLockboxImpl; + ISharedLockbox internal _sharedLockboxProxy; // This method lets each field be set individually. The selector of an output's getter method // is used to determine which field to set. @@ -171,6 +174,8 @@ contract DeploySuperchainOutput is BaseDeployIO { else if (_sel == this.superchainConfigProxy.selector) _superchainConfigProxy = ISuperchainConfig(_address); else if (_sel == this.protocolVersionsImpl.selector) _protocolVersionsImpl = IProtocolVersions(_address); else if (_sel == this.protocolVersionsProxy.selector) _protocolVersionsProxy = IProtocolVersions(_address); + else if (_sel == this.sharedLockboxImpl.selector) _sharedLockboxImpl = ISharedLockbox(_address); + else if (_sel == this.sharedLockboxProxy.selector) _sharedLockboxProxy = ISharedLockbox(_address); else revert("DeploySuperchainOutput: unknown selector"); } @@ -182,7 +187,9 @@ contract DeploySuperchainOutput is BaseDeployIO { address(this.superchainConfigImpl()), address(this.superchainConfigProxy()), address(this.protocolVersionsImpl()), - address(this.protocolVersionsProxy()) + address(this.protocolVersionsProxy()), + address(this.sharedLockboxImpl()), + address(this.sharedLockboxProxy()) ); DeployUtils.assertValidContractAddresses(addrs); @@ -190,12 +197,15 @@ contract DeploySuperchainOutput is BaseDeployIO { vm.startPrank(address(0)); address actualSuperchainConfigImpl = IProxy(payable(address(_superchainConfigProxy))).implementation(); address actualProtocolVersionsImpl = IProxy(payable(address(_protocolVersionsProxy))).implementation(); + address actualSharedLockboxImpl = IProxy(payable(address(_sharedLockboxProxy))).implementation(); vm.stopPrank(); require(actualSuperchainConfigImpl == address(_superchainConfigImpl), "100"); // nosemgrep: // sol-style-malformed-require require(actualProtocolVersionsImpl == address(_protocolVersionsImpl), "200"); // nosemgrep: // sol-style-malformed-require + require(actualSharedLockboxImpl == address(_sharedLockboxImpl), "300"); // nosemgrep: + // sol-style-malformed-require assertValidDeploy(_dsi); } @@ -225,11 +235,22 @@ contract DeploySuperchainOutput is BaseDeployIO { return _protocolVersionsProxy; } + function sharedLockboxImpl() public view returns (ISharedLockbox) { + DeployUtils.assertValidContractAddress(address(_sharedLockboxImpl)); + return _sharedLockboxImpl; + } + + function sharedLockboxProxy() public view returns (ISharedLockbox) { + DeployUtils.assertValidContractAddress(address(_sharedLockboxProxy)); + return _sharedLockboxProxy; + } + // -------- Deployment Assertions -------- function assertValidDeploy(DeploySuperchainInput _dsi) public { assertValidSuperchainProxyAdmin(_dsi); assertValidSuperchainConfig(_dsi); assertValidProtocolVersions(_dsi); + assertValidSharedLockbox(); } function assertValidSuperchainProxyAdmin(DeploySuperchainInput _dsi) internal view { @@ -280,6 +301,21 @@ contract DeploySuperchainOutput is BaseDeployIO { require(ProtocolVersion.unwrap(pv.required()) == 0, "PV-70"); require(ProtocolVersion.unwrap(pv.recommended()) == 0, "PV-80"); } + + function assertValidSharedLockbox() internal { + // Proxy checks. + ISharedLockbox sl = sharedLockboxProxy(); + + vm.startPrank(address(0)); + require(IProxy(payable(address(sl))).implementation() == address(sharedLockboxImpl()), "SLB-10"); + require(IProxy(payable(address(sl))).admin() == address(superchainProxyAdmin()), "SLB-20"); + require(sl.SUPERCHAIN_CONFIG() == address(superchainConfigProxy()), "SLB-30"); + vm.stopPrank(); + + // Implementation checks. + sl = sharedLockboxImpl(); + require(sl.SUPERCHAIN_CONFIG() == address(superchainConfigProxy()), "SLB-40"); + } } // For all broadcasts in this script we explicitly specify the deployer as `msg.sender` because for @@ -306,6 +342,7 @@ contract DeploySuperchain is Script { deploySuperchainImplementationContracts(_dsi, _dso); deployAndInitializeSuperchainConfig(_dsi, _dso); deployAndInitializeProtocolVersions(_dsi, _dso); + deploySharedLockbox(_dsi, _dso); // Transfer ownership of the ProxyAdmin from the deployer to the specified owner. transferProxyAdminOwnership(_dsi, _dso); @@ -415,6 +452,37 @@ contract DeploySuperchain is Script { _dso.set(_dso.protocolVersionsProxy.selector, address(protocolVersionsProxy)); } + function deploySharedLockbox(DeploySuperchainInput, DeploySuperchainOutput _dso) public { + IProxyAdmin superchainProxyAdmin = _dso.superchainProxyAdmin(); + ISuperchainConfig superchainConfigProxy = _dso.superchainConfigProxy(); + + vm.startBroadcast(msg.sender); + ISharedLockbox sharedLockboxImpl = ISharedLockbox( + DeployUtils.create1({ + _name: "SharedLockbox", + _args: DeployUtils.encodeConstructor( + abi.encodeCall(ISharedLockbox.__constructor__, (address(superchainConfigProxy))) + ) + }) + ); + ISharedLockbox sharedLockboxProxy = ISharedLockbox( + DeployUtils.create1({ + _name: "Proxy", + _args: DeployUtils.encodeConstructor( + abi.encodeCall(IProxy.__constructor__, (address(superchainProxyAdmin))) + ) + }) + ); + superchainProxyAdmin.upgrade(payable(address(sharedLockboxProxy)), address(sharedLockboxImpl)); + vm.stopBroadcast(); + + vm.label(address(sharedLockboxImpl), "SharedLockboxImpl"); + vm.label(address(sharedLockboxProxy), "SharedLockboxProxy"); + + _dso.set(_dso.sharedLockboxImpl.selector, address(sharedLockboxImpl)); + _dso.set(_dso.sharedLockboxProxy.selector, address(sharedLockboxProxy)); + } + function transferProxyAdminOwnership(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { address superchainProxyAdminOwner = _dsi.superchainProxyAdminOwner(); diff --git a/packages/contracts-bedrock/scripts/libraries/Types.sol b/packages/contracts-bedrock/scripts/libraries/Types.sol index db37e19457a8..6f7c39d3c88b 100644 --- a/packages/contracts-bedrock/scripts/libraries/Types.sol +++ b/packages/contracts-bedrock/scripts/libraries/Types.sol @@ -19,5 +19,6 @@ library Types { address ProtocolVersions; address SuperchainConfig; address OPContractsManager; + address SharedLockbox; } } diff --git a/packages/contracts-bedrock/snapshots/.gas-snapshot b/packages/contracts-bedrock/snapshots/.gas-snapshot index 2ab3157a714e..505885461f3a 100644 --- a/packages/contracts-bedrock/snapshots/.gas-snapshot +++ b/packages/contracts-bedrock/snapshots/.gas-snapshot @@ -4,13 +4,13 @@ GasBenchMark_L1BlockInterop_SetValuesInterop:test_setL1BlockValuesInterop_benchm GasBenchMark_L1BlockInterop_SetValuesInterop_Warm:test_setL1BlockValuesInterop_benchmark() (gas: 5099) GasBenchMark_L1Block_SetValuesEcotone:test_setL1BlockValuesEcotone_benchmark() (gas: 158531) GasBenchMark_L1Block_SetValuesEcotone_Warm:test_setL1BlockValuesEcotone_benchmark() (gas: 7597) -GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369280) -GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967465) -GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564398) -GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076613) -GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 467098) -GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512802) -GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72664) +GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369342) +GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967527) +GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564475) +GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076690) +GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 467025) +GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512729) +GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72661) GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 92973) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 68422) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 68986) diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json b/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json index 2f52ed573d37..5ca5e6bc7ffb 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json @@ -10,6 +10,11 @@ "internalType": "uint256", "name": "_disputeGameFinalityDelaySeconds", "type": "uint256" + }, + { + "internalType": "address", + "name": "_sharedLockbox", + "type": "address" } ], "stateMutability": "nonpayable", @@ -643,6 +648,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [], + "name": "sharedLockbox", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "superchainConfig", diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json b/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json index 5b9f72b9446c..8eb9ed0f74d6 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json @@ -10,6 +10,11 @@ "internalType": "uint256", "name": "_disputeGameFinalityDelaySeconds", "type": "uint256" + }, + { + "internalType": "address", + "name": "_sharedLockbox", + "type": "address" } ], "stateMutability": "nonpayable", @@ -661,6 +666,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [], + "name": "sharedLockbox", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "superchainConfig", diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 1fd49944195e..e8cdf3df5e46 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -28,12 +28,12 @@ "sourceCodeHash": "0xbe34b82900d02f71bb0949818eabe49531f7e0d8d8bae01f6dac4a296530d1aa" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0x218358b48f640b3fcb2d239f00dc1cd3b11517ad46c8e1efa44953d38da63540", - "sourceCodeHash": "0x66ac1212760db53a2bb1839e4cd17dc071d9273b8e6fb80646b79e91b3371c1a" + "initCodeHash": "0x215f439adc85bc5ebb1b234c1acc128eecb7c9a43243edbfcd9e701f128224f1", + "sourceCodeHash": "0x4c33430270b28fe0850c112c05d121477429688aae74c58754249476fb2d6c8e" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x39f66ac74341ec235fbdd0d79546283210bd8ac35a2ab2c4bd36c9722ce18411", - "sourceCodeHash": "0xbb98144285b9530e336f957d10b20363b350876597e30fd34821940896a2bae8" + "initCodeHash": "0x831af803158768b7fc229822eebda0ce7922cb1852fd770856b425023a379d47", + "sourceCodeHash": "0xe1dbe5e95ffed6587126ea65a5e4d3c8f11e1bd17a194a5665c7f0c0d32f29e3" }, "src/L1/ProtocolVersions.sol": { "initCodeHash": "0xefd4806e8737716d5d2022ca2e9e9fba0a0cb5714b026166b58e472222c7d15f", diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index 985711ed18e5..6fce8c1e16c9 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -45,6 +45,7 @@ import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; import { IDisputeGameFactory } from "src/dispute/interfaces/IDisputeGameFactory.sol"; import { IDisputeGame } from "src/dispute/interfaces/IDisputeGame.sol"; import { IL1Block } from "src/L2/interfaces/IL1Block.sol"; +import { ISharedLockbox } from "src/L1/interfaces/ISharedLockbox.sol"; /// @custom:proxied true /// @title OptimismPortal2 @@ -70,6 +71,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { /// finalized. uint256 internal immutable DISPUTE_GAME_FINALITY_DELAY_SECONDS; + /// @notice The Shared Lockbox contract. + ISharedLockbox internal immutable SHARED_LOCKBOX; + /// @notice Version of the deposit event. uint256 internal constant DEPOSIT_VERSION = 0; @@ -177,21 +181,21 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { event RespectedGameTypeSet(GameType indexed newGameType, Timestamp indexed updatedAt); /// @notice Reverts when paused. - modifier whenNotPaused() { + function _whenNotPaused() internal view { if (paused()) revert CallPaused(); - _; } /// @notice Semantic version. - /// @custom:semver 3.11.0-beta.6 + /// @custom:semver 3.11.0-beta.7 function version() public pure virtual returns (string memory) { - return "3.11.0-beta.6"; + return "3.11.0-beta.7"; } /// @notice Constructs the OptimismPortal contract. - constructor(uint256 _proofMaturityDelaySeconds, uint256 _disputeGameFinalityDelaySeconds) { + constructor(uint256 _proofMaturityDelaySeconds, uint256 _disputeGameFinalityDelaySeconds, address _sharedLockbox) { PROOF_MATURITY_DELAY_SECONDS = _proofMaturityDelaySeconds; DISPUTE_GAME_FINALITY_DELAY_SECONDS = _disputeGameFinalityDelaySeconds; + SHARED_LOCKBOX = ISharedLockbox(_sharedLockbox); initialize({ _disputeGameFactory: IDisputeGameFactory(address(0)), @@ -244,6 +248,13 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { } } + /// @notice Returns the `_token` balance of the `_account`. + /// @param _token Address of the token to check the balance of. + /// @param _account The address of the account to query the balance for. + function _balanceOf(address _token, address _account) internal view returns (uint256) { + return IERC20(_token).balanceOf(_account); + } + /// @notice Getter function for the address of the guardian. /// Public getter is legacy and will be removed in the future. Use `SuperchainConfig.guardian()` instead. /// @return Address of the guardian. @@ -267,6 +278,11 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { return DISPUTE_GAME_FINALITY_DELAY_SECONDS; } + /// @notice Getter for the address of the shared lockbox. + function sharedLockbox() public view returns (address) { + return address(SHARED_LOCKBOX); + } + /// @notice Computes the minimum gas limit for a deposit. /// The minimum gas limit linearly increases based on the size of the calldata. /// This is to prevent users from creating L2 resource usage without paying for it. @@ -321,8 +337,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { bytes[] calldata _withdrawalProof ) external - whenNotPaused { + _whenNotPaused(); + // Prevent users from creating a deposit transaction where this address is the message // sender on L2. Because this is checked here, we do not need to check again in // `finalizeWithdrawalTransaction`. @@ -385,7 +402,8 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { /// @notice Finalizes a withdrawal transaction. /// @param _tx Withdrawal transaction to finalize. - function finalizeWithdrawalTransaction(Types.WithdrawalTransaction memory _tx) external whenNotPaused { + function finalizeWithdrawalTransaction(Types.WithdrawalTransaction memory _tx) external { + _whenNotPaused(); finalizeWithdrawalTransactionExternalProof(_tx, msg.sender); } @@ -397,8 +415,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { address _proofSubmitter ) public - whenNotPaused { + _whenNotPaused(); + // Make sure that the l2Sender has not yet been set. The l2Sender is set to a value other // than the default value when a withdrawal transaction is being finalized. This check is // a defacto reentrancy guard. @@ -419,6 +438,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { bool success; (address token,) = gasPayingToken(); if (token == Constants.ETHER) { + // Unlock and receive the ETH from the shared lockbox. + if (_tx.value != 0) SHARED_LOCKBOX.unlockETH(_tx.value); + // Trigger the call to the target contract. We use a custom low level method // SafeCall.callWithMinGas to ensure two key properties // 1. Target contracts cannot force this call to run out of gas by returning a very large @@ -440,7 +462,7 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { // Read the balance of the target contract before the transfer so the consistency // of the transfer can be checked afterwards. - uint256 startBalance = IERC20(token).balanceOf(address(this)); + uint256 startBalance = _balanceOf(token, address(this)); // Transfer the ERC20 balance to the target, accounting for non standard ERC20 // implementations that may not return a boolean. This reverts if the low level @@ -448,7 +470,7 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { IERC20(token).safeTransfer({ to: _tx.target, value: _tx.value }); // The balance must be transferred exactly. - if (IERC20(token).balanceOf(address(this)) != startBalance - _tx.value) { + if (_balanceOf(token, address(this)) != startBalance - _tx.value) { revert TransferFailed(); } } @@ -505,13 +527,13 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { _balance += _mint; // Get the balance of the portal before the transfer. - uint256 startBalance = IERC20(token).balanceOf(address(this)); + uint256 startBalance = _balanceOf(token, address(this)); // Take ownership of the token. It is assumed that the user has given the portal an approval. IERC20(token).safeTransferFrom({ from: msg.sender, to: address(this), value: _mint }); // Double check that the portal now has the exact amount of token. - if (IERC20(token).balanceOf(address(this)) != startBalance + _mint) { + if (_balanceOf(token, address(this)) != startBalance + _mint) { revert TransferFailed(); } @@ -548,6 +570,11 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { (address token,) = gasPayingToken(); if (token != Constants.ETHER && msg.value != 0) revert NoValue(); + if (token == Constants.ETHER && msg.value != 0) { + // Lock the ETH in the shared lockbox. + SHARED_LOCKBOX.lockETH{ value: msg.value }(); + } + _depositTransaction({ _to: _to, _mint: msg.value, diff --git a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol index 4c238c415d34..a86e94405498 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol @@ -18,14 +18,15 @@ import { Unauthorized } from "src/libraries/PortalErrors.sol"; contract OptimismPortalInterop is OptimismPortal2 { constructor( uint256 _proofMaturityDelaySeconds, - uint256 _disputeGameFinalityDelaySeconds + uint256 _disputeGameFinalityDelaySeconds, + address _sharedLockbox ) - OptimismPortal2(_proofMaturityDelaySeconds, _disputeGameFinalityDelaySeconds) + OptimismPortal2(_proofMaturityDelaySeconds, _disputeGameFinalityDelaySeconds, _sharedLockbox) { } - /// @custom:semver +interop-beta.2 + /// @custom:semver +interop-beta.3 function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop-beta.2"); + return string.concat(super.version(), "+interop-beta.3"); } /// @notice Sets static configuration options for the L2 system. diff --git a/packages/contracts-bedrock/src/L1/interfaces/IOptimismPortal2.sol b/packages/contracts-bedrock/src/L1/interfaces/IOptimismPortal2.sol index 91f09d714314..662d6e1e92d1 100644 --- a/packages/contracts-bedrock/src/L1/interfaces/IOptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/interfaces/IOptimismPortal2.sol @@ -69,6 +69,7 @@ interface IOptimismPortal2 { function disputeGameBlacklist(IDisputeGame) external view returns (bool); function disputeGameFactory() external view returns (IDisputeGameFactory); function disputeGameFinalityDelaySeconds() external view returns (uint256); + function sharedLockbox() external view returns (address); function donateETH() external payable; function finalizeWithdrawalTransaction(Types.WithdrawalTransaction memory _tx) external; function finalizeWithdrawalTransactionExternalProof( @@ -114,5 +115,10 @@ interface IOptimismPortal2 { function systemConfig() external view returns (ISystemConfig); function version() external pure returns (string memory); - function __constructor__(uint256 _proofMaturityDelaySeconds, uint256 _disputeGameFinalityDelaySeconds) external; + function __constructor__( + uint256 _proofMaturityDelaySeconds, + uint256 _disputeGameFinalityDelaySeconds, + address _sharedLockbox + ) + external; } diff --git a/packages/contracts-bedrock/src/L1/interfaces/IOptimismPortalInterop.sol b/packages/contracts-bedrock/src/L1/interfaces/IOptimismPortalInterop.sol index 521c7232e125..276b9cea14b2 100644 --- a/packages/contracts-bedrock/src/L1/interfaces/IOptimismPortalInterop.sol +++ b/packages/contracts-bedrock/src/L1/interfaces/IOptimismPortalInterop.sol @@ -70,6 +70,7 @@ interface IOptimismPortalInterop { function disputeGameBlacklist(IDisputeGame) external view returns (bool); function disputeGameFactory() external view returns (IDisputeGameFactory); function disputeGameFinalityDelaySeconds() external view returns (uint256); + function sharedLockbox() external view returns (address); function donateETH() external payable; function finalizeWithdrawalTransaction(Types.WithdrawalTransaction memory _tx) external; function finalizeWithdrawalTransactionExternalProof( @@ -116,5 +117,10 @@ interface IOptimismPortalInterop { function systemConfig() external view returns (ISystemConfig); function version() external pure returns (string memory); - function __constructor__(uint256 _proofMaturityDelaySeconds, uint256 _disputeGameFinalityDelaySeconds) external; + function __constructor__( + uint256 _proofMaturityDelaySeconds, + uint256 _disputeGameFinalityDelaySeconds, + address _sharedLockbox + ) + external; } diff --git a/packages/contracts-bedrock/test/L1/L1StandardBridge.t.sol b/packages/contracts-bedrock/test/L1/L1StandardBridge.t.sol index 97ef01262ab6..a2e052e780e4 100644 --- a/packages/contracts-bedrock/test/L1/L1StandardBridge.t.sol +++ b/packages/contracts-bedrock/test/L1/L1StandardBridge.t.sol @@ -163,6 +163,7 @@ contract L1StandardBridge_Receive_Test is CommonTest { /// @dev Tests receive bridges ETH successfully. function test_receive_succeeds() external { assertEq(address(optimismPortal).balance, 0); + assertEq(address(sharedLockbox).balance, 0); // The legacy event must be emitted for backwards compatibility vm.expectEmit(address(l1StandardBridge)); @@ -186,7 +187,8 @@ contract L1StandardBridge_Receive_Test is CommonTest { vm.prank(alice, alice); (bool success,) = address(l1StandardBridge).call{ value: 100 }(hex""); assertEq(success, true); - assertEq(address(optimismPortal).balance, 100); + assertEq(address(optimismPortal).balance, 0); + assertEq(address(sharedLockbox).balance, 100); } } @@ -212,6 +214,8 @@ contract PreBridgeETH is CommonTest { /// on whether the bridge call is legacy or not. function _preBridgeETH(bool isLegacy, uint256 value) internal { assertEq(address(optimismPortal).balance, 0); + assertEq(address(sharedLockbox).balance, 0); + uint256 nonce = l1CrossDomainMessenger.messageNonce(); uint256 version = 0; // Internal constant in the OptimismPortal: DEPOSIT_VERSION address l1MessengerAliased = AddressAliasHelper.applyL1ToL2Alias(address(l1CrossDomainMessenger)); @@ -277,11 +281,13 @@ contract L1StandardBridge_DepositETH_Test is PreBridgeETH { /// Emits ETHDepositInitiated and ETHBridgeInitiated events. /// Calls depositTransaction on the OptimismPortal. /// Only EOA can call depositETH. - /// ETH ends up in the optimismPortal. + /// ETH ends up in the sharedLockbox. function test_depositETH_succeeds() external { _preBridgeETH({ isLegacy: true, value: 500 }); l1StandardBridge.depositETH{ value: 500 }(50000, hex"dead"); - assertEq(address(optimismPortal).balance, 500); + + assertEq(address(optimismPortal).balance, 0); + assertEq(address(sharedLockbox).balance, 500); } } @@ -310,11 +316,13 @@ contract L1StandardBridge_BridgeETH_Test is PreBridgeETH { /// Emits ETHDepositInitiated and ETHBridgeInitiated events. /// Calls depositTransaction on the OptimismPortal. /// Only EOA can call bridgeETH. - /// ETH ends up in the optimismPortal. + /// ETH ends up in the sharedLockbox. function test_bridgeETH_succeeds() external { _preBridgeETH({ isLegacy: false, value: 500 }); l1StandardBridge.bridgeETH{ value: 500 }(50000, hex"dead"); - assertEq(address(optimismPortal).balance, 500); + + assertEq(address(optimismPortal).balance, 0); + assertEq(address(sharedLockbox).balance, 500); } } @@ -336,6 +344,8 @@ contract PreBridgeETHTo is CommonTest { /// address depending on whether the bridge call is legacy or not. function _preBridgeETHTo(bool isLegacy, uint256 value) internal { assertEq(address(optimismPortal).balance, 0); + assertEq(address(sharedLockbox).balance, 0); + uint256 nonce = l1CrossDomainMessenger.messageNonce(); uint256 version = 0; // Internal constant in the OptimismPortal: DEPOSIT_VERSION address l1MessengerAliased = AddressAliasHelper.applyL1ToL2Alias(address(l1CrossDomainMessenger)); @@ -403,11 +413,13 @@ contract L1StandardBridge_DepositETHTo_Test is PreBridgeETHTo { /// Emits ETHDepositInitiated event. /// Calls depositTransaction on the OptimismPortal. /// EOA or contract can call depositETHTo. - /// ETH ends up in the optimismPortal. + /// ETH ends up in the sharedLockbox. function test_depositETHTo_succeeds() external { _preBridgeETHTo({ isLegacy: true, value: 600 }); l1StandardBridge.depositETHTo{ value: 600 }(bob, 60000, hex"dead"); - assertEq(address(optimismPortal).balance, 600); + + assertEq(address(optimismPortal).balance, 0); + assertEq(address(sharedLockbox).balance, 600); } } @@ -436,11 +448,13 @@ contract L1StandardBridge_BridgeETHTo_Test is PreBridgeETHTo { /// Emits ETHDepositInitiated and ETHBridgeInitiated events. /// Calls depositTransaction on the OptimismPortal. /// Only EOA can call bridgeETHTo. - /// ETH ends up in the optimismPortal. + /// ETH ends up in the sharedLockbox. function test_bridgeETHTo_succeeds() external { _preBridgeETHTo({ isLegacy: false, value: 600 }); l1StandardBridge.bridgeETHTo{ value: 600 }(bob, 60000, hex"dead"); - assertEq(address(optimismPortal).balance, 600); + + assertEq(address(optimismPortal).balance, 0); + assertEq(address(sharedLockbox).balance, 600); } } diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index 083644755d3c..0c06ae5ed396 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -55,6 +55,7 @@ contract OptimismPortal2_Test is CommonTest { assertEq(address(opImpl.superchainConfig()), address(0)); assertEq(opImpl.l2Sender(), Constants.DEFAULT_L2_SENDER); assertEq(opImpl.respectedGameType().raw(), deploy.cfg().respectedGameType()); + assertEq(opImpl.sharedLockbox(), address(sharedLockbox)); } /// @dev Tests that the initializer sets the correct values. @@ -69,6 +70,7 @@ contract OptimismPortal2_Test is CommonTest { assertEq(optimismPortal2.l2Sender(), Constants.DEFAULT_L2_SENDER); assertEq(optimismPortal2.paused(), false); assertEq(optimismPortal2.respectedGameType().raw(), deploy.cfg().respectedGameType()); + assertEq(optimismPortal2.sharedLockbox(), address(sharedLockbox)); } /// @dev Tests that `pause` successfully pauses @@ -145,13 +147,17 @@ contract OptimismPortal2_Test is CommonTest { _data: hex"" }); + // Expect call to the SharedLockbox to lock the funds + if (_value > 0) vm.expectCall(address(sharedLockbox), _value, abi.encodeCall(sharedLockbox.lockETH, ())); + // give alice money and send as an eoa vm.deal(alice, _value); vm.prank(alice, alice); (bool s,) = address(optimismPortal2).call{ value: _value }(hex""); assertTrue(s); - assertEq(address(optimismPortal2).balance, _value); + assertEq(address(optimismPortal2).balance, 0); + assertEq(address(sharedLockbox).balance, _value); } /// @dev Tests that `depositTransaction` reverts when the destination address is non-zero @@ -243,6 +249,9 @@ contract OptimismPortal2_Test is CommonTest { _data: _data }); + // Expect call to the SharedLockbox to lock the funds + if (_mint > 0) vm.expectCall(address(sharedLockbox), _mint, abi.encodeCall(sharedLockbox.lockETH, ())); + vm.deal(depositor, _mint); vm.prank(depositor, depositor); optimismPortal2.depositTransaction{ value: _mint }({ @@ -252,7 +261,9 @@ contract OptimismPortal2_Test is CommonTest { _isCreation: _isCreation, _data: _data }); - assertEq(address(optimismPortal2).balance, _mint); + + assertEq(address(optimismPortal2).balance, 0); + assertEq(address(sharedLockbox).balance, _mint); } /// @dev Tests that `depositTransaction` succeeds for a contract. @@ -287,6 +298,9 @@ contract OptimismPortal2_Test is CommonTest { _data: _data }); + // Expect call to the SharedLockbox to lock the funds + if (_mint > 0) vm.expectCall(address(sharedLockbox), _mint, abi.encodeCall(sharedLockbox.lockETH, ())); + vm.deal(address(this), _mint); vm.prank(address(this)); optimismPortal2.depositTransaction{ value: _mint }({ @@ -296,7 +310,9 @@ contract OptimismPortal2_Test is CommonTest { _isCreation: _isCreation, _data: _data }); - assertEq(address(optimismPortal2).balance, _mint); + + assertEq(address(optimismPortal2).balance, 0); + assertEq(address(sharedLockbox).balance, _mint); } /// @dev Tests that the gas paying token can be set. @@ -477,8 +493,8 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { // Warp beyond the chess clocks and finalize the game. vm.warp(block.timestamp + game.maxClockDuration().raw() + 1 seconds); - // Fund the portal so that we can withdraw ETH. - vm.deal(address(optimismPortal2), 0xFFFFFFFF); + // Fund the SharedLockbox so that we can withdraw ETH. + vm.deal(address(sharedLockbox), 0xFFFFFFFF); } /// @dev Asserts that the reentrant call will revert. @@ -862,9 +878,10 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { uint256 _proposedGameIndex_noData = disputeGameFactory.gameCount() - 1; // Warp beyond the chess clocks and finalize the game. vm.warp(block.timestamp + game_noData.maxClockDuration().raw() + 1 seconds); - // Fund the portal so that we can withdraw ETH. - vm.store(address(optimismPortal2), bytes32(uint256(61)), bytes32(uint256(0xFFFFFFFF))); - vm.deal(address(optimismPortal2), 0xFFFFFFFF); + + // Fund the SharedLockbox so that we can withdraw ETH. + vm.deal(address(sharedLockbox), _defaultTx_noData.value); + vm.expectCall(address(sharedLockbox), abi.encodeCall(sharedLockbox.unlockETH, (_defaultTx_noData.value))); uint256 bobBalanceBefore = bob.balance; @@ -969,6 +986,10 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { function test_finalizeWithdrawalTransaction_provenWithdrawalHashEther_succeeds() external { uint256 bobBalanceBefore = address(bob).balance; + // Fund the SharedLockbox so that we can withdraw ETH. + vm.deal(address(sharedLockbox), _defaultTx.value); + vm.expectCall(address(sharedLockbox), abi.encodeCall(sharedLockbox.unlockETH, (_defaultTx.value))); + vm.expectEmit(address(optimismPortal2)); emit WithdrawalProven(_withdrawalHash, alice, bob); vm.expectEmit(address(optimismPortal2)); @@ -1005,6 +1026,10 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { // Warp 1 second into the future so that the proof is submitted after the timestamp of game creation. vm.warp(block.timestamp + 1 seconds); + // Fund the SharedLockbox so that we can withdraw ETH. + vm.deal(address(sharedLockbox), _defaultTx.value); + vm.expectCall(address(sharedLockbox), abi.encodeCall(sharedLockbox.unlockETH, (_defaultTx.value))); + // Prove the withdrawal transaction against the invalid dispute game, as 0xb0b. vm.expectEmit(true, true, true, true); emit WithdrawalProven(_withdrawalHash, alice, bob); @@ -1182,6 +1207,10 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { uint256 bobBalanceBefore = address(bob).balance; vm.etch(bob, hex"fe"); // Contract with just the invalid opcode. + // Fund the SharedLockbox so that we can withdraw ETH. + vm.deal(address(sharedLockbox), _defaultTx.value); + vm.expectCall(address(sharedLockbox), abi.encodeCall(sharedLockbox.unlockETH, (_defaultTx.value))); + vm.expectEmit(true, true, true, true); emit WithdrawalProven(_withdrawalHash, alice, bob); vm.expectEmit(true, true, true, true); @@ -1208,6 +1237,10 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { /// @dev Tests that `finalizeWithdrawalTransaction` reverts if the withdrawal has already been /// finalized. function test_finalizeWithdrawalTransaction_onReplay_reverts() external { + // Fund the SharedLockbox so that we can withdraw ETH. + vm.deal(address(sharedLockbox), _defaultTx.value); + vm.expectCall(address(sharedLockbox), abi.encodeCall(sharedLockbox.unlockETH, (_defaultTx.value))); + vm.expectEmit(true, true, true, true); emit WithdrawalProven(_withdrawalHash, alice, bob); vm.expectEmit(true, true, true, true); @@ -1305,6 +1338,10 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { // Return a mock output root from the game. vm.mockCall(address(game), abi.encodeCall(game.rootClaim, ()), abi.encode(outputRoot)); + // Fund the SharedLockbox so that we can withdraw ETH. + vm.deal(address(sharedLockbox), _testTx.value); + vm.expectCall(address(sharedLockbox), abi.encodeCall(sharedLockbox.unlockETH, (_testTx.value))); + vm.expectEmit(true, true, true, true); emit WithdrawalProven(withdrawalHash, alice, address(this)); vm.expectEmit(true, true, true, true); @@ -1345,7 +1382,9 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { // Total ETH supply is currently about 120M ETH. uint256 value = bound(_value, 0, 200_000_000 ether); - vm.deal(address(optimismPortal2), value); + + // Add ETH to the SharedLockbox for the portal to withdraw. + vm.deal(address(sharedLockbox), value); uint256 gasLimit = bound(_gasLimit, 0, 50_000_000); uint256 nonce = l2ToL1MessagePasser.messageNonce(); @@ -1394,6 +1433,9 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { // Warp past the finalization period vm.warp(block.timestamp + optimismPortal2.proofMaturityDelaySeconds() + 1); + // Expect call to the SharedLockbox to unlock the funds + if (value > 0) vm.expectCall(address(sharedLockbox), abi.encodeCall(sharedLockbox.unlockETH, (value))); + // Finalize the withdrawal transaction vm.expectCallMinGas(_tx.target, _tx.value, uint64(_tx.gasLimit), _tx.data); optimismPortal2.finalizeWithdrawalTransaction(_tx); diff --git a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol index c3a9e9466352..09b846b1c05a 100644 --- a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol +++ b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.15; // Testing utilities -import { Test } from "forge-std/Test.sol"; +import { CommonTest } from "test/setup/CommonTest.sol"; import { Unauthorized } from "src/libraries/errors/CommonErrors.sol"; // Targent contract @@ -11,27 +11,13 @@ import { SharedLockbox } from "src/L1/SharedLockbox.sol"; // Interfaces import { IOptimismPortal } from "src/L1/interfaces/IOptimismPortal.sol"; -contract SharedLockboxTest is Test { +contract SharedLockboxTest is CommonTest { event ETHLocked(address indexed portal, uint256 amount); event ETHUnlocked(address indexed portal, uint256 amount); event AuthorizedPortal(address indexed portal); - address internal immutable SUPERCHAIN_CONFIG = makeAddr("SuperchainConfig"); - IOptimismPortal internal immutable PORTAL = IOptimismPortal(payable(makeAddr("OptimismPortal"))); - SharedLockbox public sharedLockbox; - - // TODO: Update setup to use CommonTest and simulate a real deployment environment - function setUp() public { - // Deploy the SharedLockbox contract - sharedLockbox = new SharedLockbox(SUPERCHAIN_CONFIG); - - // Etch the OptimismPortal contract code into the declared address - bytes memory code = vm.getDeployedCode("L1/OptimismPortal.sol:OptimismPortal"); - vm.etch(address(PORTAL), code); - } - /// @notice Tests it reverts when the caller is not an authorized portal. function test_lockETH_unauthorizedPortal_reverts(address _caller) public { vm.assume(!sharedLockbox.authorizedPortals(_caller)); @@ -47,7 +33,7 @@ contract SharedLockboxTest is Test { /// @notice Tests the ETH is correctly locked when the caller is an authorized portal. function test_lockETH_succeeds(address _portal, uint256 _amount) public { // Set the caller as an authorized portal - vm.prank(SUPERCHAIN_CONFIG); + vm.prank(address(superchainConfig)); sharedLockbox.authorizePortal(_portal); // Deal the ETH amount to the portal @@ -85,35 +71,35 @@ contract SharedLockboxTest is Test { /// @notice Tests the ETH is correctly unlocked when the caller is an authorized portal. function test_unlockETH_succeeds(uint256 _value) public { // Set the caller as an authorized portal - vm.prank(SUPERCHAIN_CONFIG); - sharedLockbox.authorizePortal(address(PORTAL)); + vm.prank(address(superchainConfig)); + sharedLockbox.authorizePortal(address(optimismPortal2)); // Deal the ETH amount to the lockbox vm.deal(address(sharedLockbox), _value); // Get the balance of the portal and lockbox before the unlock to compare later on the assertions - uint256 _portalBalanceBefore = address(PORTAL).balance; + uint256 _portalBalanceBefore = address(optimismPortal2).balance; uint256 _lockboxBalanceBefore = address(sharedLockbox).balance; // Expect `donateETH` function to be called on Portal - vm.expectCall(address(PORTAL), abi.encodeWithSelector(IOptimismPortal.donateETH.selector)); + vm.expectCall(address(optimismPortal2), abi.encodeWithSelector(IOptimismPortal.donateETH.selector)); // Look for the emit of the `ETHUnlocked` event vm.expectEmit(address(sharedLockbox)); - emit ETHUnlocked(address(PORTAL), _value); + emit ETHUnlocked(address(optimismPortal2), _value); // Call the `unlockETH` function with the portal - vm.prank(address(PORTAL)); + vm.prank(address(optimismPortal2)); sharedLockbox.unlockETH(_value); // Assert the portal's balance increased and the lockbox's balance decreased by the amount unlocked - assertEq(address(PORTAL).balance, _portalBalanceBefore + _value); + assertEq(address(optimismPortal2).balance, _portalBalanceBefore + _value); assertEq(address(sharedLockbox).balance, _lockboxBalanceBefore - _value); } /// @notice Tests it reverts when the caller is not the SuperchainConfig. function test_authorizePortal_notSuperchainConfig_reverts(address _caller) public { - vm.assume(_caller != SUPERCHAIN_CONFIG); + vm.assume(_caller != address(superchainConfig)); // Expect the revert with `Unauthorized` selector vm.expectRevert(Unauthorized.selector); @@ -134,7 +120,7 @@ contract SharedLockboxTest is Test { emit AuthorizedPortal(_portal); // Call the `authorizePortal` function with the SuperchainConfig - vm.prank(SUPERCHAIN_CONFIG); + vm.prank(address(superchainConfig)); sharedLockbox.authorizePortal(_portal); // Assert the portal's authorized status was updated correctly diff --git a/packages/contracts-bedrock/test/invariants/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/invariants/OptimismPortal2.t.sol index 5e0e866dcfb1..d77bc8ca0673 100644 --- a/packages/contracts-bedrock/test/invariants/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/invariants/OptimismPortal2.t.sol @@ -136,8 +136,8 @@ contract OptimismPortal2_Invariant_Harness is CommonTest { game.resolveClaim(0, 0); game.resolve(); - // Fund the portal so that we can withdraw ETH. - vm.deal(address(optimismPortal2), 0xFFFFFFFF); + // Fund the SharedLockbox so that we can withdraw ETH. + vm.deal(address(sharedLockbox), 0xFFFFFFFF); } } diff --git a/packages/contracts-bedrock/test/opcm/DeployImplementations.t.sol b/packages/contracts-bedrock/test/opcm/DeployImplementations.t.sol index 8d3feac2de0d..863e443dd169 100644 --- a/packages/contracts-bedrock/test/opcm/DeployImplementations.t.sol +++ b/packages/contracts-bedrock/test/opcm/DeployImplementations.t.sol @@ -11,6 +11,7 @@ import { IDisputeGameFactory } from "src/dispute/interfaces/IDisputeGameFactory. import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; import { IProtocolVersions } from "src/L1/interfaces/IProtocolVersions.sol"; +import { ISharedLockbox } from "src/L1/interfaces/ISharedLockbox.sol"; import { OPContractsManager } from "src/L1/OPContractsManager.sol"; import { IOptimismPortal2 } from "src/L1/interfaces/IOptimismPortal2.sol"; import { ISystemConfig } from "src/L1/interfaces/ISystemConfig.sol"; @@ -39,6 +40,7 @@ contract DeployImplementationsInput_Test is Test { string release = "dev-release"; // this means implementation contracts will be deployed ISuperchainConfig superchainConfigProxy = ISuperchainConfig(makeAddr("superchainConfigProxy")); IProtocolVersions protocolVersionsProxy = IProtocolVersions(makeAddr("protocolVersionsProxy")); + ISharedLockbox sharedLockboxProxy = ISharedLockbox(makeAddr("sharedLockboxProxy")); function setUp() public { dii = new DeployImplementationsInput(); @@ -71,6 +73,9 @@ contract DeployImplementationsInput_Test is Test { vm.expectRevert("DeployImplementationsInput: not set"); dii.standardVersionsToml(); + + vm.expectRevert("DeployImplementationsInput: not set"); + dii.sharedLockboxProxy(); } } @@ -223,6 +228,7 @@ contract DeployImplementations_Test is Test { uint256 disputeGameFinalityDelaySeconds = 500; ISuperchainConfig superchainConfigProxy = ISuperchainConfig(makeAddr("superchainConfigProxy")); IProtocolVersions protocolVersionsProxy = IProtocolVersions(makeAddr("protocolVersionsProxy")); + ISharedLockbox sharedLockboxProxy = ISharedLockbox(makeAddr("sharedLockboxProxy")); function setUp() public virtual { deployImplementations = new DeployImplementations(); @@ -342,6 +348,7 @@ contract DeployImplementations_Test is Test { vm.etch(address(superchainProxyAdmin), address(superchainProxyAdmin).code); vm.etch(address(superchainConfigProxy), address(superchainConfigProxy).code); vm.etch(address(protocolVersionsProxy), hex"01"); + vm.etch(address(sharedLockboxProxy), hex"01"); dii.set(dii.withdrawalDelaySeconds.selector, withdrawalDelaySeconds); dii.set(dii.minProposalSizeBytes.selector, minProposalSizeBytes); @@ -352,6 +359,7 @@ contract DeployImplementations_Test is Test { dii.set(dii.l1ContractsRelease.selector, release); dii.set(dii.superchainConfigProxy.selector, address(superchainConfigProxy)); dii.set(dii.protocolVersionsProxy.selector, address(protocolVersionsProxy)); + dii.set(dii.sharedLockboxProxy.selector, address(sharedLockboxProxy)); deployImplementations.run(dii, dio); @@ -365,6 +373,7 @@ contract DeployImplementations_Test is Test { assertEq(release, dii.l1ContractsRelease(), "525"); assertEq(address(superchainConfigProxy), address(dii.superchainConfigProxy()), "550"); assertEq(address(protocolVersionsProxy), address(dii.protocolVersionsProxy()), "575"); + assertEq(address(sharedLockboxProxy), address(dii.sharedLockboxProxy()), "577"); // Architecture assertions. assertEq(address(dio.mipsSingleton().oracle()), address(dio.preimageOracleSingleton()), "600"); @@ -386,6 +395,7 @@ contract DeployImplementations_Test is Test { dii.set(dii.l1ContractsRelease.selector, release); dii.set(dii.superchainConfigProxy.selector, address(superchainConfigProxy)); dii.set(dii.protocolVersionsProxy.selector, address(protocolVersionsProxy)); + dii.set(dii.sharedLockboxProxy.selector, address(sharedLockboxProxy)); // Set the challenge period to a value that is too large, using vm.store because the setter // method won't allow it. diff --git a/packages/contracts-bedrock/test/opcm/DeployOPChain.t.sol b/packages/contracts-bedrock/test/opcm/DeployOPChain.t.sol index 5280328168b9..5960a360bd4d 100644 --- a/packages/contracts-bedrock/test/opcm/DeployOPChain.t.sol +++ b/packages/contracts-bedrock/test/opcm/DeployOPChain.t.sol @@ -23,6 +23,7 @@ import { IL1ChugSplashProxy } from "src/legacy/interfaces/IL1ChugSplashProxy.sol import { IResolvedDelegateProxy } from "src/legacy/interfaces/IResolvedDelegateProxy.sol"; import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; +import { ISharedLockbox } from "src/L1/interfaces/ISharedLockbox.sol"; import { IProtocolVersions, ProtocolVersion } from "src/L1/interfaces/IProtocolVersions.sol"; import { OPContractsManager } from "src/L1/OPContractsManager.sol"; import { IProxy } from "src/universal/interfaces/IProxy.sol"; @@ -314,7 +315,7 @@ contract DeployOPChain_TestBase is Test { ProtocolVersion recommendedProtocolVersion = ProtocolVersion.wrap(2); // Define default inputs for DeployImplementations. - // `superchainConfigProxy` and `protocolVersionsProxy` are set during `setUp` since they are + // `superchainConfigProxy`, `protocolVersionsProxy` and `sharedLockboxProxy` are set during `setUp` since they are // outputs of the previous step. uint256 withdrawalDelaySeconds = 100; uint256 minProposalSizeBytes = 200; @@ -324,6 +325,7 @@ contract DeployOPChain_TestBase is Test { string release = "dev-release"; // this means implementation contracts will be deployed ISuperchainConfig superchainConfigProxy; IProtocolVersions protocolVersionsProxy; + ISharedLockbox sharedLockboxProxy; // Define default inputs for DeployOPChain. // `opcm` is set during `setUp` since it is an output of the previous step. @@ -384,6 +386,7 @@ contract DeployOPChain_TestBase is Test { // Populate the inputs for DeployImplementations based on the output of DeploySuperchain. superchainConfigProxy = dso.superchainConfigProxy(); protocolVersionsProxy = dso.protocolVersionsProxy(); + sharedLockboxProxy = dso.sharedLockboxProxy(); // Configure and deploy Implementation contracts DeployImplementations deployImplementations = createDeployImplementationsContract(); @@ -398,6 +401,7 @@ contract DeployOPChain_TestBase is Test { dii.set(dii.l1ContractsRelease.selector, release); dii.set(dii.superchainConfigProxy.selector, address(superchainConfigProxy)); dii.set(dii.protocolVersionsProxy.selector, address(protocolVersionsProxy)); + dii.set(dii.sharedLockboxProxy.selector, address(sharedLockboxProxy)); // End users of the DeployImplementations contract will need to set the `standardVersionsToml`. string memory standardVersionsTomlPath = string.concat(vm.projectRoot(), "/test/fixtures/standard-versions.toml"); diff --git a/packages/contracts-bedrock/test/opcm/DeploySuperchain.t.sol b/packages/contracts-bedrock/test/opcm/DeploySuperchain.t.sol index 924957cc1800..4afb7a60a735 100644 --- a/packages/contracts-bedrock/test/opcm/DeploySuperchain.t.sol +++ b/packages/contracts-bedrock/test/opcm/DeploySuperchain.t.sol @@ -7,6 +7,7 @@ import { stdToml } from "forge-std/StdToml.sol"; import { ProxyAdmin } from "src/universal/ProxyAdmin.sol"; import { Proxy } from "src/universal/Proxy.sol"; import { SuperchainConfig } from "src/L1/SuperchainConfig.sol"; +import { SharedLockbox } from "src/L1/SharedLockbox.sol"; import { IProtocolVersions, ProtocolVersion } from "src/L1/interfaces/IProtocolVersions.sol"; import { DeploySuperchainInput, DeploySuperchain, DeploySuperchainOutput } from "scripts/deploy/DeploySuperchain.s.sol"; @@ -60,6 +61,8 @@ contract DeploySuperchainOutput_Test is Test { SuperchainConfig superchainConfigProxy = SuperchainConfig(makeAddr("superchainConfigProxy")); IProtocolVersions protocolVersionsImpl = IProtocolVersions(makeAddr("protocolVersionsImpl")); IProtocolVersions protocolVersionsProxy = IProtocolVersions(makeAddr("protocolVersionsProxy")); + SharedLockbox sharedLockboxImpl = SharedLockbox(makeAddr("sharedLockboxImpl")); + SharedLockbox sharedLockboxProxy = SharedLockbox(makeAddr("sharedLockboxProxy")); // Ensure each address has code, since these are expected to be contracts. vm.etch(address(superchainProxyAdmin), hex"01"); @@ -67,6 +70,8 @@ contract DeploySuperchainOutput_Test is Test { vm.etch(address(superchainConfigProxy), hex"01"); vm.etch(address(protocolVersionsImpl), hex"01"); vm.etch(address(protocolVersionsProxy), hex"01"); + vm.etch(address(sharedLockboxImpl), hex"01"); + vm.etch(address(sharedLockboxProxy), hex"01"); // Set the output data. dso.set(dso.superchainProxyAdmin.selector, address(superchainProxyAdmin)); @@ -74,6 +79,8 @@ contract DeploySuperchainOutput_Test is Test { dso.set(dso.superchainConfigProxy.selector, address(superchainConfigProxy)); dso.set(dso.protocolVersionsImpl.selector, address(protocolVersionsImpl)); dso.set(dso.protocolVersionsProxy.selector, address(protocolVersionsProxy)); + dso.set(dso.sharedLockboxImpl.selector, address(sharedLockboxImpl)); + dso.set(dso.sharedLockboxProxy.selector, address(sharedLockboxProxy)); // Compare the test data to the getter methods. assertEq(address(superchainProxyAdmin), address(dso.superchainProxyAdmin()), "100"); @@ -81,6 +88,8 @@ contract DeploySuperchainOutput_Test is Test { assertEq(address(superchainConfigProxy), address(dso.superchainConfigProxy()), "300"); assertEq(address(protocolVersionsImpl), address(dso.protocolVersionsImpl()), "400"); assertEq(address(protocolVersionsProxy), address(dso.protocolVersionsProxy()), "500"); + assertEq(address(sharedLockboxImpl), address(dso.sharedLockboxImpl()), "600"); + assertEq(address(sharedLockboxProxy), address(dso.sharedLockboxProxy()), "700"); } function test_getters_whenNotSet_reverts() public { @@ -95,6 +104,12 @@ contract DeploySuperchainOutput_Test is Test { vm.expectRevert("DeployUtils: zero address"); dso.protocolVersionsProxy(); + + vm.expectRevert("DeployUtils: zero address"); + dso.sharedLockboxImpl(); + + vm.expectRevert("DeployUtils: zero address"); + dso.sharedLockboxProxy(); } function test_getters_whenAddrHasNoCode_reverts() public { @@ -116,6 +131,14 @@ contract DeploySuperchainOutput_Test is Test { dso.set(dso.protocolVersionsProxy.selector, emptyAddr); vm.expectRevert(expectedErr); dso.protocolVersionsProxy(); + + dso.set(dso.sharedLockboxImpl.selector, emptyAddr); + vm.expectRevert(expectedErr); + dso.sharedLockboxImpl(); + + dso.set(dso.sharedLockboxProxy.selector, emptyAddr); + vm.expectRevert(expectedErr); + dso.sharedLockboxProxy(); } } @@ -176,17 +199,20 @@ contract DeploySuperchain_Test is Test { assertEq(dso.superchainConfigProxy().paused(), paused, "400"); assertEq(unwrap(dso.protocolVersionsProxy().required()), unwrap(requiredProtocolVersion), "500"); assertEq(unwrap(dso.protocolVersionsProxy().recommended()), unwrap(recommendedProtocolVersion), "600"); + assertEq(address(dso.sharedLockboxProxy().SUPERCHAIN_CONFIG()), address(dso.superchainConfigProxy()), "700"); // Architecture assertions. // We prank as the zero address due to the Proxy's `proxyCallIfNotAdmin` modifier. Proxy superchainConfigProxy = Proxy(payable(address(dso.superchainConfigProxy()))); Proxy protocolVersionsProxy = Proxy(payable(address(dso.protocolVersionsProxy()))); + Proxy sharedLockboxProxy = Proxy(payable(address(dso.sharedLockboxProxy()))); vm.startPrank(address(0)); assertEq(superchainConfigProxy.implementation(), address(dso.superchainConfigImpl()), "700"); assertEq(protocolVersionsProxy.implementation(), address(dso.protocolVersionsImpl()), "800"); assertEq(superchainConfigProxy.admin(), protocolVersionsProxy.admin(), "900"); assertEq(superchainConfigProxy.admin(), address(dso.superchainProxyAdmin()), "1000"); + assertEq(sharedLockboxProxy.implementation(), address(dso.sharedLockboxImpl()), "1100"); vm.stopPrank(); // Ensure that `checkOutput` passes. This is called by the `run` function during execution, diff --git a/packages/contracts-bedrock/test/setup/CommonTest.sol b/packages/contracts-bedrock/test/setup/CommonTest.sol index 93ede20b629e..da071530233f 100644 --- a/packages/contracts-bedrock/test/setup/CommonTest.sol +++ b/packages/contracts-bedrock/test/setup/CommonTest.sol @@ -76,6 +76,12 @@ contract CommonTest is Test, Setup, Events { // Deploy L2 Setup.L2(); + // Authorize portals to interact with the SharedLockbox. + vm.startPrank(address(superchainConfig)); + sharedLockbox.authorizePortal(address(optimismPortal)); + sharedLockbox.authorizePortal(address(optimismPortal2)); + vm.stopPrank(); + // Call bridge initializer setup function bridgeInitializerSetUp(); } diff --git a/packages/contracts-bedrock/test/setup/Setup.sol b/packages/contracts-bedrock/test/setup/Setup.sol index ef2b654b2410..3ee91b8bb078 100644 --- a/packages/contracts-bedrock/test/setup/Setup.sol +++ b/packages/contracts-bedrock/test/setup/Setup.sol @@ -23,6 +23,7 @@ import { IL1CrossDomainMessenger } from "src/L1/interfaces/IL1CrossDomainMesseng import { IL2OutputOracle } from "src/L1/interfaces/IL2OutputOracle.sol"; import { ISystemConfig } from "src/L1/interfaces/ISystemConfig.sol"; import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; +import { ISharedLockbox } from "src/L1/interfaces/ISharedLockbox.sol"; import { IDataAvailabilityChallenge } from "src/L1/interfaces/IDataAvailabilityChallenge.sol"; import { IL1StandardBridge } from "src/L1/interfaces/IL1StandardBridge.sol"; import { IProtocolVersions } from "src/L1/interfaces/IProtocolVersions.sol"; @@ -87,6 +88,7 @@ contract Setup { IProtocolVersions protocolVersions; ISuperchainConfig superchainConfig; IDataAvailabilityChallenge dataAvailabilityChallenge; + ISharedLockbox sharedLockbox; // L2 contracts IL2CrossDomainMessenger l2CrossDomainMessenger = @@ -158,6 +160,7 @@ contract Setup { protocolVersions = IProtocolVersions(deploy.mustGetAddress("ProtocolVersionsProxy")); superchainConfig = ISuperchainConfig(deploy.mustGetAddress("SuperchainConfigProxy")); anchorStateRegistry = IAnchorStateRegistry(deploy.mustGetAddress("AnchorStateRegistryProxy")); + sharedLockbox = ISharedLockbox(deploy.mustGetAddress("SharedLockboxProxy")); vm.label(address(optimismPortal), "OptimismPortal"); vm.label(deploy.mustGetAddress("OptimismPortalProxy"), "OptimismPortalProxy"); @@ -180,6 +183,8 @@ contract Setup { vm.label(deploy.mustGetAddress("ProtocolVersionsProxy"), "ProtocolVersionsProxy"); vm.label(address(superchainConfig), "SuperchainConfig"); vm.label(deploy.mustGetAddress("SuperchainConfigProxy"), "SuperchainConfigProxy"); + vm.label(address(sharedLockbox), "SharedLockbox"); + vm.label(deploy.mustGetAddress("SharedLockboxProxy"), "SharedLockboxProxy"); vm.label(AddressAliasHelper.applyL1ToL2Alias(address(l1CrossDomainMessenger)), "L1CrossDomainMessenger_aliased"); if (!deploy.cfg().useFaultProofs()) { diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index 49e35e835f36..ed143e2e8265 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -43,7 +43,9 @@ contract Specification_Test is CommonTest { DELAYEDWETHOWNER, COUNCILSAFE, COUNCILSAFEOWNER, - DEPENDENCYMANAGER + DEPENDENCYMANAGER, + PORTAL, + SUPERCHAINCONFIG } /// @notice Represents the specification of a function. @@ -334,6 +336,7 @@ contract Specification_Test is CommonTest { _sel: IOptimismPortalInterop.setConfig.selector, _auth: Role.SYSTEMCONFIGOWNER }); + _addSpec({ _name: "OptimismPortalInterop", _sel: _getSel("sharedLockbox()") }); // OptimismPortal2 _addSpec({ _name: "OptimismPortal2", _sel: _getSel("depositTransaction(address,uint256,uint64,bool,bytes)") }); @@ -381,6 +384,7 @@ contract Specification_Test is CommonTest { _sel: _getSel("depositERC20Transaction(address,uint256,uint256,uint64,bool,bytes)") }); _addSpec({ _name: "OptimismPortal2", _sel: _getSel("setGasPayingToken(address,uint8,bytes32,bytes32)") }); + _addSpec({ _name: "OptimismPortal2", _sel: _getSel("sharedLockbox()") }); // ProtocolVersions _addSpec({ _name: "ProtocolVersions", _sel: _getSel("RECOMMENDED_SLOT()") }); @@ -417,6 +421,14 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "SuperchainConfig", _sel: _getSel("unpause()"), _auth: Role.GUARDIAN }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("version()") }); + // SharedLockbox + _addSpec({ _name: "SharedLockbox", _sel: _getSel("SUPERCHAIN_CONFIG()") }); + _addSpec({ _name: "SharedLockbox", _sel: _getSel("authorizedPortals(address)") }); + _addSpec({ _name: "SharedLockbox", _sel: _getSel("lockETH()"), _auth: Role.PORTAL }); + _addSpec({ _name: "SharedLockbox", _sel: _getSel("unlockETH(uint256)"), _auth: Role.PORTAL }); + _addSpec({ _name: "SharedLockbox", _sel: _getSel("authorizePortal(address)"), _auth: Role.SUPERCHAINCONFIG }); + _addSpec({ _name: "SharedLockbox", _sel: _getSel("version()") }); + // SystemConfig _addSpec({ _name: "SystemConfig", _sel: _getSel("UNSAFE_BLOCK_SIGNER_SLOT()") }); _addSpec({ _name: "SystemConfig", _sel: _getSel("START_BLOCK_SLOT()") }); From 94769899f93c7b2894917630f0d82c9bce475642 Mon Sep 17 00:00:00 2001 From: Disco <131301107+0xDiscotech@users.noreply.github.com> Date: Wed, 27 Nov 2024 17:54:32 -0300 Subject: [PATCH 03/13] feat: add liquidity migrator contract with its unit test and interface (#128) * feat: create shared lockbox contract with its interface and unit tests * chore: polish tests and interfaces * chore: run pre-pr * chore: improve natspec * chore: run pre-pr * feat: add liqudity migrator contract with its unit test and interface * chore: remove underscore on stack var * chore: add todo * chore: run pre-pr * chore: add contract title natspec and proxied * refactor: integrate testing suite with common test * chore: pre-pr * chore: add spec test --- .../snapshots/abi/LiquidityMigrator.json | 33 ++++++++++++++++ .../snapshots/semver-lock.json | 2 +- .../storageLayout/LiquidityMigrator.json | 1 + .../src/L1/LiquidityMigrator.sol | 31 +++++++++++++++ .../src/L1/SharedLockbox.sol | 6 ++- .../src/L1/interfaces/ILiquidityMigrator.sol | 12 ++++++ .../test/L1/LiquidityMigrator.t.sol | 39 +++++++++++++++++++ .../test/universal/Specs.t.sol | 3 ++ 8 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 packages/contracts-bedrock/snapshots/abi/LiquidityMigrator.json create mode 100644 packages/contracts-bedrock/snapshots/storageLayout/LiquidityMigrator.json create mode 100644 packages/contracts-bedrock/src/L1/LiquidityMigrator.sol create mode 100644 packages/contracts-bedrock/src/L1/interfaces/ILiquidityMigrator.sol create mode 100644 packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol diff --git a/packages/contracts-bedrock/snapshots/abi/LiquidityMigrator.json b/packages/contracts-bedrock/snapshots/abi/LiquidityMigrator.json new file mode 100644 index 000000000000..a96a61f9ea48 --- /dev/null +++ b/packages/contracts-bedrock/snapshots/abi/LiquidityMigrator.json @@ -0,0 +1,33 @@ +[ + { + "inputs": [ + { + "internalType": "address", + "name": "_sharedLockbox", + "type": "address" + } + ], + "stateMutability": "nonpayable", + "type": "constructor" + }, + { + "inputs": [], + "name": "migrateETH", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "ETHMigrated", + "type": "event" + } +] \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index e8cdf3df5e46..c816ede82195 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -41,7 +41,7 @@ }, "src/L1/SharedLockbox.sol": { "initCodeHash": "0x284eaf282bfbe4403ec5c692371ad2876ec872851b384e5779e6eae577f1dfbb", - "sourceCodeHash": "0x85f55d85d3fed2e5b7ddcd48c5ab8c166abc26e33f38c567baa5f7e31109366a" + "sourceCodeHash": "0x60c8a4ce8f916237ea8af9580f011840f5de3393eee9e53c3c89761e09395e91" }, "src/L1/SuperchainConfig.sol": { "initCodeHash": "0xfca12d9016c746e5c275b186e0ca40cfd65cf45a5665aab7589a669fea3abb47", diff --git a/packages/contracts-bedrock/snapshots/storageLayout/LiquidityMigrator.json b/packages/contracts-bedrock/snapshots/storageLayout/LiquidityMigrator.json new file mode 100644 index 000000000000..0637a088a01e --- /dev/null +++ b/packages/contracts-bedrock/snapshots/storageLayout/LiquidityMigrator.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol b/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol new file mode 100644 index 000000000000..f3b8060996af --- /dev/null +++ b/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import { ISharedLockbox } from "./interfaces/ISharedLockbox.sol"; + +/// @custom:proxied true +/// @title LiquidityMigrator +/// @notice A contract to migrate the OptimisPortal's ETH balance to the SharedLockbox. One-time use logic, executed in +/// a batch of transactions to enable the SharedLockbox interaction within the OptimismPortal. +contract LiquidityMigrator { + /// @notice Emitted when the contract's ETH balance is migrated to the SharedLockbox. + /// @param amount The amount corresponding to the contract's ETH balance migrated. + event ETHMigrated(uint256 amount); + + /// @notice The SharedLockbox contract. + ISharedLockbox internal immutable SHARED_LOCKBOX; + + /// @notice Constructs the LiquidityMigrator contract. + /// @param _sharedLockbox The address of the SharedLockbox contract. + constructor(address _sharedLockbox) { + SHARED_LOCKBOX = ISharedLockbox(_sharedLockbox); + } + + /// @notice Migrates the contract's whole ETH balance to the SharedLockbox. + /// One-time use logic upgraded over OptimismPortalProxy address and then deprecated by another approval. + function migrateETH() external { + uint256 balance = address(this).balance; + SHARED_LOCKBOX.lockETH{ value: balance }(); + emit ETHMigrated(balance); + } +} diff --git a/packages/contracts-bedrock/src/L1/SharedLockbox.sol b/packages/contracts-bedrock/src/L1/SharedLockbox.sol index eb53b8fcffb4..3a4c6bc2c570 100644 --- a/packages/contracts-bedrock/src/L1/SharedLockbox.sol +++ b/packages/contracts-bedrock/src/L1/SharedLockbox.sol @@ -5,6 +5,10 @@ import { ISemver } from "src/universal/interfaces/ISemver.sol"; import { IOptimismPortal } from "src/L1/interfaces/IOptimismPortal.sol"; import { Unauthorized } from "src/libraries/errors/CommonErrors.sol"; +/// @custom:proxied true +/// @title SharedLockbox +/// @notice Manages ETH liquidity locking and unlocking for authorized OptimismPortals, enabling unified ETH liquidity +/// management across chains in the superchain cluster. contract SharedLockbox is ISemver { /// @notice Emitted when ETH is locked in the lockbox by an authorized portal. /// @param portal The address of the portal that locked the ETH. @@ -23,7 +27,7 @@ contract SharedLockbox is ISemver { /// @notice The address of the SuperchainConfig contract. address public immutable SUPERCHAIN_CONFIG; - /// @notice OptimismPortals that are part of the dependency cluster authorized to interact with the SharedLockbox. + /// @notice OptimismPortals that are part of the dependency cluster authorized to interact with the SharedLockbox mapping(address => bool) public authorizedPortals; /// @notice Semantic version. diff --git a/packages/contracts-bedrock/src/L1/interfaces/ILiquidityMigrator.sol b/packages/contracts-bedrock/src/L1/interfaces/ILiquidityMigrator.sol new file mode 100644 index 000000000000..5c3cd7b03a92 --- /dev/null +++ b/packages/contracts-bedrock/src/L1/interfaces/ILiquidityMigrator.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +/// @title ILiquidityMigrator +/// @notice Interface for the LiquidityMigrator contract +interface ILiquidityMigrator { + event ETHMigrated(uint256 amount); + + function __constructor__(address _sharedLockbox) external; + + function migrateETH() external; +} diff --git a/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol b/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol new file mode 100644 index 000000000000..a06aab1e11f1 --- /dev/null +++ b/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import { CommonTest } from "test/setup/CommonTest.sol"; +import { LiquidityMigrator } from "src/L1/LiquidityMigrator.sol"; + +contract LiquidityMigratorTest is CommonTest { + event ETHMigrated(uint256 amount); + + LiquidityMigrator public migrator; + + function setUp() public virtual override { + super.setUp(); + migrator = new LiquidityMigrator(address(sharedLockbox)); + } + + /// @notice Tests the migration of the contract's ETH balance to the SharedLockbox works properly. + function test_migrateETH_succeeds(uint256 _ethAmount) public { + vm.deal(address(migrator), _ethAmount); + + // Get the balance of the migrator before the migration to compare later on the assertions + uint256 _migratorEthBalance = address(migrator).balance; + uint256 _lockboxBalanceBefore = address(sharedLockbox).balance; + + // Look for the emit of the `ETHMigrated` event + emit ETHMigrated(_migratorEthBalance); + + // Set the migrator as an authorized portal so it can lock the ETH while migrating + vm.prank(address(superchainConfig)); + sharedLockbox.authorizePortal(address(migrator)); + + // Call the `migrateETH` function with the amount + migrator.migrateETH(); + + // Assert the balances after the migration happened + assert(address(migrator).balance == 0); + assert(address(sharedLockbox).balance == _lockboxBalanceBefore + _migratorEthBalance); + } +} diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index ed143e2e8265..1cc0691c3414 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -918,6 +918,9 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "LivenessModule", _sel: _getSel("safe()") }); _addSpec({ _name: "LivenessModule", _sel: _getSel("thresholdPercentage()") }); _addSpec({ _name: "LivenessModule", _sel: _getSel("version()") }); + + // LiquidityMigrator + _addSpec({ _name: "LiquidityMigrator", _sel: _getSel("migrateETH()") }); } /// @dev Computes the selector from a function signature. From b240094a67c7027abcdba2e84f0c02a300b6b4e8 Mon Sep 17 00:00:00 2001 From: AgusDuha <81362284+agusduha@users.noreply.github.com> Date: Wed, 27 Nov 2024 18:04:38 -0300 Subject: [PATCH 04/13] feat: integrate system config with superchain config (#140) * feat: integrate portal to lockbox * fix: pr fixes * test: refactor assert * feat: integrate system config with superchain config * fix: remove OPCM interop * test: add dependency counter test --- .../scripts/checks/interfaces/main.go | 2 +- .../scripts/deploy/ChainAssertions.sol | 13 +- .../scripts/deploy/Deploy.s.sol | 8 +- .../deploy/DeployImplementations.s.sol | 53 +- .../abi/OPContractsManagerInterop.json | 671 ------------------ .../snapshots/abi/SystemConfigInterop.json | 157 +--- .../snapshots/semver-lock.json | 4 +- .../OPContractsManagerInterop.json | 30 - .../src/L1/OPContractsManagerInterop.sol | 59 -- .../src/L1/SystemConfigInterop.sol | 83 +-- .../L1/interfaces/ISystemConfigInterop.sol | 8 +- .../test/L1/SystemConfigInterop.t.sol | 50 +- .../test/universal/Specs.t.sol | 21 +- .../test/vendor/Initializable.t.sol | 1 - 14 files changed, 125 insertions(+), 1035 deletions(-) delete mode 100644 packages/contracts-bedrock/snapshots/abi/OPContractsManagerInterop.json delete mode 100644 packages/contracts-bedrock/snapshots/storageLayout/OPContractsManagerInterop.json delete mode 100644 packages/contracts-bedrock/src/L1/OPContractsManagerInterop.sol diff --git a/packages/contracts-bedrock/scripts/checks/interfaces/main.go b/packages/contracts-bedrock/scripts/checks/interfaces/main.go index 8bdbf79514e2..c270d0ec988b 100644 --- a/packages/contracts-bedrock/scripts/checks/interfaces/main.go +++ b/packages/contracts-bedrock/scripts/checks/interfaces/main.go @@ -28,7 +28,7 @@ var excludeContracts = []string{ // TODO: Interfaces that need to be fixed "IInitializable", "IOptimismMintableERC20", "ILegacyMintableERC20", - "KontrolCheatsBase", "ISystemConfigInterop", "IResolvedDelegateProxy", + "KontrolCheatsBase", "IResolvedDelegateProxy", } type ContractDefinition struct { diff --git a/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol b/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol index 1cb2599c9bce..d44705520c22 100644 --- a/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol +++ b/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol @@ -140,6 +140,7 @@ library ChainAssertions { /// @notice Asserts that the SystemConfigInterop is setup correctly function checkSystemConfigInterop( Types.ContractSet memory _contracts, + Types.ContractSet memory _proxies, DeployConfig _cfg, bool _isProxy ) @@ -147,6 +148,8 @@ library ChainAssertions { view { ISystemConfigInterop config = ISystemConfigInterop(_contracts.SystemConfig); + ISuperchainConfig superchainConfig = ISuperchainConfig(_proxies.SuperchainConfig); + console.log( "Running chain assertions on the SystemConfigInterop %s at %s", _isProxy ? "proxy" : "implementation", @@ -154,13 +157,9 @@ library ChainAssertions { ); checkSystemConfig(_contracts, _cfg, _isProxy); - if (_isProxy) { - // TODO: this is not being set in the deployment, nor is a config value. - // Update this when it has an entry in hardhat.json - require(config.dependencyManager() == address(0), "CHECK-SCFGI-10"); - } else { - require(config.dependencyManager() == address(0), "CHECK-SCFGI-20"); - } + + require(config.dependencyCounter() == 0, "CHECK-SCFGI-10"); + require(config.SUPERCHAIN_CONFIG() == address(superchainConfig), "CHECK-SCFGI-20"); } /// @notice Asserts that the L1CrossDomainMessenger is setup correctly diff --git a/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol b/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol index 15951c2d0980..cd1a4f866814 100644 --- a/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol @@ -451,7 +451,13 @@ contract Deploy is Deployer { _oracle: IPreimageOracle(address(dio.preimageOracleSingleton())) }); if (_isInterop) { - ChainAssertions.checkSystemConfigInterop({ _contracts: contracts, _cfg: cfg, _isProxy: false }); + Types.ContractSet memory proxies = _proxies(); + ChainAssertions.checkSystemConfigInterop({ + _contracts: contracts, + _proxies: proxies, + _cfg: cfg, + _isProxy: false + }); } else { ChainAssertions.checkSystemConfig({ _contracts: contracts, _cfg: cfg, _isProxy: false }); } diff --git a/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol index bc21753e5371..c0ca38b6c5d6 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol @@ -27,7 +27,6 @@ import { IL1ERC721Bridge } from "src/L1/interfaces/IL1ERC721Bridge.sol"; import { IL1StandardBridge } from "src/L1/interfaces/IL1StandardBridge.sol"; import { IOptimismMintableERC20Factory } from "src/universal/interfaces/IOptimismMintableERC20Factory.sol"; -import { OPContractsManagerInterop } from "src/L1/OPContractsManagerInterop.sol"; import { IOptimismPortalInterop } from "src/L1/interfaces/IOptimismPortalInterop.sol"; import { ISystemConfigInterop } from "src/L1/interfaces/ISystemConfigInterop.sol"; @@ -926,22 +925,16 @@ contract DeployImplementations is Script { // - `OptimismPortalInterop is OptimismPortal`: A different portal implementation is used, and // it's ABI is the same. // - `SystemConfigInterop is SystemConfig`: A different system config implementation is used, and -// it's initializer has a different signature. This signature is different because there is a -// new input parameter, the `dependencyManager`. -// - Because of the different system config initializer, there is a new input parameter (dependencyManager). +// it's constructor has a different signature. This signature is different because there is a +// new input parameter, the `superchainConfig`. +// - Because of the different system config constructor, there is a new input parameter (superchainConfig). // // Similar to how inheritance was used to develop the new portal and system config contracts, we use // inheritance to modify up to all of the deployer contracts. For this interop example, what this // means is we need: -// - An `OPContractsManagerInterop is OPContractsManager` that knows how to encode the calldata for the -// new system config initializer. // - A `DeployImplementationsInterop is DeployImplementations` that: // - Deploys OptimismPortalInterop instead of OptimismPortal. // - Deploys SystemConfigInterop instead of SystemConfig. -// - Deploys OPContractsManagerInterop instead of OPContractsManager, which contains the updated logic -// for encoding the SystemConfig initializer. -// - Updates the OPCM release setter logic to use the updated initializer. -// - A `DeployOPChainInterop is DeployOPChain` that allows the updated input parameter to be passed. // // Most of the complexity in the above flow comes from the the new input for the updated SystemConfig // initializer. If all function signatures were the same, all we'd have to change is the contract @@ -949,41 +942,6 @@ contract DeployImplementations is Script { // resolve https://github.com/ethereum-optimism/optimism/issues/11783, we just assume this new role // is the same as the proxy admin owner. contract DeployImplementationsInterop is DeployImplementations { - function createOPCMContract( - DeployImplementationsInput _dii, - DeployImplementationsOutput _dio, - OPContractsManager.Blueprints memory _blueprints, - string memory _l1ContractsRelease - ) - internal - virtual - override - returns (OPContractsManager opcm_) - { - ISuperchainConfig superchainConfigProxy = _dii.superchainConfigProxy(); - IProtocolVersions protocolVersionsProxy = _dii.protocolVersionsProxy(); - - OPContractsManager.Implementations memory implementations = OPContractsManager.Implementations({ - l1ERC721BridgeImpl: address(_dio.l1ERC721BridgeImpl()), - optimismPortalImpl: address(_dio.optimismPortalImpl()), - systemConfigImpl: address(_dio.systemConfigImpl()), - optimismMintableERC20FactoryImpl: address(_dio.optimismMintableERC20FactoryImpl()), - l1CrossDomainMessengerImpl: address(_dio.l1CrossDomainMessengerImpl()), - l1StandardBridgeImpl: address(_dio.l1StandardBridgeImpl()), - disputeGameFactoryImpl: address(_dio.disputeGameFactoryImpl()), - delayedWETHImpl: address(_dio.delayedWETHImpl()), - mipsImpl: address(_dio.mipsSingleton()) - }); - - vm.broadcast(msg.sender); - opcm_ = new OPContractsManagerInterop( - superchainConfigProxy, protocolVersionsProxy, _l1ContractsRelease, _blueprints, implementations - ); - - vm.label(address(opcm_), "OPContractsManager"); - _dio.set(_dio.opcm.selector, address(opcm_)); - } - function deployOptimismPortalImpl( DeployImplementationsInput _dii, DeployImplementationsOutput _dio @@ -1038,11 +996,14 @@ contract DeployImplementationsInterop is DeployImplementations { if (existingImplementation != address(0)) { impl = ISystemConfigInterop(existingImplementation); } else { + address superchainConfig = address(_dii.superchainConfigProxy()); vm.broadcast(msg.sender); impl = ISystemConfigInterop( DeployUtils.create1({ _name: "SystemConfigInterop", - _args: DeployUtils.encodeConstructor(abi.encodeCall(ISystemConfigInterop.__constructor__, ())) + _args: DeployUtils.encodeConstructor( + abi.encodeCall(ISystemConfigInterop.__constructor__, (superchainConfig)) + ) }) ); } diff --git a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerInterop.json b/packages/contracts-bedrock/snapshots/abi/OPContractsManagerInterop.json deleted file mode 100644 index b5758eca610f..000000000000 --- a/packages/contracts-bedrock/snapshots/abi/OPContractsManagerInterop.json +++ /dev/null @@ -1,671 +0,0 @@ -[ - { - "inputs": [ - { - "internalType": "contract ISuperchainConfig", - "name": "_superchainConfig", - "type": "address" - }, - { - "internalType": "contract IProtocolVersions", - "name": "_protocolVersions", - "type": "address" - }, - { - "internalType": "string", - "name": "_l1ContractsRelease", - "type": "string" - }, - { - "components": [ - { - "internalType": "address", - "name": "addressManager", - "type": "address" - }, - { - "internalType": "address", - "name": "proxy", - "type": "address" - }, - { - "internalType": "address", - "name": "proxyAdmin", - "type": "address" - }, - { - "internalType": "address", - "name": "l1ChugSplashProxy", - "type": "address" - }, - { - "internalType": "address", - "name": "resolvedDelegateProxy", - "type": "address" - }, - { - "internalType": "address", - "name": "anchorStateRegistry", - "type": "address" - }, - { - "internalType": "address", - "name": "permissionedDisputeGame1", - "type": "address" - }, - { - "internalType": "address", - "name": "permissionedDisputeGame2", - "type": "address" - } - ], - "internalType": "struct OPContractsManager.Blueprints", - "name": "_blueprints", - "type": "tuple" - }, - { - "components": [ - { - "internalType": "address", - "name": "l1ERC721BridgeImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "optimismPortalImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "systemConfigImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "optimismMintableERC20FactoryImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "l1CrossDomainMessengerImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "l1StandardBridgeImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "disputeGameFactoryImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "delayedWETHImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "mipsImpl", - "type": "address" - } - ], - "internalType": "struct OPContractsManager.Implementations", - "name": "_implementations", - "type": "tuple" - } - ], - "stateMutability": "nonpayable", - "type": "constructor" - }, - { - "inputs": [], - "name": "OUTPUT_VERSION", - "outputs": [ - { - "internalType": "uint256", - "name": "", - "type": "uint256" - } - ], - "stateMutability": "view", - "type": "function" - }, - { - "inputs": [], - "name": "blueprints", - "outputs": [ - { - "components": [ - { - "internalType": "address", - "name": "addressManager", - "type": "address" - }, - { - "internalType": "address", - "name": "proxy", - "type": "address" - }, - { - "internalType": "address", - "name": "proxyAdmin", - "type": "address" - }, - { - "internalType": "address", - "name": "l1ChugSplashProxy", - "type": "address" - }, - { - "internalType": "address", - "name": "resolvedDelegateProxy", - "type": "address" - }, - { - "internalType": "address", - "name": "anchorStateRegistry", - "type": "address" - }, - { - "internalType": "address", - "name": "permissionedDisputeGame1", - "type": "address" - }, - { - "internalType": "address", - "name": "permissionedDisputeGame2", - "type": "address" - } - ], - "internalType": "struct OPContractsManager.Blueprints", - "name": "", - "type": "tuple" - } - ], - "stateMutability": "view", - "type": "function" - }, - { - "inputs": [ - { - "internalType": "uint256", - "name": "_l2ChainId", - "type": "uint256" - } - ], - "name": "chainIdToBatchInboxAddress", - "outputs": [ - { - "internalType": "address", - "name": "", - "type": "address" - } - ], - "stateMutability": "pure", - "type": "function" - }, - { - "inputs": [ - { - "components": [ - { - "components": [ - { - "internalType": "address", - "name": "opChainProxyAdminOwner", - "type": "address" - }, - { - "internalType": "address", - "name": "systemConfigOwner", - "type": "address" - }, - { - "internalType": "address", - "name": "batcher", - "type": "address" - }, - { - "internalType": "address", - "name": "unsafeBlockSigner", - "type": "address" - }, - { - "internalType": "address", - "name": "proposer", - "type": "address" - }, - { - "internalType": "address", - "name": "challenger", - "type": "address" - } - ], - "internalType": "struct OPContractsManager.Roles", - "name": "roles", - "type": "tuple" - }, - { - "internalType": "uint32", - "name": "basefeeScalar", - "type": "uint32" - }, - { - "internalType": "uint32", - "name": "blobBasefeeScalar", - "type": "uint32" - }, - { - "internalType": "uint256", - "name": "l2ChainId", - "type": "uint256" - }, - { - "internalType": "bytes", - "name": "startingAnchorRoots", - "type": "bytes" - }, - { - "internalType": "string", - "name": "saltMixer", - "type": "string" - }, - { - "internalType": "uint64", - "name": "gasLimit", - "type": "uint64" - }, - { - "internalType": "GameType", - "name": "disputeGameType", - "type": "uint32" - }, - { - "internalType": "Claim", - "name": "disputeAbsolutePrestate", - "type": "bytes32" - }, - { - "internalType": "uint256", - "name": "disputeMaxGameDepth", - "type": "uint256" - }, - { - "internalType": "uint256", - "name": "disputeSplitDepth", - "type": "uint256" - }, - { - "internalType": "Duration", - "name": "disputeClockExtension", - "type": "uint64" - }, - { - "internalType": "Duration", - "name": "disputeMaxClockDuration", - "type": "uint64" - } - ], - "internalType": "struct OPContractsManager.DeployInput", - "name": "_input", - "type": "tuple" - } - ], - "name": "deploy", - "outputs": [ - { - "components": [ - { - "internalType": "contract IProxyAdmin", - "name": "opChainProxyAdmin", - "type": "address" - }, - { - "internalType": "contract IAddressManager", - "name": "addressManager", - "type": "address" - }, - { - "internalType": "contract IL1ERC721Bridge", - "name": "l1ERC721BridgeProxy", - "type": "address" - }, - { - "internalType": "contract ISystemConfig", - "name": "systemConfigProxy", - "type": "address" - }, - { - "internalType": "contract IOptimismMintableERC20Factory", - "name": "optimismMintableERC20FactoryProxy", - "type": "address" - }, - { - "internalType": "contract IL1StandardBridge", - "name": "l1StandardBridgeProxy", - "type": "address" - }, - { - "internalType": "contract IL1CrossDomainMessenger", - "name": "l1CrossDomainMessengerProxy", - "type": "address" - }, - { - "internalType": "contract IOptimismPortal2", - "name": "optimismPortalProxy", - "type": "address" - }, - { - "internalType": "contract IDisputeGameFactory", - "name": "disputeGameFactoryProxy", - "type": "address" - }, - { - "internalType": "contract IAnchorStateRegistry", - "name": "anchorStateRegistryProxy", - "type": "address" - }, - { - "internalType": "contract IAnchorStateRegistry", - "name": "anchorStateRegistryImpl", - "type": "address" - }, - { - "internalType": "contract IFaultDisputeGame", - "name": "faultDisputeGame", - "type": "address" - }, - { - "internalType": "contract IPermissionedDisputeGame", - "name": "permissionedDisputeGame", - "type": "address" - }, - { - "internalType": "contract IDelayedWETH", - "name": "delayedWETHPermissionedGameProxy", - "type": "address" - }, - { - "internalType": "contract IDelayedWETH", - "name": "delayedWETHPermissionlessGameProxy", - "type": "address" - } - ], - "internalType": "struct OPContractsManager.DeployOutput", - "name": "", - "type": "tuple" - } - ], - "stateMutability": "nonpayable", - "type": "function" - }, - { - "inputs": [], - "name": "implementations", - "outputs": [ - { - "components": [ - { - "internalType": "address", - "name": "l1ERC721BridgeImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "optimismPortalImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "systemConfigImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "optimismMintableERC20FactoryImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "l1CrossDomainMessengerImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "l1StandardBridgeImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "disputeGameFactoryImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "delayedWETHImpl", - "type": "address" - }, - { - "internalType": "address", - "name": "mipsImpl", - "type": "address" - } - ], - "internalType": "struct OPContractsManager.Implementations", - "name": "", - "type": "tuple" - } - ], - "stateMutability": "view", - "type": "function" - }, - { - "inputs": [], - "name": "l1ContractsRelease", - "outputs": [ - { - "internalType": "string", - "name": "", - "type": "string" - } - ], - "stateMutability": "view", - "type": "function" - }, - { - "inputs": [], - "name": "protocolVersions", - "outputs": [ - { - "internalType": "contract IProtocolVersions", - "name": "", - "type": "address" - } - ], - "stateMutability": "view", - "type": "function" - }, - { - "inputs": [], - "name": "superchainConfig", - "outputs": [ - { - "internalType": "contract ISuperchainConfig", - "name": "", - "type": "address" - } - ], - "stateMutability": "view", - "type": "function" - }, - { - "inputs": [ - { - "internalType": "uint256", - "name": "", - "type": "uint256" - } - ], - "name": "systemConfigs", - "outputs": [ - { - "internalType": "contract ISystemConfig", - "name": "", - "type": "address" - } - ], - "stateMutability": "view", - "type": "function" - }, - { - "inputs": [], - "name": "version", - "outputs": [ - { - "internalType": "string", - "name": "", - "type": "string" - } - ], - "stateMutability": "view", - "type": "function" - }, - { - "anonymous": false, - "inputs": [ - { - "indexed": true, - "internalType": "uint256", - "name": "outputVersion", - "type": "uint256" - }, - { - "indexed": true, - "internalType": "uint256", - "name": "l2ChainId", - "type": "uint256" - }, - { - "indexed": true, - "internalType": "address", - "name": "deployer", - "type": "address" - }, - { - "indexed": false, - "internalType": "bytes", - "name": "deployOutput", - "type": "bytes" - } - ], - "name": "Deployed", - "type": "event" - }, - { - "inputs": [ - { - "internalType": "address", - "name": "who", - "type": "address" - } - ], - "name": "AddressHasNoCode", - "type": "error" - }, - { - "inputs": [ - { - "internalType": "address", - "name": "who", - "type": "address" - } - ], - "name": "AddressNotFound", - "type": "error" - }, - { - "inputs": [], - "name": "AlreadyReleased", - "type": "error" - }, - { - "inputs": [], - "name": "BytesArrayTooLong", - "type": "error" - }, - { - "inputs": [], - "name": "DeploymentFailed", - "type": "error" - }, - { - "inputs": [], - "name": "EmptyInitcode", - "type": "error" - }, - { - "inputs": [], - "name": "IdentityPrecompileCallFailed", - "type": "error" - }, - { - "inputs": [], - "name": "InvalidChainId", - "type": "error" - }, - { - "inputs": [ - { - "internalType": "string", - "name": "role", - "type": "string" - } - ], - "name": "InvalidRoleAddress", - "type": "error" - }, - { - "inputs": [], - "name": "InvalidStartingAnchorRoots", - "type": "error" - }, - { - "inputs": [], - "name": "LatestReleaseNotSet", - "type": "error" - }, - { - "inputs": [], - "name": "NotABlueprint", - "type": "error" - }, - { - "inputs": [], - "name": "ReservedBitsSet", - "type": "error" - }, - { - "inputs": [ - { - "internalType": "bytes", - "name": "data", - "type": "bytes" - } - ], - "name": "UnexpectedPreambleData", - "type": "error" - }, - { - "inputs": [ - { - "internalType": "uint8", - "name": "version", - "type": "uint8" - } - ], - "name": "UnsupportedERCVersion", - "type": "error" - } -] \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/abi/SystemConfigInterop.json b/packages/contracts-bedrock/snapshots/abi/SystemConfigInterop.json index a459af15801b..c664e9a48d89 100644 --- a/packages/contracts-bedrock/snapshots/abi/SystemConfigInterop.json +++ b/packages/contracts-bedrock/snapshots/abi/SystemConfigInterop.json @@ -1,4 +1,15 @@ [ + { + "inputs": [ + { + "internalType": "address", + "name": "_superchainConfig", + "type": "address" + } + ], + "stateMutability": "nonpayable", + "type": "constructor" + }, { "inputs": [], "name": "BATCH_INBOX_SLOT", @@ -103,6 +114,19 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "SUPERCHAIN_CONFIG", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "UNSAFE_BLOCK_SIGNER_SLOT", @@ -196,12 +220,12 @@ }, { "inputs": [], - "name": "dependencyManager", + "name": "dependencyCounter", "outputs": [ { - "internalType": "address", + "internalType": "uint256", "name": "", - "type": "address" + "type": "uint256" } ], "stateMutability": "view", @@ -377,133 +401,6 @@ "name": "_batchInbox", "type": "address" }, - { - "components": [ - { - "internalType": "address", - "name": "l1CrossDomainMessenger", - "type": "address" - }, - { - "internalType": "address", - "name": "l1ERC721Bridge", - "type": "address" - }, - { - "internalType": "address", - "name": "l1StandardBridge", - "type": "address" - }, - { - "internalType": "address", - "name": "disputeGameFactory", - "type": "address" - }, - { - "internalType": "address", - "name": "optimismPortal", - "type": "address" - }, - { - "internalType": "address", - "name": "optimismMintableERC20Factory", - "type": "address" - }, - { - "internalType": "address", - "name": "gasPayingToken", - "type": "address" - } - ], - "internalType": "struct SystemConfig.Addresses", - "name": "_addresses", - "type": "tuple" - }, - { - "internalType": "address", - "name": "_dependencyManager", - "type": "address" - } - ], - "name": "initialize", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - }, - { - "inputs": [ - { - "internalType": "address", - "name": "_owner", - "type": "address" - }, - { - "internalType": "uint32", - "name": "_basefeeScalar", - "type": "uint32" - }, - { - "internalType": "uint32", - "name": "_blobbasefeeScalar", - "type": "uint32" - }, - { - "internalType": "bytes32", - "name": "_batcherHash", - "type": "bytes32" - }, - { - "internalType": "uint64", - "name": "_gasLimit", - "type": "uint64" - }, - { - "internalType": "address", - "name": "_unsafeBlockSigner", - "type": "address" - }, - { - "components": [ - { - "internalType": "uint32", - "name": "maxResourceLimit", - "type": "uint32" - }, - { - "internalType": "uint8", - "name": "elasticityMultiplier", - "type": "uint8" - }, - { - "internalType": "uint8", - "name": "baseFeeMaxChangeDenominator", - "type": "uint8" - }, - { - "internalType": "uint32", - "name": "minimumBaseFee", - "type": "uint32" - }, - { - "internalType": "uint32", - "name": "systemTxMaxGas", - "type": "uint32" - }, - { - "internalType": "uint128", - "name": "maximumBaseFee", - "type": "uint128" - } - ], - "internalType": "struct IResourceMetering.ResourceConfig", - "name": "_config", - "type": "tuple" - }, - { - "internalType": "address", - "name": "_batchInbox", - "type": "address" - }, { "components": [ { diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index c816ede82195..76385ca6aa6c 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -52,8 +52,8 @@ "sourceCodeHash": "0x6dbbe8716ca8cd2fba3da8dcae0ca0c4b1f3e9dd04220fb24a15666b23300927" }, "src/L1/SystemConfigInterop.sol": { - "initCodeHash": "0x443fd84f8dbc38f03e59a56b99099b5e4b28de3e860a5d16c1a21101745622a4", - "sourceCodeHash": "0x5c2e00cd8939a538eb38580d76e70d27dd1e8e6cd9328e1978468981017736e6" + "initCodeHash": "0x9376acb45613d5042ac316f3dd0fa09e7ea1f7b9e99a879b81a8cc31185ed3c7", + "sourceCodeHash": "0x40d76b65fa9074a6539de9a73f0735badb1fc1fe0a0b1b42829065ba6376b176" }, "src/L2/BaseFeeVault.sol": { "initCodeHash": "0xbf49824cf37e201181484a8a423fcad8f504dc925921a2b28e83398197858dec", diff --git a/packages/contracts-bedrock/snapshots/storageLayout/OPContractsManagerInterop.json b/packages/contracts-bedrock/snapshots/storageLayout/OPContractsManagerInterop.json deleted file mode 100644 index aa8148b34cba..000000000000 --- a/packages/contracts-bedrock/snapshots/storageLayout/OPContractsManagerInterop.json +++ /dev/null @@ -1,30 +0,0 @@ -[ - { - "bytes": "32", - "label": "l1ContractsRelease", - "offset": 0, - "slot": "0", - "type": "string" - }, - { - "bytes": "32", - "label": "systemConfigs", - "offset": 0, - "slot": "1", - "type": "mapping(uint256 => contract ISystemConfig)" - }, - { - "bytes": "256", - "label": "blueprint", - "offset": 0, - "slot": "2", - "type": "struct OPContractsManager.Blueprints" - }, - { - "bytes": "288", - "label": "implementation", - "offset": 0, - "slot": "10", - "type": "struct OPContractsManager.Implementations" - } -] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L1/OPContractsManagerInterop.sol b/packages/contracts-bedrock/src/L1/OPContractsManagerInterop.sol deleted file mode 100644 index 79e0efda9735..000000000000 --- a/packages/contracts-bedrock/src/L1/OPContractsManagerInterop.sol +++ /dev/null @@ -1,59 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.15; - -import { OPContractsManager } from "src/L1/OPContractsManager.sol"; -import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; -import { IProtocolVersions } from "src/L1/interfaces/IProtocolVersions.sol"; -import { IResourceMetering } from "src/L1/interfaces/IResourceMetering.sol"; -import { ISystemConfig } from "src/L1/interfaces/ISystemConfig.sol"; -import { ISystemConfigInterop } from "src/L1/interfaces/ISystemConfigInterop.sol"; - -contract OPContractsManagerInterop is OPContractsManager { - constructor( - ISuperchainConfig _superchainConfig, - IProtocolVersions _protocolVersions, - string memory _l1ContractsRelease, - Blueprints memory _blueprints, - Implementations memory _implementations - ) - OPContractsManager(_superchainConfig, _protocolVersions, _l1ContractsRelease, _blueprints, _implementations) - { } - - // The `SystemConfigInterop` contract has an extra `address _dependencyManager` argument - // that we must account for. - function encodeSystemConfigInitializer( - DeployInput memory _input, - DeployOutput memory _output - ) - internal - view - virtual - override - returns (bytes memory) - { - bytes4 selector = ISystemConfigInterop.initialize.selector; - (IResourceMetering.ResourceConfig memory referenceResourceConfig, ISystemConfig.Addresses memory opChainAddrs) = - defaultSystemConfigParams(selector, _input, _output); - - // TODO For now we assume that the dependency manager is the same as system config owner. - // This is currently undefined since it's not part of the standard config, so we may need - // to update where this value is pulled from in the future. To support a different dependency - // manager in this contract without an invasive change of redefining the `Roles` struct, - // we will make the change described in https://github.com/ethereum-optimism/optimism/issues/11783. - address dependencyManager = address(_input.roles.systemConfigOwner); - - return abi.encodeWithSelector( - selector, - _input.roles.systemConfigOwner, - _input.basefeeScalar, - _input.blobBasefeeScalar, - bytes32(uint256(uint160(_input.roles.batcher))), // batcherHash - _input.gasLimit, - _input.roles.unsafeBlockSigner, - referenceResourceConfig, - chainIdToBatchInboxAddress(_input.l2ChainId), - opChainAddrs, - dependencyManager - ); - } -} diff --git a/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol b/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol index 9e9503fe6236..06ab21ef91d8 100644 --- a/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol +++ b/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol @@ -13,64 +13,29 @@ import { GasPayingToken } from "src/libraries/GasPayingToken.sol"; import { StaticConfig } from "src/libraries/StaticConfig.sol"; import { Storage } from "src/libraries/Storage.sol"; -// Interfaces -import { IResourceMetering } from "src/L1/interfaces/IResourceMetering.sol"; - /// @custom:proxied true /// @title SystemConfigInterop /// @notice The SystemConfig contract is used to manage configuration of an Optimism network. /// All configuration is stored on L1 and picked up by L2 as part of the derviation of /// the L2 chain. contract SystemConfigInterop is SystemConfig { - /// @notice Storage slot where the dependency manager address is stored - /// @dev Equal to bytes32(uint256(keccak256("systemconfig.dependencymanager")) - 1) - bytes32 internal constant DEPENDENCY_MANAGER_SLOT = - 0x1708e077affb93e89be2665fb0fb72581be66f84dc00d25fed755ae911905b1c; + /// @notice Storage slot where the dependency counter is stored + /// @dev Equal to bytes32(uint256(keccak256("systemconfig.dependencyCounter")) - 1) + bytes32 internal constant DEPENDENCY_COUNTER_SLOT = + 0x4ae776ed628fe5cb57f351f2986b1f5aee6fda3f6ec83fa6166e8098945adb19; - /// @notice Initializer. - /// @param _owner Initial owner of the contract. - /// @param _basefeeScalar Initial basefee scalar value. - /// @param _blobbasefeeScalar Initial blobbasefee scalar value. - /// @param _batcherHash Initial batcher hash. - /// @param _gasLimit Initial gas limit. - /// @param _unsafeBlockSigner Initial unsafe block signer address. - /// @param _config Initial ResourceConfig. - /// @param _batchInbox Batch inbox address. An identifier for the op-node to find - /// canonical data. - /// @param _addresses Set of L1 contract addresses. These should be the proxies. - /// @param _dependencyManager The addressed allowed to add/remove from the dependency set - function initialize( - address _owner, - uint32 _basefeeScalar, - uint32 _blobbasefeeScalar, - bytes32 _batcherHash, - uint64 _gasLimit, - address _unsafeBlockSigner, - IResourceMetering.ResourceConfig memory _config, - address _batchInbox, - SystemConfig.Addresses memory _addresses, - address _dependencyManager - ) - external - { - // This method has an initializer modifier, and will revert if already initialized. - initialize({ - _owner: _owner, - _basefeeScalar: _basefeeScalar, - _blobbasefeeScalar: _blobbasefeeScalar, - _batcherHash: _batcherHash, - _gasLimit: _gasLimit, - _unsafeBlockSigner: _unsafeBlockSigner, - _config: _config, - _batchInbox: _batchInbox, - _addresses: _addresses - }); - Storage.setAddress(DEPENDENCY_MANAGER_SLOT, _dependencyManager); - } + /// @notice The address of the SuperchainConfig contract. + address public immutable SUPERCHAIN_CONFIG; - /// @custom:semver +interop-beta.4 + /// @custom:semver +interop-beta.5 function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop-beta.4"); + return string.concat(super.version(), "+interop-beta.5"); + } + + /// @notice Constructs the SystemConfig contract. + /// @param _superchainConfig The address of the SuperchainConfig contract. + constructor(address _superchainConfig) { + SUPERCHAIN_CONFIG = _superchainConfig; } /// @notice Internal setter for the gas paying token address, includes validation. @@ -102,26 +67,32 @@ contract SystemConfigInterop is SystemConfig { } } - /// @notice Adds a chain to the interop dependency set. Can only be called by the dependency manager. + /// @notice Adds a chain to the interop dependency set. Can only be called by the SuperchainConfig. /// @param _chainId Chain ID of chain to add. function addDependency(uint256 _chainId) external { - require(msg.sender == dependencyManager(), "SystemConfig: caller is not the dependency manager"); + require(msg.sender == SUPERCHAIN_CONFIG, "SystemConfig: caller is not the SuperchainConfig"); + + Storage.setUint(DEPENDENCY_COUNTER_SLOT, dependencyCounter() + 1); + IOptimismPortal(payable(optimismPortal())).setConfig( ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId) ); } - /// @notice Removes a chain from the interop dependency set. Can only be called by the dependency manager + /// @notice Removes a chain from the interop dependency set. Can only be called by the SuperchainConfig /// @param _chainId Chain ID of the chain to remove. function removeDependency(uint256 _chainId) external { - require(msg.sender == dependencyManager(), "SystemConfig: caller is not the dependency manager"); + require(msg.sender == SUPERCHAIN_CONFIG, "SystemConfig: caller is not the SuperchainConfig"); + + Storage.setUint(DEPENDENCY_COUNTER_SLOT, dependencyCounter() - 1); + IOptimismPortal(payable(optimismPortal())).setConfig( ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId) ); } - /// @notice getter for the dependency manager address - function dependencyManager() public view returns (address) { - return Storage.getAddress(DEPENDENCY_MANAGER_SLOT); + /// @notice getter for the dependency counter + function dependencyCounter() public view returns (uint256) { + return Storage.getUint(DEPENDENCY_COUNTER_SLOT); } } diff --git a/packages/contracts-bedrock/src/L1/interfaces/ISystemConfigInterop.sol b/packages/contracts-bedrock/src/L1/interfaces/ISystemConfigInterop.sol index e11d17c9f7bd..5dd822d61183 100644 --- a/packages/contracts-bedrock/src/L1/interfaces/ISystemConfigInterop.sol +++ b/packages/contracts-bedrock/src/L1/interfaces/ISystemConfigInterop.sol @@ -55,7 +55,7 @@ interface ISystemConfigInterop { function addDependency(uint256 _chainId) external; function removeDependency(uint256 _chainId) external; - function dependencyManager() external view returns (address); + function dependencyCounter() external view returns (uint256); function initialize( address _owner, uint32 _basefeeScalar, @@ -65,11 +65,11 @@ interface ISystemConfigInterop { address _unsafeBlockSigner, IResourceMetering.ResourceConfig memory _config, address _batchInbox, - ISystemConfig.Addresses memory _addresses, - address _dependencyManager + ISystemConfig.Addresses memory _addresses ) external; function version() external pure returns (string memory); + function SUPERCHAIN_CONFIG() external view returns (address); - function __constructor__() external; + function __constructor__(address _superchainConfig) external; } diff --git a/packages/contracts-bedrock/test/L1/SystemConfigInterop.t.sol b/packages/contracts-bedrock/test/L1/SystemConfigInterop.t.sol index 78337fc3b4d0..cb43b548a748 100644 --- a/packages/contracts-bedrock/test/L1/SystemConfigInterop.t.sol +++ b/packages/contracts-bedrock/test/L1/SystemConfigInterop.t.sol @@ -26,6 +26,13 @@ contract SystemConfigInterop_Test is CommonTest { super.setUp(); } + /// @dev Tests that the constructor sets the correct values. + function test_constructor_succeeds() external view { + ISystemConfigInterop impl = ISystemConfigInterop(payable(deploy.mustGetAddress("SystemConfig"))); + assertEq(impl.SUPERCHAIN_CONFIG(), address(superchainConfig)); + assertEq(_systemConfigInterop().SUPERCHAIN_CONFIG(), address(superchainConfig)); + } + /// @dev Tests that when the decimals is not 18, initialization reverts. function test_initialize_decimalsIsNot18_reverts(uint8 decimals) external { vm.assume(decimals != 18); @@ -87,20 +94,27 @@ contract SystemConfigInterop_Test is CommonTest { ) ); - vm.prank(_systemConfigInterop().dependencyManager()); + vm.prank(address(superchainConfig)); _systemConfigInterop().addDependency(_chainId); + + assertEq(_systemConfigInterop().dependencyCounter(), 1); } - /// @dev Tests that adding a dependency as not the dependency manager reverts. - function testFuzz_addDependency_notDependencyManager_reverts(uint256 _chainId) public { - require(alice != _systemConfigInterop().dependencyManager(), "SystemConfigInterop_Test: 100"); - vm.expectRevert("SystemConfig: caller is not the dependency manager"); + /// @dev Tests that adding a dependency as not the SuperchainConfig reverts. + function testFuzz_addDependency_notSuperchainConfig_reverts(uint256 _chainId) public { + require(alice != address(superchainConfig), "SystemConfigInterop_Test: 100"); + vm.expectRevert("SystemConfig: caller is not the SuperchainConfig"); vm.prank(alice); _systemConfigInterop().addDependency(_chainId); } /// @dev Tests that a dependency can be removed. function testFuzz_removeDependency_succeeds(uint256 _chainId) public { + // Add the dependency first + vm.prank(address(superchainConfig)); + _systemConfigInterop().addDependency(_chainId); + assertEq(_systemConfigInterop().dependencyCounter(), 1); + vm.expectCall( address(optimismPortal), abi.encodeCall( @@ -109,18 +123,34 @@ contract SystemConfigInterop_Test is CommonTest { ) ); - vm.prank(_systemConfigInterop().dependencyManager()); + vm.prank(address(superchainConfig)); _systemConfigInterop().removeDependency(_chainId); + + assertEq(_systemConfigInterop().dependencyCounter(), 0); } - /// @dev Tests that removing a dependency as not the dependency manager reverts. - function testFuzz_removeDependency_notDependencyManager_reverts(uint256 _chainId) public { - require(alice != _systemConfigInterop().dependencyManager(), "SystemConfigInterop_Test: 100"); - vm.expectRevert("SystemConfig: caller is not the dependency manager"); + /// @dev Tests that removing a dependency as not the SuperchainConfig reverts. + function testFuzz_removeDependency_notSuperchainConfig_reverts(uint256 _chainId) public { + require(alice != address(superchainConfig), "SystemConfigInterop_Test: 100"); + vm.expectRevert("SystemConfig: caller is not the SuperchainConfig"); vm.prank(alice); _systemConfigInterop().removeDependency(_chainId); } + function test_dependencyCounter_succeeds() public { + assertEq(_systemConfigInterop().dependencyCounter(), 0); + + // Add a dependency + vm.prank(address(superchainConfig)); + _systemConfigInterop().addDependency(1); + assertEq(_systemConfigInterop().dependencyCounter(), 1); + + // Remove the dependency + vm.prank(address(superchainConfig)); + _systemConfigInterop().removeDependency(1); + assertEq(_systemConfigInterop().dependencyCounter(), 0); + } + /// @dev Helper to clean storage and then initialize the system config with an arbitrary gas token address. function _cleanStorageAndInit(address _token) internal { // Wipe out the initialized slot so the proxy can be initialized again diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index 1cc0691c3414..fdf3d78b4fe1 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -43,7 +43,6 @@ contract Specification_Test is CommonTest { DELAYEDWETHOWNER, COUNCILSAFE, COUNCILSAFEOWNER, - DEPENDENCYMANAGER, PORTAL, SUPERCHAINCONFIG } @@ -492,7 +491,6 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("gasLimit()") }); _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("eip1559Denominator()") }); _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("eip1559Elasticity()") }); - _addSpec({ _name: "SystemConfigInterop", _sel: ISystemConfigInterop.initialize.selector }); _addSpec({ _name: "SystemConfigInterop", _sel: ISystemConfig.initialize.selector }); _addSpec({ _name: "SystemConfigInterop", _sel: ISystemConfigInterop.minimumGasLimit.selector }); _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("overhead()") }); @@ -559,13 +557,14 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("basefeeScalar()") }); _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("blobbasefeeScalar()") }); _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("maximumGasLimit()") }); - _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("addDependency(uint256)"), _auth: Role.DEPENDENCYMANAGER }); + _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("addDependency(uint256)"), _auth: Role.SUPERCHAINCONFIG }); _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("removeDependency(uint256)"), - _auth: Role.DEPENDENCYMANAGER + _auth: Role.SUPERCHAINCONFIG }); - _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("dependencyManager()") }); + _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("dependencyCounter()") }); + _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("SUPERCHAIN_CONFIG()") }); // ProxyAdmin _addSpec({ _name: "ProxyAdmin", _sel: _getSel("addressManager()") }); @@ -857,18 +856,6 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "OPContractsManager", _sel: OPContractsManager.chainIdToBatchInboxAddress.selector }); _addSpec({ _name: "OPContractsManager", _sel: OPContractsManager.implementations.selector }); - // OPContractsManagerInterop - _addSpec({ _name: "OPContractsManagerInterop", _sel: _getSel("version()") }); - _addSpec({ _name: "OPContractsManagerInterop", _sel: _getSel("superchainConfig()") }); - _addSpec({ _name: "OPContractsManagerInterop", _sel: _getSel("protocolVersions()") }); - _addSpec({ _name: "OPContractsManagerInterop", _sel: _getSel("l1ContractsRelease()") }); - _addSpec({ _name: "OPContractsManagerInterop", _sel: _getSel("systemConfigs(uint256)") }); - _addSpec({ _name: "OPContractsManagerInterop", _sel: _getSel("OUTPUT_VERSION()") }); - _addSpec({ _name: "OPContractsManagerInterop", _sel: OPContractsManager.deploy.selector }); - _addSpec({ _name: "OPContractsManagerInterop", _sel: OPContractsManager.blueprints.selector }); - _addSpec({ _name: "OPContractsManagerInterop", _sel: OPContractsManager.chainIdToBatchInboxAddress.selector }); - _addSpec({ _name: "OPContractsManagerInterop", _sel: OPContractsManager.implementations.selector }); - // DeputyGuardianModule _addSpec({ _name: "DeputyGuardianModule", diff --git a/packages/contracts-bedrock/test/vendor/Initializable.t.sol b/packages/contracts-bedrock/test/vendor/Initializable.t.sol index eb43ae187599..74485fdc9124 100644 --- a/packages/contracts-bedrock/test/vendor/Initializable.t.sol +++ b/packages/contracts-bedrock/test/vendor/Initializable.t.sol @@ -409,7 +409,6 @@ contract Initializer_Test is CommonTest { excludes[5] = "src/dispute/PermissionedDisputeGame.sol"; // TODO: Eventually remove this exclusion. Same reason as above dispute contracts. excludes[6] = "src/L1/OPContractsManager.sol"; - excludes[7] = "src/L1/OPContractsManagerInterop.sol"; // The L2OutputOracle is not always deployed (and is no longer being modified) excludes[8] = "src/L1/L2OutputOracle.sol"; From 3254d14d2e50f76803962c7bd35a9e8a92c04d80 Mon Sep 17 00:00:00 2001 From: Disco <131301107+0xDiscotech@users.noreply.github.com> Date: Thu, 28 Nov 2024 15:33:27 -0300 Subject: [PATCH 05/13] feat: manage dependency set on superchain config (#138) --- .../scripts/deploy/DeployOwnership.s.sol | 2 +- .../scripts/deploy/DeploySuperchain.s.sol | 195 ++++++++++------ .../contracts-bedrock/snapshots/.gas-snapshot | 4 +- .../snapshots/abi/SharedLockbox.json | 18 +- .../snapshots/abi/SuperchainConfig.json | 125 +++++++++- .../snapshots/semver-lock.json | 10 +- .../storageLayout/SuperchainConfig.json | 14 ++ .../src/L1/OptimismPortal2.sol | 2 - .../src/L1/SharedLockbox.sol | 4 +- .../src/L1/SuperchainConfig.sol | 73 +++++- .../src/L1/interfaces/ISharedLockbox.sol | 2 +- .../src/L1/interfaces/ISuperchainConfig.sol | 15 +- .../test/L1/SharedLockbox.t.sol | 6 +- .../test/L1/SuperchainConfig.t.sol | 213 +++++++++++++++++- .../test/universal/Specs.t.sol | 5 + 15 files changed, 582 insertions(+), 106 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol b/packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol index e126c39f18c9..1a0d351e476c 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol @@ -324,7 +324,7 @@ contract DeployOwnership is Deploy { _save: this, _salt: _implSalt(), _name: "SuperchainConfig", - _args: DeployUtils.encodeConstructor(abi.encodeCall(ISuperchainConfig.__constructor__, ())) + _args: DeployUtils.encodeConstructor(abi.encodeCall(ISuperchainConfig.__constructor__, (address(0)))) }) ); diff --git a/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol b/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol index 698e41c8c931..55fe545411ef 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol @@ -70,6 +70,7 @@ import { BaseDeployIO } from "scripts/deploy/BaseDeployIO.sol"; // All contracts of the form `DeployInput` should inherit from `BaseDeployIO`, as it provides // shared functionality for all deploy scripts, such as access to cheat codes. contract DeploySuperchainInput is BaseDeployIO { + // We use the `stdToml` library to parse TOML input files. This allows us to easily parse using stdToml for string; // All inputs are set in storage individually. We put any roles first, followed by the remaining @@ -323,6 +324,13 @@ contract DeploySuperchainOutput is BaseDeployIO { // default sender would be the broadcaster during test, but the broadcaster needs to be the deployer // since they are set to the initial proxy admin owner. contract DeploySuperchain is Script { + // The `PrecalculatedAddresses` stores the precalculated addresses so then they can be checked on the actual + // deployment. + struct PrecalculatedAddresses { + address superchainConfigProxy; + address sharedLockboxProxy; + } + // -------- Core Deployment Methods -------- function run(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { @@ -338,11 +346,8 @@ contract DeploySuperchain is Script { // Deploy the proxy admin, with the owner set to the deployer. deploySuperchainProxyAdmin(_dsi, _dso); - // Deploy and initialize the superchain contracts. - deploySuperchainImplementationContracts(_dsi, _dso); - deployAndInitializeSuperchainConfig(_dsi, _dso); - deployAndInitializeProtocolVersions(_dsi, _dso); - deploySharedLockbox(_dsi, _dso); + // Deploy implementations, proxies and then initialize the superchain contracts. + deploySuperchain(_dsi, _dso); // Transfer ownership of the ProxyAdmin from the deployer to the specified owner. transferProxyAdminOwnership(_dsi, _dso); @@ -370,39 +375,131 @@ contract DeploySuperchain is Script { _dso.set(_dso.superchainProxyAdmin.selector, address(superchainProxyAdmin)); } - function deploySuperchainImplementationContracts(DeploySuperchainInput, DeploySuperchainOutput _dso) public { - // Deploy implementation contracts. + function deploySuperchain(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { + // Precalculate the proxies addresses. Needed since there are circular dependencies between them. + PrecalculatedAddresses memory precalculatedAddresses; + precalculatedAddresses.superchainConfigProxy = vm.computeCreateAddress(msg.sender, vm.getNonce(msg.sender) + 3); + precalculatedAddresses.sharedLockboxProxy = vm.computeCreateAddress(msg.sender, vm.getNonce(msg.sender) + 7); + + // Deploy implementation contracts + deploySuperchainImplementationContracts(_dsi, _dso, precalculatedAddresses); + + // Deploy proxy contracts + deployAndInitializeSuperchainProxyContracts(_dsi, _dso, precalculatedAddresses); + } + + function deploySuperchainImplementationContracts( + DeploySuperchainInput, + DeploySuperchainOutput _dso, + PrecalculatedAddresses memory _precalculatedAddresses + ) + internal + { vm.startBroadcast(msg.sender); + + // Deploy SuperchainConfig implementation ISuperchainConfig superchainConfigImpl = ISuperchainConfig( DeployUtils.create1({ _name: "SuperchainConfig", - _args: DeployUtils.encodeConstructor(abi.encodeCall(ISuperchainConfig.__constructor__, ())) + _args: DeployUtils.encodeConstructor( + abi.encodeCall(ISuperchainConfig.__constructor__, (_precalculatedAddresses.sharedLockboxProxy)) + ) }) ); + + // Deploy ProtocolVersions implementation IProtocolVersions protocolVersionsImpl = IProtocolVersions( DeployUtils.create1({ _name: "ProtocolVersions", _args: DeployUtils.encodeConstructor(abi.encodeCall(IProtocolVersions.__constructor__, ())) }) ); + + // Deploy SharedLockbox implementation + ISharedLockbox sharedLockboxImpl = ISharedLockbox( + DeployUtils.create1({ + _name: "SharedLockbox", + _args: DeployUtils.encodeConstructor( + abi.encodeCall(ISharedLockbox.__constructor__, (_precalculatedAddresses.superchainConfigProxy)) + ) + }) + ); + vm.stopBroadcast(); vm.label(address(superchainConfigImpl), "SuperchainConfigImpl"); vm.label(address(protocolVersionsImpl), "ProtocolVersionsImpl"); + vm.label(address(sharedLockboxImpl), "SharedLockboxImpl"); _dso.set(_dso.superchainConfigImpl.selector, address(superchainConfigImpl)); _dso.set(_dso.protocolVersionsImpl.selector, address(protocolVersionsImpl)); + _dso.set(_dso.sharedLockboxImpl.selector, address(sharedLockboxImpl)); } - function deployAndInitializeSuperchainConfig(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { - address guardian = _dsi.guardian(); - bool paused = _dsi.paused(); - + function deployAndInitializeSuperchainProxyContracts( + DeploySuperchainInput _dsi, + DeploySuperchainOutput _dso, + PrecalculatedAddresses memory _precalculatedAddresses + ) + internal + { IProxyAdmin superchainProxyAdmin = _dso.superchainProxyAdmin(); - ISuperchainConfig superchainConfigImpl = _dso.superchainConfigImpl(); + // Deploy SuperchainConfig proxy + ISuperchainConfig superchainConfigProxy; + { + address guardian = _dsi.guardian(); + bool paused = _dsi.paused(); + + vm.startBroadcast(msg.sender); + superchainConfigProxy = ISuperchainConfig( + DeployUtils.create1({ + _name: "Proxy", + _args: DeployUtils.encodeConstructor( + abi.encodeCall(IProxy.__constructor__, (address(superchainProxyAdmin))) + ) + }) + ); + superchainProxyAdmin.upgradeAndCall( + payable(address(superchainConfigProxy)), + address(_dso.superchainConfigImpl()), + abi.encodeCall(ISuperchainConfig.initialize, (guardian, paused)) + ); + vm.stopBroadcast(); + } + + // Deploy ProtocolVersions proxy + IProtocolVersions protocolVersionsProxy; + { + address protocolVersionsOwner = _dsi.protocolVersionsOwner(); + ProtocolVersion requiredProtocolVersion = _dsi.requiredProtocolVersion(); + ProtocolVersion recommendedProtocolVersion = _dsi.recommendedProtocolVersion(); + IProtocolVersions protocolVersions = _dso.protocolVersionsImpl(); + + vm.startBroadcast(msg.sender); + // Deploy ProtocolVersion proxy + protocolVersionsProxy = IProtocolVersions( + DeployUtils.create1({ + _name: "Proxy", + _args: DeployUtils.encodeConstructor( + abi.encodeCall(IProxy.__constructor__, (address(superchainProxyAdmin))) + ) + }) + ); + superchainProxyAdmin.upgradeAndCall( + payable(address(protocolVersionsProxy)), + address(protocolVersions), + abi.encodeCall( + IProtocolVersions.initialize, + (protocolVersionsOwner, requiredProtocolVersion, recommendedProtocolVersion) + ) + ); + vm.stopBroadcast(); + } + + // Deploy SharedLockbox proxy vm.startBroadcast(msg.sender); - ISuperchainConfig superchainConfigProxy = ISuperchainConfig( + ISharedLockbox sharedLockboxProxy = ISharedLockbox( DeployUtils.create1({ _name: "Proxy", _args: DeployUtils.encodeConstructor( @@ -410,77 +507,27 @@ contract DeploySuperchain is Script { ) }) ); - superchainProxyAdmin.upgradeAndCall( - payable(address(superchainConfigProxy)), - address(superchainConfigImpl), - abi.encodeCall(ISuperchainConfig.initialize, (guardian, paused)) - ); + superchainProxyAdmin.upgrade(payable(address(sharedLockboxProxy)), address(_dso.sharedLockboxImpl())); vm.stopBroadcast(); vm.label(address(superchainConfigProxy), "SuperchainConfigProxy"); _dso.set(_dso.superchainConfigProxy.selector, address(superchainConfigProxy)); - } - - function deployAndInitializeProtocolVersions(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { - address protocolVersionsOwner = _dsi.protocolVersionsOwner(); - ProtocolVersion requiredProtocolVersion = _dsi.requiredProtocolVersion(); - ProtocolVersion recommendedProtocolVersion = _dsi.recommendedProtocolVersion(); - - IProxyAdmin superchainProxyAdmin = _dso.superchainProxyAdmin(); - IProtocolVersions protocolVersionsImpl = _dso.protocolVersionsImpl(); - - vm.startBroadcast(msg.sender); - IProtocolVersions protocolVersionsProxy = IProtocolVersions( - DeployUtils.create1({ - _name: "Proxy", - _args: DeployUtils.encodeConstructor( - abi.encodeCall(IProxy.__constructor__, (address(superchainProxyAdmin))) - ) - }) - ); - superchainProxyAdmin.upgradeAndCall( - payable(address(protocolVersionsProxy)), - address(protocolVersionsImpl), - abi.encodeCall( - IProtocolVersions.initialize, - (protocolVersionsOwner, requiredProtocolVersion, recommendedProtocolVersion) - ) + // To ensure deployments are correct, check that the precalculated address matches the actual address. + require( + address(superchainConfigProxy) == _precalculatedAddresses.superchainConfigProxy, + "superchain config expected address mismatch" ); - vm.stopBroadcast(); vm.label(address(protocolVersionsProxy), "ProtocolVersionsProxy"); _dso.set(_dso.protocolVersionsProxy.selector, address(protocolVersionsProxy)); - } - function deploySharedLockbox(DeploySuperchainInput, DeploySuperchainOutput _dso) public { - IProxyAdmin superchainProxyAdmin = _dso.superchainProxyAdmin(); - ISuperchainConfig superchainConfigProxy = _dso.superchainConfigProxy(); - - vm.startBroadcast(msg.sender); - ISharedLockbox sharedLockboxImpl = ISharedLockbox( - DeployUtils.create1({ - _name: "SharedLockbox", - _args: DeployUtils.encodeConstructor( - abi.encodeCall(ISharedLockbox.__constructor__, (address(superchainConfigProxy))) - ) - }) - ); - ISharedLockbox sharedLockboxProxy = ISharedLockbox( - DeployUtils.create1({ - _name: "Proxy", - _args: DeployUtils.encodeConstructor( - abi.encodeCall(IProxy.__constructor__, (address(superchainProxyAdmin))) - ) - }) - ); - superchainProxyAdmin.upgrade(payable(address(sharedLockboxProxy)), address(sharedLockboxImpl)); - vm.stopBroadcast(); - - vm.label(address(sharedLockboxImpl), "SharedLockboxImpl"); vm.label(address(sharedLockboxProxy), "SharedLockboxProxy"); - - _dso.set(_dso.sharedLockboxImpl.selector, address(sharedLockboxImpl)); _dso.set(_dso.sharedLockboxProxy.selector, address(sharedLockboxProxy)); + // To ensure deployments are correct, check that the precalculated address matches the actual address. + require( + address(sharedLockboxProxy) == _precalculatedAddresses.sharedLockboxProxy, + "shared lockbox expected address mismatch" + ); } function transferProxyAdminOwnership(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { diff --git a/packages/contracts-bedrock/snapshots/.gas-snapshot b/packages/contracts-bedrock/snapshots/.gas-snapshot index 505885461f3a..c295d6765877 100644 --- a/packages/contracts-bedrock/snapshots/.gas-snapshot +++ b/packages/contracts-bedrock/snapshots/.gas-snapshot @@ -10,8 +10,8 @@ GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 5644 GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076690) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 467025) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512729) -GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72661) +GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72728) GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 92973) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 68422) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 68986) -GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 155565) \ No newline at end of file +GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 155632) \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json b/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json index 5b707cd7ceff..567ae6901802 100644 --- a/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json +++ b/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json @@ -96,9 +96,15 @@ "internalType": "address", "name": "portal", "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "amount", + "type": "uint256" } ], - "name": "AuthorizedPortal", + "name": "ETHLocked", "type": "event" }, { @@ -117,7 +123,7 @@ "type": "uint256" } ], - "name": "ETHLocked", + "name": "ETHUnlocked", "type": "event" }, { @@ -128,15 +134,9 @@ "internalType": "address", "name": "portal", "type": "address" - }, - { - "indexed": false, - "internalType": "uint256", - "name": "amount", - "type": "uint256" } ], - "name": "ETHUnlocked", + "name": "PortalAuthorized", "type": "event" }, { diff --git a/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json b/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json index 0304f507eb50..4efd65d9bbac 100644 --- a/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json +++ b/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json @@ -1,6 +1,12 @@ [ { - "inputs": [], + "inputs": [ + { + "internalType": "address", + "name": "_sharedLockbox", + "type": "address" + } + ], "stateMutability": "nonpayable", "type": "constructor" }, @@ -30,6 +36,50 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "SHARED_LOCKBOX", + "outputs": [ + { + "internalType": "contract ISharedLockbox", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "uint256", + "name": "_chainId", + "type": "uint256" + }, + { + "internalType": "address", + "name": "_systemConfig", + "type": "address" + } + ], + "name": "addChain", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [], + "name": "dependencySet", + "outputs": [ + { + "internalType": "uint256[]", + "name": "", + "type": "uint256[]" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "guardian", @@ -61,6 +111,25 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "uint256", + "name": "_chainId", + "type": "uint256" + } + ], + "name": "isInDependencySet", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -87,6 +156,25 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "name": "systemConfigs", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "unpause", @@ -107,6 +195,31 @@ "stateMutability": "view", "type": "function" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "uint256", + "name": "chainId", + "type": "uint256" + }, + { + "indexed": true, + "internalType": "address", + "name": "systemConfig", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "portal", + "type": "address" + } + ], + "name": "ChainAdded", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -157,5 +270,15 @@ "inputs": [], "name": "Unpaused", "type": "event" + }, + { + "inputs": [], + "name": "ChainAlreadyAdded", + "type": "error" + }, + { + "inputs": [], + "name": "Unauthorized", + "type": "error" } ] \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 76385ca6aa6c..ec6549c6ec69 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -29,7 +29,7 @@ }, "src/L1/OptimismPortal2.sol": { "initCodeHash": "0x215f439adc85bc5ebb1b234c1acc128eecb7c9a43243edbfcd9e701f128224f1", - "sourceCodeHash": "0x4c33430270b28fe0850c112c05d121477429688aae74c58754249476fb2d6c8e" + "sourceCodeHash": "0xb7cbcb27240d0fd85a7a3009676abb93679e3b8723a967c1f310390464399869" }, "src/L1/OptimismPortalInterop.sol": { "initCodeHash": "0x831af803158768b7fc229822eebda0ce7922cb1852fd770856b425023a379d47", @@ -40,12 +40,12 @@ "sourceCodeHash": "0x15205131bf420aa6d03c558bb75dd49cd7439caed7ccdcbfd89c4170a48c94f5" }, "src/L1/SharedLockbox.sol": { - "initCodeHash": "0x284eaf282bfbe4403ec5c692371ad2876ec872851b384e5779e6eae577f1dfbb", - "sourceCodeHash": "0x60c8a4ce8f916237ea8af9580f011840f5de3393eee9e53c3c89761e09395e91" + "initCodeHash": "0xa6aa965a99138876c2ba0c48910068ea0d0133699a2a22e7dc867a46d9e927af", + "sourceCodeHash": "0x26b7450f991fe197a94861cf618df04feb695dbbba884ffe93f96defa63e5bd7" }, "src/L1/SuperchainConfig.sol": { - "initCodeHash": "0xfca12d9016c746e5c275b186e0ca40cfd65cf45a5665aab7589a669fea3abb47", - "sourceCodeHash": "0x39489a85bc3a5c8560f82d41b31bf7fe22f5b648f4ed538f61695a73092ea9eb" + "initCodeHash": "0x14e3ce78307d6709b29199de4380620003cbfe311a56b201ab2767c17e9e2091", + "sourceCodeHash": "0xff091439483e0669cf39c27d16ad0ef6c624d28829d079fe2c5c2c836a2db748" }, "src/L1/SystemConfig.sol": { "initCodeHash": "0x0eda38e2fb2687a324289f04ec8ad0d2afe51f45219d074740fb4a0e24ea6569", diff --git a/packages/contracts-bedrock/snapshots/storageLayout/SuperchainConfig.json b/packages/contracts-bedrock/snapshots/storageLayout/SuperchainConfig.json index 70e559b7bf6d..e818b2b6d519 100644 --- a/packages/contracts-bedrock/snapshots/storageLayout/SuperchainConfig.json +++ b/packages/contracts-bedrock/snapshots/storageLayout/SuperchainConfig.json @@ -12,5 +12,19 @@ "offset": 1, "slot": "0", "type": "bool" + }, + { + "bytes": "32", + "label": "systemConfigs", + "offset": 0, + "slot": "1", + "type": "mapping(uint256 => address)" + }, + { + "bytes": "64", + "label": "_dependencySet", + "offset": 0, + "slot": "2", + "type": "struct EnumerableSet.UintSet" } ] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index 6fce8c1e16c9..96304433f46d 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -303,8 +303,6 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { } /// @notice Accepts ETH value without triggering a deposit to L2. - /// This function mainly exists for the sake of the migration between the legacy - /// Optimism system and Bedrock. function donateETH() external payable { // Intentionally empty. } diff --git a/packages/contracts-bedrock/src/L1/SharedLockbox.sol b/packages/contracts-bedrock/src/L1/SharedLockbox.sol index 3a4c6bc2c570..af027992d8bc 100644 --- a/packages/contracts-bedrock/src/L1/SharedLockbox.sol +++ b/packages/contracts-bedrock/src/L1/SharedLockbox.sol @@ -22,7 +22,7 @@ contract SharedLockbox is ISemver { /// @notice Emitted when a portal is set as authorized to interact with the lockbox. /// @param portal The address of the authorized portal. - event AuthorizedPortal(address indexed portal); + event PortalAuthorized(address indexed portal); /// @notice The address of the SuperchainConfig contract. address public immutable SUPERCHAIN_CONFIG; @@ -65,6 +65,6 @@ contract SharedLockbox is ISemver { if (msg.sender != SUPERCHAIN_CONFIG) revert Unauthorized(); authorizedPortals[_portal] = true; - emit AuthorizedPortal(_portal); + emit PortalAuthorized(_portal); } } diff --git a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol index 51b13936c81b..ad811120bb61 100644 --- a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol +++ b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol @@ -4,12 +4,18 @@ pragma solidity 0.8.15; import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; import { ISemver } from "src/universal/interfaces/ISemver.sol"; import { Storage } from "src/libraries/Storage.sol"; +import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import { Unauthorized } from "src/libraries/errors/CommonErrors.sol"; +import { ISystemConfigInterop } from "src/L1/interfaces/ISystemConfigInterop.sol"; +import { ISharedLockbox } from "src/L1/interfaces/ISharedLockbox.sol"; /// @custom:proxied true /// @custom:audit none This contracts is not yet audited. /// @title SuperchainConfig /// @notice The SuperchainConfig contract is used to manage configuration of global superchain values. contract SuperchainConfig is Initializable, ISemver { + using EnumerableSet for EnumerableSet.UintSet; + /// @notice Enum representing different types of updates. /// @custom:value GUARDIAN Represents an update to the guardian. enum UpdateType { @@ -23,6 +29,9 @@ contract SuperchainConfig is Initializable, ISemver { /// It can only be modified by an upgrade. bytes32 public constant GUARDIAN_SLOT = bytes32(uint256(keccak256("superchainConfig.guardian")) - 1); + // The Shared Lockbox contract + ISharedLockbox public immutable SHARED_LOCKBOX; + /// @notice Emitted when the pause is triggered. /// @param identifier A string helping to identify provenance of the pause transaction. event Paused(string identifier); @@ -35,12 +44,28 @@ contract SuperchainConfig is Initializable, ISemver { /// @param data Encoded update data. event ConfigUpdate(UpdateType indexed updateType, bytes data); + /// @notice Emitted when a new chain is added as part of the dependency set. + /// @param chainId The chain ID. + /// @param systemConfig The address of the SystemConfig contract. + /// @param portal The address of the OptimismPortal contract. + event ChainAdded(uint256 indexed chainId, address indexed systemConfig, address indexed portal); + + /// @notice Thrown when the input chain is already added to the dependency set. + error ChainAlreadyAdded(); + /// @notice Semantic version. - /// @custom:semver 1.1.1-beta.1 - string public constant version = "1.1.1-beta.1"; + /// @custom:semver 1.1.1-beta.2 + string public constant version = "1.1.1-beta.2"; + + // Mapping from chainId to SystemConfig address + mapping(uint256 => address) public systemConfigs; + + // Dependency set of chains that are part of the same cluster + EnumerableSet.UintSet internal _dependencySet; /// @notice Constructs the SuperchainConfig contract. - constructor() { + constructor(address _sharedLockbox) { + SHARED_LOCKBOX = ISharedLockbox(_sharedLockbox); initialize({ _guardian: address(0), _paused: false }); } @@ -92,4 +117,46 @@ contract SuperchainConfig is Initializable, ISemver { Storage.setAddress(GUARDIAN_SLOT, _guardian); emit ConfigUpdate(UpdateType.GUARDIAN, abi.encode(_guardian)); } + + /// @notice Adds a new chain to the dependency set. + /// Adds the new chain as a dependency for all existing chains in the dependency set, and vice versa. It + /// also stores the SystemConfig address of it, and authorizes the OptimismPortal on the SharedLockbox. + /// @param _chainId The chain ID. + /// @param _systemConfig The address of the SystemConfig contract. + function addChain(uint256 _chainId, address _systemConfig) external { + // TODO: Updater role TBD, using guardian for now. + if (msg.sender != guardian()) revert Unauthorized(); + // Add to the dependency set and check it is not already added (`add()` returns false if it already exists) + if (!_dependencySet.add(_chainId)) revert ChainAlreadyAdded(); + + systemConfigs[_chainId] = _systemConfig; + + // Loop through the dependency set and update the dependency for each chain. Using length - 1 to exclude the + // current chain from the loop. + for (uint256 i; i < _dependencySet.length() - 1; i++) { + uint256 currentId = _dependencySet.at(i); + + // Add the new chain as dependency for the current chain on the loop + ISystemConfigInterop(systemConfigs[currentId]).addDependency(_chainId); + // Add the current chain on the loop as dependency for the new chain + ISystemConfigInterop(_systemConfig).addDependency(currentId); + } + + // Authorize the portal on the shared lockbox + address portal = ISystemConfigInterop(_systemConfig).optimismPortal(); + SHARED_LOCKBOX.authorizePortal(portal); + + emit ChainAdded(_chainId, _systemConfig, portal); + } + + /// @notice Checks if a chain is part or not of the dependency set. + /// @param _chainId The chain ID to check for. + function isInDependencySet(uint256 _chainId) public view returns (bool) { + return _dependencySet.contains(_chainId); + } + + /// @notice Getter for the chain ids list on the dependency set. + function dependencySet() external view returns (uint256[] memory) { + return _dependencySet.values(); + } } diff --git a/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol b/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol index 9aac053e2d42..76881326b9ca 100644 --- a/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol +++ b/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol @@ -12,7 +12,7 @@ interface ISharedLockbox is ISemver { event ETHUnlocked(address indexed portal, uint256 amount); - event AuthorizedPortal(address indexed portal); + event PortalAuthorized(address indexed portal); function SUPERCHAIN_CONFIG() external view returns (address); diff --git a/packages/contracts-bedrock/src/L1/interfaces/ISuperchainConfig.sol b/packages/contracts-bedrock/src/L1/interfaces/ISuperchainConfig.sol index dc83893958b0..24a65f9a470c 100644 --- a/packages/contracts-bedrock/src/L1/interfaces/ISuperchainConfig.sol +++ b/packages/contracts-bedrock/src/L1/interfaces/ISuperchainConfig.sol @@ -1,7 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -interface ISuperchainConfig { +import { IDependencySet } from "src/L2/interfaces/IDependencySet.sol"; +import { ISharedLockbox } from "src/L1/interfaces/ISharedLockbox.sol"; + +interface ISuperchainConfig is IDependencySet { enum UpdateType { GUARDIAN } @@ -10,15 +13,23 @@ interface ISuperchainConfig { event Initialized(uint8 version); event Paused(string identifier); event Unpaused(); + event ChainAdded(uint256 indexed chainId, address indexed systemConfig, address indexed portal); + + error ChainAlreadyAdded(); + error Unauthorized(); function GUARDIAN_SLOT() external view returns (bytes32); function PAUSED_SLOT() external view returns (bytes32); + function SHARED_LOCKBOX() external view returns (ISharedLockbox); function guardian() external view returns (address guardian_); + function systemConfigs(uint256) external view returns (address); function initialize(address _guardian, bool _paused) external; function pause(string memory _identifier) external; function paused() external view returns (bool paused_); function unpause() external; function version() external view returns (string memory); + function addChain(uint256 _chainId, address _systemConfig) external; + function dependencySet() external view returns (uint256[] memory); - function __constructor__() external; + function __constructor__(address _sharedLockbox) external; } diff --git a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol index 09b846b1c05a..5a1bddcb97cc 100644 --- a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol +++ b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol @@ -16,7 +16,7 @@ contract SharedLockboxTest is CommonTest { event ETHUnlocked(address indexed portal, uint256 amount); - event AuthorizedPortal(address indexed portal); + event PortalAuthorized(address indexed portal); /// @notice Tests it reverts when the caller is not an authorized portal. function test_lockETH_unauthorizedPortal_reverts(address _caller) public { @@ -115,9 +115,9 @@ contract SharedLockboxTest is CommonTest { // Adding this check to make it more future proof in case something changes on the setup. vm.assume(sharedLockbox.authorizedPortals(_portal) == false); - // Look for the emit of the `AuthorizedPortal` event + // Look for the emit of the `PortalAuthorized` event vm.expectEmit(address(sharedLockbox)); - emit AuthorizedPortal(_portal); + emit PortalAuthorized(_portal); // Call the `authorizePortal` function with the SuperchainConfig vm.prank(address(superchainConfig)); diff --git a/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol b/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol index 2772ec0c2a3c..6a39fd642071 100644 --- a/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol +++ b/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol @@ -5,11 +5,37 @@ import { CommonTest } from "test/setup/CommonTest.sol"; // Target contract dependencies import { IProxy } from "src/universal/interfaces/IProxy.sol"; +import { Unauthorized } from "src/libraries/errors/CommonErrors.sol"; // Target contract import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; +import { SuperchainConfig, ISharedLockbox, ISystemConfigInterop } from "src/L1/SuperchainConfig.sol"; import { DeployUtils } from "scripts/libraries/DeployUtils.sol"; +import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; + +/// @notice For testing purposes contract, with setters to facilitate replicating complex scenarios when needed. +contract SuperchainConfigForTest is SuperchainConfig { + using EnumerableSet for EnumerableSet.UintSet; + + constructor(address _sharedLockbox) SuperchainConfig(_sharedLockbox) { } + + function forTest_addChainOnDependencySet(uint256 _chainId) external { + _dependencySet.add(_chainId); + } + + function forTest_addChainAndSystemConfig(uint256 _chainId, address _systemConfig) external { + _dependencySet.add(_chainId); + systemConfigs[_chainId] = _systemConfig; + } + + function forTest_setGuardian(address _guardian) external { + bytes32 slot = GUARDIAN_SLOT; + assembly { + sstore(slot, _guardian) + } + } +} contract SuperchainConfig_Init_Test is CommonTest { /// @dev Tests that initialization sets the correct values. These are defined in CommonTest.sol. @@ -29,7 +55,9 @@ contract SuperchainConfig_Init_Test is CommonTest { ISuperchainConfig newImpl = ISuperchainConfig( DeployUtils.create1({ _name: "SuperchainConfig", - _args: DeployUtils.encodeConstructor(abi.encodeCall(ISuperchainConfig.__constructor__, ())) + _args: DeployUtils.encodeConstructor( + abi.encodeCall(ISuperchainConfig.__constructor__, (address(sharedLockbox))) + ) }) ); @@ -105,3 +133,186 @@ contract SuperchainConfig_Unpause_Test is CommonTest { assertFalse(superchainConfig.paused()); } } + +contract SuperchainConfig_AddChain_Test is CommonTest { + event ChainAdded(uint256 indexed chainId, address indexed systemConfig, address indexed portal); + + function _mockAndExpect(address _target, bytes memory _calldata, bytes memory _returnData) internal { + vm.mockCall(_target, _calldata, _returnData); + vm.expectCall(_target, _calldata); + } + + /// @notice Tests that `addChain` reverts when called by an unauthorized address. + function test_addChain_unauthorized_reverts(address _caller, uint256 _chainId, address _systemConfig) external { + vm.assume(_caller != superchainConfig.guardian()); + + vm.expectRevert(Unauthorized.selector); + vm.prank(_caller); + superchainConfig.addChain(_chainId, _systemConfig); + } + + /// @notice Tests that `addChain` reverts when the chain is already in the dependency set. + function test_addChain_chainAlreadyExists_reverts(uint256 _chainId, address _systemConfig) external { + SuperchainConfigForTest superchainConfig = new SuperchainConfigForTest(address(sharedLockbox)); + superchainConfig.forTest_addChainOnDependencySet(_chainId); + + vm.startPrank(superchainConfig.guardian()); + vm.expectRevert(SuperchainConfig.ChainAlreadyAdded.selector); + superchainConfig.addChain(_chainId, _systemConfig); + } + + /// @notice Tests that `addChain` successfully adds a chain to the dependency set when it is empty. + function test_addChain_onEmptyDependencySet_succeeds(uint256 _chainId, address _portal) external { + vm.assume(!superchainConfig.isInDependencySet(_chainId)); + + // Store the PORTAL address we expect to be used in a call in the SystemConfig OptimsimPortal slot, and expect + // it to be called + vm.store( + address(systemConfig), + bytes32(uint256(keccak256("systemconfig.optimismportal")) - 1), + bytes32(uint256(uint160(_portal))) + ); + vm.expectCall(address(systemConfig), abi.encodeWithSelector(ISystemConfigInterop.optimismPortal.selector)); + + // Mock and expect the call to authorize the portal on the SharedLockbox with the `_portal` address + _mockAndExpect( + address(sharedLockbox), abi.encodeWithSelector(ISharedLockbox.authorizePortal.selector, _portal), "" + ); + + // Expect the `addDependency` function call to not be called since the dependency set is empty + uint64 zeroCalls = 0; + vm.expectCall( + address(systemConfig), abi.encodeWithSelector(ISystemConfigInterop.addDependency.selector), zeroCalls + ); + + // Expect the ChainAdded event to be emitted + vm.expectEmit(address(superchainConfig)); + emit ChainAdded(_chainId, address(systemConfig), _portal); + + // Add the new chain to the dependency set + vm.startPrank(superchainConfig.guardian()); + superchainConfig.addChain(_chainId, address(systemConfig)); + + // Check that the new chain is in the dependency set + assertTrue(superchainConfig.isInDependencySet(_chainId)); + } + + /// @notice Tests that `addChain` successfully adds a chain to the dependency set when it is not empty. + /// This tests deploys a new SuperchainConfigForTest contract and mocks several calls regarding SystemConfig + /// and SharedLockbox contracts of the added chains with the purpose of reducing test complexity and making + /// it more readable on the trade-off of getting a less realistic environment -- but finally checking the + /// logic that is being tested when having multiple dependencies. + function test_addChain_withMultipleDependencies_succeeds(uint256 _chainId, address _portal) external { + vm.assume(_chainId > 3); + + // Deploy a new SuperchainConfigForTest contract and set the address(sharedLockbox) and guardian addresses + SuperchainConfigForTest superchainConfigForTest = new SuperchainConfigForTest(address(sharedLockbox)); + superchainConfigForTest.forTest_setGuardian(superchainConfig.guardian()); + + // Define the chains to be added to the dependency set + (uint256 chainIdOne, address systemConfigOne) = (1, makeAddr("SystemConfigOne")); + (uint256 chainIdTwo, address systemConfigTwo) = (2, makeAddr("SystemConfigTwo")); + (uint256 chainIdThree, address systemConfigThree) = (3, makeAddr("SystemConfigThree")); + + // Add the first three chains to the dependency set + superchainConfigForTest.forTest_addChainAndSystemConfig(chainIdOne, systemConfigOne); + superchainConfigForTest.forTest_addChainAndSystemConfig(chainIdTwo, systemConfigTwo); + superchainConfigForTest.forTest_addChainAndSystemConfig(chainIdThree, systemConfigThree); + + // Mock and expect the calls when looping through the first chain of the dependency set + _mockAndExpect( + systemConfigOne, abi.encodeWithSelector(ISystemConfigInterop.addDependency.selector, _chainId), "" + ); + _mockAndExpect( + address(systemConfig), abi.encodeWithSelector(ISystemConfigInterop.addDependency.selector, chainIdOne), "" + ); + + // Mock and expect the calls when looping through the second chain of the dependency set + _mockAndExpect( + systemConfigTwo, abi.encodeWithSelector(ISystemConfigInterop.addDependency.selector, _chainId), "" + ); + _mockAndExpect( + address(systemConfig), abi.encodeWithSelector(ISystemConfigInterop.addDependency.selector, chainIdTwo), "" + ); + + // Mock and expect the calls when looping through the third chain of the dependency set + _mockAndExpect( + systemConfigThree, abi.encodeWithSelector(ISystemConfigInterop.addDependency.selector, _chainId), "" + ); + _mockAndExpect( + address(systemConfig), abi.encodeWithSelector(ISystemConfigInterop.addDependency.selector, chainIdThree), "" + ); + + // Store the PORTAL address we expect to be used in a call in the SystemConfig's OptimsimPortal slot, and expect + // it to be called + vm.store( + address(systemConfig), + bytes32(uint256(keccak256("systemconfig.optimismportal")) - 1), + bytes32(uint256(uint160(_portal))) + ); + vm.expectCall(address(systemConfig), abi.encodeWithSelector(ISystemConfigInterop.optimismPortal.selector)); + + // Mock and expect the call to authorize the portal on the SharedLockbox with the `_portal` address + _mockAndExpect( + address(sharedLockbox), abi.encodeWithSelector(ISharedLockbox.authorizePortal.selector, _portal), "" + ); + + // Expect the ChainAdded event to be emitted + vm.expectEmit(address(superchainConfigForTest)); + emit ChainAdded(_chainId, address(systemConfig), _portal); + + // Add the new chain to the dependency set + vm.prank(superchainConfigForTest.guardian()); + superchainConfigForTest.addChain(_chainId, address(systemConfig)); + + // Check that the new chain is in the dependency set + assertTrue(superchainConfigForTest.isInDependencySet(_chainId)); + } +} + +contract SuperchainConfig_IsInDependencySet_Test is CommonTest { + /// @dev Tests that `isInDependencySet` returns false when the chain is not in the dependency set. Checking if empty + /// to ensure that should always be false. + function test_isInDependencySet_false_succeeds(uint256 _chainId) external view { + assert(superchainConfig.dependencySet().length == 0); + assertFalse(superchainConfig.isInDependencySet(_chainId)); + } + + /// @dev Tests that `isInDependencySet` returns true when the chain is in the dependency set. + function test_isInDependencySet_true_succeeds(uint256 _chainId) external { + SuperchainConfigForTest superchainConfig = new SuperchainConfigForTest(address(sharedLockbox)); + superchainConfig.forTest_addChainOnDependencySet(_chainId); + + assertTrue(superchainConfig.isInDependencySet(_chainId)); + } +} + +contract SuperchainConfig_DependencySet_Test is CommonTest { + using EnumerableSet for EnumerableSet.UintSet; + + EnumerableSet.UintSet internal chainIds; + + /// @notice Tests that the dependency set returns properly the dependencies added. + function test_dependencySet_succeeds(uint256[] calldata _chainIdsArray) public { + SuperchainConfigForTest superchainConfigForTest = new SuperchainConfigForTest(address(sharedLockbox)); + + // Ensure there are no repeated values on the input array + for (uint256 i; i < _chainIdsArray.length; i++) { + chainIds.add(_chainIdsArray[i]); + } + + // Add the dependencies to the dependency set + for (uint256 i; i < chainIds.length(); i++) { + superchainConfigForTest.forTest_addChainOnDependencySet(chainIds.at(i)); + } + + // Check that the dependency set has the same length as the dependencies + uint256[] memory dependencySet = superchainConfigForTest.dependencySet(); + assertEq(dependencySet.length, chainIds.length()); + + // Check that the dependency set has the same chain IDs as the dependencies + for (uint256 i; i < chainIds.length(); i++) { + assertEq(dependencySet[i], chainIds.at(i)); + } + } +} diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index fdf3d78b4fe1..f1fc1b55a6fa 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -413,12 +413,17 @@ contract Specification_Test is CommonTest { // SuperchainConfig _addSpec({ _name: "SuperchainConfig", _sel: _getSel("GUARDIAN_SLOT()") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("PAUSED_SLOT()") }); + _addSpec({ _name: "SuperchainConfig", _sel: _getSel("SHARED_LOCKBOX()") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("guardian()") }); + _addSpec({ _name: "SuperchainConfig", _sel: _getSel("systemConfigs(uint256)") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("initialize(address,bool)") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("pause(string)"), _auth: Role.GUARDIAN }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("paused()") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("unpause()"), _auth: Role.GUARDIAN }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("version()") }); + _addSpec({ _name: "SuperchainConfig", _sel: _getSel("addChain(uint256,address)") }); + _addSpec({ _name: "SuperchainConfig", _sel: _getSel("isInDependencySet(uint256)") }); + _addSpec({ _name: "SuperchainConfig", _sel: _getSel("dependencySet()") }); // SharedLockbox _addSpec({ _name: "SharedLockbox", _sel: _getSel("SUPERCHAIN_CONFIG()") }); From 2c7d996d7553dde03b3502d6bee54cec3f237172 Mon Sep 17 00:00:00 2001 From: Disco <131301107+0xDiscotech@users.noreply.github.com> Date: Thu, 28 Nov 2024 17:20:19 -0300 Subject: [PATCH 06/13] chore: add zero dependencies check (#142) --- .../snapshots/abi/SuperchainConfig.json | 5 +++ .../snapshots/semver-lock.json | 4 +-- .../src/L1/SuperchainConfig.sol | 6 +++- .../src/L1/interfaces/ISuperchainConfig.sol | 3 +- .../test/L1/LiquidityMigrator.t.sol | 1 + .../test/L1/SharedLockbox.t.sol | 5 +++ .../test/L1/SuperchainConfig.t.sol | 36 +++++++++++++++++-- 7 files changed, 53 insertions(+), 7 deletions(-) diff --git a/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json b/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json index 4efd65d9bbac..6d63bb81c862 100644 --- a/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json +++ b/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json @@ -276,6 +276,11 @@ "name": "ChainAlreadyAdded", "type": "error" }, + { + "inputs": [], + "name": "ChainAlreadyHasDependencies", + "type": "error" + }, { "inputs": [], "name": "Unauthorized", diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index ec6549c6ec69..b3296d0a5d7b 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -44,8 +44,8 @@ "sourceCodeHash": "0x26b7450f991fe197a94861cf618df04feb695dbbba884ffe93f96defa63e5bd7" }, "src/L1/SuperchainConfig.sol": { - "initCodeHash": "0x14e3ce78307d6709b29199de4380620003cbfe311a56b201ab2767c17e9e2091", - "sourceCodeHash": "0xff091439483e0669cf39c27d16ad0ef6c624d28829d079fe2c5c2c836a2db748" + "initCodeHash": "0x5cd6b119f03beea59e2ba29ea92694e6e5cbcbb1a83a02557ffbe332ce687ab6", + "sourceCodeHash": "0x1e191c7746b936427b8391c68a42b542eab0821c005a4a6a517bbe855278cced" }, "src/L1/SystemConfig.sol": { "initCodeHash": "0x0eda38e2fb2687a324289f04ec8ad0d2afe51f45219d074740fb4a0e24ea6569", diff --git a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol index ad811120bb61..d990ea041127 100644 --- a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol +++ b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol @@ -50,6 +50,9 @@ contract SuperchainConfig is Initializable, ISemver { /// @param portal The address of the OptimismPortal contract. event ChainAdded(uint256 indexed chainId, address indexed systemConfig, address indexed portal); + /// @notice Thrown when the input chain's system config already contains dependencies on its set. + error ChainAlreadyHasDependencies(); + /// @notice Thrown when the input chain is already added to the dependency set. error ChainAlreadyAdded(); @@ -122,10 +125,11 @@ contract SuperchainConfig is Initializable, ISemver { /// Adds the new chain as a dependency for all existing chains in the dependency set, and vice versa. It /// also stores the SystemConfig address of it, and authorizes the OptimismPortal on the SharedLockbox. /// @param _chainId The chain ID. - /// @param _systemConfig The address of the SystemConfig contract. + /// @param _systemConfig The SystemConfig contract address of the chain to add. function addChain(uint256 _chainId, address _systemConfig) external { // TODO: Updater role TBD, using guardian for now. if (msg.sender != guardian()) revert Unauthorized(); + if (ISystemConfigInterop(_systemConfig).dependencyCounter() != 0) revert ChainAlreadyHasDependencies(); // Add to the dependency set and check it is not already added (`add()` returns false if it already exists) if (!_dependencySet.add(_chainId)) revert ChainAlreadyAdded(); diff --git a/packages/contracts-bedrock/src/L1/interfaces/ISuperchainConfig.sol b/packages/contracts-bedrock/src/L1/interfaces/ISuperchainConfig.sol index 24a65f9a470c..efc0c598c5b1 100644 --- a/packages/contracts-bedrock/src/L1/interfaces/ISuperchainConfig.sol +++ b/packages/contracts-bedrock/src/L1/interfaces/ISuperchainConfig.sol @@ -15,8 +15,9 @@ interface ISuperchainConfig is IDependencySet { event Unpaused(); event ChainAdded(uint256 indexed chainId, address indexed systemConfig, address indexed portal); - error ChainAlreadyAdded(); error Unauthorized(); + error ChainAlreadyHasDependencies(); + error ChainAlreadyAdded(); function GUARDIAN_SLOT() external view returns (bytes32); function PAUSED_SLOT() external view returns (bytes32); diff --git a/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol b/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol index a06aab1e11f1..a793fab462bf 100644 --- a/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol +++ b/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol @@ -10,6 +10,7 @@ contract LiquidityMigratorTest is CommonTest { LiquidityMigrator public migrator; function setUp() public virtual override { + super.enableInterop(); super.setUp(); migrator = new LiquidityMigrator(address(sharedLockbox)); } diff --git a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol index 5a1bddcb97cc..b1203e4e36f9 100644 --- a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol +++ b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol @@ -18,6 +18,11 @@ contract SharedLockboxTest is CommonTest { event PortalAuthorized(address indexed portal); + function setUp() public virtual override { + super.enableInterop(); + super.setUp(); + } + /// @notice Tests it reverts when the caller is not an authorized portal. function test_lockETH_unauthorizedPortal_reverts(address _caller) public { vm.assume(!sharedLockbox.authorizedPortals(_caller)); diff --git a/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol b/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol index 6a39fd642071..4ac95f5f7ffe 100644 --- a/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol +++ b/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol @@ -137,6 +137,11 @@ contract SuperchainConfig_Unpause_Test is CommonTest { contract SuperchainConfig_AddChain_Test is CommonTest { event ChainAdded(uint256 indexed chainId, address indexed systemConfig, address indexed portal); + function setUp() public virtual override { + super.enableInterop(); + super.setUp(); + } + function _mockAndExpect(address _target, bytes memory _calldata, bytes memory _returnData) internal { vm.mockCall(_target, _calldata, _returnData); vm.expectCall(_target, _calldata); @@ -151,11 +156,31 @@ contract SuperchainConfig_AddChain_Test is CommonTest { superchainConfig.addChain(_chainId, _systemConfig); } + /// @notice Tests that `addChain` reverts when the input chain already contains dependencies on its set. + function test_addChain_alreadyHasDependencies_reverts(uint256 _chainId, address _systemConfig) external { + // Mock the number of dependencies to be greater than 0. + uint256 numberOfDependencies = 1; + _mockAndExpect( + _systemConfig, + abi.encodeWithSelector(ISystemConfigInterop.dependencyCounter.selector), + abi.encode(numberOfDependencies) + ); + + vm.startPrank(superchainConfig.guardian()); + vm.expectRevert(ISuperchainConfig.ChainAlreadyHasDependencies.selector); + superchainConfig.addChain(_chainId, _systemConfig); + } + /// @notice Tests that `addChain` reverts when the chain is already in the dependency set. function test_addChain_chainAlreadyExists_reverts(uint256 _chainId, address _systemConfig) external { SuperchainConfigForTest superchainConfig = new SuperchainConfigForTest(address(sharedLockbox)); superchainConfig.forTest_addChainOnDependencySet(_chainId); + // Mock the call over `dependencyCounter` to return 0 and avoid a previous revert + _mockAndExpect( + _systemConfig, abi.encodeWithSelector(ISystemConfigInterop.dependencyCounter.selector), abi.encode(0) + ); + vm.startPrank(superchainConfig.guardian()); vm.expectRevert(SuperchainConfig.ChainAlreadyAdded.selector); superchainConfig.addChain(_chainId, _systemConfig); @@ -175,9 +200,7 @@ contract SuperchainConfig_AddChain_Test is CommonTest { vm.expectCall(address(systemConfig), abi.encodeWithSelector(ISystemConfigInterop.optimismPortal.selector)); // Mock and expect the call to authorize the portal on the SharedLockbox with the `_portal` address - _mockAndExpect( - address(sharedLockbox), abi.encodeWithSelector(ISharedLockbox.authorizePortal.selector, _portal), "" - ); + vm.expectCall(address(sharedLockbox), abi.encodeWithSelector(ISharedLockbox.authorizePortal.selector, _portal)); // Expect the `addDependency` function call to not be called since the dependency set is empty uint64 zeroCalls = 0; @@ -219,6 +242,13 @@ contract SuperchainConfig_AddChain_Test is CommonTest { superchainConfigForTest.forTest_addChainAndSystemConfig(chainIdTwo, systemConfigTwo); superchainConfigForTest.forTest_addChainAndSystemConfig(chainIdThree, systemConfigThree); + // Mock and expect the call to `dependencyCounter` to return 0 for the new chain + _mockAndExpect( + address(systemConfig), + abi.encodeWithSelector(ISystemConfigInterop.dependencyCounter.selector), + abi.encode(0) + ); + // Mock and expect the calls when looping through the first chain of the dependency set _mockAndExpect( systemConfigOne, abi.encodeWithSelector(ISystemConfigInterop.addDependency.selector, _chainId), "" From 4a45ed2f905bd56d5e31fdf4e5188ac3a25a2619 Mon Sep 17 00:00:00 2001 From: agusduha Date: Thu, 28 Nov 2024 17:47:20 -0300 Subject: [PATCH 07/13] fix: pre pr --- .../contracts-bedrock/snapshots/semver-lock.json | 14 +++++++------- .../src/L1/OptimismPortalInterop.sol | 4 ++-- .../contracts-bedrock/src/L1/SuperchainConfig.sol | 4 ++-- .../src/L1/SystemConfigInterop.sol | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 0e793dcf8233..889a15db7506 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -32,8 +32,8 @@ "sourceCodeHash": "0xb7cbcb27240d0fd85a7a3009676abb93679e3b8723a967c1f310390464399869" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x831af803158768b7fc229822eebda0ce7922cb1852fd770856b425023a379d47", - "sourceCodeHash": "0xe1dbe5e95ffed6587126ea65a5e4d3c8f11e1bd17a194a5665c7f0c0d32f29e3" + "initCodeHash": "0xbcd9d2a47e2b6076d242762cc072387d9da5532b72d9f0286de2d683cd918b13", + "sourceCodeHash": "0xd3f70c69c35d05f0eec20ac7dae4d86b20066af2b2395387cc62ad80b5a27b06" }, "src/L1/ProtocolVersions.sol": { "initCodeHash": "0x98dc9a7ecd1919a8cf4bcca4fb28b112641b2aa5132fb5467a349fb80974957a", @@ -44,16 +44,16 @@ "sourceCodeHash": "0x26b7450f991fe197a94861cf618df04feb695dbbba884ffe93f96defa63e5bd7" }, "src/L1/SuperchainConfig.sol": { - "initCodeHash": "0x5cd6b119f03beea59e2ba29ea92694e6e5cbcbb1a83a02557ffbe332ce687ab6", - "sourceCodeHash": "0x1e191c7746b936427b8391c68a42b542eab0821c005a4a6a517bbe855278cced" + "initCodeHash": "0x56780fdb911f250dde8a5cfb2aae158405503cf295efd784ba9823667838bd99", + "sourceCodeHash": "0x3693d009d1d02508adec63bcb6bf2e7a6ec5f8255ddff8ffdcb792fae0cd4b84" }, "src/L1/SystemConfig.sol": { "initCodeHash": "0x0eda38e2fb2687a324289f04ec8ad0d2afe51f45219d074740fb4a0e24ea6569", "sourceCodeHash": "0x6dbbe8716ca8cd2fba3da8dcae0ca0c4b1f3e9dd04220fb24a15666b23300927" }, "src/L1/SystemConfigInterop.sol": { - "initCodeHash": "0x9376acb45613d5042ac316f3dd0fa09e7ea1f7b9e99a879b81a8cc31185ed3c7", - "sourceCodeHash": "0x40d76b65fa9074a6539de9a73f0735badb1fc1fe0a0b1b42829065ba6376b176" + "initCodeHash": "0x1d122143f63b5c00af41a246b1099ee968af04df7af394db874613d80bc20898", + "sourceCodeHash": "0x2e928783bce90409817c223e6746b74f4c164d614bcf0b0f26a1b82811882974" }, "src/L2/BaseFeeVault.sol": { "initCodeHash": "0xc23d0437deb5c2e9f8831b9325c77e9d07467c9286f81c0ef47cfef4e242f96b", @@ -227,4 +227,4 @@ "initCodeHash": "0x06ae2c0b39c215b7fa450d382916ce6f5c6f9f2d630e572db6b72d688255b3fd", "sourceCodeHash": "0xa014d9c992f439dee8221e065828c3326ca2c4f5db0e83431c64c20f7e51ec14" } -} +} \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol index 2e065049fa31..c22776ef3a11 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol @@ -26,9 +26,9 @@ contract OptimismPortalInterop is OptimismPortal2 { OptimismPortal2(_proofMaturityDelaySeconds, _disputeGameFinalityDelaySeconds, _sharedLockbox) { } - /// @custom:semver +interop-beta.3 + /// @custom:semver +interop-beta.4 function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop-beta.3"); + return string.concat(super.version(), "+interop-beta.4"); } /// @notice Sets static configuration options for the L2 system. diff --git a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol index ede2daf99199..369b48a33982 100644 --- a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol +++ b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol @@ -62,8 +62,8 @@ contract SuperchainConfig is Initializable, ISemver { error ChainAlreadyAdded(); /// @notice Semantic version. - /// @custom:semver 1.1.1-beta.2 - string public constant version = "1.1.1-beta.2"; + /// @custom:semver 1.1.1-beta.3 + string public constant version = "1.1.1-beta.3"; // Mapping from chainId to SystemConfig address mapping(uint256 => address) public systemConfigs; diff --git a/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol b/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol index 48efa6bca53d..c215404bd11c 100644 --- a/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol +++ b/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol @@ -29,9 +29,9 @@ contract SystemConfigInterop is SystemConfig { /// @notice The address of the SuperchainConfig contract. address public immutable SUPERCHAIN_CONFIG; - /// @custom:semver +interop-beta.5 + /// @custom:semver +interop-beta.6 function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop-beta.5"); + return string.concat(super.version(), "+interop-beta.6"); } /// @notice Constructs the SystemConfig contract. From 37dccafb18b3f8cae7f3403ed3baf7ce87afb953 Mon Sep 17 00:00:00 2001 From: AgusDuha <81362284+agusduha@users.noreply.github.com> Date: Fri, 29 Nov 2024 14:45:24 -0300 Subject: [PATCH 08/13] feat: Add pause check (#145) * feat: Add pause check Co-authored-by: 0xParticle Co-authored-by: gotzenx <78360669+gotzenx@users.noreply.github.com> Co-authored-by: Joxess * test: add tests natspecs --------- Co-authored-by: 0xParticle Co-authored-by: gotzenx <78360669+gotzenx@users.noreply.github.com> Co-authored-by: Joxess --- .../scripts/deploy/ChainAssertions.sol | 2 +- .../scripts/deploy/DeploySuperchain.s.sol | 4 +- .../snapshots/abi/SharedLockbox.json | 20 ++++++++- .../snapshots/semver-lock.json | 6 +-- .../src/L1/SharedLockbox.sol | 21 +++++++-- .../src/L1/SuperchainConfig.sol | 2 + .../src/L1/interfaces/ISharedLockbox.sol | 7 ++- .../src/libraries/errors/CommonErrors.sol | 3 ++ .../test/L1/SharedLockbox.t.sol | 43 ++++++++++++++++++- .../test/universal/Specs.t.sol | 1 + 10 files changed, 96 insertions(+), 13 deletions(-) diff --git a/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol b/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol index d44705520c22..060b7d7906cf 100644 --- a/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol +++ b/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol @@ -589,7 +589,7 @@ library ChainAssertions { ); require(address(sharedLockbox) != address(0), "CHECK-SLB-10"); - require(sharedLockbox.SUPERCHAIN_CONFIG() == address(superchainConfig), "CHECK-SLB-20"); + require(sharedLockbox.SUPERCHAIN_CONFIG() == superchainConfig, "CHECK-SLB-20"); } /// @dev Asserts that for a given contract the value of a storage slot at an offset is 1 or 0xff. diff --git a/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol b/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol index 55fe545411ef..da13f10bfc5d 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol @@ -310,12 +310,12 @@ contract DeploySuperchainOutput is BaseDeployIO { vm.startPrank(address(0)); require(IProxy(payable(address(sl))).implementation() == address(sharedLockboxImpl()), "SLB-10"); require(IProxy(payable(address(sl))).admin() == address(superchainProxyAdmin()), "SLB-20"); - require(sl.SUPERCHAIN_CONFIG() == address(superchainConfigProxy()), "SLB-30"); + require(sl.SUPERCHAIN_CONFIG() == superchainConfigProxy(), "SLB-30"); vm.stopPrank(); // Implementation checks. sl = sharedLockboxImpl(); - require(sl.SUPERCHAIN_CONFIG() == address(superchainConfigProxy()), "SLB-40"); + require(sl.SUPERCHAIN_CONFIG() == superchainConfigProxy(), "SLB-40"); } } diff --git a/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json b/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json index 567ae6901802..ef1961339528 100644 --- a/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json +++ b/packages/contracts-bedrock/snapshots/abi/SharedLockbox.json @@ -15,7 +15,7 @@ "name": "SUPERCHAIN_CONFIG", "outputs": [ { - "internalType": "address", + "internalType": "contract ISuperchainConfig", "name": "", "type": "address" } @@ -62,6 +62,19 @@ "stateMutability": "payable", "type": "function" }, + { + "inputs": [], + "name": "paused", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -139,6 +152,11 @@ "name": "PortalAuthorized", "type": "event" }, + { + "inputs": [], + "name": "Paused", + "type": "error" + }, { "inputs": [], "name": "Unauthorized", diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 889a15db7506..b46292eeff3e 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -40,12 +40,12 @@ "sourceCodeHash": "0x9b953cdbc049492d20a15d48a9081977a91f8e7c72eee3792d86871e70b1a40a" }, "src/L1/SharedLockbox.sol": { - "initCodeHash": "0xa6aa965a99138876c2ba0c48910068ea0d0133699a2a22e7dc867a46d9e927af", - "sourceCodeHash": "0x26b7450f991fe197a94861cf618df04feb695dbbba884ffe93f96defa63e5bd7" + "initCodeHash": "0x914d95090b8d5b37744a030cd9ac5a2b57c367d695f6a6ae3ad3aa894c599c3c", + "sourceCodeHash": "0x240277f19a6505220efccc90f36d5ab309a19a6fc96f6e53fe6d012b8c56e3ba" }, "src/L1/SuperchainConfig.sol": { "initCodeHash": "0x56780fdb911f250dde8a5cfb2aae158405503cf295efd784ba9823667838bd99", - "sourceCodeHash": "0x3693d009d1d02508adec63bcb6bf2e7a6ec5f8255ddff8ffdcb792fae0cd4b84" + "sourceCodeHash": "0x7cd295c29b65facd7846ea04898e1a1f890859eba9f5ebbf1ac8c76000cdc2e1" }, "src/L1/SystemConfig.sol": { "initCodeHash": "0x0eda38e2fb2687a324289f04ec8ad0d2afe51f45219d074740fb4a0e24ea6569", diff --git a/packages/contracts-bedrock/src/L1/SharedLockbox.sol b/packages/contracts-bedrock/src/L1/SharedLockbox.sol index af027992d8bc..119fd84c2b65 100644 --- a/packages/contracts-bedrock/src/L1/SharedLockbox.sol +++ b/packages/contracts-bedrock/src/L1/SharedLockbox.sol @@ -3,7 +3,8 @@ pragma solidity 0.8.15; import { ISemver } from "src/universal/interfaces/ISemver.sol"; import { IOptimismPortal } from "src/L1/interfaces/IOptimismPortal.sol"; -import { Unauthorized } from "src/libraries/errors/CommonErrors.sol"; +import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; +import { Unauthorized, Paused } from "src/libraries/errors/CommonErrors.sol"; /// @custom:proxied true /// @title SharedLockbox @@ -25,7 +26,7 @@ contract SharedLockbox is ISemver { event PortalAuthorized(address indexed portal); /// @notice The address of the SuperchainConfig contract. - address public immutable SUPERCHAIN_CONFIG; + ISuperchainConfig public immutable SUPERCHAIN_CONFIG; /// @notice OptimismPortals that are part of the dependency cluster authorized to interact with the SharedLockbox mapping(address => bool) public authorizedPortals; @@ -39,7 +40,17 @@ contract SharedLockbox is ISemver { /// @notice Constructs the SharedLockbox contract. /// @param _superchainConfig The address of the SuperchainConfig contract. constructor(address _superchainConfig) { - SUPERCHAIN_CONFIG = _superchainConfig; + SUPERCHAIN_CONFIG = ISuperchainConfig(_superchainConfig); + } + + /// @notice Reverts when paused. + function _whenNotPaused() internal view { + if (paused()) revert Paused(); + } + + /// @notice Getter for the current paused status. + function paused() public view returns (bool) { + return SUPERCHAIN_CONFIG.paused(); } /// @notice Locks ETH in the lockbox. @@ -53,6 +64,7 @@ contract SharedLockbox is ISemver { /// @notice Unlocks ETH from the lockbox. /// Called by an authorized portal when finalizing a withdrawal that requires ETH. function unlockETH(uint256 _value) external { + _whenNotPaused(); if (!authorizedPortals[msg.sender]) revert Unauthorized(); // Using `donateETH` to avoid triggering a deposit @@ -62,7 +74,8 @@ contract SharedLockbox is ISemver { /// @notice Authorizes a portal to interact with the lockbox. function authorizePortal(address _portal) external { - if (msg.sender != SUPERCHAIN_CONFIG) revert Unauthorized(); + _whenNotPaused(); + if (msg.sender != address(SUPERCHAIN_CONFIG)) revert Unauthorized(); authorizedPortals[_portal] = true; emit PortalAuthorized(_portal); diff --git a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol index 369b48a33982..c331d1b969d9 100644 --- a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol +++ b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol @@ -135,6 +135,7 @@ contract SuperchainConfig is Initializable, ISemver { // TODO: Updater role TBD, using guardian for now. if (msg.sender != guardian()) revert Unauthorized(); if (ISystemConfigInterop(_systemConfig).dependencyCounter() != 0) revert ChainAlreadyHasDependencies(); + // Add to the dependency set and check it is not already added (`add()` returns false if it already exists) if (!_dependencySet.add(_chainId)) revert ChainAlreadyAdded(); @@ -147,6 +148,7 @@ contract SuperchainConfig is Initializable, ISemver { // Add the new chain as dependency for the current chain on the loop ISystemConfigInterop(systemConfigs[currentId]).addDependency(_chainId); + // Add the current chain on the loop as dependency for the new chain ISystemConfigInterop(_systemConfig).addDependency(currentId); } diff --git a/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol b/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol index 76881326b9ca..e42d1062245a 100644 --- a/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol +++ b/packages/contracts-bedrock/src/L1/interfaces/ISharedLockbox.sol @@ -2,24 +2,29 @@ pragma solidity ^0.8.0; import { ISemver } from "src/universal/interfaces/ISemver.sol"; +import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; /// @title ISharedLockbox /// @notice Interface for the SharedLockbox contract interface ISharedLockbox is ISemver { error Unauthorized(); + error Paused(); + event ETHLocked(address indexed portal, uint256 amount); event ETHUnlocked(address indexed portal, uint256 amount); event PortalAuthorized(address indexed portal); - function SUPERCHAIN_CONFIG() external view returns (address); + function SUPERCHAIN_CONFIG() external view returns (ISuperchainConfig); function authorizedPortals(address) external view returns (bool); function __constructor__(address _superchainConfig) external; + function paused() external view returns (bool); + function unlockETH(uint256 _value) external; function lockETH() external payable; diff --git a/packages/contracts-bedrock/src/libraries/errors/CommonErrors.sol b/packages/contracts-bedrock/src/libraries/errors/CommonErrors.sol index 30ce96972a19..390d0a416f24 100644 --- a/packages/contracts-bedrock/src/libraries/errors/CommonErrors.sol +++ b/packages/contracts-bedrock/src/libraries/errors/CommonErrors.sol @@ -15,3 +15,6 @@ error TransferFailed(); /// @notice Thrown when attempting to perform an operation and the account is the zero address. error ZeroAddress(); + +/// @notice Thrown when a function is called while the contract is paused. +error Paused(); diff --git a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol index b1203e4e36f9..73c8369a4931 100644 --- a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol +++ b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.15; // Testing utilities import { CommonTest } from "test/setup/CommonTest.sol"; -import { Unauthorized } from "src/libraries/errors/CommonErrors.sol"; +import { Unauthorized, Paused as PausedError } from "src/libraries/errors/CommonErrors.sol"; // Targent contract import { SharedLockbox } from "src/L1/SharedLockbox.sol"; @@ -61,6 +61,20 @@ contract SharedLockboxTest is CommonTest { assertEq(address(sharedLockbox).balance, _lockboxBalanceBefore + _amount); } + /// @notice Tests `unlockETH` reverts when the contract is paused. + function test_unlockETH_paused_reverts(address _caller, uint256 _value) public { + // Set the paused status to true + vm.prank(superchainConfig.guardian()); + superchainConfig.pause("test"); + + // Expect the revert with `Paused` selector + vm.expectRevert(PausedError.selector); + + // Call the `unlockETH` function with the caller + vm.prank(_caller); + sharedLockbox.unlockETH(_value); + } + /// @notice Tests it reverts when the caller is not an authorized portal. function test_unlockETH_unauthorizedPortal_reverts(address _caller, uint256 _value) public { vm.assume(!sharedLockbox.authorizedPortals(_caller)); @@ -102,6 +116,20 @@ contract SharedLockboxTest is CommonTest { assertEq(address(sharedLockbox).balance, _lockboxBalanceBefore - _value); } + /// @notice Tests `authorizePortal` reverts when the contract is paused. + function test_authorizePortal_paused_reverts(address _caller, address _portal) public { + // Set the paused status to true + vm.prank(superchainConfig.guardian()); + superchainConfig.pause("test"); + + // Expect the revert with `Paused` selector + vm.expectRevert(PausedError.selector); + + // Call the `authorizePortal` function with the caller + vm.prank(_caller); + sharedLockbox.authorizePortal(_portal); + } + /// @notice Tests it reverts when the caller is not the SuperchainConfig. function test_authorizePortal_notSuperchainConfig_reverts(address _caller) public { vm.assume(_caller != address(superchainConfig)); @@ -131,4 +159,17 @@ contract SharedLockboxTest is CommonTest { // Assert the portal's authorized status was updated correctly assertEq(sharedLockbox.authorizedPortals(_portal), true); } + + /// @notice Tests the paused status is correctly returned. + function test_paused_succeeds() public { + // Assert the paused status is false + assertEq(sharedLockbox.paused(), false); + + // Set the paused status to true + vm.prank(superchainConfig.guardian()); + superchainConfig.pause("test"); + + // Assert the paused status is true + assertEq(sharedLockbox.paused(), true); + } } diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index f1fc1b55a6fa..a40f0ca785eb 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -432,6 +432,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "SharedLockbox", _sel: _getSel("unlockETH(uint256)"), _auth: Role.PORTAL }); _addSpec({ _name: "SharedLockbox", _sel: _getSel("authorizePortal(address)"), _auth: Role.SUPERCHAINCONFIG }); _addSpec({ _name: "SharedLockbox", _sel: _getSel("version()") }); + _addSpec({ _name: "SharedLockbox", _sel: _getSel("paused()") }); // SystemConfig _addSpec({ _name: "SystemConfig", _sel: _getSel("UNSAFE_BLOCK_SIGNER_SLOT()") }); From 036b1d537a988d21faaa6fcd6f894d46e50109e0 Mon Sep 17 00:00:00 2001 From: agusduha Date: Mon, 9 Dec 2024 14:23:05 -0300 Subject: [PATCH 09/13] fix: pre pr and interfaces imports --- .../interfaces/L1/ISuperchainConfig.sol | 4 ++-- .../snapshots/semver-lock.json | 20 +++++++++---------- .../src/L1/LiquidityMigrator.sol | 2 +- .../src/L1/OptimismPortal2.sol | 4 ++-- .../src/L1/OptimismPortalInterop.sol | 4 ++-- .../src/L1/SharedLockbox.sol | 6 +++--- .../src/L1/SuperchainConfig.sol | 4 ++-- .../src/L1/SystemConfigInterop.sol | 4 ++-- .../test/L1/SharedLockbox.t.sol | 2 +- 9 files changed, 25 insertions(+), 25 deletions(-) diff --git a/packages/contracts-bedrock/interfaces/L1/ISuperchainConfig.sol b/packages/contracts-bedrock/interfaces/L1/ISuperchainConfig.sol index efc0c598c5b1..c9fe89923c16 100644 --- a/packages/contracts-bedrock/interfaces/L1/ISuperchainConfig.sol +++ b/packages/contracts-bedrock/interfaces/L1/ISuperchainConfig.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import { IDependencySet } from "src/L2/interfaces/IDependencySet.sol"; -import { ISharedLockbox } from "src/L1/interfaces/ISharedLockbox.sol"; +import { IDependencySet } from "interfaces/L2/IDependencySet.sol"; +import { ISharedLockbox } from "interfaces/L1/ISharedLockbox.sol"; interface ISuperchainConfig is IDependencySet { enum UpdateType { diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 685eff299604..f8344f2ad79f 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -28,12 +28,12 @@ "sourceCodeHash": "0xb71e8bc24ea9ebb5692762005f2936ba2a00bf169e1e32f504a0f6e23a349a22" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0x215f439adc85bc5ebb1b234c1acc128eecb7c9a43243edbfcd9e701f128224f1", - "sourceCodeHash": "0xb7cbcb27240d0fd85a7a3009676abb93679e3b8723a967c1f310390464399869" + "initCodeHash": "0x3b7d8df1b988111d84149da579a40bc82b7fcf177cc151f5c504a143f4d342ea", + "sourceCodeHash": "0xf5fe46560360ad9bf5068c81e5a3e01b3df4dd8393e0a25f0b57a12cd51a5223" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0xbcd9d2a47e2b6076d242762cc072387d9da5532b72d9f0286de2d683cd918b13", - "sourceCodeHash": "0xd3f70c69c35d05f0eec20ac7dae4d86b20066af2b2395387cc62ad80b5a27b06" + "initCodeHash": "0x39b347c4895e627b493364ba4fce0a61659a3ddd4d2d1c5be72a430238b1c987", + "sourceCodeHash": "0x2512df44a0165e83babb347e4f66a24c668e0f37a56e653dca54d3bd08ab5fd8" }, "src/L1/ProtocolVersions.sol": { "initCodeHash": "0xb0ff1661226417342001fe9f0b64c340b7c074ff71579abf05399f4e742aaca1", @@ -41,19 +41,19 @@ }, "src/L1/SharedLockbox.sol": { "initCodeHash": "0x914d95090b8d5b37744a030cd9ac5a2b57c367d695f6a6ae3ad3aa894c599c3c", - "sourceCodeHash": "0x240277f19a6505220efccc90f36d5ab309a19a6fc96f6e53fe6d012b8c56e3ba" + "sourceCodeHash": "0xbc1e6321e241f18175c2c3c0cf898490a31ebe0dbaced1213c59621cdafd63c2" }, "src/L1/SuperchainConfig.sol": { - "initCodeHash": "0x56780fdb911f250dde8a5cfb2aae158405503cf295efd784ba9823667838bd99", - "sourceCodeHash": "0x7cd295c29b65facd7846ea04898e1a1f890859eba9f5ebbf1ac8c76000cdc2e1" + "initCodeHash": "0x09308bf7a01983bfcd69c61a6b4e0499126f8888f352a59ea7a26bf0c97bc158", + "sourceCodeHash": "0x21656a949166ee16854a4b0c67901c1b8b4a90c94c608f9d0e37e20e53f3a466" }, "src/L1/SystemConfig.sol": { "initCodeHash": "0x3ba55b46516de34186ff0cc92af9ca3ff916989ecb7d2fa9e82000f648607985", "sourceCodeHash": "0x4085b02ea01cd16172a1809ddd9be69c567f7b204cefc93f7c4d9071da812daa" }, "src/L1/SystemConfigInterop.sol": { - "initCodeHash": "0x1d122143f63b5c00af41a246b1099ee968af04df7af394db874613d80bc20898", - "sourceCodeHash": "0x2e928783bce90409817c223e6746b74f4c164d614bcf0b0f26a1b82811882974" + "initCodeHash": "0x78b8aea3f431fa5be3c003970a057b7fe1f7fa0aeb2551a10e7557575da5696b", + "sourceCodeHash": "0xf440ef535e53554f744895b89a4c985fd1dfd3b0de4d1cd4bc9216e64e9e16c0" }, "src/L2/BaseFeeVault.sol": { "initCodeHash": "0x6745b7be3895a5e8d373df0066d931bae29c47672ac46c2f5829bd0052cc6d9e", @@ -227,4 +227,4 @@ "initCodeHash": "0x2bfce526f82622288333d53ca3f43a0a94306ba1bab99241daa845f8f4b18bd4", "sourceCodeHash": "0xf49d7b0187912a6bb67926a3222ae51121e9239495213c975b3b4b217ee57a1b" } -} +} \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol b/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol index f3b8060996af..48634a460be4 100644 --- a/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol +++ b/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.15; -import { ISharedLockbox } from "./interfaces/ISharedLockbox.sol"; +import { ISharedLockbox } from "interfaces/L1/ISharedLockbox.sol"; /// @custom:proxied true /// @title LiquidityMigrator diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index f6cc46cbe65f..de9513abcfda 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -186,9 +186,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { } /// @notice Semantic version. - /// @custom:semver 3.11.0-beta.7 + /// @custom:semver 3.11.0-beta.8 function version() public pure virtual returns (string memory) { - return "3.11.0-beta.7"; + return "3.11.0-beta.8"; } /// @notice Constructs the OptimismPortal contract. diff --git a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol index 7c0fba75ee7a..375678f8ec39 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol @@ -26,9 +26,9 @@ contract OptimismPortalInterop is OptimismPortal2 { OptimismPortal2(_proofMaturityDelaySeconds, _disputeGameFinalityDelaySeconds, _sharedLockbox) { } - /// @custom:semver +interop-beta.4 + /// @custom:semver +interop-beta.5 function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop-beta.4"); + return string.concat(super.version(), "+interop-beta.5"); } /// @notice Sets static configuration options for the L2 system. diff --git a/packages/contracts-bedrock/src/L1/SharedLockbox.sol b/packages/contracts-bedrock/src/L1/SharedLockbox.sol index 119fd84c2b65..157668a33f0d 100644 --- a/packages/contracts-bedrock/src/L1/SharedLockbox.sol +++ b/packages/contracts-bedrock/src/L1/SharedLockbox.sol @@ -1,9 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.15; -import { ISemver } from "src/universal/interfaces/ISemver.sol"; -import { IOptimismPortal } from "src/L1/interfaces/IOptimismPortal.sol"; -import { ISuperchainConfig } from "src/L1/interfaces/ISuperchainConfig.sol"; +import { ISemver } from "interfaces/universal/ISemver.sol"; +import { IOptimismPortal } from "interfaces/L1/IOptimismPortal.sol"; +import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { Unauthorized, Paused } from "src/libraries/errors/CommonErrors.sol"; /// @custom:proxied true diff --git a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol index 3460a06b7432..f719da635367 100644 --- a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol +++ b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol @@ -62,8 +62,8 @@ contract SuperchainConfig is Initializable, ISemver { error ChainAlreadyAdded(); /// @notice Semantic version. - /// @custom:semver 1.1.1-beta.3 - string public constant version = "1.1.1-beta.3"; + /// @custom:semver 1.1.1-beta.4 + string public constant version = "1.1.1-beta.4"; // Mapping from chainId to SystemConfig address mapping(uint256 => address) public systemConfigs; diff --git a/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol b/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol index 35413f004dfe..875513c1fa02 100644 --- a/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol +++ b/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol @@ -29,9 +29,9 @@ contract SystemConfigInterop is SystemConfig { /// @notice The address of the SuperchainConfig contract. address public immutable SUPERCHAIN_CONFIG; - /// @custom:semver +interop-beta.6 + /// @custom:semver +interop-beta.7 function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop-beta.6"); + return string.concat(super.version(), "+interop-beta.7"); } /// @notice Constructs the SystemConfig contract. diff --git a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol index 73c8369a4931..183487a7103a 100644 --- a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol +++ b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol @@ -9,7 +9,7 @@ import { Unauthorized, Paused as PausedError } from "src/libraries/errors/Common import { SharedLockbox } from "src/L1/SharedLockbox.sol"; // Interfaces -import { IOptimismPortal } from "src/L1/interfaces/IOptimismPortal.sol"; +import { IOptimismPortal } from "interfaces/L1/IOptimismPortal.sol"; contract SharedLockboxTest is CommonTest { event ETHLocked(address indexed portal, uint256 amount); From 6f5c86d617223e4c9cec4a4f9ff35bcfc57b023f Mon Sep 17 00:00:00 2001 From: AgusDuha <81362284+agusduha@users.noreply.github.com> Date: Thu, 19 Dec 2024 15:23:36 -0300 Subject: [PATCH 10/13] feat: add upgrader role to superchain config (#163) --- .../interfaces/L1/ISuperchainConfig.sol | 4 ++- .../scripts/deploy/DeploySuperchain.s.sol | 3 +- .../snapshots/abi/SuperchainConfig.json | 31 +++++++++++++++++++ .../snapshots/semver-lock.json | 4 +-- .../src/L1/SuperchainConfig.sol | 30 +++++++++++++++--- .../test/L1/SuperchainConfig.t.sol | 29 ++++++++++------- .../test/universal/Specs.t.sol | 4 ++- .../test/vendor/Initializable.t.sol | 4 +-- 8 files changed, 85 insertions(+), 24 deletions(-) diff --git a/packages/contracts-bedrock/interfaces/L1/ISuperchainConfig.sol b/packages/contracts-bedrock/interfaces/L1/ISuperchainConfig.sol index c9fe89923c16..fab9d001ec49 100644 --- a/packages/contracts-bedrock/interfaces/L1/ISuperchainConfig.sol +++ b/packages/contracts-bedrock/interfaces/L1/ISuperchainConfig.sol @@ -21,10 +21,12 @@ interface ISuperchainConfig is IDependencySet { function GUARDIAN_SLOT() external view returns (bytes32); function PAUSED_SLOT() external view returns (bytes32); + function UPGRADER_SLOT() external view returns (bytes32); function SHARED_LOCKBOX() external view returns (ISharedLockbox); function guardian() external view returns (address guardian_); function systemConfigs(uint256) external view returns (address); - function initialize(address _guardian, bool _paused) external; + function upgrader() external view returns (address upgrader_); + function initialize(address _guardian, address _upgrader, bool _paused) external; function pause(string memory _identifier) external; function paused() external view returns (bool paused_); function unpause() external; diff --git a/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol b/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol index 4d21b6790904..52c8d05ddff2 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol @@ -449,6 +449,7 @@ contract DeploySuperchain is Script { ISuperchainConfig superchainConfigProxy; { address guardian = _dsi.guardian(); + address upgrader = _dsi.superchainProxyAdminOwner(); bool paused = _dsi.paused(); vm.startBroadcast(msg.sender); @@ -463,7 +464,7 @@ contract DeploySuperchain is Script { superchainProxyAdmin.upgradeAndCall( payable(address(superchainConfigProxy)), address(_dso.superchainConfigImpl()), - abi.encodeCall(ISuperchainConfig.initialize, (guardian, paused)) + abi.encodeCall(ISuperchainConfig.initialize, (guardian, upgrader, paused)) ); vm.stopBroadcast(); } diff --git a/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json b/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json index 6d63bb81c862..f8d349727689 100644 --- a/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json +++ b/packages/contracts-bedrock/snapshots/abi/SuperchainConfig.json @@ -49,6 +49,19 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "UPGRADER_SLOT", + "outputs": [ + { + "internalType": "bytes32", + "name": "", + "type": "bytes32" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -100,6 +113,11 @@ "name": "_guardian", "type": "address" }, + { + "internalType": "address", + "name": "_upgrader", + "type": "address" + }, { "internalType": "bool", "name": "_paused", @@ -182,6 +200,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [], + "name": "upgrader", + "outputs": [ + { + "internalType": "address", + "name": "upgrader_", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "version", diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index f8344f2ad79f..2808138e0524 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -44,8 +44,8 @@ "sourceCodeHash": "0xbc1e6321e241f18175c2c3c0cf898490a31ebe0dbaced1213c59621cdafd63c2" }, "src/L1/SuperchainConfig.sol": { - "initCodeHash": "0x09308bf7a01983bfcd69c61a6b4e0499126f8888f352a59ea7a26bf0c97bc158", - "sourceCodeHash": "0x21656a949166ee16854a4b0c67901c1b8b4a90c94c608f9d0e37e20e53f3a466" + "initCodeHash": "0x29061f3365691311a7e9d2dff66977253ab75efc572b06c90533cacd53a9fab1", + "sourceCodeHash": "0xaa77a0d78dce9de952900b756a25f194fe5c565498f50376c3735c2cd6e70f9d" }, "src/L1/SystemConfig.sol": { "initCodeHash": "0x3ba55b46516de34186ff0cc92af9ca3ff916989ecb7d2fa9e82000f648607985", diff --git a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol index f719da635367..8e7d262f9735 100644 --- a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol +++ b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol @@ -23,8 +23,10 @@ contract SuperchainConfig is Initializable, ISemver { /// @notice Enum representing different types of updates. /// @custom:value GUARDIAN Represents an update to the guardian. + /// @custom:value UPGRADER Represents an update to the upgrader. enum UpdateType { - GUARDIAN + GUARDIAN, + UPGRADER } /// @notice Whether or not the Superchain is paused. @@ -34,6 +36,10 @@ contract SuperchainConfig is Initializable, ISemver { /// It can only be modified by an upgrade. bytes32 public constant GUARDIAN_SLOT = bytes32(uint256(keccak256("superchainConfig.guardian")) - 1); + /// @notice The address of the upgrader, which can add a chain to the dependency set. + /// It can only be modified by an upgrade. + bytes32 public constant UPGRADER_SLOT = bytes32(uint256(keccak256("superchainConfig.upgrader")) - 1); + // The Shared Lockbox contract ISharedLockbox public immutable SHARED_LOCKBOX; @@ -74,14 +80,16 @@ contract SuperchainConfig is Initializable, ISemver { /// @notice Constructs the SuperchainConfig contract. constructor(address _sharedLockbox) { SHARED_LOCKBOX = ISharedLockbox(_sharedLockbox); - initialize({ _guardian: address(0), _paused: false }); + _disableInitializers(); } /// @notice Initializer. /// @param _guardian Address of the guardian, can pause the OptimismPortal. + /// @param _upgrader Address of the upgrader, can add a chain to the dependency set. /// @param _paused Initial paused status. - function initialize(address _guardian, bool _paused) public initializer { + function initialize(address _guardian, address _upgrader, bool _paused) public initializer { _setGuardian(_guardian); + _setUpgrader(_upgrader); if (_paused) { _pause("Initializer paused"); } @@ -92,6 +100,11 @@ contract SuperchainConfig is Initializable, ISemver { guardian_ = Storage.getAddress(GUARDIAN_SLOT); } + /// @notice Getter for the upgrader address. + function upgrader() public view returns (address upgrader_) { + upgrader_ = Storage.getAddress(UPGRADER_SLOT); + } + /// @notice Getter for the current paused status. function paused() public view returns (bool paused_) { paused_ = Storage.getBool(PAUSED_SLOT); @@ -126,14 +139,21 @@ contract SuperchainConfig is Initializable, ISemver { emit ConfigUpdate(UpdateType.GUARDIAN, abi.encode(_guardian)); } + /// @notice Sets the upgrader address. This is only callable during initialization, so an upgrade + /// will be required to change the upgrader. + /// @param _upgrader The new upgrader address. + function _setUpgrader(address _upgrader) internal { + Storage.setAddress(UPGRADER_SLOT, _upgrader); + emit ConfigUpdate(UpdateType.UPGRADER, abi.encode(_upgrader)); + } + /// @notice Adds a new chain to the dependency set. /// Adds the new chain as a dependency for all existing chains in the dependency set, and vice versa. It /// also stores the SystemConfig address of it, and authorizes the OptimismPortal on the SharedLockbox. /// @param _chainId The chain ID. /// @param _systemConfig The SystemConfig contract address of the chain to add. function addChain(uint256 _chainId, address _systemConfig) external { - // TODO: Updater role TBD, using guardian for now. - if (msg.sender != guardian()) revert Unauthorized(); + if (msg.sender != upgrader()) revert Unauthorized(); if (ISystemConfigInterop(_systemConfig).dependencyCounter() != 0) revert ChainAlreadyHasDependencies(); // Add to the dependency set and check it is not already added (`add()` returns false if it already exists) diff --git a/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol b/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol index 25c2eee03bca..c4b42664c371 100644 --- a/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol +++ b/packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol @@ -29,19 +29,20 @@ contract SuperchainConfigForTest is SuperchainConfig { systemConfigs[_chainId] = _systemConfig; } - function forTest_setGuardian(address _guardian) external { - bytes32 slot = GUARDIAN_SLOT; + function forTest_setUpgrader(address _upgrader) external { + bytes32 slot = UPGRADER_SLOT; assembly { - sstore(slot, _guardian) + sstore(slot, _upgrader) } } } contract SuperchainConfig_Init_Test is CommonTest { /// @dev Tests that initialization sets the correct values. These are defined in CommonTest.sol. - function test_initialize_unpaused_succeeds() external view { + function test_initialize_succeeds() external view { assertFalse(superchainConfig.paused()); assertEq(superchainConfig.guardian(), deploy.cfg().superchainConfigGuardian()); + assertEq(superchainConfig.upgrader(), deploy.cfg().finalSystemOwner()); } /// @dev Tests that it can be intialized as paused. @@ -64,11 +65,15 @@ contract SuperchainConfig_Init_Test is CommonTest { vm.startPrank(alice); newProxy.upgradeToAndCall( address(newImpl), - abi.encodeCall(ISuperchainConfig.initialize, (deploy.cfg().superchainConfigGuardian(), true)) + abi.encodeCall( + ISuperchainConfig.initialize, + (deploy.cfg().superchainConfigGuardian(), deploy.cfg().finalSystemOwner(), true) + ) ); assertTrue(ISuperchainConfig(address(newProxy)).paused()); assertEq(ISuperchainConfig(address(newProxy)).guardian(), deploy.cfg().superchainConfigGuardian()); + assertEq(ISuperchainConfig(address(newProxy)).upgrader(), deploy.cfg().finalSystemOwner()); } } @@ -149,7 +154,7 @@ contract SuperchainConfig_AddChain_Test is CommonTest { /// @notice Tests that `addChain` reverts when called by an unauthorized address. function test_addChain_unauthorized_reverts(address _caller, uint256 _chainId, address _systemConfig) external { - vm.assume(_caller != superchainConfig.guardian()); + vm.assume(_caller != superchainConfig.upgrader()); vm.expectRevert(Unauthorized.selector); vm.prank(_caller); @@ -166,7 +171,7 @@ contract SuperchainConfig_AddChain_Test is CommonTest { abi.encode(numberOfDependencies) ); - vm.startPrank(superchainConfig.guardian()); + vm.prank(superchainConfig.upgrader()); vm.expectRevert(ISuperchainConfig.ChainAlreadyHasDependencies.selector); superchainConfig.addChain(_chainId, _systemConfig); } @@ -181,7 +186,7 @@ contract SuperchainConfig_AddChain_Test is CommonTest { _systemConfig, abi.encodeWithSelector(ISystemConfigInterop.dependencyCounter.selector), abi.encode(0) ); - vm.startPrank(superchainConfig.guardian()); + vm.prank(superchainConfig.upgrader()); vm.expectRevert(SuperchainConfig.ChainAlreadyAdded.selector); superchainConfig.addChain(_chainId, _systemConfig); } @@ -213,7 +218,7 @@ contract SuperchainConfig_AddChain_Test is CommonTest { emit ChainAdded(_chainId, address(systemConfig), _portal); // Add the new chain to the dependency set - vm.startPrank(superchainConfig.guardian()); + vm.prank(superchainConfig.upgrader()); superchainConfig.addChain(_chainId, address(systemConfig)); // Check that the new chain is in the dependency set @@ -228,9 +233,9 @@ contract SuperchainConfig_AddChain_Test is CommonTest { function test_addChain_withMultipleDependencies_succeeds(uint256 _chainId, address _portal) external { vm.assume(_chainId > 3); - // Deploy a new SuperchainConfigForTest contract and set the address(sharedLockbox) and guardian addresses + // Deploy a new SuperchainConfigForTest contract and set the address(sharedLockbox) and upgrader addresses SuperchainConfigForTest superchainConfigForTest = new SuperchainConfigForTest(address(sharedLockbox)); - superchainConfigForTest.forTest_setGuardian(superchainConfig.guardian()); + superchainConfigForTest.forTest_setUpgrader(superchainConfig.upgrader()); // Define the chains to be added to the dependency set (uint256 chainIdOne, address systemConfigOne) = (1, makeAddr("SystemConfigOne")); @@ -292,7 +297,7 @@ contract SuperchainConfig_AddChain_Test is CommonTest { emit ChainAdded(_chainId, address(systemConfig), _portal); // Add the new chain to the dependency set - vm.prank(superchainConfigForTest.guardian()); + vm.prank(superchainConfigForTest.upgrader()); superchainConfigForTest.addChain(_chainId, address(systemConfig)); // Check that the new chain is in the dependency set diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index 01aaaffbe8f2..7af11fb93d42 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -412,11 +412,13 @@ contract Specification_Test is CommonTest { // SuperchainConfig _addSpec({ _name: "SuperchainConfig", _sel: _getSel("GUARDIAN_SLOT()") }); + _addSpec({ _name: "SuperchainConfig", _sel: _getSel("UPGRADER_SLOT()") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("PAUSED_SLOT()") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("SHARED_LOCKBOX()") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("guardian()") }); + _addSpec({ _name: "SuperchainConfig", _sel: _getSel("upgrader()") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("systemConfigs(uint256)") }); - _addSpec({ _name: "SuperchainConfig", _sel: _getSel("initialize(address,bool)") }); + _addSpec({ _name: "SuperchainConfig", _sel: _getSel("initialize(address,address,bool)") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("pause(string)"), _auth: Role.GUARDIAN }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("paused()") }); _addSpec({ _name: "SuperchainConfig", _sel: _getSel("unpause()"), _auth: Role.GUARDIAN }); diff --git a/packages/contracts-bedrock/test/vendor/Initializable.t.sol b/packages/contracts-bedrock/test/vendor/Initializable.t.sol index ee054b274455..739eec6d19d6 100644 --- a/packages/contracts-bedrock/test/vendor/Initializable.t.sol +++ b/packages/contracts-bedrock/test/vendor/Initializable.t.sol @@ -55,7 +55,7 @@ contract Initializer_Test is CommonTest { InitializeableContract({ name: "SuperchainConfig", target: deploy.mustGetAddress("SuperchainConfig"), - initCalldata: abi.encodeCall(superchainConfig.initialize, (address(0), false)) + initCalldata: abi.encodeCall(superchainConfig.initialize, (address(0), address(0), false)) }) ); // SuperchainConfigProxy @@ -63,7 +63,7 @@ contract Initializer_Test is CommonTest { InitializeableContract({ name: "SuperchainConfigProxy", target: address(superchainConfig), - initCalldata: abi.encodeCall(superchainConfig.initialize, (address(0), false)) + initCalldata: abi.encodeCall(superchainConfig.initialize, (address(0), address(0), false)) }) ); // L1CrossDomainMessengerImpl From 85d49dc59930a0a12a05559582731ae529618be0 Mon Sep 17 00:00:00 2001 From: AgusDuha <81362284+agusduha@users.noreply.github.com> Date: Thu, 19 Dec 2024 16:29:02 -0300 Subject: [PATCH 11/13] feat: use superchain config lockbox in portal (#164) * feat: use superchain config lockbox in portal * test: add new sharedlockbox test --- .../interfaces/L1/IOptimismPortal2.sol | 10 +++------- .../interfaces/L1/IOptimismPortalInterop.sol | 10 +++------- .../scripts/deploy/DeployImplementations.s.sol | 7 ++----- .../contracts-bedrock/snapshots/.gas-snapshot | 8 ++++---- .../snapshots/abi/OptimismPortal2.json | 7 +------ .../snapshots/abi/OptimismPortalInterop.json | 7 +------ .../snapshots/semver-lock.json | 8 ++++---- .../src/L1/OptimismPortal2.sol | 14 +++++--------- .../src/L1/OptimismPortalInterop.sol | 5 ++--- .../test/L1/OptimismPortal2.t.sol | 17 +++++++++++++++-- 10 files changed, 40 insertions(+), 53 deletions(-) diff --git a/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol b/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol index 4bdf44e66d24..b0693bcec9a7 100644 --- a/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol +++ b/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol @@ -7,6 +7,7 @@ import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol"; import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol"; import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol"; import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; +import { ISharedLockbox } from "interfaces/L1/ISharedLockbox.sol"; interface IOptimismPortal2 { error AlreadyFinalized(); @@ -69,7 +70,7 @@ interface IOptimismPortal2 { function disputeGameBlacklist(IDisputeGame) external view returns (bool); function disputeGameFactory() external view returns (IDisputeGameFactory); function disputeGameFinalityDelaySeconds() external view returns (uint256); - function sharedLockbox() external view returns (address); + function sharedLockbox() external view returns (ISharedLockbox); function donateETH() external payable; function finalizeWithdrawalTransaction(Types.WithdrawalTransaction memory _tx) external; function finalizeWithdrawalTransactionExternalProof( @@ -115,10 +116,5 @@ interface IOptimismPortal2 { function systemConfig() external view returns (ISystemConfig); function version() external pure returns (string memory); - function __constructor__( - uint256 _proofMaturityDelaySeconds, - uint256 _disputeGameFinalityDelaySeconds, - address _sharedLockbox - ) - external; + function __constructor__(uint256 _proofMaturityDelaySeconds, uint256 _disputeGameFinalityDelaySeconds) external; } diff --git a/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol b/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol index 846a6b6c273b..4201790d7878 100644 --- a/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol +++ b/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol @@ -8,6 +8,7 @@ import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol" import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol"; import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { ConfigType } from "interfaces/L2/IL1BlockInterop.sol"; +import { ISharedLockbox } from "interfaces/L1/ISharedLockbox.sol"; interface IOptimismPortalInterop { error AlreadyFinalized(); @@ -70,7 +71,7 @@ interface IOptimismPortalInterop { function disputeGameBlacklist(IDisputeGame) external view returns (bool); function disputeGameFactory() external view returns (IDisputeGameFactory); function disputeGameFinalityDelaySeconds() external view returns (uint256); - function sharedLockbox() external view returns (address); + function sharedLockbox() external view returns (ISharedLockbox); function donateETH() external payable; function finalizeWithdrawalTransaction(Types.WithdrawalTransaction memory _tx) external; function finalizeWithdrawalTransactionExternalProof( @@ -117,10 +118,5 @@ interface IOptimismPortalInterop { function systemConfig() external view returns (ISystemConfig); function version() external pure returns (string memory); - function __constructor__( - uint256 _proofMaturityDelaySeconds, - uint256 _disputeGameFinalityDelaySeconds, - address _sharedLockbox - ) - external; + function __constructor__(uint256 _proofMaturityDelaySeconds, uint256 _disputeGameFinalityDelaySeconds) external; } diff --git a/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol b/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol index 7b625ec9a250..57e8efb82a80 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeployImplementations.s.sol @@ -717,15 +717,13 @@ contract DeployImplementations is Script { } else { uint256 proofMaturityDelaySeconds = _dii.proofMaturityDelaySeconds(); uint256 disputeGameFinalityDelaySeconds = _dii.disputeGameFinalityDelaySeconds(); - address sharedLockbox = address(_dii.sharedLockboxProxy()); vm.broadcast(msg.sender); impl = IOptimismPortal2( DeployUtils.create1({ _name: "OptimismPortal2", _args: DeployUtils.encodeConstructor( abi.encodeCall( - IOptimismPortal2.__constructor__, - (proofMaturityDelaySeconds, disputeGameFinalityDelaySeconds, sharedLockbox) + IOptimismPortal2.__constructor__, (proofMaturityDelaySeconds, disputeGameFinalityDelaySeconds) ) ) }) @@ -959,7 +957,6 @@ contract DeployImplementationsInterop is DeployImplementations { } else { uint256 proofMaturityDelaySeconds = _dii.proofMaturityDelaySeconds(); uint256 disputeGameFinalityDelaySeconds = _dii.disputeGameFinalityDelaySeconds(); - address sharedLockbox = address(_dii.sharedLockboxProxy()); vm.broadcast(msg.sender); impl = IOptimismPortalInterop( DeployUtils.create1({ @@ -967,7 +964,7 @@ contract DeployImplementationsInterop is DeployImplementations { _args: DeployUtils.encodeConstructor( abi.encodeCall( IOptimismPortalInterop.__constructor__, - (proofMaturityDelaySeconds, disputeGameFinalityDelaySeconds, sharedLockbox) + (proofMaturityDelaySeconds, disputeGameFinalityDelaySeconds) ) ) }) diff --git a/packages/contracts-bedrock/snapshots/.gas-snapshot b/packages/contracts-bedrock/snapshots/.gas-snapshot index c295d6765877..e084f858d5c2 100644 --- a/packages/contracts-bedrock/snapshots/.gas-snapshot +++ b/packages/contracts-bedrock/snapshots/.gas-snapshot @@ -8,10 +8,10 @@ GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369342) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967527) GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564475) GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076690) -GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 467025) -GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512729) -GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72728) +GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 467064) +GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512768) +GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72706) GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 92973) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 68422) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 68986) -GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 155632) \ No newline at end of file +GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 155610) \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json b/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json index 5ca5e6bc7ffb..9ec54a2db545 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json @@ -10,11 +10,6 @@ "internalType": "uint256", "name": "_disputeGameFinalityDelaySeconds", "type": "uint256" - }, - { - "internalType": "address", - "name": "_sharedLockbox", - "type": "address" } ], "stateMutability": "nonpayable", @@ -653,7 +648,7 @@ "name": "sharedLockbox", "outputs": [ { - "internalType": "address", + "internalType": "contract ISharedLockbox", "name": "", "type": "address" } diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json b/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json index 8eb9ed0f74d6..59d72ab1fbc4 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json @@ -10,11 +10,6 @@ "internalType": "uint256", "name": "_disputeGameFinalityDelaySeconds", "type": "uint256" - }, - { - "internalType": "address", - "name": "_sharedLockbox", - "type": "address" } ], "stateMutability": "nonpayable", @@ -671,7 +666,7 @@ "name": "sharedLockbox", "outputs": [ { - "internalType": "address", + "internalType": "contract ISharedLockbox", "name": "", "type": "address" } diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 2808138e0524..2f4e592dbc5c 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -28,12 +28,12 @@ "sourceCodeHash": "0xb71e8bc24ea9ebb5692762005f2936ba2a00bf169e1e32f504a0f6e23a349a22" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0x3b7d8df1b988111d84149da579a40bc82b7fcf177cc151f5c504a143f4d342ea", - "sourceCodeHash": "0xf5fe46560360ad9bf5068c81e5a3e01b3df4dd8393e0a25f0b57a12cd51a5223" + "initCodeHash": "0x7829e8451790ad40c04b9a8283a76a0258e8120b4fcff7733262de43e0ee7f9c", + "sourceCodeHash": "0xfdf81493a292b649dcaaf622dd8a851df2bd456f62743682b74d99732ac3e482" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x39b347c4895e627b493364ba4fce0a61659a3ddd4d2d1c5be72a430238b1c987", - "sourceCodeHash": "0x2512df44a0165e83babb347e4f66a24c668e0f37a56e653dca54d3bd08ab5fd8" + "initCodeHash": "0x482e8c9ceb652e6f22ec714778e67bc0673334600818c162a48f31b095f5212f", + "sourceCodeHash": "0xeb7323d25dfcbeac165a27d4bb112c45ca9d04de82b75b2dba22b3b2181514ae" }, "src/L1/ProtocolVersions.sol": { "initCodeHash": "0xb0ff1661226417342001fe9f0b64c340b7c074ff71579abf05399f4e742aaca1", diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index de9513abcfda..37d544fef390 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -71,9 +71,6 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { /// finalized. uint256 internal immutable DISPUTE_GAME_FINALITY_DELAY_SECONDS; - /// @notice The Shared Lockbox contract. - ISharedLockbox internal immutable SHARED_LOCKBOX; - /// @notice Version of the deposit event. uint256 internal constant DEPOSIT_VERSION = 0; @@ -192,10 +189,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { } /// @notice Constructs the OptimismPortal contract. - constructor(uint256 _proofMaturityDelaySeconds, uint256 _disputeGameFinalityDelaySeconds, address _sharedLockbox) { + constructor(uint256 _proofMaturityDelaySeconds, uint256 _disputeGameFinalityDelaySeconds) { PROOF_MATURITY_DELAY_SECONDS = _proofMaturityDelaySeconds; DISPUTE_GAME_FINALITY_DELAY_SECONDS = _disputeGameFinalityDelaySeconds; - SHARED_LOCKBOX = ISharedLockbox(_sharedLockbox); initialize({ _disputeGameFactory: IDisputeGameFactory(address(0)), @@ -279,8 +275,8 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { } /// @notice Getter for the address of the shared lockbox. - function sharedLockbox() public view returns (address) { - return address(SHARED_LOCKBOX); + function sharedLockbox() public view returns (ISharedLockbox) { + return superchainConfig.SHARED_LOCKBOX(); } /// @notice Computes the minimum gas limit for a deposit. @@ -437,7 +433,7 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { (address token,) = gasPayingToken(); if (token == Constants.ETHER) { // Unlock and receive the ETH from the shared lockbox. - if (_tx.value != 0) SHARED_LOCKBOX.unlockETH(_tx.value); + if (_tx.value != 0) sharedLockbox().unlockETH(_tx.value); // Trigger the call to the target contract. We use a custom low level method // SafeCall.callWithMinGas to ensure two key properties @@ -570,7 +566,7 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { if (token == Constants.ETHER && msg.value != 0) { // Lock the ETH in the shared lockbox. - SHARED_LOCKBOX.lockETH{ value: msg.value }(); + sharedLockbox().lockETH{ value: msg.value }(); } _depositTransaction({ diff --git a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol index 375678f8ec39..2cd7e6ecf9b0 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol @@ -20,10 +20,9 @@ import { IL1BlockInterop, ConfigType } from "interfaces/L2/IL1BlockInterop.sol"; contract OptimismPortalInterop is OptimismPortal2 { constructor( uint256 _proofMaturityDelaySeconds, - uint256 _disputeGameFinalityDelaySeconds, - address _sharedLockbox + uint256 _disputeGameFinalityDelaySeconds ) - OptimismPortal2(_proofMaturityDelaySeconds, _disputeGameFinalityDelaySeconds, _sharedLockbox) + OptimismPortal2(_proofMaturityDelaySeconds, _disputeGameFinalityDelaySeconds) { } /// @custom:semver +interop-beta.5 diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index 8101bf8ac389..c34b3e14026b 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -55,7 +55,6 @@ contract OptimismPortal2_Test is CommonTest { assertEq(address(opImpl.superchainConfig()), address(0)); assertEq(opImpl.l2Sender(), Constants.DEFAULT_L2_SENDER); assertEq(opImpl.respectedGameType().raw(), deploy.cfg().respectedGameType()); - assertEq(opImpl.sharedLockbox(), address(sharedLockbox)); } /// @dev Tests that the initializer sets the correct values. @@ -70,7 +69,7 @@ contract OptimismPortal2_Test is CommonTest { assertEq(optimismPortal2.l2Sender(), Constants.DEFAULT_L2_SENDER); assertEq(optimismPortal2.paused(), false); assertEq(optimismPortal2.respectedGameType().raw(), deploy.cfg().respectedGameType()); - assertEq(optimismPortal2.sharedLockbox(), address(sharedLockbox)); + assertEq(address(optimismPortal2.sharedLockbox()), address(sharedLockbox)); } /// @dev Tests that `pause` successfully pauses @@ -134,6 +133,20 @@ contract OptimismPortal2_Test is CommonTest { assertEq(optimismPortal2.paused(), true); } + /// @dev Tests that `sharedLockbox` returns correctly + function testFuzz_sharedLockbox_succeeds(address _caller, address _lockbox) external { + // Mock and expect the SuperchainConfig's SharedLockbox + vm.mockCall( + address(superchainConfig), abi.encodeCall(superchainConfig.SHARED_LOCKBOX, ()), abi.encode(_lockbox) + ); + vm.expectCall(address(superchainConfig), 0, abi.encodeCall(superchainConfig.SHARED_LOCKBOX, ())); + + vm.prank(_caller); + address _result = address(optimismPortal2.sharedLockbox()); + + assertEq(_result, _lockbox); + } + /// @dev Tests that `receive` successdully deposits ETH. function testFuzz_receive_succeeds(uint256 _value) external { vm.expectEmit(address(optimismPortal2)); From 657c97b92d0bc2fd03becf71bcac38a550b0aa94 Mon Sep 17 00:00:00 2001 From: agusduha Date: Thu, 19 Dec 2024 17:16:14 -0300 Subject: [PATCH 12/13] fix: pre pr --- .../contracts-bedrock/snapshots/.gas-snapshot | 24 +++++++++---------- .../snapshots/semver-lock.json | 18 +++++++------- .../src/L1/OptimismPortal2.sol | 4 ++-- .../src/L1/OptimismPortalInterop.sol | 4 ++-- .../src/L1/SuperchainConfig.sol | 4 ++-- .../src/L1/SystemConfigInterop.sol | 4 ++-- .../test/L1/OptimismPortal2.t.sol | 6 ++--- .../test/L1/SharedLockbox.t.sol | 3 --- .../test/opcm/SetDisputeGameImpl.t.sol | 4 ++-- 9 files changed, 34 insertions(+), 37 deletions(-) diff --git a/packages/contracts-bedrock/snapshots/.gas-snapshot b/packages/contracts-bedrock/snapshots/.gas-snapshot index 83a74f162f4e..165a2cfd60d7 100644 --- a/packages/contracts-bedrock/snapshots/.gas-snapshot +++ b/packages/contracts-bedrock/snapshots/.gas-snapshot @@ -1,17 +1,17 @@ -GasBenchMark_L1BlockInterop_DepositsComplete:test_depositsComplete_benchmark() (gas: 7567) -GasBenchMark_L1BlockInterop_DepositsComplete_Warm:test_depositsComplete_benchmark() (gas: 5567) -GasBenchMark_L1BlockInterop_SetValuesInterop:test_setL1BlockValuesInterop_benchmark() (gas: 175677) -GasBenchMark_L1BlockInterop_SetValuesInterop_Warm:test_setL1BlockValuesInterop_benchmark() (gas: 5099) -GasBenchMark_L1Block_SetValuesEcotone:test_setL1BlockValuesEcotone_benchmark() (gas: 158531) -GasBenchMark_L1Block_SetValuesEcotone_Warm:test_setL1BlockValuesEcotone_benchmark() (gas: 7597) -GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369342) -GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967527) +GasBenchMark_L1BlockInterop_DepositsComplete:test_depositsComplete_benchmark() (gas: 7589) +GasBenchMark_L1BlockInterop_DepositsComplete_Warm:test_depositsComplete_benchmark() (gas: 5589) +GasBenchMark_L1BlockInterop_SetValuesInterop:test_setL1BlockValuesInterop_benchmark() (gas: 175722) +GasBenchMark_L1BlockInterop_SetValuesInterop_Warm:test_setL1BlockValuesInterop_benchmark() (gas: 5144) +GasBenchMark_L1Block_SetValuesEcotone:test_setL1BlockValuesEcotone_benchmark() (gas: 158553) +GasBenchMark_L1Block_SetValuesEcotone_Warm:test_setL1BlockValuesEcotone_benchmark() (gas: 7619) +GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369297) +GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967504) GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564475) -GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076690) +GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076645) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 467064) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512768) -GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72706) +GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72661) GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 92973) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 68422) -GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 68986) -GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 155610) +GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 69008) +GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 155610) \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 234596de6c92..87f3d7e28d94 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -28,12 +28,12 @@ "sourceCodeHash": "0x77e0f0f8caa9742896a3e7cdee15b0061e298f9c950159bb7231b8196517c9d2" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0x7829e8451790ad40c04b9a8283a76a0258e8120b4fcff7733262de43e0ee7f9c", - "sourceCodeHash": "0xfdf81493a292b649dcaaf622dd8a851df2bd456f62743682b74d99732ac3e482" + "initCodeHash": "0xf2b48d356afe813c35e3084bba81de2cf7d392d7fd9bb5227fc9a2c9514a196d", + "sourceCodeHash": "0xa0f411c08b04df19153908c8d459af0a708adba5062d3efa74c0398998b68a07" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x482e8c9ceb652e6f22ec714778e67bc0673334600818c162a48f31b095f5212f", - "sourceCodeHash": "0xeb7323d25dfcbeac165a27d4bb112c45ca9d04de82b75b2dba22b3b2181514ae" + "initCodeHash": "0xd122f99c1d0ae0377063169b15ad4755a675f9752d9039cae6862d356d7df3f8", + "sourceCodeHash": "0xae2fbe02c0f8685692babeed0252ae8a624dc6d3bfb082fc3807d7b84869004b" }, "src/L1/ProtocolVersions.sol": { "initCodeHash": "0x0000ec89712d8b4609873f1ba76afffd4205bf9110818995c90134dbec12e91e", @@ -44,16 +44,16 @@ "sourceCodeHash": "0xbc1e6321e241f18175c2c3c0cf898490a31ebe0dbaced1213c59621cdafd63c2" }, "src/L1/SuperchainConfig.sol": { - "initCodeHash": "0x29061f3365691311a7e9d2dff66977253ab75efc572b06c90533cacd53a9fab1", - "sourceCodeHash": "0xaa77a0d78dce9de952900b756a25f194fe5c565498f50376c3735c2cd6e70f9d" + "initCodeHash": "0x291c5a1696d6eb2a7d53f59a547d642ed6b41a4977d1e995375dfcb758151055", + "sourceCodeHash": "0x60fef6bd049c7ec6527d87319a12790561924f7d4355c5e2c3f5b06227b63b8e" }, "src/L1/SystemConfig.sol": { "initCodeHash": "0x11707ca5ef2bfc0da82e945cb97e62226852d850e8bdecb8ec22fc1ab7967894", "sourceCodeHash": "0x71cedc36b538c3db2ac43d1d9554583a3f09eee9e9a5c5d39608418bd466ee0e" }, "src/L1/SystemConfigInterop.sol": { - "initCodeHash": "0x78b8aea3f431fa5be3c003970a057b7fe1f7fa0aeb2551a10e7557575da5696b", - "sourceCodeHash": "0xf440ef535e53554f744895b89a4c985fd1dfd3b0de4d1cd4bc9216e64e9e16c0" + "initCodeHash": "0xd03b97cd34c51d95e246678303f6f870ef39495af39d888915cbd31bf58cc5e3", + "sourceCodeHash": "0x49636aeaaf48925cf6dcfce6f55f4949fc35a9c873faed1fbd30ded1841a4da1" }, "src/L2/BaseFeeVault.sol": { "initCodeHash": "0xc403d4c555d8e69a2699e01d192ae7327136701fa02da10a6d75a584b3c364c9", @@ -231,4 +231,4 @@ "initCodeHash": "0x2bfce526f82622288333d53ca3f43a0a94306ba1bab99241daa845f8f4b18bd4", "sourceCodeHash": "0xf49d7b0187912a6bb67926a3222ae51121e9239495213c975b3b4b217ee57a1b" } -} +} \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index 63177e712baf..a42afe3e79b8 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -183,9 +183,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { } /// @notice Semantic version. - /// @custom:semver 3.11.0-beta.9 + /// @custom:semver 3.11.0-beta.10 function version() public pure virtual returns (string memory) { - return "3.11.0-beta.9"; + return "3.11.0-beta.10"; } /// @notice Constructs the OptimismPortal contract. diff --git a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol index fecd4dc0d6b2..811c7136191e 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol @@ -25,9 +25,9 @@ contract OptimismPortalInterop is OptimismPortal2 { OptimismPortal2(_proofMaturityDelaySeconds, _disputeGameFinalityDelaySeconds) { } - /// @custom:semver +interop-beta.6 + /// @custom:semver +interop-beta.7 function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop-beta.6"); + return string.concat(super.version(), "+interop-beta.7"); } /// @notice Sets static configuration options for the L2 system. diff --git a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol index ab9ea3094ff7..ee85b8e144c9 100644 --- a/packages/contracts-bedrock/src/L1/SuperchainConfig.sol +++ b/packages/contracts-bedrock/src/L1/SuperchainConfig.sol @@ -68,8 +68,8 @@ contract SuperchainConfig is Initializable, ISemver { error ChainAlreadyAdded(); /// @notice Semantic version. - /// @custom:semver 1.1.1-beta.4 - string public constant version = "1.1.1-beta.4"; + /// @custom:semver 1.1.1-beta.5 + string public constant version = "1.1.1-beta.5"; // Mapping from chainId to SystemConfig address mapping(uint256 => address) public systemConfigs; diff --git a/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol b/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol index 875513c1fa02..e39a6ae6f386 100644 --- a/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol +++ b/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol @@ -29,9 +29,9 @@ contract SystemConfigInterop is SystemConfig { /// @notice The address of the SuperchainConfig contract. address public immutable SUPERCHAIN_CONFIG; - /// @custom:semver +interop-beta.7 + /// @custom:semver +interop-beta.8 function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop-beta.7"); + return string.concat(super.version(), "+interop-beta.8"); } /// @notice Constructs the SystemConfig contract. diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index 04ed8de501e8..971165cbf01e 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -162,7 +162,7 @@ contract OptimismPortal2_Test is CommonTest { uint256 portalBalanceBefore = address(optimismPortal2).balance; uint256 lockboxBalanceBefore = address(sharedLockbox).balance; - _value = bound(_value, 0, type(uint256).max - balanceBefore); + _value = bound(_value, 0, type(uint256).max - lockboxBalanceBefore); vm.expectEmit(address(optimismPortal2)); emitTransactionDeposited({ @@ -266,7 +266,7 @@ contract OptimismPortal2_Test is CommonTest { uint256 portalBalanceBefore = address(optimismPortal2).balance; uint256 lockboxBalanceBefore = address(sharedLockbox).balance; - _mint = bound(_mint, 0, type(uint256).max - balanceBefore); + _mint = bound(_mint, 0, type(uint256).max - lockboxBalanceBefore); // EOA emulation vm.expectEmit(address(optimismPortal2)); @@ -319,7 +319,7 @@ contract OptimismPortal2_Test is CommonTest { uint256 portalBalanceBefore = address(optimismPortal2).balance; uint256 lockboxBalanceBefore = address(sharedLockbox).balance; - _mint = bound(_mint, 0, type(uint256).max - balanceBefore); + _mint = bound(_mint, 0, type(uint256).max - lockboxBalanceBefore); vm.expectEmit(address(optimismPortal2)); emitTransactionDeposited({ diff --git a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol index 183487a7103a..e76a3f142fcf 100644 --- a/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol +++ b/packages/contracts-bedrock/test/L1/SharedLockbox.t.sol @@ -5,9 +5,6 @@ pragma solidity 0.8.15; import { CommonTest } from "test/setup/CommonTest.sol"; import { Unauthorized, Paused as PausedError } from "src/libraries/errors/CommonErrors.sol"; -// Targent contract -import { SharedLockbox } from "src/L1/SharedLockbox.sol"; - // Interfaces import { IOptimismPortal } from "interfaces/L1/IOptimismPortal.sol"; diff --git a/packages/contracts-bedrock/test/opcm/SetDisputeGameImpl.t.sol b/packages/contracts-bedrock/test/opcm/SetDisputeGameImpl.t.sol index 2dca24b486bd..72f7fbdaab9e 100644 --- a/packages/contracts-bedrock/test/opcm/SetDisputeGameImpl.t.sol +++ b/packages/contracts-bedrock/test/opcm/SetDisputeGameImpl.t.sol @@ -79,12 +79,12 @@ contract SetDisputeGameImpl_Test is Test { input = new SetDisputeGameImplInput(); DisputeGameFactory dgfImpl = new DisputeGameFactory(); OptimismPortal2 portalImpl = new OptimismPortal2(0, 0); - SuperchainConfig supConfigImpl = new SuperchainConfig(); + SuperchainConfig supConfigImpl = new SuperchainConfig(address(0)); Proxy supConfigProxy = new Proxy(address(1)); vm.prank(address(1)); supConfigProxy.upgradeToAndCall( - address(supConfigImpl), abi.encodeCall(supConfigImpl.initialize, (address(this), false)) + address(supConfigImpl), abi.encodeCall(supConfigImpl.initialize, (address(this), address(this), false)) ); Proxy factoryProxy = new Proxy(address(1)); From 3b9eba9d85ee30b28ca3637c45c5266eb71d1d9a Mon Sep 17 00:00:00 2001 From: AgusDuha <81362284+agusduha@users.noreply.github.com> Date: Tue, 24 Dec 2024 16:04:28 -0300 Subject: [PATCH 13/13] feat: liquidity migrator deployment (#166) * feat: liquidity migrator deployment * test: fix comment * test: fix internal variables names --- .../interfaces/L1/ILiquidityMigrator.sol | 6 +++ .../scripts/deploy/ChainAssertions.sol | 8 +++ .../scripts/deploy/Deploy.s.sol | 7 +++ .../scripts/deploy/DeploySuperchain.s.sol | 34 ++++++++++-- .../contracts-bedrock/snapshots/.gas-snapshot | 4 +- .../snapshots/abi/LiquidityMigrator.json | 26 +++++++++ .../snapshots/semver-lock.json | 4 ++ .../src/L1/LiquidityMigrator.sol | 10 +++- .../test/L1/LiquidityMigrator.t.sol | 54 ++++++++++++++----- .../test/opcm/DeploySuperchain.t.sol | 12 +++++ .../contracts-bedrock/test/setup/Setup.sol | 4 ++ .../test/universal/Specs.t.sol | 2 + 12 files changed, 151 insertions(+), 20 deletions(-) diff --git a/packages/contracts-bedrock/interfaces/L1/ILiquidityMigrator.sol b/packages/contracts-bedrock/interfaces/L1/ILiquidityMigrator.sol index 5c3cd7b03a92..f66b142bfdc6 100644 --- a/packages/contracts-bedrock/interfaces/L1/ILiquidityMigrator.sol +++ b/packages/contracts-bedrock/interfaces/L1/ILiquidityMigrator.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +import { ISharedLockbox } from "interfaces/L1/ISharedLockbox.sol"; + /// @title ILiquidityMigrator /// @notice Interface for the LiquidityMigrator contract interface ILiquidityMigrator { @@ -8,5 +10,9 @@ interface ILiquidityMigrator { function __constructor__(address _sharedLockbox) external; + function SHARED_LOCKBOX() external view returns (ISharedLockbox); + function migrateETH() external; + + function version() external view returns (string memory); } diff --git a/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol b/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol index d9912bde6ed6..fc8cd599bf51 100644 --- a/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol +++ b/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol @@ -24,6 +24,7 @@ import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol"; import { IL2OutputOracle } from "interfaces/L1/IL2OutputOracle.sol"; import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { ISharedLockbox } from "interfaces/L1/ISharedLockbox.sol"; +import { ILiquidityMigrator } from "interfaces/L1/ILiquidityMigrator.sol"; import { IL1CrossDomainMessenger } from "interfaces/L1/IL1CrossDomainMessenger.sol"; import { IOptimismPortal } from "interfaces/L1/IOptimismPortal.sol"; import { IOptimismPortal2 } from "interfaces/L1/IOptimismPortal2.sol"; @@ -601,4 +602,11 @@ library ChainAssertions { require(address(sharedLockbox) != address(0), "CHECK-SLB-10"); require(sharedLockbox.SUPERCHAIN_CONFIG() == superchainConfig, "CHECK-SLB-20"); } + + /// @notice Asserts that the LiquidityMigrator is setup correctly + function checkLiquidityMigrator(Types.ContractSet memory _contracts, address _liquidityMigrator) internal view { + ISharedLockbox sharedLockbox = ISharedLockbox(_contracts.SharedLockbox); + + require(ILiquidityMigrator(_liquidityMigrator).SHARED_LOCKBOX() == sharedLockbox, "LM-10"); + } } diff --git a/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol b/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol index b9f86088d4ef..223421aaca64 100644 --- a/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol @@ -296,6 +296,7 @@ contract Deploy is Deployer { save("ProtocolVersions", address(dso.protocolVersionsImpl())); save("SharedLockboxProxy", address(dso.sharedLockboxProxy())); save("SharedLockbox", address(dso.sharedLockboxImpl())); + save("LiquidityMigrator", address(dso.liquidityMigratorImpl())); // First run assertions for the ProtocolVersions, SuperchainConfig and SharedLockbox proxy contracts. Types.ContractSet memory contracts = _proxies(); @@ -303,6 +304,12 @@ contract Deploy is Deployer { ChainAssertions.checkSuperchainConfig({ _contracts: contracts, _cfg: cfg, _isProxy: true, _isPaused: false }); ChainAssertions.checkSharedLockbox({ _contracts: contracts, _isProxy: true }); + // Test the LiquidityMigrator contract is setup correctly. + ChainAssertions.checkLiquidityMigrator({ + _contracts: contracts, + _liquidityMigrator: mustGetAddress("LiquidityMigrator") + }); + // Then replace the SharedLockbox proxy with the implementation address and run assertions on it. contracts.SharedLockbox = mustGetAddress("SharedLockbox"); ChainAssertions.checkSharedLockbox({ _contracts: contracts, _isProxy: false }); diff --git a/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol b/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol index d2f33c034103..fcb2b5005d6a 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeploySuperchain.s.sol @@ -9,6 +9,7 @@ import { IProtocolVersions, ProtocolVersion } from "interfaces/L1/IProtocolVersi import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol"; import { IProxy } from "interfaces/universal/IProxy.sol"; import { ISharedLockbox } from "interfaces/L1/ISharedLockbox.sol"; +import { ILiquidityMigrator } from "interfaces/L1/ILiquidityMigrator.sol"; import { DeployUtils } from "scripts/libraries/DeployUtils.sol"; import { Solarray } from "scripts/libraries/Solarray.sol"; @@ -165,6 +166,7 @@ contract DeploySuperchainOutput is BaseDeployIO { IProxyAdmin internal _superchainProxyAdmin; ISharedLockbox internal _sharedLockboxImpl; ISharedLockbox internal _sharedLockboxProxy; + ILiquidityMigrator internal _liquidityMigratorImpl; // This method lets each field be set individually. The selector of an output's getter method // is used to determine which field to set. @@ -177,6 +179,7 @@ contract DeploySuperchainOutput is BaseDeployIO { else if (_sel == this.protocolVersionsProxy.selector) _protocolVersionsProxy = IProtocolVersions(_address); else if (_sel == this.sharedLockboxImpl.selector) _sharedLockboxImpl = ISharedLockbox(_address); else if (_sel == this.sharedLockboxProxy.selector) _sharedLockboxProxy = ISharedLockbox(_address); + else if (_sel == this.liquidityMigratorImpl.selector) _liquidityMigratorImpl = ILiquidityMigrator(_address); else revert("DeploySuperchainOutput: unknown selector"); } @@ -190,7 +193,8 @@ contract DeploySuperchainOutput is BaseDeployIO { address(this.protocolVersionsImpl()), address(this.protocolVersionsProxy()), address(this.sharedLockboxImpl()), - address(this.sharedLockboxProxy()) + address(this.sharedLockboxProxy()), + address(this.liquidityMigratorImpl()) ); DeployUtils.assertValidContractAddresses(addrs); @@ -246,12 +250,18 @@ contract DeploySuperchainOutput is BaseDeployIO { return _sharedLockboxProxy; } + function liquidityMigratorImpl() public view returns (ILiquidityMigrator) { + DeployUtils.assertValidContractAddress(address(_liquidityMigratorImpl)); + return _liquidityMigratorImpl; + } + // -------- Deployment Assertions -------- function assertValidDeploy(DeploySuperchainInput _dsi) public { assertValidSuperchainProxyAdmin(_dsi); assertValidSuperchainConfig(_dsi); assertValidProtocolVersions(_dsi); assertValidSharedLockbox(); + assertValidLiquidityMigrator(); } function assertValidSuperchainProxyAdmin(DeploySuperchainInput _dsi) internal view { @@ -322,6 +332,12 @@ contract DeploySuperchainOutput is BaseDeployIO { sl = sharedLockboxImpl(); require(sl.SUPERCHAIN_CONFIG() == superchainConfigProxy(), "SLB-40"); } + + function assertValidLiquidityMigrator() internal view { + // Implementation checks. + ILiquidityMigrator lm = liquidityMigratorImpl(); + require(lm.SHARED_LOCKBOX() == sharedLockboxProxy(), "LM-10"); + } } // For all broadcasts in this script we explicitly specify the deployer as `msg.sender` because for @@ -383,8 +399,8 @@ contract DeploySuperchain is Script { function deploySuperchain(DeploySuperchainInput _dsi, DeploySuperchainOutput _dso) public { // Precalculate the proxies addresses. Needed since there are circular dependencies between them. PrecalculatedAddresses memory precalculatedAddresses; - precalculatedAddresses.superchainConfigProxy = vm.computeCreateAddress(msg.sender, vm.getNonce(msg.sender) + 3); - precalculatedAddresses.sharedLockboxProxy = vm.computeCreateAddress(msg.sender, vm.getNonce(msg.sender) + 7); + precalculatedAddresses.superchainConfigProxy = vm.computeCreateAddress(msg.sender, vm.getNonce(msg.sender) + 4); + precalculatedAddresses.sharedLockboxProxy = vm.computeCreateAddress(msg.sender, vm.getNonce(msg.sender) + 8); // Deploy implementation contracts deploySuperchainImplementationContracts(_dsi, _dso, precalculatedAddresses); @@ -430,15 +446,27 @@ contract DeploySuperchain is Script { }) ); + // Deploy LiquidityMigrator implementation + ILiquidityMigrator liquidityMigratorImpl = ILiquidityMigrator( + DeployUtils.create1({ + _name: "LiquidityMigrator", + _args: DeployUtils.encodeConstructor( + abi.encodeCall(ILiquidityMigrator.__constructor__, (_precalculatedAddresses.sharedLockboxProxy)) + ) + }) + ); + vm.stopBroadcast(); vm.label(address(superchainConfigImpl), "SuperchainConfigImpl"); vm.label(address(protocolVersionsImpl), "ProtocolVersionsImpl"); vm.label(address(sharedLockboxImpl), "SharedLockboxImpl"); + vm.label(address(liquidityMigratorImpl), "LiquidityMigratorImpl"); _dso.set(_dso.superchainConfigImpl.selector, address(superchainConfigImpl)); _dso.set(_dso.protocolVersionsImpl.selector, address(protocolVersionsImpl)); _dso.set(_dso.sharedLockboxImpl.selector, address(sharedLockboxImpl)); + _dso.set(_dso.liquidityMigratorImpl.selector, address(liquidityMigratorImpl)); } function deployAndInitializeSuperchainProxyContracts( diff --git a/packages/contracts-bedrock/snapshots/.gas-snapshot b/packages/contracts-bedrock/snapshots/.gas-snapshot index 165a2cfd60d7..8e8ff71d5952 100644 --- a/packages/contracts-bedrock/snapshots/.gas-snapshot +++ b/packages/contracts-bedrock/snapshots/.gas-snapshot @@ -6,8 +6,8 @@ GasBenchMark_L1Block_SetValuesEcotone:test_setL1BlockValuesEcotone_benchmark() ( GasBenchMark_L1Block_SetValuesEcotone_Warm:test_setL1BlockValuesEcotone_benchmark() (gas: 7619) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369297) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967504) -GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564475) -GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076645) +GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564460) +GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076630) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 467064) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512768) GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72661) diff --git a/packages/contracts-bedrock/snapshots/abi/LiquidityMigrator.json b/packages/contracts-bedrock/snapshots/abi/LiquidityMigrator.json index a96a61f9ea48..c1c723a90262 100644 --- a/packages/contracts-bedrock/snapshots/abi/LiquidityMigrator.json +++ b/packages/contracts-bedrock/snapshots/abi/LiquidityMigrator.json @@ -10,6 +10,19 @@ "stateMutability": "nonpayable", "type": "constructor" }, + { + "inputs": [], + "name": "SHARED_LOCKBOX", + "outputs": [ + { + "internalType": "contract ISharedLockbox", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "migrateETH", @@ -17,6 +30,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [], + "name": "version", + "outputs": [ + { + "internalType": "string", + "name": "", + "type": "string" + } + ], + "stateMutability": "view", + "type": "function" + }, { "anonymous": false, "inputs": [ diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 87f3d7e28d94..4606ba37fc16 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -19,6 +19,10 @@ "initCodeHash": "0xd992c45b8461b9546fe4e3cecbce15d17ce366a62aab17058aad3b15bf36d21d", "sourceCodeHash": "0xa35478e9e2659a320da725a117b200dea2826175d2b17d881de1196da0cc91eb" }, + "src/L1/LiquidityMigrator.sol": { + "initCodeHash": "0x708f764a2de821caa3d520c93f1951e24128b136a5c41b06e2b1444a1a34e2e8", + "sourceCodeHash": "0x4f719e707583e2b23b9fcbd6e70935df099f45ac0efc50d1156051609bc26f69" + }, "src/L1/OPContractsManager.sol": { "initCodeHash": "0x9b704574a7005dc2aa8d6a3e0d85572493cc4bbd60033a23e437632a5fef7720", "sourceCodeHash": "0x05ed7ad68e4e9bca7334314e794a1f66e5899532bb01cfa3a7716cb2688df9d5" diff --git a/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol b/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol index 48634a460be4..37c3326c7f1c 100644 --- a/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol +++ b/packages/contracts-bedrock/src/L1/LiquidityMigrator.sol @@ -1,19 +1,25 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.15; +// Interfaces +import { ISemver } from "interfaces/universal/ISemver.sol"; import { ISharedLockbox } from "interfaces/L1/ISharedLockbox.sol"; /// @custom:proxied true /// @title LiquidityMigrator /// @notice A contract to migrate the OptimisPortal's ETH balance to the SharedLockbox. One-time use logic, executed in /// a batch of transactions to enable the SharedLockbox interaction within the OptimismPortal. -contract LiquidityMigrator { +contract LiquidityMigrator is ISemver { /// @notice Emitted when the contract's ETH balance is migrated to the SharedLockbox. /// @param amount The amount corresponding to the contract's ETH balance migrated. event ETHMigrated(uint256 amount); /// @notice The SharedLockbox contract. - ISharedLockbox internal immutable SHARED_LOCKBOX; + ISharedLockbox public immutable SHARED_LOCKBOX; + + /// @notice Semantic version. + /// @custom:semver 1.0.0-beta.1 + string public constant version = "1.0.0-beta.1"; /// @notice Constructs the LiquidityMigrator contract. /// @param _sharedLockbox The address of the SharedLockbox contract. diff --git a/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol b/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol index a793fab462bf..44d2d7ea8051 100644 --- a/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol +++ b/packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol @@ -3,38 +3,66 @@ pragma solidity 0.8.15; import { CommonTest } from "test/setup/CommonTest.sol"; import { LiquidityMigrator } from "src/L1/LiquidityMigrator.sol"; +import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol"; contract LiquidityMigratorTest is CommonTest { event ETHMigrated(uint256 amount); - LiquidityMigrator public migrator; - function setUp() public virtual override { super.enableInterop(); super.setUp(); - migrator = new LiquidityMigrator(address(sharedLockbox)); } /// @notice Tests the migration of the contract's ETH balance to the SharedLockbox works properly. function test_migrateETH_succeeds(uint256 _ethAmount) public { - vm.deal(address(migrator), _ethAmount); + vm.deal(address(liquidityMigrator), _ethAmount); // Get the balance of the migrator before the migration to compare later on the assertions - uint256 _migratorEthBalance = address(migrator).balance; - uint256 _lockboxBalanceBefore = address(sharedLockbox).balance; - - // Look for the emit of the `ETHMigrated` event - emit ETHMigrated(_migratorEthBalance); + uint256 migratorEthBalance = address(liquidityMigrator).balance; + uint256 lockboxBalanceBefore = address(sharedLockbox).balance; // Set the migrator as an authorized portal so it can lock the ETH while migrating vm.prank(address(superchainConfig)); - sharedLockbox.authorizePortal(address(migrator)); + sharedLockbox.authorizePortal(address(liquidityMigrator)); + + // Look for the emit of the `ETHMigrated` event + vm.expectEmit(address(liquidityMigrator)); + emit ETHMigrated(migratorEthBalance); // Call the `migrateETH` function with the amount - migrator.migrateETH(); + liquidityMigrator.migrateETH(); + + // Assert the balances after the migration happened + assert(address(liquidityMigrator).balance == 0); + assert(address(sharedLockbox).balance == lockboxBalanceBefore + migratorEthBalance); + } + + /// @notice Tests the migration of the portal's ETH balance to the SharedLockbox works properly. + function test_portal_migrateETH_succeeds(uint256 _ethAmount) public { + vm.deal(address(optimismPortal2), _ethAmount); + + // Get the balance of the portal before the migration to compare later on the assertions + uint256 portalEthBalance = address(optimismPortal2).balance; + uint256 lockboxBalanceBefore = address(sharedLockbox).balance; + + // Get the proxy admin address and it's owner + IProxyAdmin proxyAdmin = IProxyAdmin(deploy.mustGetAddress("ProxyAdmin")); + address proxyAdminOwner = proxyAdmin.owner(); + + // Look for the emit of the `ETHMigrated` event + vm.expectEmit(address(optimismPortal2)); + emit ETHMigrated(portalEthBalance); + + // Update the portal proxy implementation to the LiquidityMigrator contract + vm.prank(proxyAdminOwner); + proxyAdmin.upgradeAndCall({ + _proxy: payable(optimismPortal2), + _implementation: address(liquidityMigrator), + _data: abi.encodeCall(LiquidityMigrator.migrateETH, ()) + }); // Assert the balances after the migration happened - assert(address(migrator).balance == 0); - assert(address(sharedLockbox).balance == _lockboxBalanceBefore + _migratorEthBalance); + assert(address(optimismPortal2).balance == 0); + assert(address(sharedLockbox).balance == lockboxBalanceBefore + portalEthBalance); } } diff --git a/packages/contracts-bedrock/test/opcm/DeploySuperchain.t.sol b/packages/contracts-bedrock/test/opcm/DeploySuperchain.t.sol index b4904c0199e8..cd1d8d5ebf8d 100644 --- a/packages/contracts-bedrock/test/opcm/DeploySuperchain.t.sol +++ b/packages/contracts-bedrock/test/opcm/DeploySuperchain.t.sol @@ -8,6 +8,7 @@ import { ProxyAdmin } from "src/universal/ProxyAdmin.sol"; import { Proxy } from "src/universal/Proxy.sol"; import { SuperchainConfig } from "src/L1/SuperchainConfig.sol"; import { SharedLockbox } from "src/L1/SharedLockbox.sol"; +import { LiquidityMigrator } from "src/L1/LiquidityMigrator.sol"; import { IProtocolVersions, ProtocolVersion } from "interfaces/L1/IProtocolVersions.sol"; import { DeploySuperchainInput, DeploySuperchain, DeploySuperchainOutput } from "scripts/deploy/DeploySuperchain.s.sol"; @@ -63,6 +64,7 @@ contract DeploySuperchainOutput_Test is Test { IProtocolVersions protocolVersionsProxy = IProtocolVersions(makeAddr("protocolVersionsProxy")); SharedLockbox sharedLockboxImpl = SharedLockbox(makeAddr("sharedLockboxImpl")); SharedLockbox sharedLockboxProxy = SharedLockbox(makeAddr("sharedLockboxProxy")); + LiquidityMigrator liquidityMigratorImpl = LiquidityMigrator(makeAddr("liquidityMigratorImpl")); // Ensure each address has code, since these are expected to be contracts. vm.etch(address(superchainProxyAdmin), hex"01"); @@ -72,6 +74,7 @@ contract DeploySuperchainOutput_Test is Test { vm.etch(address(protocolVersionsProxy), hex"01"); vm.etch(address(sharedLockboxImpl), hex"01"); vm.etch(address(sharedLockboxProxy), hex"01"); + vm.etch(address(liquidityMigratorImpl), hex"01"); // Set the output data. dso.set(dso.superchainProxyAdmin.selector, address(superchainProxyAdmin)); @@ -81,6 +84,7 @@ contract DeploySuperchainOutput_Test is Test { dso.set(dso.protocolVersionsProxy.selector, address(protocolVersionsProxy)); dso.set(dso.sharedLockboxImpl.selector, address(sharedLockboxImpl)); dso.set(dso.sharedLockboxProxy.selector, address(sharedLockboxProxy)); + dso.set(dso.liquidityMigratorImpl.selector, address(liquidityMigratorImpl)); // Compare the test data to the getter methods. assertEq(address(superchainProxyAdmin), address(dso.superchainProxyAdmin()), "100"); @@ -90,6 +94,7 @@ contract DeploySuperchainOutput_Test is Test { assertEq(address(protocolVersionsProxy), address(dso.protocolVersionsProxy()), "500"); assertEq(address(sharedLockboxImpl), address(dso.sharedLockboxImpl()), "600"); assertEq(address(sharedLockboxProxy), address(dso.sharedLockboxProxy()), "700"); + assertEq(address(liquidityMigratorImpl), address(dso.liquidityMigratorImpl()), "800"); } function test_getters_whenNotSet_reverts() public { @@ -110,6 +115,9 @@ contract DeploySuperchainOutput_Test is Test { vm.expectRevert("DeployUtils: zero address"); dso.sharedLockboxProxy(); + + vm.expectRevert("DeployUtils: zero address"); + dso.liquidityMigratorImpl(); } function test_getters_whenAddrHasNoCode_reverts() public { @@ -139,6 +147,10 @@ contract DeploySuperchainOutput_Test is Test { dso.set(dso.sharedLockboxProxy.selector, emptyAddr); vm.expectRevert(expectedErr); dso.sharedLockboxProxy(); + + dso.set(dso.liquidityMigratorImpl.selector, emptyAddr); + vm.expectRevert(expectedErr); + dso.liquidityMigratorImpl(); } } diff --git a/packages/contracts-bedrock/test/setup/Setup.sol b/packages/contracts-bedrock/test/setup/Setup.sol index 2b52e8c4c858..9aa97b3458d1 100644 --- a/packages/contracts-bedrock/test/setup/Setup.sol +++ b/packages/contracts-bedrock/test/setup/Setup.sol @@ -26,6 +26,7 @@ import { IL2OutputOracle } from "interfaces/L1/IL2OutputOracle.sol"; import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol"; import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { ISharedLockbox } from "interfaces/L1/ISharedLockbox.sol"; +import { ILiquidityMigrator } from "interfaces/L1/ILiquidityMigrator.sol"; import { IDataAvailabilityChallenge } from "interfaces/L1/IDataAvailabilityChallenge.sol"; import { IL1StandardBridge } from "interfaces/L1/IL1StandardBridge.sol"; import { IProtocolVersions } from "interfaces/L1/IProtocolVersions.sol"; @@ -94,6 +95,7 @@ contract Setup { ISuperchainConfig superchainConfig; IDataAvailabilityChallenge dataAvailabilityChallenge; ISharedLockbox sharedLockbox; + ILiquidityMigrator liquidityMigrator; // L2 contracts IL2CrossDomainMessenger l2CrossDomainMessenger = @@ -209,6 +211,7 @@ contract Setup { superchainConfig = ISuperchainConfig(deploy.mustGetAddress("SuperchainConfigProxy")); anchorStateRegistry = IAnchorStateRegistry(deploy.mustGetAddress("AnchorStateRegistryProxy")); sharedLockbox = ISharedLockbox(deploy.mustGetAddress("SharedLockboxProxy")); + liquidityMigrator = ILiquidityMigrator(deploy.mustGetAddress("LiquidityMigrator")); vm.label(address(optimismPortal), "OptimismPortal"); vm.label(deploy.mustGetAddress("OptimismPortalProxy"), "OptimismPortalProxy"); @@ -233,6 +236,7 @@ contract Setup { vm.label(deploy.mustGetAddress("SuperchainConfigProxy"), "SuperchainConfigProxy"); vm.label(address(sharedLockbox), "SharedLockbox"); vm.label(deploy.mustGetAddress("SharedLockboxProxy"), "SharedLockboxProxy"); + vm.label(address(liquidityMigrator), "LiquidtyMigrator"); vm.label(address(anchorStateRegistry), "AnchorStateRegistryProxy"); vm.label(AddressAliasHelper.applyL1ToL2Alias(address(l1CrossDomainMessenger)), "L1CrossDomainMessenger_aliased"); diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index 9c642561dd05..ba7cea19fbd0 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -934,6 +934,8 @@ contract Specification_Test is CommonTest { // LiquidityMigrator _addSpec({ _name: "LiquidityMigrator", _sel: _getSel("migrateETH()") }); + _addSpec({ _name: "LiquidityMigrator", _sel: _getSel("SHARED_LOCKBOX()") }); + _addSpec({ _name: "LiquidityMigrator", _sel: _getSel("version()") }); } /// @dev Computes the selector from a function signature.