Page MenuHomePhabricator
Differential D12206

Small fix to the node broadcast logic
ClosedPublic
Actions

Authored by Fabien on Oct 11 2022, 14:52.

Details

Reviewers
PiRK
Group Reviewers
Restricted Project
Commits
rABC0804a0d47eaf: Small fix to the node broadcast logic
Summary
Currently, if BroadcastTransaction() is called to rebroadcast a
transaction (e.g. by ResendWalletTransactions()), then we add the
transaction to the unbroadcast set. That transaction has already been
broadcast in the past, so peers are unlikely to request it again,
meaning RemoveUnbroadcastTx() won't be called and it won't be removed
from m_unbroadcast_txids.

Net processing will therefore continue to attempt rebroadcast for the
transaction every 10-15 minutes. This will most likely continue until
the node connects to a new peer which hasn't yet seen the transaction
(or perhaps indefinitely).

Fix by only adding the transaction to the broadcast set when it's added to the mempool.

Backport of core#22261.
Depends on D12204.

Only 3/5 commits are relevant to our codebase:
https://github.com/bitcoin/bitcoin/pull/22261/commits/2837a9f1eaa2c6bf402d1d9891d9aa84c4a56033
https://github.com/bitcoin/bitcoin/pull/22261/commits/847b6ed48d7bacec9024618922e9b339d2d97676
https://github.com/bitcoin/bitcoin/pull/22261/commits/5a77abd4e657458852875a07692898982f4b1db5

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien created this revision.Oct 11 2022, 14:52
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 11 2022, 14:52
Fabien requested review of this revision.Oct 11 2022, 14:52
teamcity edited the summary of this revision. (Show Details)Oct 11 2022, 14:52
Harbormaster completed remote builds in B20441: Diff 35583.Oct 11 2022, 15:06
teamcity added a comment.Oct 11 2022, 15:07
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Tail of the build log:

rpc_users.py                              | ✓ Passed  | 5 s
rpc_whitelist.py                          | ✓ Passed  | 1 s
tool_wallet.py                            | ✓ Passed  | 4 s
tool_wallet.py --descriptors              | ✓ Passed  | 2 s
wallet_abandonconflict.py                 | ✓ Passed  | 5 s
wallet_address_types.py                   | ✓ Passed  | 12 s
wallet_address_types.py --descriptors     | ✓ Passed  | 8 s
wallet_avoidreuse.py                      | ✓ Passed  | 4 s
wallet_avoidreuse.py --descriptors        | ✓ Passed  | 4 s
wallet_backup.py                          | ✓ Passed  | 24 s
wallet_balance.py                         | ✓ Passed  | 5 s
wallet_balance.py --descriptors           | ✓ Passed  | 7 s
wallet_basic.py                           | ✓ Passed  | 16 s
wallet_coinbase_category.py               | ✓ Passed  | 1 s
wallet_create_tx.py                       | ✓ Passed  | 6 s
wallet_createwallet.py                    | ✓ Passed  | 2 s
wallet_createwallet.py --descriptors      | ✓ Passed  | 3 s
wallet_createwallet.py --usecli           | ✓ Passed  | 3 s
wallet_descriptor.py                      | ✓ Passed  | 6 s
wallet_disable.py                         | ✓ Passed  | 0 s
wallet_dump.py                            | ✓ Passed  | 4 s
wallet_encryption.py                      | ✓ Passed  | 5 s
wallet_encryption.py --descriptors        | ✓ Passed  | 5 s
wallet_groups.py                          | ✓ Passed  | 8 s
wallet_hd.py                              | ✓ Passed  | 5 s
wallet_hd.py --descriptors                | ✓ Passed  | 5 s
wallet_import_rescan.py                   | ✓ Passed  | 5 s
wallet_import_with_label.py               | ✓ Passed  | 1 s
wallet_importdescriptors.py               | ✓ Passed  | 3 s
wallet_importmulti.py                     | ✓ Passed  | 3 s
wallet_importprunedfunds.py               | ✓ Passed  | 2 s
wallet_importprunedfunds.py --descriptors | ✓ Passed  | 2 s
wallet_keypool.py                         | ✓ Passed  | 3 s
wallet_keypool_topup.py                   | ✓ Passed  | 3 s
wallet_keypool_topup.py --descriptors     | ✓ Passed  | 3 s
wallet_labels.py                          | ✓ Passed  | 1 s
wallet_labels.py --descriptors            | ✓ Passed  | 2 s
wallet_listreceivedby.py                  | ✓ Passed  | 5 s
wallet_listsinceblock.py                  | ✓ Passed  | 5 s
wallet_listsinceblock.py --descriptors    | ✓ Passed  | 6 s
wallet_listtransactions.py                | ✓ Passed  | 4 s
wallet_listtransactions.py --descriptors  | ✓ Passed  | 3 s
wallet_multiwallet.py                     | ✓ Passed  | 39 s
wallet_multiwallet.py --usecli            | ✓ Passed  | 11 s
wallet_reorgsrestore.py                   | ✓ Passed  | 3 s
wallet_resendwallettransactions.py        | ✓ Passed  | 4 s
wallet_send.py                            | ✓ Passed  | 7 s
wallet_startup.py                         | ✓ Passed  | 2 s
wallet_txn_clone.py                       | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock           | ✓ Passed  | 3 s
wallet_txn_doublespend.py                 | ✓ Passed  | 1 s
wallet_txn_doublespend.py --mineblock     | ✓ Passed  | 4 s
wallet_watchonly.py                       | ✓ Passed  | 1 s
wallet_watchonly.py --usecli              | ✓ Passed  | 1 s

ALL                                       | ✓ Passed  | 1169 s (accumulated) 
Runtime: 234 s

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1
Fabien added a comment.Oct 11 2022, 15:23

Unrelated spurious leveldb failure

PiRK accepted this revision.Oct 11 2022, 15:29
PiRK added a subscriber: PiRK.

Looks good. Test failure is unrelated (leveldb test)

This revision is now accepted and ready to land.Oct 11 2022, 15:29
This revision was landed with ongoing or failed builds.Oct 11 2022, 18:15
Closed by commit rABC0804a0d47eaf: Small fix to the node broadcast logic (authored by Fabien). · Explain Why
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC0804a0d47eaf: Small fix to the node broadcast logic.

Revision Contents
Changeset List

PathSize
src/
node/
24 lines
test/
functional/
7 lines

Diff 35589

View Options

src/node/transaction.cpp

Loading...
View Options

test/functional/mempool_unbroadcast.py

Loading...