Skip to content

fix(core/txpool/legacypool): fix flaky test TestAllowedTxSize #30975 #2172

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.

Sign up for GitHub

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
AnilChinchawale merged 1 commit into from
Mar 17, 2026
Merged

fix(core/txpool/legacypool): fix flaky test TestAllowedTxSize #30975 #2172

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 27 additions & 20 deletions core/txpool/legacypool/legacypool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1354,35 +1354,42 @@ func TestAllowedTxSize(t *testing.T) {
defer pool.Close()

account := crypto.PubkeyToAddress(key.PublicKey)
testAddBalance(pool, account, big.NewInt(10000000000000000))

// Compute maximal data size for transactions (lower bound).
//
// It is assumed the fields in the transaction (except of the data) are:
// - nonce <= 32 bytes
// - gasTip <= 32 bytes
// - gasLimit <= 32 bytes
// - recipient == 20 bytes
// - value <= 32 bytes
// - signature == 65 bytes
// All those fields are summed up to at most 213 bytes.
baseSize := uint64(213)
dataSize := txMaxSize - baseSize
minGasPrice := common.GetMinGasPrice(pool.currentHead.Load().Number)
fundedBalance := new(big.Int).Mul(minGasPrice, new(big.Int).SetUint64(pool.currentHead.Load().GasLimit))
fundedBalance.Mul(fundedBalance, big.NewInt(3))
testAddBalance(pool, account, fundedBalance)

// Find the maximum data length for the kind of transaction which will
// be generated in the pool.addRemoteSync calls below.
const largeDataLength = txMaxSize - 200 // enough to have a 5 bytes RLP encoding of the data length number
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline comment about “enough to have a 5 bytes RLP encoding of the data length number” looks inaccurate for the sizes involved here (txMaxSize is 64KB, so the RLP length-of-length for the data field is much smaller). Consider rewording this to avoid implying a specific RLP encoding size, or explain the actual threshold being targeted by the 200-byte margin.

Suggested change
const largeDataLength = txMaxSize - 200 // enough to have a 5 bytes RLP encoding of the data length number
const largeDataLength = txMaxSize - 200 // leave enough room for RLP overhead and other transaction fields

Copilot uses AI. Check for mistakes.
txWithLargeData := pricedDataTransaction(0, pool.currentHead.Load().GasLimit, minGasPrice, key, largeDataLength)
maxTxLengthWithoutData := txWithLargeData.Size() - largeDataLength
maxTxDataLength := txMaxSize - maxTxLengthWithoutData
for tx := pricedDataTransaction(0, pool.currentHead.Load().GasLimit, minGasPrice, key, maxTxDataLength); tx.Size() > txMaxSize; {
maxTxDataLength--
tx = pricedDataTransaction(0, pool.currentHead.Load().GasLimit, minGasPrice, key, maxTxDataLength)
}
minOversizedDataLength := maxTxDataLength + 1
for tx := pricedDataTransaction(0, pool.currentHead.Load().GasLimit, minGasPrice, key, minOversizedDataLength); tx.Size() <= txMaxSize; {
minOversizedDataLength++
tx = pricedDataTransaction(0, pool.currentHead.Load().GasLimit, minGasPrice, key, minOversizedDataLength)
}

// Try adding a transaction with maximal allowed size
tx := pricedDataTransaction(0, pool.currentHead.Load().GasLimit, big.NewInt(300000000), key, dataSize)
tx := pricedDataTransaction(0, pool.currentHead.Load().GasLimit, minGasPrice, key, maxTxDataLength)
if err := pool.addRemoteSync(tx); err != nil {
t.Fatalf("failed to add transaction of size %d, close to maximal: %v", int(tx.Size()), err)
}
// Try adding a transaction with random allowed size
if err := pool.addRemoteSync(pricedDataTransaction(1, pool.currentHead.Load().GasLimit, big.NewInt(300000000), key, uint64(rand.Intn(int(dataSize))))); err != nil {
if err := pool.addRemoteSync(pricedDataTransaction(1, pool.currentHead.Load().GasLimit, minGasPrice, key, uint64(rand.Intn(int(maxTxDataLength+1))))); err != nil {
t.Fatalf("failed to add transaction of random allowed size: %v", err)
Comment on lines +1365 to 1385
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maxTxDataLength/minOversizedDataLength are derived using pricedDataTransaction calls that generate fresh random payloads (and thus different signatures) on each iteration. Later you create new transactions again using those lengths (e.g. at the “Try adding…” calls), which can change the RLP-encoded size by a few bytes and reintroduce flakiness around the txMaxSize boundary. To make this deterministic, either (a) build deterministic tx data for this test (fixed byte pattern), or (b) keep/reuse the exact tx instances found by the loops, and for the “random allowed size” case ensure the generated tx is checked/adjusted to be <= txMaxSize before calling addRemoteSync.

Copilot uses AI. Check for mistakes.
}
// Try adding a transaction of minimal not allowed size
if err := pool.addRemoteSync(pricedDataTransaction(2, pool.currentHead.Load().GasLimit, big.NewInt(300000000), key, txMaxSize)); err == nil {
// Try adding a transaction with the smallest data length that is already oversized.
if err := pool.addRemoteSync(pricedDataTransaction(2, pool.currentHead.Load().GasLimit, minGasPrice, key, minOversizedDataLength)); err == nil {
t.Fatalf("expected rejection on slightly oversize transaction")
}
// Try adding a transaction of random not allowed size
if err := pool.addRemoteSync(pricedDataTransaction(2, pool.currentHead.Load().GasLimit, big.NewInt(300000000), key, dataSize+1+uint64(rand.Intn(int(10*txMaxSize))))); err == nil {
// Try adding a transaction above maximum size by more than one
if err := pool.addRemoteSync(pricedDataTransaction(2, pool.currentHead.Load().GasLimit, minGasPrice, key, minOversizedDataLength+uint64(rand.Intn(10*txMaxSize)))); err == nil {
t.Fatalf("expected rejection on oversize transaction")
}
// Run some sanity checks on the pool internals
Expand Down
Loading