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

Signature Subtraction function #67

Merged
merged 5 commits into from
May 16, 2023
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
22 changes: 22 additions & 0 deletions include/secp256k1_aggsig.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,28 @@ SECP256K1_API int secp256k1_aggsig_partial_sign(
size_t index
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5) SECP256K1_WARN_UNUSED_RESULT;

/** Subtract a partial signature from a completed signature, resulting in another partial signature
*
* Returns: 0 on failure due to an argument or unexpected calculation error
-1 if the provided signatures result in no possible nonce with
a y value that is a quadratic residue
1 if there is only one possible result
2 if there is a second possiblity for the chosen y value of the original public nonce
* Args: ctx: an existing context object, initialized for verifying (cannot be NULL)
* Out: result: the (partial) signature resulting from subtracting partial from sig64
* result_alt: alternative possible signature resulting from subtracting partial from sig64
* In: sig64: a completed signature
* partial64: the signature (partial) to subtract from sig64
*/

SECP256K1_API int secp256k1_aggsig_subtract_partial_signature(
const secp256k1_context* ctx,
unsigned char *result,
unsigned char *result_alt,
const unsigned char *sig64,
const unsigned char *partial64
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5) SECP256K1_WARN_UNUSED_RESULT;


/** Aggregate multiple signature parts into a single aggregated signature
*
Expand Down
124 changes: 124 additions & 0 deletions src/modules/aggsig/main_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,130 @@ int secp256k1_aggsig_partial_sign(const secp256k1_context* ctx, secp256k1_aggsig
return 1;
}

int secp256k1_aggsig_subtract_partial_signature(
const secp256k1_context* ctx,
unsigned char* result,
unsigned char* result_alt,
const unsigned char *sig64,
const unsigned char *partial64
) {
secp256k1_scalar tmp, tmp2;
secp256k1_fe noncesum_fe;
secp256k1_ge noncesum_ge;
secp256k1_gej noncesum_gej;
secp256k1_ge noncesum_ge_neg;
secp256k1_gej noncesum_gej_neg;
secp256k1_fe noncepartial_fe;
secp256k1_ge noncepartial_ge;
secp256k1_ge noncepartial_ge_neg;
secp256k1_gej nonceresult_gej;
secp256k1_gej nonceresult_gej_neg;
secp256k1_ge final;
int overflow;
int neg_version_has_quad = 0;
int pos_version_has_quad = 0;

VERIFY_CHECK(ctx != NULL);
ARG_CHECK(result != NULL);
ARG_CHECK(result_alt != NULL);
ARG_CHECK(sig64 != NULL);
ARG_CHECK(partial64 != NULL);
(void) ctx;

/* Scalar portion, straightforward scalar subtraction */
secp256k1_scalar_set_b32(&tmp, sig64 + 32, &overflow);
if (overflow) {
secp256k1_scalar_clear(&tmp);
return 0;
}
secp256k1_scalar_set_b32(&tmp2, partial64 + 32, &overflow);
if (overflow) {
secp256k1_scalar_clear(&tmp);
secp256k1_scalar_clear(&tmp2);
return 0;
}
secp256k1_scalar_negate(&tmp2, &tmp2);
secp256k1_scalar_add(&tmp, &tmp, &tmp2);

secp256k1_scalar_get_b32(result + 32, &tmp);
secp256k1_scalar_get_b32(result_alt + 32, &tmp);
secp256k1_scalar_clear(&tmp);
secp256k1_scalar_clear(&tmp2);

/* nonce portion
* Note that we are unable to determine with 100% certainty
* what nonce was originally chosen due to only the x coordinate
* being stored. We can sometimes determine which was correct, but
* may have to return a second possibility.
*/

/* Parse nonce sum total and negated version (R, -R) */
if (!secp256k1_fe_set_b32(&noncesum_fe, sig64)) {
return 0;
}

/* initialize nonce sum with y value that is a quadratic residue */
secp256k1_ge_set_xquad(&noncesum_ge, &noncesum_fe);
Copy link

@tromp tromp May 9, 2023

