bitcoindevkit / bdk

A modern, lightweight, descriptor-based wallet library written in Rust!
Other
874 stars 314 forks source link

BIP44, BIP49, BIP84 templates don't set coin type #578

Closed notmandatory closed 2 years ago

notmandatory commented 2 years ago

Describe the bug

When using one of the BIP(44,49,84) templates the "coin type" part of the key derivation path is always set to 0, which represents bitcoin mainnet, no matter what the type of the key(s) used are. These templates should use 1 if the type of the key is testnet, regtest, or signet.

(Originally reported on discord by marcosalejandro)

To Reproduce

    // BIP44 `pkh(key/44'/{0,1}'/0'/{0,1}/*)`
    #[test]
    fn test_bip44_template_cointype() {
        use bitcoin::util::bip32::ChildNumber::Hardened;

        let xprvkey = bitcoin::util::bip32::ExtendedPrivKey::from_str("xprv9s21ZrQH143K2fpbqApQL69a4oKdGVnVN52R82Ft7d1pSqgKmajF62acJo3aMszZb6qQ22QsVECSFxvf9uyxFUvFYQMq3QbtwtRSMjLAhMf").unwrap();
        println!("bitcoin prvkey: {}", xprvkey.network);
        assert_eq!(Network::Bitcoin, xprvkey.network);
        let xdesc = Bip44(xprvkey, KeychainKind::Internal).build().unwrap();
        println!("xdesc = {:?}", xdesc);

        if let ExtendedDescriptor::Pkh(pkh) = xdesc.0 {
            let path : Vec<ChildNumber> = pkh.into_inner().full_derivation_path().into();
            let purpose = path.get(0).unwrap();
            println!("xdesc purpose = {:?}", purpose);
            assert!(matches!(purpose, Hardened { index: 44 }));
            let coin_type = path.get(1).unwrap();
            println!("xdesc coin_type = {:?}", coin_type);
            assert!(matches!(coin_type, Hardened { index: 0 }));
        }

        let tprvkey = bitcoin::util::bip32::ExtendedPrivKey::from_str("tprv8ZgxMBicQKsPcx5nBGsR63Pe8KnRUqmbJNENAfGftF3yuXoMMoVJJcYeUw5eVkm9WBPjWYt6HMWYJNesB5HaNVBaFc1M6dRjWSYnmewUMYy").unwrap();
        println!("testnet prvkey: {}", tprvkey.network);
        assert_eq!(Network::Testnet, tprvkey.network);
        let tdesc = Bip44(tprvkey, KeychainKind::Internal).build().unwrap();
        println!("tdesc = {:?}", tdesc);

        if let ExtendedDescriptor::Pkh(pkh) = tdesc.0 {
            let path : Vec<ChildNumber> = pkh.into_inner().full_derivation_path().into();
            let purpose = path.get(0).unwrap();
            println!("tdesc purpose = {:?}", purpose);
            assert!(matches!(purpose, Hardened { index: 44 }));
            let coin_type = path.get(1).unwrap();
            println!("tdesc coin_type = {:?}", coin_type);
            assert!(matches!(coin_type, Hardened { index: 1 }));
        }
    }

test output:

