#35354 net: wait for validation queue flush for missing compact filter for an already known block
https://github.com/bitcoin/bitcoin/pull/35354 · · +563/-13 in 3 files, 3 commits · labels: P2P
Goal
- Prevent full nodes from dropping compact filter queries when block indexing lags behind new blocks
- Stops BIP 157 light clients from disconnecting or banning nodes when requests arrive before filters index
Fixes a race condition where BIP 157 compact filter requests (`getcfilters`, `getcfheaders`, `getcfcheckpt`) arrive for a newly connected block before the validation-interface callback finishes constructing the corresponding filter. Rather than ignoring the request and provoking peer disconnects, `net_processing` detects that the index is racing the chain tip and parks the request, pausing message processing for that peer until the filter is written or the request resolves.
Problem: When a new block is connected, Bitcoin Core announces it before the background validation queue builds its compact filter. BIP 157 light clients requesting the filter during this race window receive no response, treating the silence as protocol misbehavior and disconnecting or banning the Core node (#29655).
Category: P2P (#12 of 65)
P3 · bug fix
- P2 because it resolves an interoperability bug causing light clients to disconnect or ban full nodes
- Avoids dropped filter queries for announced blocks to maintain reliable peer connectivity
Fixes an intermittent protocol-level race bug reported in #29655 and #27085 where BIP 157 clients disconnect or ban Bitcoin Core nodes because newly announced block filters have not yet landed in the index. ajtowns noted the race is slow enough to occur intermittently in practice and worth handling. It also exercises the paused-peer message processing pattern that could benefit future decoupled block validation work.
Membership: Modifies net_processing.cpp to defer and serve compact filter messages across the P2P connection.
Factors: security/stability 1, bug 2, performance 0, user value 2, leverage 1
Reviewability: Ready
- Worth reviewing now with recent architectural feedback incorporated and passing CI
The author has incorporated all reviewer recommendations (handling all three message types, pausing message processing on the peer, and keeping index code untouched) and CI is green.
Author status: active, implemented the suggested peer-pausing approach, benchmarked cs_main acquisition overhead, and force-pushed on 2026-08-27
Resolved concerns:
- sedited objected to handling the race and syncing validation queues inside `BaseIndex` rather than in `net_processing`.
- mzumsande objected to modifying index code in `src/index/base.h`, requesting that `net_processing` rely only on existing `GetSummary()` data.
- ajtowns noted that `getcfheaders` was responsible for more real-world disconnects than `getcfilters`, suggesting that all three BIP 157 message types be deferred, and recommended pausing the peer entirely to preserve message ordering.
Agreement: Strong
- Reviewers agree the issue needs fixing and favor handling the race in net processing
- Objected to resolving the race in index code or stalling the message thread (sedited, mzumsande)
- Proposed parking peer message processing to preserve order without blocking others (ajtowns)
Strong: ajtowns supported the fix and provided the peer-pausing prototype; earlier objections from sedited and mzumsande were fully resolved.
All reviewers agree the bug should be resolved in net_processing. Objections regarding index encapsulation and message coverage have been implemented, and ajtowns advocated for the current peer-pausing approach.
- ajtowns: 'I think those calcs are slow enough that this is likely to occur occasionally... so is worth handling'
- ajtowns provided prototype branch: 'The idea is that you add a single "hey, I'm working on an answer to this request" queued item for the peer, and refrain from processing any further messages from that peer'
- randomlogin adopted the suggested pattern and benchmarks show negligible overhead
Objections:
| Reviewer | Kind | Harm | Status | Blocking | Author replied | Quote |
|---|---|---|---|---|---|---|
| sedited | approach | Handling peer race conditions inside BaseIndex couples indexing code to P2P transport concerns and risks stalls in other callers. | resolved | no | yes | 2026-06-21: 'Reading through this, I don't think this should be handled on the index level.' Settled: 2026-08-05: author moved logic to net_processing and rewrote the implementation |
| mzumsande | approach | Touching index internals violates separation of concerns when GetSummary already exposes sufficient state. | resolved | no | yes | 2026-08-11: 'Races in net_processing shouldn't be the business of indexes, and GetSummary() should have enough info... so I'd prefer if the index code was not touched at all' Settled: 2026-08-14: author removed index changes and used GetSummary() |
| ajtowns | scope | Handling only getcfilters leaves getcfheaders and getcfcheckpt unhandled, which are responsible for most disconnects in practice. | resolved | no | yes | 2026-08-16: 'Based on the logs in #29655 this seems backwards -- there were more disconnects for getcfheaders than getcfilters? So I think this ought to be handled for all the cases?' Settled: 2026-08-26: author updated the PR to handle all three compact filter message types via std::variant |
Support:
- ajtowns: Compact filter calculations are slow enough that races occur for light clients reacting to headers announcements, making the fix worthwhile, and the peer-pausing design keeps message processing in order.
- mzumsande: agrees that handling racing compact filter requests in net_processing makes sense to prevent disconnects
Participants: sedited (objection), mzumsande (objection), ajtowns (objection)
State derived from the lists: substantive support, no open objection (ajtowns, mzumsande)
Review verdicts (DrahtBot): 0
Files
387 lines under test/bench/ci.
- src/test/cfilter_race_tests.cpp +386/-0
- src/net_processing.cpp +176/-13
- src/test/CMakeLists.txt +1/-0
Card
This PR fixes a race condition where BIP 157 light clients request compact filters or headers for a newly announced block before the validation interface finishes writing the filter to disk. Currently, Bitcoin Core drops the request without responding, causing peers like Neutrino or Kyoto to disconnect or ban the node. The patch addresses this in net_processing by pausing message processing for the requesting peer until the filter index catches up or the request can be answered. The PR is in a clean state and ready for review after adopting an approach prototyped by ajtowns.