Merge pull request ethereum-optimism#1982 from ethereum-optimism/feat…

…/sequencer-timestamp

l2geth: update timestamp logic

.changeset/happy-ears-rhyme.md

@@ -0,0 +1,5 @@
+---
+'@eth-optimism/l2geth': patch
+---
+
+Implement updated timestamp logic

.changeset/tame-trains-relax.md

@@ -0,0 +1,5 @@
+---
+'@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 {