Skip to content

fix(key_map): derivation_path for Xprv with key origin info#872

Open
oleonardolima wants to merge 2 commits into
rust-bitcoin:masterfrom
oleonardolima:fix/getkey-xprv-bip32-with-orign
Open

fix(key_map): derivation_path for Xprv with key origin info#872
oleonardolima wants to merge 2 commits into
rust-bitcoin:masterfrom
oleonardolima:fix/getkey-xprv-bip32-with-orign

Conversation

@oleonardolima

@oleonardolima oleonardolima commented Oct 31, 2025

Copy link
Copy Markdown
Contributor

Description

The GetKey for DescriptorSecretKey (and thus KeyMap) returned the wrong key when an extended private key with key origin info was queried via KeyRequest::Bip32.

The key request path is master-relative, but the xkey is anchored at its origin (e.g. [d34db33f/84h/1h/0h]). The old code stripped the path returned by matches(); the fix strips the origin prefix instead (and uses the full path when there is no origin) before deriving from the xkey.

--

Also, this PR introduces the test get_key_xpriv_with_key_origin covering the scenarios: bare wildcard; single fixed step; fixed step then wildcard, and the matching/non-matching cases build requests from explicit (fingerprint, path) key sources.

Notes

If you'd like an overview on the issue, see: #863 (comment)

Changelog notice

### Fixed
- keymap: fix `GetKey` derivation path for `Xprv` with key origin info [#872](https://github.com/rust-bitcoin/rust-miniscript/pull/872)

Checklists

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@oleonardolima oleonardolima force-pushed the fix/getkey-xprv-bip32-with-orign branch 3 times, most recently from 9869841 to 64483f3 Compare October 31, 2025 15:51
Comment thread src/policy/concrete.rs Outdated
tcharding added a commit to tcharding/rust-miniscript that referenced this pull request Nov 3, 2025
We cannot link to a file by directory path in rustdocs. Just remove
the link.

This is breaking the docs build in rust-bitcoin#805 and is also mentioned in rust-bitcoin#872.
@tcharding tcharding mentioned this pull request Nov 3, 2025
apoelstra added a commit that referenced this pull request Nov 3, 2025
f853906 Remove link to doc file (Tobin C. Harding)

Pull request description:

  We cannot link to a file by directory path in rustdocs. Just remove the link.
  
  This is breaking the docs build in #805 and is also mentioned in #872.


ACKs for top commit:
  apoelstra:
    ACK f853906; successfully ran local tests


Tree-SHA512: d2e2eb182cb823297de53635ac23d55ff920b750d28efa7e18de08ddeda3a7f2bdffe8ff32dcf4c54ec32a33a176cea349291260388fd5e5cdad14c6c2e9802e
noahjoeris pushed a commit to noahjoeris/rust-miniscript that referenced this pull request May 27, 2026
We cannot link to a file by directory path in rustdocs. Just remove
the link.

This is breaking the docs build in rust-bitcoin#805 and is also mentioned in rust-bitcoin#872.
@oleonardolima oleonardolima force-pushed the fix/getkey-xprv-bip32-with-orign branch 5 times, most recently from 43d92da to 32867b3 Compare June 16, 2026 17:11
- fixes the implementation of `GetKey` for `Xprv` with
  `KeyRequest::Bip32`, when the the key_origin information is available.
- introduces a new test for a scenario with no wildcard used, a specific
  derivation index is used instead.
- introduce table-driven cases for `get_key_xpriv_with_key_origin` with named fields
- drop redundant key parsing, and build requests from explicit key sources

Assisted-by: Claude Opus 4.8
@oleonardolima oleonardolima force-pushed the fix/getkey-xprv-bip32-with-orign branch from 9cd3d75 to a2f4497 Compare June 16, 2026 20:09
@oleonardolima

Copy link
Copy Markdown
Contributor Author

@notmandatory I updated this PR and added new test scenarios, it should be ready to review.

cc'in @noahjoeris as you had interest in this and a review would be appreciated.

@oleonardolima oleonardolima changed the title fix(get_key): for Xprv with key origin info fix(key_map): derivation_path for Xprv with key origin info Jun 16, 2026
@oleonardolima oleonardolima marked this pull request as ready for review June 16, 2026 20:51
oleonardolima added a commit to oleonardolima/rust-miniscript that referenced this pull request Jun 16, 2026
- backports rust-bitcoin#872 fixes for the implementation of `GetKey` for `Xprv` with
  `KeyRequest::Bip32`, when the the key_origin information is available.
- introduces a new test for a scenario with no wildcard used, a specific
  derivation index is used instead.
oleonardolima added a commit to oleonardolima/rust-miniscript that referenced this pull request Jun 16, 2026
- backports introduced rust-bitcoin#872 table-driven test cases for
  `get_key_xpriv_with_key_origin` with named fields
- drop redundant key parsing, and build requests from explicit
  key sources

Assisted-by: Claude Opus 4.8

@notmandatory notmandatory left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK a2f4497

Changes look good, tests easy to follow.

@noahjoeris noahjoeris left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK a2f4497

Looking good, thanks.

@oleonardolima

Copy link
Copy Markdown
Contributor Author

ping @tcharding @apoelstra for awareness.

@apoelstra

Copy link
Copy Markdown
Member

Oops, missed this somehow. Will review it and the backport today.

@apoelstra apoelstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK a2f4497; successfully ran local tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants