Merge pull request #6818 from ethereum-optimism/fix/sys-cfg-events

contracts-bedrock: system config fix

packages/contracts-bedrock/.gas-snapshot

@@ -643,10 +643,11 @@ SequencerFeeVault_Test:test_withdraw_toL1_succeeds() (gas: 171357)
 SetPrevBaseFee_Test:test_setPrevBaseFee_succeeds() (gas: 11549)
 StandardBridge_Stateless_Test:test_isCorrectTokenPair_succeeds() (gas: 49936)
 StandardBridge_Stateless_Test:test_isOptimismMintableERC20_succeeds() (gas: 33072)
-SystemConfig_Initialize_Test:test_initialize_startBlockOverride_succeeds() (gas: 55905)
+SystemConfig_Initialize_Test:test_initialize_events_succeeds() (gas: 72059)
+SystemConfig_Initialize_Test:test_initialize_startBlockOverride_succeeds() (gas: 65240)
 SystemConfig_Initialize_Test:test_initialize_values_succeeds() (gas: 64946)
-SystemConfig_Initialize_TestFail:test_initialize_lowGasLimit_reverts() (gas: 94515)
-SystemConfig_Initialize_TestFail:test_initialize_startBlock_reverts() (gas: 64676)
+SystemConfig_Initialize_TestFail:test_initialize_lowGasLimit_reverts() (gas: 55653)
+SystemConfig_Initialize_TestFail:test_initialize_startBlock_reverts() (gas: 78350)
 SystemConfig_Setters_TestFail:test_setBatcherHash_notOwner_reverts() (gas: 15607)
 SystemConfig_Setters_TestFail:test_setGasConfig_notOwner_reverts() (gas: 15577)
 SystemConfig_Setters_TestFail:test_setGasLimit_notOwner_reverts() (gas: 15676)

packages/contracts-bedrock/semver-lock.json

@@ -6,7 +6,7 @@
   "src/L1/L1StandardBridge.sol": "0xbd7b303cefe46bc14bf1a2b81e5702ff45ce9c5257524e59778e11c75f7f5bdc",
   "src/L1/L2OutputOracle.sol": "0x05ea17a834563ffa50cade81189b120b6f0805ba316d6a9893c8cf8b231e57e3",
   "src/L1/OptimismPortal.sol": "0xeefcc16d30e14ed7ce9970f3aeaf1d5668324b3fc1ddb4790da5804cfdd78980",
-  "src/L1/SystemConfig.sol": "0x29beec0a03b9602a53e3ceaec2354972d917f8b80f1b3a8f03f4fb7a67753fce",
+  "src/L1/SystemConfig.sol": "0xd5904b53153f2e32be28f2ce1ea51374b19fdf4f478f486f02cf67d8b62b7c29",
   "src/L2/BaseFeeVault.sol": "0xd8df28898799b80c370e77e9aad09f79235dfda2bf13e56daf21997cfe54200d",
   "src/L2/GasPriceOracle.sol": "0xb7d8c4f3ea8db31900125e341aae42a862a2b7d3f1c1aa60c97dc2d0e022b7ba",
   "src/L2/L1Block.sol": "0x38ea78a9611656a60ae4d58db75e96413a638e3ccb2e935052441f98a1fd3105",

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

@@ -98,11 +98,11 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     /// @notice The block at which the op-node can start searching for logs from.
     uint256 public startBlock;
 
-    /// @custom:semver 1.5.0
+    /// @custom:semver 1.6.0
     /// @notice Constructs the SystemConfig contract. Cannot set
     ///         the owner to `address(0)` due to the Ownable contract's
     ///         implementation, so set it to `address(0xdEaD)`
