Merge pull request #4093 from ethereum-optimism/sc/ctb-make-interval-changeable

feat(ctb): make SUBMISSION_INTERVAL modifiable

.changeset/quick-bananas-pretend.md

+---
+'@eth-optimism/contracts-bedrock': patch
+'@eth-optimism/core-utils': patch
+'@eth-optimism/sdk': patch
+---
+
+Refactors the L2OutputOracle to key the l2Outputs mapping by index instead of by L2 block number.

indexer/integration_tests/bedrock_test.go

@@ -204,6 +204,12 @@ func TestBedrockIndexer(t *testing.T) {
 		wParams, err := withdrawals.ProveWithdrawalParameters(context.Background(), proofCl, receiptCl, wdTx.Hash(), finHeader)
 		require.NoError(t, err)
 
+		oracle, err := bindings.NewL2OutputOracleCaller(predeploys.DevL2OutputOracleAddr, l1Client)
+		require.Nil(t, err)
+
+		l2OutputIndex, err := oracle.GetL2OutputIndexAfter(&bind.CallOpts{}, wParams.BlockNumber)
+		require.Nil(t, err)
+
 		l1Opts.Value = big.NewInt(0)
 		// Prove our withdrawal
 		proveTx, err := portal.ProveWithdrawalTransaction(
@@ -216,7 +222,7 @@ func TestBedrockIndexer(t *testing.T) {
 				GasLimit: wParams.GasLimit,
 				Data:     wParams.Data,
 			},
-			wParams.BlockNumber,
+			l2OutputIndex,
 			wParams.OutputRootProof,
 			wParams.WithdrawalProof,
 		)

op-e2e/actions/l2_proposer_test.go

@@ -73,7 +73,7 @@ func TestProposer(gt *testing.T) {
 	outputOracleContract, err := bindings.NewL2OutputOracle(sd.DeploymentsL1.L2OutputOracleProxy, miner.EthClient())
 	require.NoError(t, err)
 	block := sequencer.SyncStatus().FinalizedL2
-	outputOnL1, err := outputOracleContract.GetL2Output(nil, new(big.Int).SetUint64(block.Number))
+	outputOnL1, err := outputOracleContract.GetL2OutputAfter(nil, new(big.Int).SetUint64(block.Number))
 	require.NoError(t, err)
 	require.Less(t, block.Time, outputOnL1.Timestamp.Uint64(), "output is registered with L1 timestamp of proposal tx, past L2 block")
 	outputComputed, err := sequencer.RollupClient().OutputAtBlock(t.Ctx(), block.Number)

op-e2e/actions/user.go

@@ -397,6 +397,8 @@ func (s *CrossLayerUser) ProveWithdrawal(t Testing, l2TxHash common.Hash) common
 	require.NoError(t, err)
 	l2OutputBlock, err := s.L2.env.EthCl.BlockByNumber(t.Ctx(), l2OutputBlockNr)
 	require.NoError(t, err)
+	l2OutputIndex, err := s.L1.env.Bindings.L2OutputOracle.GetL2OutputIndexAfter(&bind.CallOpts{}, l2OutputBlockNr)
+	require.NoError(t, err)
 
 	// Check if the L2 output is even old enough to include the withdrawal
 	if l2OutputBlock.NumberU64() < l2WithdrawalBlock.NumberU64() {
@@ -421,7 +423,7 @@ func (s *CrossLayerUser) ProveWithdrawal(t Testing, l2TxHash common.Hash) common
 			GasLimit: params.GasLimit,
 			Data:     params.Data,
 		},
-		params.BlockNumber,
+		l2OutputIndex,
 		params.OutputRootProof,
 		params.WithdrawalProof,
 	)

op-e2e/system_test.go

@@ -104,7 +104,7 @@ func TestL2OutputSubmitter(t *testing.T) {
 		// timestamp set in the contract constructor.
 		if l2ooBlockNumber.Cmp(initialOutputBlockNumber) > 0 {
 			// Retrieve the l2 output committed at this updated timestamp.
-			committedL2Output, err := l2OutputOracle.GetL2Output(&bind.CallOpts{}, l2ooBlockNumber)
+			committedL2Output, err := l2OutputOracle.GetL2OutputAfter(&bind.CallOpts{}, l2ooBlockNumber)
 			require.NotEqual(t, [32]byte{}, committedL2Output.OutputRoot, "Empty L2 Output")
 			require.Nil(t, err)
 
@@ -842,6 +842,12 @@ func TestWithdrawals(t *testing.T) {
 	portal, err := bindings.NewOptimismPortal(predeploys.DevOptimismPortalAddr, l1Client)
 	require.Nil(t, err)
 
+	oracle, err := bindings.NewL2OutputOracleCaller(predeploys.DevL2OutputOracleAddr, l1Client)
+	require.Nil(t, err)
+
+	l2OutputIndex, err := oracle.GetL2OutputIndexAfter(&bind.CallOpts{}, params.BlockNumber)
+	require.Nil(t, err)
+
 	opts.Value = nil
 
 	// Prove withdrawal
@@ -855,7 +861,7 @@ func TestWithdrawals(t *testing.T) {
 			GasLimit: params.GasLimit,
 			Data:     params.Data,
 		},
-		params.BlockNumber,
+		l2OutputIndex,
 		params.OutputRootProof,
 		params.WithdrawalProof,
 	)

op-node/withdrawals/utils.go

@@ -91,7 +91,7 @@ loop:
 	}
 
 	// Now wait for it to be finalized
-	output, err := l2OO.GetL2Output(opts, l2BlockNumber)
+	output, err := l2OO.GetL2OutputAfter(opts, l2BlockNumber)
 	if err != nil {
 		return 0, err
 	}

packages/contracts-bedrock/.storage-layout

@@ -64,21 +64,19 @@
 ➡ contracts/L1/L2OutputOracle.sol:L2OutputOracle
 =======================
 
-+---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------+
++---------------------+-------------------------------+------+--------+-------+------------------------------------------------+
 | Name                | Type                          | Slot | Offset | Bytes | Contract                                       |
-+================================================================================================================================================+
++==============================================================================================================================+
 | _initialized        | uint8                         | 0    | 0      | 1     | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
-|---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------|
+|---------------------+-------------------------------+------+--------+-------+------------------------------------------------|
 | _initializing       | bool                          | 0    | 1      | 1     | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
-|---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------|
+|---------------------+-------------------------------+------+--------+-------+------------------------------------------------|
 | startingBlockNumber | uint256                       | 1    | 0      | 32    | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
-|---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------|
+|---------------------+-------------------------------+------+--------+-------+------------------------------------------------|
 | startingTimestamp   | uint256                       | 2    | 0      | 32    | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
-|---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------|
-| latestBlockNumber   | uint256                                         | 3    | 0      | 32    | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
-|---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------|
-| l2Outputs           | mapping(uint256 => struct Types.OutputProposal) | 4    | 0      | 32    | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
-+---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------+
+|---------------------+-------------------------------+------+--------+-------+------------------------------------------------|
+| l2Outputs           | struct Types.OutputProposal[] | 3    | 0      | 32    | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
++---------------------+-------------------------------+------+--------+-------+------------------------------------------------+
 
 =======================
 ➡ contracts/L1/OptimismPortal.sol:OptimismPortal

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

@@ -25,7 +25,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
     struct ProvenWithdrawal {
         bytes32 outputRoot;
         uint128 timestamp;
-        uint128 l2BlockNumber;
+        uint128 l2OutputIndex;
     }
 
     /**
@@ -153,13 +153,13 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
      * @notice Proves a withdrawal transaction.
      *
      * @param _tx              Withdrawal transaction to finalize.
-     * @param _l2BlockNumber   L2 block number of the outputRoot.
+     * @param _l2OutputIndex   L2 output index to prove against.
      * @param _outputRootProof Inclusion proof of the L2ToL1MessagePasser contract's storage root.
      * @param _withdrawalProof Inclusion proof of the withdrawal in L2ToL1MessagePasser contract.
      */
     function proveWithdrawalTransaction(
         Types.WithdrawalTransaction memory _tx,
-        uint256 _l2BlockNumber,
+        uint256 _l2OutputIndex,
         Types.OutputRootProof calldata _outputRootProof,
         bytes[] calldata _withdrawalProof
     ) external {
@@ -177,7 +177,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
 
         // Get the output root and load onto the stack to prevent multiple mloads. This will
         // fail if there is no output root for the given block number.
-        bytes32 outputRoot = L2_ORACLE.getL2Output(_l2BlockNumber).outputRoot;
+        bytes32 outputRoot = L2_ORACLE.getL2Output(_l2OutputIndex).outputRoot;
 
         // Verify that the output root can be generated with the elements in the proof.
         require(
@@ -195,7 +195,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
         // Only allow re-proving a withdrawal transaction if the output root has changed.
         require(
             provenWithdrawal.timestamp == 0 ||
-                (_l2BlockNumber == provenWithdrawal.l2BlockNumber &&
+                (_l2OutputIndex == provenWithdrawal.l2OutputIndex &&
                     outputRoot != provenWithdrawal.outputRoot),
             "OptimismPortal: withdrawal hash has already been proven"
         );
@@ -219,7 +219,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
         provenWithdrawals[withdrawalHash] = ProvenWithdrawal({
             outputRoot: outputRoot,
             timestamp: uint128(block.timestamp),
-            l2BlockNumber: uint128(_l2BlockNumber)
+            l2OutputIndex: uint128(_l2OutputIndex)
         });
 
         // Emit a `WithdrawalProven` event.
@@ -263,7 +263,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
 
         // Grab the OutputProposal from the L2 Oracle
         Types.OutputProposal memory proposal = L2_ORACLE.getL2Output(
-            provenWithdrawal.l2BlockNumber
+            provenWithdrawal.l2OutputIndex
         );
 
         // Check that the output proposal hasn't been updated.
@@ -313,13 +313,13 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
     }
 
     /**
-     * @notice Determine if a given block number is finalized. Reverts if the call to
+     * @notice Determine if a given output is finalized. Reverts if the call to
      *         L2_ORACLE.getL2Output reverts. Returns a boolean otherwise.
      *
-     * @param _l2BlockNumber The number of the L2 block.
+     * @param _l2OutputIndex Index of the L2 output to check.
      */
-    function isBlockFinalized(uint256 _l2BlockNumber) external view returns (bool) {
-        return _isFinalizationPeriodElapsed(L2_ORACLE.getL2Output(_l2BlockNumber).timestamp);
+    function isOutputFinalized(uint256 _l2OutputIndex) external view returns (bool) {
+        return _isFinalizationPeriodElapsed(L2_ORACLE.getL2Output(_l2OutputIndex).timestamp);
     }
 
     /**

packages/contracts-bedrock/contracts/libraries/Types.sol

@@ -13,7 +13,8 @@ library Types {
      */
     struct OutputProposal {
         bytes32 outputRoot;
-        uint256 timestamp;
+        uint128 timestamp;
+        uint128 l2BlockNumber;
     }
 
     /**

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

@@ -129,10 +129,7 @@ contract L2OutputOracle_Initializer is CommonTest {
         vm.prank(multisig);
         proxy.upgradeToAndCall(
             address(oracleImpl),
-            abi.encodeCall(
-                L2OutputOracle.initialize,
-                (startingBlockNumber, startingTimestamp)
-            )
+            abi.encodeCall(L2OutputOracle.initialize, (startingBlockNumber, startingTimestamp))
         );
         oracle = L2OutputOracle(address(proxy));
         vm.label(address(oracle), "L2OutputOracle");

packages/core-utils/src/optimism/hashing.ts

@@ -17,6 +17,7 @@ export interface BedrockOutputData {
   outputRoot: string
   l1Timestamp: number
   l2BlockNumber: number
+  l2OutputIndex: number
 }
 
 /**

packages/sdk/src/cross-chain-messenger.ts

@@ -962,37 +962,36 @@ export class CrossChainMessenger {
       throw new Error(`cannot get a state root for an L1 to L2 message`)
     }
 
-    const l2OutputOracleParameters = await this.getL2OutputOracleParameters()
-
-    // TODO: better way to do this
-    let number =
-      resolved.blockNumber - l2OutputOracleParameters.startingBlockNumber
-    while (number % l2OutputOracleParameters.submissionInterval !== 0) {
-      number++
-    }
-
-    // TODO: Handle old messages from before Bedrock upgrade.
-    const events = await this.contracts.l1.L2OutputOracle.queryFilter(
-      this.contracts.l1.L2OutputOracle.filters.OutputProposed(
-        undefined,
-        undefined,
-        number
-      )
+    // Try to find the output index that corresponds to the block number attached to the message.
+    // We'll explicitly handle "cannot get output" errors as a null return value, but anything else
+    // needs to get thrown. Might need to revisit this in the future to be a little more robust
+    // when connected to RPCs that don't return nice error messages.
+    let l2OutputIndex: BigNumber
+    try {
+      l2OutputIndex =
+        await this.contracts.l1.L2OutputOracle.getL2OutputIndexAfter(
+          resolved.blockNumber
         )
-
-    if (events.length === 0) {
+    } catch (err) {
+      if (err.message.includes('L2OutputOracle: cannot get output')) {
         return null
+      } else {
+        throw err
       }
-
-    // Should not happen
-    if (events.length > 1) {
-      throw new Error(`multiple output roots found for message`)
     }
 
+    // Now pull the proposal out given the output index. Should always work as long as the above
+    // codepath completed successfully.
+    const proposal = await this.contracts.l1.L2OutputOracle.getL2Output(
+      l2OutputIndex
+    )
+
+    // Format everything and return it nicely.
     return {
-      outputRoot: events[0].args.l2Output,
-      l1Timestamp: events[0].args.l1Timestamp.toNumber(),
-      l2BlockNumber: events[0].args.l2BlockNumber.toNumber(),
+      outputRoot: proposal.outputRoot,
+      l1Timestamp: proposal.timestamp.toNumber(),
+      l2BlockNumber: proposal.l2BlockNumber.toNumber(),
+      l2OutputIndex: l2OutputIndex.toNumber(),
     }
   }
 
@@ -1774,7 +1773,7 @@ export class CrossChainMessenger {
           withdrawalTx.minGasLimit,
           withdrawalTx.message,
         ],
-        output.l2BlockNumber,
+        output.l2OutputIndex,
         [
           proof.outputRootProof.version,
           proof.outputRootProof.stateRoot,

specs/proposals.md

@@ -124,7 +124,7 @@ function deleteL2Output(bytes32 _l2Output) external
 /**
  * @notice Computes the block number of the next L2 block that needs to be checkpointed.
  */
-function nextBlockNumber() public view returns (uint256) {
+function getNextBlockNumber() public view returns (uint256) {
 ```
 
 ## Security Considerations