discard transactions from the pool that error during execution (#1490)

zk/stages/stage_sequence_execute.go

@@ -378,6 +378,7 @@ func sequencingBatchStep(
 					log.Trace(fmt.Sprintf("[%s] Yielded transactions from the pool", logPrefix), "txCount", len(batchState.blockState.transactionsForInclusion))
 				}
 
+				badTxIndexes := make([]int, 0)
 				for i, transaction := range batchState.blockState.transactionsForInclusion {
 					txHash := transaction.Hash()
 					effectiveGas := batchState.blockState.getL1EffectiveGases(cfg, i)
@@ -412,9 +413,12 @@ func sequencingBatchStep(
 							continue
 						}
 
-						// if running in normal operation mode and error != nil then just allow the code to continue
-						// It is safe because this approach ensures that the problematic transaction (the one that caused err != nil to be returned) is kept in yielded
-						// Each transaction in yielded will be reevaluated at the end of each batch
+						// if we have an error at this point something has gone wrong, either in the pool or otherwise
+						// to stop the pool growing and hampering further processing of good transactions here
+						// we mark it for being discarded
+						log.Warn(fmt.Sprintf("[%s] error adding transaction to batch, discarding from pool", logPrefix), "hash", txHash, "err", err)
+						badTxIndexes = append(badTxIndexes, i)
+						batchState.blockState.transactionsToDiscard = append(batchState.blockState.transactionsToDiscard, batchState.blockState.transactionHashesToSlots[txHash])
 					}
 
 					switch anyOverflow {
@@ -501,6 +505,12 @@ func sequencingBatchStep(
 					}
 				}
 
+				// remove transactions that have been marked for removal
+				for i := len(badTxIndexes) - 1; i >= 0; i-- {
+					idx := badTxIndexes[i]
+					batchState.blockState.transactionsForInclusion = append(batchState.blockState.transactionsForInclusion[:idx], batchState.blockState.transactionsForInclusion[idx+1:]...)
+				}
+
 				if batchState.isL1Recovery() {
 					// just go into the normal loop waiting for new transactions to signal that the recovery
 					// has finished as far as it can go
@@ -523,6 +533,7 @@ func sequencingBatchStep(
 		}
 
 		cfg.txPool.RemoveMinedTransactions(batchState.blockState.builtBlockElements.txSlots)
+		cfg.txPool.RemoveMinedTransactions(batchState.blockState.transactionsToDiscard)
 
 		if batchState.isLimboRecovery() {
 			stateRoot := block.Root()

zk/stages/stage_sequence_execute_state.go

@@ -257,6 +257,7 @@ type BlockState struct {
 	transactionHashesToSlots map[common.Hash]common.Hash
 	builtBlockElements       BuiltBlockElements
 	blockL1RecoveryData      *zktx.DecodedBatchL2Data
+	transactionsToDiscard    []common.Hash
 }
 
 func newBlockState() *BlockState {