contracts-bedrock: refactor

packages/contracts-bedrock/deploy-config/devnetL1.json

@@ -45,5 +45,6 @@
   "l2GenesisRegolithTimeOffset": "0x0",
   "faultGameAbsolutePrestate": "0x41c7ae758795765c6664a5d39bf63841c71ff191e9189522bad8ebff5d4eca98",
   "faultGameMaxDepth": 4,
-  "faultGameMaxDuration": 300
+  "faultGameMaxDuration": 300,
+  "systemConfigStartBlock": 0
 }

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

@@ -393,7 +393,6 @@ contract Deploy is Deployer {
         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 == 0);
         require(resourceConfig.elasticityMultiplier == 0);
@@ -477,7 +476,6 @@ 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");
@@ -497,12 +495,15 @@ contract Deploy is Deployer {
                     uint64(cfg.l2GenesisBlockGasLimit()),
                     cfg.p2pSequencerAddress(),
                     Constants.DEFAULT_RESOURCE_CONFIG(),
-                    mustGetAddress("L1CrossDomainMessengerProxy"),
-                    mustGetAddress("L1ERC721BridgeProxy"),
-                    mustGetAddress("L1StandardBridgeProxy"),
-                    mustGetAddress("L2OutputOracleProxy"),
-                    mustGetAddress("OptimismPortalProxy"),
-                    0 // TODO: this needs to come from deploy config
+                    cfg.systemConfigStartBlock(),
+                    cfg.batchInboxAddress(),
+                    SystemConfig.Addresses({
+                        l1CrossDomainMessenger: mustGetAddress("L1CrossDomainMessengerProxy"),
+                        l1ERC721Bridge: mustGetAddress("L1ERC721BridgeProxy"),
+                        l1StandardBridge: mustGetAddress("L1StandardBridgeProxy"),
+                        l2OutputOracle: mustGetAddress("L2OutputOracleProxy"),
+                        optimismPortal: mustGetAddress("OptimismPortalProxy")
+                    })
                 )
             )
         });
@@ -531,8 +532,7 @@ contract Deploy is Deployer {
         require(config.l2OutputOracle() == mustGetAddress("L2OutputOracleProxy"));
         require(config.optimismPortal() == mustGetAddress("OptimismPortalProxy"));
         require(config.l1CrossDomainMessenger() == mustGetAddress("L1CrossDomainMessengerProxy"));
-        require(config.startBlock() == 0); // TODO
-        */
+        require(config.startBlock() == cfg.systemConfigStartBlock());
     }
 
     /// @notice Initialize the L1StandardBridge

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

@@ -47,6 +47,7 @@ contract DeployConfig is Script {
     uint256 public faultGameAbsolutePrestate;
     uint256 public faultGameMaxDepth;
     uint256 public faultGameMaxDuration;
+    uint256 public systemConfigStartBlock;
 
     constructor(string memory _path) {
         console.log("DeployConfig: reading file %s", _path);
@@ -87,6 +88,7 @@ contract DeployConfig is Script {
         gasPriceOracleScalar = stdJson.readUint(_json, "$.gasPriceOracleScalar");
         eip1559Denominator = stdJson.readUint(_json, "$.eip1559Denominator");
         eip1559Elasticity = stdJson.readUint(_json, "$.eip1559Elasticity");
+        systemConfigStartBlock = stdJson.readUint(_json, "$.systemConfigStartBlock");
 
         if (block.chainid == Chains.LocalDevnet || block.chainid == Chains.GethDevnet) {
             faultGameAbsolutePrestate = stdJson.readUint(_json, "$.faultGameAbsolutePrestate");

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

@@ -31,7 +31,6 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         address l1StandardBridge;
         address l2OutputOracle;
         address optimismPortal;
-        //address batchInbox;
     }
 
     /// @notice Version identifier, used for upgrades.
@@ -108,7 +107,15 @@ contract SystemConfig is OwnableUpgradeable, Semver {
                 systemTxMaxGas: 0,
                 maximumBaseFee: 0
             }),
-            _startBlock: 0
+            _startBlock: 0,
+            _batchInbox: address(0),
+            _addresses: SystemConfig.Addresses({
+                l1CrossDomainMessenger: address(0),
+                l1ERC721Bridge: address(0),
+                l1StandardBridge: address(0),
+                l2OutputOracle: address(0),
+                optimismPortal: address(0)
+            })
         });
     }
 
@@ -121,6 +128,13 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     /// @param _gasLimit          Initial gas limit.
     /// @param _unsafeBlockSigner Initial unsafe block signer address.
     /// @param _config            Initial ResourceConfig.
