Does `rust-miniscript` need a `KeyMap`?

[nymius]

I'm moving whatever implements GetKey to rust-psbt, as it is only used in the context of PSBT signing.

One question arose while doing this was: Should I implement GetKey for KeyMap or should I implement it for the more general BTreeMap<DescriptorPublicKey, DescriptorSecretKey> type?

The problem is that parse_descriptor, extensively used in BDK, returns a KeyMap, not a BTreeMap, and it's not possible to access the inner type because it's private, so the only option left is to implement GetKey for KeyMap in rust-psbt.

But creating this implementation in rust-psbt for a type in an upstream dependency that was created just to fill that implementation is odd.

In addition, downstream projects wrap this type again and re-implement GetKey for those types, without composing the implementations, which I also find odd. I don't expect downstream projects to require many custom GetKey implementations.

If there is no strong reason against this:

a. Can we remove KeyMap and return BTreeMap directly? or
b. Can we expose the inner BTreeMap and avoid implementing GetKey for KeyMap?

Other questions:

• Is KeyMap used for something else beside containerizing keys for Psbt.sign?
• Is KeyMap used to simplify types?
• Is KeyMap used because of a type abstraction? Could a conversion to BTreeMap be possible in this case?

[apoelstra]

I don't think we ever thought of KeyMap as being PSBT-specific or even targeted at PSBT. Its purpose is to enable parsing descriptors that contain secret keys without making it easy to accidentally expose a secret-key-laden "descriptor".

The GetKey trait, of course, comes from the requirements of PSBT, and we do implement GetKey (from rust-bitcoin) since #851 (June 2025). The code you link is from February 2025, before we added that impl (which didn't exist because, again, we were not thinking about KeyMap's interaction with PSBT at all).

If there is no strong reason against this:

a. Can we remove KeyMap and return BTreeMap directly? or b. Can we expose the inner BTreeMap and avoid implementing GetKey for KeyMap?

No, because this would prevent us from replacing it with a HashMap, say, which we may want to do for efficiency reasons when std is enabled.

Although I see that we already implement an into_iter which directly returns btree_map::IntoIter which is an abstraction leak that we should fix.

We can certainly implement better APIs for doing lookups and iteration, if those are missing.

• Is KeyMap used for something else beside containerizing keys for Psbt.sign?
• Is KeyMap used to simplify types?
• Is KeyMap used because of a type abstraction? Could a conversion to BTreeMap be possible in this case?

I mean, you can call into_iter then call BTreeMap::collect on that.

[nymius]

I don't think we ever thought of KeyMap as being PSBT-specific or even targeted at PSBT. Its purpose is to enable parsing descriptors that contain secret keys without making it easy to accidentally expose a secret-key-laden "descriptor".

Good, it was a personal bias then.

No, because this would prevent us from replacing it with a HashMap, say, which we may want to do for efficiency reasons when std is enabled.

No problem with this, but I've seen other crates aliasing HashMap behind BTreeMap in the prelude, why that cannot be applied here?

I mean, you can call into_iter then call BTreeMap::collect on that.

I started this journey while trying to motivate me to not do something like the following:

impl GetKey for DescriptorSecretKey { ...
impl GetKey for $set<DescriptorSecretKey> { ...
impl GetKey for $map<DescriptorPublicKey, DescriptorSecretKey> { ...
impl_get_key_for_descriptor_map!(BTreeMap);
#[cfg(feature = "std")]
impl_get_key_for_descriptor_map!(HashMap);

So reading your answer the response should be: we don't want to make easy to expose secret-key-laden descriptors., and I should not implement it like that.

Instead, this approach https://git.rust-bitcoin.org/rust-bitcoin/rust-psbt/pulls/188 is ok, and there is no strong motivation to keep https://github.com/bitcoindevkit/bdk-tx/blob/master/src/signer.rs#L16-L67 in the future.

[apoelstra]

No problem with this, but I've seen other crates aliasing HashMap behind BTreeMap

The trait bounds for keys are different between HashMap and BTreeMap, and HashMap has a ton of APIs (e.g. with_capacity) that BTreeMap does not.

Crates can do this within their own boundary but we can't have anything in our public API that conflates the two.

So reading your answer the response should be: we don't want to make easy to expose secret-key-laden descriptors., and I should not implement it like that.

I should weaken my claim a bit. I don't want it to be easy for users to have a descriptor with secret keys in it, e.g Descriptor<SecretKey> should be missing a bunch of functionality. This is because we treat descriptors as public types and all our APIs assume they're public types and make no effort to contain data being copied or serialized.

But if your API only operates on secret keys directly, or on maps between public and secret keys, that's great.