Skip to content

fix(core/txpool/legacypool): fix flaky TestAllowedTxSize #31142

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

Closed
96 changes: 83 additions & 13 deletions core/txpool/legacypool/legacypool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ import (
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/event"
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/rlp"
"github.com/ethereum/go-ethereum/trie"
"github.com/holiman/uint256"
"github.com/stretchr/testify/require"
)

var (
Expand Down Expand Up @@ -1195,35 +1197,36 @@ func TestPendingGlobalLimiting(t *testing.T) {
func TestAllowedTxSize(t *testing.T) {
t.Parallel()

// Create a test account and fund it
pool, key := setupPool()
pool, _ := setupPool()
defer pool.Close()

// We use a deterministic ecdsa key to obtain signed transactions of pre-determined sizes.
key, err := crypto.HexToECDSA("e521e757356c1f197f704502dfa10333993e3705ca3d304d6a349461ed67ae71")
require.NoError(t, err)

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

// 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
txWithLargeData := pricedDataTransaction(0, pool.currentHead.Load().GasLimit, big.NewInt(1), key, largeDataLength)
maxTxLengthWithoutData := txWithLargeData.Size() - largeDataLength // 103 bytes
maxTxDataLength := txMaxSize - maxTxLengthWithoutData // 131072 - 103 = 130953 bytes
gasLimit := pool.currentHead.Load().GasLimit

// Try adding a transaction with maximal allowed size
tx := pricedDataTransaction(0, pool.currentHead.Load().GasLimit, big.NewInt(1), key, maxTxDataLength)
tx := sizedTx(t, txMaxSize, 0, gasLimit, key)
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(1), key, uint64(rand.Intn(int(maxTxDataLength+1))))); err != nil {
tx = sizedTx(t, txMaxSize/2, 1, gasLimit, key)
if err := pool.addRemoteSync(tx); err != nil {
t.Fatalf("failed to add transaction of random allowed size: %v", err)
}
// Try adding a transaction above maximum size by one
if err := pool.addRemoteSync(pricedDataTransaction(2, pool.currentHead.Load().GasLimit, big.NewInt(1), key, maxTxDataLength+1)); err == nil {
tx = sizedTx(t, txMaxSize+1, 2, gasLimit, key)
if err := pool.addRemoteSync(tx); err == nil {
t.Fatalf("expected rejection on slightly oversize transaction")
}
// Try adding a transaction above maximum size by more than one
if err := pool.addRemoteSync(pricedDataTransaction(2, pool.currentHead.Load().GasLimit, big.NewInt(1), key, maxTxDataLength+1+uint64(rand.Intn(10*txMaxSize)))); err == nil {
// Try adding a transaction above maximum size by two
tx = sizedTx(t, txMaxSize+2, 2, gasLimit, key)
if err := pool.addRemoteSync(tx); err == nil {
t.Fatalf("expected rejection on oversize transaction")
}
// Run some sanity checks on the pool internals
Expand All @@ -1239,6 +1242,73 @@ func TestAllowedTxSize(t *testing.T) {
}
}

// sizedTx generates a transaction with the size matching the `targetSize` given
// as argument. It uses the nonce, gasLimit and key given as arguments to generate the transaction.
// Note around 1% of all possible sizes under [txMaxSize] cannot be generated by this function.
func sizedTx(t *testing.T, targetSize, nonce, gasLimit uint64, key *ecdsa.PrivateKey) (
tx *types.Transaction) {
t.Helper()
gasPrice := big.NewInt(1)

// 1 byte *big.Int + 32 byte *bit.Int + 32 byte *big.Int
// with an RLP encoding ranging from 1 + 1 + 1 to 2 + 33 + 33 bytes
const targetSignatureLength uint64 = 65 // most of the time it's 1 + 32 + 32 bytes

// Enforce the target size is above the minimum signed transaction size
txNoData := types.NewTransaction(nonce, common.Address{}, big.NewInt(0), gasLimit, gasPrice, nil)
minimumSize := txNoData.Size() + targetSignatureLength
require.GreaterOrEqual(t, targetSize, minimumSize, "target size is less than minimum size")

// Find data length to reach the target size, assuming a signature of length 65.
// This is done because the data length header varies from 0 to 5 bytes.
dataLength := targetSize - minimumSize
data := make([]byte, dataLength)
for dataLength > 0 {
txWithData := types.NewTransaction(nonce, common.Address{}, big.NewInt(0), gasLimit, gasPrice, data)
signedTx, err := types.SignTx(txWithData, types.HomesteadSigner{}, key)
switch {
case err != nil:
require.NoError(t, err, "signing transaction")
case txSignatureLen(signedTx) != targetSignatureLength:
// try again with other data to get a signature of the desired length
for i := range data {
data[i]++
}
continue
case signedTx.Size() == targetSize:
return signedTx
case signedTx.Size() < targetSize:
t.Fatalf("target size %d cannot be reached (%d smaller for data length %d)", targetSize, signedTx.Size(), dataLength)
default: // larger than targetSize
dataLength--
data = data[:dataLength]
}
}
t.Fatalf("target size %d cannot be reached", targetSize)
return nil
}

func txSignatureLen(tx *types.Transaction) uint64 {
v, r, s := tx.RawSignatureValues()
empty := &types.LegacyTx{}
signatureOnly := &types.LegacyTx{
V: v, R: r, S: s,
}
return rlpSize(signatureOnly) - rlpSize(empty)
}

func rlpSize(x any) uint64 {
return uint64(len(mustRLP(x)))
}

func mustRLP(x any) []byte {
b, err := rlp.EncodeToBytes(x)
if err != nil {
panic(err)
}
return b
}

// Tests that if transactions start being capped, transactions are also removed from 'all'
func TestCapClearsFromAll(t *testing.T) {
t.Parallel()
Expand Down