BroadcastTransaction() now waits for validation callbacks using the built-in validation_signals>SyncWithValidationInterfaceQueue() instead of creating a local std::promise and scheduling a lambda. This removes an unnecessary allocation and uses the canonical API.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33962.
See the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
111@@ -115,9 +112,6 @@ TransactionError BroadcastTransaction(NodeContext& node,
112 // with a transaction to/from their wallet, immediately call some
113 // wallet RPC, and get a stale result because callbacks have not
114 // yet been processed.
115- node.validation_signals->CallFunctionInValidationInterfaceQueue([&promise] {
116- promise.set_value();
117- });
118 callback_set = true;
Nit: The callback_set variable seems unnecessary now.
0diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp
1index 05450299e5..0b6213f7fd 100644
2--- a/src/node/transaction.cpp
3+++ b/src/node/transaction.cpp
4@@ -45,7 +45,6 @@ TransactionError BroadcastTransaction(NodeContext& node,
5
6 Txid txid = tx->GetHash();
7 Wtxid wtxid = tx->GetWitnessHash();
8- bool callback_set = false;
9
10 {
11 LOCK(cs_main);
12@@ -102,22 +101,19 @@ TransactionError BroadcastTransaction(NodeContext& node,
13 }
14 break;
15 }
16-
17- if (wait_callback && node.validation_signals) {
18- // For transactions broadcast from outside the wallet, make sure
19- // that the wallet has been notified of the transaction before
20- // continuing.
21- //
22- // This prevents a race where a user might call sendrawtransaction
23- // with a transaction to/from their wallet, immediately call some
24- // wallet RPC, and get a stale result because callbacks have not
25- // yet been processed.
26- callback_set = true;
27- }
28 }
29 } // cs_main
30
31- if (callback_set) {
32+ if (wait_callback && node.validation_signals) {
33+ // For transactions broadcast from outside the wallet, make sure
34+ // that the wallet has been notified of the transaction before
35+ // continuing.
36+ //
37+ // This prevents a race where a user might call sendrawtransaction
38+ // with a transaction to/from their wallet, immediately call some
39+ // wallet RPC, and get a stale result because callbacks have not
40+ // yet been processed.
41+
42 // Wait until Validation Interface clients have been notified of the
43 // transaction entering the mempool.
44 node.validation_signals->SyncWithValidationInterfaceQueue();
Nit: The
callback_setvariable seems unnecessary now.
Why would it be unnecessary now? Sure, it is harmless to call SyncWithValidationInterfaceQueue when the tx is in the mempool, but this will need to be mentioned
ACK d09e9306cab86c0917cc0ae198ef1993d764d106
Looks fine, this function doc highlights the similarity:
🚧 At least one of the CI tasks failed.
Task lint: https://github.com/bitcoin/bitcoin/actions/runs/19769125602/job/56649201966
LLM reason (✨ experimental): CI failed due to trailing whitespace detected by lint in src/node/transaction.cpp.
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
