extended bdk-cli key derive to output xprv

[i5hi]

Output:

{
  "xprv": "tprv8ZgxMBicQKsPeCmJHoy1pwYwgTyWz2WG1W9MAuqwTFbj7AtiPVdy1C6oZJFMLMyZskVCXsf9XcG2mSVm9twaKkqhKXCh96fbuaPUxBz5Sr4/84'/1'/0'/*",
  "xpub": "[33b0970f/84'/1'/0']tpubDCyioiPrh9dHkBdomkQhvTFeFnKec2XU9a3BM4Z9Y4vVXGWjMj9je3dcV7HjqN21PMH8M98ejEEQwoof8FE4JkD1jqKJhxFwJAxYtAgeieN/*"
}

I've been using the same format for xprv as xpub [fingerprint/hardened/path]xprv/*, not sure how to get the xprv in that format - or if it is required.

[i5hi]

Sorry, this is wrong. Please ignore. Will push an update.

[i5hi]

secret_key.to_string() here gives the parent xprv provided.

I was able to calculate the child xprv using the following

 let _child = xprv.derive_priv(&secp, &deriv_path).unwrap();

However, this does not contain the path for the descriptor and requires some additional sting concatenation. Is there a better way of going about doing this?
I looked into the DescriptorKey and DescriptorSecretKey Enums, was unable to find a way.

[notmandatory]

I think what we need to do is below. It doesn't split the derivation path into hardended/unhardened parts but I think this is probably better behavior since it gives the user more control. The test_key_derive test will also need to be updated with this change.

            let deriv_path: DerivationPath = DerivationPath::from_str(path.as_str())?;

            let derived_xprv = &xprv.derive_priv(&secp, &deriv_path)?;
            let origin:KeySource = (xprv.fingerprint(&secp), deriv_path);
            
            let derived_xprv_desc_key: DescriptorKey<Segwitv0> =
                 derived_xprv.into_descriptor_key(Some(origin), DerivationPath::default())?;
            
            if let Secret(desc_seckey, _, _) = derived_xprv_desc_key {
                let desc_pubkey = desc_seckey.as_public(&secp).unwrap();
                Ok(json!({"xpub": desc_pubkey.to_string(), "xprv": desc_seckey.to_string()}))
            } else {
                Err(Error::Key(Message("Invalid key variant".to_string())))
            }

[i5hi]

yeah, that looks more clear with the child xprv being explicitly derived at the path.

[i5hi]

As you said, the derivation path isnt being separated now, so the the resulting outputs now show the entire path within the prefix [ ] as below:

{
  "xprv": "[566844c5/84'/1'/0'/0]tprv8ixoZoiRo3LDHENAysmxxFaf6nhmnNA582inQFbtWdPMivf7B7pjuYBQVuLC5bkM7tJZEDbfoivENsGZPBnQg1n52Kuc1P8X2Ei3XJuJX7c/*",
  "xpub": "[566844c5/84'/1'/0'/0]tpubDFeqiDkfwR1tAhPxsXSZMfEmfpDhwhLyhLKZgmeBvuBkZQusoWeL62oGg2oTNGcENeKdwuGepAB85eMvyLemabYe9PSqv6cr5mFXktHc3Ka/*"
}

[notmandatory]

As you said, the derivation path isnt being separated now, so the the resulting outputs now show the entire path within the prefix [ ] as below:

Ya I'm not sure if this is a bug or a feature. Is this OK for your use case?

Here's a way to fix it (borrowed some code from rust-miniscript) , may be what most users would expect:

            let deriv_path = DerivationPath::from_str(path.as_str())?;

            let deriv_path_len = (&deriv_path).as_ref().len();
            let normal_suffix_len = (&deriv_path)
                .into_iter()
                .rev()
                .take_while(|c| c.is_normal())
                .count();
            
            let deriv_path_hardened = DerivationPath::from(&deriv_path[..(deriv_path_len - normal_suffix_len)]);
            let deriv_path_normal = DerivationPath::from(&deriv_path[(deriv_path_len - normal_suffix_len)..]);
            
            let derived_xprv = &xprv.derive_priv(&secp, &deriv_path_hardened)?;
            let origin:KeySource = (xprv.fingerprint(&secp), deriv_path_hardened);

            let derived_xprv_desc_key: DescriptorKey<Segwitv0> =
                derived_xprv.into_descriptor_key(Some(origin), deriv_path_normal)?;

            if let Secret(desc_seckey, _, _) = derived_xprv_desc_key {
                let desc_pubkey = desc_seckey.as_public(&secp).unwrap();
                Ok(json!({"xpub": desc_pubkey.to_string(), "xprv": desc_seckey.to_string()}))
            } else {
                Err(Error::Key(Message("Invalid key variant".to_string())))
            }

