Add wallet birthday to CreateParams
#348
Conversation
There has been discussion offline on how to improve the experience for recovering wallets using block-by-block scanning methods. A birthday has come up multiple times, so I introduce one here in `CreateParams`. I believe the `expect` may only be hit when attempting to over-write the genesis block. If we want to return the error, it would require a break in the error type. IMO this seems unnecessary, as a birthday should not attempt to replace genesis. This may be used immediately in `Wallet::latest_checkpoint` or similar.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #348 +/- ##
==========================================
- Coverage 84.99% 84.93% -0.07%
==========================================
Files 23 23
Lines 8237 8249 +12
==========================================
+ Hits 7001 7006 +5
- Misses 1236 1243 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| .unwrap_or(genesis_block(network).block_hash()); | ||
| let (chain, chain_changeset) = LocalChain::from_genesis_hash(genesis_hash); | ||
| let (mut chain, chain_changeset) = LocalChain::from_genesis_hash(genesis_hash); | ||
| if let Some(birthday) = params.birthday { |
There was a problem hiding this comment.
How will this interact with Esplora or Electrum syncing? I do wonder if we could at least limit how far we go back in full-scan syncing.
There was a problem hiding this comment.
I believe this should have no effect. Both sources are based on script pub keys and I am under the impression the different between sync and full scan should only be a matter of how many scripts are queried. I may be wrong but perhaps @oleonardolima knows more precisely.
| /// Begin wallet scanning from the specified birthday. Particularly useful for block-based | ||
| /// scanning sources. | ||
| pub fn birthday(mut self, birthday: BlockId) -> Self { | ||
| self.birthday = Some(birthday); |
There was a problem hiding this comment.
Hmm, it's kind of odd to having to always having to remember the birthday outside of the wallet and setting it on CreateParams. Shouldn't the wallet itself also remember its birthday, and would it need to be persisted as part of the ChangeSet?
There was a problem hiding this comment.
I agree that it's a better idea to persist the birthday in the ChangeSet, but that'd make it a breaking change.
There was a problem hiding this comment.
One can either immediately persist the change using take_staged, in which case they would use LoadParams next time they interact with the wallet, or they can wait for a sync result, in which case they persist and would also use LoadParams. The birthday is no longer required as long as the persistence succeeds.
|
I was thinking that it should be possible for a chain source's result to automatically set the wallet birthday, instead of having to set it manually based on a result. Not sure if this would require going the |
5 participants
There has been discussion offline on how to improve the experience for recovering wallets using block-by-block scanning methods. A birthday has come up multiple times, so I introduce one here in
CreateParams.I believe the
expectmay only be hit when attempting to over-write the genesis block. If we want to return the error, it would require a break in the error type. IMO this seems unnecessary, as a birthday should not attempt to replace genesis.This may be used immediately in
Wallet::latest_checkpointor similar.Changelog notice
birthdayparameter toCreateParamsChecklists
All Submissions:
just pbefore pushingNew Features:
Bugfixes: