Merge pull request #1982 from ethereum-optimism/feat/sequencer-timestamp

l2geth: update timestamp logic

.changeset/happy-ears-rhyme.md

+---
+'@eth-optimism/l2geth': patch
+---
+
+Implement updated timestamp logic

.changeset/tame-trains-relax.md

+---
+'@eth-optimism/integration-tests': patch
+---
+
+Update timestamp assertion for new logic

integration-tests/test/ovmcontext.spec.ts

@@ -2,7 +2,7 @@ import { expect } from './shared/setup'
 
 /* Imports: External */
 import { ethers } from 'hardhat'
-import { injectL2Context } from '@eth-optimism/core-utils'
+import { injectL2Context, expectApprox } from '@eth-optimism/core-utils'
 import { predeploys } from '@eth-optimism/contracts'
 import { Contract, BigNumber } from 'ethers'
 
@@ -74,9 +74,11 @@ describe('OVM Context: Layer 2 EVM Context', () => {
       const l1BlockNumber = await OVMContextStorage.l1BlockNumbers(i)
       expect(l1BlockNumber.toNumber()).to.deep.equal(l1Block.number)
 
-      // L1 and L2 blocks will have the same timestamp.
+      // L1 and L2 blocks will have approximately the same timestamp.
       const timestamp = await OVMContextStorage.timestamps(i)
-      expect(timestamp.toNumber()).to.deep.equal(l1Block.timestamp)
+      expectApprox(timestamp.toNumber(), l1Block.timestamp, {
+        percentUpperDeviation: 5,
+      })
       expect(timestamp.toNumber()).to.deep.equal(l2Block.timestamp)
 
       // Difficulty should always be zero.

integration-tests/test/shared/env.ts

@@ -33,6 +33,7 @@ export class OptimismEnv {
   addressManager: Contract
   l1Bridge: Contract
   l1Messenger: Contract
+  l1BlockNumber: Contract
   ctc: Contract
   scc: Contract
 
@@ -59,6 +60,7 @@ export class OptimismEnv {
     this.addressManager = args.addressManager
     this.l1Bridge = args.l1Bridge
     this.l1Messenger = args.l1Messenger
+    this.l1BlockNumber = args.l1BlockNumber
     this.ovmEth = args.ovmEth
     this.l2Bridge = args.l2Bridge
     this.l2Messenger = args.l2Messenger
@@ -113,12 +115,17 @@ export class OptimismEnv {
       .connect(l2Wallet)
       .attach(predeploys.OVM_SequencerFeeVault)
 
+    const l1BlockNumber = getContractFactory('iOVM_L1BlockNumber')
+      .connect(l2Wallet)
+      .attach(predeploys.OVM_L1BlockNumber)
+
     return new OptimismEnv({
       addressManager,
       l1Bridge,
       ctc,
       scc,
       l1Messenger,
+      l1BlockNumber,
       ovmEth,
       gasPriceOracle,
       sequencerFeeVault,

integration-tests/test/stress-tests.spec.ts

@@ -211,4 +211,29 @@ describe('stress tests', () => {
       )
     }).timeout(STRESS_TEST_TIMEOUT)
   })
+
+  // These tests depend on an archive node due to the historical `eth_call`s
+  describe('Monotonicity Checks', () => {
+    it('should have monotonic timestamps and l1 blocknumbers', async () => {
+      const tip = await env.l2Provider.getBlock('latest')
+      const prev = {
+        block: await env.l2Provider.getBlock(0),
+        l1BlockNumber: await env.l1BlockNumber.getL1BlockNumber({
+          blockTag: 0,
+        }),
+      }
+      for (let i = 1; i < tip.number; i++) {
+        const block = await env.l2Provider.getBlock(i)
+        expect(block.timestamp).to.be.gte(prev.block.timestamp)
+
+        const l1BlockNumber = await env.l1BlockNumber.getL1BlockNumber({
+          blockTag: i,
+        })
+        expect(l1BlockNumber.gt(prev.l1BlockNumber))
+
+        prev.block = block
+        prev.l1BlockNumber = l1BlockNumber
+      }
+    })
+  })
 })

l2geth/rollup/sync_service.go

@@ -436,7 +436,7 @@ func (s *SyncService) SequencerLoop() {
 		}
 		s.txLock.Unlock()
 
-		if err := s.updateContext(); err != nil {
+		if err := s.updateL1BlockNumber(); err != nil {
 			log.Error("Could not update execution context", "error", err)
 		}
 	}
@@ -599,17 +599,15 @@ func (s *SyncService) GasPriceOracleOwnerAddress() *common.Address {
 
 /// Update the execution context's timestamp and blocknumber
 /// over time. This is only necessary for the sequencer.
-func (s *SyncService) updateContext() error {
+func (s *SyncService) updateL1BlockNumber() error {
 	context, err := s.client.GetLatestEthContext()
 	if err != nil {
-		return err
+		return fmt.Errorf("Cannot get eth context: %w", err)
 	}
-	current := time.Unix(int64(s.GetLatestL1Timestamp()), 0)
-	next := time.Unix(int64(context.Timestamp), 0)
-	if next.Sub(current) > s.timestampRefreshThreshold {
-		log.Info("Updating Eth Context", "timetamp", context.Timestamp, "blocknumber", context.BlockNumber)
+	latest := s.GetLatestL1BlockNumber()
+	if context.BlockNumber > latest {
+		log.Info("Updating L1 block number", "blocknumber", context.BlockNumber)
 		s.SetLatestL1BlockNumber(context.BlockNumber)
-		s.SetLatestL1Timestamp(context.Timestamp)
 	}
 	return nil
 }
@@ -798,31 +796,61 @@ func (s *SyncService) applyTransactionToTip(tx *types.Transaction) error {
 			return fmt.Errorf("Queue origin L1 to L2 transaction without a timestamp: %s", tx.Hash().Hex())
 		}
 	}
-	// If there is no OVM timestamp assigned to the transaction, then assign a
-	// timestamp and blocknumber to it. This should only be the case for queue
-	// origin sequencer transactions that come in via RPC. The L1 to L2
-	// transactions that come in via `enqueue` should have a timestamp set based
-	// on the L1 block that it was included in.
-	// Note that Ethereum Layer one consensus rules dictate that the timestamp
-	// must be strictly increasing between blocks, so no need to check both the
-	// timestamp and the blocknumber.
+
+	// If there is no L1 timestamp assigned to the transaction, then assign a
+	// timestamp to it. The property that L1 to L2 transactions have the same
+	// timestamp as the L1 block that it was included in is removed for better
+	// UX. This functionality can be added back in during a future release. For
+	// now, the sequencer will assign a timestamp to each transaction.
 	ts := s.GetLatestL1Timestamp()
 	bn := s.GetLatestL1BlockNumber()
-	if tx.L1Timestamp() == 0 {
-		tx.SetL1Timestamp(ts)
-		tx.SetL1BlockNumber(bn)
-	} else if tx.L1Timestamp() > s.GetLatestL1Timestamp() {
-		// If the timestamp of the transaction is greater than the sync
-		// service's locally maintained timestamp, update the timestamp and
-		// blocknumber to equal that of the transaction's. This should happen
-		// with `enqueue` transactions.
-		s.SetLatestL1Timestamp(tx.L1Timestamp())
-		s.SetLatestL1BlockNumber(tx.L1BlockNumber().Uint64())
-		log.Debug("Updating OVM context based on new transaction", "timestamp", ts, "blocknumber", tx.L1BlockNumber().Uint64(), "queue-origin", tx.QueueOrigin())
+
+	// The L1Timestamp is 0 for QueueOriginSequencer transactions when
+	// running as the sequencer, the transactions are coming in via RPC.
+	// This code path also runs for replicas/verifiers so any logic involving
+	// `time.Now` can only run for the sequencer. All other nodes must listen
+	// to what the sequencer says is the timestamp, otherwise there will be a
+	// network split.
+	// Note that it should never be possible for the timestamp to be set to
+	// 0 when running as a verifier.
+	shouldMalleateTimestamp := !s.verifier && tx.QueueOrigin() == types.QueueOriginL1ToL2
+	if tx.L1Timestamp() == 0 || shouldMalleateTimestamp {
+		// Get the latest known timestamp
+		current := time.Unix(int64(ts), 0)
+		// Get the current clocktime
+		now := time.Now()
+		// If enough time has passed, then assign the
+		// transaction to have the timestamp now. Otherwise,
+		// use the current timestamp
+		if now.Sub(current) > s.timestampRefreshThreshold {
+			current = now
+		}
+		tx.SetL1Timestamp(uint64(current.Unix()))
+	} else if tx.L1Timestamp() == 0 && s.verifier {
+		// This should never happen
+		log.Error("No tx timestamp found when running as verifier", "hash", tx.Hash().Hex())
 	} else if tx.L1Timestamp() < s.GetLatestL1Timestamp() {
+		// This should never happen, but sometimes does
 		log.Error("Timestamp monotonicity violation", "hash", tx.Hash().Hex())
 	}
 
+	l1BlockNumber := tx.L1BlockNumber()
+	// Set the L1 blocknumber
+	if l1BlockNumber == nil {
+		tx.SetL1BlockNumber(bn)
+	} else if l1BlockNumber.Uint64() > s.GetLatestL1BlockNumber() {
+		s.SetLatestL1BlockNumber(l1BlockNumber.Uint64())
+	} else {
+		// l1BlockNumber < latest l1BlockNumber
+		// indicates an error
+		log.Error("Blocknumber monotonicity violation", "hash", tx.Hash().Hex())
+	}
+
+	// Store the latest timestamp value
+	if tx.L1Timestamp() > ts {
+		s.SetLatestL1Timestamp(tx.L1Timestamp())
+	}
+
 	index := s.GetLatestIndex()
 	if tx.GetMeta().Index == nil {
 		if index == nil {
@@ -1186,24 +1214,6 @@ func (s *SyncService) syncTransactionRange(start, end uint64, backend Backend) e
 	return nil
 }
 
-// updateEthContext will update the OVM execution context's
-// timestamp and blocknumber if enough time has passed since
-// it was last updated. This is a sequencer only function.
-func (s *SyncService) updateEthContext() error {
-	context, err := s.client.GetLatestEthContext()
-	if err != nil {
-		return fmt.Errorf("Cannot get eth context: %w", err)
-	}
-	current := time.Unix(int64(s.GetLatestL1Timestamp()), 0)
-	next := time.Unix(int64(context.Timestamp), 0)
-	if next.Sub(current) > s.timestampRefreshThreshold {
-		log.Info("Updating Eth Context", "timetamp", context.Timestamp, "blocknumber", context.BlockNumber)
-		s.SetLatestL1BlockNumber(context.BlockNumber)
-		s.SetLatestL1Timestamp(context.Timestamp)
-	}
-	return nil
-}
-
 // SubscribeNewTxsEvent registers a subscription of NewTxsEvent and
 // starts sending event to the given channel.
 func (s *SyncService) SubscribeNewTxsEvent(ch chan<- core.NewTxsEvent) event.Subscription {

l2geth/rollup/sync_service_test.go

@@ -25,71 +25,153 @@ import (
 	"github.com/ethereum-optimism/optimism/l2geth/rollup/rcfg"
 )
 
-func setupLatestEthContextTest() (*SyncService, *EthContext) {
-	service, _, _, _ := newTestSyncService(false, nil)
-	resp := &EthContext{
-		BlockNumber: uint64(10),
-		BlockHash:   common.Hash{},
-		Timestamp:   uint64(service.timestampRefreshThreshold.Seconds()) + 1,
+// Test that the timestamps are updated correctly.
+// This impacts execution, for `block.timestamp`
+func TestSyncServiceTimestampUpdate(t *testing.T) {
+	service, txCh, _, err := newTestSyncService(false, nil)
+	if err != nil {
+		t.Fatal(err)
 	}
-	setupMockClient(service, map[string]interface{}{
-		"GetLatestEthContext": resp,
-	})
 
-	return service, resp
-}
+	// Get the timestamp from the sync service
+	// It should be initialized to 0
+	ts := service.GetLatestL1Timestamp()
+	if ts != 0 {
+		t.Fatalf("Unexpected timestamp: %d", ts)
+	}
 
-// Test that if applying a transaction fails
-func TestSyncServiceContextUpdated(t *testing.T) {
-	service, resp := setupLatestEthContextTest()
+	// Create a mock transaction and assert that its timestamp
+	// a value. This tests the case that the timestamp does
+	// not get malleated when it is set to a non zero value
+	timestamp := uint64(1)
+	tx1 := setMockTxL1Timestamp(mockTx(), timestamp)
+	if tx1.GetMeta().L1Timestamp != timestamp {
+		t.Fatalf("Expecting mock timestamp to be %d", timestamp)
+	}
+	if tx1.GetMeta().QueueOrigin != types.QueueOriginSequencer {
+		t.Fatalf("Expecting mock queue origin to be queue origin sequencer")
+	}
+
+	go func() {
+		err = service.applyTransactionToTip(tx1)
+	}()
+	event1 := <-txCh
 
-	// should get the expected context
-	expectedCtx := &OVMContext{
-		blockNumber: 0,
-		timestamp:   0,
+	// Ensure that the timestamp isn't malleated
+	if event1.Txs[0].GetMeta().L1Timestamp != timestamp {
+		t.Fatalf("Timestamp was malleated: %d", event1.Txs[0].GetMeta().L1Timestamp)
+	}
+	// Ensure that the timestamp in the sync service was updated
+	if service.GetLatestL1Timestamp() != timestamp {
+		t.Fatal("timestamp updated in sync service")
 	}
 
-	if service.OVMContext != *expectedCtx {
-		t.Fatal("context was not instantiated to the expected value")
+	// Now test the case for when a transaction is malleated.
+	// If the timestamp is 0, then it should be malleated and set
+	// equal to whatever the latestL1Timestamp is
+	tx2 := mockTx()
+	if tx2.GetMeta().L1Timestamp != 0 {
+		t.Fatal("Expecting mock timestamp to be 0")
 	}
+	go func() {
+		err = service.applyTransactionToTip(tx2)
+	}()
+	event2 := <-txCh
 
-	// run the update context call once
-	err := service.updateContext()
+	// Ensure that the sync service timestamp is updated
+	if service.GetLatestL1Timestamp() == 0 {
+		t.Fatal("timestamp not updated")
+	}
+	// Ensure that the timestamp is malleated to be equal to what the sync
+	// service has as the latest timestamp
+	if event2.Txs[0].GetMeta().L1Timestamp != service.GetLatestL1Timestamp() {
+		t.Fatal("unexpected timestamp update")
+	}
+
+	// L1ToL2 transactions should have their timestamp malleated
+	// Be sure to set the timestamp to a non zero value so that
+	// its specifically testing the fact its a deposit tx
+	tx3 := setMockQueueOrigin(setMockTxL1Timestamp(mockTx(), 100), types.QueueOriginL1ToL2)
+	// Get a reference to the timestamp before transaction execution
+	ts3 := service.GetLatestL1Timestamp()
+
+	go func() {
+		err = service.applyTransactionToTip(tx3)
+	}()
+	event3 := <-txCh
+
+	if event3.Txs[0].GetMeta().L1Timestamp != ts3 {
+		t.Fatal("bad malleation")
+	}
+	// Ensure that the timestamp didn't change
+	if ts3 != service.GetLatestL1Timestamp() {
+		t.Fatal("timestamp updated when it shouldn't have")
+	}
+}
+
+// Test that the L1 blocknumber is updated correctly
+func TestSyncServiceL1BlockNumberUpdate(t *testing.T) {
+	service, txCh, _, err := newTestSyncService(false, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	// should get the expected context
-	expectedCtx = &OVMContext{
-		blockNumber: resp.BlockNumber,
-		timestamp:   resp.Timestamp,
+	// Get the L1 blocknumber from the sync service
+	// It should be initialized to 0
+	bn := service.GetLatestL1BlockNumber()
+	if bn != 0 {
+		t.Fatalf("Unexpected timestamp: %d", bn)
+	}
+
+	tx1 := setMockTxL1BlockNumber(mockTx(), new(big.Int).SetUint64(1))
+	go func() {
+		err = service.applyTransactionToTip(tx1)
+	}()
+	event1 := <-txCh
+
+	// Ensure that the L1 blocknumber was not
+	// malleated
+	if event1.Txs[0].L1BlockNumber().Uint64() != 1 {
+		t.Fatal("wrong l1 blocknumber")
 	}
 
-	if service.OVMContext != *expectedCtx {
-		t.Fatal("context was not updated to the expected response even though enough time passed")
+	// Ensure that the latest L1 blocknumber was
+	// updated
+	if service.GetLatestL1BlockNumber() != 1 {
+		t.Fatal("sync service latest l1 blocknumber not updated")
 	}
 
-	// updating the context should be a no-op if time advanced by less than
-	// the refresh period
-	resp.BlockNumber += 1
-	resp.Timestamp += uint64(service.timestampRefreshThreshold.Seconds())
-	setupMockClient(service, map[string]interface{}{
-		"GetLatestEthContext": resp,
-	})
+	// Ensure that a tx without a L1 blocknumber gets one
+	// assigned
+	tx2 := setMockTxL1BlockNumber(mockTx(), nil)
+	if tx2.L1BlockNumber() != nil {
+		t.Fatal("non nil l1 blocknumber")
+	}
+	go func() {
+		err = service.applyTransactionToTip(tx2)
+	}()
+	event2 := <-txCh
 
-	// call it again
-	err = service.updateContext()
-	if err != nil {
-		t.Fatal(err)
+	if event2.Txs[0].L1BlockNumber() == nil {
+		t.Fatal("tx not assigned an l1 blocknumber")
+	}
+	if event2.Txs[0].L1BlockNumber().Uint64() != service.GetLatestL1BlockNumber() {
+		t.Fatal("tx assigned incorrect l1 blocknumber")
 	}
 
-	// should not get the context from the response because it was too soon
-	unexpectedCtx := &OVMContext{
-		blockNumber: resp.BlockNumber,
-		timestamp:   resp.Timestamp,
+	// Ensure that the latest L1 blocknumber doesn't go backwards
+	latest := service.GetLatestL1BlockNumber()
+	tx3 := setMockTxL1BlockNumber(mockTx(), new(big.Int).SetUint64(latest-1))
+	go func() {
+		err = service.applyTransactionToTip(tx3)
+	}()
+	event3 := <-txCh
+	if service.GetLatestL1BlockNumber() != latest {
+		t.Fatal("block number went backwards")
 	}
-	if service.OVMContext == *unexpectedCtx {
-		t.Fatal("context should not be updated because not enough time passed")
+
+	if event3.Txs[0].L1BlockNumber().Uint64() != latest-1 {
+		t.Fatal("l1 block number was malleated")
 	}
 }
 
@@ -257,20 +339,32 @@ func TestTransactionToTipTimestamps(t *testing.T) {
 		}
 	}
 
+	// Ensure that the timestamp was updated correctly
+	ts := service.GetLatestL1Timestamp()
+	if ts != tx2.L1Timestamp() {
+		t.Fatal("timestamp not updated correctly")
+	}
+
 	// Send a transaction with no timestamp and then let it be updated
-	// by the sync service. This will prevent monotonicity errors as well
+	// by the sync service. This will prevent monotonicity errors as well.
 	// as give timestamps to queue origin sequencer transactions
-	ts := service.GetLatestL1Timestamp()
+	// Ensure that the timestamp is set to `time.Now`
+	// when it is not set.
 	tx3 := setMockTxL1Timestamp(mockTx(), 0)
+	now := time.Now()
 	go func() {
 		err = service.applyTransactionToTip(tx3)
 	}()
 	result := <-txCh
 	service.chainHeadCh <- core.ChainHeadEvent{}
 
-	if result.Txs[0].L1Timestamp() != ts {
+	if result.Txs[0].L1Timestamp() != uint64(now.Unix()) {
 		t.Fatal("Timestamp not updated correctly")
 	}
+
+	if service.GetLatestL1Timestamp() != uint64(now.Unix()) {
+		t.Fatal("latest timestamp not updated correctly")
+	}
 }
 
 func TestApplyIndexedTransaction(t *testing.T) {
@@ -1036,6 +1130,13 @@ func setMockTxL1Timestamp(tx *types.Transaction, ts uint64) *types.Transaction {
 	return tx
 }
 
+func setMockTxL1BlockNumber(tx *types.Transaction, bn *big.Int) *types.Transaction {
+	meta := tx.GetMeta()
+	meta.L1BlockNumber = bn
+	tx.SetTransactionMeta(meta)
+	return tx
+}
+
 func setMockTxIndex(tx *types.Transaction, index uint64) *types.Transaction {
 	meta := tx.GetMeta()
 	meta.Index = &index
@@ -1050,6 +1151,13 @@ func setMockQueueIndex(tx *types.Transaction, index uint64) *types.Transaction {
 	return tx
 }
 
+func setMockQueueOrigin(tx *types.Transaction, qo types.QueueOrigin) *types.Transaction {
+	meta := tx.GetMeta()
+	meta.QueueOrigin = qo
+	tx.SetTransactionMeta(meta)
+	return tx
+}
+
 func newUint64(n uint64) *uint64 {
 	return &n
 }