+    /// @param _startBlock        Starting block for the op-node to search for logs from.
+    ///                           Contracts that were deployed before this field existed
+    ///                           need to have this field set manually via an override.
+    ///                           Newly deployed contracts should set this value to uint256(0).
+    /// @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.
     function initialize(
         address _owner,
         uint256 _overhead,
@@ -129,11 +143,26 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         uint64 _gasLimit,
         address _unsafeBlockSigner,
         ResourceMetering.ResourceConfig memory _config,
-        uint256 _startBlock
+        uint256 _startBlock,
+        address _batchInbox,
+        SystemConfig.Addresses memory _addresses
     ) public reinitializer(2) {
         __Ownable_init();
         transferOwnership(_owner);
 
+        overhead = _overhead;
+        scalar = _scalar;
+        batcherHash = _batcherHash;
+        gasLimit = _gasLimit;
+
+        _setAddress(_unsafeBlockSigner, UNSAFE_BLOCK_SIGNER_SLOT);
+        _setAddress(_batchInbox, BATCH_INBOX_SLOT);
+        _setAddress(_addresses.l1CrossDomainMessenger, L1_CROSS_DOMAIN_MESSENGER_SLOT);
+        _setAddress(_addresses.l1ERC721Bridge, L1_ERC_721_BRIDGE_SLOT);
+        _setAddress(_addresses.l1StandardBridge, L1_STANDARD_BRIDGE_SLOT);
+        _setAddress(_addresses.l2OutputOracle, L2_OUTPUT_ORACLE_SLOT);
+        _setAddress(_addresses.optimismPortal, OPTIMISM_PORTAL_SLOT);
+
         // 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.
@@ -144,41 +173,10 @@ contract SystemConfig is OwnableUpgradeable, Semver {
             startBlock = block.number;
         }
 
-        overhead = _overhead;
-        scalar = _scalar;
-        batcherHash = _batcherHash;
-        gasLimit = _gasLimit;
-        _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);
-        _setAddress(_batchInbox, BATCH_INBOX_SLOT);
-    }
-
     /// @notice Returns the minimum L2 gas limit that can be safely set for the system to
     ///         operate. The L2 gas limit must be larger than or equal to the amount of
     ///         gas that is allocated for deposits per block plus the amount of gas that

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

@@ -30,14 +30,22 @@ contract SystemConfig_Init is CommonTest {
             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
+                    alice,                                   //_owner,
+                    2100,                                    //_overhead,
+                    1000000,                                 //_scalar,
+                    bytes32(hex"abcd"),                      //_batcherHash,
+                    30_000_000,                              //_gasLimit,
+                    address(1),                              //_unsafeBlockSigner,
+                    Constants.DEFAULT_RESOURCE_CONFIG(),     //_config,
+                    0,                                       //_startBlock
+                    address(0),                              // _batchInbox
+                    SystemConfig.Addresses({                 // _addresses
+                        l1CrossDomainMessenger: address(0),
+                        l1ERC721Bridge: address(0),
+                        l1StandardBridge: address(0),
+                        l2OutputOracle: address(0),
+                        optimismPortal: address(0)
+                    })
                 )
             )
         );
@@ -68,14 +76,22 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
             abi.encodeCall(
                 SystemConfig.initialize,
                 (
-                    alice,                 //_owner,
-                    2100,                  //_overhead,
-                    1000000,               //_scalar,
-                    bytes32(hex"abcd"),    //_batcherHash,
-                    minimumGasLimit - 1,   //_gasLimit,
-                    address(1),            //_unsafeBlockSigner,
-                    cfg,                   //_config,
-                    0                      //_startBlock
+                    alice,                                  //_owner,
+                    2100,                                   //_overhead,
+                    1000000,                                //_scalar,
+                    bytes32(hex"abcd"),                     //_batcherHash,
+                    minimumGasLimit - 1,                    //_gasLimit,
+                    address(1),                             //_unsafeBlockSigner,
+                    cfg,                                    //_config,
+                    0,                                      //_startBlock
+                    address(0),                             // _batchInbox
+                    SystemConfig.Addresses({                // _addresses
+                        l1CrossDomainMessenger: address(0),
+                        l1ERC721Bridge: address(0),
+                        l1StandardBridge: address(0),
+                        l2OutputOracle: address(0),
+                        optimismPortal: address(0)
+                    })
                 )
             )
         );

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

@@ -14,6 +14,14 @@ contract SystemConfig_GasLimitLowerBound_Invariant is Test {
         Proxy proxy = new Proxy(msg.sender);
         SystemConfig configImpl = new SystemConfig();
 
+        SystemConfig.Addresses memory addrs = SystemConfig.Addresses({
+            l1CrossDomainMessenger: address(0),
+            l1ERC721Bridge: address(0),
+            l1StandardBridge: address(0),
+            l2OutputOracle: address(0),
+            optimismPortal: address(0)
+        });
+
         proxy.upgradeToAndCall(
             address(configImpl),
             abi.encodeCall(
@@ -26,7 +34,9 @@ contract SystemConfig_GasLimitLowerBound_Invariant is Test {
                     30_000_000,                           // gas limit
                     address(1),                           // unsafe block signer
                     Constants.DEFAULT_RESOURCE_CONFIG(),  // resource config
-                    0                                     //_startBlock
+                    0,                                    //_startBlock
+                    address(0),                           // _batchInbox
+                    addrs                                 // addresses
                 )
             )
         );