wallet: Only broadcast resubmitted transactions that were added to the mempool #36482

achow101 commented at 9:48 PM on October 8, 2026: member

As discussed in #36403, one of the issues with current rebroadcast behavior is that the rebroadcast begins an unconditional relay of a transaction to peers. While this is broadcast is limited to peers that have not previously been inv'd the transaction, there is still a privacy leak in this double relay of a transaction, especially so for transactions that are still in nodes' mempools.

This PR changes the rebroadcast to only rebroadcast transactions if the transaction was not already present in our mempool. This provides some additional privacy while retaining intended rebroadcast behavior as transactions that have likely fallen out of nodes' mempools are the ones that actually need to be rebroadcast.

The wallet rebroadcast tests are additionally refactored for isolation so that their behaviors can still be tested after this change. Additionally, a test for the loading resubmission was added, as well as a test for the new behavior.

test: Refactor wallet_resendwallettransactions for test case isolation 2121d61323

test: Test wallet unconfirmed resubmission on load 858be1c447

node: Add option to relay txs only if they are added to mempool

BroadcastTransaction takes different modes of broadcasting. Add one
which will attempt to add a transaction to the mempool, and if that
succeeds, broadcast the transaction to peers. If the transaction already
exists in the mempool, it will not be rebroadcast.

4446226ea9

wallet: Only rebroadcast transactions that were re-added to the mempool 8aed98fef4

test: Check that wallet txs already in the mempool are not rebroadcast 4dbd27f37b

DrahtBot added the label Wallet on Oct 8, 2026

DrahtBot commented at 9:48 PM on October 8, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36482
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36482.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36482#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	polespinasa

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36566 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36566.svg"> (private broadcast: wait synchronously for mempool acceptance by andrewtoth)
#36540 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36540.svg"> (mempool: remove time-based transaction expiry by ismaelsadeeq)
#36481 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36481.svg"> (wallet, rpc, gui: Allow users to manually rebroadcast a wallet transaction by achow101)
#36398 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36398.svg"> (wallet: only rebroadcast transactions originating from the wallet by mzumsande)
#36330 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36330.svg"> (wallet: don't resubmit transactions under -privatebroadcast by instagibbs)
#36257 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36257.svg"> (qa: assert_equals -> assert_true/assert_false by hodlinator)
#10102 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/10102.svg"> (Multiprocess bitcoin by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

polespinasa commented at 10:24 PM on October 8, 2026: member

concept ACK

in test/functional/wallet_resendwallettransactions.py:45 in 8aed98fef4

  41 | @@ -42,6 +42,19 @@ def mine_empty_block(self, node):
  42 |          block.solve()
  43 |          node.submitblock(block.serialize().hex())
  44 |  
  45 | +    def evict_mempool_transaction(self, conflict_utxo, node, wallet, indep_wallet):

mzumsande commented at 3:32 PM on October 9, 2026:

This replacement pattern with indep_utxo is just the mechanism that an attacker could do to probe for which nodes a given address belong to, so this doesn't appear to completely solve the receiver issue from #3828.

murchandamus commented at 6:25 PM on October 21, 2026: member

This PR introduces the new MEMPOOL_AND_BROADCAST_ADDED TxBroadcast case, and uses it when the wallet resubmits transactions to the node’s mempool instead of the transaction being rebroadcast to all peers that it hadn’t been inventoried. The tests check that the rebroadcast happens when the transaction is added to the mempool and doesn’t happen when the transaction was already present. This is an obvious privacy improvement.

The only open question I have at the moment is whether the combination of using MEMPOOL_NO_BROADCAST on wallet loading and then MEMPOOL_AND_BROADCAST_ADDED on the wallet’s rebroadcast could prevent a transaction from being resubmitted to the network. E.g., if a wallet submits a low-feerate transaction, then the node is turned off for some period during which a feerate spike causes most of the network to drop the transaction, then the node is turned back on, wouldn’t this mean that the wallet’s transaction is not rebroadcast at all?

Also, I’m not sure whether using MEMPOOL_AND_BROADCAST_ADDED during wallet loading would fix that, if the node still had the transaction in its mempool from before going offline.

fametrano commented at 7:43 AM on October 25, 2026: contributor

After a restart, this branch does not announce a wallet transaction that peers already received while our own mempool keeps it, so the answer to murch's question is yes.

I built 4dbd27f37b and its base f739c7876b and ran a regtest script that connects a fresh P2PInterface peer, moves mocktime 36 hours ahead so that MaybeResendWalletTxs runs, and sends an unrelated transaction as a barrier. A wallet transaction still in the mempool is announced on base and not on the branch, as intended. One that expired and was accepted again is announced on both. One rejected as non-final is announced on neither.

With -persistmempool=0, the load-time resubmission puts the transaction back without broadcast. With mempool.dat, it comes back already out of the unbroadcast set, because a peer requested it before shutdown. The branch announced nothing in either case, and with -persistmempool=0 nothing over two periodic resubmissions and six ReattemptInitialBroadcast windows; base announced it at each resubmission. The same holds whenever our mempool keeps a transaction that peers dropped, until it expires from our mempool (two weeks by default). A node offline for days now takes its own mempool as a proxy for everyone else's. test_resubmit_on_load stops before the first periodic resubmission; extending it would pin whichever behaviour is intended.

Smaller: the comment above ResubmitWalletTransactions still names MEMPOOL_AND_BROADCAST_TO_ALL for the periodic path, and "resubmit N unconfirmed transactions" now counts transactions not relayed. I can share the script as a gist.