Skip to content
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

Merged
merged 4 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 55 additions & 13 deletions examples/musig.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,51 @@ int create_keypair(const secp256k1_context* ctx, struct signer_secrets *signer_s
return 1;
}

/* Tweak the pubkey corresponding to the provided keyagg cache, update the cache
* and return the tweaked aggregate pk. */
int tweak(const secp256k1_context* ctx, secp256k1_xonly_pubkey *agg_pk, secp256k1_musig_keyagg_cache *cache) {
secp256k1_pubkey output_pk;
unsigned char ordinary_tweak[32] = "this could be a BIP32 tweak....";
unsigned char xonly_tweak[32] = "this could be a taproot tweak..";


/* Ordinary tweaking which, for example, allows deriving multiple child
* public keys from a single aggregate key using BIP32 */
if (!secp256k1_musig_pubkey_ec_tweak_add(ctx, NULL, cache, ordinary_tweak)) {
return 0;
}
/* Note that we did not provided an output_pk argument, because the
* resulting pk is also saved in the cache and so if one is just interested
* in signing the output_pk argument is unnecessary. On the other hand, if
* one is not interested in signing, the same output_pk can be obtained by
* calling `secp256k1_musig_pubkey_get` right after key aggregation to get
* the full pubkey and then call `secp256k1_ec_pubkey_tweak_add`. */

/* Xonly tweaking which, for example, allows creating taproot commitments */
if (!secp256k1_musig_pubkey_xonly_tweak_add(ctx, &output_pk, cache, xonly_tweak)) {
return 0;
}
/* Note that if we wouldn't care about signing, we can arrive at the same
* output_pk by providing the untweaked public key to
* `secp256k1_xonly_pubkey_tweak_add` (after converting it to an xonly pubkey
* if necessary with `secp256k1_xonly_pubkey_from_pubkey`). */

/* Now we convert the output_pk to an xonly pubkey to allow to later verify
* the Schnorr signature against it. For this purpose we can ignore the
* `pk_parity` output argument; we would need it if we would have to open
* the taproot commitment. */
if (!secp256k1_xonly_pubkey_from_pubkey(ctx, agg_pk, NULL, &output_pk)) {
return 0;
}
return 1;
}

