obront - Submission interval is unreasonably restricted, bricking migration process or immutably setting incorrect params

[sherlock-admin]

obront

medium

Submission interval is unreasonably restricted, bricking migration process or immutably setting incorrect params

Summary

The L2OutputOracle unintentionally blocks reasonable parameter settings for the SUBMISSION_INTERVAL and L2_BLOCK_TIME variables (including those intended to be used by Bedrock) from being used, resulting in either a bricked migration or an immutable mis-setting of important parameters.

Vulnerability Detail

L2OutputOracle.sol takes in a _submissionInterval and _l2BlockTime are arguments to the constructor function.

The Submission Interval is how many L2 blocks can pass until they must be checkpointed on L1 again. It is measured in blocks and is used in the following calculation:

function nextBlockNumber() public view returns (uint256) {
    return latestBlockNumber() + SUBMISSION_INTERVAL;
}

The L2 Block Time is the number of seconds between L2 blocks. It is used in the following calculation:

function computeL2Timestamp(uint256 _l2BlockNumber) public view returns (uint256) {
    return startingTimestamp + ((_l2BlockNumber - startingBlockNumber) * L2_BLOCK_TIME);
}

These two variables are completely unrelated and should not influence one another. For example, both of the below parameters are acceptable:

• SI = 1 // L2BT = 6 => we create an L2 block every 6 seconds and post each one to L1
• SI = 12 // L2BT = 1 => we create an L2 block each second but only post to L1 every 12

However, the following check is included in the constructor when these values are immutably set:

require(
    _submissionInterval > _l2BlockTime,
    "L2OutputOracle: submission interval must be greater than L2 block time"
);

Bedrock intends to use the values of SI == 1 (each L2 block is checkpointed on L1) and L2BT = 2 (there's an L2 block every 2 seconds), which do not pass this check.

Impact

If these parameters are entereed correctly, the migration process will brick when L2OutputOracle.sol deployment fails.

If they are entered incorrectly, these important parameters will be immutably mis-set.

Code Snippet

https://github.com/ethereum-optimism/optimism/blob/9b9f78c6613c6ee53b93ca43c71bb74479f4b975/packages/contracts-bedrock/contracts/L1/L2OutputOracle.sol#L100-L103

Tool used

Manual Review

Recommendation

Remove this check, as there should be no requirements on the relationship between these two variables.

[hrishibhat]

Sponsor comment: This check can likely be removed, but this is a low issue that will not impact the system in its intended configuration.

[GalloDaSballo]

QA imo