Skip to content

dgb: fix silently-dead tx-relay probe in send_shares (#905) - #914

Merged
frstrtr merged 1 commit into
masterfrom
dgb/tx-relay-dead-probe-905
Jul 27, 2026
Merged

dgb: fix silently-dead tx-relay probe in send_shares (#905)#914
frstrtr merged 1 commit into
masterfrom
dgb/tx-relay-dead-probe-905

Conversation

@frstrtr

@frstrtr frstrtr commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Port of BTC #880 tx-relay probe fix to the DGB lane.

Problem

src/impl/dgb/node.cpp NodeImpl::send_shares probed a TOP-LEVEL member obj->m_new_transaction_hashes that no DGB share type declares — the field is nested in m_tx_info (dgb::ShareTxInfo, present on Share/v17 + NewShare/v33 only). The requires-clause was therefore always false, so the remember_tx/forget_tx relay was silently dead for every DGB share version.

Fix

New SSOT src/impl/dgb/coin/share_tx_relay_refs.hpp :: dgb::append_share_tx_refs; send_shares routes needed_txs collection through it, guarding on requires { obj->m_tx_info } — live for v17/v33, compiled out for v34/35/36 (correct-by-construction, never silently dead). Faithful to BTC node.cpp:778 requires{obj->m_tx_info}.

Test

dgb_share_tx_relay_refs_test (KAT), registered in test/CMakeLists.txt and the build.yml --target allowlist (both occurrences, #143 trap). Built green on VM111, 4/4 PASS. Fails-before proven: reverting the probe to the top-level member collapses V17+V33 refs to 0 (FAIL) while segwit/v36 stay green — the KAT guards the fix, not unreachable code.

Scope / isolation

Single-coin DGB tree + additive build.yml allowlist entry. No shared-core or cross-coin changes. Per-coin isolation preserved.

Consistent with the shared approach on ltc #875 and bch #913. Gates #880 (coins tx-completeness gate, currently blocked pending this fix).

NodeImpl::send_shares gated its remember_tx/forget_tx relay on a
top-level obj->m_new_transaction_hashes that no dgb share type declares
-- the field is nested in m_tx_info (dgb::ShareTxInfo). The probe was
therefore always false, so DGB never forwarded a referenced new-tx hash
to a peer for any share version (v17..v36).

Route the collection through a new SSOT, dgb::append_share_tx_refs,
which guards on m_tx_info: live for v17/v33 (which carry it), compiled
out for v34/v35/v36 (which do not) -- correct-by-construction, never
silently dead. Faithful port of the btc fix behind #880.

Adds a fails-before KAT (dgb_share_tx_relay_refs_test) registered in the
CI --target allowlist: reverting the probe to the top-level member
collapses the v17/v33 expectations to zero.
@frstrtr
frstrtr merged commit 37515dc into master Jul 27, 2026
51 checks passed
@frstrtr
frstrtr deleted the dgb/tx-relay-dead-probe-905 branch July 27, 2026 05:10
frstrtr added a commit that referenced this pull request Jul 27, 2026
…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.
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant