Merge pull request #4546 from ethereum-optimism/fix/deterministic-storage

contracts-bedrock: deterministic storage

op-node/node/runtime_config.go

@@ -16,7 +16,7 @@ import (
 var (
 	// UnsafeBlockSignerAddressSystemConfigStorageSlot is the storage slot identifier of the unsafeBlockSigner
 	// `address` storage value in the SystemConfig L1 contract.
-	UnsafeBlockSignerAddressSystemConfigStorageSlot = common.Hash{31: 103}
+	UnsafeBlockSignerAddressSystemConfigStorageSlot = common.HexToHash("0x65a7ed542fb37fe237fdfbdd70b31598523fe5b32879e307bae27a0bd9581c08")
 )
 
 type RuntimeCfgL1Source interface {

packages/contracts-bedrock/.gas-snapshot

@@ -379,8 +379,8 @@ SequencerFeeVault_Test:test_minWithdrawalAmount_succeeds() (gas: 5420)
 SequencerFeeVault_Test:test_receive_succeeds() (gas: 17336)
 SequencerFeeVault_Test:test_withdraw_notEnough_reverts() (gas: 9309)
 SequencerFeeVault_Test:test_withdraw_succeeds() (gas: 159816)
-SystemConfig_Initialize_TestFail:test_initialize_lowGasLimit_reverts() (gas: 61952)
-SystemConfig_Setters_TestFail:test_setBatcherHash_notOwner_reverts() (gas: 10523)
-SystemConfig_Setters_TestFail:test_setGasConfig_notOwner_reverts() (gas: 10510)
-SystemConfig_Setters_TestFail:test_setGasLimit_notOwner_reverts() (gas: 10614)
+SystemConfig_Initialize_TestFail:test_initialize_lowGasLimit_reverts() (gas: 61966)
+SystemConfig_Setters_TestFail:test_setBatcherHash_notOwner_reverts() (gas: 10545)
+SystemConfig_Setters_TestFail:test_setGasConfig_notOwner_reverts() (gas: 10532)
+SystemConfig_Setters_TestFail:test_setGasLimit_notOwner_reverts() (gas: 10636)
 SystemConfig_Setters_TestFail:test_setUnsafeBlockSigner_notOwner_reverts() (gas: 10638)

packages/contracts-bedrock/.storage-layout

@@ -104,29 +104,27 @@
 ➡ contracts/L1/SystemConfig.sol:SystemConfig
 =======================
 
-+-------------------+-------------+------+--------+-------+--------------------------------------------+
-| Name              | Type        | Slot | Offset | Bytes | Contract                                   |
-+======================================================================================================+
-| _initialized      | uint8       | 0    | 0      | 1     | contracts/L1/SystemConfig.sol:SystemConfig |
-|-------------------+-------------+------+--------+-------+--------------------------------------------|
-| _initializing     | bool        | 0    | 1      | 1     | contracts/L1/SystemConfig.sol:SystemConfig |
-|-------------------+-------------+------+--------+-------+--------------------------------------------|
-| __gap             | uint256[50] | 1    | 0      | 1600  | contracts/L1/SystemConfig.sol:SystemConfig |
-|-------------------+-------------+------+--------+-------+--------------------------------------------|
-| _owner            | address     | 51   | 0      | 20    | contracts/L1/SystemConfig.sol:SystemConfig |
-|-------------------+-------------+------+--------+-------+--------------------------------------------|
-| __gap             | uint256[49] | 52   | 0      | 1568  | contracts/L1/SystemConfig.sol:SystemConfig |
-|-------------------+-------------+------+--------+-------+--------------------------------------------|
-| overhead          | uint256     | 101  | 0      | 32    | contracts/L1/SystemConfig.sol:SystemConfig |
-|-------------------+-------------+------+--------+-------+--------------------------------------------|
-| scalar            | uint256     | 102  | 0      | 32    | contracts/L1/SystemConfig.sol:SystemConfig |
-|-------------------+-------------+------+--------+-------+--------------------------------------------|
-| unsafeBlockSigner | address     | 103  | 0      | 20    | contracts/L1/SystemConfig.sol:SystemConfig |
-|-------------------+-------------+------+--------+-------+--------------------------------------------|
-| batcherHash       | bytes32     | 104  | 0      | 32    | contracts/L1/SystemConfig.sol:SystemConfig |
-|-------------------+-------------+------+--------+-------+--------------------------------------------|
-| gasLimit          | uint64      | 105  | 0      | 8     | contracts/L1/SystemConfig.sol:SystemConfig |
-+-------------------+-------------+------+--------+-------+--------------------------------------------+
++---------------+-------------+------+--------+-------+--------------------------------------------+
+| Name          | Type        | Slot | Offset | Bytes | Contract                                   |
++==================================================================================================+
+| _initialized  | uint8       | 0    | 0      | 1     | contracts/L1/SystemConfig.sol:SystemConfig |
+|---------------+-------------+------+--------+-------+--------------------------------------------|
+| _initializing | bool        | 0    | 1      | 1     | contracts/L1/SystemConfig.sol:SystemConfig |
+|---------------+-------------+------+--------+-------+--------------------------------------------|
+| __gap         | uint256[50] | 1    | 0      | 1600  | contracts/L1/SystemConfig.sol:SystemConfig |
+|---------------+-------------+------+--------+-------+--------------------------------------------|
+| _owner        | address     | 51   | 0      | 20    | contracts/L1/SystemConfig.sol:SystemConfig |
+|---------------+-------------+------+--------+-------+--------------------------------------------|
+| __gap         | uint256[49] | 52   | 0      | 1568  | contracts/L1/SystemConfig.sol:SystemConfig |
+|---------------+-------------+------+--------+-------+--------------------------------------------|
+| overhead      | uint256     | 101  | 0      | 32    | contracts/L1/SystemConfig.sol:SystemConfig |
+|---------------+-------------+------+--------+-------+--------------------------------------------|
+| scalar        | uint256     | 102  | 0      | 32    | contracts/L1/SystemConfig.sol:SystemConfig |
+|---------------+-------------+------+--------+-------+--------------------------------------------|
+| batcherHash   | bytes32     | 103  | 0      | 32    | contracts/L1/SystemConfig.sol:SystemConfig |
+|---------------+-------------+------+--------+-------+--------------------------------------------|
+| gasLimit      | uint64      | 104  | 0      | 8     | contracts/L1/SystemConfig.sol:SystemConfig |
++---------------+-------------+------+--------+-------+--------------------------------------------+
 
 =======================
 ➡ contracts/legacy/DeployerWhitelist.sol:DeployerWhitelist

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

@@ -34,6 +34,13 @@ contract SystemConfig is OwnableUpgradeable, Semver {
      */
     uint256 public constant VERSION = 0;
 
+    /**
+     * @notice Storage slot that the unsafe block signer is stored at. Storing it at this
+     *         deterministic storage slot allows for decoupling the storage layout from the way
+     *         that `solc` lays out storage. The `op-node` uses a storage proof to fetch this value.
+     */
+    bytes32 public constant UNSAFE_BLOCK_SIGNER_SLOT = keccak256("systemconfig.unsafeblocksigner");
+
     /**
      * @notice Minimum gas limit. This should not be lower than the maximum deposit gas resource
      *         limit in the ResourceMetering contract used by OptimismPortal, to ensure the L2
@@ -51,13 +58,6 @@ contract SystemConfig is OwnableUpgradeable, Semver {
      */
     uint256 public scalar;
 
-    /**
-     * @notice Address corresponding to the key that can propagate unsafe blocks
-     *         across the p2p network. This value should not be tightly packed
-     *         into a storage slot with another value to make state proofs more simple.
-     */
-    address public unsafeBlockSigner;
-
     /**
      * @notice Identifier for the batcher. For version 1 of this configuration, this is represented
      *         as an address left-padded with zeros to 32 bytes.
@@ -122,7 +122,21 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         scalar = _scalar;
         batcherHash = _batcherHash;
         gasLimit = _gasLimit;
-        unsafeBlockSigner = _unsafeBlockSigner;
+        _setUnsafeBlockSigner(_unsafeBlockSigner);
+    }
+
+    /**
+     * @notice High level getter for the unsafe block signer address.
+     *         Unsafe blocks can be propagated across the p2p network
+     *         if they are signed by the key corresponding to this address.
+     */
+    function unsafeBlockSigner() public view returns (address) {
+        address addr;
+        bytes32 slot = UNSAFE_BLOCK_SIGNER_SLOT;
+        assembly {
+            addr := sload(slot)
+        }
+        return addr;
     }
 
     /**
@@ -153,12 +167,26 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     }
 
     function setUnsafeBlockSigner(address _unsafeBlockSigner) external onlyOwner {
-        unsafeBlockSigner = _unsafeBlockSigner;
+        _setUnsafeBlockSigner(_unsafeBlockSigner);
 
         bytes memory data = abi.encode(_unsafeBlockSigner);
         emit ConfigUpdate(VERSION, UpdateType.UNSAFE_BLOCK_SIGNER, 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 Updates the L2 gas limit.
      *