txnprovider/txpool: release sender ids for remote txns that reach no sub-pool

[AskAlexSharov]

processRemoteTxns and AddLocalTxns both register senders for the whole batch before validating it, so every distinct recovered address gets an entry in senderIDs and senderID2Addr. The only eviction is in flushLocked, walking p.deletedTxns, which is appended solely by discardLocked — reachable only for a txn that was admitted to a sub-pool. A txn rejected by validateTxns (insufficient funds, underpriced, nonce too low, spammer) never becomes a metaTxn, so its two entries are never removed. One peer streaming correctly-signed, zero-balance transactions from fresh offline keys, or an exposed eth_sendRawTransaction, grows both maps until restart.

Not the largest instance of this leak: sendersBatch.onNewBlock allocates an id for every account changed in every block and those are never evicted either, which on mainnet dwarfs the batch paths. That one needs a different mechanism (the ids stay live for onSenderStateChange) and is left for a separate change.

Both new tests are red on main.

[Copilot]

🟢 Approval recommended

The cleanup is covered by regression tests, with no unresolved issues.

Pull request overview

Prevents sender-ID map leaks when rejected transactions never enter a txpool sub-pool.

Changes:

• Cleans up unused sender mappings after remote and local transaction processing.
• Adds regression tests for both rejection paths.

File summaries

File	Description
txnprovider/txpool/pool.go	Removes sender IDs not referenced by pooled transactions.
txnprovider/txpool/pool_sender_leak_test.go	Tests that rejected transactions do not leak sender mappings.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[awskii]

LGTM — approving. 1 finding, all minor; nothing blocking.

---

Pre-existing, not introduced here:

• fromDB is the third register-then-reject site and is not swept — txnprovider/txpool/pool.go:2721

  The PR fixes the register-senders-before-validating pattern on the two batch paths (processRemoteTxns at pool.go:531 and AddLocalTxns at pool.go:1473), but fromDB has the identical shape and is untouched by this change:

txn.SenderID, txn.Traced = p.senders.getOrCreateID(addr, p.logger)   // 2721
...
reason, err := p.validateTx(txn, isLocalTx, cacheView)
if reason != txpoolcfg.NotSet && reason != txpoolcfg.Success {
    continue                                                        // 2729
}

Concrete case: a row in kv.PoolTransaction whose sender has since spent its balance. On startup, getOrCreateID allocates an id for that address, validateTx returns InsufficientFunds, and the loop continues. The txn never becomes a metaTxn, so it never enters p.deletedTxns and flushLocked's eviction never sees it — exactly the leak the PR body describes. The id stays in senderIDs/senderID2Addr for the process lifetime.

Materially smaller than the paths fixed: it is one-shot at startup and bounded by the number of distinct senders in the persisted pool (thousands, not unbounded), and the same row set recurs on every restart rather than accumulating. Reported because it is the same defect the change is closing and the sweep helper already exists to close it.

Everything else the source checked held. forgetUnusedSenders runs inside p.lock at both call sites (defer LIFO puts it ahead of p.lock.Unlock), senderIDsOf snapshots eagerly so the Resize(0) at pool.go:575 cannot race it, and p.all.hasTxns is a sound liveness predicate: every one of the ten discardLocked call sites is paired with a sub-pool removal, so p.all is a superset of p.pending/p.baseFee/p.queued and of p.byHash. On the processRemoteTxns error path the batch survives with stale SenderID fields, but registerNewSenders unconditionally reassigns them on the retry, so no id dangles into sendersBatch.info's panic("must not happen").