Choose a reason for hiding this comment

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

I don't know what all these 4 calls do. Could you add comments for each? I wish I could just click on the name and be taken to their definition, but sadly I don't have such an editor.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have an editor set up for ansi-C code traversal either, but core secp256k1 functions are all documented in src/field.h for field elements functions and src/group.h for group elements.

static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a);

static int secp256k1_ge_set_xquad(secp256k1_ge *r, const secp256k1_fe *x);

Copy link

Choose a reason for hiding this comment

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

I mean you should add comments in the style of "/* Parse nonce sum total and negated version (R, -R) */" for every reader's benefit, so it's clear why you need to call that specific function...

secp256k1_gej_set_ge(&noncesum_gej, &noncesum_ge);

/* also initialize negated version -R */
secp256k1_ge_neg(&noncesum_ge_neg, &noncesum_ge);
secp256k1_gej_set_ge(&noncesum_gej_neg, &noncesum_ge_neg);

/* Parse provided partial-sig nonce and negate */
if (!secp256k1_fe_set_b32(&noncepartial_fe, partial64)) {
return 0;
}

/* Initialize negated version of partial sum, which we're going
to subtract from the nonce total */
secp256k1_ge_set_xquad(&noncepartial_ge, &noncepartial_fe);
Copy link

Choose a reason for hiding this comment

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

Similarly for these 2.

Copy link
Member Author

Choose a reason for hiding this comment

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

should be in the same file as above

secp256k1_ge_neg(&noncepartial_ge_neg, &noncepartial_ge);

/* Try positive (Rr = R-Rs) */
secp256k1_gej_add_ge(&nonceresult_gej, &noncesum_gej, &noncepartial_ge_neg);

/* Now try neg (Rr = -R-Rs) */
secp256k1_gej_add_ge(&nonceresult_gej_neg, &noncesum_gej_neg, &noncepartial_ge_neg);

pos_version_has_quad = secp256k1_gej_has_quad_y_var(&nonceresult_gej);
neg_version_has_quad = secp256k1_gej_has_quad_y_var(&nonceresult_gej_neg);

/* If ONLY the positive 'version' of Rr (=R-Rs) or only the
negative version of Rr (=-R-Rs) results in a QR, then we know
for certain what the original x value was */
if (pos_version_has_quad && !neg_version_has_quad) {
secp256k1_ge_set_gej(&final, &nonceresult_gej);
secp256k1_fe_normalize_var(&final.x);
secp256k1_fe_get_b32(result, &final.x);
return 1;
} else if (!pos_version_has_quad && neg_version_has_quad) {
secp256k1_ge_set_gej(&final, &nonceresult_gej_neg);
secp256k1_fe_normalize_var(&final.x);
secp256k1_fe_get_b32(result, &final.x);
return 1;
} else if (pos_version_has_quad && neg_version_has_quad) {
/* if both versions result in a QR, it could have been either,
so we need to return both possibilities and indicate the user
needs to potentially check a second value */
secp256k1_ge_set_gej(&final, &nonceresult_gej);
secp256k1_fe_normalize_var(&final.x);
secp256k1_fe_get_b32(result, &final.x);

secp256k1_ge_set_gej(&final, &nonceresult_gej_neg);
secp256k1_fe_normalize_var(&final.x);
secp256k1_fe_get_b32(result_alt, &final.x);
return 2;
} else {
/* if neither result in a QR, then the signature is invalid according
to our construction. Note this should never happen with signatures constructed
via this particular API, and there's a chance that signatures using a different
convention for selecting y coordinates could still return a valid but 'incorrect'
value. */
return -1;
}
}

