txpool: re-relay txs that haven't been relayed and need to be
What the developer wrote
Authored by j-berman
txpool: re-relay txs that haven't been relayed and need to be
Fixes the following circumstance:
1. Wallet submits tx to node.
2. Node adds tx to pool and queues for relay.
3. Node shuts off before the relay executes in the strand.
4. Node restarts.
5. Now, the fix ensures the node will re-relay the tx in the
get_relayable_transactions loop.
Also makes sure that stem txs don't have last_relayed_time set
to max, which would prevent them from ever being re-relayed on
a restart as well.
Context: https://github.com/seraphis-migration/monero/issues/365#issuecomment-4811722029
What changed, and why it matters
This patch fixes a bug in how Monero nodes remember whether they have shared new transactions with peers. If a node received a transaction but shut down before it could broadcast it, the node would previously never try again after restarting, so the transaction could sit forgotten in the node's memory pool. The fix makes the node re-broadcast such unshared transactions during its normal cleanup loop. It also fixes a related timing issue for privacy-preserving 'stem' transactions so they are not permanently blocked from re-broadcasting.
Treat as a routine reliability/availability fix. No immediate incident response is warranted, but nodes should update to ensure submitted transactions are reliably propagated after restarts. Monitor for any related transaction propagation anomalies.
Security signals we found
Transaction propagation failure could allow a transaction to languish in only one node's mempool
Stem/dandelion relay timing bug could suppress privacy transaction re-broadcast indefinitely
Fix is defensive availability/reliability improvement rather than an active exploit patch
Evidence from the diff
In tx_pool.cpp, when a transaction is added to the memory pool, last_relayed_time is now set for stem relays (using dandelionpp_embargo_average) in addition to forward relays, instead of being left at numeric_limits max. In get_relayable_transactions, the local/fluff/block cases now check meta.relayed and immediately treat unrelayed transactions as relayable, rather than comparing last_relayed_time against the relay delay. This ensures transactions that were queued but not yet broadcast before a restart are re-relayed.
Changed components
src/cryptonote_core/tx_pool.cppMonero transaction memory pool relay logicDandelion++ stem transaction handlingInspect captured patch +5 / −2
diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp
index 1b6e880..a5e5c3f 100644
--- a/src/cryptonote_core/tx_pool.cpp
+++ b/src/cryptonote_core/tx_pool.cpp
@@ -302,9 +302,10 @@ namespace cryptonote
{
using clock = std::chrono::system_clock;
auto last_relayed_time = std::numeric_limits<decltype(meta.last_relayed_time)>::max();
- if (tx_relay == relay_method::forward)
+ if (tx_relay == relay_method::forward || tx_relay == relay_method::stem)
{
- last_relayed_time = clock::to_time_t(clock::now() + crypto::random_poisson_seconds{forward_delay_average}());
+ const auto delay = tx_relay == relay_method::forward ? forward_delay_average : dandelionpp_embargo_average;
+ last_relayed_time = clock::to_time_t(clock::now() + crypto::random_poisson_seconds{delay}());
set_if_less(m_next_check, time_t(last_relayed_time));
}
// else the `set_relayed` function will adjust the time accordingly later
@@ -840,6 +841,8 @@ namespace cryptonote
case relay_method::local:
case relay_method::fluff:
case relay_method::block:
+ if (!meta.relayed)
+ break; // if it hasn't been relayed yet, relay it
if (now - meta.last_relayed_time <= get_relay_delay(meta.last_relayed_time, meta.receive_time))
return true; // continue to next tx
break;
Why this scored 35/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.