Skip to content

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

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

fix(net): validate peer transactions before mempool insertion#33
ace-degen wants to merge 2 commits 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 opened a write txn and called mempool.put directly. MemPool::put
only guards against double-spends within the mempool; it does not verify
signatures, address binding, input existence, or fees.

Fix

Run State::validate_transaction in the NewTransaction handler before
MemPool::put, mirroring the RPC path. One line; the ? converts the state::Error
via the existing net_task::Error::State(#[from] Box<state::Error>) impl.

Test

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

  1. State::validate_transaction rejects it with an AuthorizationError, but
  2. the bare MemPool::put (what the P2P handler did) accepts it and leaves the
    invalid transaction resident in the mempool.

(The test lives in lib/state/mod.rs rather than lib/mempool.rs only because the
state UTXO set used to fund the input is private to the state module.)

validate_transaction(forged tx) => Err(AuthorizationError)
test state::p2p_validation_bypass_tests::p2p_path_accepts_transaction_that_validation_rejects ... ok
test result: ok. 1 passed; 0 failed

Note

This is one shared bug pattern across the LayerTwo-Labs Rust L2 stack (thunder-rust,
coinshift-rs, photon, plain-bitassets, plain-bitnames, truthcoin-dc); each repo
carries the same gap in its own net_task.rs and gets the same fix.

The P2P `NewTransaction` handler inserts peer transactions into the mempool
via `MemPool::put` without first calling `State::validate_transaction`, unlike
the RPC submit path. This regression test pins the invariant: a forged-signature
transaction spending a real UTXO is rejected by `validate_transaction` yet
accepted by `MemPool::put` on its own.
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 relay amplification and mempool bloat (worst on
non-mining nodes, which never run the self-healing template builder). Not theft
— invalid transactions are still rejected at block-connect.

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.
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