net: delay stale evaluation and expose time_added in private broadcast

Mccalabrese · Mccalabrese · commit 325afe664d10 · 2026-03-25T11:49:05.000-04:00
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -1863,7 +1863,8 @@ std::vector<CTransactionRef> PeerManagerImpl::AbortPrivateBroadcast(const uint25
     std::vector<CTransactionRef> removed_txs;
 
     size_t connections_cancelled{0};
-    for (const auto& [tx, _] : snapshot) {
+    for (const auto& tx_info : snapshot) {
+        const CTransactionRef& tx{tx_info.tx};
         if (tx->GetHash().ToUint256() != id && tx->GetWitnessHash().ToUint256() != id) continue;
         if (const auto peer_acks{m_tx_for_private_broadcast.Remove(tx)}) {
             removed_txs.push_back(tx);
diff --git a/src/private_broadcast.cpp b/src/private_broadcast.cpp
@@ -7,9 +7,6 @@
 
 #include <algorithm>
 
-/// If a transaction is not received back from the network for this duration
-/// after it is broadcast, then we consider it stale / for rebroadcasting.
-static constexpr auto STALE_DURATION{1min};
 
 bool PrivateBroadcast::Add(const CTransactionRef& tx)
     EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
@@ -93,12 +90,14 @@ std::vector<CTransactionRef> PrivateBroadcast::GetStale() const
     EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
 {
     LOCK(m_mutex);
-    const auto stale_time{NodeClock::now() - STALE_DURATION};
+    const auto now{NodeClock::now()};
     std::vector<CTransactionRef> stale;
     for (const auto& [tx, state] : m_transactions) {
         const Priority p{DerivePriority(state.send_statuses)};
-        if (p.last_confirmed < stale_time) {
-            stale.push_back(tx);
+        if (p.num_confirmed == 0) {
+            if (state.time_added < now - INITIAL_STALE_DURATION) stale.push_back(tx);
+        } else {
+            if (p.last_confirmed < now - STALE_DURATION) stale.push_back(tx);
         }
     }
     return stale;
@@ -117,7 +116,7 @@ std::vector<PrivateBroadcast::TxBroadcastInfo> PrivateBroadcast::GetBroadcastInf
         for (const auto& status : state.send_statuses) {
             peers.emplace_back(PeerSendInfo{.address = status.address, .sent = status.picked, .received = status.confirmed});
         }
-        entries.emplace_back(TxBroadcastInfo{.tx = tx, .peers = std::move(peers)});
+        entries.emplace_back(TxBroadcastInfo{.tx = tx, .time_added = state.time_added, .peers = std::move(peers)});
     }
 
     return entries;
diff --git a/src/private_broadcast.h b/src/private_broadcast.h
@@ -30,6 +30,15 @@
 class PrivateBroadcast
 {
 public:
+
+    /// If a transaction is not sent to any peer for this duration,
+    /// then we consider it stale / for rebroadcasting.
+    static constexpr auto INITIAL_STALE_DURATION{5min};
+
+    /// If a transaction is not received back from the network for this duration
+    /// after it is broadcast, then we consider it stale / for rebroadcasting.
+    static constexpr auto STALE_DURATION{1min};
+
     struct PeerSendInfo {
         CService address;
         NodeClock::time_point sent;
@@ -38,6 +47,7 @@ class PrivateBroadcast
 
     struct TxBroadcastInfo {
         CTransactionRef tx;
+        NodeClock::time_point time_added;
         std::vector<PeerSendInfo> peers;
     };
 
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
@@ -155,6 +155,7 @@ static RPCHelpMan getprivatebroadcastinfo()
                                 {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"},
                                 {RPCResult::Type::STR_HEX, "wtxid", "The transaction witness hash in hex"},
                                 {RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded transaction data"},
+                                {RPCResult::Type::NUM_TIME, "time_added", "The time this transaction was added to the private broadcast queue (seconds since epoch)"},
                                 {RPCResult::Type::ARR, "peers", "Per-peer send and acknowledgment information for this transaction",
                                     {
                                         {RPCResult::Type::OBJ, "", "",
@@ -183,6 +184,7 @@ static RPCHelpMan getprivatebroadcastinfo()
                 o.pushKV("txid", tx_info.tx->GetHash().ToString());
                 o.pushKV("wtxid", tx_info.tx->GetWitnessHash().ToString());
                 o.pushKV("hex", EncodeHexTx(*tx_info.tx));
+                o.pushKV("time_added", TicksSinceEpoch<std::chrono::seconds>(tx_info.time_added));
                 UniValue peers(UniValue::VARR);
                 for (const auto& peer : tx_info.peers) {
                     UniValue p(UniValue::VOBJ);
diff --git a/src/test/private_broadcast_tests.cpp b/src/test/private_broadcast_tests.cpp
@@ -90,6 +90,13 @@ BOOST_AUTO_TEST_CASE(basic)
     BOOST_CHECK(!pb.DidNodeConfirmReception(recipient2));
     BOOST_CHECK(!pb.DidNodeConfirmReception(nonexistent_recipient));
 
+    // 1. Freshly added transactions should NOT be stale yet.
+    BOOST_CHECK_EQUAL(pb.GetStale().size(), 0);
+
+    // 2. Fast-forward the mock clock past the INITIAL_STALE_DURATION.
+    SetMockTime(Now<NodeSeconds>() + PrivateBroadcast::INITIAL_STALE_DURATION + 1min);
+
+    // 3. Now that the initial duration has passed, both unconfirmed transactions should be stale.
     BOOST_CHECK_EQUAL(pb.GetStale().size(), 2);
 
     // Confirm reception by recipient1.
@@ -102,20 +109,21 @@ BOOST_AUTO_TEST_CASE(basic)
     const auto infos{pb.GetBroadcastInfo()};
     BOOST_CHECK_EQUAL(infos.size(), 2);
     {
-        const auto& [tx, peers]{find_tx_info(infos, tx_for_recipient1)};
+        const auto& peers{find_tx_info(infos, tx_for_recipient1).peers};
         BOOST_CHECK_EQUAL(peers.size(), 1);
         BOOST_CHECK_EQUAL(peers[0].address.ToStringAddrPort(), addr1.ToStringAddrPort());
         BOOST_CHECK(peers[0].received.has_value());
     }
     {
-        const auto& [tx, peers]{find_tx_info(infos, tx_for_recipient2)};
+        const auto& peers{find_tx_info(infos, tx_for_recipient2).peers};
         BOOST_CHECK_EQUAL(peers.size(), 1);
         BOOST_CHECK_EQUAL(peers[0].address.ToStringAddrPort(), addr2.ToStringAddrPort());
         BOOST_CHECK(!peers[0].received.has_value());
     }
 
-    BOOST_CHECK_EQUAL(pb.GetStale().size(), 1);
-    BOOST_CHECK_EQUAL(pb.GetStale()[0], tx_for_recipient2);
+    const auto stale_state{pb.GetStale()};
+    BOOST_CHECK_EQUAL(stale_state.size(), 1);
+    BOOST_CHECK_EQUAL(stale_state[0], tx_for_recipient2);
 
     SetMockTime(Now<NodeSeconds>() + 10h);
 
@@ -131,4 +139,22 @@ BOOST_AUTO_TEST_CASE(basic)
     BOOST_CHECK(!pb.PickTxForSend(/*will_send_to_nodeid=*/nonexistent_recipient, /*will_send_to_address=*/addr_nonexistent).has_value());
 }
 
+BOOST_AUTO_TEST_CASE(stale_unpicked_tx)
+{
+    SetMockTime(Now<NodeSeconds>());
+
+    PrivateBroadcast pb;
+    const auto tx{MakeDummyTx(/*id=*/42, /*num_witness=*/0)};
+    BOOST_REQUIRE(pb.Add(tx));
+
+    // Unpicked transactions use the longer INITIAL_STALE_DURATION.
+    BOOST_CHECK_EQUAL(pb.GetStale().size(), 0);
+    SetMockTime(Now<NodeSeconds>() + PrivateBroadcast::INITIAL_STALE_DURATION - 1min);
+    BOOST_CHECK_EQUAL(pb.GetStale().size(), 0);
+    SetMockTime(Now<NodeSeconds>() + 2min);
+    const auto stale_state{pb.GetStale()};
+    BOOST_REQUIRE_EQUAL(stale_state.size(), 1);
+    BOOST_CHECK_EQUAL(stale_state[0], tx);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
