Skip to content

388: fix incorrect example#1874

Merged
jonatack merged 1 commit into
bitcoin:masterfrom
bigspider:bip388-musig-fix
Jun 25, 2025
Merged

388: fix incorrect example#1874
jonatack merged 1 commit into
bitcoin:masterfrom
bigspider:bip388-musig-fix

Conversation

@bigspider

Copy link
Copy Markdown
Contributor

As per the test vectors, musig key expressions require the aggregation step to precede the change/address_index derivations in wallet policies.

As per the test vectors, musig key expressions require the aggregation step
to precede the change/address_index derivations.
@jonatack jonatack added Proposed BIP modification PR by non-owner to update BIP content Bug fix and removed Proposed BIP modification PR by non-owner to update BIP content labels Jun 21, 2025
Comment thread bip-0388.mediawiki
* <tt>wsh(multi(2,@0/**,@1/**))</tt> - SegWit 2-of-2 multisig, keys in order.
* <tt>sh(sortedmulti(2,@0/**,@1/**,@2/**))</tt> - Legacy 2-of-3 multisig, sorted keys.
* <tt>tr(musig(@0/**,@1/**))</tt> - MuSig2 2-of-2 in the taproot keypath
* <tt>tr(musig(@0,@1)/**)</tt> - MuSig2 2-of-2 in the taproot keypath

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does remove the contradiction between the example here being the same as the invalid policy example in line 313 below.

Is there further explanation somewhere about the statement in line 313 "Derivation before aggregation is not allowed in wallet policies (despite being allowed in BIP-390)"?

@bigspider bigspider Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I commented here on why I believed that it was undesirable even for descriptors, and in more detail here commenting on the severe performance difference between the two - and also the additional implementation complexity it causes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Fine for a different pull, but if there is no further explanation on this in the BIP, it may be good to add some (along with those links).

@jonatack jonatack merged commit f9017e5 into bitcoin:master Jun 25, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants