Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion miner/scroll_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
Expand Down
26 changes: 26 additions & 0 deletions miner/scroll_worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion params/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
15 changes: 15 additions & 0 deletions rollup/circuitcapacitychecker/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ package circuitcapacitychecker
import (
"math/rand"

"github.com/scroll-tech/go-ethereum/common"
"github.com/scroll-tech/go-ethereum/core/types"
)

type CircuitCapacityChecker struct {
ID uint64
countdown int
nextError *error

skipHash string
skipError error
}

// NewCircuitCapacityChecker creates a new CircuitCapacityChecker
Expand All @@ -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,
Expand Down Expand Up @@ -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
}