-
Notifications
You must be signed in to change notification settings - Fork 725
[Wallet] Improve rescan API + avoid permanent locks during RescanFromTime #2281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Wallet] Improve rescan API + avoid permanent locks during RescanFromTime #2281
Conversation
>>> backports bitcoin/bitcoin@ccf84bb 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.
This way CWallet::RescanFromTime callers don't need to subtract TIMESTAMP_WINDOW themselves. This is pure refactoring, there is no change in behavior.
>>> backports bitcoin/bitcoin@bc356b4 and adapt RPC importsaplingkey and importsaplingviewingkey
a01d3cd to
29c267e
Compare
|
Rebased on master. Ready to go. |
furszy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial quick code review ACK 29c267e
Overall is looking really nice 👌
Fuzzbawls
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functional ACK
Great improvement in terms of the wallet's responsiveness! have been able to import/rescan addresses and wallets with thousands of transactions without issue.
One commented change in the importmulti help output that shouldn't be, otherwise good to go
Fuzzbawls
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 29c267e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 1cde8b4 and merging
Based on top of:
Backports bitcoin#10412, and bitcoin#11281