BIP-373: denote different public key types/purposes consistently - #1705
Conversation
3c53392 to
953e807
Compare
I find the term "compressed" more clear than "plain", so I don't think that part of the change is an improvement. Similarly I find "compressed" vs "x-only" easier to understand than "plain 33 bytes" vs "x-only". I would just change: "Why the plain aggregate public key instead of x-only?" to say "compressed". |
| | rowspan="2"|<tt>PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS = 0x1a</tt> | ||
| | <tt><33 byte plain aggregate pubkey></tt> | ||
| | <tt><33 byte compressed pubkey>*</tt> | ||
| | <tt><33 byte plain participant pubkey>*</tt> |
There was a problem hiding this comment.
Could also say "33 byte participant pubkey (compressed)"
There was a problem hiding this comment.
I like keeping "compressed" as well, maybe consistently throughout in the manner @Sjors suggests here.
There was a problem hiding this comment.
Done as suggested. Not really fond of the rendering now:
But not sure how to improve that either.
| | rowspan="2"| 0, 2 | ||
| |- | ||
| | The MuSig2 aggregate plain public key<ref>'''Why the plain aggregate public key instead of x-only?''' | ||
| | The MuSig2 plain aggregate public key<ref>'''Why the plain aggregate public key instead of x-only?''' |
There was a problem hiding this comment.
Maybe "The MuSig2 aggregate public key (compressed)"
| derived from, following [[bip-0328.mediawiki|BIP 328]], and therefore will | ||
| need to include the evenness. Furthermore, PSBT_IN_TAP_BIP32_DERIVATION fields include fingerprints | ||
| to identify master keys, and these fingerprints require full compressed public keys. By including | ||
| to identify master keys, and these fingerprints require full plain public keys. By including |
There was a problem hiding this comment.
require the y-coordinate of the public key, so x-only serialisation can't be used.
I think more important than the concrete term is to stick to one and use it consistently, so I'm also fine with changing all occurences to "compressed" if there are no objections. My impression was that "compressed" is significantly less used now than it was in the past (when there was only 65 vs. 33 bytes), especially since x-only pubkeys could then be seen as "even more compressed" 😅 // EDIT: slightly off-topic for the BIP repo here, but if we change everything to "compressed", the RPC help for |
That's true, but there's no other word for x-inclusive key. |
jonatack
added
the
Proposed BIP modification
jonatack
added
the
Pending acceptance
Improve the clarity of the BIP w.r.t. pubkeys in the following ways:
- be specific about the purpose of pubkey types in PSBT fields
("plain pubkey" alone doesn't say a lot, especially if it's used
repeatedly within a field)
- replace all uses of "plain pubkey" by "compressed pubkey"
(the only category that should matter is whether the pubkey type
is "x-only" or "plain")
- use consistent word order, e.g. prefer
"compressed aggregate public key" over "aggregate compressed public key"
953e807 to
50e8208
Compare
|
"Plain pubkey" is the terminology used by BIP 327, hence its usage here. However, if "compressed pubkey" is clearer to readers, then I'm find with that. ACK 50e8208 |
jonatack
removed
the
Pending acceptance
|
ACK 50e8208 |
rkrux
left a comment
rkrux
left a comment
There was a problem hiding this comment.
Post Merge ACK 50e8208
Using the word "plain" for pubkey here seemed quite plain to me (no pun intended). I find the usage of "compressed pubkey" instead much clearer - plain pubkey could also mean a 32 byte pubkey to someone who didn't go through (or remember) BIP 327.
Differentiating between "aggregate" and "participant" pubkeys wherever necessary is also quite helpful for the reader.
|
Can these changes be added in BIP 174 as well? Recently, I got confused by the outdated terminology there. |
|
Could you please open a PR to that end, @rkrux? |
|
Sure, will open soon. |
|
Raised it here: #2085 |
7 participants
While reviewing bitcoin/bitcoin#31247 a while ago I noticed that the mentioned public keys in the PSBT fields of this BIP are quite confusing. This is mostly due to a non-consistent mention of types (plain vs. x-only) and a purpose that is sometimes missing. This PR aims to improve this in the following ways:
Another possibility would be to even get rid of the "plain/compressed" terminology completely. From the fact that "x-only" is not explicitly mentioned and the size of 33 bytes it should be obvious that this is a plain public key.