Merge pull request #6566 from ethereum-optimism/feat/oracle-immutables

feat: enable L2OutputOracle impl usage by multiple proxies

op-proposer/proposer/abi_test.go

@@ -33,11 +33,8 @@ func setupL2OutputOracle() (common.Address, *bind.TransactOpts, *backends.Simula
 		backend,
 		big.NewInt(10),
 		big.NewInt(2),
-		big.NewInt(0),
-		big.NewInt(0),
-		from,
-		common.Address{0xdd},
-		big.NewInt(100))
+		big.NewInt(100),
+	)
 	if err != nil {
 		return common.Address{}, nil, nil, nil, err
 	}

packages/chain-mon/test/fault-mon/helpers.spec.ts

@@ -3,6 +3,7 @@ import '@nomiclabs/hardhat-ethers'
 import { Contract, utils } from 'ethers'
 import { toRpcHexString } from '@eth-optimism/core-utils'
 import Artifact__L2OutputOracle from '@eth-optimism/contracts-bedrock/forge-artifacts/L2OutputOracle.sol/L2OutputOracle.json'
+import Artifact__Proxy from '@eth-optimism/contracts-bedrock/forge-artifacts/Proxy.sol/Proxy.json'
 import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
 
 import { expect } from './setup'
@@ -29,22 +30,43 @@ describe('helpers', () => {
   })
 
   let L2OutputOracle: Contract
