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

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.

Co-authored-by: frstrtr <frstrtr@users.noreply.github.com>

Author: frstrtr

.github/workflows/build.yml

@@ -156,7 +156,7 @@ jobs:
                     dgb_gentx_coinbase_test dgb_connection_coinbase_test dgb_won_block_serialize_test dgb_pplns_payout_split_test nmc_auxpow_merkle_test nmc_template_builder_test nmc_underfill_guard_test nmc_auxpow_wire_test nmc_reconstruct_won_block_test nmc_mempool_name_test nmc_block_broadcast_test nmc_host_dualpath_test nmc_fallback_path_conformance_test dgb_gentx_share_path_test dgb_conn_pplns_producer_test dgb_other_tx_resolver_test \
                     dgb_other_tx_assembler_test dgb_reconstruct_won_block_test dgb_reconstruct_closure_test dgb_gentx_unpack_test dgb_work_source_test dgb_template_builder_test dgb_embedded_coin_node_test dgb_embedded_tx_select_test dgb_template_other_txs_test dgb_coinbase_value_parity_test dgb_submit_classify_test dgb_aux_parent_coinbase_parity_test dgb_template_capture_test dgb_aux_doge_db_commitment_bind_test dgb_aux_doge_mm_commitment_test dgb_aux_doge_dc_proof_test dgb_aux_doge_bind_parsers_test dgb_compact_blocks_bip152_parity_test dgb_aux_dual_target_select_test dgb_aux_broadcast_path_election_test dgb_aux_doge_submit_test dgb_aux_doge_embed_livewire_test dgb_aux_doge_dc_layout_verifier_test \
                     rpc_request_test softfork_check_test genesis_check_test algo_select_test digishield_walk_test header_chain_test \
-                    dgb_coin_node_seam_test dgb_block_broadcast_test dgb_won_block_dispatch_test dgb_forced_won_share_dualpath_test dgb_scrypt_pow_test dgb_nonce_grinder_test dgb_regrind_block_test dgb_won_block_finalize_test dgb_share_target_genesis_test dgb_share_target_retarget_test dgb_share_bits_oracle_pin_test dgb_pool_msg_wire_test dgb_get_shares_walk_test dgb_download_stops_test dgb_think_p1_walk_bounds_test dgb_think_p1_desired_emit_test dgb_think_p6_desired_cutoff_test dgb_think_p4_head_keys_test dgb_think_p3_best_head_test dgb_g1_oracle_byte_parity_test dgb_think_p2_walk_bounds_test dgb_expected_time_to_block_test dgb_tail_score_endpoints_test dgb_pool_attempts_per_second_test dgb_pool_efficiency_test dgb_think_p5_best_share_punish_test dgb_auto_ratchet_tail_guard_test dgb_auto_ratchet_sim_test dgb_binomial_conf_interval_test dgb_desired_version_tally_test dgb_min_protocol_ratchet_test dgb_get_height_and_last_endpoints_test dgb_chain_walk_window_test dgb_redistribute_delegate_ghal_test dgb_share_weight_decay_test dgb_naughty_propagation_test dgb_hash_format_parity_test dgb_emergency_decay_saturation_test dgb_arith256_muldiv_kat_test v37_test \
+                    dgb_coin_node_seam_test dgb_block_broadcast_test dgb_won_block_dispatch_test dgb_forced_won_share_dualpath_test dgb_scrypt_pow_test dgb_nonce_grinder_test dgb_regrind_block_test dgb_won_block_finalize_test dgb_share_target_genesis_test dgb_share_target_retarget_test dgb_share_bits_oracle_pin_test dgb_pool_msg_wire_test dgb_get_shares_walk_test dgb_download_stops_test dgb_think_p1_walk_bounds_test dgb_think_p1_desired_emit_test dgb_think_p6_desired_cutoff_test dgb_think_p4_head_keys_test dgb_think_p3_best_head_test dgb_g1_oracle_byte_parity_test dgb_think_p2_walk_bounds_test dgb_expected_time_to_block_test dgb_tail_score_endpoints_test dgb_pool_attempts_per_second_test dgb_pool_efficiency_test dgb_think_p5_best_share_punish_test dgb_auto_ratchet_tail_guard_test dgb_auto_ratchet_sim_test dgb_binomial_conf_interval_test dgb_desired_version_tally_test dgb_min_protocol_ratchet_test dgb_get_height_and_last_endpoints_test dgb_chain_walk_window_test dgb_redistribute_delegate_ghal_test dgb_share_weight_decay_test dgb_naughty_propagation_test dgb_hash_format_parity_test dgb_emergency_decay_saturation_test dgb_arith256_muldiv_kat_test dgb_share_tx_relay_refs_test v37_test \
             -j8
 
       - name: Run tests
