bedrock: use block number for key in output oracle (#2707)

* bedrock: use block number for key in output oracle

* contracts: Apply seaport style to natspec comments

Inspo: https://github.com/ProjectOpenSea/seaport/blob/main/contracts/Seaport.sol

* bedrock: Improve oracle error messages

* bedrock: update node and proposer with new oracle interface

* bedrock: add tests for oracle reorg protection.

* Fix op-proposer & withdrawals logic

* Golang PR Fixes

* specs: use block number for key in output oracle

* bedrock: Fix CamelCase on oracle events

* bedrock: Update arg name in usage of getL2Output

* bedrock: Oracle add computeL2Timestamp and check on append
Co-authored-by: Joshua Gutow <jgutow@optimism.io>

.changeset/selfish-queens-laugh.md

+---
+'@eth-optimism/contracts-bedrock': minor
+---
+
+Replaces L2 timestamps with block numbers as the key in mapping(uint => OutputProposal).

op-e2e/setup.go

@@ -52,10 +52,11 @@ func deriveAccount(w accounts.Wallet, path string) accounts.Account {
 
 type L2OOContractConfig struct {
 	SubmissionFrequency   *big.Int
-	L2StartTime           *big.Int
-	L2BlockTime           *big.Int
+	L2StartingBlock       *big.Int
 	GenesisL2Output       [32]byte
 	HistoricalTotalBlocks *big.Int
+	L2StartingTimeStamp   *big.Int
+	L2BlockTime           *big.Int
 }
 
 type DepositContractConfig struct {
@@ -84,6 +85,8 @@ type SystemConfig struct {
 
 	Nodes          map[string]*rollupNode.Config // Per node config. Don't use populate rollup.Config
 	Loggers        map[string]log.Logger
+	ProposerLogger log.Logger
+	BatcherLogger  log.Logger
 	RollupConfig   rollup.Config // Shared rollup configs
 
 	L1BlockTime uint64
@@ -392,7 +395,9 @@ func (cfg SystemConfig) start() (*System, error) {
 	sys.cfg.RollupConfig.Genesis = sys.RolupGenesis
 	sys.cfg.RollupConfig.BatchSenderAddress = batchSubmitterAddr
 	sys.cfg.RollupConfig.P2PSequencerAddress = p2pSignerAddr
-	sys.cfg.L2OOCfg.L2StartTime = new(big.Int).SetUint64(l2GenesisTime)
+	sys.cfg.L2OOCfg.L2StartingBlock = new(big.Int).SetUint64(l2GenesisID.Number)
+	sys.cfg.L2OOCfg.L2StartingTimeStamp = new(big.Int).SetUint64(l2Genesis.Timestamp)
+	sys.cfg.L2OOCfg.L2BlockTime = new(big.Int).SetUint64(2)
 
 	// Deploy Deposit Contract
 	deployerPrivKey, err := sys.wallet.PrivateKey(accounts.Account{
@@ -414,10 +419,11 @@ func (cfg SystemConfig) start() (*System, error) {
 		opts,
 		l1Client,
 		sys.cfg.L2OOCfg.SubmissionFrequency,
-		sys.cfg.L2OOCfg.L2BlockTime,
 		sys.cfg.L2OOCfg.GenesisL2Output,
 		sys.cfg.L2OOCfg.HistoricalTotalBlocks,
-		sys.cfg.L2OOCfg.L2StartTime,
+		sys.cfg.L2OOCfg.L2StartingBlock,
+		sys.cfg.L2OOCfg.L2StartingTimeStamp,
+		sys.cfg.L2OOCfg.L2BlockTime,
 		l2OutputSubmitterAddr,
 	)
 	sys.cfg.DepositCFG.L2Oracle = sys.L2OOContractAddr
@@ -554,7 +560,7 @@ func (cfg SystemConfig) start() (*System, error) {
 		LogTerminal:               true,
 		Mnemonic:                  sys.cfg.Mnemonic,
 		L2OutputHDPath:            sys.cfg.L2OutputHDPath,
-	}, "", log.New())
+	}, "", cfg.ProposerLogger)
 	if err != nil {
 		return nil, fmt.Errorf("unable to setup l2 output submitter: %w", err)
 	}
@@ -590,7 +596,7 @@ func (cfg SystemConfig) start() (*System, error) {
 		SequencerHistoryDBFilename: sys.sequencerHistoryDBFileName,
 		SequencerGenesisHash:       sys.RolupGenesis.L2.Hash.String(),
 		SequencerBatchInboxAddress: sys.cfg.RollupConfig.BatchInboxAddress.String(),
-	}, "", log.New())
+	}, "", cfg.BatcherLogger)
 	if err != nil {
 		return nil, fmt.Errorf("failed to setup batch submitter: %w", err)
 	}

op-e2e/system_test.go

@@ -88,7 +88,6 @@ func defaultSystemConfig(t *testing.T) SystemConfig {
 		L2OOCfg: L2OOContractConfig{
 			// L2 Start time is set based off of the L2 Genesis time
 			SubmissionFrequency:   big.NewInt(2),
-			L2BlockTime:           big.NewInt(1),
 			HistoricalTotalBlocks: big.NewInt(0),
 		},
 		L2OutputHDPath:             l2OutputHDPath,
@@ -117,6 +116,8 @@ func defaultSystemConfig(t *testing.T) SystemConfig {
 			"verifier":  testlog.Logger(t, log.LvlError).New("role", "verifier"),
 			"sequencer": testlog.Logger(t, log.LvlError).New("role", "sequencer"),
 		},
+		ProposerLogger: testlog.Logger(t, log.LvlCrit).New("role", "proposer"), // Proposer is noisy on shutdown
+		BatcherLogger:  testlog.Logger(t, log.LvlCrit).New("role", "batcher"),  // Batcher (txmgr really) is noisy on shutdown
 		RollupConfig: rollup.Config{
 			BlockTime:         1,
 			MaxSequencerDrift: 10,
@@ -153,11 +154,11 @@ func TestL2OutputSubmitter(t *testing.T) {
 	require.Nil(t, err)
 	rollupClient := rollupclient.NewRollupClient(rollupRPCClient)
 
-	//  StateRootOracle is already deployed
+	//  OutputOracle is already deployed
 	l2OutputOracle, err := bindings.NewL2OutputOracleCaller(sys.L2OOContractAddr, l1Client)
 	require.Nil(t, err)
 
-	initialSroTimestamp, err := l2OutputOracle.LatestBlockTimestamp(&bind.CallOpts{})
+	initialOutputBlockNumber, err := l2OutputOracle.LatestBlockNumber(&bind.CallOpts{})
 	require.Nil(t, err)
 
 	// Wait until the second output submission from L2. The output submitter submits outputs from the
@@ -174,20 +175,15 @@ func TestL2OutputSubmitter(t *testing.T) {
 	ticker := time.NewTicker(1 * time.Second)
 	defer ticker.Stop()
 	for {
-		l2ooTimestamp, err := l2OutputOracle.LatestBlockTimestamp(&bind.CallOpts{})
+		l2ooBlockNumber, err := l2OutputOracle.LatestBlockNumber(&bind.CallOpts{})
 		require.Nil(t, err)
 
 		// Wait for the L2 output oracle to have been changed from the initial
 		// timestamp set in the contract constructor.
-		if l2ooTimestamp.Cmp(initialSroTimestamp) > 0 {
+		if l2ooBlockNumber.Cmp(initialOutputBlockNumber) > 0 {
 			// Retrieve the l2 output committed at this updated timestamp.
-			committedL2Output, err := l2OutputOracle.GetL2Output(&bind.CallOpts{}, l2ooTimestamp)
-			require.Nil(t, err)
-
-			// Compute the committed L2 output's L2 block number.
-			l2ooBlockNumber, err := l2OutputOracle.ComputeL2BlockNumber(
-				&bind.CallOpts{}, l2ooTimestamp,
-			)
+			committedL2Output, err := l2OutputOracle.GetL2Output(&bind.CallOpts{}, l2ooBlockNumber)
+			require.NotEqual(t, [32]byte{}, committedL2Output.OutputRoot, "Empty L2 Output")
 			require.Nil(t, err)
 
 			// Fetch the corresponding L2 block and assert the committed L2
@@ -765,19 +761,19 @@ func TestWithdrawals(t *testing.T) {
 	tx, err = l2withdrawer.InitiateWithdrawal(l2opts, fromAddr, big.NewInt(21000), nil)
 	require.Nil(t, err, "sending initiate withdraw tx")
 
-	receipt, err = waitForTransaction(tx.Hash(), l2Seq, 3*time.Duration(cfg.L1BlockTime)*time.Second)
+	receipt, err = waitForTransaction(tx.Hash(), l2Verif, 5*time.Duration(cfg.L1BlockTime)*time.Second)
 	require.Nil(t, err, "withdrawal initiated on L2 sequencer")
 	require.Equal(t, receipt.Status, types.ReceiptStatusSuccessful, "transaction failed")
 
 	// Verify L2 balance after withdrawal
 	ctx, cancel = context.WithTimeout(context.Background(), 1*time.Second)
 	defer cancel()
-	header, err := l2Seq.HeaderByNumber(ctx, receipt.BlockNumber)
+	header, err := l2Verif.HeaderByNumber(ctx, receipt.BlockNumber)
 	require.Nil(t, err)
 
 	ctx, cancel = context.WithTimeout(context.Background(), 1*time.Second)
 	defer cancel()
-	endBalance, err = l2Seq.BalanceAt(ctx, fromAddr, nil)
+	endBalance, err = l2Verif.BalanceAt(ctx, fromAddr, nil)
 	require.Nil(t, err)
 
 	// Take fee into account
@@ -793,25 +789,17 @@ func TestWithdrawals(t *testing.T) {
 	require.Nil(t, err)
 
 	// Wait for finalization and then create the Finalized Withdrawal Transaction
-	l2OutputOracle, err := bindings.NewL2OutputOracleCaller(sys.L2OOContractAddr, l1Client)
-	require.Nil(t, err)
-
 	ctx, cancel = context.WithTimeout(context.Background(), 10*time.Duration(cfg.L1BlockTime)*time.Second)
 	defer cancel()
-	timestamp, err := withdrawals.WaitForFinalizationPeriod(ctx, l1Client, sys.DepositContractAddr, header.Time)
-	require.Nil(t, err)
-
-	ctx, cancel = context.WithTimeout(context.Background(), 1*time.Second)
-	defer cancel()
-	blockNumber, err := l2OutputOracle.ComputeL2BlockNumber(&bind.CallOpts{Context: ctx}, new(big.Int).SetUint64(timestamp))
+	blockNumber, err := withdrawals.WaitForFinalizationPeriod(ctx, l1Client, sys.DepositContractAddr, receipt.BlockNumber)
 	require.Nil(t, err)
 
 	ctx, cancel = context.WithTimeout(context.Background(), 1*time.Second)
 	defer cancel()
-	header, err = l2Seq.HeaderByNumber(ctx, blockNumber)
+	header, err = l2Verif.HeaderByNumber(ctx, new(big.Int).SetUint64(blockNumber))
 	require.Nil(t, err)
 
-	rpc, err := rpc.Dial(sys.nodes["sequencer"].WSEndpoint())
+	rpc, err := rpc.Dial(sys.nodes["verifier"].WSEndpoint())
 	require.Nil(t, err)
 	l2client := withdrawals.NewClient(rpc)
 
@@ -831,7 +819,7 @@ func TestWithdrawals(t *testing.T) {
 		params.Value,
 		params.GasLimit,
 		params.Data,
-		params.Timestamp,
+		params.BlockNumber,
 		params.OutputRootProof,
 		params.WithdrawalProof,
 	)

op-node/withdrawals/utils.go

@@ -20,13 +20,13 @@ import (
 	"github.com/ethereum/go-ethereum/rpc"
 )
 
-// WaitForFinalizationPeriod waits until the timestamp has been submitted to the L2 Output Oracle on L1 and
-// then waits for the finalization period to be up.
+// WaitForFinalizationPeriod waits until there is OutputProof for an L2 block number larger than the supplied l2BlockNumber
+// and that the output is finalized.
 // This functions polls and can block for a very long time if used on mainnet.
-// This returns the timestamp to use for the proof generation.
-func WaitForFinalizationPeriod(ctx context.Context, client *ethclient.Client, portalAddr common.Address, timestamp uint64) (uint64, error) {
+// This returns the block number to use for the proof generation.
+func WaitForFinalizationPeriod(ctx context.Context, client *ethclient.Client, portalAddr common.Address, l2BlockNumber *big.Int) (uint64, error) {
+	l2BlockNumber = new(big.Int).Set(l2BlockNumber) // Don't clobber caller owned l2BlockNumber
 	opts := &bind.CallOpts{Context: ctx}
-	timestampBig := new(big.Int).SetUint64(timestamp)
 
 	portal, err := bindings.NewOptimismPortalCaller(portalAddr, client)
 	if err != nil {
@@ -40,21 +40,32 @@ func WaitForFinalizationPeriod(ctx context.Context, client *ethclient.Client, po
 	if err != nil {
 		return 0, err
 	}
+	submissionInterval, err := l2OO.SUBMISSIONINTERVAL(opts)
+	if err != nil {
+		return 0, err
+	}
+	// Convert blockNumber to submission interval boundary
+	rem := new(big.Int)
+	l2BlockNumber, rem = l2BlockNumber.DivMod(l2BlockNumber, submissionInterval, rem)
+	if rem.Cmp(common.Big0) != 0 {
+		l2BlockNumber = l2BlockNumber.Add(l2BlockNumber, common.Big1)
+	}
+	l2BlockNumber = l2BlockNumber.Mul(l2BlockNumber, submissionInterval)
 
 	finalizationPeriod, err := portal.FINALIZATIONPERIODSECONDS(opts)
 	if err != nil {
 		return 0, err
 	}
 
-	next, err := l2OO.LatestBlockTimestamp(opts)
+	latest, err := l2OO.LatestBlockNumber(opts)
 	if err != nil {
 		return 0, err
 	}
 
-	// Now poll
+	// Now poll for the output to be submitted on chain
 	var ticker *time.Ticker
-	diff := new(big.Int).Sub(timestampBig, next)
-	if diff.Cmp(big.NewInt(60)) > 0 {
+	diff := new(big.Int).Sub(l2BlockNumber, latest)
+	if diff.Cmp(big.NewInt(10)) > 0 {
 		ticker = time.NewTicker(time.Minute)
 	} else {
 		ticker = time.NewTicker(time.Second)
@@ -64,12 +75,12 @@ loop:
 	for {
 		select {
 		case <-ticker.C:
-			next, err = l2OO.LatestBlockTimestamp(opts)
+			latest, err = l2OO.LatestBlockNumber(opts)
 			if err != nil {
 				return 0, err
 			}
-			// Already passed next
-			if next.Cmp(timestampBig) > 0 {
+			// Already passed the submitted block (likely just equals rather than >= here).
+			if latest.Cmp(l2BlockNumber) >= 0 {
 				break loop
 			}
 		case <-ctx.Done():
@@ -78,10 +89,13 @@ loop:
 	}
 
 	// Now wait for it to be finalized
-	output, err := l2OO.GetL2Output(opts, next)
+	output, err := l2OO.GetL2Output(opts, l2BlockNumber)
 	if err != nil {
 		return 0, err
 	}
+	if output.OutputRoot == [32]byte{} {
+		return 0, errors.New("empty output root. likely no proposal at timestamp")
+	}
 	targetTimestamp := new(big.Int).Add(output.Timestamp, finalizationPeriod)
 	targetTime := time.Unix(targetTimestamp.Int64(), 0)
 	// Assume clock is relatively correct
@@ -96,7 +110,7 @@ loop:
 				return 0, err
 			}
 			if header.Time > targetTimestamp.Uint64() {
-				return next.Uint64(), nil
+				return l2BlockNumber.Uint64(), nil
 			}
 		case <-ctx.Done():
 			return 0, ctx.Err()
@@ -131,21 +145,21 @@ func NewClient(client *rpc.Client) *Client {
 
 }
 
-// FinalizedWithdrawalParameters is the set of paramets to pass to the FinalizedWithdrawal function
+// FinalizedWithdrawalParameters is the set of parameters to pass to the FinalizedWithdrawal function
 type FinalizedWithdrawalParameters struct {
 	Nonce           *big.Int
 	Sender          common.Address
 	Target          common.Address
 	Value           *big.Int
 	GasLimit        *big.Int
-	Timestamp       *big.Int
+	BlockNumber     *big.Int
 	Data            []byte
 	OutputRootProof bindings.WithdrawalVerifierOutputRootProof
 	WithdrawalProof []byte // RLP Encoded list of trie nodes to prove L2 storage
 }
 
 // FinalizeWithdrawalParameters queries L2 to generate all withdrawal parameters and proof necessary to finalize an withdrawal on L1.
-// The header provided is very imporant. It should be a block (timestamp) for which there is a submitted output in the L2 Output Oracle
+// The header provided is very important. It should be a block (timestamp) for which there is a submitted output in the L2 Output Oracle
 // contract. If not, the withdrawal will fail as it the storage proof cannot be verified if there is no submitted state root.
 func FinalizeWithdrawalParameters(ctx context.Context, l2client ProofClient, txHash common.Hash, header *types.Header) (FinalizedWithdrawalParameters, error) {
 	// Transaction receipt
@@ -194,7 +208,7 @@ func FinalizeWithdrawalParameters(ctx context.Context, l2client ProofClient, txH
 		Target:      ev.Target,
 		Value:       ev.Value,
 		GasLimit:    ev.GasLimit,
-		Timestamp: new(big.Int).SetUint64(header.Time),
+		BlockNumber: new(big.Int).Set(header.Number),
 		Data:        ev.Data,
 		OutputRootProof: bindings.WithdrawalVerifierOutputRootProof{
 			Version:               [32]byte{}, // Empty for version 1

op-proposer/drivers/l2output/driver.go

@@ -95,58 +95,41 @@ func (d *Driver) GetBlockRange(
 		Context: ctx,
 	}
 
-	// Determine the next uncommitted L2 block number. We do so by transforming
-	// the timestamp of the latest committed L2 block into its block number and
-	// adding one.
-	l2ooTimestamp, err := d.l2ooContract.LatestBlockTimestamp(callOpts)
+	// Determine the last committed L2 Block Number
+	start, err := d.l2ooContract.LatestBlockNumber(callOpts)
 	if err != nil {
-		d.l.Error(name+" unable to get latest block timestamp", "err", err)
-		return nil, nil, err
-	}
-	start, err := d.l2ooContract.ComputeL2BlockNumber(callOpts, l2ooTimestamp)
-	if err != nil {
-		d.l.Error(name+" unable to compute latest l2 block number", "err", err)
+		d.l.Error(name+" unable to get latest block number", "err", err)
 		return nil, nil, err
 	}
 	start.Add(start, bigOne)
 
-	// Next we need to obtain the current timestamp and the next timestamp at
-	// which we will need to submit an L2 output. The former is done by simply
-	// adding the submission interval to the latest committed block's timestamp;
-	// the latter inspects the timestamp of the latest block.
-	nextTimestamp, err := d.l2ooContract.NextTimestamp(callOpts)
+	// Next determine the L2 block that we need to commit
+	nextBlockNumber, err := d.l2ooContract.NextBlockNumber(callOpts)
 	if err != nil {
-		d.l.Error(name+" unable to get next block timestamp", "err", err)
+		d.l.Error(name+" unable to get next block number", "err", err)
 		return nil, nil, err
 	}
-	latestHeader, err := d.cfg.L1Client.HeaderByNumber(ctx, nil)
+	latestHeader, err := d.cfg.L2Client.HeaderByNumber(ctx, nil)
 	if err != nil {
 		d.l.Error(name+" unable to retrieve latest header", "err", err)
 		return nil, nil, err
 	}
-	currentTimestamp := big.NewInt(int64(latestHeader.Time))
+	currentBlockNumber := big.NewInt(latestHeader.Number.Int64())
 
-	// If the submission window has yet to elapsed, we must wait before
-	// submitting our L2 output commitment. Return start as the end value which
-	// will signal that there is no work to be done.
-	if currentTimestamp.Cmp(nextTimestamp) < 0 {
+	// If we do not have the new L2 Block number
+	if currentBlockNumber.Cmp(nextBlockNumber) < 0 {
 		d.l.Info(name+" submission interval has not elapsed",
-			"currentTimestamp", currentTimestamp, "nextTimestamp", nextTimestamp)
+			"currentBlockNumber", currentBlockNumber, "nextBlockNumber", nextBlockNumber)
 		return start, start, nil
 	}
 
 	d.l.Info(name+" submission interval has elapsed",
-		"currentTimestamp", currentTimestamp, "nextTimestamp", nextTimestamp)
+		"currentBlockNumber", currentBlockNumber, "nextBlockNumber", nextBlockNumber)
 
 	// Otherwise the submission interval has elapsed. Transform the next
 	// expected timestamp into its L2 block number, and add one since end is
 	// exclusive.
-	end, err := d.l2ooContract.ComputeL2BlockNumber(callOpts, nextTimestamp)
-	if err != nil {
-		d.l.Error(name+" unable to compute next l2 block number", "err", err)
-		return nil, nil, err
-	}
-	end.Add(end, bigOne)
+	end := new(big.Int).Add(nextBlockNumber, bigOne)
 
 	return start, end, nil
 }
@@ -174,35 +157,11 @@ func (d *Driver) CraftTx(
 		return nil, err
 	}
 
-	// Fetch the next expected timestamp that we will submit along with the
-	// L2Output.
-	callOpts := &bind.CallOpts{
-		Pending: false,
-		Context: ctx,
-	}
-	timestamp, err := d.l2ooContract.NextTimestamp(callOpts)
-	if err != nil {
-		return nil, err
-	}
-
-	// Sanity check that we are submitting against the same expected timestamp.
-	expCheckpointBlock, err := d.l2ooContract.ComputeL2BlockNumber(
-		callOpts, timestamp,
-	)
-	if err != nil {
-		return nil, err
-	}
-	if nextCheckpointBlock.Cmp(expCheckpointBlock) != 0 {
-		return nil, fmt.Errorf("expected next checkpoint block to be %d, "+
-			"found %d", nextCheckpointBlock.Uint64(),
-			expCheckpointBlock.Uint64())
-	}
-
 	numElements := new(big.Int).Sub(start, end).Uint64()
 	d.l.Info(name+" checkpoint constructed", "start", start, "end", end,
 		"nonce", nonce, "blocks_committed", numElements, "checkpoint_block", nextCheckpointBlock)
 
-	header, err := d.cfg.L1Client.HeaderByNumber(ctx, nil)
+	l1Header, err := d.cfg.L1Client.HeaderByNumber(ctx, nil)
 	if err != nil {
 		return nil, fmt.Errorf("error resolving checkpoint block: %v", err)
 	}
@@ -212,8 +171,8 @@ func (d *Driver) CraftTx(
 		return nil, fmt.Errorf("error resolving checkpoint block: %v", err)
 	}
 
-	if l2Header.Time != timestamp.Uint64() {
-		return nil, fmt.Errorf("invalid timestamp: next timestamp is %v, timestamp of block is %v", timestamp, l2Header.Time)
+	if l2Header.Number.Cmp(nextCheckpointBlock) != 0 {
+		return nil, fmt.Errorf("invalid blockNumber: next blockNumber is %v, blockNumber of block is %v", nextCheckpointBlock, l2Header.Number)
 	}
 
 	opts, err := bind.NewKeyedTransactorWithChainID(
@@ -226,7 +185,7 @@ func (d *Driver) CraftTx(
 	opts.Nonce = nonce
 	opts.NoSend = true
 
-	return d.l2ooContract.AppendL2Output(opts, l2OutputRoot, timestamp, header.Hash(), header.Number)
+	return d.l2ooContract.AppendL2Output(opts, l2OutputRoot, nextCheckpointBlock, l1Header.Hash(), l1Header.Number)
 }
 
 // UpdateGasPrice signs an otherwise identical txn to the one provided but with

packages/contracts-bedrock/.gas-snapshot

@@ -38,7 +38,7 @@ L1StandardBridge_Test:test_depositERC20To() (gas: 454650)
 L1StandardBridge_Test:test_depositETH() (gas: 247054)
 L1StandardBridge_Test:test_depositETHTo() (gas: 204938)
 L1StandardBridge_Test:test_donateETH() (gas: 17545)
-L1StandardBridge_Test:test_finalizeERC20Withdrawal() (gas: 438817)
+L1StandardBridge_Test:test_finalizeERC20Withdrawal() (gas: 438824)
 L1StandardBridge_Test:test_finalizeETHWithdrawal() (gas: 48005)
 L1StandardBridge_Test:test_initialize() (gas: 14885)
 L1StandardBridge_Test:test_onlyEOADepositERC20() (gas: 12085)
@@ -56,21 +56,21 @@ L2CrossDomainMessenger_Test:test_L2MessengerSendMessage() (gas: 119659)
 L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 133096)
 L2CrossDomainMessenger_Test:test_L2MessengerXDomainSenderReverts() (gas: 10588)
 L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 54859)
-L2OutputOracleTest:testCannot_appendCurrentTimestamp() (gas: 18627)
-L2OutputOracleTest:testCannot_appendEmptyOutput() (gas: 16756)
-L2OutputOracleTest:testCannot_appendFutureTimestamp() (gas: 18708)
-L2OutputOracleTest:testCannot_appendOutputIfNotSequencer() (gas: 16458)
-L2OutputOracleTest:testCannot_appendUnexpectedTimestamp() (gas: 18893)
-L2OutputOracleTest:testCannot_computePreHistoricalL2BlockNumber() (gas: 11093)
-L2OutputOracleTest:testCannot_deleteL2Output_ifNotSequencer() (gas: 18793)
-L2OutputOracleTest:testCannot_deleteWrongL2Output() (gas: 77352)
-L2OutputOracleTest:test_appendingAnotherOutput() (gas: 68582)
-L2OutputOracleTest:test_computeL2BlockNumber() (gas: 14755)
-L2OutputOracleTest:test_constructor() (gas: 33752)
-L2OutputOracleTest:test_deleteL2Output() (gas: 64338)
-L2OutputOracleTest:test_getL2Output() (gas: 74601)
-L2OutputOracleTest:test_latestBlockTimestamp() (gas: 68377)
-L2OutputOracleTest:test_nextTimestamp() (gas: 9236)
+L2OutputOracleTest:testCannot_appendEmptyOutput() (gas: 18442)
+L2OutputOracleTest:testCannot_appendFutureTimetamp() (gas: 20072)
+L2OutputOracleTest:testCannot_appendOnWrongFork() (gas: 20710)
+L2OutputOracleTest:testCannot_appendOutputIfNotSequencer() (gas: 17829)
+L2OutputOracleTest:testCannot_appendUnexpectedBlockNumber() (gas: 20313)
+L2OutputOracleTest:testCannot_deleteL2Output_ifNotSequencer() (gas: 18805)
+L2OutputOracleTest:testCannot_deleteWrongL2Output() (gas: 79498)
+L2OutputOracleTest:test_appendWithBlockhashAndHeight() (gas: 69365)
+L2OutputOracleTest:test_appendingAnotherOutput() (gas: 70714)
+L2OutputOracleTest:test_computeL2Timestamp() (gas: 19230)
+L2OutputOracleTest:test_constructor() (gas: 33908)
+L2OutputOracleTest:test_deleteL2Output() (gas: 66081)
+L2OutputOracleTest:test_getL2Output() (gas: 76274)
+L2OutputOracleTest:test_latestBlockNumber() (gas: 70075)
+L2OutputOracleTest:test_nextBlockNumber() (gas: 9279)
 L2StandardBridge_Test:test_cannotWithdrawEthWithoutSendingIt() (gas: 21578)
 L2StandardBridge_Test:test_finalizeDeposit() (gas: 93165)
 L2StandardBridge_Test:test_finalizeDeposit_failsToCompleteOutboundTransfer() (gas: 140106)
@@ -108,7 +108,7 @@ OptimismMintableTokenFactory_Test:test_initializeShouldRevert() (gas: 12696)
 OptimismPortal_Test:test_OptimismPortalConstructor() (gas: 11413)
 OptimismPortal_Test:test_OptimismPortalContractCreationReverts() (gas: 9214)
 OptimismPortal_Test:test_OptimismPortalReceiveEth() (gas: 121706)
-OptimismPortal_Test:test_cannotVerifyRecentWithdrawal() (gas: 21863)
+OptimismPortal_Test:test_cannotVerifyRecentWithdrawal() (gas: 21886)
 OptimismPortal_Test:test_depositTransaction_NoValueContract() (gas: 70746)
 OptimismPortal_Test:test_depositTransaction_NoValueEOA() (gas: 71114)
 OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract() (gas: 70773)
@@ -117,7 +117,7 @@ OptimismPortal_Test:test_depositTransaction_withEthValueAndContractContractCreat
 OptimismPortal_Test:test_depositTransaction_withEthValueAndEOAContractCreation() (gas: 69947)
 OptimismPortal_Test:test_depositTransaction_withEthValueFromContract() (gas: 77478)
 OptimismPortal_Test:test_depositTransaction_withEthValueFromEOA() (gas: 78049)
-OptimismPortal_Test:test_invalidWithdrawalProof() (gas: 28541)
+OptimismPortal_Test:test_invalidWithdrawalProof() (gas: 33702)
 Proxy_Test:test_clashingFunctionSignatures() (gas: 101427)
 Proxy_Test:test_implementationKey() (gas: 20942)
 Proxy_Test:test_implementationProxyCallIfNotAdmin() (gas: 30021)
@@ -163,5 +163,5 @@ ResourceMetering_Test:test_useMoreThanMaxReverts() (gas: 16024)
 SequencerFeeVault_Test:test_constructor() (gas: 7611)
 SequencerFeeVault_Test:test_minWithdrawalAmount() (gas: 5429)
 SequencerFeeVault_Test:test_receive() (gas: 17280)
-SequencerFeeVault_Test:test_revertWithdraw() (gas: 9245)
-SequencerFeeVault_Test:test_withdraw() (gas: 147297)
+SequencerFeeVault_Test:test_revertWithdraw() (gas: 9266)
+SequencerFeeVault_Test:test_withdraw() (gas: 147300)

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

@@ -150,7 +150,7 @@ contract OptimismPortal is ResourceMetering {
      * @param _value           ETH to send to the target.
      * @param _gasLimit        Minumum gas to be forwarded to the target.
      * @param _data            Data to send to the target.
-     * @param _l2Timestamp     L2 timestamp of the outputRoot.
+     * @param _l2BlockNumber   L2 block number of the outputRoot.
      * @param _outputRootProof Inclusion proof of the withdrawer contracts storage root.
      * @param _withdrawalProof Inclusion proof for the given withdrawal in the withdrawer contract.
      */
@@ -161,7 +161,7 @@ contract OptimismPortal is ResourceMetering {
         uint256 _value,
         uint256 _gasLimit,
         bytes calldata _data,
-        uint256 _l2Timestamp,
+        uint256 _l2BlockNumber,
         WithdrawalVerifier.OutputRootProof calldata _outputRootProof,
         bytes calldata _withdrawalProof
     ) external payable {
@@ -179,7 +179,7 @@ contract OptimismPortal is ResourceMetering {
         );
 
         // Get the output root.
-        L2OutputOracle.OutputProposal memory proposal = L2_ORACLE.getL2Output(_l2Timestamp);
+        L2OutputOracle.OutputProposal memory proposal = L2_ORACLE.getL2Output(_l2BlockNumber);
 
         // Ensure that enough time has passed since the proposal was submitted before allowing a
         // withdrawal. Under the assumption that the fault proof mechanism is operating correctly,

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

@@ -47,33 +47,35 @@ contract L2OutputOracle_Initializer is CommonTest {
     L2OutputOracle oracle;
 
     // Constructor arguments
-    address sequencer = 0x000000000000000000000000000000000000AbBa;
-    uint256 submissionInterval = 1800;
-    uint256 l2BlockTime = 2;
+    uint256 submissionInterval = 42;
     bytes32 genesisL2Output = keccak256(abi.encode(0));
-    uint256 historicalTotalBlocks = 100;
-
-    // Cache of the initial L2 timestamp
-    uint256 startingBlockTimestamp;
+    uint256 historicalTotalBlocks = 199;
+    uint256 startingBlockNumber = 200;
+    uint256 startingTimestamp = 1000;
+    uint256 l2BlockTime = 2;
+    address sequencer = 0x000000000000000000000000000000000000AbBa;
 
-    // By default the first block has timestamp zero, which will cause underflows in the tests
-    uint256 initTime = 1000;
+    // Test data
+    uint256 initL1Time;
 
     function setUp() public virtual {
         _setUp();
 
-        // Move time forward so we have a non-zero starting timestamp
-        vm.warp(initTime);
+        // By default the first block has timestamp and number zero, which will cause underflows in the
+        // tests, so we'll move forward to these block values.
+        initL1Time = startingTimestamp + 1;
+        vm.warp(initL1Time);
+        vm.roll(startingBlockNumber);
         // Deploy the L2OutputOracle and transfer owernship to the sequencer
         oracle = new L2OutputOracle(
             submissionInterval,
-            l2BlockTime,
             genesisL2Output,
             historicalTotalBlocks,
-            initTime,
+            startingBlockNumber,
+            startingTimestamp,
+            l2BlockTime,
             sequencer
         );
-        startingBlockTimestamp = block.timestamp;
     }
 }
 

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

 //SPDX-License-Identifier: MIT
 pragma solidity 0.8.10;
 
-import { CommonTest } from "./CommonTest.t.sol";
+import { L2OutputOracle_Initializer } from "./CommonTest.t.sol";
 
 import { AddressAliasHelper } from "../libraries/AddressAliasHelper.sol";
 import { L2OutputOracle } from "../L1/L2OutputOracle.sol";
 import { OptimismPortal } from "../L1/OptimismPortal.sol";
 import { WithdrawalVerifier } from "../libraries/Lib_WithdrawalVerifier.sol";
 
-contract OptimismPortal_Test is CommonTest {
+contract OptimismPortal_Test is L2OutputOracle_Initializer {
     event TransactionDeposited(
         address indexed from,
         address indexed to,
@@ -20,19 +20,11 @@ contract OptimismPortal_Test is CommonTest {
     );
 
     // Dependencies
-    L2OutputOracle oracle;
+    // L2OutputOracle oracle;
     OptimismPortal op;
 
-    function setUp() external {
-        _setUp();
-        oracle = new L2OutputOracle(
-            1800,
-            2,
-            keccak256(abi.encode(0)),
-            100,
-            1,
-            address(666)
-        );
+    function setUp() public override {
+        L2OutputOracle_Initializer.setUp();
         op = new OptimismPortal(oracle, 7 days);
     }
 
@@ -44,15 +36,7 @@ contract OptimismPortal_Test is CommonTest {
 
     function test_OptimismPortalReceiveEth() external {
         vm.expectEmit(true, true, false, true);
-        emit TransactionDeposited(
-            alice,
-            alice,
-            100,
-            100,
-            100_000,
-            false,
-            hex""
-        );
+        emit TransactionDeposited(alice, alice, 100, 100, 100_000, false, hex"");
 
         // give alice money and send as an eoa
         vm.deal(alice, 2**64);
@@ -254,7 +238,8 @@ contract OptimismPortal_Test is CommonTest {
     // function test_verifyWithdrawal() external {}
 
     function test_cannotVerifyRecentWithdrawal() external {
-        WithdrawalVerifier.OutputRootProof memory outputRootProof = WithdrawalVerifier.OutputRootProof({
+        WithdrawalVerifier.OutputRootProof memory outputRootProof = WithdrawalVerifier
+            .OutputRootProof({
                 version: bytes32(0),
                 stateRoot: bytes32(0),
                 withdrawerStorageRoot: bytes32(0),
@@ -262,39 +247,26 @@ contract OptimismPortal_Test is CommonTest {
             });
 
         vm.expectRevert("OptimismPortal: proposal is not yet finalized");
-        op.finalizeWithdrawalTransaction(
-            0,
-            alice,
-            alice,
-            0,
-            0,
-            hex"",
-            0,
-            outputRootProof,
-            hex""
-        );
+        op.finalizeWithdrawalTransaction(0, alice, alice, 0, 0, hex"", 0, outputRootProof, hex"");
     }
 
     function test_invalidWithdrawalProof() external {
-        WithdrawalVerifier.OutputRootProof memory outputRootProof = WithdrawalVerifier.OutputRootProof({
+        WithdrawalVerifier.OutputRootProof memory outputRootProof = WithdrawalVerifier
+            .OutputRootProof({
                 version: bytes32(0),
                 stateRoot: bytes32(0),
                 withdrawerStorageRoot: bytes32(0),
                 latestBlockhash: bytes32(0)
             });
 
-        vm.warp(oracle.nextTimestamp() + op.FINALIZATION_PERIOD_SECONDS());
-        vm.expectRevert("OptimismPortal: invalid output root proof");
-        op.finalizeWithdrawalTransaction(
-            0,
-            alice,
-            alice,
-            0,
-            0,
-            hex"",
-            0,
-            outputRootProof,
-            hex""
+        vm.warp(
+            oracle.getL2Output(
+                oracle.latestBlockNumber()
+            ).timestamp
+            + op.FINALIZATION_PERIOD_SECONDS()
         );
+
+        vm.expectRevert("OptimismPortal: invalid output root proof");
+        op.finalizeWithdrawalTransaction(0, alice, alice, 0, 0, hex"", 0, outputRootProof, hex"");
     }
 }

specs/proposals.md

@@ -91,28 +91,28 @@ where:
 ## L2 Output Oracle Smart Contract
 
 L2 blocks are produced at a constant rate of `L2_BLOCK_TIME` (2 seconds).
-A new L2 output MUST be appended to the chain once per `SUBMISSION_INTERVAL` (1800 seconds). Note that this interval is\
-based on L2 time. It is OK to have L2 outputs submitted at larger or small intervals.
+A new L2 output MUST be appended to the chain once per `SUBMISSION_INTERVAL` which is based on a number of blocks.
+The exact number is yet to be determined, and will depend on the design of the fault proving game.
 
 The L2 Output Oracle contract implements the following interface:
 
 ```js
 /**
- * Accepts an L2 output checkpoint and the timestamp of the corresponding L2
- * block. The timestamp must be equal to the current value returned by
- * `nextTimestamp()` in order to be accepted.
+ * @notice Accepts an L2 outputRoot and the timestamp of the corresponding L2 block. The
+ * timestamp must be equal to the current value returned by `nextTimestamp()` in order to be
+ * accepted.
  * This function may only be called by the Sequencer.
  * @param _l2Output      The L2 output of the checkpoint block.
- * @param _l2timestamp The L2 block timestamp that resulted in _l2Output.
+ * @param _l2BlockNumber The L2 block number that resulted in _l2Output.
  * @param _l1Blockhash   A block hash which must be included in the current chain.
- * @param _l1Blocknumber The block number with the specified block hash.
- */
-function appendL2Output(
+ * @param _l1BlockNumber The block number with the specified block hash.
+*/
+  function appendL2Output(
       bytes32 _l2Output,
-    uint256 _l2timestamp,
+      uint256 _l2BlockNumber,
       bytes32 _l1Blockhash,
-    uint256 _l1Blocknumber
-)
+      uint256 _l1BlockNumber
+  )
 
 /**
  * @notice Deletes the most recent output.
@@ -122,15 +122,9 @@ function appendL2Output(
 function deleteL2Output(bytes32 _l2Output) external
 
 /**
- * Computes the timestamp of the next L2 block that needs to be checkpointed.
- */
-function nextTimestamp() public view returns (uint256)
-
-/**
- * Computes the L2 block number given a target L2 block timestamp.
- * @param _timestamp The L2 block timestamp of the target block.
+ * @notice Computes the block number of the next L2 block that needs to be checkpointed.
  */
-function computeL2BlockNumber(uint256 _timestamp) public view returns (uint256)
+function nextBlockNumber() public view returns (uint256) {
 ```
 
 ## Security Considerations