test: add test for rebroadcast of transaction received via p2p by mzumsande · Pull Request #34359 · bitcoin/bitcoin

mzumsande · 2026-01-21T02:42:05Z

The wallet doesn't only rebroadcast transactions it created, but also relevant transactions received via p2p.
Since this is not self-evident, add test coverage for it.

DrahtBot · 2026-01-21T02:42:14Z

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34359.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko, vasild, furszy, danielabrozzoni

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34533 (wallet: resubmit transactions with private broadcast if enabled by vasild)

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.

vasild · 2026-01-22T11:30:47Z

2026-01-21T03:34:27.4304152Z �[0;36m test 2026-01-21T03:34:27.140268Z TestFramework (INFO): node0 sends a tx to node1 and disconnects �[0m
...
2026-01-21T03:34:27.4319719Z �[0;34m node0 2026-01-21T03:34:27.149584Z (mocktime: 2026-02-07T03:50:31Z) [httpworker.1] [wallet/wallet.h:938] [WalletLogPrintf] [default_wallet] Submitting wtx c255f5b61919a60877d4e91eb81f656f85e743513119f5ddcc9cffc4ffb62b45 to mempool and for broadcast to peers �[0m
...
2026-01-21T03:34:27.4320508Z �[0;34m node0 2026-01-21T03:34:27.149947Z (mocktime: 2026-02-07T03:50:31Z) [httpworker.1] [wallet/wallet.h:938] [WalletLogPrintf] [default_wallet] CommitTransaction(): Transaction cannot be broadcast immediately, bad-txns-inputs-missingorspent �[0m

So, node0 did not like the transaction it generated and failed to broadcast it to node1: bad-txns-inputs-missingorspent. Happens only sometimes.

mzumsande · 2026-01-22T12:03:16Z

yes - because of the wallet bug, the wallet creates an invalid transaction, trying to double-spend an utxo that was already used in a block. The node then rejects that tx obviously, and the rpc querying for the tx fails.
I'll rebase on #34358 with the next push. [Edit:done now]

mzumsande · 2026-01-29T03:04:37Z

rebased/undrafted after merge of #34358

vasild

ACK 5c9dedf

test/functional/wallet_resendwallettransactions.py

mzumsande · 2026-02-09T10:24:41Z

5c9dedf to b2c9eac: addressed feedback

vasild

ACK b2c9eac

mzumsande · 2026-02-21T07:03:10Z

b2c9eac to 990e2cd: rebased

vasild

ACK 990e2cd

maflcko · 2026-02-26T18:38:16Z

test/functional/wallet_resendwallettransactions.py

    def set_test_params(self):
-        self.num_nodes = 1
+        self.num_nodes = 2
+        self.extra_args = [["-whitelist=relay,noban@127.0.0.1"], ["-whitelist=relay,noban@127.0.0.1"]]


Is there a reason why this is needed? Without this, sending the tx fails, but I don't see why.

node0 must have node1 with the noban permission in order to send the transaction to it without delay in this part of the test:

self.log.info("node0 sends a tx to node1 and disconnects") recv_addr = node1.getnewaddress() recv_txid = node.sendtoaddress(recv_addr, 1)

bitcoin/src/net_processing.cpp

Lines 5981 to 5989 in b6b8f8a

bool fSendTrickle = node.HasPermission(NetPermissionFlags::NoBan);

if (tx_relay->m_next_inv_send_time < current_time) {

fSendTrickle = true;

if (node.IsInboundConn()) {

tx_relay->m_next_inv_send_time = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL, node.m_network_key);

} else {

tx_relay->m_next_inv_send_time = current_time + m_rng.rand_exp_duration(OUTBOUND_INVENTORY_BROADCAST_INTERVAL);

}

}

Note that in this part of the test the time is frozen and does not flow because setmocktime() has been called on node0 so it never sets fSendTrickle to true and thus never sends the transaction to node1.

Btw this suffices:

Suggested change

self.extra_args = [["-whitelist=relay,noban@127.0.0.1"], ["-whitelist=relay,noban@127.0.0.1"]]

self.extra_args = [["-whitelist=noban@127.0.0.1"], []]

Ok, that is pretty obvious, thx. Could also add a comment, like # Needed due to mocktime. A third alternative could be self.noban_tx_relay = True # Needed due to mocktime.

... or to advance the mocktime across NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL, node.m_network_key) but that would always leave, at least in theory, some chance that we did not advance enough.

used self.noban_tx_relay, which is more common

maflcko

lgtm. Feel free to ignore the nits.

I guess this doesn't add any coverage in terms of new paths/lines, because the wallet does not differentiate between tx sources, but if this ever were to change in the future, this test would already be there.

review ACK 990e2cd 🦉

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 990e2cd0babf7a2ee90d7b40e91a0c7477511957 🦉
Qvh92TAwBuuBKdQwPindEYsKDLnaNpJ8iBuqe/CM1y4flECUTk5rYxTez0Xkgs5tWg/eb7685VUp8bnGdkSgCQ==

maflcko · 2026-02-27T08:48:01Z

test/functional/wallet_resendwallettransactions.py

+            node.setmocktime(now + RESEND_TIMER_LIMIT)
            # Tell scheduler to call MaybeResendWalletTxs now.
            node.mockscheduler(60)
        # Give some time for trickle to occur


nit: I guess this is no longer needed and can be removed now? Also -- unrelated, the peer_second.wait_for_broadcast([txid]) can be moved into the assert_debug_log scope, so that the timeout=2-polling on it can be removed?

test/functional/wallet_resendwallettransactions.py

The wallet doesn't only rebroadcast transactions it created, but also relevant transactions received via p2p. Since this is not self-evident, add test coverage for it.

mzumsande · 2026-02-27T11:44:22Z

990e2cd to 73e3853: addressed feedback

I guess this doesn't add any coverage in terms of new paths/lines, because the wallet does not differentiate between tx sources, but if this ever were to change in the future, this test would already be there.

Yes, my main motivation is to have a test for this behavior before discussing changes to it.

I knew that rebroadcast has privacy downsides, but I always thought that just making sure your txns confirm in a couple of hours would be sufficient to mitigate it, which seemed annoying but manageable to me.

But I never realized that just passively (not submitting any txns) running a Bitcoin core node for a day with a loaded wallet used a long time ago (maybe even with private broadcast then) would put you at risk of being deanonymized via dusting / rebroadcast tracking - even though I'm sure more regular wallet contributors knew this, after all there is the ancient issue #3828.

maflcko · 2026-02-27T12:20:48Z

Just addressed some nits:

re-ACK 73e3853 🦌

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 73e385311055eb6c0e3ddb2221f927a3872af1fb 🦌
zjBj3GEv01zf0Guo3qrZXQrLOPpIZFK4Wx5VdzynhAezS9nYDqHGcmp3CUltj2Hh6GlStWdIsBrwFwXA3CVbBQ==

vasild

ACK 73e3853

furszy

ACK 73e3853

danielabrozzoni

tACK 73e3853

It wasn't obvious to me that the wallet rebroadcasts transactions it's the recipient of, so it's nice to have a test that makes this a bit more explicit.

I left a micro-nit in case you need to retouch, otherwise it's good as-is.

danielabrozzoni · 2026-03-04T21:19:41Z

test/functional/wallet_resendwallettransactions.py

        node.getmempoolentry(txid)
        node.getmempoolentry(child_txid)

+        self.log.info("Test rebroadcast of transactions received by others")


nit: I find "received by others" to be slightly confusing, because instead of reading it as "p2p tx message that someone sent us", I read it as "transaction whose recipient is someone else", which is the opposite of what's happening here (node1 does the rebroadcast and is the recipient).

Maybe "Test rebroadcast of incoming wallet transactions" is clearer.

DrahtBot added the Tests label Jan 21, 2026

mzumsande mentioned this pull request Jan 21, 2026

Clients leak IPs if they are recipients of a transaction #3828

Open

mzumsande force-pushed the 202601_test_rebroadcast branch from 6abf771 to 051ca37 Compare January 21, 2026 03:10

DrahtBot added the CI failed label Jan 21, 2026

vasild mentioned this pull request Jan 22, 2026

wallet: fix removeprunedfunds bug with conflicting transactions #34358

Merged

mzumsande force-pushed the 202601_test_rebroadcast branch from 051ca37 to ce8560d Compare January 26, 2026 03:18

DrahtBot removed the CI failed label Jan 26, 2026

mzumsande force-pushed the 202601_test_rebroadcast branch from ce8560d to 5c9dedf Compare January 29, 2026 03:03

mzumsande marked this pull request as ready for review January 29, 2026 03:05

vasild mentioned this pull request Feb 2, 2026

Private broadcast tracking #34476

Open

13 tasks

vasild approved these changes Feb 4, 2026

View reviewed changes

test/functional/wallet_resendwallettransactions.py Outdated Show resolved Hide resolved

test/functional/wallet_resendwallettransactions.py Outdated Show resolved Hide resolved

test/functional/wallet_resendwallettransactions.py Show resolved Hide resolved

DrahtBot mentioned this pull request Feb 8, 2026

wallet: resubmit transactions with private broadcast if enabled #34533

Draft

mzumsande force-pushed the 202601_test_rebroadcast branch from 5c9dedf to b2c9eac Compare February 9, 2026 10:21

vasild approved these changes Feb 13, 2026

View reviewed changes

DrahtBot mentioned this pull request Feb 14, 2026

test: Set assert_debug_log timeout to 0 #34581

Merged

DrahtBot added the Needs rebase label Feb 18, 2026

mzumsande force-pushed the 202601_test_rebroadcast branch from b2c9eac to 990e2cd Compare February 21, 2026 07:02

DrahtBot removed the Needs rebase label Feb 21, 2026

vasild approved these changes Feb 23, 2026

View reviewed changes

maflcko reviewed Feb 26, 2026

View reviewed changes

maflcko approved these changes Feb 27, 2026

View reviewed changes

test: add test for rebroadcast of transaction received via p2p

73e3853

The wallet doesn't only rebroadcast transactions it created, but also relevant transactions received via p2p. Since this is not self-evident, add test coverage for it.

mzumsande force-pushed the 202601_test_rebroadcast branch from 990e2cd to 73e3853 Compare February 27, 2026 11:33

DrahtBot requested a review from vasild February 27, 2026 12:20

vasild approved these changes Feb 27, 2026

View reviewed changes

furszy reviewed Feb 28, 2026

View reviewed changes

DrahtBot mentioned this pull request Mar 4, 2026

Multiprocess bitcoin #10102

Draft

danielabrozzoni reviewed Mar 4, 2026

View reviewed changes

sedited merged commit 3201abe into bitcoin:master Mar 11, 2026
26 checks passed

	bool fSendTrickle = node.HasPermission(NetPermissionFlags::NoBan);
	if (tx_relay->m_next_inv_send_time < current_time) {
	fSendTrickle = true;
	if (node.IsInboundConn()) {
	tx_relay->m_next_inv_send_time = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL, node.m_network_key);
	} else {
	tx_relay->m_next_inv_send_time = current_time + m_rng.rand_exp_duration(OUTBOUND_INVENTORY_BROADCAST_INTERVAL);
	}
	}

	self.extra_args = [["-whitelist=relay,noban@127.0.0.1"], ["-whitelist=relay,noban@127.0.0.1"]]
	self.extra_args = [["-whitelist=noban@127.0.0.1"], []]

Conversation

mzumsande commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

vasild commented Jan 22, 2026

Uh oh!

mzumsande commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mzumsande commented Jan 29, 2026

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mzumsande commented Feb 9, 2026

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

mzumsande commented Feb 21, 2026

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mzumsande commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Feb 27, 2026

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

danielabrozzoni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mzumsande commented Jan 21, 2026 •

edited

Loading

DrahtBot commented Jan 21, 2026 •

edited

Loading

mzumsande commented Jan 22, 2026 •

edited

Loading

mzumsande commented Feb 27, 2026 •

edited

Loading