bips icon indicating copy to clipboard operation
bips copied to clipboard

BIPs for MuSig2 derivation, descriptors, and PSBT fields

Open achow101 opened this issue 1 year ago • 10 comments

This PR contains 3 proposed BIPs, the main contents of which were sent to the bitcoin-dev mailing list in October. There have been a few changes, notably the addition of a new BIP for the derivation methodology which was split from the descriptors BIP.

I've also changed the PSBT BIP to use 33 byte plain aggregate pubkeys rather than xonly.

achow101 avatar Jan 15 '24 20:01 achow101

I've also changed the PSBT BIP to use 33 byte plain aggregate pubkeys rather than xonly.

It would be helpful to document the reasoning in the commit msg or here in the PR to have a record of design decisions.

benma avatar Jan 17 '24 09:01 benma

I've also changed the PSBT BIP to use 33 byte plain aggregate pubkeys rather than xonly.

It would be helpful to document the reasoning in the commit msg or here in the PR to have a record of design decisions.

I've added a rationale section which addresses this.

achow101 avatar Jan 17 '24 18:01 achow101

I've made a minor change with the descriptors regarding sorting. Keys in a musig() can be specified in any order, but will be sorted prior to aggregation, similar to sortedmulti(). However for PSBTs, keys must be supplied in the expected order for aggregation (this is unchanged), which means sorting them if they weren't already.

achow101 avatar Apr 16 '24 13:04 achow101

I've made a minor change with the descriptors regarding sorting. Keys in a musig() can be specified in any order, but will be sorted prior to aggregation, similar to sortedmulti(). However for PSBTs, keys must be supplied in the expected order for aggregation (this is unchanged), which means sorting them if they weren't already.

What is the rationale for this change? It seems to me that this complicates parsing/processing the descriptor unnecessarily. The only practical advantage I know of sortedmulti versus multi is that with multi an external observer can possibly fingerprint who signed different UTXOs even without knowing the descriptor; but that's not the case anyway for musig.

bigspider avatar Apr 17 '24 09:04 bigspider

What is the rationale for this change?

I had a discussion involving @jonasnick and @sipa about this. Since BIP 327 suggests to do sorting, we decided it made sense to do in descriptors as well.

achow101 avatar Apr 17 '24 14:04 achow101

I had a discussion involving @jonasnick and @sipa about this. Since BIP 327 suggests to do sorting, we decided it made sense to do in descriptors as well.

BIP 327 discusses the possibility of sorting and describes a canonical sorting, but I didn't read it as 'suggesting' one way or another.

In general, I'd prefer maximum simplicity and less chances of malleability in descriptors, and this imho goes against both (albeit slightly, not a huge deal).

Note that this removes functionality, as one of $n! - 1$ of the $n!$ possible combinations become inexpressible with descriptors.

bigspider avatar Apr 18 '24 08:04 bigspider

The functionality that I would suggest to consider for removal is derivation before aggregation: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-November/022132.html

Unless there are use cases for it that I'm not aware of?

bigspider avatar Apr 18 '24 08:04 bigspider

The formatting seems to be in order on these documents. What is the status of the discussion on these proposals?

murchandamus avatar May 01 '24 19:05 murchandamus

In #1434 it was mentioned that this proposal may conflict with the way PSBT is used by an existing implementation. Has there been any communication to reconcile that?

murchandamus avatar May 02 '24 19:05 murchandamus

In #1434 it was mentioned that this proposal may conflict with the way PSBT is used by an existing implementation. Has there been any communication to reconcile that?

I've changed to skip the conflicting 0x19 fiend number, and have also emailed them about using proprietary fields in the future.

achow101 avatar May 16 '24 18:05 achow101

It would be great if at least a first test vectors could be added.

Given that there are still a few active questions and some things may still end up being changed, I prefer to wait a bit longer before committing to the current specs with test vectors.

There are a few for the descriptors BIP.

I would recommend to split this PR to make each BIP a separate pull request.

This was considered originally, however there is a dependency on the derivation BIP from both the descriptor and psbt BIPs, so I'm not sure it makes sense to have them be separate.

achow101 avatar May 22 '24 18:05 achow101

BIP-0327 allows participant pubkeys to be duplicates.

Since the psbt fields are using the pubkeys (and not the signer's position in the MuSig) in order to identify the nonce/partial_sig contributions, and given that duplicate pubkeys are AFAIK of no practical value, musig() key expressions should perhaps explicitly forbid repeated pubkeys.

bigspider avatar May 27 '24 13:05 bigspider

Updated with assigned numbers

achow101 avatar Jun 11 '24 19:06 achow101

and given that duplicate pubkeys are AFAIK of no practical value, musig() key expressions should perhaps explicitly forbid repeated pubkeys.

Added a sentence for that.

It’s not clear to me whether the presence of test vectors would be necessary for the specification to be considered complete,

That's not been an issue before. Previous draft proposals have been merged without test vectors completed yet.

I was wondering whether you would like us to wait for more review or the addition of test vectors before considering merging, or whether you consider this PR ready to be merged—regarding the formal criteria for BIPs, these seem ready to go.

I consider these ready for merging.

achow101 avatar Jun 19 '24 00:06 achow101