@@ -343,7 +343,7 @@ jobs:
                     dgb_gentx_coinbase_test dgb_connection_coinbase_test dgb_won_block_serialize_test dgb_pplns_payout_split_test nmc_auxpow_merkle_test nmc_template_builder_test nmc_underfill_guard_test nmc_auxpow_wire_test nmc_reconstruct_won_block_test nmc_mempool_name_test nmc_block_broadcast_test nmc_host_dualpath_test nmc_fallback_path_conformance_test dgb_gentx_share_path_test dgb_conn_pplns_producer_test dgb_other_tx_resolver_test \
                     dgb_other_tx_assembler_test dgb_reconstruct_won_block_test dgb_reconstruct_closure_test dgb_gentx_unpack_test dgb_work_source_test dgb_template_builder_test dgb_embedded_coin_node_test dgb_embedded_tx_select_test dgb_template_other_txs_test dgb_coinbase_value_parity_test dgb_submit_classify_test dgb_aux_parent_coinbase_parity_test dgb_template_capture_test dgb_aux_doge_db_commitment_bind_test dgb_aux_doge_mm_commitment_test dgb_aux_doge_dc_proof_test dgb_aux_doge_bind_parsers_test dgb_compact_blocks_bip152_parity_test dgb_aux_dual_target_select_test dgb_aux_broadcast_path_election_test dgb_aux_doge_submit_test dgb_aux_doge_embed_livewire_test dgb_aux_doge_dc_layout_verifier_test \
                     rpc_request_test softfork_check_test genesis_check_test algo_select_test digishield_walk_test header_chain_test \
-                    dgb_coin_node_seam_test dgb_block_broadcast_test dgb_won_block_dispatch_test dgb_forced_won_share_dualpath_test dgb_scrypt_pow_test dgb_nonce_grinder_test dgb_regrind_block_test dgb_won_block_finalize_test dgb_share_target_genesis_test dgb_share_target_retarget_test dgb_share_bits_oracle_pin_test dgb_pool_msg_wire_test dgb_get_shares_walk_test dgb_download_stops_test dgb_think_p1_walk_bounds_test dgb_think_p1_desired_emit_test dgb_think_p6_desired_cutoff_test dgb_think_p4_head_keys_test dgb_think_p3_best_head_test dgb_g1_oracle_byte_parity_test dgb_think_p2_walk_bounds_test dgb_expected_time_to_block_test dgb_tail_score_endpoints_test dgb_pool_attempts_per_second_test dgb_pool_efficiency_test dgb_think_p5_best_share_punish_test dgb_auto_ratchet_tail_guard_test dgb_auto_ratchet_sim_test dgb_binomial_conf_interval_test dgb_desired_version_tally_test dgb_min_protocol_ratchet_test dgb_get_height_and_last_endpoints_test dgb_chain_walk_window_test dgb_redistribute_delegate_ghal_test dgb_share_weight_decay_test dgb_naughty_propagation_test dgb_hash_format_parity_test dgb_emergency_decay_saturation_test dgb_arith256_muldiv_kat_test test_coin_broadcaster test_multiaddress_pplns test_pplns_stress \
+                    dgb_coin_node_seam_test dgb_block_broadcast_test dgb_won_block_dispatch_test dgb_forced_won_share_dualpath_test dgb_scrypt_pow_test dgb_nonce_grinder_test dgb_regrind_block_test dgb_won_block_finalize_test dgb_share_target_genesis_test dgb_share_target_retarget_test dgb_share_bits_oracle_pin_test dgb_pool_msg_wire_test dgb_get_shares_walk_test dgb_download_stops_test dgb_think_p1_walk_bounds_test dgb_think_p1_desired_emit_test dgb_think_p6_desired_cutoff_test dgb_think_p4_head_keys_test dgb_think_p3_best_head_test dgb_g1_oracle_byte_parity_test dgb_think_p2_walk_bounds_test dgb_expected_time_to_block_test dgb_tail_score_endpoints_test dgb_pool_attempts_per_second_test dgb_pool_efficiency_test dgb_think_p5_best_share_punish_test dgb_auto_ratchet_tail_guard_test dgb_auto_ratchet_sim_test dgb_binomial_conf_interval_test dgb_desired_version_tally_test dgb_min_protocol_ratchet_test dgb_get_height_and_last_endpoints_test dgb_chain_walk_window_test dgb_redistribute_delegate_ghal_test dgb_share_weight_decay_test dgb_naughty_propagation_test dgb_hash_format_parity_test dgb_emergency_decay_saturation_test dgb_arith256_muldiv_kat_test dgb_share_tx_relay_refs_test test_coin_broadcaster test_multiaddress_pplns test_pplns_stress \
                     v37_test \
             -j8
 

