It works like this:
sendrawtransactionis called and given a transaction- that transaction is added to the priority queue for broadcasting
- 3 new connections are started
- Each of those 3 connections picks the highest priority transaction and sends it, presumably this is the tx from 1.
- Periodically a resend-stale job is run which picks stale transactions and resends them. A transaction is considered stale if its reception was confirmed by some peer but we did not receive it back from the network for some time.
The issue is that the code from 5. would consider a freshly added transaction (after 2. before 4.) as stale. These odd messages might be logged:
02026-03-19T11:20:30Z [privatebroadcast] Requesting 3 new connections due to txid=123, wtxid=456
12026-03-19T11:20:32Z [privatebroadcast] Reattempting broadcast of stale txid=123 wtxid=456
This will not impede the broadcast. At worse some excess connections will be created and maybe the transaction will be broadcast more times. Or if the transaction makes it back to us before the last connection sends it, then that excess connection will be “in vain”, or it will pick the next highest priority transaction to send.
I think this deserves fixing because it is sub-optimal and the logs look odd - “stale just 2 seconds after being added!?”.
Looking at PrivateBroadcast::DerivePriority() for such fresh transactions it is going to return a priority for which the time in Priority::last_confirmed is default constructed, i.e. 0. And that will later be compared to and be less than the stale time. One possible fix would be in PrivateBroadcast::GetStale() to also check if p.num_picked is greater than 0. This will omit such fresh not-yet-picked transactions. I am not sure if that would make it possible for a tx to stay forever in this “fresh” state if something happens with its 3 connections and it is never picked. Maybe it would be better to remember the time the transaction was taken (when it was added to the queue) and also consider it stale at some point based on that.
