Skip to content

388: fix incorrect example#1874

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

388: fix incorrect example#1874
jonatack merged 1 commit intobitcoin: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.
* <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)"?

Copy link
Copy Markdown
Contributor Author

@bigspider bigspider Jun 23, 2025

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