From c686afee2243ea49c96428ddfa569e537fcdedf2 Mon Sep 17 00:00:00 2001 From: AgusDuha <81362284+agusduha@users.noreply.github.com> Date: Wed, 28 Aug 2024 15:28:06 -0300 Subject: [PATCH] feat: add constructor to superchain ERC20 beacon (#34) --- .../contracts-bedrock/scripts/L2Genesis.s.sol | 21 ++++- packages/contracts-bedrock/semver-lock.json | 4 +- .../abi/OptimismSuperchainERC20Beacon.json | 13 ++- .../src/L2/OptimismSuperchainERC20Beacon.sol | 16 ++-- .../src/libraries/Predeploys.sol | 2 +- .../contracts-bedrock/test/L2Genesis.t.sol | 14 ++-- .../contracts-bedrock/test/Predeploys.t.sol | 80 +++++++++++-------- 7 files changed, 95 insertions(+), 55 deletions(-) diff --git a/packages/contracts-bedrock/scripts/L2Genesis.s.sol b/packages/contracts-bedrock/scripts/L2Genesis.s.sol index b80ba64ed5eb..5ab9e1e13165 100644 --- a/packages/contracts-bedrock/scripts/L2Genesis.s.sol +++ b/packages/contracts-bedrock/scripts/L2Genesis.s.sol @@ -26,6 +26,7 @@ import { L1StandardBridge } from "src/L1/L1StandardBridge.sol"; import { FeeVault } from "src/universal/FeeVault.sol"; import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol"; import { Process } from "scripts/libraries/Process.sol"; +import { OptimismSuperchainERC20Beacon } from "src/L2/OptimismSuperchainERC20Beacon.sol"; interface IInitializable { function initialize(address _addr) external; @@ -513,10 +514,24 @@ contract L2Genesis is Deployer { _setImplementationCode(Predeploys.OPTIMISM_SUPERCHAIN_ERC20_FACTORY); } - /// @notice This predeploy is following the safety invariant #1. - /// This contract has no initializer. + /// @notice This predeploy is following the safety invariant #2. function setOptimismSuperchainERC20Beacon() internal { - _setImplementationCode(Predeploys.OPTIMISM_SUPERCHAIN_ERC20_BEACON); + // TODO: Precalculate the address of the implementation contract + address superchainERC20Impl = address(uint160(uint256(keccak256("OptimismSuperchainERC20")))); + console.log("Setting %s implementation at: %s", "OptimismSuperchainERC20", superchainERC20Impl); + vm.etch(superchainERC20Impl, vm.getDeployedCode("OptimismSuperchainERC20.sol:OptimismSuperchainERC20")); + + OptimismSuperchainERC20Beacon beacon = new OptimismSuperchainERC20Beacon(superchainERC20Impl); + console.log( + "Setting %s implementation at: %s", + "OptimismSuperchainERC20Beacon", + Predeploys.OPTIMISM_SUPERCHAIN_ERC20_BEACON + ); + vm.etch(Predeploys.OPTIMISM_SUPERCHAIN_ERC20_BEACON, address(beacon).code); + + /// Reset so its not included state dump + vm.etch(address(beacon), ""); + vm.resetNonce(address(beacon)); } /// @notice Sets all the preinstalls. diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json index f35d74c67151..6c3af2008f00 100644 --- a/packages/contracts-bedrock/semver-lock.json +++ b/packages/contracts-bedrock/semver-lock.json @@ -120,8 +120,8 @@ "sourceCodeHash": "0x910d43a17800df64dbc104f69ef1f900ca761cec4949c01d1c1126fde5268349" }, "src/L2/OptimismSuperchainERC20Beacon.sol": { - "initCodeHash": "0xc9893a1ba9b6354ccbdc96918510c9402a0934a59f0cc4a6165955a8e8ee1f7d", - "sourceCodeHash": "0xd069f4bddd08c1435d286819e1bb765648a5ad861fa327547f52de65a372769e" + "initCodeHash": "0x99ce8095b23c124850d866cbc144fee6cee05dbc6bb5d83acadfe00b90cf42c7", + "sourceCodeHash": "0x00eb41c15cf548dfb6425f317d85262c1edd5f1ad2386a6a76695781d158cf16" }, "src/L2/OptimismSuperchainERC20Factory.sol": { "initCodeHash": "0x98011045722178751e4a1112892f7d9a11bc1f5e42ac18205b6d30a1f1476d24", diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismSuperchainERC20Beacon.json b/packages/contracts-bedrock/snapshots/abi/OptimismSuperchainERC20Beacon.json index a06b5b5d140e..0bdfc64ed2fe 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismSuperchainERC20Beacon.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismSuperchainERC20Beacon.json @@ -1,4 +1,15 @@ [ + { + "inputs": [ + { + "internalType": "address", + "name": "_implementation", + "type": "address" + } + ], + "stateMutability": "nonpayable", + "type": "constructor" + }, { "inputs": [], "name": "implementation", @@ -9,7 +20,7 @@ "type": "address" } ], - "stateMutability": "pure", + "stateMutability": "view", "type": "function" }, { diff --git a/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20Beacon.sol b/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20Beacon.sol index 7a19e1e9af9b..2c00410c29eb 100644 --- a/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20Beacon.sol +++ b/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20Beacon.sol @@ -1,24 +1,26 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.25; +pragma solidity 0.8.15; -import { IBeacon } from "@openzeppelin/contracts-v5/proxy/beacon/IBeacon.sol"; +import { IBeacon } from "@openzeppelin/contracts/proxy/beacon/IBeacon.sol"; import { ISemver } from "src/universal/ISemver.sol"; -/// @custom:proxied /// @custom:predeployed 0x4200000000000000000000000000000000000027 /// @title OptimismSuperchainERC20Beacon /// @notice OptimismSuperchainERC20Beacon is the beacon proxy for the OptimismSuperchainERC20 implementation. contract OptimismSuperchainERC20Beacon is IBeacon, ISemver { - /// TODO: Replace with real implementation address /// @notice Address of the OptimismSuperchainERC20 implementation. - address internal constant IMPLEMENTATION_ADDRESS = 0x0000000000000000000000000000000000000000; + address internal immutable IMPLEMENTATION; /// @notice Semantic version. /// @custom:semver 1.0.0-beta.1 string public constant version = "1.0.0-beta.1"; + constructor(address _implementation) { + IMPLEMENTATION = _implementation; + } + /// @inheritdoc IBeacon - function implementation() external pure override returns (address) { - return IMPLEMENTATION_ADDRESS; + function implementation() external view override returns (address) { + return IMPLEMENTATION; } } diff --git a/packages/contracts-bedrock/src/libraries/Predeploys.sol b/packages/contracts-bedrock/src/libraries/Predeploys.sol index 9e252d52d1b7..26a0b7f95603 100644 --- a/packages/contracts-bedrock/src/libraries/Predeploys.sol +++ b/packages/contracts-bedrock/src/libraries/Predeploys.sol @@ -136,7 +136,7 @@ library Predeploys { /// @notice Returns true if the predeploy is not proxied. function notProxied(address _addr) internal pure returns (bool) { - return _addr == GOVERNANCE_TOKEN || _addr == WETH; + return _addr == GOVERNANCE_TOKEN || _addr == WETH || _addr == OPTIMISM_SUPERCHAIN_ERC20_BEACON; } /// @notice Returns true if the address is a defined predeploy that is embedded into new OP-Stack chains. diff --git a/packages/contracts-bedrock/test/L2Genesis.t.sol b/packages/contracts-bedrock/test/L2Genesis.t.sol index ee993fe1110c..a1a464a5082e 100644 --- a/packages/contracts-bedrock/test/L2Genesis.t.sol +++ b/packages/contracts-bedrock/test/L2Genesis.t.sol @@ -147,18 +147,18 @@ contract L2GenesisTest is Test { genesis.setPredeployProxies(); genesis.writeGenesisAllocs(_path); - // 2 predeploys do not have proxies - assertEq(getCodeCount(_path, "Proxy.sol:Proxy"), Predeploys.PREDEPLOY_COUNT - 2); + // 3 predeploys do not have proxies + assertEq(getCodeCount(_path, "Proxy.sol:Proxy"), Predeploys.PREDEPLOY_COUNT - 3); - // 23 proxies have the implementation set if useInterop is true and 17 if useInterop is false - assertEq(getPredeployCountWithSlotSet(_path, Constants.PROXY_IMPLEMENTATION_ADDRESS), _useInterop ? 23 : 17); + // 22 proxies have the implementation set if useInterop is true and 17 if useInterop is false + assertEq(getPredeployCountWithSlotSet(_path, Constants.PROXY_IMPLEMENTATION_ADDRESS), _useInterop ? 22 : 17); - // All proxies except 2 have the proxy 1967 admin slot set to the proxy admin + // All proxies except 3 have the proxy 1967 admin slot set to the proxy admin assertEq( getPredeployCountWithSlotSetToValue( _path, Constants.PROXY_OWNER_ADDRESS, bytes32(uint256(uint160(Predeploys.PROXY_ADMIN))) ), - Predeploys.PREDEPLOY_COUNT - 2 + Predeploys.PREDEPLOY_COUNT - 3 ); // Also see Predeploys.t.test_predeploysSet_succeeds which uses L1Genesis for the CommonTest prestate. @@ -185,7 +185,7 @@ contract L2GenesisTest is Test { genesis.writeGenesisAllocs(_path); uint256 expected = 0; - expected += 2048 - 2; // predeploy proxies + expected += 2048 - 3; // predeploy proxies expected += 21; // predeploy implementations (excl. legacy erc20-style eth and legacy message sender) expected += 256; // precompiles expected += 13; // preinstalls diff --git a/packages/contracts-bedrock/test/Predeploys.t.sol b/packages/contracts-bedrock/test/Predeploys.t.sol index d39eb8b0ff61..0d9e4879fd6b 100644 --- a/packages/contracts-bedrock/test/Predeploys.t.sol +++ b/packages/contracts-bedrock/test/Predeploys.t.sol @@ -7,26 +7,21 @@ import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; /// @title PredeploysTest -contract PredeploysTest is CommonTest { +contract PredeploysBaseTest is CommonTest { ////////////////////////////////////////////////////// /// Internal helpers ////////////////////////////////////////////////////// - /// @dev Returns true if the address is a predeploy that is active (i.e. embedded in L2 genesis). - function _isPredeploy(address _addr) internal pure returns (bool) { - return _addr == Predeploys.L2_TO_L1_MESSAGE_PASSER || _addr == Predeploys.L2_CROSS_DOMAIN_MESSENGER - || _addr == Predeploys.L2_STANDARD_BRIDGE || _addr == Predeploys.L2_ERC721_BRIDGE - || _addr == Predeploys.SEQUENCER_FEE_WALLET || _addr == Predeploys.OPTIMISM_MINTABLE_ERC20_FACTORY - || _addr == Predeploys.OPTIMISM_MINTABLE_ERC721_FACTORY || _addr == Predeploys.L1_BLOCK_ATTRIBUTES - || _addr == Predeploys.GAS_PRICE_ORACLE || _addr == Predeploys.DEPLOYER_WHITELIST || _addr == Predeploys.WETH - || _addr == Predeploys.L1_BLOCK_NUMBER || _addr == Predeploys.LEGACY_MESSAGE_PASSER - || _addr == Predeploys.PROXY_ADMIN || _addr == Predeploys.BASE_FEE_VAULT || _addr == Predeploys.L1_FEE_VAULT - || _addr == Predeploys.GOVERNANCE_TOKEN || _addr == Predeploys.SCHEMA_REGISTRY || _addr == Predeploys.EAS; + /// @dev Returns true if the address is an interop predeploy. + function _isInterop(address _addr) internal pure returns (bool) { + return _addr == Predeploys.CROSS_L2_INBOX || _addr == Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER + || _addr == Predeploys.SUPERCHAIN_WETH || _addr == Predeploys.ETH_LIQUIDITY + || _addr == Predeploys.OPTIMISM_SUPERCHAIN_ERC20_FACTORY || _addr == Predeploys.OPTIMISM_SUPERCHAIN_ERC20_BEACON; } - /// @dev Returns true if the address is not proxied. - function _notProxied(address _addr) internal pure returns (bool) { - return _addr == Predeploys.GOVERNANCE_TOKEN || _addr == Predeploys.WETH; + /// @dev Returns true if the address is a predeploy that has a different code in the interop mode. + function _interopCodeDiffer(address _addr) internal pure returns (bool) { + return _addr == Predeploys.L1_BLOCK_ATTRIBUTES || _addr == Predeploys.L2_STANDARD_BRIDGE; } /// @dev Returns true if the account is not meant to be in the L2 genesis anymore. @@ -34,22 +29,17 @@ contract PredeploysTest is CommonTest { return _addr == Predeploys.L1_MESSAGE_SENDER; } + /// @dev Returns true if the predeploy is initializable. function _isInitializable(address _addr) internal pure returns (bool) { - return !( - _addr == Predeploys.LEGACY_MESSAGE_PASSER || _addr == Predeploys.DEPLOYER_WHITELIST - || _addr == Predeploys.GAS_PRICE_ORACLE || _addr == Predeploys.SEQUENCER_FEE_WALLET - || _addr == Predeploys.BASE_FEE_VAULT || _addr == Predeploys.L1_FEE_VAULT - || _addr == Predeploys.L1_BLOCK_NUMBER || _addr == Predeploys.L1_BLOCK_ATTRIBUTES - || _addr == Predeploys.L2_TO_L1_MESSAGE_PASSER || _addr == Predeploys.OPTIMISM_MINTABLE_ERC721_FACTORY - || _addr == Predeploys.PROXY_ADMIN || _addr == Predeploys.SCHEMA_REGISTRY || _addr == Predeploys.EAS - || _addr == Predeploys.GOVERNANCE_TOKEN - ); + return _addr == Predeploys.L2_CROSS_DOMAIN_MESSENGER || _addr == Predeploys.L2_STANDARD_BRIDGE + || _addr == Predeploys.L2_ERC721_BRIDGE || _addr == Predeploys.OPTIMISM_MINTABLE_ERC20_FACTORY; } + /// @dev Returns true if the predeploy uses immutables. function _usesImmutables(address _addr) internal pure returns (bool) { return _addr == Predeploys.OPTIMISM_MINTABLE_ERC721_FACTORY || _addr == Predeploys.SEQUENCER_FEE_WALLET || _addr == Predeploys.BASE_FEE_VAULT || _addr == Predeploys.L1_FEE_VAULT || _addr == Predeploys.EAS - || _addr == Predeploys.GOVERNANCE_TOKEN; + || _addr == Predeploys.GOVERNANCE_TOKEN || _addr == Predeploys.OPTIMISM_SUPERCHAIN_ERC20_BEACON; } function test_predeployToCodeNamespace() external pure { @@ -67,9 +57,7 @@ contract PredeploysTest is CommonTest { ); } - /// @dev Tests that the predeploy addresses are set correctly. They have code - /// and the proxied accounts have the correct admin. - function test_predeploys_succeeds() external { + function _test_predeploys(bool _useInterop) internal { uint256 count = 2048; uint160 prefix = uint160(0x420) << 148; @@ -77,23 +65,25 @@ contract PredeploysTest is CommonTest { for (uint256 i = 0; i < count; i++) { address addr = address(prefix | uint160(i)); - bytes memory code = addr.code; - assertTrue(code.length > 0); - address implAddr = Predeploys.predeployToCodeNamespace(addr); if (_isOmitted(addr)) { assertEq(implAddr.code.length, 0, "must have no code"); continue; } - bool isPredeploy = _isPredeploy(addr); + + bool isPredeploy = Predeploys.isSupportedPredeploy(addr, _useInterop); + + bytes memory code = addr.code; + if (isPredeploy) assertTrue(code.length > 0); + + bool proxied = Predeploys.notProxied(addr) == false; if (!isPredeploy) { // All of the predeploys, even if inactive, have their admin set to the proxy admin - assertEq(EIP1967Helper.getAdmin(addr), Predeploys.PROXY_ADMIN, "Admin mismatch"); + if (proxied) assertEq(EIP1967Helper.getAdmin(addr), Predeploys.PROXY_ADMIN, "Admin mismatch"); continue; } - bool proxied = _notProxied(addr) == false; string memory cname = Predeploys.getName(addr); assertNotEq(cname, "", "must have a name"); @@ -118,7 +108,7 @@ contract PredeploysTest is CommonTest { string.concat("Implementation mismatch for ", vm.toString(addr)) ); assertNotEq(implAddr.code.length, 0, "predeploy implementation account must have code"); - if (!_usesImmutables(addr)) { + if (!_usesImmutables(addr) && !_interopCodeDiffer(addr)) { // can't check bytecode if it's modified with immutables in genesis. assertEq(implAddr.code, supposedCode, "proxy implementation contract should match contract source"); } @@ -129,3 +119,25 @@ contract PredeploysTest is CommonTest { } } } + +contract PredeploysTest is PredeploysBaseTest { + /// @dev Tests that the predeploy addresses are set correctly. They have code + /// and the proxied accounts have the correct admin. + function test_predeploys_succeeds() external { + _test_predeploys(false); + } +} + +contract PredeploysInteropTest is PredeploysBaseTest { + /// @notice Test setup. Enabling interop to get all predeploys. + function setUp() public virtual override { + super.enableInterop(); + super.setUp(); + } + + /// @dev Tests that the predeploy addresses are set correctly. They have code + /// and the proxied accounts have the correct admin. Using interop. + function test_predeploys_succeeds() external { + _test_predeploys(true); + } +}