bitcoin prvkey: bitcoin
xdesc = (pkh(XPub(DescriptorXKey { origin: Some((65af6f2e, m/44'/0'/0')), xkey: ExtendedPubKey { network: Bitcoin, depth: 3, parent_fingerprint: 714b621c, child_number: Hardened { index: 0 }, public_key: PublicKey { compressed: true, key: PublicKey(c3ce4936cde277e08e4604bc3381520747dcdec0d8b3ee4b5e5428436a385a5a47456d0b8d75187bb488ce073dd4759a1738db9e356a5bdaaaaf5ae459f9ebfe) }, chain_code: f9fd1286c60681930bbc5e74c2769ac2898a3564502ebe5039905e010e2b5787 }, derivation_path: m/1, wildcard: Unhardened })), {XPub(DescriptorXKey { origin: Some((65af6f2e, m/44'/0'/0')), xkey: ExtendedPubKey { network: Bitcoin, depth: 3, parent_fingerprint: 714b621c, child_number: Hardened { index: 0 }, public_key: PublicKey { compressed: true, key: PublicKey(c3ce4936cde277e08e4604bc3381520747dcdec0d8b3ee4b5e5428436a385a5a47456d0b8d75187bb488ce073dd4759a1738db9e356a5bdaaaaf5ae459f9ebfe) }, chain_code: f9fd1286c60681930bbc5e74c2769ac2898a3564502ebe5039905e010e2b5787 }, derivation_path: m/1, wildcard: Unhardened }): XPrv(DescriptorXKey { origin: None, xkey: ExtendedPrivKey { network: Bitcoin, depth: 0, parent_fingerprint: 00000000, child_number: Normal { index: 0 }, private_key: [private key data], chain_code: 3d6a59ac46bf8fd55349efa02581a8f1439dea8a62fbfe4de3bdf7c4ba629578 }, derivation_path: m/44'/0'/0'/1, wildcard: Unhardened })}, {Bitcoin})
xdesc purpose = Hardened { index: 44 }
xdesc coin_type = Hardened { index: 0 }
testnet prvkey: testnet
tdesc = (pkh(XPub(DescriptorXKey { origin: Some((34b00776, m/44'/0'/0')), xkey: ExtendedPubKey { network: Testnet, depth: 3, parent_fingerprint: 1c00d808, child_number: Hardened { index: 0 }, public_key: PublicKey { compressed: true, key: PublicKey(f5f3776c7305f9adac93dc3b841d3018c54dd34c2e12cf45f6192daa679a99971c73da5604f0cc2a108a542095eef7e60587ca62d05d852f771bd5c2f8ad22db) }, chain_code: d7543f5ecbe9620f3249206b2fb085f4a905a8eb20f0d12c85407b5cd09ef515 }, derivation_path: m/1, wildcard: Unhardened })), {XPub(DescriptorXKey { origin: Some((34b00776, m/44'/0'/0')), xkey: ExtendedPubKey { network: Testnet, depth: 3, parent_fingerprint: 1c00d808, child_number: Hardened { index: 0 }, public_key: PublicKey { compressed: true, key: PublicKey(f5f3776c7305f9adac93dc3b841d3018c54dd34c2e12cf45f6192daa679a99971c73da5604f0cc2a108a542095eef7e60587ca62d05d852f771bd5c2f8ad22db) }, chain_code: d7543f5ecbe9620f3249206b2fb085f4a905a8eb20f0d12c85407b5cd09ef515 }, derivation_path: m/1, wildcard: Unhardened }): XPrv(DescriptorXKey { origin: None, xkey: ExtendedPrivKey { network: Testnet, depth: 0, parent_fingerprint: 00000000, child_number: Normal { index: 0 }, private_key: [private key data], chain_code: 07c7c6226c3cf2cd58bb416e0e124fa396a1559c95a3362c603c728c2c9de4f6 }, derivation_path: m/44'/0'/0'/1, wildcard: Unhardened })}, {Regtest, Signet, Testnet})
tdesc purpose = Hardened { index: 44 }
tdesc coin_type = Hardened { index: 0 }

assertion failed: matches!(coin_type, Hardened { index : 1 })

Expected behavior

According to BIP44 Coin Type for Testnet the ChildNumber at index 1 should be Hardened { index: 1 }.

Build environment

Additional context

The Bip44, Bip49, and Bip84 macros all use the descriptor::template::expand_make_bipxx macro and this is where the coin type part of the derivation path is set.

jbesraa commented 2 years ago

happy to handle this.

I see that the coin type is set here:

  pub(super) fn make_bipxx_private<K: DerivableKey<$ctx>>(
                bip: u32,
                key: K,
                keychain: KeychainKind,
            ) -> Result<impl IntoDescriptorKey<$ctx>, DescriptorError> {
                let mut derivation_path = Vec::with_capacity(4);
                derivation_path.push(bip32::ChildNumber::from_hardened_idx(bip)?);
      ->        derivation_path.push(bip32::ChildNumber::from_hardened_idx(0)?);
                derivation_path.push(bip32::ChildNumber::from_hardened_idx(0)?);

                match keychain {
                    KeychainKind::External => {
                        derivation_path.push(bip32::ChildNumber::from_normal_idx(0)?)
                    }
                    KeychainKind::Internal => {
                        derivation_path.push(bip32::ChildNumber::from_normal_idx(1)?)
                    }
                };

                let derivation_path: bip32::DerivationPath = derivation_path.into();

                Ok((key, derivation_path))
            }

I wonder how would this macro know if this is testnet or mainnet ?

afilini commented 2 years ago

I think the best way to fix this would be to add a network argument to DescriptorTemplate::build, then you can adapt the macro and everything accordingly.

It's an API break but I think it's reasonable

jbesraa commented 2 years ago

initial pr https://github.com/bitcoindevkit/bdk/pull/585/files