-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MuSig: Add Minimal Compatibility with BIP32 Tweaking #151
Changes from all commits
c519b46
3710736
57a1792
8088edd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -223,16 +223,77 @@ SECP256K1_API int secp256k1_musig_pubkey_agg( | |||||||||||
size_t n_pubkeys | ||||||||||||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(5); | ||||||||||||
|
||||||||||||
/** Tweak an x-only public key in a given keyagg_cache by adding | ||||||||||||
* the generator multiplied with `tweak32` to it. | ||||||||||||
/** Obtain the aggregate public key from a keyagg_cache. | ||||||||||||
* | ||||||||||||
* This is only useful if you need the non-xonly public key, in particular for | ||||||||||||
* ordinary (non-xonly) tweaking or batch-verifying multiple key aggregations | ||||||||||||
* (not implemented). | ||||||||||||
* | ||||||||||||
* Returns: 0 if the arguments are invalid, 1 otherwise | ||||||||||||
* Args: ctx: pointer to a context object | ||||||||||||
* Out: agg_pk: the MuSig-aggregated public key. | ||||||||||||
* In: keyagg_cache: pointer to a `musig_keyagg_cache` struct initialized by | ||||||||||||
* `musig_pubkey_agg` | ||||||||||||
*/ | ||||||||||||
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_get( | ||||||||||||
const secp256k1_context* ctx, | ||||||||||||
secp256k1_pubkey *agg_pk, | ||||||||||||
secp256k1_musig_keyagg_cache *keyagg_cache | ||||||||||||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); | ||||||||||||
|
||||||||||||
/** Apply ordinary "EC" tweaking to a public key in a given keyagg_cache by | ||||||||||||
* adding the generator multiplied with `tweak32` to it. This is useful for | ||||||||||||
* deriving child keys from an aggregate public key via BIP32. | ||||||||||||
* | ||||||||||||
* The tweaking method is the same as `secp256k1_ec_pubkey_tweak_add`. So after | ||||||||||||
* the following pseudocode buf and buf2 have identical contents (absent | ||||||||||||
* earlier failures). | ||||||||||||
* | ||||||||||||
* secp256k1_musig_pubkey_agg(..., keyagg_cache, pubkeys, ...) | ||||||||||||
* secp256k1_musig_pubkey_get(..., agg_pk, keyagg_cache) | ||||||||||||
* secp256k1_musig_pubkey_ec_tweak_add(..., output_pk, tweak32, keyagg_cache) | ||||||||||||
* secp256k1_ec_pubkey_serialize(..., buf, output_pk) | ||||||||||||
* secp256k1_ec_pubkey_tweak_add(..., agg_pk, tweak32) | ||||||||||||
* secp256k1_ec_pubkey_serialize(..., buf2, agg_pk) | ||||||||||||
real-or-random marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
* | ||||||||||||
* This function is required if you want to _sign_ for a tweaked aggregate key. | ||||||||||||
* On the other hand, if you are only computing a public key, but not intending | ||||||||||||
* to create a signature for it, you can just use | ||||||||||||
* `secp256k1_ec_pubkey_tweak_add`. | ||||||||||||
* | ||||||||||||
* Returns: 0 if the arguments are invalid or the resulting public key would be | ||||||||||||
* invalid (only when the tweak is the negation of the corresponding | ||||||||||||
* secret key). 1 otherwise. | ||||||||||||
* Args: ctx: pointer to a context object initialized for verification | ||||||||||||
* Out: output_pubkey: pointer to a public key to store the result. Will be set | ||||||||||||
* to an invalid value if this function returns 0. If you | ||||||||||||
* do not need it, this arg can be NULL. | ||||||||||||
Comment on lines
+268
to
+270
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
My eyes were grepping for "(can be NULL)" with the brackets and I felt some unease. I think it's good to be this consistent. edit: Ahh I notice this part and other are copied from below. Well I suggest either ignore it or change it below. Sorry, can't "suggest" the code for the other function here (not in context of changed lines). Next time I'll just make a nit comment on top of it... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we can find a (tree-wide) wording that's both easily "eye-greppable" and understandable :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think just putting the entire sentence in parentheses would do the trick. But let's not hold up this PR further. |
||||||||||||
* In/Out: keyagg_cache: pointer to a `musig_keyagg_cache` struct initialized by | ||||||||||||
* `musig_pubkey_agg` | ||||||||||||
* In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid | ||||||||||||
* according to `secp256k1_ec_seckey_verify`, this function | ||||||||||||
* returns 0. For uniformly random 32-byte arrays the | ||||||||||||
* chance of being invalid is negligible (around 1 in | ||||||||||||
* 2^128). | ||||||||||||
*/ | ||||||||||||
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_ec_tweak_add( | ||||||||||||
const secp256k1_context* ctx, | ||||||||||||
secp256k1_pubkey *output_pubkey, | ||||||||||||
secp256k1_musig_keyagg_cache *keyagg_cache, | ||||||||||||
const unsigned char *tweak32 | ||||||||||||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4); | ||||||||||||
|
||||||||||||
/** Apply x-only tweaking to a public key in a given keyagg_cache by adding the | ||||||||||||
* generator multiplied with `tweak32` to it. This is useful for creating | ||||||||||||
* Taproot outputs. | ||||||||||||
* | ||||||||||||
* The tweaking method is the same as `secp256k1_xonly_pubkey_tweak_add`. So in | ||||||||||||
* the following pseudocode xonly_pubkey_tweak_add_check (absent earlier | ||||||||||||
* failures) returns 1. | ||||||||||||
* | ||||||||||||
* secp256k1_musig_pubkey_agg(..., agg_pk, keyagg_cache, pubkeys, ...) | ||||||||||||
* secp256k1_musig_pubkey_tweak_add(..., output_pubkey, tweak32, keyagg_cache) | ||||||||||||
* secp256k1_xonly_pubkey_serialize(..., buf, output_pubkey) | ||||||||||||
* secp256k1_musig_pubkey_xonly_tweak_add(..., output_pk, tweak32, keyagg_cache) | ||||||||||||
* secp256k1_xonly_pubkey_serialize(..., buf, output_pk) | ||||||||||||
real-or-random marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
* secp256k1_xonly_pubkey_tweak_add_check(..., buf, ..., agg_pk, tweak32) | ||||||||||||
* | ||||||||||||
* This function is required if you want to _sign_ for a tweaked aggregate key. | ||||||||||||
|
@@ -255,7 +316,7 @@ SECP256K1_API int secp256k1_musig_pubkey_agg( | |||||||||||
* chance of being invalid is negligible (around 1 in | ||||||||||||
* 2^128). | ||||||||||||
*/ | ||||||||||||
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_tweak_add( | ||||||||||||
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_xonly_tweak_add( | ||||||||||||
const secp256k1_context* ctx, | ||||||||||||
secp256k1_pubkey *output_pubkey, | ||||||||||||
secp256k1_musig_keyagg_cache *keyagg_cache, | ||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(not this PR:) We should think about the terminology here. "Non-xonly" is totally fine to get this merged but I think it can be improved.