DefiniteDescriptorKey: provide additional methods for converting to a DescriptorPublicKey

[apoelstra]

Fixes #491

I didn't provide a mutable version of this. I'm not sure if we want to support this or not; generally public keys should be immutable after creation.

[LLFourn]

Concept ACK. I think From<DefiniteDescriptorKey> for DescriptorPublicKey should be included. The to_ method seems superfluous given you have the as_ there. Also the From implementation for references is unusual -- Maybe AsRef instead?

If we allowed as_mut this would violate the guarantees of DefiniteDescriptor since it would let you put a wildcard in there.

[apoelstra]

The to_ method seems superfluous given you have the as_ there.

Agreed, I just put it in for completeness. I'll drop it and people can call to_.clone().

Also the From implementation for references is unusual -- Maybe AsRef instead?

Is it unusual? I'm happy to do AsRef instead. TBH I have no idea what's idiomatic and what's not.

If we allowed as_mut this would violate the guarantees of DefiniteDescriptor since it would let you put a wildcard in there.

Ah, great point.

[LLFourn]

Also the From implementation for references is unusual -- Maybe AsRef instead?

Is it unusual? I'm happy to do AsRef instead. TBH I have no idea what's idiomatic and what's not.

I think so. I checked out std::convert::From and I can't see a single & to & implementation. TBH I don't really understand what's in good taste either but there are two choices that afaik are preferable to From<&..> for &..: Borrow or AsRef. I'm not exactly sure when to go for which one but the docs make it clear that AsRef is meant for conversion things where you want to take a T: AsRef<DescriptorPublicKey> for your function which I guess is the goal here. As opposed to From, AsRef will let you to pass in DescriptorPublicKey, &DefiniteDescriptorKey even &mut DescriptorPublicKey and so on. The downside is you have to also implement AsRef<DescriptorPublicKey> for DescriptorPublicKey.

[apoelstra]

Ok, after writing e16e7dd#r1025217984 I realize that AsRef is what feels more natural to a user :)

I checked out std::convert::From and I can't see a single & to & implementation

Huh, I could've sworn I saw this in stdlib some time ago. Maybe this is a pre-1.0 memory or a fever dream :).

you have to also implement AsRef for DescriptorPublicKey

No, there's a blanket AsRef<T> for &T impl that will suffice https://doc.rust-lang.org/std/convert/trait.AsRef.html

[LLFourn]

No, there's a blanket AsRef<T> for &T impl that will suffice https://doc.rust-lang.org/std/convert/trait.AsRef.html

No there isn't! From that page:

Unlike AsRef, Borrow has a blanket impl for any T, and can be used to accept either a reference or a value.

Fevered dreams indeed!

[apoelstra]

Lol, ok. I'm starting to internalize what these trains are supposed to mean. Let's use Borrow then :)

[apoelstra]

Pushed to use Borrow instead.

[LLFourn]

Lol, ok. I'm starting to internalize what these trains are supposed to mean.

😅

nit: the to_ is till there.

ACK 0b569a2

[apoelstra]

lol, removed the to_

[sanket1729]

ACK 1477dbb