[notmandatory]

One final thing, when you're all done be sure to run cargo fmt, it looks like that check is failing.

[i5hi]

I think it should be fine for now. I tried running the code you shared and changed the tests but they are failing. I'll spend a little more time with it and get back. Might be better to display it the most commonly expected format.

[notmandatory]

I think it is better to split the path and then the original test should pass without any changes except to also verify the xprv.

[notmandatory]

@vmenond can you also setup gpg signing for your commits? It's not a big deal but it is best practice and good thing to have for other projects you're working on. Github provides good instructions for signing commits. And there's also a blog post on how to retroactively sign git commits. I'm happy to help if you run into any problems.

[i5hi]

@notmandatory surely! I will get on this next session and push a final commit with a signature. been a bit caught up with work, hence the delayed responses.

[i5hi]

GOT IT! Small typo.

let derived_xprv = &xprv.derive_priv(&secp, &deriv_path_hardened)?;

TO

let derived_xprv = &xprv.derive_priv(&secp, &deriv_path)?;

All tests passing. Also double checked at https://bip32jp.github.io/english/

[i5hi]

Sorry about the messy commit history. Had a few issues getting a valid signature.

Commit 078b75d should be all good now.

[thunderbiscuit]

I'm trying to understand the workflow for this. Does the user provide the path as a new argument? Could you post an example of the use of the command? Cheers!

Edit: Oh wait. I might have misunderstood the PR. Is this only about the format of the returned json-ish object printed by bdk-cli? The first post jumps right into it, it feels like I might have missed part of the conversation that might have happened elsewhere and lead to this PR.

[i5hi]

the command used to originally only output the child xpub.

If you run bdk-cli key help, you will notice

derive      Derive an xpub from an xprv and a derivation path string (ie. "m/84'/1'/0'/0")

Which also needs an update

For the given input:

cargo run  key derive --path m/84h/0h/1h --xprv tprv8ZgxMBicQKsPdZn4NNm7gjMMnLxq443NoZYPZ4a46wC7ZE9Q6EMTudm6az7HYW37uWGgJ428cmgg6E16n28Scn8hfCPCr9po5j6FcNboXFY

Old output:

{  "xpub": "[94430292/84'/0'/1']tpubDCgN8DyJc9eKe7oncvGeXUmFPmUUKYykXvhNvEV8gjSQjgASPUbeX6A37NoqGYt2bDWDM1Kz3HfEdRAJpUCM9JvNmH8HxKN8eh87LN5DdER/*"

This part of the PR was discussed over Discord. Then, after making the change to output the xprv as well, this thread started with the first post - unable to get the child key pair to output in the same descriptor format.

New output:

{
  "xprv": "[94430292/84'/0'/1']tprv8fzKyow4TmxekemzjGc48578pjxYADnqxd6bdiSqGTe1uBufm5n4LbYAwGZJ68GmMKH9qTUbgCwTK8EyALCe2m4KGG1sPxQsFE5zLbACr71/*",
  "xpub": "[94430292/84'/0'/1']tpubDCgN8DyJc9eKe7oncvGeXUmFPmUUKYykXvhNvEV8gjSQjgASPUbeX6A37NoqGYt2bDWDM1Kz3HfEdRAJpUCM9JvNmH8HxKN8eh87LN5DdER/*"
}

[thunderbiscuit]

Oh nice! The new derive subcommand with the --path argument totally slipped by me. I didn't know it had been added.

Great stuff. 🔥

[notmandatory]

Hi thanks for getting everything signed! I'm traveling but will be able to re-review when I'm back in a few days.

[notmandatory]

I took a look and would like to discuss pro/con of not splitting the hardened/non-hardened path. Looks like you went with just deriving from the whole path including any non-hardened parts at the end. But I think most users would expect to just derive up to the last hardened part of the path with the unhardened part of the path added after the derived key part. This way also the test remains the same for the xpub with the addition of an assert for the xprv.

I have a code suggestion #25 (comment) for how to do it. Also I'd like to help you do an interactive rebase to cleanup your commit history. We can chat on Discord about how to do it if you like.

[i5hi]

@notmandatory Made changes based on your suggestions.

Should be good to go now.