-    constructor() Semver(1, 5, 0) {
+    constructor() Semver(1, 6, 0) {
         initialize({
             _owner: address(0xdEaD),
             _overhead: 0,
@@ -165,12 +165,12 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         __Ownable_init();
         transferOwnership(_owner);
 
-        overhead = _overhead;
-        scalar = _scalar;
-        batcherHash = _batcherHash;
-        gasLimit = _gasLimit;
+        // These are set in ascending order of their UpdateTypes.
+        _setBatcherHash(_batcherHash);
+        _setGasConfig({ _overhead: _overhead, _scalar: _scalar });
+        _setGasLimit(_gasLimit);
+        _setUnsafeBlockSigner(_unsafeBlockSigner);
 
-        _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);
@@ -282,28 +282,47 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         }
     }
 
-    /// @notice Updates the unsafe block signer address.
+    /// @notice Updates the unsafe block signer address. Can only be called by the owner.
     /// @param _unsafeBlockSigner New unsafe block signer address.
     function setUnsafeBlockSigner(address _unsafeBlockSigner) external onlyOwner {
+        _setUnsafeBlockSigner(_unsafeBlockSigner);
+    }
+
+    /// @notice Updates the unsafe block signer address.
+    /// @param _unsafeBlockSigner New unsafe block signer address.
+    function _setUnsafeBlockSigner(address _unsafeBlockSigner) internal {
         _setAddress(_unsafeBlockSigner, UNSAFE_BLOCK_SIGNER_SLOT);
 
         bytes memory data = abi.encode(_unsafeBlockSigner);
         emit ConfigUpdate(VERSION, UpdateType.UNSAFE_BLOCK_SIGNER, data);
     }
 
-    /// @notice Updates the batcher hash.
+    /// @notice Updates the batcher hash. Can only be called by the owner.
     /// @param _batcherHash New batcher hash.
     function setBatcherHash(bytes32 _batcherHash) external onlyOwner {
+        _setBatcherHash(_batcherHash);
+    }
+
+    /// @notice Internal function for updating the batcher hash.
+    /// @param _batcherHash New batcher hash.
+    function _setBatcherHash(bytes32 _batcherHash) internal {
         batcherHash = _batcherHash;
 
         bytes memory data = abi.encode(_batcherHash);
         emit ConfigUpdate(VERSION, UpdateType.BATCHER, data);
     }
 
-    /// @notice Updates gas config.
+    /// @notice Updates gas config. Can only be called by the owner.
     /// @param _overhead New overhead value.
     /// @param _scalar   New scalar value.
     function setGasConfig(uint256 _overhead, uint256 _scalar) external onlyOwner {
+        _setGasConfig(_overhead, _scalar);
+    }
+
+    /// @notice Internal function for updating the gas config.
+    /// @param _overhead New overhead value.
+    /// @param _scalar   New scalar value.
+    function _setGasConfig(uint256 _overhead, uint256 _scalar) internal {
         overhead = _overhead;
         scalar = _scalar;
 
@@ -311,9 +330,15 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         emit ConfigUpdate(VERSION, UpdateType.GAS_CONFIG, data);
     }
 
-    /// @notice Updates the L2 gas limit.
+    /// @notice Updates the L2 gas limit. Can only be called by the owner.
     /// @param _gasLimit New gas limit.
     function setGasLimit(uint64 _gasLimit) external onlyOwner {
+        _setGasLimit(_gasLimit);
+    }
+
+    /// @notice Internal function for updating the L2 gas limit.
+    /// @param _gasLimit New gas limit.
+    function _setGasLimit(uint64 _gasLimit) internal {
         require(_gasLimit >= minimumGasLimit(), "SystemConfig: gas limit too low");
         gasLimit = _gasLimit;
 

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

@@ -51,6 +51,9 @@ contract CommonTest is Test {
 
     event TransactionDeposited(address indexed from, address indexed to, uint256 indexed version, bytes opaqueData);
 
+    /// @dev OpenZeppelin Ownable.sol transferOwnership event
+    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
+
     FFIInterface ffi;
 
     function setUp() public virtual {

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

@@ -26,9 +26,6 @@ contract XDomainSetter3 is CrossDomainOwnable3 {
 contract CrossDomainOwnable3_Test is Messenger_Initializer {
     XDomainSetter3 setter;
 
-    /// @dev OpenZeppelin Ownable.sol transferOwnership event
-    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
-
     /// @dev CrossDomainOwnable3.sol transferOwnership event
     event OwnershipTransferred(address indexed previousOwner, address indexed newOwner, bool isLocal);
 

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

@@ -18,6 +18,8 @@ contract SystemConfig_Init is CommonTest {
     SystemConfig sysConf;
     SystemConfig systemConfigImpl;
 
+    event ConfigUpdate(uint256 indexed version, SystemConfig.UpdateType indexed updateType, bytes data);
+
     // Dummy addresses used to test getters
     address constant batchInbox = address(0x18);
     address constant l1CrossDomainMessenger = address(0x20);
@@ -138,6 +140,51 @@ contract SystemConfig_Initialize_Test is SystemConfig_Init {
         );
         assertEq(sysConf.startBlock(), startBlock);
     }
+
+    /// @dev Ensures that the events are emitted during initialization.
+    function test_initialize_events_succeeds() external {
+        // Wipe out the initialized slot so the proxy can be initialized again
+        vm.store(address(sysConf), bytes32(0), bytes32(0));
+        vm.store(address(sysConf), bytes32(uint256(106)), bytes32(0));
+        assertEq(sysConf.startBlock(), 0);
+
+        // The order depends here
+        vm.expectEmit(true, true, true, true, address(sysConf));
+        emit ConfigUpdate(0, SystemConfig.UpdateType.BATCHER, abi.encode(batcherHash));
+        vm.expectEmit(true, true, true, true, address(sysConf));
+        emit ConfigUpdate(0, SystemConfig.UpdateType.GAS_CONFIG, abi.encode(overhead, scalar));
+        vm.expectEmit(true, true, true, true, address(sysConf));
+        emit ConfigUpdate(0, SystemConfig.UpdateType.GAS_LIMIT, abi.encode(gasLimit));
+        vm.expectEmit(true, true, true, true, address(sysConf));
+        emit ConfigUpdate(0, SystemConfig.UpdateType.UNSAFE_BLOCK_SIGNER, abi.encode(unsafeBlockSigner));
+
+        vm.prank(multisig);
+        Proxy(payable(address(sysConf))).upgradeToAndCall(
+            address(systemConfigImpl),
+            abi.encodeCall(
+                SystemConfig.initialize,
+                (
+                    alice, // _owner,
+                    overhead, // _overhead,
+                    scalar, // _scalar,
+                    batcherHash, // _batcherHash
+                    gasLimit, // _gasLimit,
+                    unsafeBlockSigner, // _unsafeBlockSigner,
+                    Constants.DEFAULT_RESOURCE_CONFIG(), // _config,
+                    0, // _startBlock
+                    batchInbox, // _batchInbox
+                    SystemConfig.Addresses({ // _addresses
+                        l1CrossDomainMessenger: l1CrossDomainMessenger,
+                        l1ERC721Bridge: l1ERC721Bridge,
+                        l1StandardBridge: l1StandardBridge,
+                        l2OutputOracle: l2OutputOracle,
+                        optimismPortal: optimismPortal,
+                        optimismMintableERC20Factory: optimismMintableERC20Factory
+                    })
+                )
+            )
+        );
+    }
 }
 
 contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
@@ -318,8 +365,6 @@ contract SystemConfig_Setters_TestFail is SystemConfig_Init {
 }
 
 contract SystemConfig_Setters_Test is SystemConfig_Init {
-    event ConfigUpdate(uint256 indexed version, SystemConfig.UpdateType indexed updateType, bytes data);
-
     /// @dev Tests that `setBatcherHash` updates the batcher hash successfully.
     function testFuzz_setBatcherHash_succeeds(bytes32 newBatcherHash) external {
         vm.expectEmit(true, true, true, true);