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

1. 
   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

2. 
   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.

3. DrahtBot added the label Wallet on Dec 9, 2022
4. 
   w0xlt commented at 2:23 am on December 10, 2022: contributor

   Approach ACK
5. fanquake added the label Needs backport (24.x) on Dec 10, 2022
6. 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:

   0                // 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
7. 
   ishaanam commented at 0:45 am on December 13, 2022: contributor

   utACK 57f8f618f47526205e515b5ee8f9ab2f1eb625cd
8. 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
9. achow101 force-pushed on Dec 13, 2022
10. 
    ishaanam commented at 0:18 am on December 20, 2022: contributor

    re-utACK 378400953424598fd78ccec5ba8cc38bc253c550
11. 
    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.

12. 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.
13. furszy approved
14. 
    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.
15. achow101 merged this on Jan 11, 2023
16. achow101 closed this on Jan 11, 2023

    ---

17. sidhujag referenced this in commit 6b56fce29a on Jan 11, 2023
18. fanquake referenced this in commit 286c90106a on Jan 12, 2023
19. fanquake removed the label Needs backport (24.x) on Jan 12, 2023
20. 
    fanquake commented at 9:59 am on January 12, 2023: member

    Backported in #26878.
21. fanquake referenced this in commit 24a23d8c6b on Jan 12, 2023
22. fanquake referenced this in commit 428dcd51e6 on Feb 22, 2023
23. glozow referenced this in commit c8c85ca16e on Feb 27, 2023
24. bitcoin locked this on Jan 12, 2024

Contributors
achow101 DrahtBot w0xlt ishaanam furszy maflcko fanquake

Labels
Wallet