Merge pull request #1785 from ethereum-optimism/bugfix/fix-deadlock-2

l2geth: fix deadlock on invalid txs

.changeset/spicy-spoons-jog.md

+---
+'@eth-optimism/l2geth': patch
+---
+
+Fixes deadlock

l2geth/core/events.go

@@ -22,7 +22,10 @@ import (
 )
 
 // NewTxsEvent is posted when a batch of transactions enter the transaction pool.
-type NewTxsEvent struct{ Txs []*types.Transaction }
+type NewTxsEvent struct {
+	Txs   []*types.Transaction
+	ErrCh chan error
+}
 
 // NewMinedBlockEvent is posted when a block has been imported.
 type NewMinedBlockEvent struct{ Block *types.Block }

l2geth/core/tx_pool.go

@@ -1090,7 +1090,7 @@ func (pool *TxPool) runReorg(done chan struct{}, reset *txpoolResetRequest, dirt
 		for _, set := range events {
 			txs = append(txs, set.Flatten()...)
 		}
-		pool.txFeed.Send(NewTxsEvent{txs})
+		pool.txFeed.Send(NewTxsEvent{Txs: txs})
 	}
 }
 

l2geth/miner/worker.go

@@ -506,7 +506,10 @@ func (w *worker) mainLoop() {
 				}
 				w.pendingMu.Unlock()
 			} else {
-				log.Debug("Problem committing transaction", "msg", err)
+				log.Error("Problem committing transaction", "msg", err)
+				if ev.ErrCh != nil {
+					ev.ErrCh <- err
+				}
 			}
 
 		case ev := <-w.txsCh:
@@ -781,6 +784,11 @@ func (w *worker) commitTransactions(txs *types.TransactionsByPriceAndNonce, coin
 	}
 
 	var coalescedLogs []*types.Log
+	// UsingOVM
+	// Keep track of the number of transactions being added to the block.
+	// Blocks should only have a single transaction. This value is used to
+	// compute a success return value
+	var txCount int
 
 	for {
 		// In the following three cases, we will interrupt the execution of the transaction.
@@ -814,6 +822,8 @@ func (w *worker) commitTransactions(txs *types.TransactionsByPriceAndNonce, coin
 			break
 		}
 
+		txCount++
+
 		// Error may be ignored here. The error has already been checked
 		// during transaction acceptance is the transaction pool.
 		//
@@ -881,7 +891,7 @@ func (w *worker) commitTransactions(txs *types.TransactionsByPriceAndNonce, coin
 	if interrupt != nil {
 		w.resubmitAdjustCh <- &intervalAdjust{inc: false}
 	}
-	return false
+	return txCount == 0
 }
 
 // commitNewTx is an OVM addition that mines a block with a single tx in it.

l2geth/rollup/sync_service.go

@@ -806,9 +806,9 @@ func (s *SyncService) applyTransactionToTip(tx *types.Transaction) error {
 	// 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.
+	ts := s.GetLatestL1Timestamp()
+	bn := s.GetLatestL1BlockNumber()
 	if tx.L1Timestamp() == 0 {
-		ts := s.GetLatestL1Timestamp()
-		bn := s.GetLatestL1BlockNumber()
 		tx.SetL1Timestamp(ts)
 		tx.SetL1BlockNumber(bn)
 	} else if tx.L1Timestamp() > s.GetLatestL1Timestamp() {
@@ -816,17 +816,15 @@ func (s *SyncService) applyTransactionToTip(tx *types.Transaction) error {
 		// service's locally maintained timestamp, update the timestamp and
 		// blocknumber to equal that of the transaction's. This should happen
 		// with `enqueue` transactions.
-		ts := tx.L1Timestamp()
-		bn := tx.L1BlockNumber()
-		s.SetLatestL1Timestamp(ts)
-		s.SetLatestL1BlockNumber(bn.Uint64())
-		log.Debug("Updating OVM context based on new transaction", "timestamp", ts, "blocknumber", bn.Uint64(), "queue-origin", tx.QueueOrigin())
+		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())
 	} else if tx.L1Timestamp() < s.GetLatestL1Timestamp() {
 		log.Error("Timestamp monotonicity violation", "hash", tx.Hash().Hex())
 	}
 
+	index := s.GetLatestIndex()
 	if tx.GetMeta().Index == nil {
-		index := s.GetLatestIndex()
 		if index == nil {
 			tx.SetIndex(0)
 		} else {
@@ -846,21 +844,36 @@ func (s *SyncService) applyTransactionToTip(tx *types.Transaction) error {
 	log.Debug("Applying transaction to tip", "index", *tx.GetMeta().Index, "hash", tx.Hash().Hex(), "origin", tx.QueueOrigin().String())
 
 	txs := types.Transactions{tx}
-	s.txFeed.Send(core.NewTxsEvent{Txs: txs})
+	errCh := make(chan error, 1)
+	s.txFeed.Send(core.NewTxsEvent{
+		Txs:   txs,
+		ErrCh: errCh,
+	})
 	// Block until the transaction has been added to the chain
 	log.Trace("Waiting for transaction to be added to chain", "hash", tx.Hash().Hex())
-	<-s.chainHeadCh
-
-	// Update the cache when the transaction is from the owner
-	// of the gas price oracle
-	sender, _ := types.Sender(s.signer, tx)
-	owner := s.GasPriceOracleOwnerAddress()
-	if owner != nil && sender == *owner {
-		if err := s.updateGasPriceOracleCache(nil); err != nil {
-			return err
+
+	select {
+	case err := <-errCh:
+		log.Error("Got error waiting for transaction to be added to chain", "msg", err)
+		s.SetLatestL1Timestamp(ts)
+		s.SetLatestL1BlockNumber(bn)
+		s.SetLatestIndex(index)
+		return err
+	case <-s.chainHeadCh:
+		// Update the cache when the transaction is from the owner
+		// of the gas price oracle
+		sender, _ := types.Sender(s.signer, tx)
+		owner := s.GasPriceOracleOwnerAddress()
+		if owner != nil && sender == *owner {
+			if err := s.updateGasPriceOracleCache(nil); err != nil {
+				s.SetLatestL1Timestamp(ts)
+				s.SetLatestL1BlockNumber(bn)
+				s.SetLatestIndex(index)
+				return err
+			}
 		}
+		return nil
 	}
-	return nil
 }
 
 // applyBatchedTransaction applies transactions that were batched to layer one.