Skip to content

fix(net): validate peer transactions before mempool insertion#87

Open
ace-degen wants to merge 1 commit into
LayerTwo-Labs:masterfrom
ace-degen:fix/p2p-validate-transactions
Open

fix(net): validate peer transactions before mempool insertion#87
ace-degen wants to merge 1 commit into
LayerTwo-Labs:masterfrom
ace-degen:fix/p2p-validate-transactions

Conversation

@ace-degen

Copy link
Copy Markdown

Summary

The P2P NewTransaction handler in lib/node/net_task.rs inserted peer-supplied
transactions into the mempool via MemPool::put without first running
State::validate_transaction. The RPC submission path (Node::submit_transaction)
does validate. Because of this asymmetry, a transaction received from a peer with
an invalid signature (or one that fails balance/fee checks) is accepted into the
mempool and re-broadcast to all peers, even though it can never be included in a
valid block.

Any peer can exploit this to flood the network's mempools with invalid transactions:

  • network-wide mempool poisoning / wasted bandwidth (each node accepts and relays),
  • non-mining nodes never run the template builder, so the junk stays resident in
    their mempools indefinitely (unbounded growth from free, keyless traffic).

(The block-template path is self-healing: Node::get_transactions re-validates and
drops invalid txs before building a template, so they never reach a block — the
impact is relay amplification and mempool bloat, not poisoned templates.)

(Severity: network-level DoS — no direct theft, since invalid transactions are still
rejected at block-connect time.)

Root cause

The handler called only regenerate_proof (which generates a Utreexo proof, not a
validation step) followed by mempool.put (which only guards against double-spends
within the mempool). Neither verifies signatures, address binding, input existence,
or fees.

Fix

Run State::validate_transaction in the NewTransaction handler before
MemPool::put, mirroring the RPC path.

Test

Adds lib/mempool.rs::p2p_validation_bypass_tests, a runtime regression test that
builds a transaction spending a funded UTXO but signs it with the wrong key, then
asserts:

  1. State::validate_transaction rejects it with an Authorization error, but
  2. MemPool::put on its own accepts it and the invalid transaction is left resident
    in the mempool.
running 1 test
[validate_transaction(forged tx) => Err(Authorization)]
test mempool::p2p_validation_bypass_tests::p2p_path_accepts_transaction_that_validation_rejects ... ok
test result: ok. 1 passed; 0 failed

The P2P `NewTransaction` handler inserted peer-supplied transactions into
the mempool via `MemPool::put` without first running
`State::validate_transaction`, unlike the RPC path
(`Node::submit_transaction`). A transaction with an invalid signature (or
failing balance/fee checks) was therefore accepted into the mempool and
re-broadcast to all peers, even though it can never be included in a valid
block. Any peer can use this to flood the network's mempools with invalid
transactions (network-wide DoS / wasted bandwidth; miners build templates
around transactions that will be rejected).

Run `validate_transaction` in the handler before `mempool.put`, matching the
RPC path. Add a regression test pinning the invariant: a forged-signature
transaction is rejected by `validate_transaction` while `MemPool::put` alone
accepts it.
@ace-degen ace-degen force-pushed the fix/p2p-validate-transactions branch from d05b5aa to 0c755b6 Compare June 10, 2026 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant