fix: prioritized tx not being cleared after skipping it (#764)

* fix: prioritized tx not being cleared after skipping it

* fix nonce in comment

* chore: auto version bump [bot]

* chore: auto version bump [bot]

---------

Co-authored-by: omerfirmak <omerfirmak@users.noreply.github.com>
Co-authored-by: Péter Garamvölgyi <peter@scroll.io>
Co-authored-by: Thegaram <Thegaram@users.noreply.github.com>

miner/scroll_worker.go

@@ -775,7 +775,7 @@ func (w *worker) handlePipelineResult(res *pipeline.Result) error {
 	if res != nil && res.OverflowingTx != nil {
 		if res.FinalBlock == nil {
 			// first txn overflowed the circuit, skip
-			log.Trace("Circuit capacity limit reached for a single tx", "tx", res.OverflowingTx.Hash().String(),
+			log.Info("Circuit capacity limit reached for a single tx", "tx", res.OverflowingTx.Hash().String(),
 				"isL1Message", res.OverflowingTx.IsL1MessageTx(), "reason", res.CCCErr.Error())
 
 			// Store skipped transaction in local db
@@ -789,6 +789,7 @@ func (w *worker) handlePipelineResult(res *pipeline.Result) error {
 			if overflowingL1MsgTx := res.OverflowingTx.AsL1MessageTx(); overflowingL1MsgTx != nil {
 				rawdb.WriteFirstQueueIndexNotInL2Block(w.eth.ChainDb(), w.currentPipeline.Header.ParentHash, overflowingL1MsgTx.QueueIndex+1)
 			} else {
+				w.prioritizedTx = nil
 				w.eth.TxPool().RemoveTx(res.OverflowingTx.Hash(), true)
 			}
 		} else if !res.OverflowingTx.IsL1MessageTx() {

miner/scroll_worker_test.go

@@ -862,6 +862,12 @@ func TestPrioritizeOverflowTx(t *testing.T) {
 	tx1, _ := types.SignTx(types.NewTransaction(b.txPool.Nonce(testBankAddress)+1, testUserAddress, big.NewInt(0), params.TxGas, big.NewInt(5*params.InitialBaseFee), nil), types.HomesteadSigner{}, testBankKey)
 	// B --> A (nonce: 0, gas: 20)
 	tx2, _ := types.SignTx(types.NewTransaction(b.txPool.Nonce(testUserAddress), testBankAddress, big.NewInt(0), params.TxGas, big.NewInt(20*params.InitialBaseFee), nil), types.HomesteadSigner{}, testUserKey)
+	// B --> A (nonce: 1, gas: 20)
+	tx3, _ := types.SignTx(types.NewTransaction(b.txPool.Nonce(testUserAddress)+1, testBankAddress, big.NewInt(0), params.TxGas, big.NewInt(20*params.InitialBaseFee), nil), types.HomesteadSigner{}, testUserKey)
+	// B --> A (nonce: 2, gas: 20)
+	tx4, _ := types.SignTx(types.NewTransaction(b.txPool.Nonce(testUserAddress)+2, testBankAddress, big.NewInt(0), params.TxGas, big.NewInt(20*params.InitialBaseFee), nil), types.HomesteadSigner{}, testUserKey)
+	// A --> B (nonce: 2, gas: 5)
+	tx5, _ := types.SignTx(types.NewTransaction(b.txPool.Nonce(testBankAddress)+2, testUserAddress, big.NewInt(0), params.TxGas, big.NewInt(5*params.InitialBaseFee), nil), types.HomesteadSigner{}, testBankKey)
 
 	// Process 2 transactions with gas order: tx0 > tx1, tx1 will overflow.
 	b.txPool.AddRemotesSync([]*types.Transaction{tx0, tx1})
@@ -898,6 +904,26 @@ func TestPrioritizeOverflowTx(t *testing.T) {
 	case <-time.After(3 * time.Second): // Worker needs 1s to include new changes.
 		t.Fatalf("timeout")
 	}
+
+	w.getCCC().Skip(tx4.Hash(), circuitcapacitychecker.ErrBlockRowConsumptionOverflow)
+	assert.Equal([]error{nil, nil, nil}, b.txPool.AddRemotesSync([]*types.Transaction{tx3, tx4, tx5}))
+
+	w.start()
+	expectedTxns := []common.Hash{tx3.Hash(), tx5.Hash()}
+	for i := 0; i < 2; i++ {
+		select {
+		case ev := <-sub.Chan():
+			block := ev.Data.(core.NewMinedBlockEvent).Block
+			assert.Equal(1, len(block.Transactions()))
+			assert.Equal(expectedTxns[i], block.Transactions()[0].Hash())
+			if _, err := chain.InsertChain([]*types.Block{block}); err != nil {
+				t.Fatalf("failed to insert new mined block %d: %v", block.NumberU64(), err)
+			}
+		case <-time.After(3 * time.Second): // Worker needs 1s to include new changes.
+			t.Fatalf("timeout")
+		}
+	}
+	assert.False(b.txPool.Has(tx4.Hash()))
 }
 
 func TestSkippedTransactionDatabaseEntries(t *testing.T) {

params/version.go

@@ -24,7 +24,7 @@ import (
 const (
 	VersionMajor = 5         // Major version component of the current release
 	VersionMinor = 3         // Minor version component of the current release
-	VersionPatch = 20        // Patch version component of the current release
+	VersionPatch = 21        // Patch version component of the current release
 	VersionMeta  = "mainnet" // Version metadata to append to the version string
 )
 

rollup/circuitcapacitychecker/mock.go

@@ -5,6 +5,7 @@ package circuitcapacitychecker
 import (
 	"math/rand"
 
+	"github.com/scroll-tech/go-ethereum/common"
 	"github.com/scroll-tech/go-ethereum/core/types"
 )
 
@@ -12,6 +13,9 @@ type CircuitCapacityChecker struct {
 	ID        uint64
 	countdown int
 	nextError *error
+
+	skipHash  string
+	skipError error
 }
 
 // NewCircuitCapacityChecker creates a new CircuitCapacityChecker
@@ -36,6 +40,11 @@ func (ccc *CircuitCapacityChecker) ApplyTransaction(traces *types.BlockTrace) (*
 			return nil, err
 		}
 	}
+	if ccc.skipError != nil {
+		if traces.Transactions[0].TxHash == ccc.skipHash {
+			return nil, ccc.skipError
+		}
+	}
 	return &types.RowConsumption{types.SubCircuitRowUsage{
 		Name:      "mock",
 		RowNumber: 1,
@@ -67,3 +76,9 @@ func (ccc *CircuitCapacityChecker) ScheduleError(cnt int, err error) {
 	ccc.countdown = cnt
 	ccc.nextError = &err
 }
+
+// Skip forced CCC to return always an error for a given txn
+func (ccc *CircuitCapacityChecker) Skip(txnHash common.Hash, err error) {
+	ccc.skipHash = txnHash.String()
+	ccc.skipError = err
+}