bdk
bdk copied to clipboard
Slight rethinking of `Wallet` constructors (breaking change)
Why This PR
Our current "constructors" on the Wallet type are as follow:
1. Wallet::new()
2. Wallet::new_with_genesis_hash()
3. Wallet::load_from_changeset()
4. Wallet::new_or_load()
5. Wallet::new_or_load_with_genesis_hash()
This PR proposes a revised set of constructors offering clearer boundaries between them and their intended use:
1. Wallet::new()
2. Wallet::new_with_genesis_hash()
3. Wallet::load()
4. Wallet::load_from_changeset() -> renamed load_no_priv()
A few observations on our current set of constructors and what led me to propose this PR:
Wallet.new_or_load(). So... do you want to create a new wallet or load an existing one? It feels like you should know. If you don't know whether you're about to create a new wallet or load one from persistence, to me your issue lies upstream of this line in your code (but maybe there is a use case I'm not thinking of? let me know).- The
new_or_load_with_genesis_hash()is sort of named backwards (you don't load with a specific genesis hash, you can only do that on a new wallet). The clearer name for it would benew_with_genesis_hash_or_load(). - Loading a wallet is always done with a changeset; there is no other way to do it. In this sense,
Wallet.load_from_changeset()is not optimally named. In fact, the name is also missing the core feature of the constructor: your wallet will not have private keys. An extra commit on this PR renames this method toload_no_priv(); feel free to bikeshed on the name I don't know that it's a particular good one yet and we can also just take that commit out if people like the load_from_changeset name.
Note that this PR doesn't touch the inner logic of the constructors. I simply renamed the methods, removed the Option on the changeset when calling the load methods, and fixed up the tests to break apart the behaviour tested. It did require removing the NewOrLoadError and adding the required variants to the LoadError.
Changelog notice
Breaking
- The Wallet.new_or_load() method was removed [#500]
- The Wallet.load_from_changeset() method was renamed Wallet.load_no_priv() [#500]
Added
- A new method Wallet.load() was added to construct wallets [#500]
[#500]: https://github.com/bitcoindevkit/bdk/pull/1500
Checklists
All Submissions:
- [x] I've signed all my commits
- [x] I followed the contribution guidelines
- [x] I ran
cargo fmtandcargo clippybefore committing
New Features:
- [x] I've added tests for the new feature
- [x] I've added docs for the new feature