f fix state machine test and improve documentation

Author: jonasnick

src/modules/musig/tests_impl.h

@@ -361,8 +361,14 @@ void musig_api_tests(secp256k1_scratch_space *scratch) {
     secp256k1_context_destroy(vrfy);
 }
 
-/* Returns the messagehash of a session where get_public_nonce was called with different signers than initialized */
-int musig_state_machine_diff_signer_test(unsigned char *msghash, secp256k1_pubkey *pks, secp256k1_pubkey *combined_pk, unsigned char *pk_hash, const unsigned char * const *nonce_commitments, unsigned char *msg, secp256k1_pubkey *nonce_other, unsigned char *sk, unsigned char *session_id) {
+/* Initializes two sessions, one use the given parameters (session_id,
+ * nonce_commitments, etc.) except that `session_tmp` uses new signers with different
+ * public keys. The point of this test is to call `musig_session_get_public_nonce`
+ * with signers from `session_tmp` who have different public keys than the correct
+ * ones and return the resulting messagehash. This should not result in a different
+ * messagehash because the public keys of the signers are only used during session
+ * initialization. */
+int musig_state_machine_diff_signer_msghash_test(unsigned char *msghash, secp256k1_pubkey *pks, secp256k1_pubkey *combined_pk, unsigned char *pk_hash, const unsigned char * const *nonce_commitments, unsigned char *msg, secp256k1_pubkey *nonce_other, unsigned char *sk, unsigned char *session_id) {
     secp256k1_musig_session session;
     secp256k1_musig_session session_tmp;
     unsigned char nonce_commitment[32];
@@ -383,20 +389,26 @@ int musig_state_machine_diff_signer_test(unsigned char *msghash, secp256k1_pubke
 
     CHECK(secp256k1_musig_session_initialize(ctx, &session, signers, nonce_commitment, session_id, msg, combined_pk, pk_hash, pks, 2, 0, sk) == 1);
     CHECK(memcmp(nonce_commitment, nonce_commitments[1], 32) == 0);
-    CHECK(secp256k1_musig_session_get_public_nonce(ctx, &session, signers, &nonce, nonce_commitments, 2) == 1);
+    /* Call get_public_nonce with different signers than the signers the session was
+     * initialized with. */
+    CHECK(secp256k1_musig_session_get_public_nonce(ctx, &session_tmp, signers, &nonce, nonce_commitments, 2) == 1);
+    CHECK(secp256k1_musig_session_get_public_nonce(ctx, &session, signers_tmp, &nonce, nonce_commitments, 2) == 1);
     CHECK(secp256k1_musig_set_nonce(ctx, &signers[0], nonce_other) == 1);
     CHECK(secp256k1_musig_set_nonce(ctx, &signers[1], &nonce) == 1);
     CHECK(secp256k1_musig_session_combine_nonces(ctx, &session, signers, 2, NULL, NULL) == 1);
 
     return secp256k1_musig_compute_messagehash(ctx, msghash, &session);
 }
 
-/* Creates a new session and tries to combine nonces with given signers_other
- */
-int musig_state_machine_diff_signers_combine_nonce_test(secp256k1_pubkey *pks, secp256k1_pubkey *combined_pk, unsigned char *pk_hash, unsigned char *nonce_commitment_other, secp256k1_pubkey *nonce_other, unsigned char *msg, unsigned char *sk, secp256k1_musig_session_signer_data *signers_other) {
-    /* Create new session (different session id) */
+/* Creates a new session (with a different session id) and tries to use that session
+ * to combine nonces with given signers_other. This should fail, because the nonce
+ * commitments of signers_other do not match the nonce commitments the new session
+ * was initialized with. If do_test is 0, the correct signers are being used and
+ * therefore the function should return 1. */
+int musig_state_machine_diff_signers_combine_nonce_test(secp256k1_pubkey *pks, secp256k1_pubkey *combined_pk, unsigned char *pk_hash, unsigned char *nonce_commitment_other, secp256k1_pubkey *nonce_other, unsigned char *msg, unsigned char *sk, secp256k1_musig_session_signer_data *signers_other, int do_test) {
     secp256k1_musig_session session;
     secp256k1_musig_session_signer_data signers[2];
+    secp256k1_musig_session_signer_data *signers_to_use;
     unsigned char nonce_commitment[32];
     unsigned char session_id[32];
     secp256k1_pubkey nonce;
@@ -410,12 +422,19 @@ int musig_state_machine_diff_signers_combine_nonce_test(secp256k1_pubkey *pks, s
     CHECK(secp256k1_musig_session_get_public_nonce(ctx, &session, signers, &nonce, ncs, 2) == 1);
     CHECK(secp256k1_musig_set_nonce(ctx, &signers[0], nonce_other) == 1);
     CHECK(secp256k1_musig_set_nonce(ctx, &signers[1], &nonce) == 1);
-
-    return secp256k1_musig_session_combine_nonces(ctx, &session, signers_other, 2, NULL, NULL);
+    CHECK(secp256k1_musig_set_nonce(ctx, &signers[1], &nonce) == 1);
+    secp256k1_musig_session_combine_nonces(ctx, &session, signers_other, 2, NULL, NULL);
+    if (do_test) {
+        signers_to_use = signers_other;
+    } else {
+        signers_to_use = signers;
+    }
+    return secp256k1_musig_session_combine_nonces(ctx, &session, signers_to_use, 2, NULL, NULL);
 }
 
-/* Initialize session with given msg and try to sign. Creates MuSig session
- * with a given signer and a signer with sk. Should fail if msg is NULL */
+/* Recreates a session with the given session_id, signers, pk, msg etc. parameters
+ * and tries to sign and verify the other signers partial signature. Both should fail
+ * if msg is NULL. */
 int musig_state_machine_missing_msg_test(secp256k1_pubkey *pks, secp256k1_pubkey *combined_pk, unsigned char *pk_hash, unsigned char *nonce_commitment_other, secp256k1_pubkey *nonce_other, secp256k1_musig_partial_signature *partial_sig_other, unsigned char *sk, unsigned char *session_id, unsigned char *msg) {
     secp256k1_musig_session session;
     secp256k1_musig_session_signer_data signers[2];
@@ -443,10 +462,10 @@ int musig_state_machine_missing_msg_test(secp256k1_pubkey *pks, secp256k1_pubkey
     return partial_sign || partial_verify;
 }
 
-
-/* If do_combine is 0, don't combine nonces before verifying and combining
- * partial sigs. Creates MuSig session between given signer and new signer with
- * sk, session_id and partial_sig. */
+/* Recreates a session with the given session_id, signers, pk, msg etc. parameters
+ * and tries to verify and combine partial sigs. If do_combine is 0, the
+ * combine_nonces step is left out. In that case verify and combine should fail and
+ * this function should return 0. */
 int musig_state_machine_missing_combine_test(secp256k1_pubkey *pks, secp256k1_pubkey *combined_pk, unsigned char *pk_hash, unsigned char *nonce_commitment_other, secp256k1_pubkey *nonce_other, secp256k1_musig_partial_signature *partial_sig_other, unsigned char *msg, unsigned char *sk, unsigned char *session_id, secp256k1_musig_partial_signature *partial_sig, int do_combine) {
     secp256k1_musig_session session;
     secp256k1_musig_session_signer_data signers[2];
@@ -538,7 +557,8 @@ void musig_state_machine_tests(secp256k1_scratch_space *scratch) {
     CHECK(secp256k1_musig_set_nonce(ctx, &signers1[1], &nonce[1]) == 1);
 
     /* Can't combine nonces from signers of a different session */
-    CHECK(musig_state_machine_diff_signers_combine_nonce_test(pk, &combined_pk, pk_hash, nonce_commitment[0], &nonce[0], msg, sk[1], signers1) == 0);
+    CHECK(musig_state_machine_diff_signers_combine_nonce_test(pk, &combined_pk, pk_hash, nonce_commitment[0], &nonce[0], msg, sk[1], signers1, 1) == 0);
+    CHECK(musig_state_machine_diff_signers_combine_nonce_test(pk, &combined_pk, pk_hash, nonce_commitment[0], &nonce[0], msg, sk[1], signers1, 0) == 1);
 
     /* Partially sign */
     CHECK(secp256k1_musig_partial_sign(ctx, &session[0], &partial_sig[0]) == 1);
@@ -547,10 +567,11 @@ void musig_state_machine_tests(secp256k1_scratch_space *scratch) {
     CHECK(secp256k1_musig_partial_sign(ctx, &session[1], &partial_sig[1]) == 0);
     CHECK(secp256k1_musig_session_combine_nonces(ctx, &session[1], signers1, 2, NULL, NULL) == 1);
     CHECK(secp256k1_musig_partial_sig_verify(ctx, &session[1], &signers1[0], &partial_sig[0]) == 1);
-    /* messagehash should be the same as a session whose get_public_nonce was
-     * called with different signers. */
+    /* messagehash should be the same as a session whose get_public_nonce was called
+     * with different signers (i.e. they diff in public keys). This is because the
+     * public keys of the signers is set in stone when initializing the session. */
     CHECK(secp256k1_musig_compute_messagehash(ctx, msghash1, &session[1]) == 1);
-    CHECK(musig_state_machine_diff_signer_test(msghash2, pk, &combined_pk, pk_hash, ncs, msg, &nonce[0], sk[1], session_id[1]) == 1);
+    CHECK(musig_state_machine_diff_signer_msghash_test(msghash2, pk, &combined_pk, pk_hash, ncs, msg, &nonce[0], sk[1], session_id[1]) == 1);
     CHECK(memcmp(msghash1, msghash2, 32) == 0);
     CHECK(secp256k1_musig_partial_sign(ctx, &session[1], &partial_sig[1]) == 1);
     CHECK(secp256k1_musig_partial_sig_verify(ctx, &session[1], &signers1[1], &partial_sig[1]) == 1);