wip

packages/contracts-bedrock/scripts/Deploy.s.sol

@@ -385,32 +385,29 @@ contract Deploy is Deployer {
 
     /// @notice Deploy the SystemConfig
     function deploySystemConfig() broadcast() public returns (address) {
-        bytes32 batcherHash = bytes32(uint256(uint160(cfg.batchSenderAddress())));
-
-        SystemConfig config = new SystemConfig({
-            _owner: cfg.finalSystemOwner(),
-            _overhead: cfg.gasPriceOracleOverhead(),
-            _scalar: cfg.gasPriceOracleScalar(),
-            _batcherHash: batcherHash,
-            _gasLimit: uint64(cfg.l2GenesisBlockGasLimit()),
-            _unsafeBlockSigner: cfg.p2pSequencerAddress(),
-            _config: Constants.DEFAULT_RESOURCE_CONFIG()
-        });
+        SystemConfig config = new SystemConfig();
 
-        require(config.owner() == cfg.finalSystemOwner());
-        require(config.overhead() == cfg.gasPriceOracleOverhead());
-        require(config.scalar() == cfg.gasPriceOracleScalar());
-        require(config.unsafeBlockSigner() == cfg.p2pSequencerAddress());
-        require(config.batcherHash() == batcherHash);
+        require(config.owner() == address(0));
+        require(config.overhead() == 0);
+        require(config.scalar() == 0);
+        require(config.unsafeBlockSigner() == address(0));
+        require(config.batcherHash() == bytes32(0));
 
         ResourceMetering.ResourceConfig memory rconfig = Constants.DEFAULT_RESOURCE_CONFIG();
         ResourceMetering.ResourceConfig memory resourceConfig = config.resourceConfig();
-        require(resourceConfig.maxResourceLimit == rconfig.maxResourceLimit);
-        require(resourceConfig.elasticityMultiplier == rconfig.elasticityMultiplier);
-        require(resourceConfig.baseFeeMaxChangeDenominator == rconfig.baseFeeMaxChangeDenominator);
-        require(resourceConfig.systemTxMaxGas == rconfig.systemTxMaxGas);
-        require(resourceConfig.minimumBaseFee == rconfig.minimumBaseFee);
-        require(resourceConfig.maximumBaseFee == rconfig.maximumBaseFee);
+        require(resourceConfig.maxResourceLimit == 0);
+        require(resourceConfig.elasticityMultiplier == 0);
+        require(resourceConfig.baseFeeMaxChangeDenominator == 0);
+        require(resourceConfig.systemTxMaxGas == 0);
+        require(resourceConfig.minimumBaseFee == 0);
+        require(resourceConfig.maximumBaseFee == 0);
+
+        require(config.l1ERC721Bridge() == address(0));
+        require(config.l1StandardBridge() == address(0));
+        require(config.l2OutputOracle() == address(0));
+        require(config.optimismPortal() == address(0));
+        require(config.l1CrossDomainMessenger() == address(0));
+        require(config.startBlock() == 0);
 
         save("SystemConfig", address(config));
         console.log("SystemConfig deployed at %s", address(config));
@@ -480,6 +477,7 @@ contract Deploy is Deployer {
 
     /// @notice Initialize the SystemConfig
     function initializeSystemConfig() broadcast() public {
+        /*
         ProxyAdmin proxyAdmin = ProxyAdmin(mustGetAddress("ProxyAdmin"));
         address systemConfigProxy = mustGetAddress("SystemConfigProxy");
         address systemConfig = mustGetAddress("SystemConfig");
@@ -503,11 +501,10 @@ contract Deploy is Deployer {
                     mustGetAddress("L1ERC721BridgeProxy"),
                     mustGetAddress("L1StandardBridgeProxy"),
                     mustGetAddress("L2OutputOracleProxy"),
-                    mustGetAddress("OptimismPortalProxy")
+                    mustGetAddress("OptimismPortalProxy"),
+                    0 // TODO: this needs to come from deploy config
                 )
             )
-
-            // TODO: assert that addresses are correct
         });
 
         SystemConfig config = SystemConfig(systemConfigProxy);
@@ -529,6 +526,13 @@ contract Deploy is Deployer {
         require(resourceConfig.minimumBaseFee == rconfig.minimumBaseFee);
         require(resourceConfig.maximumBaseFee == rconfig.maximumBaseFee);
 
+        require(config.l1ERC721Bridge() == mustGetAddress("L1ERC721BridgeProxy"));
+        require(config.l1StandardBridge() == mustGetAddress("L1StandardBridgeProxy"));
+        require(config.l2OutputOracle() == mustGetAddress("L2OutputOracleProxy"));
+        require(config.optimismPortal() == mustGetAddress("OptimismPortalProxy"));
+        require(config.l1CrossDomainMessenger() == mustGetAddress("L1CrossDomainMessengerProxy"));
+        require(config.startBlock() == 0); // TODO
+        */
     }
 
     /// @notice Initialize the L1StandardBridge

packages/contracts-bedrock/src/L1/SystemConfig.sol

@@ -25,6 +25,15 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         UNSAFE_BLOCK_SIGNER
     }
 
+    struct Addresses {
+        address l1CrossDomainMessenger;
+        address l1ERC721Bridge;
+        address l1StandardBridge;
+        address l2OutputOracle;
+        address optimismPortal;
+        //address batchInbox;
+    }
+
     /// @notice Version identifier, used for upgrades.
     uint256 public constant VERSION = 0;
 
@@ -34,12 +43,24 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     ///         proof to fetch this value.
     bytes32 public constant UNSAFE_BLOCK_SIGNER_SLOT = keccak256("systemconfig.unsafeblocksigner");
 
+    /// @notice
     bytes32 public constant L1_CROSS_DOMAIN_MESSENGER_SLOT = keccak256("systemconfig.l1crossdomainmessenger");
+
+    /// @notice
     bytes32 public constant L1_ERC_721_BRIDGE_SLOT = keccak256("systemconfig.l1erc721bridge");
+
+    /// @notice
     bytes32 public constant L1_STANDARD_BRIDGE_SLOT = keccak256("systemconfig.l1standardbridge");
+
+    /// @notice
     bytes32 public constant L2_OUTPUT_ORACLE_SLOT = keccak256("systemconfig.l2outputoracle");
+
+    /// @notice
     bytes32 public constant OPTIMISM_PORTAL_SLOT = keccak256("systemconfig.optimismportal");
 
+    /// @notice
+    bytes32 public constant BATCH_INBOX_SLOT = keccak256("systemconfig.batchinbox");
+
     /// @notice Fixed L2 gas overhead. Used as part of the L2 fee calculation.
     uint256 public overhead;
 
@@ -66,40 +87,28 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     event ConfigUpdate(uint256 indexed version, UpdateType indexed updateType, bytes data);
 
     /// @notice The block at which the op-node can start searching for
-    ///         logs from
-    uint256 startBlock;
+    ///         logs from.
+    uint256 public startBlock;
 
-    /// @custom:semver 1.3.1
+    /// @custom:semver 1.4.0
     /// @notice Constructs the SystemConfig contract.
-    /// @param _owner             Initial owner of the contract.
-    /// @param _overhead          Initial overhead value.
-    /// @param _scalar            Initial scalar value.
-    /// @param _batcherHash       Initial batcher hash.
-    /// @param _gasLimit          Initial gas limit.
-    /// @param _unsafeBlockSigner Initial unsafe block signer address.
-    /// @param _config            Initial resource config.
-    constructor(
-        address _owner,
-        uint256 _overhead,
-        uint256 _scalar,
-        bytes32 _batcherHash,
-        uint64 _gasLimit,
-        address _unsafeBlockSigner,
-        ResourceMetering.ResourceConfig memory _config
-    ) Semver(1, 3, 1) {
+    constructor() Semver(1, 4, 0) {
         initialize({
-            _owner: _owner,
-            _overhead: _overhead,
-            _scalar: _scalar,
-            _batcherHash: _batcherHash,
-            _gasLimit: _gasLimit,
-            _unsafeBlockSigner: _unsafeBlockSigner,
-            _config: _config,
-            _l1CrossDomainMessenger: address(0),
-            _l1ERC721Bridge: address(0),
-            _l1StandardBridge: address(0),
-            _l2OutputOracle: address(0),
-            _optimismPortal: address(0)
+            _owner: address(0),
+            _overhead: 0,
+            _scalar: 0,
+            _batcherHash: bytes32(0),
+            _gasLimit: 0,
+            _unsafeBlockSigner: address(0),
+            _config: ResourceMetering.ResourceConfig({
+                maxResourceLimit: 0,
+                elasticityMultiplier: 0,
+                baseFeeMaxChangeDenominator: 0,
+                minimumBaseFee: 0,
+                systemTxMaxGas: 0,
+                maximumBaseFee: 0
+            }),
+            _startBlock: 0
         });
     }
 
@@ -120,44 +129,54 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         uint64 _gasLimit,
         address _unsafeBlockSigner,
         ResourceMetering.ResourceConfig memory _config,
-        address _l1CrossDomainMessenger,
-        address _l1ERC721Bridge,
-        address _l1StandardBridge,
-        address _l2OutputOracle,
-        address _optimismPortal
-    ) public initializer {
+        uint256 _startBlock
+    ) public reinitializer(2) {
         __Ownable_init();
         transferOwnership(_owner);
 
-        // TODO: this needs to be backwards compatible with
-        // chains that already have it set.
-        startBlock = block.number;
-        // Could add an extra arg that is an override and do
-        // something like:
-        /*
-        if (_startBlock != 0) startBlock = _startBlock;
-        else startBlock = block.number;
-        */
-       // And then in the next upgrade, remove the extra arg
-        // and move to something like:
-       /*
-        if (startBlock == 0) startBlock = block.number;
-        */
+        // The start block for the op-node to start searching for logs
+        // needs to be set in a backwards compatible way. Only allow setting
+        // the start block with an override if it has previously never been set.
+        if (_startBlock != 0) {
+            require(startBlock == 0, "SystemConfig: cannot override an already set start block");
+            startBlock = _startBlock;
+        } else if (startBlock == 0) {
+            startBlock = block.number;
+        }
 
         overhead = _overhead;
         scalar = _scalar;
         batcherHash = _batcherHash;
         gasLimit = _gasLimit;
-        _setUnsafeBlockSigner(_unsafeBlockSigner);
+        _setAddress(_unsafeBlockSigner, UNSAFE_BLOCK_SIGNER_SLOT);
 
+        _setResourceConfig(_config);
+        require(_gasLimit >= minimumGasLimit(), "SystemConfig: gas limit too low");
+    }
+
+    /// @notice Sets all of the addresses. These are not set in the `initialize`
+    ///         function because it results in stack too deep errors when that
+    ///         many constructor arguments are used. It is more simple to use
+    ///         another function.
+    /// @param _l1CrossDomainMessenger   Address of the L1CrossDomainMessenger.
+    /// @param _l1ERC721Bridge           Address of the L1ERC721Bridge.
+    /// @param _l1StandardBridge         Address of the L1StandardBridge.
+    /// @param _l2OutputOracle           Address of the L2OutputOracle.
+    /// @param _batchInbox               Address of the BatchInbox.
+    function setAddresses(
+        address _l1CrossDomainMessenger,
+        address _l1ERC721Bridge,
+        address _l1StandardBridge,
+        address _l2OutputOracle,
+        address _optimismPortal,
+        address _batchInbox
+    ) external onlyOwner {
         _setAddress(_l1CrossDomainMessenger, L1_CROSS_DOMAIN_MESSENGER_SLOT);
         _setAddress(_l1ERC721Bridge, L1_ERC_721_BRIDGE_SLOT);
         _setAddress(_l1StandardBridge, L1_STANDARD_BRIDGE_SLOT);
         _setAddress(_l2OutputOracle, L2_OUTPUT_ORACLE_SLOT);
         _setAddress(_optimismPortal, OPTIMISM_PORTAL_SLOT);
-
-        _setResourceConfig(_config);
-        require(_gasLimit >= minimumGasLimit(), "SystemConfig: gas limit too low");
+        _setAddress(_batchInbox, BATCH_INBOX_SLOT);
     }
 
     /// @notice Returns the minimum L2 gas limit that can be safely set for the system to
@@ -189,7 +208,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     /// @param _slot The storage slot to store the address in.
     /// @dev WARNING! This function must be used cautiously, as it allows for overwriting values
     ///      in arbitrary storage slots. Solc will add checks that the data passed as `_addr`
-    ///      is 20 bytes or less. 
+    ///      is 20 bytes or less.
     function _setAddress(address _addr, bytes32 _slot) internal {
         bytes32 slot = _slot;
         assembly {
@@ -197,10 +216,50 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         }
     }
 
+    /// @notice
+    function _getAddress(bytes32 _slot) internal view returns (address) {
+        address addr;
+        bytes32 slot = _slot;
+        assembly {
+            addr := sload(slot)
+        }
+        return addr;
+    }
+
+    /// @notice Getter for the L1CrossDomainMessenger address.
+    function l1CrossDomainMessenger() external view returns (address) {
+        return _getAddress(L1_CROSS_DOMAIN_MESSENGER_SLOT);
+    }
+
+    /// @notice Getter for the L1ERC721Bridge address.
+    function l1ERC721Bridge() external view returns (address) {
+        return _getAddress(L1_ERC_721_BRIDGE_SLOT);
+    }
+
+    /// @notice Getter for the L1StandardBridge address.
+    function l1StandardBridge() external view returns (address) {
+        return _getAddress(L1_STANDARD_BRIDGE_SLOT);
+    }
+
+    /// @notice Getter for the L2OutputOracle address.
+    function l2OutputOracle() external view returns (address) {
+        return _getAddress(L2_OUTPUT_ORACLE_SLOT);
+    }
+
+    /// @notice Getter for the OptimismPortal address.
+    function optimismPortal() external view returns (address) {
+        return _getAddress(OPTIMISM_PORTAL_SLOT);
+    }
+
+    /// @notice Getter for the BatchInbox address.
+    function batchInbox() external view returns (address) {
+        return _getAddress(BATCH_INBOX_SLOT);
+    }
+
     /// @notice Updates the unsafe block signer address.
     /// @param _unsafeBlockSigner New unsafe block signer address.
     function setUnsafeBlockSigner(address _unsafeBlockSigner) external onlyOwner {
-        _setUnsafeBlockSigner(_unsafeBlockSigner);
+        _setAddress(_unsafeBlockSigner, UNSAFE_BLOCK_SIGNER_SLOT);
 
         bytes memory data = abi.encode(_unsafeBlockSigner);
         emit ConfigUpdate(VERSION, UpdateType.UNSAFE_BLOCK_SIGNER, data);
@@ -236,17 +295,6 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         emit ConfigUpdate(VERSION, UpdateType.GAS_LIMIT, data);
     }
 
-    /// @notice Low level setter for the unsafe block signer address.
-    ///         This function exists to deduplicate code around storing the unsafeBlockSigner
-    ///         address in storage.
-    /// @param _unsafeBlockSigner New unsafeBlockSigner value.
-    function _setUnsafeBlockSigner(address _unsafeBlockSigner) internal {
-        bytes32 slot = UNSAFE_BLOCK_SIGNER_SLOT;
-        assembly {
-            sstore(slot, _unsafeBlockSigner)
-        }
-    }
-
     /// @notice A getter for the resource config.
     ///         Ensures that the struct is returned instead of a tuple.
     /// @return ResourceConfig

packages/contracts-bedrock/test/CommonTest.t.sol

@@ -202,17 +202,37 @@ contract Portal_Initializer is L2OutputOracle_Initializer {
     function setUp() public virtual override {
         super.setUp();
 
-        ResourceMetering.ResourceConfig memory config = Constants.DEFAULT_RESOURCE_CONFIG();
-
-        systemConfig = new SystemConfig({
-            _owner: address(1),
-            _overhead: 0,
-            _scalar: 10000,
-            _batcherHash: bytes32(0),
-            _gasLimit: 30_000_000,
-            _unsafeBlockSigner: address(0),
-            _config: config
-        });
+        Proxy systemConfigProxy = new Proxy(multisig);
+
+        SystemConfig systemConfigImpl = new SystemConfig();
+
+        vm.prank(multisig);
+        systemConfigProxy.upgradeToAndCall(
+            address(systemConfigImpl),
+            hex""
+            /*
+            abi.encodeCall(
+                SystemConfig.initialize,
+                (
+                    address(1),                           //_owner,
+                    0,                                    //_overhead,
+                    10000,                                //_scalar,
+                    bytes32(0),                           //_batcherHash,
+                    30_000_000,                           //_gasLimit,
+                    address(0),                           //_unsafeBlockSigner,
+                    Constants.DEFAULT_RESOURCE_CONFIG(),  //_config,
+                    address(0x40),                        //_l1CrossDomainMessenger,
+                    address(0x41),                        //_l1ERC721Bridge,
+                    address(0x42),                        //_l1StandardBridge,
+                    address(0x43),                        //_l2OutputOracle,
+                    address(0x44),                        //_optimismPortal,
+                    0                                     //_startBlock
+                )
+            )
+            */
+        );
+
+        systemConfig = SystemConfig(address(systemConfigProxy));
 
         opImpl = new OptimismPortal();
 

packages/contracts-bedrock/test/SystemConfig.t.sol

@@ -9,34 +9,40 @@ import { Constants } from "../src/libraries/Constants.sol";
 
 // Target contract dependencies
 import { ResourceMetering } from "../src/L1/ResourceMetering.sol";
+import { Proxy } from "../src/universal/Proxy.sol";
 
 // Target contract
 import { SystemConfig } from "../src/L1/SystemConfig.sol";
 
 contract SystemConfig_Init is CommonTest {
     SystemConfig sysConf;
+    SystemConfig systemConfigImpl;
 
     function setUp() public virtual override {
         super.setUp();
 
-        ResourceMetering.ResourceConfig memory config = ResourceMetering.ResourceConfig({
-            maxResourceLimit: 20_000_000,
-            elasticityMultiplier: 10,
-            baseFeeMaxChangeDenominator: 8,
-            minimumBaseFee: 1 gwei,
-            systemTxMaxGas: 1_000_000,
-            maximumBaseFee: type(uint128).max
-        });
+        Proxy proxy = new Proxy(multisig);
+        systemConfigImpl = new SystemConfig();
+
+        vm.prank(multisig);
+        proxy.upgradeToAndCall(
+            address(systemConfigImpl),
+            abi.encodeCall(
+                SystemConfig.initialize,
+                (
+                    alice,                                //_owner,
+                    2100,                                 //_overhead,
+                    1000000,                              //_scalar,
+                    bytes32(hex"abcd"),                   //_batcherHash,
+                    30_000_000,                           //_gasLimit,
+                    address(1),                           //_unsafeBlockSigner,
+                    Constants.DEFAULT_RESOURCE_CONFIG(),  //_config,
+                    0                                     //_startBlock
+                )
+            )
+        );
 
-        sysConf = new SystemConfig({
-            _owner: alice,
-            _overhead: 2100,
-            _scalar: 1000000,
-            _batcherHash: bytes32(hex"abcd"),
-            _gasLimit: 30_000_000,
-            _unsafeBlockSigner: address(1),
-            _config: config
-        });
+        sysConf = SystemConfig(address(proxy));
     }
 }
 
@@ -54,16 +60,25 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
             maximumBaseFee: type(uint128).max
         });
 
+        vm.store(address(sysConf), bytes32(0), bytes32(0));
         vm.expectRevert("SystemConfig: gas limit too low");
-        new SystemConfig({
-            _owner: alice,
-            _overhead: 0,
-            _scalar: 0,
-            _batcherHash: bytes32(hex""),
-            _gasLimit: minimumGasLimit - 1,
-            _unsafeBlockSigner: address(1),
-            _config: cfg
-        });
+        vm.prank(multisig);
+        Proxy(payable(address(sysConf))).upgradeToAndCall(
+            address(systemConfigImpl),
+            abi.encodeCall(
+                SystemConfig.initialize,
+                (
+                    alice,                 //_owner,
+                    2100,                  //_overhead,
+                    1000000,               //_scalar,
+                    bytes32(hex"abcd"),    //_batcherHash,
+                    minimumGasLimit - 1,   //_gasLimit,
+                    address(1),            //_unsafeBlockSigner,
+                    cfg,                   //_config,
+                    0                      //_startBlock
+                )
+            )
+        );
     }
 }
 