src/impl/dgb/coin/share_tx_relay_refs.hpp

@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+#pragma once
+
+#include <vector>
+
+#include <core/uint256.hpp>
+
+namespace dgb
+{
+
+// SSOT: the new-transaction hashes a share references for peer tx-relay.
+//
+// DGB shares carry these INSIDE m_tx_info (dgb::ShareTxInfo), and only on the
+// pre-segwit variants Share (v17) and NewShare (v33). v34/v35/v36 declare no
+// m_tx_info member, so the `requires { obj->m_tx_info; }` probe compiles the
+// body out for them: correct-by-construction, never silently dead.
+//
+// This replaces the defective `requires { obj->m_new_transaction_hashes; }`
+// probe in send_shares(), which named a TOP-LEVEL member NO dgb share type
+// declares (the field is nested in m_tx_info). That probe was ALWAYS false, so
+// the remember_tx/forget_tx relay block never forwarded a single tx hash to a
+// peer for ANY share version. Faithful port of the btc fix behind #880
+// (src/impl/btc/node.cpp: guard on obj->m_tx_info, iterate the nested member).
+template <typename ShareT>
+inline void append_share_tx_refs(const ShareT* obj, std::vector<uint256>& out)
+{
+    if constexpr (requires { obj->m_tx_info; })
+        for (const auto& th : obj->m_tx_info.m_new_transaction_hashes)
+            out.push_back(th);
+}
+
+} // namespace dgb

src/impl/dgb/node.cpp

@@ -11,6 +11,7 @@
 #include <impl/dgb/pool_efficiency.hpp>
 #include <impl/dgb/expected_time_to_block.hpp>
 #include <impl/dgb/coin/binomial_conf_interval.hpp>
+#include <impl/dgb/coin/share_tx_relay_refs.hpp>  // SSOT: per-share new-tx-ref probe (#905)
 
 #include <algorithm>
 #include <filesystem>
