wallet: remove outdated arguments from chain scanning methods #35123

pull rkrux wants to merge 4 commits into bitcoin:master from rkrux:remove-fupdate changing 7 files +21 −24

rkrux commented at 2:42 PM on April 20, 2026: contributor

This caught my attention while going through #32993.

The corresponding boolean arguments from CWallet methods related to updating transactions during the blockchain scanning have been removed because effectively these arguments were always passed as true making the need for the boolean arguments unnecessary. The only falsy call sites were in the unit tests that don't need to test scenarios that never happen in actuality.
wallet: remove update argument from RescanFromTime method
Its only usage passes true.
54e4c0be8f

wallet: remove fUpdate argument from ScanForWalletTransactions

Only the unit test code passes false, the actual code passes
true always. I don't see a reason why the tests need to exercise
a behaviour that never happens in production.

6e796e1f47

wallet: remove update_tx argument from SyncTransaction
Its only usage passes true.
94845df073
wallet: remove fUpdate argument from AddToWalletIfInvolvingMe
Its only usage passes true.
dc84a31014
DrahtBot renamed this:
~~wallet: remove outdated arguments from wallet scanning methods~~
wallet: remove outdated arguments from wallet scanning methods
on Apr 20, 2026
DrahtBot added the label Wallet on Apr 20, 2026
DrahtBot commented at 2:42 PM on April 20, 2026: contributor


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/35123.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK Bicaru20, sedited, l0rinc, achow101

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:
- #34861 (wallet: Add importdescriptors interface by polespinasa)
- #34681 (wallet: refactor ScanForWalletTransactions by Eunovo)
- #34400 (wallet: parallel fast rescan (approx 5x speed up with 8 threads) by Eunovo)
- #33392 (wallet, rpc: add UTXO set check and incremental rescan to importdescriptors by musaHaruna)
- #27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
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.
Bicaru20 commented at 9:49 PM on April 20, 2026: none

ACK dc84a3101430cdc0353f610835966f4e4942ff03. I also think that makes sense to remove the update argument if it is not used.
sedited approved
sedited commented at 4:45 PM on May 10, 2026: contributor

ACK dc84a3101430cdc0353f610835966f4e4942ff03

Type	Reviewers
ACK	Bicaru20, sedited, l0rinc, achow101

in src/wallet/wallet.cpp:1224 in dc84a31014

1220 | @@ -1221,7 +1221,6 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS
1221 |          }
1222 |  
1223 |          bool fExisted = mapWallet.contains(tx.GetHash());
1224 | -        if (fExisted && !fUpdate) return false;
1225 |          if (fExisted || IsMine(tx) || IsFromMe(tx))

l0rinc commented at 2:13 PM on May 13, 2026:

dc84a31 wallet: remove fUpdate argument from AddToWalletIfInvolvingMe:

Now that fExisted is only used in a single place we might as well inline it

l0rinc approved
l0rinc commented at 2:38 PM on May 13, 2026: contributor

untested code review ACK dc84a3101430cdc0353f610835966f4e4942ff03

Someone more familiar with the wallet should investigate if removing the correct high-level direction (similarly to #35228 (comment)) - but codewise the change seems correct to me. Happy to reack if my suggestion is taken.
rkrux renamed this:
~~wallet: remove outdated arguments from wallet scanning methods~~
wallet: remove outdated arguments from chain scanning methods
on May 13, 2026
achow101 commented at 9:28 PM on May 13, 2026: member

ACK dc84a3101430cdc0353f610835966f4e4942ff03

The last usage of fUpdate=false was importwallet which was removed with the removal of the legacy wallet.
achow101 merged this on May 13, 2026
achow101 closed this on May 13, 2026

Contributors

Labels

Linked (view graph)

#32993 fuzz: wallet: add target for tx scanning

wallet: remove outdated arguments from chain scanning methods #35123

Code Coverage & Benchmarks

Reviews

Conflicts