btc: F3 known-txs retention/backable primitives + unit KAT - #868
Merged
Conversation
Port the pure, unit-testable known-txs bookkeeping from the DASH variant to
src/impl/btc/known_txs_retention.hpp:
- retain_template_txs(): rolling-window retention with F1 template-identity
(tx-hash SET) dedup — N re-registrations of one template consume one slot
while the window still holds N distinct templates; a tx leaves known_txs
only once absent from every retained set.
- all_txs_backable(): the F3 broadcast gate — a share is backable only when
we hold the bytes of every referenced new-tx.
Adds src/impl/btc/test/known_txs_retention_test.cpp (rides btc_share_test):
dedup + eviction correctness, and the backable gate (defeating the gate flips
BtcKnownTxsBackable.UnheldTxIsNotBackable red). btc_share_test 112/112 green.
This is the header primitive + coverage only. Call-site wiring at send_shares
and the mint path is deferred: BTC v35 shares carry no per-share tx refs, so
that wiring is a structural design decision tracked with the integrator.
This was referenced Jul 26, 2026
frstrtr
added a commit
that referenced
this pull request
Jul 26, 2026
… was sent Two reward-critical share-loss fixes, ported to BTC, DGB and BCH from the already-correct DASH implementation (src/impl/dash/node.cpp). F3 -- tx-completeness broadcast gate. send_shares() collected the referenced new-tx bytes with `if (it != m_known_txs.end()) full_txs.emplace_back(...)` and no else branch: a tx we do not hold was silently omitted and the share was written to the peer anyway. Canonical p2pool then 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. send_shares now filters the outgoing batch through partition_backable() (gate on the BYTES in m_known_txs, not on the peer's possibly-stale have_tx advert) and the now-unreachable omission path increments a counter that logs a warning. F2 -- mark only what was sent. broadcast_share() inserted each walked hash into m_shared_share_hashes INSIDE the chain-walk loop, before send_shares ran. send_shares returned void and has abandon paths (tracker try_to_lock miss, empty batch, and now the F3 gate), and zero connected peers abandons the batch too. A marked-but-unsent share is lost permanently: the next walk breaks on the first marked hash, so nothing ever re-pushes it -- silent PPLNS-credit loss with no retry path. send_shares now returns the hashes it actually wrote and broadcast_share marks only those, after the peer loop, via broadcast_and_mark(). m_last_broadcast_to likewise records what was written rather than what was offered, so a withheld share is never blamed for a peer disconnect (which would mark it rejected and block all future re-broadcast). Consensus surface: NONE. Both changes decide only WHETHER a share goes on the wire. Share bytes, minting, PPLNS weighting and payout are untouched, and a withheld share stays in the chain and is re-offered once its txs arrive. Primitives follow the per-coin header pattern established for BTC in #868: src/impl/{btc,dgb,bch}/known_txs_retention.hpp. The LTC lane is concurrently hoisting all_txs_backable into src/core/; these four per-coin headers (including dash) should be collapsed onto that hoist once both land. Tests ride existing allowlisted CI targets -- no new add_executable, no build.yml change: btc -> btc_share_test (broadcast_gate_test.cpp) dgb -> dgb_share_test (broadcast_gate_test.cpp, known_txs_retention_test.cpp) bch -> bch_embedded_block_broadcast_test (broadcast_gate_test.cpp; the bch test tree has no GTest harness, so the TU exposes a checks function the host main() calls) Mutation-verified red: deleting the gate reddens 2 btc / 4 bch assertions; restoring mark-before-send reddens 4 btc / 5 bch assertions.
frstrtr
added a commit
that referenced
this pull request
Jul 26, 2026
… was sent Two reward-critical share-loss fixes, ported to BTC, DGB and BCH from the already-correct DASH implementation (src/impl/dash/node.cpp). F3 -- tx-completeness broadcast gate. send_shares() collected the referenced new-tx bytes with `if (it != m_known_txs.end()) full_txs.emplace_back(...)` and no else branch: a tx we do not hold was silently omitted and the share was written to the peer anyway. Canonical p2pool then 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. send_shares now filters the outgoing batch through partition_backable() (gate on the BYTES in m_known_txs, not on the peer's possibly-stale have_tx advert) and the now-unreachable omission path increments a counter that logs a warning. F2 -- mark only what was sent. broadcast_share() inserted each walked hash into m_shared_share_hashes INSIDE the chain-walk loop, before send_shares ran. send_shares returned void and has abandon paths (tracker try_to_lock miss, empty batch, and now the F3 gate), and zero connected peers abandons the batch too. A marked-but-unsent share is lost permanently: the next walk breaks on the first marked hash, so nothing ever re-pushes it -- silent PPLNS-credit loss with no retry path. send_shares now returns the hashes it actually wrote and broadcast_share marks only those, after the peer loop, via broadcast_and_mark(). m_last_broadcast_to likewise records what was written rather than what was offered, so a withheld share is never blamed for a peer disconnect (which would mark it rejected and block all future re-broadcast). Consensus surface: NONE. Both changes decide only WHETHER a share goes on the wire. Share bytes, minting, PPLNS weighting and payout are untouched, and a withheld share stays in the chain and is re-offered once its txs arrive. Primitives follow the per-coin header pattern established for BTC in #868: src/impl/{btc,dgb,bch}/known_txs_retention.hpp. The LTC lane is concurrently hoisting all_txs_backable into src/core/; these four per-coin headers (including dash) should be collapsed onto that hoist once both land. Tests ride existing allowlisted CI targets -- no new add_executable, no build.yml change: btc -> btc_share_test (broadcast_gate_test.cpp) dgb -> dgb_share_test (broadcast_gate_test.cpp, known_txs_retention_test.cpp) bch -> bch_embedded_block_broadcast_test (broadcast_gate_test.cpp; the bch test tree has no GTest harness, so the TU exposes a checks function the host main() calls) Mutation-verified red: deleting the gate reddens 2 btc / 4 bch assertions; restoring mark-before-send reddens 4 btc / 5 bch assertions.
frstrtr
added a commit
that referenced
this pull request
Jul 27, 2026
… was sent Two reward-critical share-loss fixes, ported to BTC, DGB and BCH from the already-correct DASH implementation (src/impl/dash/node.cpp). F3 -- tx-completeness broadcast gate. send_shares() collected the referenced new-tx bytes with `if (it != m_known_txs.end()) full_txs.emplace_back(...)` and no else branch: a tx we do not hold was silently omitted and the share was written to the peer anyway. Canonical p2pool then 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. send_shares now filters the outgoing batch through partition_backable() (gate on the BYTES in m_known_txs, not on the peer's possibly-stale have_tx advert) and the now-unreachable omission path increments a counter that logs a warning. F2 -- mark only what was sent. broadcast_share() inserted each walked hash into m_shared_share_hashes INSIDE the chain-walk loop, before send_shares ran. send_shares returned void and has abandon paths (tracker try_to_lock miss, empty batch, and now the F3 gate), and zero connected peers abandons the batch too. A marked-but-unsent share is lost permanently: the next walk breaks on the first marked hash, so nothing ever re-pushes it -- silent PPLNS-credit loss with no retry path. send_shares now returns the hashes it actually wrote and broadcast_share marks only those, after the peer loop, via broadcast_and_mark(). m_last_broadcast_to likewise records what was written rather than what was offered, so a withheld share is never blamed for a peer disconnect (which would mark it rejected and block all future re-broadcast). Consensus surface: NONE. Both changes decide only WHETHER a share goes on the wire. Share bytes, minting, PPLNS weighting and payout are untouched, and a withheld share stays in the chain and is re-offered once its txs arrive. Primitives follow the per-coin header pattern established for BTC in #868: src/impl/{btc,dgb,bch}/known_txs_retention.hpp. The LTC lane is concurrently hoisting all_txs_backable into src/core/; these four per-coin headers (including dash) should be collapsed onto that hoist once both land. Tests ride existing allowlisted CI targets -- no new add_executable, no build.yml change: btc -> btc_share_test (broadcast_gate_test.cpp) dgb -> dgb_share_test (broadcast_gate_test.cpp, known_txs_retention_test.cpp) bch -> bch_embedded_block_broadcast_test (broadcast_gate_test.cpp; the bch test tree has no GTest harness, so the TU exposes a checks function the host main() calls) Mutation-verified red: deleting the gate reddens 2 btc / 4 bch assertions; restoring mark-before-send reddens 4 btc / 5 bch assertions.
frstrtr
added a commit
that referenced
this pull request
Jul 29, 2026
… was sent (#880) * btc/dgb/bch: gate share broadcast on tx completeness + mark only what was sent Two reward-critical share-loss fixes, ported to BTC, DGB and BCH from the already-correct DASH implementation (src/impl/dash/node.cpp). F3 -- tx-completeness broadcast gate. send_shares() collected the referenced new-tx bytes with `if (it != m_known_txs.end()) full_txs.emplace_back(...)` and no else branch: a tx we do not hold was silently omitted and the share was written to the peer anyway. Canonical p2pool then 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. send_shares now filters the outgoing batch through partition_backable() (gate on the BYTES in m_known_txs, not on the peer's possibly-stale have_tx advert) and the now-unreachable omission path increments a counter that logs a warning. F2 -- mark only what was sent. broadcast_share() inserted each walked hash into m_shared_share_hashes INSIDE the chain-walk loop, before send_shares ran. send_shares returned void and has abandon paths (tracker try_to_lock miss, empty batch, and now the F3 gate), and zero connected peers abandons the batch too. A marked-but-unsent share is lost permanently: the next walk breaks on the first marked hash, so nothing ever re-pushes it -- silent PPLNS-credit loss with no retry path. send_shares now returns the hashes it actually wrote and broadcast_share marks only those, after the peer loop, via broadcast_and_mark(). m_last_broadcast_to likewise records what was written rather than what was offered, so a withheld share is never blamed for a peer disconnect (which would mark it rejected and block all future re-broadcast). Consensus surface: NONE. Both changes decide only WHETHER a share goes on the wire. Share bytes, minting, PPLNS weighting and payout are untouched, and a withheld share stays in the chain and is re-offered once its txs arrive. Primitives follow the per-coin header pattern established for BTC in #868: src/impl/{btc,dgb,bch}/known_txs_retention.hpp. The LTC lane is concurrently hoisting all_txs_backable into src/core/; these four per-coin headers (including dash) should be collapsed onto that hoist once both land. Tests ride existing allowlisted CI targets -- no new add_executable, no build.yml change: btc -> btc_share_test (broadcast_gate_test.cpp) dgb -> dgb_share_test (broadcast_gate_test.cpp, known_txs_retention_test.cpp) bch -> bch_embedded_block_broadcast_test (broadcast_gate_test.cpp; the bch test tree has no GTest harness, so the TU exposes a checks function the host main() calls) Mutation-verified red: deleting the gate reddens 2 btc / 4 bch assertions; restoring mark-before-send reddens 4 btc / 5 bch assertions. * dgb: rescope to the reachable readvertise path; label broadcast_share pre-wiring A caller-side reachability trace shows DGB is not symmetric with btc/bch: 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 (#884). DGB's live path into send_shares is readvertise_best_share() (ROOT-2 re-advert on best-change plus a 10s one-shot timer), which re-pushes PEER-RECEIVED shares and deliberately bypasses the de-dup set. Verified independently: set_mint_share_fn / set_create_share_fn / broadcast_share / notify_local_share have zero occurrences in main_dgb.cpp; the only set_mint_share_fn binding in the tree is main_dash.cpp:1854 plus dgb's own work_source_test. readvertise_best_share is called from node.cpp:1767 and 1774. BTC (main_btc.cpp:1336 after lk.unlock() at :1323) and BCH (pool_entrypoint.hpp:564 after lk.unlock() at :562) are both live and unchanged. Changes: - dgb: extract the readvertise recording rule into readvertise_and_record() and wire readvertise_best_share() through it. This is DGB's live reward-critical marking discipline: m_last_broadcast_to is what a peer disconnect within 10s converts into m_rejected_share_hashes, and the readvertise walk `continue`s past rejected hashes permanently. Recording the OFFERED batch would let a share the F3 gate merely withheld be blamed for someone else's drop and excluded from every future re-advert -- a propagation loss for the whole sharechain, since DGB re-advertises peer-received shares. - dgb: rescope the KATs onto that live path. The gate cases (partition_backable) and the new DgbReadvertiseRecording cases drive code production executes; the broadcast_and_mark cases are renamed DgbBroadcastMarkingPrewired_NotReachedToday so no reader mistakes them for live protection. Removes the broadcast_share de-dup-walk KAT, which modelled an unreachable function -- the shape of the LTC #873 miss. - dgb: label NodeImpl::broadcast_share in source as pre-wiring for #884. The F2 fix stays so the seam is correct when the mint path is bound and so the three coins remain symmetric, but it protects nothing at runtime today. - btc/bch: document the ROOT-2 interaction at readvertise_best(). Marking is now a strict subset of what the pre-fix code marked, so the walk breaks later or in the same place, never earlier: the re-advert re-pushes a superset of what it used to. Monotonically better, strictly better for head shares minted while no peer was connected. It does not fully close ROOT-2 here -- shares already accepted by another peer stay marked and the walk still breaks, as before. That needs the ltc/dgb-style de-dup-bypass readvertise; pre-existing, out of scope. No behaviour change in this commit for btc/bch. Mutation-verified red on the DGB live path: recording the offered batch instead of what was written reddens WithheldShareIsNeverBlamedForAPeerDrop; dropping the empty-send skip reddens PeerThatReceivedNothingIsNotRecorded; deleting the gate reddens 2 DgbBroadcastGate cases. * btc/bch/dgb: route F3 send-path tx-completeness gate through the per-coin new_tx_hashes SSOT The old flat m_new_transaction_hashes probe was FALSE for every share variant (v17/v33 nest the refs in m_tx_info; v34+ carry none), so the F3 gate collected empty refs and treated every share as vacuously backable -- it never withheld an incomplete share on any version. Route each coin through its SSOT accessor from #913/#914 (btc/bch::new_tx_hashes, dgb::append_share_tx_refs) so the gate sees the real refs. Adds BtcBroadcastGate.RealShareTxInfoRefsWithheld driving a REAL v17/v33 share (not a top-level mock); mutation-verify confirms it goes red when the probe is reverted to the dead flat form. * btc: mirror ltc broadcast lock-discipline trace; pin send-path reachability Instrument btc::NodeImpl::broadcast_share with the deferred/acquired/ reached-send counters and IsNull/contains guards that ltc already carries, and add broadcast_lock_discipline_test.cpp driving a REAL btc::NodeImpl against a populated sharechain. Proves the F3 send path (tx-completeness gate + send_shares + mark-after-send) is reachable, not dead code behind an always-held lock: reached_send is 0 when the caller holds the tracker mutex EXCLUSIVELY on the calling thread and 1 when it does not -- the shape main_btc create_share_fn guarantees by dropping the exclusive lock (lk.unlock) before calling broadcast_share. Rides the allowlisted btc_share_test target (no new add_executable, #769). --------- Co-authored-by: frstrtr <frstrtr@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the pure known-txs bookkeeping from the DASH variant to
src/impl/btc/known_txs_retention.hpp:btc::retain_template_txs()— rolling-window retention with F1 template-identity (tx-hash SET) dedup.btc::all_txs_backable()— the F3 broadcast gate (hold the bytes of every referenced new-tx).New
src/impl/btc/test/known_txs_retention_test.cpp(rides btc_share_test, no new target): dedup + eviction correctness + the backable gate. Defeating the gate flipsBtcKnownTxsBackable.UnheldTxIsNotBackablered. btc_share_test 112/112 green.Scope:
src/impl/btc/only; nothing lifted to bitcoin_family/ or src/core/. Header primitive + coverage only — call-site wiring at send_shares/mint is deferred pending a structural decision (BTC v35 shares carry no per-share tx refs).