packages/contracts-bedrock/test/invariants/SystemConfig.t.sol

@@ -3,24 +3,35 @@ pragma solidity 0.8.15;
 
 import { Test } from "forge-std/Test.sol";
 import { SystemConfig } from "../../src/L1/SystemConfig.sol";
+import { Proxy } from "../../src/universal/Proxy.sol";
 import { ResourceMetering } from "../../src/L1/ResourceMetering.sol";
 import { Constants } from "../../src/libraries/Constants.sol";
 
 contract SystemConfig_GasLimitLowerBound_Invariant is Test {
     SystemConfig public config;
 
-    function setUp() public {
-        ResourceMetering.ResourceConfig memory cfg = Constants.DEFAULT_RESOURCE_CONFIG();
+    function setUp() external {
+        Proxy proxy = new Proxy(msg.sender);
+        SystemConfig configImpl = new SystemConfig();
 
-        config = new SystemConfig({
-            _owner: address(0xbeef),
-            _overhead: 2100,
-            _scalar: 1000000,
-            _batcherHash: bytes32(hex"abcd"),
-            _gasLimit: 30_000_000,
-            _unsafeBlockSigner: address(1),
-            _config: cfg
-        });
+        proxy.upgradeToAndCall(
+            address(configImpl),
+            abi.encodeCall(
+                configImpl.initialize,
+                (
+                    address(0xbeef),                      // owner
+                    2100,                                 // overhead
+                    1000000,                              // scalar
+                    bytes32(hex"abcd"),                   // batcher hash
+                    30_000_000,                           // gas limit
+                    address(1),                           // unsafe block signer
+                    Constants.DEFAULT_RESOURCE_CONFIG(),  // resource config
+                    0                                     //_startBlock
+                )
+            )
+        );
+
+        config = SystemConfig(address(proxy));
 
         // Set the target contract to the `config`
         targetContract(address(config));