txpool: malformed sender signature on any tx drops sibling txs in the same Transactions/PooledTransactions packet

[yperbasis]

Summary

Same shape of bug as #20809 / ethereum-bounty/erigon#7, but on the sender signature path rather than the EIP-7702 auth-tuple path — and it applies to any transaction type, not just SetCode.

A Transactions (0x02) or PooledTransactions (0x0a) devp2p packet containing one transaction with an unrecoverable sender signature (e.g. r=0) causes Erigon's parser to return ErrParseTxn. The batch loop in pool_txn_packets.go only swallows ErrRejected; on ErrParseTxn it aborts, dropping every sibling transaction that had already been parsed in the same packet, and fetch.go then kicks the peer.

A malicious peer can craft a cheap dummy tx with r=0, splice it into a packet alongside whatever sibling txs they want to suppress, and selectively censor those siblings while looking honest at the gossip layer.

Code

Sender recovery error path — pool_txn_parser.go:436-438:

addr, err := txn.Sender(*signer)
if err != nil {
    return 0, fmt.Errorf("%w: recovering sender from signature: %s", ErrParseTxn, err) //nolint
}

Batch loops that only continue on ErrRejected — pool_txn_packets.go:181-187 and pool_txn_packets.go:212-218:

pos, err = ctx.ParseTransaction(payload, pos, txnSlots.Txns[i], …)
if err != nil {
    if errors.Is(err, ErrRejected) {
        txnSlots.Resize(uint(i))
        i--
        continue
    }
    return 0, err
}

Geth, by contrast, defers sender recovery (typically lazy / at admission), so a malformed sender signature on one tx does not poison its packet siblings.

Suggested fix

Mirror the EIP-7702 fix in #20809 for sender-sig recovery. Two reasonable options:

1. Narrow: change the wrap from ErrParseTxn to ErrRejected and return p (the position past the consumed tx) instead of 0, so ParseTransactions / ParsePooledTransactions66 skip the bad tx and continue with siblings. The position past the tx is already known by the time Step 8 runs. Bad tx is dropped, siblings survive, peer is not kicked.
2. Proper: defer sender recovery to the txpool's validateTx / admission stage — closer to Geth's design — so parsing is a pure structural transform that never touches signatures. More invasive.

I'd start with the narrow fix and consider (2) as a separate refactor.

Repro sketch

The same approach as the test added in #20809 (TestEIP7702BatchPoisoning), but with the middle tx being a plain DynamicFeeTransaction whose R is set to zero post-signing instead of a SetCode tx with a poisoned auth tuple. With the current code ParseTransactions returns ErrParseTxn and slots.Txns ends up populated only up to the bad tx; with a fix all three should survive.

Scope

Out of scope of #20809 because the bounty issue (ethereum-bounty/erigon#7) was scoped to EIP-7702. Filing this so the broader pattern doesn't get lost.

[VBulikov]

yperbasis added Imp2 as Importance