net: wait for validation queue flush for missing compact filter for an already known block #35354

pull randomlogin wants to merge 3 commits into bitcoin:master from randomlogin:p2p-cbf-wait-for-indexed changing 3 files +563 −13
  1. randomlogin commented at 10:07 PM on May 21, 2026: none

    This PR is another approach to fix a race condition bug when a yet-unconstructed compact filter is requested for an already known block.

    Fixes: #29655, #27085 Related discussion: 1, 2 Another approach which received NACK: #35262

    The problem: Bitcoin Core receives a new block and processes it, then advertises it; however, by that moment the compact filter might not yet have been constructed. If a peer asks for a filter in that time window, Bitcoin Core simply does not respond. From the peer's perspective this is a misbehaviour (we advertised a block and don't provide filters for it), which results in disconnects or bans. While it does not critically affect core nodes, it breaks the network topology for BIP 157 clients.

    More precisely, BIP157 specifies that for getcfilters requestor:

    StopHash MUST be known to belong to a block accepted by the receiving peer. This is the case if the peer had previously sent a headers or inv message with that block or any descendents. A node that receives getcfilters with an unknown StopHash SHOULD NOT respond.
    

    Current behaviour clearly contradicts the above specification.

    Proposed solution:

    As was suggested, we can:

    1. attempt to respond to a request (getcfilters, getcfheaders, getcfcheckpt which uses LookupFilterHeader); if it fails,
    2. check that we know the requested block hash and that the height difference between the last constructed filter block and the requested block is small — from which we deduce that pending validation events would fix the miss,
    3. wait for validation-queue flush and then retry.

    Waiting for the validation queue flush parks the single msghand thread, which blocks all peers' ProcessMessages for the duration of the drain. So the trigger has to be tight: an external actor can otherwise force us into that wait.

    This PR introduces BaseIndex::WaitForRacingWrite(target, max_ahead), which only drains when:

    1. the index is synced (m_synced == true),
    2. we have a non-null target block index,
    3. the target is at most max_ahead blocks ahead of the index's last processed block (and not below it).

    BlockFilterIndex calls this from all four lookup paths (LookupFilter, LookupFilterHeader, LookupFilterRange, LookupFilterHashRange) with max_ahead = CF_MAX_BLOCKS_AHEAD_RACE_WAIT = 2, retrying the lookup once after the drain.

    Reorg handling:

    The same primitive cleanly handles the reorg cases the bug originally exposed:

    • New chain extends past the old tip (ahead > 0): the queued BlockConnected events for the new branch will, in order, run BaseIndex::BlockConnected's internal Rewind (which calls CustomRemove on the disconnected blocks — BlockFilterIndex::CustomRemove migrates each old filter from its height-keyed slot to a hash-keyed slot) and then CustomAppend for the new blocks. After the drain, both the old (hash-keyed) and new (height-keyed) filters are reachable.
    • Same-height sibling reorg (ahead == 0): the index's last processed block and the requested block sit at the same height on different branches. The first lookup misses (height-keyed slot still holds the old block's data, hash-keyed slot for the new block doesn't exist yet). The ahead == 0 branch of the helper drains the queue, which runs the disconnect/connect for the sibling reorg, after which the retry succeeds.
    • Stale-branch lookup after the reorg has settled (ahead < 0): the helper does not wait — there is no in-flight callback that would help. The lookup falls back to the hash-keyed slot independently via the existing dual-key storage.

    This PR does not affect block propagation, as the very root of this bug is the independence of block processing and filter construction in CustomAppend. Nor does it affect IBD: during IBD m_synced is false, the helper returns immediately, and we simply don't respond to compact filter messages (the writes are happening on the sync thread, not via the validation queue, so draining wouldn't help anyway). The IBD short-circuit also closes the DoS surface during the period when the node is most resource-constrained.

    Drive-by:

    LookupFilterHeader previously held m_cs_headers_cache across the DB lookup. Now that the lookup path can call SyncWithValidationInterfaceQueue (which must not be invoked under a lock that a queued callback might need), the cache lock is narrowed to wrap only the cache reads/writes, and the cache insert is switched to try_emplace since the check-then-insert is no longer atomic.

    Tests:

    A new cfilter_race_tests suite uses a PreFilterBlocker CValidationInterface registered before the filter index, so its own BlockConnected fires first on the scheduler thread and stalls it before the filter index gets to run CustomAppend. While the scheduler is parked, the test issues the lookup on a worker thread and asserts:

    1. the worker remains blocked (proves the wait path is engaged — a regression that bypasses it would let the worker return immediately with false);
    2. after releasing the scheduler, the worker returns the correct filter/header/range.

    Cases covered:

    • cfilter_available_during_append_windowLookupFilter for the new tip racing with BlockConnected.
    • cfilter_range_available_during_append_windowLookupFilterRange covering indexed history plus the racing tip.
    • cfilter_header_available_during_append_windowLookupFilterHeader on the racing tip.
    • cfilter_available_during_same_height_reorg — same-height sibling reorg path (ahead == 0).

    I've used Claude Code for this PR.

  2. DrahtBot added the label P2P on May 21, 2026
  3. DrahtBot commented at 10:08 PM on May 21, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

    LLM Linter (✨ experimental)

    Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

    • NetMsg::Make(NetMsgType::VERSION, PROTOCOL_VERSION, uint64_t{NODE_NETWORK}, int64_t{0}, uint64_t{NODE_NETWORK}, CAddress::V1_NETWORK(CService{})) in src/test/cfilter_race_tests.cpp (deferred_cfilter_response_reaches_peer)
    • NetMsg::Make(NetMsgType::VERSION, PROTOCOL_VERSION, uint64_t{NODE_NETWORK}, int64_t{0}, uint64_t{NODE_NETWORK}, CAddress::V1_NETWORK(CService{})) in src/test/cfilter_race_tests.cpp (pending_request_pauses_message_processing)
    • NetMsg::Make(NetMsgType::VERSION, PROTOCOL_VERSION, uint64_t{NODE_NETWORK}, int64_t{0}, uint64_t{NODE_NETWORK}, CAddress::V1_NETWORK(CService{})) in src/test/cfilter_race_tests.cpp (deferred_cfheaders_response_reaches_peer)

    <sup>2026-08-27 07:11:34</sup>

  4. randomlogin marked this as ready for review on May 21, 2026
  5. DrahtBot added the label CI failed on May 22, 2026
  6. DrahtBot commented at 10:47 AM on May 22, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/26255963544/job/77353717155</sub> <sub>LLM reason (✨ experimental): CI failed because ctest detected a test failure in cfilter_race_tests (1 test failed).</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>

  7. randomlogin force-pushed on May 22, 2026
  8. randomlogin commented at 10:50 AM on May 22, 2026: none

    Squashed two commits in one (previously there was a distinct commit for tests addition).

  9. DrahtBot removed the label CI failed on May 22, 2026
  10. sedited requested review from ajtowns on May 25, 2026
  11. sedited commented at 9:47 PM on June 21, 2026: contributor

    Reading through this, I don't think this should be handled on the index level. While I'm still not sold that this is not something that is just a bit flawed in the spec, I would prefer an approach where net_processing dealt with this directly in the case of an index query returning nothing by checking if the requested block lies in the two block tolerance range and if so syncing the validation interface queue.

  12. sedited requested review from rustaceanrob on Jun 21, 2026
  13. sedited commented at 9:56 AM on July 24, 2026: contributor

    @randomlogin how do you want to proceed here?

  14. randomlogin commented at 3:52 PM on July 28, 2026: none

    @sedited sorry for a long response.

    There are two ways: the first one is simply to move race-window check logic to net_processing and then again wait for SyncWithValidationInterfaceQueue.

    On the other hand, instead of blocking the caller (msghand) on SyncWithValidationInterfaceQueue completion, on a request and hitting the race-window (known block, no filters yet) we could remember the peer awaiting response, move on to other work happening and once the filters are constructed send a response.

    Something like PeerManagerImpl::ProcessPendingCFilterRequests.

    This will not block other p2p messages while we wait for filters being constructed.

    However I'm not sure that it's worth, as with the window gap being small enough we'd save only fractions of second. Additionally we could use WakeMessageHandler to avoid waiting next loop iteration (up to 100ms).

    Does that sound right to you?

  15. ajtowns commented at 9:03 AM on July 29, 2026: contributor

    On the other hand, instead of blocking the caller (msghand) on SyncWithValidationInterfaceQueue completion, on a request and hitting the race-window (known block, no filters yet) we could remember the peer awaiting response, move on to other work happening and once the filters are constructed send a response.

    That seems like a plausible approach to me, fwiw. Just set a flag in ProcessMessage when you receive the request that this peer wants a a cfilter response, then in SendMessages attempt to handle the cfilter requests, deferring it to a later call if the block is pending.

    That will mean cfilter responses for that peer may be out of order vs other messages on that connection, but I think that's okay?

  16. mzumsande commented at 10:13 AM on July 29, 2026: contributor

    The problem formulation in the OP is not precise: Currently, when we receive a block, we first build the filter and then advertise the block, so there usually is no race.The race can only happen if the client asks for the newest headers themselves unprompted. This could happen because they just newly connected, or because they do active getheaders polling for some other reason. But this explains why this is very rare.

    In my opinion, both a SyncWithValidationInterfaceQueue call in net_processing (it's not like we have anything more urgent to do than processing the block anyway?) and to schedule it for later sound fine.

  17. randomlogin commented at 1:31 PM on July 29, 2026: none

    The problem formulation in the OP is not precise: Currently, when we receive a block, we first build the filter and then advertise the block, so there usually is no race.

    OP is precise for BIP152 block transmission, when we go through BIP152, we transmit compact block header to peers before we run BlockConnected which internally calls filters construction. (To be accurate, I wrongly called it advertising, but rather it is an actual sending of the header).

    Also I took a moment to review how other requests are handled when we announced compact block header:

    • if a peer asks getheaders in the moment we have not yet connected the block, they will receive stale response
    • getdata for a block - we synchronously connect the block and then respond (which blocks all other peers for the time block connects)

    Then maybe it is better to have some kind of a unified approach for both getdata and getcfilters, for example both block and wait for block connection/filters constructions to complete?

    Deferring and responding later does not increase blocking time, and personally is more appealing and I'd go this way.

  18. mzumsande commented at 1:40 PM on July 29, 2026: contributor

    OP is precise for BIP152 block transmission

    But how does this make sense / how is this a relevant use case? To be chosen as a high-bandwidth block relay peer, you need to be a full node. In this case you don't need to download the blockfilters for new blocks from peers - this is a feature meant for light clients.

  19. randomlogin commented at 2:39 PM on July 29, 2026: none

    @mzumsande you are right, I confused BIP152 with BIP130 (light clients use BIP130). And as you pointed, this bug cannot happen with BIP130 header transmission.

  20. ajtowns commented at 6:06 AM on July 30, 2026: contributor

    In my opinion, both a SyncWithValidationInterfaceQueue call in net_processing (it's not like we have anything more urgent to do than processing the block anyway?)

    My understanding is that the way we currently work is:

    • send the compact block info to peers that have requested high bandwidth cb relay ASAP
    • pause the message handling thread to validate the block
    • queue the cfilter updates for another thread
    • send headers and inv messages to non-cb and low-bandwidth cb peers
    • resume the message handling thread
    • message handling thread "immediately" processes a cfilter request, and we attempt to respond before the cfilter updates have finished

    Apart from the hb compact block path, we don't announce blocks until we've validated them, but we do announce them before we've calculated their compact filter.

    I don't think we should block the message handling thread further, because we do have more important things to do: responding to getblocktxn requests that came in while we were validating the block (ideally we'd be responding to those requests while validating the block), and getdata requests that came in after our headers message.

    Those "immediate" requests could come from peers that selected us as hb for cb, or could just be a very quick response to a headers announcement compared to the calculations required to update the compact filters. I think those calcs are slow enough that this is likely to occur occasionally even for peers that aren't getting the high-bandwidth compact block path, so is worth handling, but it would probably good to confirm the exact flow and timings? (How long does calculating the compact filters actually take? How much does that vary across blocks? What's the real turn-around time for real compact filter clients between seeing a header and requesting the compact filter?)

  21. mzumsande commented at 12:19 PM on July 30, 2026: contributor

    Yes, that makes sense - I hadn't thought of getblocktxn requests.

    • send headers and inv messages to non-cb and low-bandwidth cb peers
    • resume the message handling thread

    I'd say it's

    • queue the headers and inv message to non-cb and low-bandwidth cb peers for the other thread (UpdatedBlockTip())
    • resume the message handling thread which sends out headers and inv messages to non-cb and low-bandwidth cb peers once the queue entry has been processed
  22. randomlogin force-pushed on Aug 5, 2026
  23. randomlogin commented at 1:30 AM on August 5, 2026: none

    Changes

    Added deferring of a response, we store the racing request in a PendingCFilterRequest struct in a Peer and then check it in SendMessages.

    To avoid code repetition, added a helper TryRespondToCFilterRequest which is used in both original ProcessGetCFilters and SendMessages handlings of a request.

    Rewrote the test.

    If we receive two requests while the first one has not yet been responded, we do not overwrite it, thus responding only to the first one.

    Benchmarks

    Added benchmarks commit (b1d4868485770777e46950460852d61f9ad4b6c2) to this PR (I'll squash it later).

    Here is the data for the initial filter construction since genesis as well as for processing during normal operation, i.e. I have an already running node in which I turned on filters and measured the times. Initial blocks' measurements are shown over latest 100k blocks, since ~861k).

    Metric mean median p90 p99 max
    Cfilters construction (from 861k initial) 4.50ms 4.50ms 6.74ms 8.04ms 112.86ms
    Cfilters construction (212 live blocks) 5.92ms 6.10ms 8.35ms 11.33ms 13.96ms
    Connect block (212 live blocks) 172.85ms 51.14ms 63.89ms 4408.39ms 10423.97ms

    Maximum time (112 ms for filters construction) is perhaps some I/O problem or something else not connected to normal behaviour. All times include I/O.

    We can say filters processing takes around 10% of block processing.

    Computer that runs benchmarks has the following specs:

    • CPU: Intel Xeon E5-2686 v4 (Broadwell) — 2 vCPUs, 2.30GHz, 1 thread/core
    • RAM: 3.8GB

    Going further

    1. Currently only added handling getcfilters, but the same bug stands for other bip157 requests: getcfheaders and getcfcheckpt. It is even less likely for them to encounter a race, do we want them handled in the same way?

    2. Should we call m_connman.WakeMessageHandler() in block connection, or the current approach is fine?

  24. randomlogin force-pushed on Aug 5, 2026
  25. randomlogin force-pushed on Aug 5, 2026
  26. DrahtBot added the label CI failed on Aug 5, 2026
  27. DrahtBot removed the label CI failed on Aug 6, 2026
  28. randomlogin requested review from mzumsande on Aug 10, 2026
  29. in src/index/blockfilterindex.cpp:268 in b1d4868485
     263 | +
     264 | +    ++m_num_blocks_total;
     265 | +    m_time_construct += time_filter - time_start;
     266 | +    m_time_header += time_header - time_filter;
     267 | +    m_time_write += time_write - time_header;
     268 | +    m_time_total += time_write - time_start;
    


    ajtowns commented at 2:09 AM on August 11, 2026:

    The benchmarking commit should be removed, not squashed, presumably? It doesn't seem like it'll be useful beyond developing this PR?

    I think this PR should be marked as draft if it has commits that need updating/replacing prior to merge.


    randomlogin commented at 3:47 PM on August 14, 2026:

    Originally I thought it might be useful on its own. Removed benchmark commit.

  30. in src/net_processing.cpp:266 in 21c65a024c
     261 | +        BlockFilterType filter_type;
     262 | +        uint32_t start_height;
     263 | +        uint256 stop_hash;
     264 | +    };
     265 | +    /** At most one outstanding deferred cfilter request at a time. */
     266 | +    std::optional<PendingCFilterRequest> m_pending_cfilter_request GUARDED_BY(NetEventsInterface::g_msgproc_mutex);
    


    ajtowns commented at 2:11 AM on August 11, 2026:

    Rather than an optional, I'd suggest considering std::list<PendingCFilterREquest> and using push_back to add requests to the end, and front to get the oldest request in order to attempt to service it. Allows handling multiple requests, though still needs a limit to avoid growing unbounded. Also wastes a little less memory than an optional if there's nothing queued, which should be true for most peers for most of the time.


    randomlogin commented at 3:48 PM on August 14, 2026:

    Changed an optional in favor of a list with up to MAX_PENDING_CFILTER_REQUESTS=2 entries.

  31. in src/index/base.h:170 in 21c65a024c
     165 | +    ///
     166 | +    /// This is a cheap, non-blocking check only -- it does not wait for the
     167 | +    /// validation-interface queue to drain. Callers that get `true` should
     168 | +    /// treat the miss as pending and retry later, rather than treating it as a
     169 | +    /// hard failure.
     170 | +    bool IsRacing(const CBlockIndex* target, int max_ahead) const;
    


    mzumsande commented at 4:18 PM on August 11, 2026:

    Races in net_processing shouldn't be the business of indexes, and GetSummary() should have enough info (or we could add more if not), so I'd prefer if the index code was not touched at all for this and the net_processing code would decide whether there is a possible race.


    randomlogin commented at 3:48 PM on August 14, 2026:

    Removed IsRacing, now using GetSummary. No changes in GetSummary were required.

  32. randomlogin force-pushed on Aug 14, 2026
  33. randomlogin force-pushed on Aug 14, 2026
  34. DrahtBot added the label Needs rebase on Aug 14, 2026
  35. randomlogin force-pushed on Aug 14, 2026
  36. DrahtBot removed the label Needs rebase on Aug 15, 2026
  37. ajtowns commented at 5:31 AM on August 16, 2026: contributor

    Sorry for repeatedly requesting changes.

    Currently only added handling getcfilters, but the same bug stands for other bip157 requests: getcfheaders and getcfcheckpt. It is even less likely for them to encounter a race, do we want them handled in the same way?

    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? Not at all clear to me why it's not, so maybe I've missed something?

    I wonder if it wouldn't be better to adopt the "pause this peer" approach, rather than queueing messages but continuing. Here's a somewhat vibecoded branch that demos that (and delays all the messages when needed): https://github.com/ajtowns/bitcoin/commits/202608-cfilter-park/

    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 (without interfering with outgoing messages like block announcements or other peers in general). Not sure if checking the index for updates every msgproc loop is okay or there should be an additional pause there.

  38. DrahtBot added the label CI failed on Aug 17, 2026
  39. DrahtBot removed the label CI failed on Aug 17, 2026
  40. randomlogin commented at 5:27 PM on August 18, 2026: none

    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? Not at all clear to me why it's not, so maybe I've missed something?

    I didn't implement it as wanted to receive feedback for the approach itself first.

    I also checked my logs when I encountered this bug with https://github.com/2140-dev/kyoto, and indeed I also encountered it with getcfheaders.

    Yes, all three cbf messages should be handled the same way.

    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 (without interfering with outgoing messages like block announcements or other peers in general).

    It seems this approach is conceptually simpler, but as we defer all other messages from a peer including ping or getdata my concerns are:

    1. We potentially slow down block propagation, as we don't respond to a "legitimate" getdata during the race window. (Though it's quite questionable why bip157 peer would ask for a full block without having filters/cfheaders for it first, but it's a presupposition what a bip157 peer has to be.)
    2. We logically mix handling of compact filter related messages with other ones. While per se it is not harmful, it feels not very appealing.

    If we try to specify which messages we still should process after a request is parked during the racing window (e.g. getdata), it defeats the whole idea of simplicity. It seems it's the same as to specify which requests we should defer (as in my commit).

    Also I'm not sure that preserving the order of responses is important, it seems any (reasonable) client matches the response by its content.

    Not sure if checking the index for updates every msgproc loop is okay or there should be an additional pause there.

    I'll try to benchmark that.

  41. ajtowns commented at 12:33 AM on August 21, 2026: contributor

    It seems this approach is conceptually simpler, but as we defer all other messages from a peer including ping or getdata my concerns are:

    1. We potentially slow down block propagation, as we don't respond to a "legitimate" `getdata` during the race window. (Though it's quite questionable why bip157 peer would ask for a full block without having filters/cfheaders for it first, but it's a presupposition what a bip157 peer has to be.)

    I think that's very unlikely -- compact block filters are for light clients; so any node requesting compact filters isn't going to have every block and isn't going to be a reliable part of the fast path for block relay. I'd suspect most such nodes wouldn't even store full blocks for the ones they request, instead just keeping the txs they're interested in and a merkle path back to the block header?

    2. We logically mix handling of compact filter related messages with other ones. While per se it is not harmful, it feels not very appealing.

    I would say that by pausing that peer until we can reply to its request we avoid mixing handling of compact filter messages with other ones.

    If we try to specify which messages we still should process after a request is parked during the racing window (e.g. getdata), it defeats the whole idea of simplicity. It seems it's the same as to specify which requests we should defer (as in my commit).

    Right, the point is to pause all message processing for that peer, so that message processing for that peer remains in order.

    Also I'm not sure that preserving the order of responses is important, it seems any (reasonable) client matches the response by its content.

    Preserving the order is valuable for our own tests (if we send message X followed by PING, we won't receive the PONG corresponding to our PING until X has been dealt with), and possibly for state updates.

    I've been thinking about it in terms of separating out block validation into a separate thread from message processing (suggested by @sipa offline, see also A and B), where it would be a nice win if we could respond to compactblock requests while validating the block, while still pausing nodes that send us messages that need to wait for the validation to finish (so that observable behaviour doesn't change at a logical level, and in particular so that our own tests don't get heavily impacted). I think getting the "pause" part of that change in here would be nice, particularly if it's simple.

  42. net: refactor: split serving compact filter requests from message parsing
    Move the serving logic of the getcfilters/getcfheaders/getcfcheckpt
    handlers into ServeGetCFilters()/ServeGetCFHeaders()/ServeGetCFCheckPt(),
    so they can be reused when the request gets parked in the next commit.
    No behavior change.
    a345a746f9
  43. net: defer response to racing compact filters requests
    Previously compact filters requests might falsely be ignored because
    the requested block has been connected to the chain, but the filters
    have not yet been constructed.
    
    This commit adds deferring of such racing requests. We store the pending
    request in a per-peer `m_pending_request`, and defer processing further
    messages from that peer until the pending request can be dealt with.
    
    Checking if we have a race condition is done via inspecting `GetSummary`
    of the index. Pending requests are resolved when the index catches up
    with the tip, or when the requested stop hash is covered by the index.
    
    Also adds a unit test for the race-condition.
    ac49cd5a0a
  44. randomlogin force-pushed on Aug 26, 2026
  45. randomlogin commented at 2:53 PM on August 26, 2026: none

    I took some time to gain more understanding on how net_processing works and your comments make a perfect sense.

    Some suggestions on top of your commit:

    1. Added a guard for initial block download. On a fresh node with enabled compact filters PeerManagerImpl::CFilterIndexMayBeRacing would return true, meaning we pause the peer for the whole duration needed to construct filters from the genesis block, which takes hours. Also we could store a timestamp when parking occured and release the pause if it takes too long, though I am not sure if it is really needed.

    2. Now we have several cases where we process peer's messages not in a "usual" way, when we might return early and not poll any new peer's messages.

      These cases are:

      • continue responding to a huge getdata message
      • parked a compact filter related message
      • we processed an orphan tx and have a result
      • in future here will be a case when we validate a block broadcast by this peer, we early exit if we haven't finished yet

      Should we combine all of the above into something like PreprocessMessages or ProcessAwaitingMessages or ProcessPausedPeer?

    3. Made minor renamings in your code.

    4. Here are some claude-coded benchmarks, I ran them locally on my computer.

    • AttemptParkedRequest() is retried on every ProcessMessages() pass for a peer with a pending request, not gated on new traffic — so n parked peers add n extra cs_main acquisitions to every message-handler sweep for as long as they stay parked.

      Per-call cost (bench_bitcoin, isolated — both paths take cs_main, differ in what's done under it):

      | path | ns/op | |---|---:| | not racing (Tip() compare only) | 66 | | racing (2 lookups + ancestor walk) | 99 |

      Negligible alone — microseconds/sec even at high parked-peer counts. The real cost is repetition, not the call.

    • Retry-rate impact (regtest, P2P sim). Natural race windows are sub-ms, too short to observe anything here, so the filter index's per-block indexing was artificially delayed ~200ms to hold the window open long enough to measure — this is a stress test of what happens if the window ever widens (slow index, edge case), not a description of current behavior.

      With that delay in place: ThreadMessageHandler's idle sleep is 100ms, so n parked peers add up to 10·n acquisitions/sec. n peers parked on getcfilters vs. n peers sending an ordinary getdata (control — resolves immediately, no repeat cost), measuring an unrelated peer's ping RTT:

      | n | +cs_main acq/s | racing max | control max | |---:|---:|---:|---:| | 10 | 100 | 101ms | 51ms | | 50 | 500 | 152ms | 52ms | | 100 | 1,000 | 302ms | 51ms |

      Control stays flat; racing tail grows with the acquisition rate — isolates the effect to parking, not to connection count. Since this depends on an artificially widened window rather than natural behavior, it supports bounding max parked duration with a timeout rather than leaving it open-ended, but isn't evidence of a problem today.

  46. DrahtBot added the label CI failed on Aug 26, 2026
  47. DrahtBot commented at 5:47 PM on August 26, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task macOS native, fuzz: https://github.com/bitcoin/bitcoin/actions/runs/32980342303/job/98234423973</sub> <sub>LLM reason (✨ experimental): CI failed because the fuzz target rpc hit an error (“Error processing input …”), returning exit code 1.</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>

  48. maflcko closed this on Aug 27, 2026

  49. maflcko reopened this on Aug 27, 2026

  50. net: add IBD guard for parked cfilter request 4849c0555c
  51. randomlogin force-pushed on Aug 27, 2026
  52. DrahtBot removed the label CI failed on Aug 27, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-08-31 18:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me