contracts-bedrock: l2 output oracle storage

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/src/L1/L2OutputOracle.sol

@@ -14,19 +14,13 @@ 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.
-    uint256 public immutable SUBMISSION_INTERVAL;
+    uint256 internal immutable _SUBMISSION_INTERVAL;
 
     /// @notice The time between L2 blocks in seconds. Once set, this value MUST NOT be modified.
-    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;
+    uint256 internal immutable _L2_BLOCK_TIME;
 
     /// @notice The minimum time (in seconds) that must elapse before a withdrawal can be finalized.
-    uint256 public immutable FINALIZATION_PERIOD_SECONDS;
+    uint256 internal immutable _FINALIZATION_PERIOD_SECONDS;
 
     /// @notice The number of the first L2 block recorded in this contract.
     uint256 public startingBlockNumber;
@@ -37,6 +31,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 upgrade.
+    /// @custom:network-specific
+    address internal _CHALLENGER;
+
+    /// @notice The address of the proposer. Can be updated via upgrade.
+    /// @custom:network-specific
+    address internal _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,44 +56,48 @@ 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,
             "L2OutputOracle: submission interval must be greater than 0"
         );
 
-        SUBMISSION_INTERVAL = _submissionInterval;
-        L2_BLOCK_TIME = _l2BlockTime;
-        PROPOSER = _proposer;
-        CHALLENGER = _challenger;
-        FINALIZATION_PERIOD_SECONDS = _finalizationPeriodSeconds;
+        _SUBMISSION_INTERVAL = _submissionInterval;
+        _L2_BLOCK_TIME = _l2BlockTime;
+        _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)
+    /// @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
-        initializer
+        reinitializer(2)
     {
         require(
             _startingTimestamp <= block.timestamp,
@@ -100,6 +106,64 @@ contract L2OutputOracle is Initializable, Semver {
 
         startingTimestamp = _startingTimestamp;
         startingBlockNumber = _startingBlockNumber;
+
+        _PROPOSER = _proposer;
+        _CHALLENGER = _challenger;
+    }
+
+    /// @notice Getter for the output proposal submission interval.
+    /// @custom:legacy
+    function SUBMISSION_INTERVAL() external view returns (uint256) {
+        return _SUBMISSION_INTERVAL;
+    }
+
+    /// @notice Getter for the output proposal submission interval.
+    function submissionInterval() external view returns (uint256) {
+        return _SUBMISSION_INTERVAL;
+    }
+
+    /// @notice Getter for the L2 block time.
+    /// @custom:legacy
+    function L2_BLOCK_TIME() external view returns (uint256) {
+        return _L2_BLOCK_TIME;
+    }
+
+    /// @notice Getter for the L2 block time.
+    function l2BlockTime() external view returns (uint256) {
+        return _L2_BLOCK_TIME;
+    }
+
+    /// @notice Getter for the finalization period.
+    /// @custom:legacy
+    function FINALIZATION_PERIOD_SECONDS() external view returns (uint256) {
+        return _FINALIZATION_PERIOD_SECONDS;
+    }
+
+    /// @notice Getter for the finalization period.
+    function finalizationPeriodSeconds() external view returns (uint256) {
+        return _FINALIZATION_PERIOD_SECONDS;
+    }
+
+    /// @notice Getter for the challenger address.
+    /// @custom:legacy
+    function CHALLENGER() external view returns (address) {
+        return _CHALLENGER;
+    }
+
+    /// @notice Getter for the challenger address.
+    function challenger() external view returns (address) {
+        return _CHALLENGER;
+    }
+
+    /// @notice Getter for the proposer address.
+    /// @custom:legacy
+    function PROPOSER() external view returns (address) {
+        return _PROPOSER;
+    }
+
+    /// @notice Getter for the proposer address.
+    function proposer() external view returns (address) {
+        return _PROPOSER;
     }
 
     /// @notice Deletes all output proposals after and including the proposal that corresponds to
@@ -109,7 +173,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"
         );
 
@@ -121,7 +185,7 @@ contract L2OutputOracle is Initializable, Semver {
 
         // Do not allow deleting any outputs that have already been finalized.
         require(
-            block.timestamp - l2Outputs[_l2OutputIndex].timestamp < FINALIZATION_PERIOD_SECONDS,
+            block.timestamp - l2Outputs[_l2OutputIndex].timestamp < _FINALIZATION_PERIOD_SECONDS,
             "L2OutputOracle: cannot delete outputs that have already been finalized"
         );
 
@@ -149,7 +213,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"
         );
 
@@ -278,13 +342,13 @@ contract L2OutputOracle is Initializable, Semver {
     /// @notice Computes the block number of the next L2 block that needs to be checkpointed.
     /// @return Next L2 block number.
     function nextBlockNumber() public view returns (uint256) {
-        return latestBlockNumber() + SUBMISSION_INTERVAL;
+        return latestBlockNumber() + _SUBMISSION_INTERVAL;
     }
 
     /// @notice Returns the L2 timestamp corresponding to a given L2 block number.
     /// @param _l2BlockNumber The L2 block number of the target block.
     /// @return L2 timestamp of the given block.
     function computeL2Timestamp(uint256 _l2BlockNumber) public view returns (uint256) {
-        return startingTimestamp + ((_l2BlockNumber - startingBlockNumber) * L2_BLOCK_TIME);
+        return startingTimestamp + ((_l2BlockNumber - startingBlockNumber) * _L2_BLOCK_TIME);
     }
 }

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,41 @@ 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
         });
     }
+
+    /* For some reason, this test causes foundry to stack overflow
+    /// @dev Tests that the constructor reverts if the starting timestamp is invalid.
+    function test_constructor_badTimestamp_reverts() external {
+        Proxy proxy = new Proxy({
+            _admin: alice
+        });
+
+        vm.store(address(proxy), bytes32(0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc), bytes32(uint256(uint160(address(oracle)))));
+
+        vm.expectRevert("L2OutputOracle: starting L2 timestamp must be less than current time");
+        proxy.upgradeToAndCall({
+            _implementation: address(oracle),
+            _data: abi.encodeCall(
+                L2OutputOracle.initialize,
+                (
+                    0,
+                    block.timestamp + 1,
+                    address(0),
+                    address(0)
+                )
+            )
+        });
+
+        L2OutputOracle(address(proxy)).initialize({
+            _startingBlockNumber: 0,
+            _startingTimestamp: block.timestamp + 1,
+            _proposer: address(0),
+            _challenger: address(0)
+        });
+    }
+    */
 }
 
 contract L2OutputOracle_getter_Test is L2OutputOracle_Initializer {
@@ -423,25 +438,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 +498,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));