Improve wallet rescan API #10412
Improve wallet rescan API #10412
Conversation
|
utACK 4182e6c |
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).
No change in behavior.
|
Rebased 2de5c79 -> 9bb66ab (pr/scanclean.2 -> pr/scanclean.3) |
|
|
||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Shouldnt the docs (here and above) be in the .h and not the .cpp?
There was a problem hiding this comment.
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.
|
|
||
| /** | ||
| * Scan active chain for relevant transactions after importing keys. This should | ||
| * be called whenever new keys are added to the wallet, with the oldest key |
There was a problem hiding this comment.
Maybe move TIMESTAMP_WINDOW into an implicit thing that this function applies to the provided timestamp (and move its definition out of wallet.h)?
There was a problem hiding this comment.
Done in deaf48b. 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.
This way CWallet::RescanFromTime callers don't need to subtract TIMESTAMP_WINDOW themselves. This is pure refactoring, there is no change in behavior.
There was a problem hiding this comment.
Added 2 commits 9bb66ab -> deaf48b (pr/scanclean.3 -> pr/scanclean.5, compare)
|
|
||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
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.
|
|
||
| /** | ||
| * Scan active chain for relevant transactions after importing keys. This should | ||
| * be called whenever new keys are added to the wallet, with the oldest key |
There was a problem hiding this comment.
Done in deaf48b. 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.
|
utACK deaf48b. |
deaf48b Handle TIMESTAMP_WINDOW within CWallet::RescanFromTime (Russell Yanofsky) 5b2be2b Make CWallet::RescanFromTime comment less ambiguous (Russell Yanofsky) 9bb66ab Add RescanFromTime method and use from rpcdump (Russell Yanofsky) ccf84bb Move birthday optimization out of ScanForWalletTransactions (Russell Yanofsky) Tree-SHA512: cd38433b8f5c5e44ecfba830a6a26bd9a9d0f4a22ae42bce17773d1a6fb25e1ee4289484996dad2d7acfa03059917ff062459f25030a761da7083ba5fbc87bc9
deaf48b Handle TIMESTAMP_WINDOW within CWallet::RescanFromTime (Russell Yanofsky) 5b2be2b Make CWallet::RescanFromTime comment less ambiguous (Russell Yanofsky) 9bb66ab Add RescanFromTime method and use from rpcdump (Russell Yanofsky) ccf84bb Move birthday optimization out of ScanForWalletTransactions (Russell Yanofsky) Tree-SHA512: cd38433b8f5c5e44ecfba830a6a26bd9a9d0f4a22ae42bce17773d1a6fb25e1ee4289484996dad2d7acfa03059917ff062459f25030a761da7083ba5fbc87bc9
…ng RescanFromTime 1cde8b4 [Trivial] Fix some comment/output: 'bitcoind' --> 'pivxd' (random-zebra) 29c267e Mention that other RPC calls report keys as "imported" while txns are still missing (Jonas Schnelli) f773203 Reduce cs_main lock in ReadBlockFromDisk, only read GetBlockPos under the lock (Jonas Schnelli) 3e01665 Make sure WalletRescanReserver has successfully reserved the rescan (random-zebra) 4d05672 Add RAII wallet rescan reserver (Jonas Schnelli) c3fc22d Avoid pemanent cs_main/cs_wallet lock during wallet rescans (random-zebra) 69a7288 Handle TIMESTAMP_WINDOW within CWallet::RescanFromTime (Russell Yanofsky) fb12767 Make CWallet::RescanFromTime comment less ambiguous (Russell Yanofsky) 2d84bee Add RescanFromTime method and use from rpcdump (Russell Yanofsky) e8dfb32 Move birthday optimization out of ScanForWalletTransactions (random-zebra) Pull request description: Based on top of: - [x] #2279 Backports bitcoin#10412, and bitcoin#11281 ACKs for top commit: furszy: ACK 1cde8b4 and merging Tree-SHA512: 1c76ca9b7925299df796f72e171d70d1df9765d3b8d5976d7d280dd318474911cd8f173f686e57422a6f77fef145e4a9aae480b293e6b77ca4f93750688e997c
5 participants
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.