DRAFT: Eliminate duplicate crypto types like PublicKey

[stevenroose]

Some of you with a good memory will think "here he goes again".. Well, here I go again :)

First commit eliminates bitcoin::PublicKey. I plan to replace bitcoin::PrivateKey with a wif::Key type that is not used elsewhere, just used for WIF format.

Then maybe I have some idea for signatures.

I'm currently on the move, though, so I might not have time for this immediatelly.

[apoelstra]

concept NACK. This forces the user to keep track of compressedness outside of the public key type.

If you don't want to keep track of compressedness then just use Taproot where there's only one key type.

[tcharding]

Is there some way we can make legacy keys (ones that need to know about compressedness) second class citizens. This PR is the result of some chats @stevenroose and I have been having. I was wanting to add LegacyPubkey and SegwitV0Pubkey (same for private keys) but Steven suggested removing the keys and having an extension trait for the things we add. I believe the underlying problem is that 90% of the time no-one cares about compressedness, hence the second class citizen thing.

[tcharding]

Am I right in thinking that the compressedness is only used when using a PublicKey to create a p2pkh using a uncompressed serialization to create the pubkeyhash? If so its an encoding concept, right? And the issue is we want encodings to be roundtripable. This is a similar problem to the Address's network field. We are mixing up encoding details with type abstractions then munging things in other places because of it.

[apoelstra]

Am I right in thinking that the compressedness is only used when using a PublicKey to create a p2pkh using a uncompressed serialization to create the pubkeyhash

No. It is used any time a public key appears anywhere in a pre-segwit script.

[apoelstra]

If so its an encoding concept, right

No more than any other property of a public key is "an encoding concept". If you flip the compressedness bit then valid signatures become invalid.

[tcharding]

Ok, thanks for the clarification. Hijacking Steven's thread here but what do you think of legacy versions of keys @apoelstra? useful, useful-but-waste-of-time, unuseful?

[stevenroose]

concept NACK. This forces the user to keep track of compressedness outside of the public key type.

I see the need for some way to bundle a public key together with whether this key ought to be used in Script in an compressed or uncompressed form. Though I do feel like it's fair to make this complicated for the <0.01% of cases. You can't deny that any new thing, protocol, wallet, whatever, will always use compressed keys. The only use-case for uncompressed keys is to recover coins that were stored years ago and are almost always one-offs.

My main problem is that, just because our bitcoin crate has a PublicKey type, "we" (i.e. the lib devs here) feel inclined to use it, even though it almost never makes sense. In all this refactor commit, the only 3 places where the compressed made sense were:

• address creation: accomodated by adding _uncompressed variants and arguably we can create address getters of wif::Key to prevent loss of information for users trying to recover a wif key that don't know about compressedness
• pushing keys into a script: also created _uncompressed variant; haven't thought if we could somehow also accomodate this in wif::Key
• psbt's partial_sigs, which is what I'm thinking about right now, it's the trickiest one, I have a commit that makes it into a BTreeMap<Vec<u8>, ecdsa::Signature>, but that's not ideal..

[stevenroose]

Maybe an alternative is to rename it PublicKeyWithCompressedness and almost never use it, but then if needed, take arguments that are impl Into<PublicKeyWithCompressedness> and we already have Fromsecp::PublicKey` for it. I don't really like how it again pollutes the API with almost always irrelevant information, though.

[apoelstra]

Ok, how about:

• We rename PublicKey to LegacyPublicKey
• We use LegacyPublicKey in a new method Script::push_legacy_key, in p2pkh address computation, and in LegacyPublicKey's own de/serialization methods.
• For everything else we use secp256k1::PublicKey. We should re-export this wherever so that it's just as accessibe as XOnlyPublicKey.

The result is pretty close to this PR.

[tcharding]

Sweeeet, now we are getting somewhere!

[sanket1729]

This will change a lot of things for many users. I would like to test this PR out on rust-miniscript repo to have an informed opinion on the implications of this one.

I liked the conceptual separation of bitcoin::PublicKey and EC key inside it. Most people should only use PublicKey type and we should always have secp signature APIs available via bitcoin types. Why should a rust-bitcoin user care about the EC representations of points? Having other APIs like point_add, and tweak_ on the main Key might be more confusing?

I would pretty much prefer a new type PubKey(secp256k1::PublicKey) over reusing the secp keys to present the compressedkeys. I prefer this ergonomically over importing a trait. That said, I would only know the implications once I start using this.

I hope that none of code breaks because we are silently changing the rules of fundamental data type in rust-bitcoin.

[stevenroose]

@sanket1729 compressedness is not a property of a public key though. It's only a property of how you serialize it. So a "compressed key" isn't even really a thing, other than "a public key serialized to bytes in compressed format".

@apoelstra I can live with that.

The one remaining issue then is the psbt::Input::partial_sigs field, which maps pubkeys to signatures. They are pushed onto the stack, so they might have to be LegacyPublicKey in that case. Personally, I wouldn't mind just not supporting uncompressed keys in PSBT.. PSBT is several years more recent than probably the last wallet that did uncompressed keys had any users.. (see my comment here).

That being said, let me give a shot at that change maybe later tonight.

[apoelstra]

We could maybe coerce uncompressed keys to compressed ones in PSBT for mapping purposes.

This would cause collisions/problems if somebody was using both the uncompressed and compressed version of a single key ... but we won't be the only software that such users will be unable to use. I think this would let us support very old uncompressed-only wallets without any problems for normal users.

I note that BIP 174/170 do not mention the word "compressed" once. Nor do they mention hybrid keys, which are supported by Bitcoin's consensus code but AFAIK literally no wallet has ever supported (there is no way to encode these private keys using WIF so you'd have to do something custom). So it might be alright to just not support uncompressed keys at all, with the justification that they're barely more useful/supported than hybrid keys are..