/* Sign a message hash with the given key pairs and store the result in sig */
int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, struct signer *signer, const unsigned char* msg32, unsigned char *sig64) {
int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, struct signer *signer, const secp256k1_musig_keyagg_cache *cache, const unsigned char *msg32, unsigned char *sig64) {
int i;
const secp256k1_xonly_pubkey *pubkeys[N_SIGNERS];
const secp256k1_musig_pubnonce *pubnonces[N_SIGNERS];
const secp256k1_musig_partial_sig *partial_sigs[N_SIGNERS];
/* The same for all signers */
secp256k1_musig_keyagg_cache cache;
secp256k1_musig_session session;

for (i = 0; i < N_SIGNERS; i++) {
Expand All @@ -86,29 +123,25 @@ int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, st
if (!secp256k1_musig_nonce_gen(ctx, &signer_secrets[i].secnonce, &signer[i].pubnonce, session_id, seckey, msg32, NULL, NULL)) {
return 0;
}
pubkeys[i] = &signer[i].pubkey;
pubnonces[i] = &signer[i].pubnonce;
}
/* Communication round 1: A production system would exchange public nonces
* here before moving on. */
for (i = 0; i < N_SIGNERS; i++) {
secp256k1_musig_aggnonce agg_pubnonce;

/* Create aggregate pubkey, aggregate nonce and initialize signer data */
if (!secp256k1_musig_pubkey_agg(ctx, NULL, NULL, &cache, pubkeys, N_SIGNERS)) {
return 0;
}
/* Create aggregate nonce and initialize the session */
if (!secp256k1_musig_nonce_agg(ctx, &agg_pubnonce, pubnonces, N_SIGNERS)) {
return 0;
}
if (!secp256k1_musig_nonce_process(ctx, &session, &agg_pubnonce, msg32, &cache, NULL)) {
if (!secp256k1_musig_nonce_process(ctx, &session, &agg_pubnonce, msg32, cache, NULL)) {
return 0;
}
/* partial_sign will clear the secnonce by setting it to 0. That's because
* you must _never_ reuse the secnonce (or use the same session_id to
* create a secnonce). If you do, you effectively reuse the nonce and
* leak the secret key. */
if (!secp256k1_musig_partial_sign(ctx, &signer[i].partial_sig, &signer_secrets[i].secnonce, &signer_secrets[i].keypair, &cache, &session)) {
if (!secp256k1_musig_partial_sign(ctx, &signer[i].partial_sig, &signer_secrets[i].secnonce, &signer_secrets[i].keypair, cache, &session)) {
return 0;
}
partial_sigs[i] = &signer[i].partial_sig;
Expand All @@ -127,7 +160,7 @@ int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, st
* fine to first verify the aggregate sig, and only verify the individual
* sigs if it does not work.
*/
if (!secp256k1_musig_partial_sig_verify(ctx, &signer[i].partial_sig, &signer[i].pubnonce, &signer[i].pubkey, &cache, &session)) {
if (!secp256k1_musig_partial_sig_verify(ctx, &signer[i].partial_sig, &signer[i].pubnonce, &signer[i].pubkey, cache, &session)) {
return 0;
}
}
Expand All @@ -141,6 +174,7 @@ int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, st
struct signer signers[N_SIGNERS];
const secp256k1_xonly_pubkey *pubkeys_ptr[N_SIGNERS];
secp256k1_xonly_pubkey agg_pk;
secp256k1_musig_keyagg_cache cache;
unsigned char msg[32] = "this_could_be_the_hash_of_a_msg!";
unsigned char sig[64];

Expand All @@ -156,13 +190,21 @@ int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, st
}
printf("ok\n");
printf("Combining public keys...");
if (!secp256k1_musig_pubkey_agg(ctx, NULL, &agg_pk, NULL, pubkeys_ptr, N_SIGNERS)) {
/* If you just want to aggregate and not sign the cache can be NULL */
if (!secp256k1_musig_pubkey_agg(ctx, NULL, &agg_pk, &cache, pubkeys_ptr, N_SIGNERS)) {
printf("FAILED\n");
return 1;
}
printf("ok\n");
printf("Tweaking................");
/* Optionally tweak the aggregate key */
if (!tweak(ctx, &agg_pk, &cache)) {
printf("FAILED\n");
return 1;
}
printf("ok\n");
printf("Signing message.........");
if (!sign(ctx, signer_secrets, signers, msg, sig)) {
if (!sign(ctx, signer_secrets, signers, &cache, msg, sig)) {
printf("FAILED\n");
return 1;
}
Expand Down
71 changes: 66 additions & 5 deletions include/secp256k1_musig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Comment on lines +228 to +230
Copy link
Collaborator

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.

*
* 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* 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.
* Out: output_pubkey: pointer to a public key to store the result (can be NULL).
* Will be set to an invalid value if this function returns 0.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(can be NULL) is imho less informative than If you do not need it, this arg can be NULL..

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 :)

Copy link
Collaborator

Choose a reason for hiding this comment

The 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.
Expand All @@ -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,
Expand Down
26 changes: 24 additions & 2 deletions src/modules/musig/keyagg_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,21 @@ int secp256k1_musig_pubkey_agg(const secp256k1_context* ctx, secp256k1_scratch_s
return 1;
}

int secp256k1_musig_pubkey_tweak_add(const secp256k1_context* ctx, secp256k1_pubkey *output_pubkey, secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *tweak32) {
int secp256k1_musig_pubkey_get(const secp256k1_context* ctx, secp256k1_pubkey *agg_pk, secp256k1_musig_keyagg_cache *keyagg_cache) {
secp256k1_keyagg_cache_internal cache_i;
VERIFY_CHECK(ctx != NULL);
ARG_CHECK(agg_pk != NULL);
memset(agg_pk, 0, sizeof(*agg_pk));
ARG_CHECK(keyagg_cache != NULL);

if(!secp256k1_keyagg_cache_load(ctx, &cache_i, keyagg_cache)) {
return 0;
}
secp256k1_pubkey_save(agg_pk, &cache_i.pk);
return 1;
}

static int secp256k1_musig_pubkey_tweak_add_internal(const secp256k1_context* ctx, secp256k1_pubkey *output_pubkey, secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *tweak32, int xonly) {
secp256k1_keyagg_cache_internal cache_i;
int overflow = 0;
secp256k1_scalar tweak;
Expand All @@ -263,7 +277,7 @@ int secp256k1_musig_pubkey_tweak_add(const secp256k1_context* ctx, secp256k1_pub
if (overflow) {
return 0;
}
if (secp256k1_extrakeys_ge_even_y(&cache_i.pk)) {
if (xonly && secp256k1_extrakeys_ge_even_y(&cache_i.pk)) {
jonasnick marked this conversation as resolved.
Show resolved Hide resolved
cache_i.internal_key_parity ^= 1;
secp256k1_scalar_negate(&cache_i.tweak, &cache_i.tweak);
}
Expand All @@ -280,4 +294,12 @@ int secp256k1_musig_pubkey_tweak_add(const secp256k1_context* ctx, secp256k1_pub
return 1;
}

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) {
return secp256k1_musig_pubkey_tweak_add_internal(ctx, output_pubkey, keyagg_cache, tweak32, 0);
}

int secp256k1_musig_pubkey_xonly_tweak_add(const secp256k1_context* ctx, secp256k1_pubkey *output_pubkey, secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *tweak32) {
return secp256k1_musig_pubkey_tweak_add_internal(ctx, output_pubkey, keyagg_cache, tweak32, 1);
}

#endif
Loading