l2geth: update timestamp logic

This commit updates the timestamp updating logic
such that `time.Now` is used instead of relying on
L1 timestamps. This gives a higher fidelity for the
`TIMESTAMP` opcode as well as makes the time on L2
be closer to the time on L1.

L1 to L2 transactions no longer have the property of
having the same timestamp on L2 as the timestamp
of the L1 block they were included in.

This should be able to be turned on without needing
hardfork logic as replicas should always accept the
timestamp that the sequencer sets. The sequencer is
a trusted entity in the existing implementation and
it is expected that the sequencer will become more
trustless in future iterations of the protocol.

This change is added to improve both UX and devex.
Users are confused by the timestamps on Etherscan
being ~15 minutes behind. This is due to the timestamps
being set based on L1 block numbers, and the system
only pulls L1 data once a secure amount of PoW
is placed on top. Developers would like the timestamps
to have a higher fidelity and be closer to the timestamps
on L1.

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
 }