From: Steve Myers <steve@notmandatory.org>
Date: Thu, 14 Apr 2022 16:56:46 +0000 (-0700)
Subject: Merge bitcoindevkit/bdk#583: Improve keys traits, simplify `rpcwallet` example
X-Git-Tag: v0.18.0~4^2
X-Git-Url: http://internal-gitweb-vhost/script/%22https:/enum.FileStoreError.html?a=commitdiff_plain;h=a328607d27feb5d556df99ec6beabc41cc655160;p=bdk

Merge bitcoindevkit/bdk#583: Improve keys traits, simplify `rpcwallet` example

44758f9483b049acbcd9f8948e29cd6f6add0cc2 Simplify the `rpcwallet` example using the improved keys traits (Alekos Filini)
c350064dae43fd9c892bb7dd272b14ce62f9e82d Update changelog changes in `keys` mod (Alekos Filini)
92746440db964b5e50f4017828516b36eb14e765 [keys] Make `GenerateKey<K>` clonable if K is (Alekos Filini)
e4eb95fb9cfbae3596deb310984fa18462d1b30c [keys] Implement `DerivableKey<Ctx>` for `(GeneratedKey<Mnemonic, Ctx>, Option<String>)` (Alekos Filini)

Pull request description:

  ### Description

  I recently looked at the `rpcwallet` example and immediately thought the key generation part was very confusing. I made a few little changes to our traits and now I believe it's much better.

  I'm copying the next few paragraphs from the first commit in this PR which explains why it's better not to extract the key from the `GeneratedKey` wrapper:

  > This lets us use a tuple of (generated mnemonic, optional passphrase) as a `DerivableKey` directly, without extracting the inner mnemonic from the `GeneratedKey` wrapper. For BIP39 keys specifically it doesn't make much difference because the mnemonic format doesn't encode the network, but in general this is not the case and having a consistent API will make it harder for people to make mistakes.

  > To explain why we should not extract the key: some key formats (like BIP32 extended keys) are network-specific, meaning that if somebody tries to use a Testnet key on a Mainnet BDK wallet, it won't work.

  > However, when we generate a new key we would like to be able to use that key on any network, but we need to set some kind of placeholder for the `network` field in the structure. This is why (or at least one of the reasons why) we wrap the key in the `GeneratedKey` struct: we keep track of the "valid_networks" separately, which means that even if we set our BIP32 xprv to be a "Mainnet" key, once we go try creating a wallet with that key BDK is smart enough to understand that `GeneratedKey`s have  their own separate set of valid networks and it will use that set to validate whether the key can be used in the wallet or not.

  ### Notes to the reviewers

  I'm adding this to the `0.18` milestone even if the feature freeze is today. I don't think we should wait for this PR to start the release branch, but since it's just a minor addition to our traits I believe we could merge this afterwards to the release branch if we manage to get it reviewed and ready within the next week.

  Otherwise it'll just wait for `0.19`, it's not a big deal.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [ ] I've added tests for the new feature
  * [ ] I've added docs for the new feature
  * [x] I've updated `CHANGELOG.md`

ACKs for top commit:
  notmandatory:
    tACK 44758f9483b049acbcd9f8948e29cd6f6add0cc2

Tree-SHA512: 31a8b5a3cd56c7c39688c4804026a8b6cee950c50ea880efcddc150ef523e2ddcc544831bf996a58b5cf3b7bc4a086c1008dc0728604e5134d331c68855c2d5b
---

a328607d27feb5d556df99ec6beabc41cc655160