[WIP] wallet: use BlockFilterIndex in ScanForWalletTransactions #19116

pull pstratem wants to merge 4 commits into bitcoin:master from pstratem:2020-05-29-generate-pubkeys changing 6 files +66 −5

pstratem commented at 10:24 PM on May 30, 2020: contributor

goal is to improve rescan performance significantly, lots more work to do
wallet: add ScriptPubKeyMan::GetScriptPubKeys virtual function
LegacyScriptPubKey::GetScriptPubKeys is a stub
55c9ced642
pstratem force-pushed on May 30, 2020
wallet: partially implement LegacyScriptPubKeyMan::GetScriptPubKeys
implements p2pkh
872661b298
MarcoFalke commented at 10:57 PM on May 30, 2020: member

Have you seen #15845 ? It might be easier to take that code and then fix up the two (or three) bugs.
MarcoFalke commented at 11:01 PM on May 30, 2020: member

Concept ACK either way
pstratem commented at 11:07 PM on May 30, 2020: contributor

@MarcoFalke I hadn't, it seems you took basically the same approach that I intend to. I'll certainly review your PR and read the comments.
DrahtBot added the label Wallet on May 30, 2020
chain: add filterMatchesAny to chain interface
taken from #15845
b59f89b1fd
DrahtBot commented at 5:39 AM on May 31, 2020: contributor


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #19136 (wallet: add dumpwalletdescriptor RPC by achow101)
- #18982 (wallet: Minimal fix to restore conflicted transaction notifications by ryanofsky)
- #15719 (Wallet passive startup by ryanofsky)
- #10102 ([experimental] Multiprocess bitcoin by ryanofsky)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
pstratem commented at 4:27 AM on June 1, 2020: contributor

this actually works for p2pkh transactions as it is now, benchmarks incoming
pstratem force-pushed on Jun 1, 2020
pstratem force-pushed on Jun 1, 2020

wallet: Use blockfilterindex "basic" to accelerate rescan time

we avoid issues with the keypool expanding during a rescan with the
script_pub_key_filter_set flag

fe107fcdfe

pstratem force-pushed on Jun 2, 2020
DrahtBot added the label Needs rebase on Jun 2, 2020
DrahtBot commented at 10:29 PM on June 2, 2020: contributor



🐙 This pull request conflicts with the target branch and needs rebase.
pstratem commented at 8:41 PM on June 6, 2020: contributor

some rescanblockchain benchmarks, from my laptop:

42 minutes on master 20 minutes using the BASIC filter 4 minutes using a filter with fixed siphash parameters and avoiding recalculating the siphashes for every block
pstratem marked this as a draft on Jun 7, 2020
jonatack commented at 5:59 AM on June 7, 2020: contributor

Concept ACK
ryanofsky commented at 9:47 PM on June 30, 2020: contributor

Concept & approach ACK. Seems to pick up nicely where #15845 left off, updated to work for descriptor and legacy wallets
jb55 commented at 9:06 PM on November 26, 2020: contributor

Concept ACK as I grumpily wait while my wallets are rescanning

in src/interfaces/chain.cpp:190 in fe107fcdfe

 186 | @@ -186,6 +187,20 @@ class ChainImpl : public Chain
 187 |          }
 188 |          return nullopt;
 189 |      }
 190 | +    Optional<bool> filterMatchesAny(const uint256& hash, const GCSFilter::ElementSet& filter_set) override

fanquake commented at 2:15 AM on March 15, 2021:

Please use std::optional in new code.

DrahtBot commented at 11:21 AM on December 15, 2021: contributor
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
- Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
- Is it no longer relevant? ➡️ Please close.
- Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.
DrahtBot commented at 1:07 PM on March 21, 2022: contributor
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
- Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
- Is it no longer relevant? ➡️ Please close.
- Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.
MarcoFalke closed this on Mar 21, 2022
MarcoFalke added the label Up for grabs on Mar 21, 2022
MarcoFalke commented at 1:59 PM on March 21, 2022: member

Marked up for grabs, along with #15845
MarcoFalke commented at 5:51 AM on September 8, 2022: member

Picked up in #25957
MarcoFalke removed the label Up for grabs on Sep 8, 2022
bitcoin locked this on Sep 8, 2023

Contributors

Labels

[WIP] wallet: use BlockFilterIndex in ScanForWalletTransactions #19116

Conflicts