Improve wallet rescan API #10412

pull ryanofsky wants to merge 4 commits into bitcoin:master from ryanofsky:pr/scanclean changing 3 files +51 −22

ryanofsky commented at 8:03 PM on May 16, 2017: member

This PR is just refactoring. There are no changes in behavior. It implements a request from @TheBlueMatt in #9827 (comment) to make rescan calls use timestamps instead of block index pointers.
ryanofsky force-pushed on May 16, 2017
fanquake added the label Refactoring on May 17, 2017
fanquake added the label RPC/REST/ZMQ on May 17, 2017
fanquake added the label Wallet on May 17, 2017
jonasschnelli commented at 6:44 AM on May 24, 2017: contributor

utACK 4182e6c89640f8745f0461f5ef610f343ef26296

Move birthday optimization out of ScanForWalletTransactions

This change has no effect on wallet behavior.

On wallet startup, the transaction scan avoids reading any blocks with
timestamps older than the wallet birthday (less than nTimeFirstKey -
TIMESTAMP_WINDOW). This block skipping code currently resides in
CWallet::ScanForWalletTransactions but it doesn't really belong there because
it makes the implementation unnecessarily fragile and hard to understand, and
it never has any effect except at startup (because all other callers do their
rescans based on timestamps other than, but always greater or equal to,
nTimeFirstKey).

ccf84bb9c1

Add RescanFromTime method and use from rpcdump
No change in behavior.
9bb66ab660
ryanofsky force-pushed on Jun 5, 2017
ryanofsky commented at 3:49 PM on June 5, 2017: member

Rebased 2de5c79de7536c0edbfa4e0cb0175a9b8608d720 -> 9bb66ab66013fc3024fad00bceeb22a330b5bc1b (pr/scanclean.2 -> pr/scanclean.3)
in src/wallet/wallet.cpp:1466 in 9bb66ab660 outdated
```
1462 | @@ -1459,6 +1463,34 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
1463 |  }
1464 |  
1465 |  /**
1466 | + * Scan active chain for relevant transactions after importing keys. This should
```
TheBlueMatt commented at 8:38 PM on June 22, 2017:

Shouldnt the docs (here and above) be in the .h and not the .cpp?

ryanofsky commented at 8:54 PM on June 22, 2017:

I put the docs here to be consistent with CWallet::ScanForWalletTransactions below. But I'd be happy to move all the wallet method documentation to one place or the other in a followup commit or PR if there's a correct place for it.
in src/wallet/wallet.cpp:1468 in 9bb66ab660 outdated
```
1462 | @@ -1459,6 +1463,34 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
1463 |  }
1464 |  
1465 |  /**
1466 | + * Scan active chain for relevant transactions after importing keys. This should
1467 | + * be called whenever new keys are added to the wallet, with the oldest key
1468 | + * creation time minus TIMESTAMP_WINDOW.
```
TheBlueMatt commented at 8:39 PM on June 22, 2017:

Maybe move TIMESTAMP_WINDOW into an implicit thing that this function applies to the provided timestamp (and move its definition out of wallet.h)?

ryanofsky commented at 9:24 PM on June 22, 2017:

Done in deaf48b046e573f6774d19e74b58918ed777cb14. I think it's probably a good change, though it doesn't allow hiding the TIMESTAMP_WINDOW constant in wallet, because it's still used by the pruneblockchain implementation and also in importmulti error messages.
TheBlueMatt commented at 8:43 PM on June 22, 2017: member

utACK 9bb66ab66013fc3024fad00bceeb22a330b5bc1b
Make CWallet::RescanFromTime comment less ambiguous 5b2be2b787

Handle TIMESTAMP_WINDOW within CWallet::RescanFromTime

This way CWallet::RescanFromTime callers don't need to subtract
TIMESTAMP_WINDOW themselves.

This is pure refactoring, there is no change in behavior.

deaf48b046

ryanofsky commented at 9:27 PM on June 22, 2017: member

Added 2 commits 9bb66ab66013fc3024fad00bceeb22a330b5bc1b -> deaf48b046e573f6774d19e74b58918ed777cb14 (pr/scanclean.3 -> pr/scanclean.5, compare)
ryanofsky force-pushed on Jun 22, 2017
laanwj commented at 2:40 PM on June 24, 2017: member

utACK deaf48b.
laanwj merged this on Jun 24, 2017
laanwj closed this on Jun 24, 2017
laanwj referenced this in commit bef02fb6ac on Jun 24, 2017
PastaPastaPasta referenced this in commit d9be0fdbcc on Jul 6, 2019
PastaPastaPasta referenced this in commit 9670af2293 on Jul 8, 2019
PastaPastaPasta referenced this in commit e0d15e0db3 on Jul 9, 2019
PastaPastaPasta referenced this in commit 5eca7445a9 on Jul 11, 2019
barrystyle referenced this in commit 92c180eb1d on Jan 22, 2020
furszy referenced this in commit 169260c6aa on Apr 13, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Refactoring Wallet RPC/REST/ZMQ

Linked (view graph)

#9827 Improve ScanForWalletTransactions return value