From df8d57896cdf172462be69e347a5b7c2cf4b96d1 Mon Sep 17 00:00:00 2001 From: frstrtr Date: Fri, 19 Jun 2026 12:06:20 +0000 Subject: [PATCH] bch(M5): drop evicted-then-arrived block from the download queue expire() requeues a timed-out in-flight block to the FRONT of the pending queue and records it in m_evicted. If the (merely slow) peer then delivers that block, on_block_received bumped false_evict_count but left the hash in the queue -- so the next next_requests() drain re-getdata'd a block already in hand. Remove the hash from the queue on the false-evict path (linear scan; eviction is rare, healthy sync keeps false_evict at 0). A hash already re-issued sits in m_in_flight, not the queue, so the scan is a no-op there. Adds an evicted-then-arrived test pinning both legs: false_evict_count flags the premature eviction (distinct from a clean arrival) AND queued()==0 / a following drain is empty -- no re-download. Refreshes the stale header note that still claimed eviction was deferred (it is implemented and NodeP2P-driven). PURE block-download window plumbing; p2pool-merged-v36 surface NONE; per-coin isolation src/impl/bch/ only; header + test build-inert (bch skip-green). --- src/impl/bch/coin/block_download.hpp | 22 ++++++++++++++---- src/impl/bch/test/block_download_test.cpp | 28 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/impl/bch/coin/block_download.hpp b/src/impl/bch/coin/block_download.hpp index 49e1277f0..16b5703d3 100644 --- a/src/impl/bch/coin/block_download.hpp +++ b/src/impl/bch/coin/block_download.hpp @@ -27,10 +27,11 @@ // re-requested, so an overlapping getheaders locator batch or a re-announce // cannot double-download. // -// NOT IN THIS SLICE (deferred per integrator 2026-06-18): in-flight timeout / -// eviction (a block requested but never delivered by a stalling peer). That is -// robustness hardening worth doing only once blocks are actually flowing; this -// slice builds the happy-path window first. +// IN-FLIGHT TIMEOUT / EVICTION (expire()): a block getdata'd but never delivered +// by a stalling peer is timed out, requeued oldest-first, and re-issued; an +// evicted block the peer later delivers anyway is flagged via false_evict_count +// and dropped from the queue so it is not re-downloaded. NodeP2P drives this on +// a steady-clock timer (see p2p_node.hpp BLOCK_DL_TIMEOUT_SEC). // // p2pool-merged-v36 SURFACE: NONE -- pure SPV/IBD wire-sync plumbing; no PoW // hash, share format, coinbase commitment, AuxPoW, or PPLNS math. PER-COIN @@ -109,7 +110,18 @@ class BlockDownloadWindow { // On a healthy peer with a generous timeout NOTHING expires, so this // stays 0 -- a non-zero value means the BLOCK_DL_TIMEOUT_SEC fired on a // request the peer was merely slow to answer, not genuinely stalled. - if (m_evicted.erase(h)) ++m_false_evict_count; + if (m_evicted.erase(h)) { + ++m_false_evict_count; + // expire() requeued this hash to the FRONT when it timed out; the + // (merely slow) peer has now delivered it, so drop it from the pending + // queue -- otherwise the next drain would re-getdata a block we already + // hold. Linear scan is fine: eviction is rare (a healthy sync keeps + // false_evict at 0). If it was already re-issued it is in m_in_flight, + // not m_queue, and the scan simply finds nothing. + for (auto qit = m_queue.begin(); qit != m_queue.end(); ++qit) { + if (*qit == h) { m_queue.erase(qit); break; } + } + } auto it = m_in_flight.find(h); if (it == m_in_flight.end()) { // Unsolicited or already handled: still remember it so a later diff --git a/src/impl/bch/test/block_download_test.cpp b/src/impl/bch/test/block_download_test.cpp index 0d5567626..b27308bbc 100644 --- a/src/impl/bch/test/block_download_test.cpp +++ b/src/impl/bch/test/block_download_test.cpp @@ -201,6 +201,34 @@ int main() assert(w.reissue_count() == 1); } + // ---- evicted-then-arrived: false_evict accounting + NO re-download ------ + // A block we expired() but the (merely slow) peer delivers anyway is a + // PREMATURE eviction. The window must (a) flag it via false_evict_count + // -- distinct from a clean arrival -- and (b) drop it from the pending + // queue so the next drain does NOT re-getdata a block we already hold. + { + BlockDownloadWindow w(2); + w.enqueue(hashes(0, 2)); // [H0 H1] + w.next_requests(/*now=*/100); // H0,H1 issued @100 + assert(w.on_block_received(H(1)) == true); // H1 arrives clean + assert(w.false_evict_count() == 0); // a clean arrival is NOT premature + + // H0 stalls past the timeout -> evicted + requeued to the front. + auto evicted = w.expire(/*now=*/200, /*timeout=*/50); + assert(evicted.size() == 1 && evicted[0] == H(0)); + assert(w.reissue_count() == 1); + assert(w.queued() == 1 && w.in_flight() == 0); + + // The peer was merely slow: H0 arrives AFTER its eviction. + assert(w.on_block_received(H(0)) == false); // unsolicited (no longer in flight) + assert(w.false_evict_count() == 1); // flagged as a premature eviction + + // ...and it must NOT be re-requested -- we already hold the body. + assert(w.queued() == 0); + assert(w.next_requests(/*now=*/300).empty()); + assert(w.idle()); + } + std::cout << "bch block_download window: ALL PASS\n"; return 0; }