p2p: use header time for compact block recency #36055

1. 
   xternet commented at 7:50 PM on August 21, 2026: none

   Summary

   • use the announced block header's timestamp to check unsolicited compact-block recency
   • preserve the existing 20-target-spacing cutoff
   • leave CanDirectFetch() and its other callers unchanged
   • add regression coverage for compact-block relay when the active tip is stale

   CanDirectFetch() checks the active tip's timestamp. After 20 target spacings without a block, this causes a recent unsolicited compact block from a high-bandwidth peer to be ignored.

   This change uses the announced block header's timestamp for that local check instead. Although header timestamps are miner-selected, this only affects whether reconstruction is attempted; existing proof-of-work, chain-work, peer, resource, and block-validation checks still apply.

   Fixes #33578

   Test coverage

   The functional test keeps the active tip stale and verifies that:

   • an old unsolicited compact block is ignored
   • a distinct compact block with a recent header triggers GETBLOCKTXN
   • mock time is restored after the test

   Validation

   test/functional/p2p_compactblocks.py

2. DrahtBot added the label P2P on Aug 21, 2026
3. 
   DrahtBot commented at 7:50 PM on August 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/36055.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline and AI policy for information on the review process.

   Type	Reviewers
   Concept ACK	instagibbs

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

4. p2p: use header time for compact block recency ce51893c5c
5. xternet force-pushed on Aug 22, 2026
6. xternet marked this as ready for review on Aug 22, 2026
7. in src/net_processing.cpp:4895 in ce51893c5c

   4890 | @@ -4891,8 +4891,8 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
   4891 |              return;
   4892 |          }
   4893 |  
   4894 | -        // If we're not close to tip yet, give up and let parallel block fetch work its magic
   4895 | -        if (!already_in_flight && !CanDirectFetch()) {
   4896 | +        // If this is not a recent block, give up and let parallel block fetch work its magic
   4897 | +        if (!already_in_flight && pindex->Time() <= NodeClock::now() - m_chainparams.GetConsensus().PowTargetSpacing() * 20) {
   

   ---

   ---

   

   vicjuma commented at 8:53 PM on August 23, 2026:

   Is this even necessary? The essence is to avoid depending on inbound data. Using the already validated time is crucial.

   ---

   

   xternet commented at 8:45 PM on August 24, 2026:

   I agree that avoiding unprocessed inbound data matters. Using the announced header timestamp is not strictly the only solution, but it seems like the best localized tradeoff here.

   pindex is available only after ProcessNewBlockHeaders() processes the header. The remaining downside is that miners control nTime, so a miner with a valid header could make it appear recent and trigger unnecessary reconstruction. This does not bypass proof-of-work, chain-work, resource limits, or later full-block validation.

   For context, this is my rough ranking. The weights are: fixing the bug 35%, DoS safety 30%, simplicity 20%, and restart/race handling 15%.

   #	Solution	Pros	Cons	final_score
   1	Announced header time	Small, stateless, fixes #33578	Miner can trigger bounded reconstruction work	8.6
   2	Exited IBD + best-header proximity + local first-seen time	Strongest timestamp-free option	Much more state, testing, and restart handling	7.9
   3	Best-header proximity	Timestamp-free; detects block sync lag	Cannot detect an eclipsed stale node	7.8
   4	Previously exited IBD	Existing receiver-owned state	Remains latched after the node becomes stale	7.4
   5	received_new_header	Small and timestamp-free	Fails if another peer supplied the header first	6.8
   6	Remove the recency gate	Simple and fixes the relay failure	Weakens the DoS defense during historical sync	6.8
   7	Receiver-recorded first-seen time	Uses the node’s clock	Requires new state; old unseen headers appear recent	6.7
   8	Keep CanDirectFetch()	Simple and conservative	Leaves #33578 unfixed	5.9

   My recommendation is to keep the current check because the downside is limited and the existing protections remain in place. I would be happy to consider another simple, timestamp-free solution if I have missed one—especially one that avoids disproportionate state or complexity.

8. sedited requested review from davidgumberg on Aug 27, 2026
9. 
   instagibbs commented at 10:11 AM on August 27, 2026: member

   I'm not sure we landed on a concept ACK quite yet from the issue.... and I think the discussion should continue there

10. 
    sedited commented at 10:18 AM on August 27, 2026: contributor

    Ok, given that, I'm closing this again in the meantime, since I don't get the sense that the author truly understands this change. Would be good if either of you @instagibbs and @davidgumberg could leave a comment on the issue indicating what should be done there, or if we just leave this unfixed.

11. sedited closed this on Aug 27, 2026

    ---

Contributors

xternetDrahtBotvicjumainstagibbssedited

Review Requested
davidgumberg