p2p: prevent BIP35 requesters from bypassing inbound tx-relay capacity #35874

l0rinc commented at 11:55 PM on August 3, 2026: contributor

Problem: When a node offers NODE_BLOOM, an inbound peer can advertise fRelay=false and request BIP35 mempool inventory while remaining outside the inbound transaction-relay capacity introduced in #28463.

Fix: Ignore mempool requests while the peer has transaction relay disabled, unless the connection has the mempool permission. A BIP37 filterload message enables transaction relay and applies the existing capacity limit, preserving bitcoinj's SPV filterload-then-mempool sequence. Targeted getdata requests remain unchanged.

DrahtBot added the label P2P on Aug 3, 2026

DrahtBot commented at 11:55 PM on August 3, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35874.

Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

l0rinc marked this as ready for review on Aug 4, 2026

l0rinc force-pushed on Aug 4, 2026

l0rinc renamed this:
~~p2p: account BIP35 requests against relay capacity~~
p2p: enforce relay limit for BIP35 requesters
on Aug 4, 2026

l0rinc force-pushed on Aug 4, 2026

DrahtBot added the label CI failed on Aug 4, 2026

DrahtBot commented at 5:25 AM on August 4, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task riscv32 bare metal, static libbitcoin_consensus: https://github.com/bitcoin/bitcoin/actions/runs/30880088557/job/91899376958</sub> <sub>LLM reason (✨ experimental): CI failed because cloning the binutils-gdb submodule (sourceware.org/git/binutils-gdb.git) errored with “fatal: the remote end hung up unexpectedly” (exit code 2).</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot closed this on Aug 4, 2026

DrahtBot reopened this on Aug 4, 2026

DrahtBot removed the label CI failed on Aug 4, 2026

in src/net_processing.cpp:5180 in dd1af38900 outdated

5174 | @@ -5191,6 +5175,11 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
5175 |          }
5176 |  
5177 |          if (auto tx_relay = peer.GetTxRelay(); tx_relay != nullptr) {
5178 | +            if (pfrom.IsInboundConn() && !pfrom.m_relays_txs) {
5179 | +                // BIP35 consumes tx-relay capacity while fRelay=false still disables ongoing announcements.
5180 | +                pfrom.m_relays_txs = true;

darosior commented at 5:25 PM on August 4, 2026:

I think that's a protocol change? Prior to this we would not flood a peer that set BIP 37 fRelay to false with INV's until they set a filter. After this change, we would flood them with non-filtered INV's provided they previously sent a MEMPOOL message.

l0rinc commented at 8:15 PM on August 4, 2026:

Valid question. If I understood your objection correctly, it applies to TxRelay::m_relay_txs, while we're changing CNode::m_relays_txs. To make sure, I added a test that requests a BIP35 snapshot and verifies that a transaction added afterward is announced to a regular relay peer but not to the requester. It passes before and after the production change. Could you please check whether it satisfies your criterion? Thanks for the hint.

l0rinc force-pushed on Aug 4, 2026

l0rinc force-pushed on Aug 5, 2026

l0rinc renamed this:
~~p2p: enforce relay limit for BIP35 requesters~~
p2p: prevent BIP35 requesters from bypassing inbound tx-relay capacity
on Aug 5, 2026

DrahtBot added the label CI failed on Aug 5, 2026

l0rinc closed this on Aug 5, 2026

l0rinc reopened this on Aug 5, 2026

DrahtBot removed the label CI failed on Aug 5, 2026

gmaxwell commented at 11:31 PM on August 12, 2026: contributor

Should fRelay=false be able to request inventory at all? Do any existing pieces of software do this (outside of surveillance software, of course)?

I think it probably shouldn't be allowed. Historically fRelay=false peers have been treated as a reduced bandwidth and surveillance privacy risk, but I think that's mooted if they're doing mempool requests.

l0rinc commented at 3:25 AM on August 13, 2026: contributor

Thanks @gmaxwell.

Do any existing pieces of software do this?

I'm not sure about the details, my goal was to close a potential DoS angle. A quick search (and some AI tutoring) turned up bitcoinj, it seems that in SPV mode with Bloom filtering, bitcoinj sets fRelay=false, then sends filterload before mempool. I'm not sure whether this is the case you had in mind.

This suggests ignoring non-permissioned mempool requests until the peer enables transaction relay, for example by sending filterload. @msgilligan, is that accurate? Does bitcoinj ever send mempool without first loading a filter?

schildbach commented at 9:38 AM on August 13, 2026: contributor

Should fRelay=false be able to request inventory at all?

Historically, bitcoinj has walked the tx dependency chain for its risk analysis. It uses getdata messages for this.

Does bitcoinj ever send mempool without first loading a filter?

In the SPV case (which I assume is what you care about in this context), bitcoinj sets the filter first via filterload, then immediately follows up with mempool.

(Afaicr, we never used the ability to set a new filter for a connection, or add data to one. Rather, we drop the connection and build a new one.)

test: characterize BIP35 before tx relay

An inbound peer with `fRelay=false` currently receives BIP35 inventory before enabling transaction relay.
The adjacent BIP37 test covers the bitcoinj SPV sequence by sending `filterload` before `mempool`.

f4bb34381a

p2p: require tx relay for BIP35 requests

Ignore BIP35 `mempool` requests while transaction relay is disabled, unless the connection has the `mempool` permission.

A BIP37 `filterload` message enables relay and applies the existing inbound capacity limit before bitcoinj's SPV flow sends `mempool`.
Targeted `getdata` requests remain unchanged.

fc657d872f

l0rinc force-pushed on Aug 15, 2026

l0rinc commented at 4:34 AM on August 15, 2026: contributor

Rebased and reworked this based on the review feedback. The PR now ignores non-permissioned mempool requests while tx relay is disabled, preserving existing BIP37 and targeted getdata behavior. Thanks @gmaxwell and @schildbach for helping clarify the intended behavior and guiding this toward a simpler solution.