Commit 136ea178 authored by Kelvin Fichter's avatar Kelvin Fichter

feat(ctb): make SUBMISSION_INTERVAL modifiable

Updates the L2OutputOracle so that SUBMISSION_INTERVAL can be modified.
This is a relatively significant refactor that makes the l2Outputs
mapping keyed by index rather than L2 block number.
parent 66f7de23
---
'@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.
...@@ -204,6 +204,12 @@ func TestBedrockIndexer(t *testing.T) { ...@@ -204,6 +204,12 @@ func TestBedrockIndexer(t *testing.T) {
wParams, err := withdrawals.ProveWithdrawalParameters(context.Background(), proofCl, receiptCl, wdTx.Hash(), finHeader) wParams, err := withdrawals.ProveWithdrawalParameters(context.Background(), proofCl, receiptCl, wdTx.Hash(), finHeader)
require.NoError(t, err) 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) l1Opts.Value = big.NewInt(0)
// Prove our withdrawal // Prove our withdrawal
proveTx, err := portal.ProveWithdrawalTransaction( proveTx, err := portal.ProveWithdrawalTransaction(
...@@ -216,7 +222,7 @@ func TestBedrockIndexer(t *testing.T) { ...@@ -216,7 +222,7 @@ func TestBedrockIndexer(t *testing.T) {
GasLimit: wParams.GasLimit, GasLimit: wParams.GasLimit,
Data: wParams.Data, Data: wParams.Data,
}, },
wParams.BlockNumber, l2OutputIndex,
wParams.OutputRootProof, wParams.OutputRootProof,
wParams.WithdrawalProof, wParams.WithdrawalProof,
) )
......
This diff is collapsed.
This source diff could not be displayed because it is too large. You can view the blob instead.
This diff is collapsed.
...@@ -73,7 +73,7 @@ func TestProposer(gt *testing.T) { ...@@ -73,7 +73,7 @@ func TestProposer(gt *testing.T) {
outputOracleContract, err := bindings.NewL2OutputOracle(sd.DeploymentsL1.L2OutputOracleProxy, miner.EthClient()) outputOracleContract, err := bindings.NewL2OutputOracle(sd.DeploymentsL1.L2OutputOracleProxy, miner.EthClient())
require.NoError(t, err) require.NoError(t, err)
block := sequencer.SyncStatus().FinalizedL2 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.NoError(t, err)
require.Less(t, block.Time, outputOnL1.Timestamp.Uint64(), "output is registered with L1 timestamp of proposal tx, past L2 block") 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) outputComputed, err := sequencer.RollupClient().OutputAtBlock(t.Ctx(), block.Number)
......
...@@ -397,6 +397,8 @@ func (s *CrossLayerUser) ProveWithdrawal(t Testing, l2TxHash common.Hash) common ...@@ -397,6 +397,8 @@ func (s *CrossLayerUser) ProveWithdrawal(t Testing, l2TxHash common.Hash) common
require.NoError(t, err) require.NoError(t, err)
l2OutputBlock, err := s.L2.env.EthCl.BlockByNumber(t.Ctx(), l2OutputBlockNr) l2OutputBlock, err := s.L2.env.EthCl.BlockByNumber(t.Ctx(), l2OutputBlockNr)
require.NoError(t, err) 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 // Check if the L2 output is even old enough to include the withdrawal
if l2OutputBlock.NumberU64() < l2WithdrawalBlock.NumberU64() { if l2OutputBlock.NumberU64() < l2WithdrawalBlock.NumberU64() {
...@@ -421,7 +423,7 @@ func (s *CrossLayerUser) ProveWithdrawal(t Testing, l2TxHash common.Hash) common ...@@ -421,7 +423,7 @@ func (s *CrossLayerUser) ProveWithdrawal(t Testing, l2TxHash common.Hash) common
GasLimit: params.GasLimit, GasLimit: params.GasLimit,
Data: params.Data, Data: params.Data,
}, },
params.BlockNumber, l2OutputIndex,
params.OutputRootProof, params.OutputRootProof,
params.WithdrawalProof, params.WithdrawalProof,
) )
......
...@@ -104,7 +104,7 @@ func TestL2OutputSubmitter(t *testing.T) { ...@@ -104,7 +104,7 @@ func TestL2OutputSubmitter(t *testing.T) {
// timestamp set in the contract constructor. // timestamp set in the contract constructor.
if l2ooBlockNumber.Cmp(initialOutputBlockNumber) > 0 { if l2ooBlockNumber.Cmp(initialOutputBlockNumber) > 0 {
// Retrieve the l2 output committed at this updated timestamp. // 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.NotEqual(t, [32]byte{}, committedL2Output.OutputRoot, "Empty L2 Output")
require.Nil(t, err) require.Nil(t, err)
...@@ -842,6 +842,12 @@ func TestWithdrawals(t *testing.T) { ...@@ -842,6 +842,12 @@ func TestWithdrawals(t *testing.T) {
portal, err := bindings.NewOptimismPortal(predeploys.DevOptimismPortalAddr, l1Client) portal, err := bindings.NewOptimismPortal(predeploys.DevOptimismPortalAddr, l1Client)
require.Nil(t, err) 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 opts.Value = nil
// Prove withdrawal // Prove withdrawal
...@@ -855,7 +861,7 @@ func TestWithdrawals(t *testing.T) { ...@@ -855,7 +861,7 @@ func TestWithdrawals(t *testing.T) {
GasLimit: params.GasLimit, GasLimit: params.GasLimit,
Data: params.Data, Data: params.Data,
}, },
params.BlockNumber, l2OutputIndex,
params.OutputRootProof, params.OutputRootProof,
params.WithdrawalProof, params.WithdrawalProof,
) )
......
...@@ -91,7 +91,7 @@ loop: ...@@ -91,7 +91,7 @@ loop:
} }
// Now wait for it to be finalized // Now wait for it to be finalized
output, err := l2OO.GetL2Output(opts, l2BlockNumber) output, err := l2OO.GetL2OutputAfter(opts, l2BlockNumber)
if err != nil { if err != nil {
return 0, err return 0, err
} }
......
This diff is collapsed.
...@@ -64,21 +64,19 @@ ...@@ -64,21 +64,19 @@
➡ contracts/L1/L2OutputOracle.sol:L2OutputOracle ➡ contracts/L1/L2OutputOracle.sol:L2OutputOracle
======================= =======================
+---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------+ +---------------------+-------------------------------+------+--------+-------+------------------------------------------------+
| Name | Type | Slot | Offset | Bytes | Contract | | Name | Type | Slot | Offset | Bytes | Contract |
+================================================================================================================================================+ +==============================================================================================================================+
| _initialized | uint8 | 0 | 0 | 1 | contracts/L1/L2OutputOracle.sol:L2OutputOracle | | _initialized | uint8 | 0 | 0 | 1 | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
|---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------| |---------------------+-------------------------------+------+--------+-------+------------------------------------------------|
| _initializing | bool | 0 | 1 | 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 | | startingBlockNumber | uint256 | 1 | 0 | 32 | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
|---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------| |---------------------+-------------------------------+------+--------+-------+------------------------------------------------|
| startingTimestamp | uint256 | 2 | 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 | struct Types.OutputProposal[] | 3 | 0 | 32 | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
|---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------| +---------------------+-------------------------------+------+--------+-------+------------------------------------------------+
| l2Outputs | mapping(uint256 => struct Types.OutputProposal) | 4 | 0 | 32 | contracts/L1/L2OutputOracle.sol:L2OutputOracle |
+---------------------+-------------------------------------------------+------+--------+-------+------------------------------------------------+
======================= =======================
➡ contracts/L1/OptimismPortal.sol:OptimismPortal ➡ contracts/L1/OptimismPortal.sol:OptimismPortal
......
...@@ -25,7 +25,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -25,7 +25,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
struct ProvenWithdrawal { struct ProvenWithdrawal {
bytes32 outputRoot; bytes32 outputRoot;
uint128 timestamp; uint128 timestamp;
uint128 l2BlockNumber; uint128 l2OutputIndex;
} }
/** /**
...@@ -153,13 +153,13 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -153,13 +153,13 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
* @notice Proves a withdrawal transaction. * @notice Proves a withdrawal transaction.
* *
* @param _tx Withdrawal transaction to finalize. * @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 _outputRootProof Inclusion proof of the L2ToL1MessagePasser contract's storage root.
* @param _withdrawalProof Inclusion proof of the withdrawal in L2ToL1MessagePasser contract. * @param _withdrawalProof Inclusion proof of the withdrawal in L2ToL1MessagePasser contract.
*/ */
function proveWithdrawalTransaction( function proveWithdrawalTransaction(
Types.WithdrawalTransaction memory _tx, Types.WithdrawalTransaction memory _tx,
uint256 _l2BlockNumber, uint256 _l2OutputIndex,
Types.OutputRootProof calldata _outputRootProof, Types.OutputRootProof calldata _outputRootProof,
bytes[] calldata _withdrawalProof bytes[] calldata _withdrawalProof
) external { ) external {
...@@ -177,7 +177,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -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 // 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. // 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. // Verify that the output root can be generated with the elements in the proof.
require( require(
...@@ -195,7 +195,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -195,7 +195,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
// Only allow re-proving a withdrawal transaction if the output root has changed. // Only allow re-proving a withdrawal transaction if the output root has changed.
require( require(
provenWithdrawal.timestamp == 0 || provenWithdrawal.timestamp == 0 ||
(_l2BlockNumber == provenWithdrawal.l2BlockNumber && (_l2OutputIndex == provenWithdrawal.l2OutputIndex &&
outputRoot != provenWithdrawal.outputRoot), outputRoot != provenWithdrawal.outputRoot),
"OptimismPortal: withdrawal hash has already been proven" "OptimismPortal: withdrawal hash has already been proven"
); );
...@@ -219,7 +219,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -219,7 +219,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
provenWithdrawals[withdrawalHash] = ProvenWithdrawal({ provenWithdrawals[withdrawalHash] = ProvenWithdrawal({
outputRoot: outputRoot, outputRoot: outputRoot,
timestamp: uint128(block.timestamp), timestamp: uint128(block.timestamp),
l2BlockNumber: uint128(_l2BlockNumber) l2OutputIndex: uint128(_l2OutputIndex)
}); });
// Emit a `WithdrawalProven` event. // Emit a `WithdrawalProven` event.
...@@ -263,7 +263,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -263,7 +263,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
// Grab the OutputProposal from the L2 Oracle // Grab the OutputProposal from the L2 Oracle
Types.OutputProposal memory proposal = L2_ORACLE.getL2Output( Types.OutputProposal memory proposal = L2_ORACLE.getL2Output(
provenWithdrawal.l2BlockNumber provenWithdrawal.l2OutputIndex
); );
// Check that the output proposal hasn't been updated. // Check that the output proposal hasn't been updated.
...@@ -313,13 +313,13 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -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. * 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) { function isOutputFinalized(uint256 _l2OutputIndex) external view returns (bool) {
return _isFinalizationPeriodElapsed(L2_ORACLE.getL2Output(_l2BlockNumber).timestamp); return _isFinalizationPeriodElapsed(L2_ORACLE.getL2Output(_l2OutputIndex).timestamp);
} }
/** /**
......
...@@ -13,7 +13,8 @@ library Types { ...@@ -13,7 +13,8 @@ library Types {
*/ */
struct OutputProposal { struct OutputProposal {
bytes32 outputRoot; bytes32 outputRoot;
uint256 timestamp; uint128 timestamp;
uint128 l2BlockNumber;
} }
/** /**
......
...@@ -129,10 +129,7 @@ contract L2OutputOracle_Initializer is CommonTest { ...@@ -129,10 +129,7 @@ contract L2OutputOracle_Initializer is CommonTest {
vm.prank(multisig); vm.prank(multisig);
proxy.upgradeToAndCall( proxy.upgradeToAndCall(
address(oracleImpl), address(oracleImpl),
abi.encodeCall( abi.encodeCall(L2OutputOracle.initialize, (startingBlockNumber, startingTimestamp))
L2OutputOracle.initialize,
(startingBlockNumber, startingTimestamp)
)
); );
oracle = L2OutputOracle(address(proxy)); oracle = L2OutputOracle(address(proxy));
vm.label(address(oracle), "L2OutputOracle"); vm.label(address(oracle), "L2OutputOracle");
......
...@@ -17,6 +17,7 @@ export interface BedrockOutputData { ...@@ -17,6 +17,7 @@ export interface BedrockOutputData {
outputRoot: string outputRoot: string
l1Timestamp: number l1Timestamp: number
l2BlockNumber: number l2BlockNumber: number
l2OutputIndex: number
} }
/** /**
......
...@@ -962,37 +962,36 @@ export class CrossChainMessenger { ...@@ -962,37 +962,36 @@ export class CrossChainMessenger {
throw new Error(`cannot get a state root for an L1 to L2 message`) throw new Error(`cannot get a state root for an L1 to L2 message`)
} }
const l2OutputOracleParameters = await this.getL2OutputOracleParameters() // 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
// TODO: better way to do this // needs to get thrown. Might need to revisit this in the future to be a little more robust
let number = // when connected to RPCs that don't return nice error messages.
resolved.blockNumber - l2OutputOracleParameters.startingBlockNumber let l2OutputIndex: BigNumber
while (number % l2OutputOracleParameters.submissionInterval !== 0) { try {
number++ l2OutputIndex =
await this.contracts.l1.L2OutputOracle.getL2OutputIndexAfter(
resolved.blockNumber
)
} catch (err) {
if (err.message.includes('L2OutputOracle: cannot get output')) {
return null
} else {
throw err
}
} }
// TODO: Handle old messages from before Bedrock upgrade. // Now pull the proposal out given the output index. Should always work as long as the above
const events = await this.contracts.l1.L2OutputOracle.queryFilter( // codepath completed successfully.
this.contracts.l1.L2OutputOracle.filters.OutputProposed( const proposal = await this.contracts.l1.L2OutputOracle.getL2Output(
undefined, l2OutputIndex
undefined,
number
)
) )
if (events.length === 0) { // Format everything and return it nicely.
return null
}
// Should not happen
if (events.length > 1) {
throw new Error(`multiple output roots found for message`)
}
return { return {
outputRoot: events[0].args.l2Output, outputRoot: proposal.outputRoot,
l1Timestamp: events[0].args.l1Timestamp.toNumber(), l1Timestamp: proposal.timestamp.toNumber(),
l2BlockNumber: events[0].args.l2BlockNumber.toNumber(), l2BlockNumber: proposal.l2BlockNumber.toNumber(),
l2OutputIndex: l2OutputIndex.toNumber(),
} }
} }
...@@ -1774,7 +1773,7 @@ export class CrossChainMessenger { ...@@ -1774,7 +1773,7 @@ export class CrossChainMessenger {
withdrawalTx.minGasLimit, withdrawalTx.minGasLimit,
withdrawalTx.message, withdrawalTx.message,
], ],
output.l2BlockNumber, output.l2OutputIndex,
[ [
proof.outputRootProof.version, proof.outputRootProof.version,
proof.outputRootProof.stateRoot, proof.outputRootProof.stateRoot,
......
...@@ -124,7 +124,7 @@ function deleteL2Output(bytes32 _l2Output) external ...@@ -124,7 +124,7 @@ function deleteL2Output(bytes32 _l2Output) external
/** /**
* @notice Computes the block number of the next L2 block that needs to be checkpointed. * @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 ## Security Considerations
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment