net: delay stale evaluation and expose time_added in private broadcast
What the developer wrote
Authored by Mccalabrese
net: delay stale evaluation and expose time_added in private broadcast
What changed, and why it matters
This Bitcoin Core commit tweaks how the node decides whether a privately broadcast transaction is 'stale' and should be re-broadcast. Previously, a transaction could be marked stale just one minute after it was added, even if it had never actually been sent to any peer. Now, transactions that have not yet been picked for sending use a longer five-minute window, while already-sent transactions keep the one-minute window. The commit also exposes the 'time_added' field in an RPC diagnostic so users can see when each transaction entered the private-broadcast queue. There is no claim in the commit that this fixes a security vulnerability; it reads as a robustness/usability improvement.
Treat as a normal code-quality/robustness patch. Reviewers may want to confirm that the 5-minute initial window does not delay rebroadcast excessively under adversarial network conditions, and that exposing time_added in the RPC does not leak sensitive timing information to unauthorized RPC users. No urgent security action is indicated by the diff itself.
Security signals we found
Change in staleness/rebroadcast timing for private transaction broadcast
New RPC field exposure (time_added) for diagnostic transparency
No explicit security framing or CVE reference in commit message or diff
No input validation, cryptography, or consensus code modified
Evidence from the diff
The change is in the PrivateBroadcast class used for the ‘sendtxrcncl’ / private transaction broadcast path. It splits the stale-detection logic: for transactions with zero confirmed sends (num_confirmed == 0), staleness is measured from state.time_added using a new INITIAL_STALE_DURATION of 5 minutes; for transactions with at least one confirmed send, staleness is measured from the last confirmed time using the existing STALE_DURATION of 1 minute. The constants are moved to the header, time_added is included in TxBroadcastInfo, and the getprivatebroadcastinfo RPC now returns time_added. AbortPrivateBroadcast is updated to use named struct members instead of structured bindings because the TxBroadcastInfo layout changed. Tests are added/updated to cover the new behavior.
Changed components
src/private_broadcast.cppsrc/private_broadcast.hsrc/net_processing.cppsrc/rpc/mempool.cppsrc/test/private_broadcast_tests.cppInspect captured patch +50 / −12
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index e36d147c..23a9db3b 100644
--- 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
index ce42dfd8..9c107da6 100644
--- 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
index 27f42166..2ecf5f6a 100644
--- 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 @@ public:
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
index 1a94d10b..6eeb545f 100644
--- 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
index 73bdbbb5..048284b2 100644
--- 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()
Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.