int secp256k1_aggsig_combine_signatures(const secp256k1_context* ctx, secp256k1_aggsig_context* aggctx, unsigned char *sig64, const secp256k1_aggsig_partial_signature *partial, size_t n_sigs) {
size_t i;
secp256k1_scalar s;
Expand Down
60 changes: 60 additions & 0 deletions src/modules/aggsig/tests_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ void test_aggsig_api(void) {
unsigned char seckeys[5][32];
secp256k1_pubkey pubkeys[5];
secp256k1_aggsig_partial_signature partials[5];
unsigned char sub_result[64];
unsigned char sub_result_alt[64];
int sub_return_val;
int sub_check;
secp256k1_aggsig_context *aggctx;
unsigned char seed[32] = { 1, 2, 3, 4, 0 };
unsigned char sig[64];
Expand All @@ -39,6 +43,20 @@ void test_aggsig_api(void) {
size_t i;
size_t j;

const unsigned char no_qr_const_1[] = {
0xCA,0x69,0xE3,0x16,0xC1,0xF5,0x4E,0xFE,
0x6C,0x23,0xFF,0x8F,0x1D,0x29,0xB9,0xB3,
0xE2,0x22,0xA6,0x79,0x9D,0x3E,0xB1,0x1C,
0xF5,0xC,0x8A,0x5,0x26,0x47,0xE8,0xDF
};

const unsigned char no_qr_const_2[] = {
0xA1,0xB9,0xFF,0xD9,0x7E,0xC9,0x11,0xD,
0x53,0xA2,0xCC,0x88,0x63,0x9A,0x31,0x9C,
0x84,0x36,0xE5,0x0,0xB4,0x1C,0x18,0x2A,
0xC4,0x9F,0x2,0x68,0x64,0x49,0x3F,0xC7,
};

secp256k1_context_set_error_callback(none, counting_illegal_callback_fn, &ecount);
secp256k1_context_set_error_callback(sign, counting_illegal_callback_fn, &ecount);
secp256k1_context_set_error_callback(vrfy, counting_illegal_callback_fn, &ecount);
Expand Down Expand Up @@ -134,6 +152,7 @@ void test_aggsig_api(void) {
CHECK(!secp256k1_aggsig_verify(vrfy, scratch, sig, msg, pubkeys, 0));
CHECK(secp256k1_aggsig_verify(vrfy, scratch, sig, msg, pubkeys, 5));
CHECK(ecount == 15);

CHECK(!secp256k1_aggsig_verify(none, scratch, sig, msg, pubkeys, 5));
CHECK(ecount == 16);

Expand Down Expand Up @@ -264,7 +283,48 @@ void test_aggsig_api(void) {
msg[2]=3;
CHECK(!secp256k1_aggsig_verify_single(vrfy, combined_sig, msg, NULL, &combiner_sum_2, NULL, NULL, 0));
CHECK(!secp256k1_aggsig_verify_single(vrfy, combined_sig, msg, NULL, &combiner_sum_2, &combiner_sum_2, NULL, 0));

/* Test subtracting partial sig from the completed sig (as used in early payment proofs)*/
sub_return_val = secp256k1_aggsig_subtract_partial_signature(none, sub_result, sub_result_alt, combined_sig, sigs[0]);

/* Check non-0 return */
CHECK(sub_return_val);

/* And this will never be -1 */
CHECK(sub_return_val != -1);

/* If there's only one possible result, it should be the only one that needs to be tested */
if (sub_return_val == 1) {
for (j = 0; j < 64; j++){
CHECK(sub_result[j] == sigs[1][j]);
}
}

/* if there are two possible results, both potentially need to be checked */
if (sub_return_val == 2) {
sub_check = 1;
for (j = 0; j < 64; j++){
if (sub_result[j] != sigs[1][j]) {
sub_check = 0;
}
}
if (!sub_check) {
sub_check = 1;
for (j = 0; j < 64; j++){
if (sub_result_alt[j] != sigs[1][j]) {
sub_check = 0;
}
}
}
CHECK(sub_check);
}
}
/* check subtraction function returns -1 for values known to result in no possiblity being a quadratic residue */
memcpy(combined_sig, no_qr_const_1, 32);
memcpy(sigs[0], no_qr_const_2, 32);
sub_return_val = secp256k1_aggsig_subtract_partial_signature(none, sub_result, sub_result_alt, combined_sig, sigs[0]);
CHECK(sub_return_val == -1);

/*** End aggsig for Grin exchange test ***/

/* cleanup */
Expand Down