diff --git a/src/impl/bch/known_txs_retention.hpp b/src/impl/bch/known_txs_retention.hpp new file mode 100644 index 000000000..14d481bfc --- /dev/null +++ b/src/impl/bch/known_txs_retention.hpp @@ -0,0 +1,181 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// Pure, unit-testable bookkeeping for the known-transaction pool that backs +// share broadcast (remember_tx forwarding). Ported to the BCH variant from +// src/impl/dash/known_txs_retention.hpp so the retention/eviction and +// broadcast-gate semantics can be exercised without a live node. +// +// Two concerns: +// retain_template_txs() — the rolling-window retention with template-identity +// dedup (code review F1): register_template_txs is invoked per producer-job +// cache MISS, i.e. per (prev_share_hash, payout_script), NOT per template +// rotation. A ~50-payout-script cluster therefore re-registers the SAME +// template ~50 times on one tip; pushing each unconditionally collapses the +// N-slot window to a single template within seconds. Dedup by the template's +// tx-hash SET (its identity): a set already retained refreshes recency and +// consumes NO new slot, so N re-registrations of one template use ONE slot +// and the window holds N DISTINCT templates as intended. +// +// all_txs_backable() — the canonical broadcast gate (F3): a share may be sent +// only when we HOLD the bytes of every referenced new-tx (so remember_tx can +// always forward them). This is what makes the "referenced unknown +// transaction" disconnect unreachable by construction, and — paired with the +// mint-time decline of shares whose txs are not retained — makes "every share +// we mint/broadcast is backable" a by-construction invariant. +// +// partition_backable() — the batch form of the gate, applied by send_shares() +// to the shares it is about to write to one peer. +// +// broadcast_and_mark() — the F2 marking policy: broadcast_share() may mark a +// share "already shared" only AFTER a peer actually accepted it. Marking the +// whole chain-walk up front (the pre-fix pattern) loses every share in a +// batch that send_shares then abandons — the gate skipped it, the tracker +// try_to_lock missed, or there were no peers — because the de-dup set then +// breaks the walk forever and nothing re-pushes it. +// +// NOTE (de-dup): the same primitives now exist per-coin (btc/dgb/bch/dash) while +// the LTC lane hoists all_txs_backable into src/core/. Once both land, these +// per-coin headers should be collapsed onto that core hoist. + +#pragma once + +#include +#include +#include +#include + +#include + +namespace bch { + +// Retain the tx set of a newly-registered template in a rolling window of the +// last `cap` DISTINCT templates, inserting its tx bytes into `known_txs`. +// +// recent_sets : rolling history of distinct template tx-hash sets +// (front = oldest, back = newest). +// known_txs : uint256 -> tx map; needs insert_or_assign(k,v) and erase(k). +// hashes/txs : parallel arrays of the template's tx hashes and bytes. +// cap : number of DISTINCT templates to retain. +// +// A tx is erased from `known_txs` only once it has fallen out of EVERY retained +// set. F1 dedup: if this template's tx set is already retained, its recency is +// refreshed (moved to back) and no new slot is consumed. +template +inline void retain_template_txs(std::deque>& recent_sets, + TxMap& known_txs, + const std::vector& hashes, + const std::vector& txs, + std::size_t cap) +{ + if (cap == 0) + return; + + std::set new_set(hashes.begin(), hashes.end()); + + // F1 dedup by template identity (the tx-hash set). If this template is + // already retained, refresh its recency (move to back) — no new slot, no + // re-eviction. This is what stops ~50 payout-script re-registrations of one + // template from evicting the rest of the window. + for (auto it = recent_sets.begin(); it != recent_sets.end(); ++it) { + if (*it == new_set) { + if (std::next(it) != recent_sets.end()) { + std::set refreshed = std::move(*it); + recent_sets.erase(it); + recent_sets.push_back(std::move(refreshed)); + } + return; + } + } + + // Distinct template: insert its tx bytes and consume a slot. + const std::size_t n = std::min(hashes.size(), txs.size()); + for (std::size_t i = 0; i < n; ++i) + known_txs.insert_or_assign(hashes[i], txs[i]); + recent_sets.push_back(std::move(new_set)); + + // Evict the oldest slots beyond `cap`; a tx is removed from `known_txs` only + // once it is absent from EVERY remaining retained set. + while (recent_sets.size() > cap) { + const std::set evicted = std::move(recent_sets.front()); + recent_sets.pop_front(); + for (const auto& h : evicted) { + bool still_retained = false; + for (const auto& s : recent_sets) { + if (s.count(h)) { still_retained = true; break; } + } + if (!still_retained) + known_txs.erase(h); + } + } +} + +// True iff every hash in `tx_hashes` is present in `held` (the bytes we hold, +// e.g. m_known_txs). The canonical broadcast gate: a share is backable — safe to +// send without risking the peer's "referenced unknown transaction" disconnect — +// only when we can forward every referenced tx. +template +inline bool all_txs_backable(const std::vector& tx_hashes, + const Held& held) +{ + for (const auto& h : tx_hashes) + if (held.find(h) == held.end()) + return false; + return true; +} + +// Batch form of the F3 gate. Keeps in `shares` only the entries every one of +// whose referenced new-tx hashes we hold, and returns how many were dropped. +// `hashes_of(share)` yields that share's referenced new-tx hashes. +// +// Skipping costs no propagation: a share we downloaded is by definition already +// on the network we fetched it from, and it will be re-offered on a later +// broadcast once its tx bytes arrive (F2 leaves it un-marked). +template +inline std::size_t partition_backable(std::vector& shares, + const Held& held, + HashesOf hashes_of) +{ + std::vector sendable; + sendable.reserve(shares.size()); + std::size_t skipped = 0; + for (auto& share : shares) { + if (all_txs_backable(hashes_of(share), held)) + sendable.push_back(std::move(share)); + else + ++skipped; + } + shares.swap(sendable); + return skipped; +} + +// F2 marking policy. Offers the batch to every peer via `send(peer)` — which +// returns the hashes it ACTUALLY wrote to that peer — and marks in `marked` +// only the hashes that reached at least one peer. Returns that count. +// +// The pre-fix code marked the whole chain-walk before calling send_shares, so a +// batch abandoned by every peer (gate skip, try_to_lock miss, empty peer set) +// stayed marked forever: the next broadcast_share walk breaks on the first +// marked hash and the share is never re-pushed. That is silent, permanent share +// loss with no retry path — a PPLNS-credit loss for whoever mined it. +template +inline std::size_t broadcast_and_mark(MarkedSet& marked, Peers& peers, + const std::vector& to_send, + SendFn send) +{ + if (to_send.empty()) + return 0; + + // NOTE: `to_send` is deliberately NOT what gets marked. Marking the offered + // batch here is precisely the pre-fix bug this signature exists to make + // un-writable by accident — only `send`'s report may be marked. + std::set actually_sent; + for (auto& entry : peers) { + const std::vector sent = send(entry.second); + actually_sent.insert(sent.begin(), sent.end()); + } + for (const auto& h : actually_sent) + marked.insert(h); + return actually_sent.size(); +} + +} // namespace bch diff --git a/src/impl/bch/node.cpp b/src/impl/bch/node.cpp index 9e0e29cbd..bc84717b3 100644 --- a/src/impl/bch/node.cpp +++ b/src/impl/bch/node.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: AGPL-3.0-or-later #include "node.hpp" +#include "known_txs_retention.hpp" // bch::all_txs_backable / partition_backable / broadcast_and_mark (F2/F3) #include "share_tx_refs.hpp" // bch::new_tx_hashes -- uniform send-side probe (#905) @@ -684,8 +685,13 @@ std::vector NodeImpl::handle_get_share(std::vector hash return shares; } -void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hashes) +std::vector NodeImpl::send_shares(peer_ptr peer, + const std::vector& share_hashes) { + // Returns the hashes of the shares actually WRITTEN to this peer (F2). The + // caller marks only those as broadcast, so a share skipped here — by the + // try_to_lock miss below or by the F3 tx-completeness gate — is re-offered + // on the next broadcast instead of being silently dropped forever. // try_to_lock per the architectural rule (node.hpp:67) — see freeze // analysis in handle_get_share above. If we can't acquire NOW, skip // this batch. The shares are still in our chain; the next broadcast @@ -697,7 +703,7 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash if (defer_log++ % 50 == 0) LOG_INFO << "[send_shares] tracker busy — skipping send to " << peer->addr().to_string() << " (will retry next cycle)"; - return; + return {}; } // Collect shares that exist in our chain (skip rejected) @@ -717,7 +723,51 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash } if (shares.empty()) - return; + return {}; + + // --- F3 tx-completeness broadcast gate -------------------------------- + // A share whose referenced new-tx BYTES we do not hold cannot be backed by + // the remember_tx forward below, and canonical p2pool drops the connection + // on "referenced unknown transaction" (p2p.py) — which isolates us from the + // sharechain and orphans our shares. Shares pulled via sharereply arrive + // WITHOUT their tx bytes, so this is reachable in normal operation. + // + // Gate on the BYTES in m_known_txs, not on the peer's have_tx advert: that + // advert can be stale (the peer may have dropped the tx) and sending + // hash-only for a tx the peer no longer holds is exactly the disconnect. + // m_remote_txs is used ONLY below to choose the hash-vs-bytes encoding. + // + // CONSENSUS-NEUTRAL: this decides only WHETHER we broadcast. Share bytes, + // minting and payout are untouched; a skipped share stays in our chain and + // is re-offered once its txs arrive (send_shares reports it as unsent, so + // broadcast_share leaves it un-marked). + { + const size_t skipped_incomplete = partition_backable( + shares, m_known_txs, [](ShareType& share) { + std::vector hashes; + share.invoke([&](auto* obj) { + // #880: route through the bch::new_tx_hashes SSOT (#913) + // instead of the dead flat probe -- v17/v33 nest the list in + // m_tx_info, v34+ carry none. The old probe was FALSE for + // every variant, so this F3 gate never withheld a share. + if (const auto* new_txs = bch::new_tx_hashes(obj)) + hashes.assign(new_txs->begin(), new_txs->end()); + }); + return hashes; + }); + if (skipped_incomplete > 0) + { + static int skip_log = 0; + if (skip_log++ % 20 == 0) + LOG_INFO << "[send_shares] skipped " << skipped_incomplete + << " share(s) whose new-tx bytes we do not hold " + "(downloaded via sharereply / evicted) to " + << peer->addr().to_string() + << " — avoids canonical 'unknown transaction' disconnect"; + } + if (shares.empty()) + return {}; + } // Collect transactions that the peer doesn't know about std::set needed_txs; @@ -749,6 +799,8 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash std::vector known_hashes; // hashes in peer's remote set std::vector full_txs; // full txs otherwise + size_t missing = 0; + for (const auto& th : needed_txs) { if (peer->m_remote_txs.count(th)) @@ -760,9 +812,20 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash auto it = m_known_txs.find(th); if (it != m_known_txs.end()) full_txs.emplace_back(it->second); + else + ++missing; // unreachable after the gate above — see below } } + // Defence in depth: the gate should already have removed every share + // with an unheld tx, so this must stay at zero. If it ever fires, the + // gate's view of the referenced hashes disagrees with needed_txs and + // the peer may drop these shares — make that loud rather than silent. + if (missing > 0) + LOG_WARNING << "[send_shares] " << missing << " referenced tx(s) not in " + "m_known_txs after the completeness gate — peer may drop " + "these shares (share/tx hash accounting mismatch?)"; + if (!known_hashes.empty() || !full_txs.empty()) { auto rtx_msg = message_remember_tx::make_raw(known_hashes, full_txs); @@ -799,6 +862,13 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash LOG_INFO << "[Pool] Sent " << shares.size() << " shares (+" << needed_txs.size() << " txs) to " << peer->addr().to_string(); + + // F2: report exactly which shares reached this peer. + std::vector sent; + sent.reserve(shares.size()); + for (auto& share : shares) + sent.push_back(share.hash()); + return sent; } void NodeImpl::broadcast_share(const uint256& share_hash) @@ -821,7 +891,14 @@ void NodeImpl::broadcast_share(const uint256& share_hash) return; } - // Walk the chain back from share_hash, collecting un-broadcast shares + // Walk the chain back from share_hash, collecting un-broadcast shares. + // + // F2: do NOT mark them shared here. send_shares can abandon the whole batch + // — the tx-completeness gate skipped it, its tracker try_to_lock missed, or + // there are zero peers — and a share marked before that never gets another + // chance: the next walk breaks on the first marked hash, so the tip is + // permanently withheld from the sharechain and its PPLNS credit is lost. + // We mark only what actually reached >= 1 peer, below. std::vector to_send; int32_t height = m_chain->get_height(share_hash); int32_t walk = std::min(height, 5); @@ -832,7 +909,6 @@ void NodeImpl::broadcast_share(const uint256& share_hash) break; if (m_rejected_share_hashes.count(hash)) continue; // skip shares previously rejected by peers - m_shared_share_hashes.insert(hash); to_send.push_back(hash); } @@ -840,10 +916,16 @@ void NodeImpl::broadcast_share(const uint256& share_hash) return; auto now = std::chrono::steady_clock::now(); - for (auto& [nonce, peer] : m_peers) { - send_shares(peer, to_send); - m_last_broadcast_to[peer->addr()] = {to_send, now}; - } + broadcast_and_mark(m_shared_share_hashes, m_peers, to_send, [&](peer_ptr& peer) { + std::vector sent = send_shares(peer, to_send); + // Record what we ACTUALLY wrote, not what we offered: a peer that drops + // within 10s marks this record's hashes rejected, and a rejected hash is + // never re-broadcast. Blaming a share we withheld would turn a transient + // gate skip into permanent loss. + if (!sent.empty()) + m_last_broadcast_to[peer->addr()] = {sent, now}; + return sent; + }); } void NodeImpl::notify_local_share(const uint256& share_hash) @@ -983,6 +1065,20 @@ uint256 NodeImpl::advertised_best_share() return uint256::ZERO; } +// ROOT-2 re-advert. Unlike ltc/dgb (which have a dedicated readvertise that +// bypasses the de-dup set), this delegates to broadcast_share, so the walk still +// breaks on the first hash already in m_shared_share_hashes. +// +// Interaction with the F2 marking change: marking is now a strict SUBSET of what +// the pre-fix code marked (only hashes a peer actually accepted). The walk breaks +// later or in the same place, never earlier, so this re-advert re-pushes a +// superset of what it used to — monotonically better, no regression. It is +// strictly better in the case that motivated ROOT-2: head shares minted while no +// peer was connected are no longer marked, so a peer that handshook during the +// empty window now receives them. It does NOT fully close ROOT-2 here — head +// shares already accepted by some OTHER peer stay marked and the walk still +// breaks, exactly as before. Closing that needs the ltc/dgb-style de-dup-bypass +// readvertise; pre-existing gap, out of scope for this change. void NodeImpl::readvertise_best() { if (m_peers.empty()) diff --git a/src/impl/bch/node.hpp b/src/impl/bch/node.hpp index da0c3695d..d163cd811 100644 --- a/src/impl/bch/node.hpp +++ b/src/impl/bch/node.hpp @@ -605,9 +605,13 @@ class NodeImpl : public pool::SharechainNode& share_hashes); + /// Send a set of shares (with any needed txs) to a single peer, and return + /// the subset actually written (F2). Shares dropped by the tx-completeness + /// gate, or the whole batch when the tracker lock is unavailable, are + /// reported as NOT sent, so the caller leaves them un-marked and retries + /// them on the next broadcast. + std::vector send_shares(peer_ptr peer, + const std::vector& share_hashes); /// Broadcast a locally-generated (or newly-received) share to all peers. void broadcast_share(const uint256& share_hash); diff --git a/src/impl/bch/test/CMakeLists.txt b/src/impl/bch/test/CMakeLists.txt index 9a343c10b..d36464147 100644 --- a/src/impl/bch/test/CMakeLists.txt +++ b/src/impl/bch/test/CMakeLists.txt @@ -222,6 +222,12 @@ if(BUILD_TESTING) # allowlisted target rather than a new add_executable -- a new one would # silently report "Not Run" (see merged PR #868). Still bch-only, so the # per-coin isolation invariant holds. + # F2/F3 share-broadcast gate KATs ride bch_embedded_block_broadcast_test (an + # already-allowlisted --target). The bch test tree has no GTest harness and a + # standalone target would need a build.yml entry this repo's CI token cannot + # add, which CTest would then report as a silently-passing NOT_BUILT sentinel. + target_sources(bch_embedded_block_broadcast_test PRIVATE broadcast_gate_test.cpp) + add_executable(bch_g2_block_assembly_roundtrip_test g2_block_assembly_roundtrip_test.cpp ${CMAKE_SOURCE_DIR}/src/impl/bch/coin/transaction.cpp) diff --git a/src/impl/bch/test/broadcast_gate_test.cpp b/src/impl/bch/test/broadcast_gate_test.cpp new file mode 100644 index 000000000..cc09e2a75 --- /dev/null +++ b/src/impl/bch/test/broadcast_gate_test.cpp @@ -0,0 +1,211 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +// --------------------------------------------------------------------------- +// F3 tx-completeness broadcast gate + F2 mark-only-what-was-sent — BCH KATs. +// +// Both primitives are the ones send_shares()/broadcast_share() in +// src/impl/bch/node.cpp actually call, so these checks pin the shipped policy: +// +// partition_backable() — F3. send_shares must NOT write a share whose +// referenced new-tx BYTES we do not hold: canonical p2pool disconnects on +// "referenced unknown transaction" (p2p.py), which isolates us from the +// sharechain and orphans our shares. Pre-fix, send_shares looked the tx up +// and simply omitted it when absent (`if (it != m_known_txs.end())` with no +// else), sending the share anyway. +// +// broadcast_and_mark() — F2. broadcast_share may add a hash to +// m_shared_share_hashes only AFTER a peer accepted it. Pre-fix it marked the +// whole chain-walk up front, so a batch that send_shares then abandoned (F3 +// skip, tracker try_to_lock miss, zero peers) was withheld FOREVER: the next +// walk breaks on the first marked hash and nothing ever re-pushes it. +// +// Red-able: the walk-stranding block reproduces the pre-fix marking order beside +// the shipped one and asserts they differ; restoring mark-before-send reddens +// the shipped half. The gate block asserts a skip count of 1; deleting the gate +// reddens it. +// +// HARNESS: the bch test tree is plain int main()/assert (no GTest), so this TU +// exposes run_share_broadcast_gate_checks() and rides the already-allowlisted +// bch_embedded_block_broadcast_test executable — no build.yml --target allowlist +// change, no NOT_BUILT sentinel risk. +// Consensus surface: NONE. These primitives decide only WHETHER a share is put +// on the wire; share bytes, minting and payout are untouched. +// --------------------------------------------------------------------------- + +#include +#include +#include +#include +#include + +#include +#include + +namespace { + +int g_failures = 0; + +void check(bool ok, const char* what) +{ + if (!ok) { + ++g_failures; + std::cerr << " FAIL: " << what << "\n"; + } +} + +uint256 h(const char* hex) { uint256 v; v.SetHex(hex); return v; } + +// Stand-in for a share on the broadcast path: its hash plus the new-tx hashes +// it references. The gate is agnostic to everything else in a share. +struct FakeShare { + uint256 hash; + std::vector new_txs; +}; + +std::vector refs_of(FakeShare& s) { return s.new_txs; } + +// A peer that accepts exactly the hashes send_shares would have written to it. +struct FakePeer { std::set accepts; }; + +// The tip-first walk broadcast_share performs, verbatim in its essentials: it +// BREAKS on the first hash already in the de-dup set. +std::vector walk(const std::vector& chain_tip_first, + const std::set& marked) +{ + std::vector to_send; + for (const auto& hash : chain_tip_first) { + if (marked.count(hash)) + break; + to_send.push_back(hash); + } + return to_send; +} + +} // namespace + +int run_share_broadcast_gate_checks() +{ + g_failures = 0; + + const uint256 S1 = h("11"), S2 = h("22"), S3 = h("33"); + const uint256 TA = h("aa"), TB = h("bb"), TC = h("cc"); + + // -- F3: an unbacked share is removed; backable ones survive, in order ---- + { + const std::set held{TA, TB}; // TC bytes NOT held + std::vector batch{{S1, {TA}}, {S2, {TC}}, {S3, {TA, TB}}}; + + const std::size_t skipped = bch::partition_backable(batch, held, refs_of); + check(skipped == 1u, "F3 gate skips exactly the unbacked share"); + check(batch.size() == 2u, "F3 gate keeps the two backable shares"); + if (batch.size() == 2u) { + check(batch[0].hash == S1, "F3 gate preserves batch order (S1)"); + check(batch[1].hash == S3, "F3 gate preserves batch order (S3)"); + } + } + + // -- F3: a share referencing no new txs is trivially backable ------------- + { + const std::set held; // hold nothing at all + std::vector batch{{S1, {}}}; + check(bch::partition_backable(batch, held, refs_of) == 0u, + "F3 gate never withholds a share with no tx refs"); + check(batch.size() == 1u, "F3 gate keeps the no-refs share"); + } + + // -- F3: every share unbacked -> empty batch, so nothing is reported sent - + { + const std::set held; + std::vector batch{{S1, {TA}}, {S2, {TB}}}; + check(bch::partition_backable(batch, held, refs_of) == 2u, + "F3 gate skips a wholly-unbacked batch"); + check(batch.empty(), "F3 gate empties a wholly-unbacked batch"); + } + + // -- F2: a batch no peer accepted leaves the de-dup set untouched --------- + { + std::set marked; + std::map peers{{1, {}}, {2, {}}}; + const std::size_t n = bch::broadcast_and_mark( + marked, peers, std::vector{S1, S2}, + [](FakePeer&) { return std::vector{}; }); + check(n == 0u, "F2 reports nothing sent when no peer accepted"); + check(marked.empty(), "F2 marks nothing when no peer accepted"); + } + + // -- F2: zero peers -> nothing sent, therefore nothing marked ------------- + { + std::set marked; + std::map peers; + check(bch::broadcast_and_mark(marked, peers, std::vector{S1, S2}, + [](FakePeer&) { + return std::vector{}; + }) == 0u, + "F2 reports nothing sent with zero peers"); + check(marked.empty(), "F2 marks nothing with zero peers"); + } + + // -- F2: exactly the union of what peers accepted is marked, no more ------ + { + std::set marked; + std::map peers{{1, {{S1}}}, {2, {{S1, S3}}}}; + const std::size_t n = bch::broadcast_and_mark( + marked, peers, std::vector{S1, S2, S3}, [](FakePeer& p) { + return std::vector(p.accepts.begin(), p.accepts.end()); + }); + check(n == 2u, "F2 counts the union of accepted hashes"); + check(marked.count(S1) == 1u, "F2 marks an accepted hash"); + check(marked.count(S3) == 1u, "F2 marks an accepted hash"); + check(marked.count(S2) == 0u, "F2 leaves a withheld share un-marked"); + } + + // -- The regression: mark-before-send strands the walk permanently -------- + // Round 1: TB is missing so S2 is withheld and no peer accepts anything. + { + const std::vector chain{S2, S1}; // S2 is the tip + std::map one_peer{{1, {}}}; + + // pre-fix ordering: mark the walk, THEN discover nothing was sent + { + std::set marked; + auto to_send = walk(chain, marked); + check(to_send.size() == 2u, "pre-fix walk yields the full batch once"); + for (const auto& hash : to_send) marked.insert(hash); // the bug + bch::broadcast_and_mark(marked, one_peer, to_send, + [](FakePeer&) { return std::vector{}; }); + // Round 2, TB has since arrived — but the tip is marked, so nothing + // is ever re-pushed. Silent, permanent share loss, no retry path. + check(walk(chain, marked).empty(), + "pre-fix ordering strands the walk (documents the bug)"); + } + + // shipped ordering: mark only what a peer accepted + { + std::set marked; + auto offered = walk(chain, marked); + check(offered.size() == 2u, "shipped walk yields the batch"); + bch::broadcast_and_mark(marked, one_peer, offered, + [](FakePeer&) { return std::vector{}; }); + check(marked.empty(), "shipped ordering marks nothing on an abandoned send"); + + auto retry = walk(chain, marked); + check(retry.size() == 2u, "shipped ordering re-offers the batch"); + if (!retry.empty()) + check(retry[0] == S2, "shipped retry still starts at the tip"); + + // ...and once a peer accepts them they are marked exactly once. + std::map live{{1, {{S1, S2}}}}; + bch::broadcast_and_mark(marked, live, retry, [](FakePeer& p) { + return std::vector(p.accepts.begin(), p.accepts.end()); + }); + check(marked.size() == 2u, "shipped ordering marks after a real send"); + check(walk(chain, marked).empty(), "shipped ordering then de-dups"); + } + } + + if (g_failures == 0) + std::cout << "share_broadcast_gate (F2/F3) KATs: ALL PASS\n"; + else + std::cerr << "share_broadcast_gate (F2/F3) KATs: " << g_failures + << " FAILURE(S)\n"; + return g_failures; +} diff --git a/src/impl/bch/test/embedded_block_broadcast_test.cpp b/src/impl/bch/test/embedded_block_broadcast_test.cpp index 3cf38cfe7..0ee13f4ac 100644 --- a/src/impl/bch/test/embedded_block_broadcast_test.cpp +++ b/src/impl/bch/test/embedded_block_broadcast_test.cpp @@ -60,6 +60,11 @@ struct SinkThrew : std::exception { } // namespace +// F2/F3 share-broadcast gate KATs (broadcast_gate_test.cpp). Kept in its own +// TU but run from this already-allowlisted executable, so it is compiled and +// executed in CI without a new --target the token cannot add to build.yml. +int run_share_broadcast_gate_checks(); + int main() { boost::asio::io_context ioc; TestConfig config; @@ -157,6 +162,9 @@ int main() { CHECK(!threw_out); } + // Share-broadcast completeness gate + mark-after-send (separate TU). + failures += run_share_broadcast_gate_checks(); + if (failures == 0) { std::cout << "embedded_block_broadcast_test: ALL PASS\n"; return 0; diff --git a/src/impl/btc/known_txs_retention.hpp b/src/impl/btc/known_txs_retention.hpp index 23a2aee75..615b96eca 100644 --- a/src/impl/btc/known_txs_retention.hpp +++ b/src/impl/btc/known_txs_retention.hpp @@ -23,10 +23,19 @@ // mint-time decline of shares whose txs are not retained — makes "every share // we mint/broadcast is backable" a by-construction invariant. // -// NOTE (BTC wiring): the call-sites that plug these primitives into -// send_shares / create_share_fn depend on how the BTC share model carries its -// referenced new-tx hashes; that wiring is tracked separately. This header is -// the pure primitive + its unit coverage. +// partition_backable() — the batch form of the gate, applied by send_shares() +// to the shares it is about to write to one peer. +// +// broadcast_and_mark() — the F2 marking policy: broadcast_share() may mark a +// share "already shared" only AFTER a peer actually accepted it. Marking the +// whole chain-walk up front (the pre-fix pattern) loses every share in a +// batch that send_shares then abandons — the gate skipped it, the tracker +// try_to_lock missed, or there were no peers — because the de-dup set then +// breaks the walk forever and nothing re-pushes it. +// +// NOTE (de-dup): the same primitives now exist per-coin (btc/dgb/bch/dash) while +// the LTC lane hoists all_txs_backable into src/core/. Once both land, these +// per-coin headers should be collapsed onto that core hoist. #pragma once @@ -133,4 +142,59 @@ inline std::vector select_backable_shares( return out; } +// Batch form of the F3 gate. Keeps in `shares` only the entries every one of +// whose referenced new-tx hashes we hold, and returns how many were dropped. +// `hashes_of(share)` yields that share's referenced new-tx hashes. +// +// Skipping costs no propagation: a share we downloaded is by definition already +// on the network we fetched it from, and it will be re-offered on a later +// broadcast once its tx bytes arrive (F2 leaves it un-marked). +template +inline std::size_t partition_backable(std::vector& shares, + const Held& held, + HashesOf hashes_of) +{ + std::vector sendable; + sendable.reserve(shares.size()); + std::size_t skipped = 0; + for (auto& share : shares) { + if (all_txs_backable(hashes_of(share), held)) + sendable.push_back(std::move(share)); + else + ++skipped; + } + shares.swap(sendable); + return skipped; +} + +// F2 marking policy. Offers the batch to every peer via `send(peer)` — which +// returns the hashes it ACTUALLY wrote to that peer — and marks in `marked` +// only the hashes that reached at least one peer. Returns that count. +// +// The pre-fix code marked the whole chain-walk before calling send_shares, so a +// batch abandoned by every peer (gate skip, try_to_lock miss, empty peer set) +// stayed marked forever: the next broadcast_share walk breaks on the first +// marked hash and the share is never re-pushed. That is silent, permanent share +// loss with no retry path — a PPLNS-credit loss for whoever mined it. +template +inline std::size_t broadcast_and_mark(MarkedSet& marked, Peers& peers, + const std::vector& to_send, + SendFn send) +{ + if (to_send.empty()) + return 0; + + // NOTE: `to_send` is deliberately NOT what gets marked. Marking the offered + // batch here is precisely the pre-fix bug this signature exists to make + // un-writable by accident — only `send`'s report may be marked. + std::set actually_sent; + for (auto& entry : peers) { + const std::vector sent = send(entry.second); + actually_sent.insert(sent.begin(), sent.end()); + } + for (const auto& h : actually_sent) + marked.insert(h); + return actually_sent.size(); +} + } // namespace btc diff --git a/src/impl/btc/node.cpp b/src/impl/btc/node.cpp index 20d4dc55d..09165cdb3 100644 --- a/src/impl/btc/node.cpp +++ b/src/impl/btc/node.cpp @@ -1,6 +1,7 @@ // SPDX-License-Identifier: AGPL-3.0-or-later #include "node.hpp" #include "known_txs_retention.hpp" // F3 retain_template_txs + select_backable_shares +#include "share_tx_refs.hpp" // btc::new_tx_hashes -- uniform gate probe (#880) #include "coin/template_other_txs.hpp" // deserialize_template_other_txs (GBT data[] -> MutableTransaction) #include @@ -727,8 +728,12 @@ void NodeImpl::register_template_txs(const uint256& share_hash, << m_template_recent_sets.size(); } -void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hashes) +std::vector NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hashes) { + // Returns the hashes of the shares actually WRITTEN to this peer (F2). The + // caller marks only those as broadcast, so a share skipped here — by the + // try_to_lock miss below or by the F3 tx-completeness gate — is re-offered + // on the next broadcast instead of being silently dropped forever. // try_to_lock per the architectural rule (node.hpp:67) — see freeze // analysis in handle_get_share above. If we can't acquire NOW, skip // this batch. The shares are still in our chain; the next broadcast @@ -740,7 +745,7 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash if (defer_log++ % 50 == 0) LOG_INFO << "[send_shares] tracker busy — skipping send to " << peer->addr().to_string() << " (will retry next cycle)"; - return; + return {}; } // Collect shares that exist in our chain (skip rejected) @@ -760,7 +765,52 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash } if (shares.empty()) - return; + return {}; + + // --- F3 tx-completeness broadcast gate -------------------------------- + // A share whose referenced new-tx BYTES we do not hold cannot be backed by + // the remember_tx forward below, and canonical p2pool drops the connection + // on "referenced unknown transaction" (p2p.py) — which isolates us from the + // sharechain and orphans our shares. Shares pulled via sharereply arrive + // WITHOUT their tx bytes, so this is reachable in normal operation. + // + // Gate on the BYTES in m_known_txs, not on the peer's have_tx advert: that + // advert can be stale (the peer may have dropped the tx) and sending + // hash-only for a tx the peer no longer holds is exactly the disconnect. + // m_remote_txs is used ONLY below to choose the hash-vs-bytes encoding. + // + // CONSENSUS-NEUTRAL: this decides only WHETHER we broadcast. Share bytes, + // minting and payout are untouched; a skipped share stays in our chain and + // is re-offered once its txs arrive (send_shares reports it as unsent, so + // broadcast_share leaves it un-marked). + { + const size_t skipped_incomplete = partition_backable( + shares, m_known_txs, [](ShareType& share) { + std::vector hashes; + share.invoke([&](auto* obj) { + // #880: route through the btc::new_tx_hashes SSOT instead + // of the dead flat probe -- v17/v33 nest the list in + // m_tx_info, v34+ carry none. The old probe was FALSE for + // every variant, so this F3 gate saw empty refs -> every + // share vacuously backable -> the gate no-op'd all versions. + if (const auto* new_txs = btc::new_tx_hashes(obj)) + hashes.assign(new_txs->begin(), new_txs->end()); + }); + return hashes; + }); + if (skipped_incomplete > 0) + { + static int skip_log = 0; + if (skip_log++ % 20 == 0) + LOG_INFO << "[send_shares] skipped " << skipped_incomplete + << " share(s) whose new-tx bytes we do not hold " + "(downloaded via sharereply / evicted) to " + << peer->addr().to_string() + << " — avoids canonical 'unknown transaction' disconnect"; + } + if (shares.empty()) + return {}; + } // (b) Extract each share's referenced new-tx hashes. BTC shares carry these // inside m_tx_info (btc::ShareTxInfo), NOT as a top-level @@ -806,7 +856,7 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash shares = std::move(kept_shares); per_share_refs = std::move(kept_refs); if (shares.empty()) - return; + return {}; } // Collect transactions the peer doesn't know about, over the surviving @@ -824,6 +874,8 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash std::vector known_hashes; // hashes in peer's remote set std::vector full_txs; // full txs otherwise + size_t missing = 0; + for (const auto& th : needed_txs) { if (peer->m_remote_txs.count(th)) @@ -835,9 +887,20 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash auto it = m_known_txs.find(th); if (it != m_known_txs.end()) full_txs.emplace_back(it->second); + else + ++missing; // unreachable after the gate above — see below } } + // Defence in depth: the gate should already have removed every share + // with an unheld tx, so this must stay at zero. If it ever fires, the + // gate's view of the referenced hashes disagrees with needed_txs and + // the peer may drop these shares — make that loud rather than silent. + if (missing > 0) + LOG_WARNING << "[send_shares] " << missing << " referenced tx(s) not in " + "m_known_txs after the completeness gate — peer may drop " + "these shares (share/tx hash accounting mismatch?)"; + if (!known_hashes.empty() || !full_txs.empty()) { auto rtx_msg = message_remember_tx::make_raw(known_hashes, full_txs); @@ -874,10 +937,20 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash LOG_INFO << "[Pool] Sent " << shares.size() << " shares (+" << needed_txs.size() << " txs) to " << peer->addr().to_string(); + + // F2: report exactly which shares reached this peer. + std::vector sent; + sent.reserve(shares.size()); + for (auto& share : shares) + sent.push_back(share.hash()); + return sent; } void NodeImpl::broadcast_share(const uint256& share_hash) { + if (share_hash.IsNull()) + return; + // try_to_lock per the architectural rule (node.hpp:67) — see freeze // analysis in handle_get_share above. If think+clean holds the // exclusive lock right now, defer this broadcast: the share is still @@ -888,15 +961,48 @@ void NodeImpl::broadcast_share(const uint256& share_hash) std::shared_lock lock(m_tracker_mutex, std::try_to_lock); if (!lock.owns_lock()) { + const uint64_t deferred = + m_broadcast_deferred.fetch_add(1, std::memory_order_relaxed) + 1; static int defer_log = 0; if (defer_log++ % 50 == 0) LOG_INFO << "[broadcast_share] tracker busy — deferring broadcast of " << share_hash.GetHex().substr(0, 16) << " (next cycle will pick it up)"; + // A long run of deferrals with ZERO successes is not contention — it + // is a caller holding m_tracker_mutex EXCLUSIVELY across this call on + // this thread, which can never grant a shared lock, so the node + // silently broadcasts nothing at all. Say so loudly and once. + if (deferred >= 20 && m_broadcast_acquired.load(std::memory_order_relaxed) == 0) { + static bool warned = false; + if (!warned) { + warned = true; + LOG_WARNING << "[broadcast_share] " << deferred << " consecutive" + << " deferrals with ZERO successful acquisitions — a" + " caller is almost certainly holding the tracker" + " mutex EXCLUSIVELY across this call, which can" + " never grant a shared lock. NO SHARE IS BEING" + " BROADCAST. Drop that lock before calling" + " broadcast_share (main_btc create_share_fn does)."; + } + } return; } + m_broadcast_acquired.fetch_add(1, std::memory_order_relaxed); - // Walk the chain back from share_hash, collecting un-broadcast shares + // The share may have been pruned between mint and here; get_height / + // get_chain on an absent hash is not a defined query, so bail rather + // than walk garbage (mirrors the ltc guard). + if (!m_chain || !m_chain->contains(share_hash)) + return; + + // Walk the chain back from share_hash, collecting un-broadcast shares. + // + // F2: do NOT mark them shared here. send_shares can abandon the whole batch + // — the tx-completeness gate skipped it, its tracker try_to_lock missed, or + // there are zero peers — and a share marked before that never gets another + // chance: the next walk breaks on the first marked hash, so the tip is + // permanently withheld from the sharechain and its PPLNS credit is lost. + // We mark only what actually reached >= 1 peer, below. std::vector to_send; int32_t height = m_chain->get_height(share_hash); int32_t walk = std::min(height, 5); @@ -907,18 +1013,29 @@ void NodeImpl::broadcast_share(const uint256& share_hash) break; if (m_rejected_share_hashes.count(hash)) continue; // skip shares previously rejected by peers - m_shared_share_hashes.insert(hash); to_send.push_back(hash); } if (to_send.empty()) return; + // The walk produced a non-empty batch and we hold the shared lock: the + // per-peer send loop (F3 gate + send_shares + F2 mark) is now entered. + // This is the count that stays at ZERO if a caller holds the exclusive + // lock across the call. + m_broadcast_reached_send.fetch_add(1, std::memory_order_relaxed); + auto now = std::chrono::steady_clock::now(); - for (auto& [nonce, peer] : m_peers) { - send_shares(peer, to_send); - m_last_broadcast_to[peer->addr()] = {to_send, now}; - } + broadcast_and_mark(m_shared_share_hashes, m_peers, to_send, [&](peer_ptr& peer) { + std::vector sent = send_shares(peer, to_send); + // Record what we ACTUALLY wrote, not what we offered: a peer that drops + // within 10s marks this record's hashes rejected, and a rejected hash is + // never re-broadcast. Blaming a share we withheld would turn a transient + // gate skip into permanent loss. + if (!sent.empty()) + m_last_broadcast_to[peer->addr()] = {sent, now}; + return sent; + }); } void NodeImpl::notify_local_share(const uint256& share_hash) @@ -1058,6 +1175,20 @@ uint256 NodeImpl::advertised_best_share() return uint256::ZERO; } +// ROOT-2 re-advert. Unlike ltc/dgb (which have a dedicated readvertise that +// bypasses the de-dup set), this delegates to broadcast_share, so the walk still +// breaks on the first hash already in m_shared_share_hashes. +// +// Interaction with the F2 marking change: marking is now a strict SUBSET of what +// the pre-fix code marked (only hashes a peer actually accepted). The walk breaks +// later or in the same place, never earlier, so this re-advert re-pushes a +// superset of what it used to — monotonically better, no regression. It is +// strictly better in the case that motivated ROOT-2: head shares minted while no +// peer was connected are no longer marked, so a peer that handshook during the +// empty window now receives them. It does NOT fully close ROOT-2 here — head +// shares already accepted by some OTHER peer stay marked and the walk still +// breaks, exactly as before. Closing that needs the ltc/dgb-style de-dup-bypass +// readvertise; pre-existing gap, out of scope for this change. void NodeImpl::readvertise_best() { if (m_peers.empty()) diff --git a/src/impl/btc/node.hpp b/src/impl/btc/node.hpp index 43f73ed42..37df4294d 100644 --- a/src/impl/btc/node.hpp +++ b/src/impl/btc/node.hpp @@ -377,9 +377,13 @@ class NodeImpl : public pool::SharechainNode& share_hashes); + /// Send a set of shares (with any needed txs) to a single peer, and return + /// the subset actually written (F2). Shares dropped by the tx-completeness + /// gate, or the whole batch when the tracker lock is unavailable, are + /// reported as NOT sent, so the caller leaves them un-marked and retries + /// them on the next broadcast. + std::vector send_shares(peer_ptr peer, + const std::vector& share_hashes); /// F3: retain a freshly-minted share's TEMPLATE tx set so every referenced /// new-tx is forwardable (remember_tx) and the share is backable. Called @@ -394,6 +398,19 @@ class NodeImpl : public pool::SharechainNode()> m_local_miner_scripts_fn; std::string m_node_payout_script_hex; std::set m_shared_share_hashes; // de-dup set for broadcast_share + std::atomic m_broadcast_deferred{0}; + std::atomic m_broadcast_acquired{0}; + std::atomic m_broadcast_reached_send{0}; std::set m_rejected_share_hashes; // shares rejected by peers — never re-broadcast std::set m_downloading_shares; // hashes currently being fetched diff --git a/src/impl/btc/share_tx_refs.hpp b/src/impl/btc/share_tx_refs.hpp new file mode 100644 index 000000000..1db302b5b --- /dev/null +++ b/src/impl/btc/share_tx_refs.hpp @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// Uniform access to a share's "new transaction hashes" list across the BTC +// share variants. +// +// WHY THIS EXISTS. The BTC share variants do NOT all spell this list the same +// way, and they do not all HAVE it: +// +// btc::Share (v17) -> obj->m_tx_info.m_new_transaction_hashes +// btc::NewShare (v33) -> obj->m_tx_info.m_new_transaction_hashes +// btc::SegwitMiningShare (v34) -> none (carries types::DataSegwitShare, no +// btc::PaddingBugfixShare (v35) m_tx_info member -- share_check leaves the +// btc::MergedMiningShare (v36) tx list empty for version >= 34) +// +// node.cpp's send_shares() F3 broadcast gate (partition_backable) probed +// `requires { obj->m_new_transaction_hashes; }` directly on the share object. +// That expression is FALSE for every one of the five variants -- v17/v33 nest +// the list inside m_tx_info, v34+ have no list -- so the gate collected an empty +// ref list for every share, every share was vacuously backable, and the gate +// no-op'd for ALL versions (#880): a v17/v33 share referencing a tx the peer +// lacked was broadcast anyway, tripping the canonical "referenced unknown +// transaction" disconnect. The per_share_refs relay block below the gate already +// spells the nested list correctly (#871), which is why relay worked and only +// the gate probe was dead. +// +// Probing through one accessor removes the class of bug: a new share variant +// either exposes the list in one of the two known spellings and is handled, or +// exposes none and is reported as "no list" (nothing to gate on) -- never +// silently dead because of a member-name mismatch. Mirrors bch::new_tx_hashes +// (src/impl/bch/share_tx_refs.hpp, #913) and ltc::new_tx_hashes +// (src/impl/ltc/share_tx_refs.hpp, #873); BTC's variant topology is identical. + +#pragma once + +#include + +#include + +namespace btc { + +// Pointer to the share's new-transaction-hash list, or nullptr when this share +// variant carries none. A pointer (not a copy) so the broadcast hot path does no +// per-share allocation. Lifetime is the share object's. +template +inline auto new_tx_hashes(ShareObj* obj) +{ + if constexpr (requires { obj->m_tx_info.m_new_transaction_hashes; }) + return &obj->m_tx_info.m_new_transaction_hashes; + else if constexpr (requires { obj->m_new_transaction_hashes; }) + return &obj->m_new_transaction_hashes; + else + return static_cast*>(nullptr); +} + +// Compile-time pin of the fact above: true iff the share variant exposes the +// list under the FLAT spelling that the old probe assumed. Held false by the KAT +// for every BTC variant -- if this ever becomes true, the flat branch of +// new_tx_hashes() is what handles it, not a silent skip. +template +inline constexpr bool has_flat_new_tx_hashes = + requires(ShareObj* o) { o->m_new_transaction_hashes; }; + +// True iff the share variant nests the list inside m_tx_info (v17 / v33). +template +inline constexpr bool has_nested_new_tx_hashes = + requires(ShareObj* o) { o->m_tx_info.m_new_transaction_hashes; }; + +} // namespace btc diff --git a/src/impl/btc/test/CMakeLists.txt b/src/impl/btc/test/CMakeLists.txt index 788167148..0ab6572a6 100644 --- a/src/impl/btc/test/CMakeLists.txt +++ b/src/impl/btc/test/CMakeLists.txt @@ -1,7 +1,7 @@ if (BUILD_TESTING AND GTest_FOUND) # btc twin of ltc share_test — uniquely named to avoid the CMP0002 target # collision with src/impl/ltc/test (both subdirs build in the same tree). - add_executable(btc_share_test share_test.cpp f11_donation_invariance_test.cpp f10b_version_punish_removal_test.cpp g01_share_format_parity_test.cpp auto_ratchet_sim_test.cpp block_broadcast_guard_test.cpp block_broadcast_connect_test.cpp regtest_sharechain_isolation_test.cpp stratum_merkle_branch_test.cpp witness_commitment_merkle_test.cpp finder_fee_integer_exact_test.cpp rpc_conf_test.cpp genesis_check_test.cpp won_share_dualpath_test.cpp gentx_unpack_test.cpp block_assembly_test.cpp gentx_coinbase_test.cpp template_capture_test.cpp template_other_txs_test.cpp reconstruct_test.cpp known_txs_retention_test.cpp won_block_mints_share_test.cpp) + add_executable(btc_share_test share_test.cpp f11_donation_invariance_test.cpp f10b_version_punish_removal_test.cpp g01_share_format_parity_test.cpp auto_ratchet_sim_test.cpp block_broadcast_guard_test.cpp block_broadcast_connect_test.cpp regtest_sharechain_isolation_test.cpp stratum_merkle_branch_test.cpp witness_commitment_merkle_test.cpp finder_fee_integer_exact_test.cpp rpc_conf_test.cpp genesis_check_test.cpp won_share_dualpath_test.cpp gentx_unpack_test.cpp block_assembly_test.cpp gentx_coinbase_test.cpp template_capture_test.cpp template_other_txs_test.cpp reconstruct_test.cpp known_txs_retention_test.cpp won_block_mints_share_test.cpp broadcast_gate_test.cpp broadcast_lock_discipline_test.cpp) target_link_libraries(btc_share_test PRIVATE GTest::gtest_main GTest::gtest core btc diff --git a/src/impl/btc/test/broadcast_gate_test.cpp b/src/impl/btc/test/broadcast_gate_test.cpp new file mode 100644 index 000000000..06be34d73 --- /dev/null +++ b/src/impl/btc/test/broadcast_gate_test.cpp @@ -0,0 +1,313 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +// --------------------------------------------------------------------------- +// F3 tx-completeness broadcast gate + F2 mark-only-what-was-sent — BTC KATs. +// +// Both primitives are the ones send_shares()/broadcast_share() in +// src/impl/btc/node.cpp actually call, so these KATs pin the shipped policy: +// +// partition_backable() — F3. send_shares must NOT write a share whose +// referenced new-tx BYTES we do not hold: canonical p2pool disconnects on +// "referenced unknown transaction" (p2p.py), which isolates us from the +// sharechain and orphans our shares. Pre-fix, send_shares looked the tx up +// and simply omitted it when absent (`if (it != m_known_txs.end())` with no +// else), sending the share anyway. +// +// broadcast_and_mark() — F2. broadcast_share may add a hash to +// m_shared_share_hashes only AFTER a peer accepted it. Pre-fix it marked the +// whole chain-walk up front, so a batch that send_shares then abandoned (F3 +// skip, tracker try_to_lock miss, zero peers) was withheld FOREVER: the next +// walk breaks on the first marked hash and nothing ever re-pushes it. +// +// Red-able: WalkStrandedWhenMarkedBeforeSend reproduces the pre-fix marking +// order side by side with the shipped one and asserts they differ — restoring +// mark-before-send makes the shipped half fail. GateWithholdsUnbackedShare +// asserts a skip count of 1; deleting the gate flips it red. +// +// Rides the already-allowlisted btc_share_test executable — no build.yml +// --target allowlist change, no NOT_BUILT sentinel risk. +// Consensus surface: NONE. These primitives decide only WHETHER a share is put +// on the wire; share bytes, minting and payout are untouched. +// --------------------------------------------------------------------------- + +#include + +#include +#include +#include +#include + +#include +#include +#include // real btc share variants + ShareType +#include // btc::new_tx_hashes SSOT (#880) + +namespace { + +uint256 h(const char* hex) { uint256 v; v.SetHex(hex); return v; } + +// Share hashes S1..S3 and tx hashes TA..TC. +const uint256 S1 = h("11"), S2 = h("22"), S3 = h("33"); +const uint256 TA = h("aa"), TB = h("bb"), TC = h("cc"); + +// Stand-in for a share on the broadcast path: its hash plus the new-tx hashes +// it references. The gate is agnostic to everything else in a share. +struct FakeShare { + uint256 hash; + std::vector new_txs; +}; + +std::vector refs_of(FakeShare& s) { return s.new_txs; } + +// A peer that accepts exactly the hashes send_shares would have written to it. +struct FakePeer { std::set accepts; }; + +} // namespace + +// F3: a share referencing a tx whose bytes we do not hold is removed from the +// outgoing batch; the backable ones survive, in order. +TEST(BtcBroadcastGate, GateWithholdsUnbackedShare) +{ + const std::set held{TA, TB}; // TC bytes NOT held + + std::vector batch{ + {S1, {TA}}, + {S2, {TC}}, // references an unheld tx -> must be withheld + {S3, {TA, TB}}, + }; + + const std::size_t skipped = btc::partition_backable(batch, held, refs_of); + + EXPECT_EQ(skipped, 1u); + ASSERT_EQ(batch.size(), 2u); + EXPECT_EQ(batch[0].hash, S1); + EXPECT_EQ(batch[1].hash, S3); // order preserved +} + +// F3: a share referencing no new txs is trivially backable (the common case for +// an empty-template share) — the gate must not withhold it. +TEST(BtcBroadcastGate, ShareWithNoTxRefsIsAlwaysSendable) +{ + const std::set held; // hold nothing at all + std::vector batch{{S1, {}}}; + + EXPECT_EQ(btc::partition_backable(batch, held, refs_of), 0u); + ASSERT_EQ(batch.size(), 1u); + EXPECT_EQ(batch[0].hash, S1); +} + +// F3: when every share is unbacked the batch empties, and send_shares reports +// nothing sent (which is what keeps F2 from marking them). +TEST(BtcBroadcastGate, WholeBatchUnbackedEmptiesTheSend) +{ + const std::set held; + std::vector batch{{S1, {TA}}, {S2, {TB}}}; + + EXPECT_EQ(btc::partition_backable(batch, held, refs_of), 2u); + EXPECT_TRUE(batch.empty()); +} + +// F2: a batch no peer accepted must leave the de-dup set untouched, so the next +// broadcast walk re-offers it. +TEST(BtcBroadcastMarking, NothingMarkedWhenNoPeerAccepted) +{ + std::set marked; + std::map peers{{1, {}}, {2, {}}}; + + const std::size_t n = btc::broadcast_and_mark( + marked, peers, std::vector{S1, S2}, + [](FakePeer&) { return std::vector{}; }); + + EXPECT_EQ(n, 0u); + EXPECT_TRUE(marked.empty()); +} + +// F2: with zero peers connected nothing is sent, therefore nothing is marked. +TEST(BtcBroadcastMarking, NothingMarkedWithZeroPeers) +{ + std::set marked; + std::map peers; + + EXPECT_EQ(btc::broadcast_and_mark(marked, peers, std::vector{S1, S2}, + [](FakePeer&) { return std::vector{}; }), + 0u); + EXPECT_TRUE(marked.empty()); +} + +// F2: exactly the union of what the peers accepted gets marked — no more. +TEST(BtcBroadcastMarking, OnlyAcceptedHashesAreMarked) +{ + std::set marked; + std::map peers{{1, {{S1}}}, {2, {{S1, S3}}}}; + + const std::size_t n = btc::broadcast_and_mark( + marked, peers, std::vector{S1, S2, S3}, [](FakePeer& p) { + return std::vector(p.accepts.begin(), p.accepts.end()); + }); + + EXPECT_EQ(n, 2u); + EXPECT_TRUE(marked.count(S1)); + EXPECT_TRUE(marked.count(S3)); + EXPECT_FALSE(marked.count(S2)); // withheld by the gate -> retryable +} + +// The regression itself. broadcast_share walks back from the tip and BREAKS on +// the first already-marked hash. Round 1: TB is missing so S2 is withheld and no +// peer accepts anything. Pre-fix (mark first) the walk is stranded forever; +// shipped (mark after send) it re-offers the batch in round 2, when TB arrives. +TEST(BtcBroadcastMarking, WalkStrandedWhenMarkedBeforeSend) +{ + // The tip-first walk broadcast_share performs, verbatim in its essentials. + auto walk = [](const std::vector& chain_tip_first, + const std::set& marked) { + std::vector to_send; + for (const auto& hash : chain_tip_first) { + if (marked.count(hash)) + break; + to_send.push_back(hash); + } + return to_send; + }; + + const std::vector chain{S2, S1}; // S2 is the tip + std::map one_peer{{1, {}}}; + + // ---- pre-fix ordering: mark the walk, THEN discover nothing was sent ---- + { + std::set marked; + auto to_send = walk(chain, marked); + ASSERT_EQ(to_send.size(), 2u); + for (const auto& hash : to_send) marked.insert(hash); // the bug + btc::broadcast_and_mark(marked, one_peer, to_send, + [](FakePeer&) { return std::vector{}; }); + + // Round 2, TB has since arrived and S2 is backable — but the walk is + // dead: the tip is marked, so nothing is ever re-pushed. Silent, + // permanent share loss with no retry path. + EXPECT_TRUE(walk(chain, marked).empty()); + } + + // ---- shipped ordering: mark only what a peer accepted -------------------- + { + std::set marked; + auto to_send = walk(chain, marked); + ASSERT_EQ(to_send.size(), 2u); + btc::broadcast_and_mark(marked, one_peer, to_send, + [](FakePeer&) { return std::vector{}; }); + EXPECT_TRUE(marked.empty()); + + // Round 2: the walk still yields the full batch, so the share is retried. + auto retry = walk(chain, marked); + ASSERT_EQ(retry.size(), 2u); + EXPECT_EQ(retry[0], S2); + + // ...and once a peer accepts them they are marked exactly once. + std::map live{{1, {{S1, S2}}}}; + btc::broadcast_and_mark(marked, live, retry, [](FakePeer& p) { + return std::vector(p.accepts.begin(), p.accepts.end()); + }); + EXPECT_EQ(marked.size(), 2u); + EXPECT_TRUE(walk(chain, marked).empty()); // now correctly de-duped + } +} + + +// --------------------------------------------------------------------------- +// #880 REGRESSION: the F3 gate above must be driven by the PRODUCTION probe, +// not a hand-rolled stand-in. The FakeShare KATs exercise partition_backable's +// MECHANICS with a top-level new_txs member and a bespoke refs_of -- exactly +// the shape that let the real bug hide. In production send_shares probed +// `requires { obj->m_new_transaction_hashes; }`, which is FALSE for every BTC +// share variant (the list is nested in m_tx_info), so the gate collected empty +// refs, every share was vacuously backable, and the gate silently no-op'd for +// ALL versions -- a share referencing a tx the peer lacked was broadcast anyway +// and tripped the canonical "referenced unknown transaction" disconnect. +// +// These KATs build REAL btc share types and drive them through the SAME +// btc::new_tx_hashes SSOT that node.cpp's partition_backable refs_of now uses. +// FAILS-BEFORE: revert btc::new_tx_hashes (or the gate) to the flat probe and +// the v17/v33 expectations collapse -- the unbacked share is no longer withheld. +// --------------------------------------------------------------------------- +namespace { + +// The exact refs_of node.cpp installs on partition_backable: route the variant +// through the btc::new_tx_hashes SSOT. +std::vector production_refs_of(btc::ShareType& share) +{ + std::vector hashes; + share.invoke([&](auto* obj) { + if (const auto* new_txs = btc::new_tx_hashes(obj)) + hashes.assign(new_txs->begin(), new_txs->end()); + }); + return hashes; +} + +} // namespace + +// Accessor SSOT: a REAL v17 share nests its new-tx hashes in m_tx_info; the +// production probe must surface them (the dead flat probe returned nullptr). +TEST(BtcBroadcastGate, RealV17ShareRefsResolvedViaSsot) +{ + btc::Share s; + s.m_tx_info.m_new_transaction_hashes = {TA, TC}; + + const auto* refs = btc::new_tx_hashes(&s); + ASSERT_NE(refs, nullptr); + ASSERT_EQ(refs->size(), 2u); // dead flat probe -> nullptr + EXPECT_EQ((*refs)[0], TA); + EXPECT_EQ((*refs)[1], TC); +} + +// Accessor SSOT: v33 uses the same nested carrier. +TEST(BtcBroadcastGate, RealV33ShareRefsResolvedViaSsot) +{ + btc::NewShare s; + s.m_tx_info.m_new_transaction_hashes = {TC}; + + const auto* refs = btc::new_tx_hashes(&s); + ASSERT_NE(refs, nullptr); + ASSERT_EQ(refs->size(), 1u); + EXPECT_EQ((*refs)[0], TC); +} + +// Accessor SSOT: v34/v35/v36 carry no m_tx_info -> nullptr, compiled out. +TEST(BtcBroadcastGate, SegwitAndMergedVariantsHaveNoRefs) +{ + btc::SegwitMiningShare v34; + btc::PaddingBugfixShare v35; + btc::MergedMiningShare v36; + EXPECT_EQ(btc::new_tx_hashes(&v34), nullptr); + EXPECT_EQ(btc::new_tx_hashes(&v35), nullptr); + EXPECT_EQ(btc::new_tx_hashes(&v36), nullptr); +} + +// The gate end-to-end: REAL share variants + the production refs_of through +// btc::partition_backable. A v17 share referencing a tx whose bytes we do NOT +// hold must be withheld; the backable ones survive, in order. Pre-fix the dead +// probe made every share vacuously backable -> skipped==0, nothing withheld. +TEST(BtcBroadcastGate, RealShareTxInfoRefsWithheld) +{ + std::vector raws; // own the heap shares for cleanup + auto v17 = [&](std::vector refs) { + auto* raw = new btc::Share(); + raw->m_tx_info.m_new_transaction_hashes = std::move(refs); + raws.push_back(raw); + btc::ShareType sv; sv = raw; return sv; + }; + + const std::set held{TA, TB}; // TC bytes NOT held + + std::vector batch; + batch.push_back(v17({TA})); // backable + batch.push_back(v17({TC})); // references unheld TC -> withheld + batch.push_back(v17({TA, TB})); // backable + + const std::size_t skipped = + btc::partition_backable(batch, held, production_refs_of); + + EXPECT_EQ(skipped, 1u); // dead-probe regression -> 0 + ASSERT_EQ(batch.size(), 2u); + EXPECT_EQ(production_refs_of(batch[0]).front(), TA); // order preserved + EXPECT_EQ(production_refs_of(batch[1]).front(), TA); + + for (auto* raw : raws) delete raw; +} diff --git a/src/impl/btc/test/broadcast_lock_discipline_test.cpp b/src/impl/btc/test/broadcast_lock_discipline_test.cpp new file mode 100644 index 000000000..6a910ec33 --- /dev/null +++ b/src/impl/btc/test/broadcast_lock_discipline_test.cpp @@ -0,0 +1,184 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// Wiring regression for btc::NodeImpl::broadcast_share, driven against a REAL +// btc::NodeImpl and a REAL populated sharechain. +// +// The bug this pins is not a decision-function bug and no KAT over the pure +// helpers (partition_backable / broadcast_and_mark, see broadcast_gate_test) +// can see it. broadcast_share opens with +// +// std::shared_lock lock(m_tracker_mutex, std::try_to_lock); +// +// and a std::shared_mutex REFUSES a shared lock to a thread that already holds +// it EXCLUSIVELY. The stratum mining-submit path in main_btc.cpp creates the +// local share while holding exactly that mutex under a unique_lock. If it were +// to call broadcast_share inside that scope, every broadcast of every locally +// minted share would take the "tracker busy — deferring" early return and this +// node would put none of its own shares on the wire — and everything downstream +// of that return (the F3 tx-completeness gate this PR #880 routes through the +// send path, the per-peer send, the mark-only-what-was-sent bookkeeping) would +// be unreachable code. A gate on dead code passes every test while gating +// nothing; this is the failure mode this trace exists to rule out. +// +// The production caller does the right thing: main_btc create_share_fn drops +// the exclusive lock (lk.unlock()) BEFORE calling broadcast_share, so the send +// loop is reachable. These cases hold that down from both sides: called under +// an exclusive lock on the calling thread nothing downstream runs; called +// without one the walk runs and the per-peer send loop is entered. A future +// refactor that moves broadcast_share back inside the lock scope reddens this. +// +// Folded into the EXISTING allowlisted btc_share_test target (a new +// add_executable would be absent from build.yml -- reported "Not Run" by CTest, +// the #769 trap). + +#include + +#include +#include +#include + +#include +#include +#include + +namespace { + +uint256 H(uint64_t n) { return uint256(n); } + +// Minimal concrete NodeImpl: the default btc::NodeImpl ctor takes no io_context, +// opens no LevelDB and starts no timers, so a unit test can own one. The +// io_context ctor is what normally points m_chain at the tracker chain; we do +// that by hand here. +struct TestNode : public btc::NodeImpl +{ + TestNode() : btc::NodeImpl() { m_chain = &m_tracker.chain; } + + // Satisfy core::ICommunicator: this unit test drives no sockets. + void handle(std::unique_ptr, const NetService&) override {} + + using btc::NodeImpl::broadcast_share; + using btc::NodeImpl::m_shared_share_hashes; + using btc::NodeImpl::m_tracker; + + // Append a v36 share (the version this lane mints — carries no new-tx list, + // so the F3 gate is a no-op for it) on top of `prev`. + void add_share(const uint256& hash, const uint256& prev) + { + auto* s = new btc::MergedMiningShare(hash, prev); + m_tracker.chain.add(s); + } +}; + +// A two-share chain: genesis (null parent) then a tip on top of it, so the +// 5-deep walk in broadcast_share has something to collect. +std::unique_ptr make_node_with_chain(uint256& tip_out) +{ + auto node = std::make_unique(); + node->add_share(H(1), uint256::ZERO); + node->add_share(H(2), H(1)); + tip_out = H(2); + return node; +} + +} // namespace + +TEST(BtcBroadcastLockDiscipline, ExclusiveLockOnCallingThreadBlocksTheEntireBroadcast) +{ + // THE defect this trace exists to rule out, reproduced exactly: the caller + // holds the tracker mutex exclusively and calls broadcast_share on the same + // thread. + uint256 tip; + auto node = make_node_with_chain(tip); + + { + std::unique_lock exclusive(node->tracker_mutex()); + ASSERT_TRUE(exclusive.owns_lock()); + + node->broadcast_share(tip); + + EXPECT_EQ(node->broadcast_deferred_count(), 1u); + EXPECT_EQ(node->broadcast_acquired_count(), 0u) + << "a shared_mutex cannot grant a shared lock to a thread already " + "holding it exclusively"; + EXPECT_EQ(node->broadcast_reached_send_count(), 0u) + << "nothing downstream of the try-lock runs — the F3 gate, the " + "per-peer send and the mark bookkeeping are all unreachable"; + } + + EXPECT_TRUE(node->m_shared_share_hashes.empty()); +} + +TEST(BtcBroadcastLockDiscipline, WithoutTheCallersLockTheBroadcastReachesTheSendLoop) +{ + // The main_btc create_share_fn shape: the submit path drops its exclusive + // lock (lk.unlock()) before calling broadcast_share, so by the time we get + // here no exclusive lock is held on this thread. + uint256 tip; + auto node = make_node_with_chain(tip); + + node->broadcast_share(tip); + + EXPECT_EQ(node->broadcast_deferred_count(), 0u); + EXPECT_EQ(node->broadcast_acquired_count(), 1u); + EXPECT_EQ(node->broadcast_reached_send_count(), 1u) + << "the walk produced a batch and the per-peer send loop was entered — " + "this is what stays at zero if the caller holds the exclusive lock"; +} + +TEST(BtcBroadcastLockDiscipline, DeferredThenRetriedSucceeds) +{ + // A deferral must be recoverable: the share is still in our chain and stays + // un-marked, so the next cycle picks it up. This is the property that makes + // the try-lock safe — and that an always-locked inline call site could + // never reach, because its lock would be held on every attempt. + uint256 tip; + auto node = make_node_with_chain(tip); + + { + std::unique_lock exclusive(node->tracker_mutex()); + node->broadcast_share(tip); + } + ASSERT_EQ(node->broadcast_reached_send_count(), 0u); + + node->broadcast_share(tip); + EXPECT_EQ(node->broadcast_deferred_count(), 1u); + EXPECT_EQ(node->broadcast_reached_send_count(), 1u); +} + +TEST(BtcBroadcastLockDiscipline, ZeroPeersMarksNothingOnTheRealNode) +{ + // F2 on the real node: the walk ran and the send loop was entered, but with + // no peers connected nothing reached a socket, so nothing may be marked + // broadcast. Marking at walk time would retire both shares here — the next + // walk breaks on a marked hash, with no retry path. + uint256 tip; + auto node = make_node_with_chain(tip); + + node->broadcast_share(tip); + + ASSERT_EQ(node->broadcast_reached_send_count(), 1u); + EXPECT_TRUE(node->m_shared_share_hashes.empty()) + << "no peer, no byte on the wire, therefore no mark"; + + // ...and because nothing was marked, a later attempt still has work to do. + node->broadcast_share(tip); + EXPECT_EQ(node->broadcast_reached_send_count(), 2u) + << "an unmarked share is retried, not silently retired"; +} + +TEST(BtcBroadcastLockDiscipline, NullAndUnknownShareHashesAreRejectedNotWalked) +{ + // Both guards must short-circuit before the chain walk: a null hash and a + // hash that was never added to our chain must not reach the send loop. + uint256 tip; + auto node = make_node_with_chain(tip); + + node->broadcast_share(uint256::ZERO); + EXPECT_EQ(node->broadcast_reached_send_count(), 0u); + + node->broadcast_share(H(9999)); // never added to the chain + EXPECT_EQ(node->broadcast_reached_send_count(), 0u); + + node->broadcast_share(tip); // still works for a live hash + EXPECT_EQ(node->broadcast_reached_send_count(), 1u); +} diff --git a/src/impl/dgb/known_txs_retention.hpp b/src/impl/dgb/known_txs_retention.hpp new file mode 100644 index 000000000..b6ce35a72 --- /dev/null +++ b/src/impl/dgb/known_txs_retention.hpp @@ -0,0 +1,231 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// Pure, unit-testable bookkeeping for the known-transaction pool that backs +// share broadcast (remember_tx forwarding). Ported to the DGB variant from +// src/impl/dash/known_txs_retention.hpp so the retention/eviction and +// broadcast-gate semantics can be exercised without a live node. +// +// Two concerns: +// retain_template_txs() — the rolling-window retention with template-identity +// dedup (code review F1): register_template_txs is invoked per producer-job +// cache MISS, i.e. per (prev_share_hash, payout_script), NOT per template +// rotation. A ~50-payout-script cluster therefore re-registers the SAME +// template ~50 times on one tip; pushing each unconditionally collapses the +// N-slot window to a single template within seconds. Dedup by the template's +// tx-hash SET (its identity): a set already retained refreshes recency and +// consumes NO new slot, so N re-registrations of one template use ONE slot +// and the window holds N DISTINCT templates as intended. +// +// all_txs_backable() — the canonical broadcast gate (F3): a share may be sent +// only when we HOLD the bytes of every referenced new-tx (so remember_tx can +// always forward them). This is what makes the "referenced unknown +// transaction" disconnect unreachable by construction, and — paired with the +// mint-time decline of shares whose txs are not retained — makes "every share +// we mint/broadcast is backable" a by-construction invariant. +// +// partition_backable() — the batch form of the gate, applied by send_shares() +// to the shares it is about to write to one peer. +// +// broadcast_and_mark() — the F2 marking policy: broadcast_share() may mark a +// share "already shared" only AFTER a peer actually accepted it. Marking the +// whole chain-walk up front (the pre-fix pattern) loses every share in a +// batch that send_shares then abandons — the gate skipped it, the tracker +// try_to_lock missed, or there were no peers — because the de-dup set then +// breaks the walk forever and nothing re-pushes it. +// +// readvertise_and_record() — DGB-ONLY. The marking discipline for the +// advertise-only ROOT-2 re-push, which is DGB's LIVE path (see below). +// +// DGB REACHABILITY — differs from btc/bch; read this before trusting the file. +// main_dgb.cpp binds NO create/mint-share fn, so dgb::NodeImpl::broadcast_share +// and notify_local_share have ZERO callers repo-wide: DGB never mints a local +// share today (tracked as #884). The live DGB path into send_shares is +// readvertise_best_share() (ROOT-2 re-advert, on best-change and on a timer), +// which re-pushes PEER-RECEIVED shares and deliberately bypasses the de-dup +// set. Therefore, on DGB: +// partition_backable() — LIVE (send_shares, reached via readvertise) +// readvertise_and_record() — LIVE (readvertise_best_share) +// broadcast_and_mark() — PRE-WIRED; dead until #884 binds the mint seam. +// Anything asserting the DGB gate through a locally-minted share broadcast is +// exercising a path production never takes. +// +// NOTE (de-dup): the same primitives now exist per-coin (btc/dgb/bch/dash) while +// the LTC lane hoists all_txs_backable into src/core/. Once both land, these +// per-coin headers should be collapsed onto that core hoist. + +#pragma once + +#include +#include +#include +#include + +#include + +namespace dgb { + +// Retain the tx set of a newly-registered template in a rolling window of the +// last `cap` DISTINCT templates, inserting its tx bytes into `known_txs`. +// +// recent_sets : rolling history of distinct template tx-hash sets +// (front = oldest, back = newest). +// known_txs : uint256 -> tx map; needs insert_or_assign(k,v) and erase(k). +// hashes/txs : parallel arrays of the template's tx hashes and bytes. +// cap : number of DISTINCT templates to retain. +// +// A tx is erased from `known_txs` only once it has fallen out of EVERY retained +// set. F1 dedup: if this template's tx set is already retained, its recency is +// refreshed (moved to back) and no new slot is consumed. +template +inline void retain_template_txs(std::deque>& recent_sets, + TxMap& known_txs, + const std::vector& hashes, + const std::vector& txs, + std::size_t cap) +{ + if (cap == 0) + return; + + std::set new_set(hashes.begin(), hashes.end()); + + // F1 dedup by template identity (the tx-hash set). If this template is + // already retained, refresh its recency (move to back) — no new slot, no + // re-eviction. This is what stops ~50 payout-script re-registrations of one + // template from evicting the rest of the window. + for (auto it = recent_sets.begin(); it != recent_sets.end(); ++it) { + if (*it == new_set) { + if (std::next(it) != recent_sets.end()) { + std::set refreshed = std::move(*it); + recent_sets.erase(it); + recent_sets.push_back(std::move(refreshed)); + } + return; + } + } + + // Distinct template: insert its tx bytes and consume a slot. + const std::size_t n = std::min(hashes.size(), txs.size()); + for (std::size_t i = 0; i < n; ++i) + known_txs.insert_or_assign(hashes[i], txs[i]); + recent_sets.push_back(std::move(new_set)); + + // Evict the oldest slots beyond `cap`; a tx is removed from `known_txs` only + // once it is absent from EVERY remaining retained set. + while (recent_sets.size() > cap) { + const std::set evicted = std::move(recent_sets.front()); + recent_sets.pop_front(); + for (const auto& h : evicted) { + bool still_retained = false; + for (const auto& s : recent_sets) { + if (s.count(h)) { still_retained = true; break; } + } + if (!still_retained) + known_txs.erase(h); + } + } +} + +// True iff every hash in `tx_hashes` is present in `held` (the bytes we hold, +// e.g. m_known_txs). The canonical broadcast gate: a share is backable — safe to +// send without risking the peer's "referenced unknown transaction" disconnect — +// only when we can forward every referenced tx. +template +inline bool all_txs_backable(const std::vector& tx_hashes, + const Held& held) +{ + for (const auto& h : tx_hashes) + if (held.find(h) == held.end()) + return false; + return true; +} + +// Batch form of the F3 gate. Keeps in `shares` only the entries every one of +// whose referenced new-tx hashes we hold, and returns how many were dropped. +// `hashes_of(share)` yields that share's referenced new-tx hashes. +// +// Skipping costs no propagation: a share we downloaded is by definition already +// on the network we fetched it from, and it will be re-offered on a later +// broadcast once its tx bytes arrive (F2 leaves it un-marked). +template +inline std::size_t partition_backable(std::vector& shares, + const Held& held, + HashesOf hashes_of) +{ + std::vector sendable; + sendable.reserve(shares.size()); + std::size_t skipped = 0; + for (auto& share : shares) { + if (all_txs_backable(hashes_of(share), held)) + sendable.push_back(std::move(share)); + else + ++skipped; + } + shares.swap(sendable); + return skipped; +} + +// F2 marking policy. Offers the batch to every peer via `send(peer)` — which +// returns the hashes it ACTUALLY wrote to that peer — and marks in `marked` +// only the hashes that reached at least one peer. Returns that count. +// +// The pre-fix code marked the whole chain-walk before calling send_shares, so a +// batch abandoned by every peer (gate skip, try_to_lock miss, empty peer set) +// stayed marked forever: the next broadcast_share walk breaks on the first +// marked hash and the share is never re-pushed. That is silent, permanent share +// loss with no retry path — a PPLNS-credit loss for whoever mined it. +template +inline std::size_t broadcast_and_mark(MarkedSet& marked, Peers& peers, + const std::vector& to_send, + SendFn send) +{ + if (to_send.empty()) + return 0; + + // NOTE: `to_send` is deliberately NOT what gets marked. Marking the offered + // batch here is precisely the pre-fix bug this signature exists to make + // un-writable by accident — only `send`'s report may be marked. + std::set actually_sent; + for (auto& entry : peers) { + const std::vector sent = send(entry.second); + actually_sent.insert(sent.begin(), sent.end()); + } + for (const auto& h : actually_sent) + marked.insert(h); + return actually_sent.size(); +} + +// Advertise-only ROOT-2 re-push (readvertise_best_share) — DGB's LIVE marking +// path. Offers `to_send` to every peer via `send`, which reports what it +// ACTUALLY wrote, and records that report — never the offered batch — through +// `record(peer, sent)`. A peer that received nothing gets NO record at all. +// Returns the number of peers reached. +// +// Why the recording rule is reward-critical and not cosmetic: m_last_broadcast_to +// is what converts a peer disconnect within 10s into m_rejected_share_hashes +// entries, and the readvertise walk `continue`s past every rejected hash — +// permanently. Recording the OFFERED batch would let a share the F3 completeness +// gate merely withheld be blamed for someone else's disconnect, turning a +// transient, self-healing skip into permanent exclusion from every future +// re-advert. DGB re-advertises PEER-RECEIVED shares, so that is a propagation +// loss for the whole sharechain, not only for us. +template +inline std::size_t readvertise_and_record(Peers& peers, + const std::vector& to_send, + SendFn send, + RecordFn record) +{ + if (to_send.empty()) + return 0; + + std::size_t peers_reached = 0; + for (auto& entry : peers) { + const std::vector sent = send(entry.second); + if (sent.empty()) + continue; // nothing written -> nothing a later drop may blame + record(entry.second, sent); + ++peers_reached; + } + return peers_reached; +} + +} // namespace dgb diff --git a/src/impl/dgb/node.cpp b/src/impl/dgb/node.cpp index 68cef923a..c939209ad 100644 --- a/src/impl/dgb/node.cpp +++ b/src/impl/dgb/node.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: AGPL-3.0-or-later #include "node.hpp" +#include "known_txs_retention.hpp" // dgb::all_txs_backable / partition_backable / broadcast_and_mark (F2/F3) #include #include @@ -660,8 +661,13 @@ std::vector NodeImpl::handle_get_share(std::vector hash return shares; } -void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hashes) +std::vector NodeImpl::send_shares(peer_ptr peer, + const std::vector& share_hashes) { + // Returns the hashes of the shares actually WRITTEN to this peer (F2). The + // caller marks only those as broadcast, so a share skipped here — by the + // try_to_lock miss below or by the F3 tx-completeness gate — is re-offered + // on the next broadcast instead of being silently dropped forever. // try_to_lock per the architectural rule (node.hpp:67) — see freeze // analysis in handle_get_share above. If we can't acquire NOW, skip // this batch. The shares are still in our chain; the next broadcast @@ -673,7 +679,7 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash if (defer_log++ % 50 == 0) LOG_INFO << "[send_shares] tracker busy — skipping send to " << peer->addr().to_string() << " (will retry next cycle)"; - return; + return {}; } // Collect shares that exist in our chain (skip rejected) @@ -693,7 +699,50 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash } if (shares.empty()) - return; + return {}; + + // --- F3 tx-completeness broadcast gate -------------------------------- + // A share whose referenced new-tx BYTES we do not hold cannot be backed by + // the remember_tx forward below, and canonical p2pool drops the connection + // on "referenced unknown transaction" (p2p.py) — which isolates us from the + // sharechain and orphans our shares. Shares pulled via sharereply arrive + // WITHOUT their tx bytes, so this is reachable in normal operation. + // + // Gate on the BYTES in m_known_txs, not on the peer's have_tx advert: that + // advert can be stale (the peer may have dropped the tx) and sending + // hash-only for a tx the peer no longer holds is exactly the disconnect. + // m_remote_txs is used ONLY below to choose the hash-vs-bytes encoding. + // + // CONSENSUS-NEUTRAL: this decides only WHETHER we broadcast. Share bytes, + // minting and payout are untouched; a skipped share stays in our chain and + // is re-offered once its txs arrive (send_shares reports it as unsent, so + // broadcast_share leaves it un-marked). + { + const size_t skipped_incomplete = partition_backable( + shares, m_known_txs, [](ShareType& share) { + std::vector hashes; + share.invoke([&](auto* obj) { + // #880: route through the dgb::append_share_tx_refs SSOT + // (#914) instead of the dead flat probe -- the refs live in + // m_tx_info on v17/v33, absent v34+. The old probe was FALSE + // for every version, so this F3 gate never withheld a share. + dgb::append_share_tx_refs(obj, hashes); + }); + return hashes; + }); + if (skipped_incomplete > 0) + { + static int skip_log = 0; + if (skip_log++ % 20 == 0) + LOG_INFO << "[send_shares] skipped " << skipped_incomplete + << " share(s) whose new-tx bytes we do not hold " + "(downloaded via sharereply / evicted) to " + << peer->addr().to_string() + << " — avoids canonical 'unknown transaction' disconnect"; + } + if (shares.empty()) + return {}; + } // Collect transactions that the peer doesn't know about. A share's // referenced new-tx hashes live INSIDE m_tx_info (dgb::ShareTxInfo) on @@ -722,6 +771,8 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash std::vector known_hashes; // hashes in peer's remote set std::vector full_txs; // full txs otherwise + size_t missing = 0; + for (const auto& th : needed_txs) { if (peer->m_remote_txs.count(th)) @@ -733,9 +784,20 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash auto it = m_known_txs.find(th); if (it != m_known_txs.end()) full_txs.emplace_back(it->second); + else + ++missing; // unreachable after the gate above — see below } } + // Defence in depth: the gate should already have removed every share + // with an unheld tx, so this must stay at zero. If it ever fires, the + // gate's view of the referenced hashes disagrees with needed_txs and + // the peer may drop these shares — make that loud rather than silent. + if (missing > 0) + LOG_WARNING << "[send_shares] " << missing << " referenced tx(s) not in " + "m_known_txs after the completeness gate — peer may drop " + "these shares (share/tx hash accounting mismatch?)"; + if (!known_hashes.empty() || !full_txs.empty()) { auto rtx_msg = message_remember_tx::make_raw(known_hashes, full_txs); @@ -772,8 +834,21 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector& share_hash LOG_INFO << "[Pool] Sent " << shares.size() << " shares (+" << needed_txs.size() << " txs) to " << peer->addr().to_string(); + + // F2: report exactly which shares reached this peer. + std::vector sent; + sent.reserve(shares.size()); + for (auto& share : shares) + sent.push_back(share.hash()); + return sent; } +// PRE-WIRING — NOT REACHED ON DGB TODAY (#884). main_dgb.cpp binds no +// create/mint-share fn, so this function and notify_local_share have zero +// callers repo-wide: DGB never mints a local share. The F2 mark-after-send fix +// below is applied so the seam is correct on the day #884 binds the mint path, +// and so btc/dgb/bch stay symmetric — it protects nothing at runtime yet. DGB's +// LIVE send path is readvertise_best_share() above. void NodeImpl::broadcast_share(const uint256& share_hash) { // try_to_lock per the architectural rule (node.hpp:67) — see freeze @@ -794,7 +869,14 @@ void NodeImpl::broadcast_share(const uint256& share_hash) return; } - // Walk the chain back from share_hash, collecting un-broadcast shares + // Walk the chain back from share_hash, collecting un-broadcast shares. + // + // F2: do NOT mark them shared here. send_shares can abandon the whole batch + // — the tx-completeness gate skipped it, its tracker try_to_lock missed, or + // there are zero peers — and a share marked before that never gets another + // chance: the next walk breaks on the first marked hash, so the tip is + // permanently withheld from the sharechain and its PPLNS credit is lost. + // We mark only what actually reached >= 1 peer, below. std::vector to_send; int32_t height = m_chain->get_height(share_hash); int32_t walk = std::min(height, 5); @@ -805,7 +887,6 @@ void NodeImpl::broadcast_share(const uint256& share_hash) break; if (m_rejected_share_hashes.count(hash)) continue; // skip shares previously rejected by peers - m_shared_share_hashes.insert(hash); to_send.push_back(hash); } @@ -813,10 +894,16 @@ void NodeImpl::broadcast_share(const uint256& share_hash) return; auto now = std::chrono::steady_clock::now(); - for (auto& [nonce, peer] : m_peers) { - send_shares(peer, to_send); - m_last_broadcast_to[peer->addr()] = {to_send, now}; - } + broadcast_and_mark(m_shared_share_hashes, m_peers, to_send, [&](peer_ptr& peer) { + std::vector sent = send_shares(peer, to_send); + // Record what we ACTUALLY wrote, not what we offered: a peer that drops + // within 10s marks this record's hashes rejected, and a rejected hash is + // never re-broadcast. Blaming a share we withheld would turn a transient + // gate skip into permanent loss. + if (!sent.empty()) + m_last_broadcast_to[peer->addr()] = {sent, now}; + return sent; + }); } void NodeImpl::notify_local_share(const uint256& share_hash) @@ -1037,11 +1124,19 @@ void NodeImpl::readvertise_best_share() if (to_send.empty()) return; + // Advertise-only path: deliberately ignores the de-dup set, so nothing is + // marked here. But record only what was ACTUALLY written — a share the F3 + // completeness gate withheld must not be blamed for a later peer drop, which + // would mark it rejected and make the walk above `continue` past it forever. + // This is DGB's LIVE reward-critical marking path; broadcast_share below is + // dead until the mint seam is bound (#884). auto now = std::chrono::steady_clock::now(); - for (auto& [nonce, peer] : m_peers) { - send_shares(peer, to_send); - m_last_broadcast_to[peer->addr()] = {to_send, now}; - } + readvertise_and_record( + m_peers, to_send, + [&](peer_ptr& peer) { return send_shares(peer, to_send); }, + [&](peer_ptr& peer, const std::vector& sent) { + m_last_broadcast_to[peer->addr()] = {sent, now}; + }); LOG_INFO << "[readvertise] re-pushed " << to_send.size() << " head share(s) to " << m_peers.size() << " peer(s) (ROOT-2)"; } diff --git a/src/impl/dgb/node.hpp b/src/impl/dgb/node.hpp index 86b785d4d..abe14fb26 100644 --- a/src/impl/dgb/node.hpp +++ b/src/impl/dgb/node.hpp @@ -382,9 +382,13 @@ class NodeImpl : public pool::SharechainNode& share_hashes); + /// Send a set of shares (with any needed txs) to a single peer, and return + /// the subset actually written (F2). Shares dropped by the tx-completeness + /// gate, or the whole batch when the tracker lock is unavailable, are + /// reported as NOT sent, so the caller leaves them un-marked and retries + /// them on the next broadcast. + std::vector send_shares(peer_ptr peer, + const std::vector& share_hashes); /// Broadcast a locally-generated (or newly-received) share to all peers. void broadcast_share(const uint256& share_hash); diff --git a/src/impl/dgb/test/CMakeLists.txt b/src/impl/dgb/test/CMakeLists.txt index 3dda7a383..2ebffe1b3 100644 --- a/src/impl/dgb/test/CMakeLists.txt +++ b/src/impl/dgb/test/CMakeLists.txt @@ -3,7 +3,11 @@ # verified into a test executable. Gated on BUILD_TESTING + GTest like the # sibling coins; registered via gtest_add_tests for the CI --target allowlist. if (BUILD_TESTING AND GTest_FOUND) - add_executable(dgb_share_test share_test.cpp) + # Rides dgb_share_test (already in the build.yml --target allowlist) so the + # F1 retention + F2/F3 broadcast-gate KATs are actually compiled and run; + # a fresh target would need a build.yml entry and would otherwise report + # the silently-passing NOT_BUILT sentinel. + add_executable(dgb_share_test share_test.cpp known_txs_retention_test.cpp broadcast_gate_test.cpp) target_link_libraries(dgb_share_test PRIVATE GTest::gtest_main GTest::gtest core dgb diff --git a/src/impl/dgb/test/broadcast_gate_test.cpp b/src/impl/dgb/test/broadcast_gate_test.cpp new file mode 100644 index 000000000..5af1c13f7 --- /dev/null +++ b/src/impl/dgb/test/broadcast_gate_test.cpp @@ -0,0 +1,248 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +// --------------------------------------------------------------------------- +// F3 tx-completeness broadcast gate + broadcast marking discipline — DGB KATs. +// +// READ FIRST — DGB REACHABILITY. DGB is NOT symmetric with btc/bch here, and +// these KATs are deliberately scoped to the path production actually takes: +// +// main_dgb.cpp binds NO create/mint-share fn, so dgb::NodeImpl::broadcast_share +// and notify_local_share have ZERO callers repo-wide — DGB never mints a local +// share today (tracked as #884). The LIVE DGB path into send_shares is +// readvertise_best_share() (ROOT-2 re-advert, fired on best-change and on a +// timer), which re-pushes PEER-RECEIVED shares and deliberately bypasses the +// broadcast de-dup set. +// +// LIVE on DGB: partition_backable() (send_shares, via readvertise) +// readvertise_and_record() (readvertise_best_share) +// PRE-WIRED, dead: broadcast_and_mark() (broadcast_share; #884) +// +// So the reward-critical assertions below drive readvertise_and_record, NOT +// broadcast_and_mark. A DGB test that proved the gate by broadcasting a locally +// minted share would be exercising a path production never takes — the exact +// shape of the LTC #873 miss, where both the test AND its negative control +// passed against an unreachable function. +// +// The pre-wired broadcast_and_mark coverage is kept but named +// DgbBroadcastMarkingPrewired_NotReachedToday so a reader cannot mistake it for +// live protection; it exists so the seam is correct on the day #884 lands. +// +// Rides the already-allowlisted dgb_share_test executable — no build.yml +// --target allowlist change, no NOT_BUILT sentinel risk. +// Consensus surface: NONE. These primitives decide only WHETHER a share is put +// on the wire; share bytes, minting and payout are untouched. +// --------------------------------------------------------------------------- + +#include + +#include +#include +#include + +#include +#include + +namespace { + +uint256 h(const char* hex) { uint256 v; v.SetHex(hex); return v; } + +// Share hashes S1..S3 and tx hashes TA..TC. +const uint256 S1 = h("11"), S2 = h("22"), S3 = h("33"); +const uint256 TA = h("aa"), TB = h("bb"), TC = h("cc"); + +// Stand-in for a share on the broadcast path: its hash plus the new-tx hashes +// it references. The gate is agnostic to everything else in a share. +struct FakeShare { + uint256 hash; + std::vector new_txs; +}; + +std::vector refs_of(FakeShare& s) { return s.new_txs; } + +// A peer that accepts exactly the hashes send_shares would have written to it. +struct FakePeer { std::set accepts; }; + +// The readvertise_best_share tip walk, verbatim in its essentials: it does NOT +// consult the de-dup set, but it DOES skip every peer-rejected hash — and that +// skip is permanent, which is what makes the recording rule reward-critical. +std::vector readvertise_walk(const std::vector& chain_tip_first, + const std::set& rejected) +{ + std::vector to_send; + for (const auto& hash : chain_tip_first) { + if (rejected.count(hash)) + continue; + to_send.push_back(hash); + } + return to_send; +} + +} // namespace + +// =========================== LIVE PATH: F3 gate ============================= +// Reached in production via readvertise_best_share -> send_shares. + +// A share referencing a tx whose bytes we do not hold is removed from the +// outgoing batch; the backable ones survive, in order. +TEST(DgbBroadcastGate, GateWithholdsUnbackedShare) +{ + const std::set held{TA, TB}; // TC bytes NOT held + + std::vector batch{ + {S1, {TA}}, + {S2, {TC}}, // references an unheld tx -> must be withheld + {S3, {TA, TB}}, + }; + + const std::size_t skipped = dgb::partition_backable(batch, held, refs_of); + + EXPECT_EQ(skipped, 1u); + ASSERT_EQ(batch.size(), 2u); + EXPECT_EQ(batch[0].hash, S1); + EXPECT_EQ(batch[1].hash, S3); // order preserved +} + +// A share referencing no new txs is trivially backable (the common case for an +// empty-template share) — the gate must not withhold it. +TEST(DgbBroadcastGate, ShareWithNoTxRefsIsAlwaysSendable) +{ + const std::set held; // hold nothing at all + std::vector batch{{S1, {}}}; + + EXPECT_EQ(dgb::partition_backable(batch, held, refs_of), 0u); + ASSERT_EQ(batch.size(), 1u); + EXPECT_EQ(batch[0].hash, S1); +} + +// When every share is unbacked the batch empties, so send_shares reports nothing +// written — which is what keeps the readvertise record empty below. +TEST(DgbBroadcastGate, WholeBatchUnbackedEmptiesTheSend) +{ + const std::set held; + std::vector batch{{S1, {TA}}, {S2, {TB}}}; + + EXPECT_EQ(dgb::partition_backable(batch, held, refs_of), 2u); + EXPECT_TRUE(batch.empty()); +} + +// ================= LIVE PATH: readvertise recording discipline ============== +// Reached in production via readvertise_best_share (best-change + timer). + +// The DGB regression. readvertise offers three shares; the F3 gate withholds S2 +// because we lack its tx bytes. The per-peer record must contain ONLY what was +// written, because that record is what a peer disconnect within 10s converts +// into m_rejected_share_hashes — and the readvertise walk skips rejected hashes +// PERMANENTLY. Recording the offered batch would blame S2 for someone else's +// drop and exclude it from every future re-advert. +TEST(DgbReadvertiseRecording, WithheldShareIsNeverBlamedForAPeerDrop) +{ + const std::vector offered{S1, S2, S3}; + // The peer receives the gated batch: S2 was withheld by partition_backable. + std::map peers{{1, {{S1, S3}}}}; + + std::vector recorded; + bool recorded_any = false; + const std::size_t reached = dgb::readvertise_and_record( + peers, offered, + [](FakePeer& p) { + return std::vector(p.accepts.begin(), p.accepts.end()); + }, + [&](FakePeer&, const std::vector& sent) { + recorded = sent; + recorded_any = true; + }); + + EXPECT_EQ(reached, 1u); + ASSERT_TRUE(recorded_any); + const std::set rec(recorded.begin(), recorded.end()); + EXPECT_TRUE(rec.count(S1)); + EXPECT_TRUE(rec.count(S3)); + EXPECT_FALSE(rec.count(S2)) << "withheld share must not be in the record"; + + // The peer now drops inside the 10s window: everything recorded for it is + // marked rejected. S2 must survive that, and stay eligible for re-advert. + const auto retry = readvertise_walk({S3, S2, S1}, rec); + ASSERT_EQ(retry.size(), 1u); + EXPECT_EQ(retry[0], S2) << "S2 must still be re-advertisable after the drop"; +} + +// A peer that received NOTHING gets no record at all, so a later drop has +// nothing to blame — the whole batch stays eligible for the next re-advert. +TEST(DgbReadvertiseRecording, PeerThatReceivedNothingIsNotRecorded) +{ + const std::vector offered{S1, S2}; + std::map peers{{1, {}}, {2, {}}}; + + std::size_t records = 0; + const std::size_t reached = dgb::readvertise_and_record( + peers, offered, + [](FakePeer&) { return std::vector{}; }, + [&](FakePeer&, const std::vector&) { ++records; }); + + EXPECT_EQ(reached, 0u); + EXPECT_EQ(records, 0u); + + // Nothing rejected -> the next walk still offers the whole batch. + EXPECT_EQ(readvertise_walk({S2, S1}, /*rejected=*/{}).size(), 2u); +} + +// With zero peers connected nothing is sent and nothing is recorded. +TEST(DgbReadvertiseRecording, ZeroPeersRecordsNothing) +{ + std::map peers; + std::size_t records = 0; + EXPECT_EQ(dgb::readvertise_and_record( + peers, std::vector{S1}, + [](FakePeer&) { return std::vector{}; }, + [&](FakePeer&, const std::vector&) { ++records; }), + 0u); + EXPECT_EQ(records, 0u); +} + +// An empty offer is a no-op: readvertise_best_share returns before the peer loop. +TEST(DgbReadvertiseRecording, EmptyOfferIsANoOp) +{ + std::map peers{{1, {{S1}}}}; + std::size_t sends = 0, records = 0; + EXPECT_EQ(dgb::readvertise_and_record( + peers, std::vector{}, + [&](FakePeer&) { ++sends; return std::vector{S1}; }, + [&](FakePeer&, const std::vector&) { ++records; }), + 0u); + EXPECT_EQ(sends, 0u); + EXPECT_EQ(records, 0u); +} + +// ============ PRE-WIRED (NOT REACHED ON DGB TODAY — see #884) =============== +// broadcast_and_mark backs dgb::NodeImpl::broadcast_share, which has zero +// callers because main_dgb.cpp binds no mint seam. These KATs pin the seam so it +// is correct the day #884 binds it; they assert NOTHING about DGB today. The +// live equivalents are the DgbReadvertiseRecording cases above. + +TEST(DgbBroadcastMarkingPrewired_NotReachedToday, NothingMarkedWhenNoPeerAccepted) +{ + std::set marked; + std::map peers{{1, {}}, {2, {}}}; + + const std::size_t n = dgb::broadcast_and_mark( + marked, peers, std::vector{S1, S2}, + [](FakePeer&) { return std::vector{}; }); + + EXPECT_EQ(n, 0u); + EXPECT_TRUE(marked.empty()); +} + +TEST(DgbBroadcastMarkingPrewired_NotReachedToday, OnlyAcceptedHashesAreMarked) +{ + std::set marked; + std::map peers{{1, {{S1}}}, {2, {{S1, S3}}}}; + + const std::size_t n = dgb::broadcast_and_mark( + marked, peers, std::vector{S1, S2, S3}, [](FakePeer& p) { + return std::vector(p.accepts.begin(), p.accepts.end()); + }); + + EXPECT_EQ(n, 2u); + EXPECT_TRUE(marked.count(S1)); + EXPECT_TRUE(marked.count(S3)); + EXPECT_FALSE(marked.count(S2)); // withheld by the gate -> retryable +} diff --git a/src/impl/dgb/test/known_txs_retention_test.cpp b/src/impl/dgb/test/known_txs_retention_test.cpp new file mode 100644 index 000000000..664404cfe --- /dev/null +++ b/src/impl/dgb/test/known_txs_retention_test.cpp @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +// F3 backable-broadcast gate + F1 template-identity retention — DGB unit KAT. +// +// Covers the two pure primitives ported from the DASH variant into +// src/impl/dgb/known_txs_retention.hpp: +// +// retain_template_txs() — F1 template-identity dedup: N re-registrations of +// ONE template consume ONE slot while the rolling window still holds N +// DISTINCT templates; a tx leaves known_txs only once it is absent from +// EVERY retained set. +// +// all_txs_backable() — F3 broadcast gate: a share is backable (safe to send +// without the peer's "referenced unknown transaction" disconnect) only when +// we hold the bytes of every referenced new-tx. +// +// Non-hollow guard: DgbKnownTxsBackable.UnheldTxIsNotBackable asserts false for a share +// referencing an unheld tx; deleting the gate (all_txs_backable → always true) +// flips it red. DgbKnownTxsRetention.OneTemplateConsumesOneSlot asserts size==1 after N +// re-registrations; dropping the dedup (push every registration) flips it red. + +#include + +#include +#include +#include +#include + +#include +#include + +namespace { + +// Distinct, deterministic uint256 tx hashes A..E. +uint256 h(const char* hex) +{ + uint256 v; v.SetHex(hex); return v; +} +const uint256 A = h("aa"); +const uint256 B = h("bb"); +const uint256 C = h("cc"); +const uint256 D = h("dd"); +const uint256 E = h("ee"); + +// A tx-bytes stand-in; the retention logic is agnostic to the payload type. +using TxMap = std::map; + +} // namespace + +// F1: re-registering the SAME template (identical tx-hash set) N times consumes +// exactly ONE slot and refreshes recency — it must NOT collapse the window. +TEST(DgbKnownTxsRetention, OneTemplateConsumesOneSlot) +{ + std::deque> recent; + TxMap known; + + const std::vector hashes{A, B}; + const std::vector txs{1, 2}; + + for (int i = 0; i < 5; ++i) + dgb::retain_template_txs(recent, known, hashes, txs, /*cap=*/3); + + EXPECT_EQ(recent.size(), 1u); // dedup: 5 registrations -> 1 slot + EXPECT_EQ(known.size(), 2u); // A,B inserted once + EXPECT_TRUE(known.count(A)); + EXPECT_TRUE(known.count(B)); +} + +// F1: the window holds N DISTINCT templates, and a tx is evicted only once it +// has fallen out of EVERY retained set (B survives in T2 after T1 is evicted). +TEST(DgbKnownTxsRetention, WindowHoldsDistinctTemplatesAndEvictsCorrectly) +{ + std::deque> recent; + TxMap known; + const std::vector two{1, 2}; + + dgb::retain_template_txs(recent, known, std::vector{A, B}, two, 3); // T1 + dgb::retain_template_txs(recent, known, std::vector{B, C}, two, 3); // T2 + dgb::retain_template_txs(recent, known, std::vector{C, D}, two, 3); // T3 + EXPECT_EQ(recent.size(), 3u); + for (const auto& x : {A, B, C, D}) EXPECT_TRUE(known.count(x)) << "missing before evict"; + + // T4 pushes the window past cap=3 -> oldest (T1={A,B}) evicted. + dgb::retain_template_txs(recent, known, std::vector{D, E}, two, 3); // T4 + EXPECT_EQ(recent.size(), 3u); + EXPECT_FALSE(known.count(A)); // A only lived in T1 -> erased + EXPECT_TRUE(known.count(B)); // B still retained via T2 -> kept + EXPECT_TRUE(known.count(C)); + EXPECT_TRUE(known.count(D)); + EXPECT_TRUE(known.count(E)); +} + +// F3: a share whose referenced new-txs we all hold is backable; one unheld tx +// makes it NOT backable (must not be broadcast). +TEST(DgbKnownTxsBackable, UnheldTxIsNotBackable) +{ + const std::set held{A, B}; + + EXPECT_TRUE(dgb::all_txs_backable(std::vector{A, B}, held)); // all held -> send + EXPECT_FALSE(dgb::all_txs_backable(std::vector{A, C}, held)); // C unheld -> withhold + EXPECT_TRUE(dgb::all_txs_backable(std::vector{}, held)); // no refs -> trivially backable +}