BIP-373: denote different public key types/purposes consistently

theStack commented at 0:43 am on November 29, 2024: contributor

While reviewing https://github.com/bitcoin/bitcoin/pull/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:

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; explicitly call it “aggregate pubkey” or “participant pubkey” if applicable)
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”

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.

theStack force-pushed on Nov 29, 2024

Sjors commented at 9:08 am on November 29, 2024: member

replace all uses of “compressed pubkey” by “plain pubkey” (the only category that should matter is whether the pubkey type is “x-only” or “plain”; in the latter case, it’s obvious from the size of 33 bytes that the key is compressed)

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”.

in bip-0373.mediawiki:49 in 953e807ca2 outdated

45@@ -46,53 +46,53 @@ The new per-input types are defined as follows:
46 | rowspan="2"|MuSig2 Participant Public Keys
47 | rowspan="2"|<tt>PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS = 0x1a</tt>
48 | <tt><33 byte plain aggregate pubkey></tt>
49-| <tt><33 byte compressed pubkey>*</tt>
50+| <tt><33 byte plain participant pubkey>*</tt>

Sjors commented at 9:10 am on November 29, 2024:

Could also say “33 byte participant pubkey (compressed)”

jonatack commented at 1:59 am on December 5, 2024:

I like keeping “compressed” as well, maybe consistently throughout in the manner @Sjors suggests here.

theStack commented at 11:52 pm on December 5, 2024:

Done as suggested. Not really fond of the rendering now:

But not sure how to improve that either.

in bip-0373.mediawiki:54 in 953e807ca2 outdated

51 | rowspan="2"|
52 | rowspan="2"|
53 | rowspan="2"| 0, 2
54 |-
55-| The MuSig2 aggregate plain public key<ref>'''Why the plain aggregate public key instead of x-only?'''
56+| The MuSig2 plain aggregate public key<ref>'''Why the plain aggregate public key instead of x-only?'''

Sjors commented at 9:11 am on November 29, 2024:

Maybe “The MuSig2 aggregate public key (compressed)”

theStack commented at 11:53 pm on December 5, 2024:

Done.

in bip-0373.mediawiki:58 in 953e807ca2 outdated

56+| The MuSig2 plain aggregate public key<ref>'''Why the plain aggregate public key instead of x-only?'''
57 BIP 32 requires public keys to include their evenness byte. Aggregate public keys are expected to be
58 derived from, following [[bip-0328.mediawiki|BIP 328]], and therefore will
59 need to include the evenness. Furthermore, PSBT_IN_TAP_BIP32_DERIVATION fields include fingerprints
60-to identify master keys, and these fingerprints require full compressed public keys. By including
61+to identify master keys, and these fingerprints require full plain public keys. By including

Sjors commented at 9:13 am on November 29, 2024:

require the y-coordinate of the public key, so x-only serialisation can’t be used.

theStack commented at 11:53 pm on December 5, 2024:

Included.

theStack commented at 11:50 am on November 29, 2024: contributor

replace all uses of “compressed pubkey” by “plain pubkey” (the only category that should matter is whether the pubkey type is “x-only” or “plain”; in the latter case, it’s obvious from the size of 33 bytes that the key is compressed)

I find the term “compressed” more clear than “plain”, so I don’t think that part of the change is an improvement.

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 decodepsbt (https://github.com/bitcoin/bitcoin/pull/31247/commits/abb8228944a213443c7d64e47c60ac96a21cd422) should probably updated to use that as well for consistency

Sjors commented at 12:45 pm on November 29, 2024: member

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” 😅

That’s true, but there’s no other word for x-inclusive key.

jonatack added the label Proposed BIP modification on Dec 4, 2024

jonatack assigned jonatack on Dec 5, 2024

jonatack unassigned jonatack on Dec 5, 2024

jonatack requested review from achow101 on Dec 5, 2024

jonatack added the label Pending acceptance on Dec 5, 2024

BIP-373: denote different public key types consistently

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"

50e820836d

theStack force-pushed on Dec 5, 2024

achow101 commented at 4:44 pm on December 6, 2024: member

“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 50e820836dde563291458f4fd09ee1381ae76b76

in bip-0373.mediawiki:58 in 50e820836d

57+| The MuSig2 aggregate public key (compressed) <ref>'''Why the compressed aggregate public key instead of x-only?'''
58 BIP 32 requires public keys to include their evenness byte. Aggregate public keys are expected to be
59 derived from, following [[bip-0328.mediawiki|BIP 328]], and therefore will
60 need to include the evenness. Furthermore, PSBT_IN_TAP_BIP32_DERIVATION fields include fingerprints
61-to identify master keys, and these fingerprints require full compressed public keys. By including
62+to identify master keys, and these fingerprints require y-coordinate of the public key,

jonatack commented at 4:14 pm on December 7, 2024:

nit

0to identify master keys, and these fingerprints require the y-coordinate of the public key,

kdmukai commented at 3:14 am on December 11, 2024:

The previous sentence is confusing as well:

“Aggregate public keys are expected to be derived from, following BIP 328, and therefore […]”

To a layman (me!) it reads like there’s a word missing (“derived from… what?”). But I see now that it’s referring to them as the starting point (“from” the aggregate public key to a BIP32 public key). Perhaps clearer to say:

“BIP 32 public keys are expected to be derived from aggregate public keys (per BIP 328) and therefore…”

jonatack commented at 9:54 pm on December 13, 2024:

The previous sentence is confusing as well: @kdmukai WDYT about

0-BIP 32 requires public keys to include their evenness byte. Aggregate public keys are expected to be
1-derived from, following [[bip-0328.mediawiki|BIP 328]], and therefore will
2-need to include the evenness. Furthermore, PSBT_IN_TAP_BIP32_DERIVATION fields include fingerprints
3+BIP 32 requires public keys to include their evenness byte. Per [[bip-0328.mediawiki|BIP 328]], BIP 32
4+public keys can be derived from a BIP 327 MuSig2 aggregate public key; therefore, the latter needs
5+to include the evenness byte as well. Furthermore, PSBT_IN_TAP_BIP32_DERIVATION fields include fingerprints

kdmukai commented at 2:28 pm on December 14, 2024:

@jonatack YES! Much clearer. Minor edit suggestion: swap and slightly rework the first two sentences:

“BIP 32 public keys can be derived from a BIP 327 MuSig2 aggregate public key (see: BIP 328). But since BIP 32 requires public keys to include their evenness byte, BIP 327 MuSig2 aggregate public keys should (“must”?) include their evenness byte as well.”

(minor minor note: I tend to avoid “latter” / “former” constructions as for some reason my brain just finds them extra taxing to understand. The repetition I suggest above is a little annoying but prioritizes absolute clarity)

jonatack commented at 6:20 pm on December 14, 2024:

@kdmukai LGTM (with “must”, I believe). Can you open a pull?

jonatack commented at 9:08 pm on December 17, 2024:

Resolving this thread; done in #1717.

jonatack commented at 4:15 pm on December 7, 2024: member

LGTM. @Sjors?

jonatack removed the label Pending acceptance on Dec 7, 2024

jonatack requested review from Sjors on Dec 7, 2024

Sjors commented at 3:02 am on December 9, 2024: member

ACK https://github.com/bitcoin/bips/commit/50e820836dde563291458f4fd09ee1381ae76b76

jonatack merged this on Dec 9, 2024

jonatack closed this on Dec 9, 2024

theStack deleted the branch on Dec 9, 2024

BIP-373: denote different public key types/purposes consistently #1705