-
Notifications
You must be signed in to change notification settings - Fork 84
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
dcr: enable rescans for spv wallets #2767
base: master
Are you sure you want to change the base?
Conversation
80f00f5
to
989ca7e
Compare
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.
We should also reset the last query in the transaction db. Also in BTC.
client/asset/dcr/spv.go
Outdated
@@ -1030,6 +1031,11 @@ func newVSPClient(w vspclient.Wallet, vspHost, vspPubKey string, log dex.Logger) | |||
}, log) | |||
} | |||
|
|||
// Rescan performs a blocking rescan, sending updates on the channel. | |||
func (w *spvWallet) Rescan(ctx context.Context, c chan wallet.RescanProgress) { | |||
w.dcrWallet.RescanProgressFromHeight(ctx, w.spv, 0, c) |
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.
Instead of 0, why not start rescanning from the earliest possible wallet birthday. We have a defaultWalletBirthday
in btc.
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.
Should use the new seed's encoded birthday time.
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.
I've removed birthday configuration from spv wallets altogether and plumbed through the seed birthday where it's needed. This does mean that wallets that use an old seed for a rescan or wallet recovery will use the default wallet birthday from 2021. This actually doesn't matter to recovery of a dcr wallet, since dcr always scans the full blockchain for the initial scan. dcrwallet actually has no concept of a birthday, from what I can tell. But we still use the seed birthday for limiting the scope of a rescan.
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.
You can rescan from a birthday in the next release. decred/dcrwallet@5e8f132
@@ -211,24 +204,6 @@ func (w *btcSPVWallet) Start() (SPVService, error) { | |||
w.chainClient = chain.NewNeutrinoClient(w.chainParams, w.cl) | |||
w.Wallet = btcw | |||
|
|||
oldBday := btcw.Manager.Birthday() |
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.
updateDBBirthday
is no longer used. Can be deleted in the btc, bch, and ltc wallets.
@@ -284,25 +284,6 @@ func CommonConfigOpts(symbol string /* upper-case */, withApiFallback bool) []*a | |||
return opts | |||
} | |||
|
|||
// SPVConfigOpts are the options common to built-in SPV wallets. | |||
func SPVConfigOpts(symbol string) []*asset.ConfigOption { | |||
return []*asset.ConfigOption{{ |
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.
Now existing users will all have to rescan from the default birthday. This feature was very helpful for me once when I has having a hard time getting the wallet to sync and I knew the wallet was completely empty before I deposited coins just a few days earlier.
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.
I think this is worth it to keep things simple, although I also was using this to get sync to work. New users won't need to do anything though after this change.
We shouldn't build around bugs though, and btc not syncing is some bug or bugs in neutrino I think. Hopefully they get fixed...
// Begin potentially asynchronous wallet rescan operation. | ||
if err = wallet.rescan(c.ctx); err != nil { | ||
if err = wallet.rescan(c.ctx, bday); err != nil { |
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.
Why do we need to pass the birthday here if it's already provided in CreateWallet
? I see you used it in dcr but not btc.
Makes Decred SPV wallet an
asset.Rescanner