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

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add `sqlite-bundled` feature for deployments that need a bundled version of sqlite, ie. for mobile platforms.
 - Added `Wallet::get_signers()`, `Wallet::descriptor_checksum()` and `Wallet::get_address_validators()`, exposed the `AsDerived` trait.
 - Deprecate `database::Database::flush()`, the function is only needed for the sled database on mobile, instead for mobile use the sqlite database.
+- Improve key generation traits
 
 ## [v0.17.0] - [v0.16.1]
 
@@ -441,4 +442,4 @@ final transaction is created by calling `finish` on the builder.
 [v0.16.0]: https://github.com/bitcoindevkit/bdk/compare/v0.15.0...v0.16.0
 [v0.16.1]: https://github.com/bitcoindevkit/bdk/compare/v0.16.0...v0.16.1
 [v0.17.0]: https://github.com/bitcoindevkit/bdk/compare/v0.16.1...v0.17.0
-[unreleased]: https://github.com/bitcoindevkit/bdk/compare/v0.17.0...HEAD
+[unreleased]: https://github.com/bitcoindevkit/bdk/compare/v0.17.0...HEAD

examples/rpcwallet.rs

@@ -7,7 +7,6 @@
 // licenses.
 
 use bdk::bitcoin::secp256k1::Secp256k1;
-use bdk::bitcoin::util::bip32::ExtendedPrivKey;
 use bdk::bitcoin::Amount;
 use bdk::bitcoin::Network;
 use bdk::bitcoincore_rpc::RpcApi;
@@ -16,7 +15,7 @@ use bdk::blockchain::rpc::{Auth, RpcBlockchain, RpcConfig};
 use bdk::blockchain::ConfigurableBlockchain;
 
 use bdk::keys::bip39::{Language, Mnemonic, WordCount};
-use bdk::keys::{DerivableKey, ExtendedKey, GeneratableKey, GeneratedKey};
+use bdk::keys::{DerivableKey, GeneratableKey, GeneratedKey};
 
 use bdk::miniscript::miniscript::Segwitv0;
 
@@ -85,8 +84,8 @@ fn main() -> Result<(), Box<dyn Error>> {
     // create unique wallet name.
     // This is a special utility function exposed via `bdk::wallet_name_from_descriptor()`
     let wallet_name = wallet_name_from_descriptor(
-        Bip84(xprv, KeychainKind::External),
-        Some(Bip84(xprv, KeychainKind::Internal)),
+        Bip84(xprv.clone(), KeychainKind::External),
+        Some(Bip84(xprv.clone(), KeychainKind::Internal)),
         Network::Regtest,
         &Secp256k1::new(),
     )?;
@@ -112,8 +111,8 @@ fn main() -> Result<(), Box<dyn Error>> {
 
     // Combine Database + Descriptor to create the final wallet
     let wallet = Wallet::new(
-        Bip84(xprv, KeychainKind::External),
-        Some(Bip84(xprv, KeychainKind::Internal)),
+        Bip84(xprv.clone(), KeychainKind::External),
+        Some(Bip84(xprv.clone(), KeychainKind::Internal)),
         Network::Regtest,
         database,
     )?;
@@ -216,18 +215,15 @@ fn main() -> Result<(), Box<dyn Error>> {
 // Helper function demonstrating privatekey extraction using bip39 mnemonic
 // The mnemonic can be shown to user to safekeeping and the same wallet
 // private descriptors can be recreated from it.
-fn generate_random_ext_privkey() -> Result<ExtendedPrivKey, Box<dyn Error>> {
+fn generate_random_ext_privkey() -> Result<impl DerivableKey<Segwitv0> + Clone, Box<dyn Error>> {
     // a Bip39 passphrase can be set optionally
     let password = Some("random password".to_string());
 
-    // Generate a random mnemonic, and use that to create an Extended PrivateKey
-    let mnemonic: GeneratedKey<_, Segwitv0> =
-        Mnemonic::generate((WordCount::Words12, Language::English))
-            .map_err(|e| e.expect("Unknown Error"))?;
-    let mnemonic = mnemonic.into_key();
-    let xkey: ExtendedKey = (mnemonic, password).into_extended_key()?;
-    let xprv = xkey
-        .into_xprv(Network::Regtest)
-        .expect("Expected Private Key");
-    Ok(xprv)
+    // Generate a random mnemonic, and use that to create a "DerivableKey"
+    let mnemonic: GeneratedKey<_, _> = Mnemonic::generate((WordCount::Words12, Language::English))
+        .map_err(|e| e.expect("Unknown Error"))?;
+
+    // `Ok(mnemonic)` would also work if there's no passphrase and it would
+    // yield the same result as this construct with `password` = `None`.
+    Ok((mnemonic, password))
 }

src/keys/bip39.rs

@@ -94,6 +94,23 @@ impl<Ctx: ScriptContext> DerivableKey<Ctx> for MnemonicWithPassphrase {
     }
 }
 
+#[cfg_attr(docsrs, doc(cfg(feature = "keys-bip39")))]
+impl<Ctx: ScriptContext> DerivableKey<Ctx> for (GeneratedKey<Mnemonic, Ctx>, Option<String>) {
+    fn into_extended_key(self) -> Result<ExtendedKey<Ctx>, KeyError> {
+        let (mnemonic, passphrase) = self;
+        (mnemonic.into_key(), passphrase).into_extended_key()
+    }
+
+    fn into_descriptor_key(
+        self,
+        source: Option<bip32::KeySource>,
+        derivation_path: bip32::DerivationPath,
+    ) -> Result<DescriptorKey<Ctx>, KeyError> {
+        let (mnemonic, passphrase) = self;
+        (mnemonic.into_key(), passphrase).into_descriptor_key(source, derivation_path)
+    }
+}
+
 #[cfg_attr(docsrs, doc(cfg(feature = "keys-bip39")))]
 impl<Ctx: ScriptContext> DerivableKey<Ctx> for Mnemonic {
     fn into_extended_key(self) -> Result<ExtendedKey<Ctx>, KeyError> {

src/keys/mod.rs

@@ -548,6 +548,16 @@ impl<K, Ctx: ScriptContext> Deref for GeneratedKey<K, Ctx> {
     }
 }
 
+impl<K: Clone, Ctx: ScriptContext> Clone for GeneratedKey<K, Ctx> {
+    fn clone(&self) -> GeneratedKey<K, Ctx> {
+        GeneratedKey {
+            key: self.key.clone(),
+            valid_networks: self.valid_networks.clone(),
+            phantom: self.phantom,
+        }
+    }
+}
+
 // Make generated "derivable" keys themselves "derivable". Also make sure they are assigned the
 // right `valid_networks`.
 impl<Ctx, K> DerivableKey<Ctx> for GeneratedKey<K, Ctx>