wallet: Skip rescanning if wallet is more recent than tip #26679

pull achow101 wants to merge 1 commits into bitcoin:master from achow101:skip-newer-wallet-rescan changing 1 files +18 −11

achow101 commented at 10:16 PM on December 9, 2022: member

If a wallet has key birthdates that are more recent than the currrent chain tip, or a bestblock height higher than the current tip, we should not attempt to rescan as there is nothing to scan for.

Fixes #26655
DrahtBot commented at 10:16 PM on December 9, 2022: contributor



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



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK ishaanam, w0xlt, furszy

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Wallet on Dec 9, 2022
w0xlt commented at 2:23 AM on December 10, 2022: contributor

Approach ACK
fanquake added the label Needs backport (24.x) on Dec 10, 2022
NicolasDorier cross-referenced this on Dec 12, 2022 from issue NBXplorer create default RPC wallet when possible by NicolasDorier
NicolasDorier cross-referenced this on Dec 12, 2022 from issue Pruned node unable to start when a wallet is present by NicolasDorier

Type	Reviewers
ACK	ishaanam, w0xlt, furszy

in src/wallet/wallet.cpp:3198 in 57f8f618f4 outdated

3193 | +        }
3194 | +        if (time_first_key) {
3195 | +            FoundBlock found = FoundBlock().height(rescan_height);
3196 | +            chain.findFirstBlockWithTimeAndHeight(*time_first_key - TIMESTAMP_WINDOW, rescan_height, found);
3197 | +            if (!found.found) {
3198 | +                // We were unable to find a block a block that had a time more recent than our earliest timestamp

ishaanam commented at 11:01 PM on December 12, 2022:

nit:

                // We were unable to find a block that had a time more recent than our earliest timestamp

achow101 commented at 8:55 PM on December 13, 2022:

Done

ishaanam commented at 12:45 AM on December 13, 2022: contributor

utACK 57f8f618f47526205e515b5ee8f9ab2f1eb625cd

wallet: Skip rescanning if wallet is more recent than tip

If a wallet has key birthdates that are more recent than the currrent
chain tip, or a bestblock height higher than the current tip, we should
not attempt to rescan as there is nothing to scan for.

3784009534

achow101 force-pushed on Dec 13, 2022
ishaanam commented at 12:18 AM on December 20, 2022: contributor

re-utACK 378400953424598fd78ccec5ba8cc38bc253c550
w0xlt commented at 3:20 AM on December 20, 2022: contributor

utACK https://github.com/bitcoin/bitcoin/pull/26679/commits/378400953424598fd78ccec5ba8cc38bc253c550

Ci error seems unrelated.
in src/wallet/wallet.cpp:3195 in 3784009534
```
3190 | +        for (auto spk_man : walletInstance->GetAllScriptPubKeyMans()) {
3191 | +            int64_t time = spk_man->GetTimeFirstKey();
3192 | +            if (!time_first_key || time < *time_first_key) time_first_key = time;
3193 | +        }
3194 | +        if (time_first_key) {
3195 | +            FoundBlock found = FoundBlock().height(rescan_height);
```
furszy commented at 2:14 PM on January 9, 2023:

what is the reason behind the height set here? It doesn't seems to be used anywhere inside findFirstBlockWithTimeAndHeight nor below.

maflcko commented at 2:34 PM on January 9, 2023:

It is used in ScanForWalletTransactions, below?

furszy commented at 3:03 PM on January 9, 2023:

yeah ok, digested it now. thanks. Was expecting FoundBlock to be an inputs-only container when it's actually the outputs container.
furszy approved
furszy commented at 7:39 PM on January 9, 2023: member

Code review ACK 37840095 A test covering the existent failure would be nice too.
achow101 merged this on Jan 11, 2023
achow101 closed this on Jan 11, 2023
sidhujag referenced this in commit 6b56fce29a on Jan 11, 2023
fanquake referenced this in commit 286c90106a on Jan 12, 2023
fanquake cross-referenced this on Jan 12, 2023 from issue [24.x] Backports by fanquake
fanquake removed the label Needs backport (24.x) on Jan 12, 2023
fanquake commented at 9:59 AM on January 12, 2023: member

Backported in #26878.
fanquake referenced this in commit 24a23d8c6b on Jan 12, 2023
ryanofsky cross-referenced this on Feb 13, 2023 from issue Multiprocess bitcoin by ryanofsky
fanquake referenced this in commit 428dcd51e6 on Feb 22, 2023
glozow referenced this in commit c8c85ca16e on Feb 27, 2023
maflcko cross-referenced this on Mar 3, 2023 from issue Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data") by GregTonoski
pinheadmz cross-referenced this on May 25, 2023 from issue wallet: improve IBD sync time by skipping block scanning prior birth time by furszy
bitcoin locked this on Jan 12, 2024

Contributors

Labels

Linked (view graph)

#10102 Multiprocess bitcoin #26655 Pruned node unable to start when a wallet is present #26878 [24.x] Backports #27188 Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")#27469 wallet: improve IBD sync time by skipping block scanning prior birth time

wallet: Skip rescanning if wallet is more recent than tip #26679

Reviews