+  let Proxy: Contract
   beforeEach(async () => {
+    const Factory__Proxy = new hre.ethers.ContractFactory(
+      Artifact__Proxy.abi,
+      Artifact__Proxy.bytecode.object,
+      signer
+    )
+
+    Proxy = await Factory__Proxy.deploy(signer.address)
+
     const Factory__L2OutputOracle = new hre.ethers.ContractFactory(
       Artifact__L2OutputOracle.abi,
       Artifact__L2OutputOracle.bytecode.object,
       signer
     )
 
-    L2OutputOracle = await Factory__L2OutputOracle.deploy(
+    const L2OutputOracleImplementation = await Factory__L2OutputOracle.deploy(
       deployConfig.l2OutputOracleSubmissionInterval,
       deployConfig.l2BlockTime,
-      deployConfig.l2OutputOracleStartingBlockNumber,
-      deployConfig.l2OutputOracleStartingTimestamp,
-      deployConfig.l2OutputOracleProposer,
-      deployConfig.l2OutputOracleChallenger,
       deployConfig.finalizationPeriodSeconds
     )
+
+    await Proxy.upgradeToAndCall(
+      L2OutputOracleImplementation.address,
+      L2OutputOracleImplementation.interface.encodeFunctionData('initialize', [
+        deployConfig.l2OutputOracleStartingBlockNumber,
+        deployConfig.l2OutputOracleStartingTimestamp,
+        deployConfig.l2OutputOracleProposer,
+        deployConfig.l2OutputOracleChallenger,
+      ])
+    )
+
+    L2OutputOracle = new hre.ethers.Contract(
+      Proxy.address,
+      Artifact__L2OutputOracle.abi,
+      signer
+    )
   })
 
   describe('findOutputForIndex', () => {

packages/contracts-bedrock/.storage-layout

@@ -49,6 +49,8 @@
 | startingBlockNumber | uint256                       | 1    | 0      | 32    | src/L1/L2OutputOracle.sol:L2OutputOracle |
 | startingTimestamp   | uint256                       | 2    | 0      | 32    | src/L1/L2OutputOracle.sol:L2OutputOracle |
 | l2Outputs           | struct Types.OutputProposal[] | 3    | 0      | 32    | src/L1/L2OutputOracle.sol:L2OutputOracle |
+| challenger          | address                       | 4    | 0      | 20    | src/L1/L2OutputOracle.sol:L2OutputOracle |
+| proposer            | address                       | 5    | 0      | 20    | src/L1/L2OutputOracle.sol:L2OutputOracle |
 
 =======================
 ➡ src/L1/OptimismPortal.sol:OptimismPortal

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

@@ -311,20 +311,21 @@ contract Deploy is Deployer {
         L2OutputOracle oracle = new L2OutputOracle({
             _submissionInterval: cfg.l2OutputOracleSubmissionInterval(),
             _l2BlockTime: cfg.l2BlockTime(),
-            _startingBlockNumber: cfg.l2OutputOracleStartingBlockNumber(),
-            _startingTimestamp: cfg.l2OutputOracleStartingTimestamp(),
-            _proposer: cfg.l2OutputOracleProposer(),
-            _challenger: cfg.l2OutputOracleChallenger(),
             _finalizationPeriodSeconds: cfg.finalizationPeriodSeconds()
         });
 
         require(oracle.SUBMISSION_INTERVAL() == cfg.l2OutputOracleSubmissionInterval());
+        require(oracle.submissionInterval() == cfg.l2OutputOracleSubmissionInterval());
         require(oracle.L2_BLOCK_TIME() == cfg.l2BlockTime());
-        require(oracle.PROPOSER() == cfg.l2OutputOracleProposer());
-        require(oracle.CHALLENGER() == cfg.l2OutputOracleChallenger());
+        require(oracle.l2BlockTime() == cfg.l2BlockTime());
+        require(oracle.PROPOSER() == address(0));
+        require(oracle.proposer() == address(0));
+        require(oracle.CHALLENGER() == address(0));
+        require(oracle.challenger() == address(0));
         require(oracle.FINALIZATION_PERIOD_SECONDS() == cfg.finalizationPeriodSeconds());
-        require(oracle.startingBlockNumber() == cfg.l2OutputOracleStartingBlockNumber());
-        require(oracle.startingTimestamp() == cfg.l2OutputOracleStartingTimestamp());
+        require(oracle.finalizationPeriodSeconds() == cfg.finalizationPeriodSeconds());
+        require(oracle.startingBlockNumber() == 0);
+        require(oracle.startingTimestamp() == 0);
 
         save("L2OutputOracle", address(oracle));
         console.log("L2OutputOracle deployed at %s", address(oracle));
@@ -651,7 +652,9 @@ contract Deploy is Deployer {
                 L2OutputOracle.initialize,
                 (
                     cfg.l2OutputOracleStartingBlockNumber(),
-                    cfg.l2OutputOracleStartingTimestamp()
+                    cfg.l2OutputOracleStartingTimestamp(),
+                    cfg.l2OutputOracleProposer(),
+                    cfg.l2OutputOracleChallenger()
                 )
             )
         });
@@ -661,10 +664,15 @@ contract Deploy is Deployer {
         console.log("L2OutputOracle version: %s", version);
 
         require(oracle.SUBMISSION_INTERVAL() == cfg.l2OutputOracleSubmissionInterval());
+        require(oracle.submissionInterval() == cfg.l2OutputOracleSubmissionInterval());
         require(oracle.L2_BLOCK_TIME() == cfg.l2BlockTime());
+        require(oracle.l2BlockTime() == cfg.l2BlockTime());
         require(oracle.PROPOSER() == cfg.l2OutputOracleProposer());
+        require(oracle.proposer() == cfg.l2OutputOracleProposer());
         require(oracle.CHALLENGER() == cfg.l2OutputOracleChallenger());
+        require(oracle.challenger() == cfg.l2OutputOracleChallenger());
         require(oracle.FINALIZATION_PERIOD_SECONDS() == cfg.finalizationPeriodSeconds());
+        require(oracle.finalizationPeriodSeconds() == cfg.finalizationPeriodSeconds());
         require(oracle.startingBlockNumber() == cfg.l2OutputOracleStartingBlockNumber());
         require(oracle.startingTimestamp() == cfg.l2OutputOracleStartingTimestamp());
     }

packages/contracts-bedrock/semver-lock.json

@@ -2,7 +2,7 @@
   "src/L1/L1CrossDomainMessenger.sol": "0x2b276f14475869cfd81868b03dc72b91dd726a787c9568caf4849fe34830b207",
   "src/L1/L1ERC721Bridge.sol": "0xac9d8e236a1b35c358f9800834f65375cf4270155e817cf3d0f2bac1968f9107",
   "src/L1/L1StandardBridge.sol": "0x26fd79e041c403f4bc68758c410fcc801975e7648c0b51a2c4a6e8c44fabcbfd",
-  "src/L1/L2OutputOracle.sol": "0xd6c5eb38732077c4705f46a61be68a7beccc069a99ed1d07b8e1fc6e1de8ffa6",
+  "src/L1/L2OutputOracle.sol": "0x8f32ccb4c5cb63a459a0e79ee412177dc03fd279fdaaf1dac69e8c714902e857",
   "src/L1/OptimismPortal.sol": "0xeaa47a63e8a3bcfdb7dfd3e6c8608369e34e362d9de82f3acf13cbc27c070bf7",
   "src/L1/SystemConfig.sol": "0x8e2b5103d2eb93b74af2e2f96a4505e637cdc3c44d80cf5ec2eca70060e1deff",
   "src/L2/BaseFeeVault.sol": "0xa596e60762f16192cfa86459fcb9f4da9d8f756106eedac643a1ffeafbbfcc5f",

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

@@ -14,18 +14,21 @@ contract L2OutputOracle is Initializable, Semver {
     /// @notice The interval in L2 blocks at which checkpoints must be submitted.
     ///         Although this is immutable, it can safely be modified by upgrading the
     ///         implementation contract.
+    ///         Public getter is legacy and will be removed in the future. Use `submissionInterval`
+    ///         instead.
+    /// @custom:legacy
     uint256 public immutable SUBMISSION_INTERVAL;
 
     /// @notice The time between L2 blocks in seconds. Once set, this value MUST NOT be modified.
+    ///         Public getter is legacy and will be removed in the future. Use `l2BlockTime`
+    ///         instead.
+    /// @custom:legacy
     uint256 public immutable L2_BLOCK_TIME;
 
-    /// @notice The address of the challenger. Can be updated via upgrade.
-    address public immutable CHALLENGER;
-
-    /// @notice The address of the proposer. Can be updated via upgrade.
-    address public immutable PROPOSER;
-
     /// @notice The minimum time (in seconds) that must elapse before a withdrawal can be finalized.
+    ///         Public getter is legacy and will be removed in the future. Use
+    //          `finalizationPeriodSeconds` instead.
+    /// @custom:legacy
     uint256 public immutable FINALIZATION_PERIOD_SECONDS;
 
     /// @notice The number of the first L2 block recorded in this contract.
@@ -37,6 +40,14 @@ contract L2OutputOracle is Initializable, Semver {
     /// @notice An array of L2 output proposals.
     Types.OutputProposal[] internal l2Outputs;
 
+    /// @notice The address of the challenger. Can be updated via reinitialize.
+    /// @custom:network-specific
+    address public challenger;
+
+    /// @notice The address of the proposer. Can be updated via reinitialize.
+    /// @custom:network-specific
+    address public proposer;
+
     /// @notice Emitted when an output is proposed.
     /// @param outputRoot    The output root.
     /// @param l2OutputIndex The index of the output in the l2Outputs array.
@@ -54,23 +65,17 @@ contract L2OutputOracle is Initializable, Semver {
     /// @param newNextOutputIndex  Next L2 output index after the deletion.
     event OutputsDeleted(uint256 indexed prevNextOutputIndex, uint256 indexed newNextOutputIndex);
 
-    /// @custom:semver 1.3.1
+    /// @custom:semver 1.4.0
     /// @notice Constructs the L2OutputOracle contract.
     /// @param _submissionInterval  Interval in blocks at which checkpoints must be submitted.
     /// @param _l2BlockTime         The time per L2 block, in seconds.
-    /// @param _startingBlockNumber The number of the first L2 block.
-    /// @param _startingTimestamp   The timestamp of the first L2 block.
-    /// @param _proposer            The address of the proposer.
-    /// @param _challenger          The address of the challenger.
+    /// @param _finalizationPeriodSeconds The amount of time that must pass for an output proposal
+    //                                    to be considered canonical.
     constructor(
         uint256 _submissionInterval,
         uint256 _l2BlockTime,
-        uint256 _startingBlockNumber,
-        uint256 _startingTimestamp,
-        address _proposer,
-        address _challenger,
         uint256 _finalizationPeriodSeconds
-    ) Semver(1, 3, 1) {
+    ) Semver(1, 4, 0) {
         require(_l2BlockTime > 0, "L2OutputOracle: L2 block time must be greater than 0");
         require(
             _submissionInterval > 0,
@@ -79,20 +84,27 @@ contract L2OutputOracle is Initializable, Semver {
 
         SUBMISSION_INTERVAL = _submissionInterval;
         L2_BLOCK_TIME = _l2BlockTime;
-        PROPOSER = _proposer;
-        CHALLENGER = _challenger;
         FINALIZATION_PERIOD_SECONDS = _finalizationPeriodSeconds;
 
-        initialize(_startingBlockNumber, _startingTimestamp);
+        initialize({
+            _startingBlockNumber: 0,
+            _startingTimestamp: 0,
+            _proposer: address(0),
+            _challenger: address(0)
+        });
     }
 
     /// @notice Initializer.
     /// @param _startingBlockNumber Block number for the first recoded L2 block.
     /// @param _startingTimestamp   Timestamp for the first recoded L2 block.
-    function initialize(uint256 _startingBlockNumber, uint256 _startingTimestamp)
-        public
-        initializer
-    {
+    /// @param _proposer            The address of the proposer.
+    /// @param _challenger          The address of the challenger.
+    function initialize(
+        uint256 _startingBlockNumber,
+        uint256 _startingTimestamp,
+        address _proposer,
+        address _challenger
+    ) public reinitializer(2) {
         require(
             _startingTimestamp <= block.timestamp,
             "L2OutputOracle: starting L2 timestamp must be less than current time"
@@ -100,6 +112,37 @@ contract L2OutputOracle is Initializable, Semver {
 
         startingTimestamp = _startingTimestamp;
         startingBlockNumber = _startingBlockNumber;
+        proposer = _proposer;
+        challenger = _challenger;
+    }
+
+    /// @notice Getter for the output proposal submission interval.
+    function submissionInterval() external view returns (uint256) {
+        return SUBMISSION_INTERVAL;
+    }
+
+    /// @notice Getter for the L2 block time.
+    function l2BlockTime() external view returns (uint256) {
+        return L2_BLOCK_TIME;
+    }
+
+    /// @notice Getter for the finalization period.
+    function finalizationPeriodSeconds() external view returns (uint256) {
+        return FINALIZATION_PERIOD_SECONDS;
+    }
+
+    /// @notice Getter for the challenger address. This will be removed
+    ///         in the future, use `challenger` instead.
+    /// @custom:legacy
+    function CHALLENGER() external view returns (address) {
+        return challenger;
+    }
+
+    /// @notice Getter for the proposer address. This will be removed in the
+    ///         future, use `proposer` instead.
+    /// @custom:legacy
+    function PROPOSER() external view returns (address) {
+        return proposer;
     }
 
     /// @notice Deletes all output proposals after and including the proposal that corresponds to
@@ -109,7 +152,7 @@ contract L2OutputOracle is Initializable, Semver {
     // solhint-disable-next-line ordering
     function deleteL2Outputs(uint256 _l2OutputIndex) external {
         require(
-            msg.sender == CHALLENGER,
+            msg.sender == challenger,
             "L2OutputOracle: only the challenger address can delete outputs"
         );
 
@@ -149,7 +192,7 @@ contract L2OutputOracle is Initializable, Semver {
         uint256 _l1BlockNumber
     ) external payable {
         require(
-            msg.sender == PROPOSER,
+            msg.sender == proposer,
             "L2OutputOracle: only the proposer address can propose new outputs"
         );
 

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

@@ -105,6 +105,7 @@ contract L2OutputOracle_Initializer is CommonTest {
     uint256 internal l2BlockTime = 2;
     uint256 internal startingBlockNumber = 200;
     uint256 internal startingTimestamp = 1000;
+    uint256 internal finalizationPeriodSeconds = 7 days;
     address guardian;
 
     // Test data
@@ -157,17 +158,21 @@ contract L2OutputOracle_Initializer is CommonTest {
         oracleImpl = new L2OutputOracle({
             _submissionInterval: submissionInterval,
             _l2BlockTime: l2BlockTime,
-            _startingBlockNumber: startingBlockNumber,
-            _startingTimestamp: startingTimestamp,
-            _proposer: proposer,
-            _challenger: owner,
-            _finalizationPeriodSeconds: 7 days
+            _finalizationPeriodSeconds: finalizationPeriodSeconds
         });
         Proxy proxy = new Proxy(multisig);
         vm.prank(multisig);
         proxy.upgradeToAndCall(
             address(oracleImpl),
-            abi.encodeCall(L2OutputOracle.initialize, (startingBlockNumber, startingTimestamp))
+            abi.encodeCall(
+                L2OutputOracle.initialize,
+                (
+                    startingBlockNumber,
+                    startingTimestamp,
+                    proposer,
+                    owner
+                )
+            )
         );
         oracle = L2OutputOracle(address(proxy));
         vm.label(address(oracle), "L2OutputOracle");

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

@@ -18,27 +18,18 @@ contract L2OutputOracle_constructor_Test is L2OutputOracle_Initializer {
     /// @dev Tests that constructor sets the initial values correctly.
     function test_constructor_succeeds() external {
         assertEq(oracle.PROPOSER(), proposer);
+        assertEq(oracle.proposer(), proposer);
         assertEq(oracle.CHALLENGER(), owner);
+        assertEq(oracle.challenger(), owner);
         assertEq(oracle.SUBMISSION_INTERVAL(), submissionInterval);
+        assertEq(oracle.submissionInterval(), submissionInterval);
         assertEq(oracle.latestBlockNumber(), startingBlockNumber);
         assertEq(oracle.startingBlockNumber(), startingBlockNumber);
         assertEq(oracle.startingTimestamp(), startingTimestamp);
-    }
-
-    /// @dev Tests that the constructor reverts if the starting timestamp is invalid.
-    function test_constructor_badTimestamp_reverts() external {
-        vm.expectRevert("L2OutputOracle: starting L2 timestamp must be less than current time");
-
-        // startingTimestamp is in the future
-        new L2OutputOracle({
-            _submissionInterval: submissionInterval,
-            _l2BlockTime: l2BlockTime,
-            _startingBlockNumber: startingBlockNumber,
-            _startingTimestamp: block.timestamp + 1,
-            _proposer: proposer,
-            _challenger: owner,
-            _finalizationPeriodSeconds: 7 days
-        });
+        assertEq(oracle.L2_BLOCK_TIME(), l2BlockTime);
+        assertEq(oracle.l2BlockTime(), l2BlockTime);
+        assertEq(oracle.finalizationPeriodSeconds(), finalizationPeriodSeconds);
+        assertEq(oracle.FINALIZATION_PERIOD_SECONDS(), finalizationPeriodSeconds);
     }
 
     /// @dev Tests that the constructor reverts if the l2BlockTime is invalid.
@@ -47,10 +38,6 @@ contract L2OutputOracle_constructor_Test is L2OutputOracle_Initializer {
         new L2OutputOracle({
             _submissionInterval: submissionInterval,
             _l2BlockTime: 0,
-            _startingBlockNumber: startingBlockNumber,
-            _startingTimestamp: block.timestamp,
-            _proposer: proposer,
-            _challenger: owner,
             _finalizationPeriodSeconds: 7 days
         });
     }
@@ -61,13 +48,23 @@ contract L2OutputOracle_constructor_Test is L2OutputOracle_Initializer {
         new L2OutputOracle({
             _submissionInterval: 0,
             _l2BlockTime: l2BlockTime,
-            _startingBlockNumber: startingBlockNumber,
-            _startingTimestamp: block.timestamp,
-            _proposer: proposer,
-            _challenger: owner,
             _finalizationPeriodSeconds: 7 days
         });
     }
+
+    /// @dev Tests that initialize reverts if the starting timestamp is invalid.
+    function test_initialize_badTimestamp_reverts() external {
+        // Reset the initialized field in the 0th storage slot
+        // so that initialize can be called again.
+        vm.store(address(oracle), bytes32(uint256(0)), bytes32(uint256(0)));
+        vm.expectRevert("L2OutputOracle: starting L2 timestamp must be less than current time");
+        oracle.initialize({
+            _startingBlockNumber: 0,
+            _startingTimestamp: block.timestamp + 1,
+            _proposer: address(0),
+            _challenger: address(0)
+        });
+    }
 }
 
 contract L2OutputOracle_getter_Test is L2OutputOracle_Initializer {
@@ -423,25 +420,54 @@ contract L2OutputOracleUpgradeable_Test is L2OutputOracle_Initializer {
 
     /// @dev Tests that the proxy is initialized with the correct values.
     function test_initValuesOnProxy_succeeds() external {
+        assertEq(oracle.SUBMISSION_INTERVAL(), submissionInterval);
+        assertEq(oracle.submissionInterval(), submissionInterval);
+        assertEq(oracle.L2_BLOCK_TIME(), l2BlockTime);
+        assertEq(oracle.l2BlockTime(), l2BlockTime);
+        assertEq(oracle.startingBlockNumber(), startingBlockNumber);
+        assertEq(oracle.startingTimestamp(), startingTimestamp);
+        assertEq(oracle.finalizationPeriodSeconds(), finalizationPeriodSeconds);
+        assertEq(oracle.PROPOSER(), proposer);
+        assertEq(oracle.proposer(), proposer);
+        assertEq(oracle.CHALLENGER(), owner);
+        assertEq(oracle.challenger(), owner);
+    }
+
+    /// @dev Tests that the impl is created with the correct values.
+    function test_initValuesOnImpl_succeeds() external {
         assertEq(submissionInterval, oracleImpl.SUBMISSION_INTERVAL());
         assertEq(l2BlockTime, oracleImpl.L2_BLOCK_TIME());
-        assertEq(startingBlockNumber, oracleImpl.startingBlockNumber());
-        assertEq(startingTimestamp, oracleImpl.startingTimestamp());
 
-        assertEq(proposer, oracleImpl.PROPOSER());
-        assertEq(owner, oracleImpl.CHALLENGER());
+        // The values that are set in the initialize function should be all
+        // zero values in the implementation contract.
+        assertEq(oracleImpl.startingBlockNumber(), 0);
+        assertEq(oracleImpl.startingTimestamp(), 0);
+        assertEq(oracleImpl.PROPOSER(), address(0));
+        assertEq(oracleImpl.proposer(), address(0));
+        assertEq(oracleImpl.CHALLENGER(), address(0));
+        assertEq(oracleImpl.challenger(), address(0));
     }
 
     /// @dev Tests that the proxy cannot be initialized twice.
     function test_initializeProxy_alreadyInitialized_reverts() external {
         vm.expectRevert("Initializable: contract is already initialized");
-        L2OutputOracle(payable(proxy)).initialize(startingBlockNumber, startingTimestamp);
+        L2OutputOracle(payable(proxy)).initialize({
+            _startingBlockNumber: startingBlockNumber,
+            _startingTimestamp: startingTimestamp,
+            _proposer: address(1),
+            _challenger: address(2)
+        });
     }
 
     /// @dev Tests that the implementation contract cannot be initialized twice.
     function test_initializeImpl_alreadyInitialized_reverts() external {
         vm.expectRevert("Initializable: contract is already initialized");
-        L2OutputOracle(oracleImpl).initialize(startingBlockNumber, startingTimestamp);
+        L2OutputOracle(oracleImpl).initialize({
+            _startingBlockNumber: startingBlockNumber,
+            _startingTimestamp: startingTimestamp,
+            _proposer: address(1),
+            _challenger: address(2)
+        });
     }
 
     /// @dev Tests that the proxy can be successfully upgraded.
@@ -454,7 +480,7 @@ contract L2OutputOracleUpgradeable_Test is L2OutputOracle_Initializer {
         vm.startPrank(multisig);
         proxy.upgradeToAndCall(
             address(nextImpl),
-            abi.encodeWithSelector(NextImpl.initialize.selector, 2)
+            abi.encodeWithSelector(NextImpl.initialize.selector, 3)
         );
         assertEq(proxy.implementation(), address(nextImpl));