@@ -694,19 +695,23 @@ void NodeImpl::send_shares(peer_ptr peer, const std::vector<uint256>& share_hash
     if (shares.empty())
         return;
 
-    // Collect transactions that the peer doesn't know about
+    // Collect transactions that the peer doesn't know about. A share's
+    // referenced new-tx hashes live INSIDE m_tx_info (dgb::ShareTxInfo) on
+    // v17/v33 and are absent on v34/v35/v36. The pre-fix probe named a
+    // top-level m_new_transaction_hashes NO dgb share type declares, so it was
+    // ALWAYS false and this relay block was silently dead for every version.
+    // append_share_tx_refs is the SSOT that guards on m_tx_info (#905).
     std::set<uint256> needed_txs;
     for (auto& share : shares)
     {
         share.invoke([&](auto* obj) {
-            if constexpr (requires { obj->m_new_transaction_hashes; })
+            std::vector<uint256> refs;
+            dgb::append_share_tx_refs(obj, refs);
+            for (const auto& th : refs)
             {
-                for (const auto& th : obj->m_new_transaction_hashes)
-                {
-                    if (!peer->m_remote_txs.count(th) &&
-                        !peer->m_remembered_txs.count(th))
-                        needed_txs.insert(th);
-                }
+                if (!peer->m_remote_txs.count(th) &&
+                    !peer->m_remembered_txs.count(th))
+                    needed_txs.insert(th);
             }
         });
     }

src/impl/dgb/test/CMakeLists.txt

@@ -17,6 +17,22 @@ if (BUILD_TESTING AND GTest_FOUND)
     include_directories(${gtest_SOURCE_DIR}/include ${gtest_SOURCE_DIR})
     gtest_add_tests(dgb_share_test "" AUTO)
 
+    # --- #905 tx-relay dead-probe regression (port of btc #880) ------------
+    # Pins coin/share_tx_relay_refs.hpp: append_share_tx_refs, the SSOT
+    # send_shares() uses to collect a share's referenced new-tx hashes for
+    # remember_tx relay. Guards against the top-level m_new_transaction_hashes
+    # probe that matched no dgb share type (always-false -> silently dead relay
+    # for every version). Links the dgb OBJECT lib like dgb_share_test (pulls
+    # share.hpp share codec). MUST appear in BOTH this registration AND the
+    # build.yml --target allowlist (#143 NOT_BUILT trap).
+    add_executable(dgb_share_tx_relay_refs_test share_tx_relay_refs_test.cpp)
+    target_link_libraries(dgb_share_tx_relay_refs_test PRIVATE
+        GTest::gtest_main GTest::gtest
+        core dgb
+        c2pool_payout c2pool_merged_mining c2pool_hashrate c2pool_storage
+        dgb_coin pool sharechain)
+    gtest_add_tests(dgb_share_tx_relay_refs_test "" AUTO)
+
     # --- #82 broadcaster-gate: faithful as_block FRAMING -----------------------
     # Pins coin/block_assembly.hpp (share->block reassembly: merkle_root
     # reconstruction from the gentx hash + the share`s merkle_link, then

src/impl/dgb/test/share_tx_relay_refs_test.cpp

@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+// DGB tx-relay dead-probe regression (#905, port of the btc #880 fix).
+//
+// send_shares() collects the new-tx hashes each share references so it can
+// remember_tx-forward them to a peer. The pre-fix probe guarded on a TOP-LEVEL
+// m_new_transaction_hashes that NO dgb share type declares (the field is nested
+// in m_tx_info), so the collection was ALWAYS empty and DGB never relayed a tx
+// byte to a peer for ANY share version. This pins append_share_tx_refs, the
+// SSOT send_shares() now uses:
+//   - v17/v33 (carry m_tx_info)  -> the referenced hashes ARE collected
+//   - v34/v35/v36 (no m_tx_info) -> compiled out, vacuously empty
+//
+// FAILS-BEFORE: revert append_share_tx_refs to probe obj->m_new_transaction_hashes
+// and the v17/v33 expectations below collapse to 0 -- the historical dead path.
+
+#include <gtest/gtest.h>
+
+#include <vector>
+
+#include <core/uint256.hpp>
+#include <impl/dgb/share.hpp>
+#include <impl/dgb/coin/share_tx_relay_refs.hpp>
+
+namespace {
+
+uint256 mk(const char* hex) { uint256 h; h.SetHex(hex); return h; }
+
+const char* H1 = "1111111111111111111111111111111111111111111111111111111111111111";
+const char* H2 = "2222222222222222222222222222222222222222222222222222222222222222";
+
+// v17: m_tx_info present -> referenced hashes collected in order.
+TEST(DGB_tx_relay_refs, V17CollectsNestedNewTxHashes)
+{
+    dgb::Share s;
+    s.m_tx_info.m_new_transaction_hashes = {mk(H1), mk(H2)};
+
+    std::vector<uint256> refs;
+    dgb::append_share_tx_refs(&s, refs);
+
+    ASSERT_EQ(refs.size(), 2u);          // dead-probe regression makes this 0
+    EXPECT_EQ(refs[0], mk(H1));
+    EXPECT_EQ(refs[1], mk(H2));
+}
+
+// v33: same nested carrier -> collected.
+TEST(DGB_tx_relay_refs, V33CollectsNestedNewTxHashes)
+{
+    dgb::NewShare s;
+    s.m_tx_info.m_new_transaction_hashes = {mk(H1)};
+
+    std::vector<uint256> refs;
+    dgb::append_share_tx_refs(&s, refs);
+
+    ASSERT_EQ(refs.size(), 1u);          // dead-probe regression makes this 0
+    EXPECT_EQ(refs[0], mk(H1));
+}
+
+// v34/v35: no m_tx_info member -> probe compiled out, vacuously empty.
+TEST(DGB_tx_relay_refs, SegwitVariantsCarryNoRefsByConstruction)
+{
+    dgb::SegwitMiningShare v34;
+    dgb::PaddingBugfixShare v35;
+
+    std::vector<uint256> refs;
+    dgb::append_share_tx_refs(&v34, refs);
+    dgb::append_share_tx_refs(&v35, refs);
+
+    EXPECT_TRUE(refs.empty());
+}
+
+// v36 merged-mining: no m_tx_info member -> vacuously empty.
+TEST(DGB_tx_relay_refs, MergedMiningShareCarriesNoRefs)
+{
+    dgb::MergedMiningShare v36;
+
+    std::vector<uint256> refs;
+    dgb::append_share_tx_refs(&v36, refs);
+
+    EXPECT_TRUE(refs.empty());
+}
+
+} // namespace