fixup: jonasnick comments

doc/musig-reference.py

@@ -236,6 +236,7 @@ def get_session_key_agg_coeff(session_ctx: SessionContext, P: Point) -> int:
     (_, pubkeys, _, _, _) = session_ctx
     return key_agg_coeff(pubkeys, bytes_from_point(P))
 
+# Callers should overwrite secnonce with zeros after calling sign.
 def sign(secnonce: bytes, sk: bytes, session_ctx: SessionContext) -> bytes:
     (Q, gacc_v, _, b, R, e) = get_session_values(session_ctx)
     k_1_ = int_from_bytes(secnonce[0:32])
@@ -254,15 +255,15 @@ def sign(secnonce: bytes, sk: bytes, session_ctx: SessionContext) -> bytes:
     s = (k_1 + b * k_2 + e * a * d) % n
     psig = bytes_from_int(s)
     pubnonce = cbytes(point_mul(G, k_1_)) + cbytes(point_mul(G, k_2_))
-    assert partial_sig_verify_internal(psig, pubnonce, bytes_from_point(P), session_ctx)
+    partial_sig_verify_internal(psig, pubnonce, bytes_from_point(P), session_ctx)
     return psig
 
-def partial_sig_verify(psig: bytes, pubnonces: List[bytes], pubkeys: List[bytes], tweaks: List[bytes], is_xonly: List[bool], msg: bytes, i: int) -> bool:
+def partial_sig_verify(psig: bytes, pubnonces: List[bytes], pubkeys: List[bytes], tweaks: List[bytes], is_xonly: List[bool], msg: bytes, i: int) -> None:
     aggnonce = nonce_agg(pubnonces)
     session_ctx = SessionContext(aggnonce, pubkeys, tweaks, is_xonly, msg)
-    return partial_sig_verify_internal(psig, pubnonces[i], pubkeys[i], session_ctx)
+    partial_sig_verify_internal(psig, pubnonces[i], pubkeys[i], session_ctx)
 
-def partial_sig_verify_internal(psig: bytes, pubnonce: bytes, pk_: bytes, session_ctx: SessionContext) -> bool:
+def partial_sig_verify_internal(psig: bytes, pubnonce: bytes, pk_: bytes, session_ctx: SessionContext) -> None:
     (Q, gacc_v, _, b, R, e) = get_session_values(session_ctx)
     s = int_from_bytes(psig)
     assert s < n
@@ -274,7 +275,7 @@ def partial_sig_verify_internal(psig: bytes, pubnonce: bytes, pk_: bytes, sessio
     g_ = g_v * gacc_v % n
     P = point_mul(lift_x(pk_), g_)
     a = get_session_key_agg_coeff(session_ctx, P)
-    return point_mul(G, s) == point_add(R_, point_mul(P, e * a % n))
+    assert point_mul(G, s) == point_add(R_, point_mul(P, e * a % n))
 
 def partial_sig_agg(psigs: List[bytes], session_ctx: SessionContext) -> bytes:
     (Q, _, tacc_v, _, R, e) = get_session_values(session_ctx)
@@ -293,6 +294,17 @@ def partial_sig_agg(psigs: List[bytes], session_ctx: SessionContext) -> bytes:
 # See `musig_test_vectors_keyagg` and `musig_test_vectors_sign` in
 # https://github.com/ElementsProject/secp256k1-zkp/blob/master/src/modules/musig/tests_impl.h
 #
+
+# Modified version of partial_sig_verify that returns True or False to
+# indicate success or failure, instead of raising an assertion on failure.
+# This version is more convenient to use in tests.
+def partial_sig_verify_test(psig: bytes, pubnonces: List[bytes], pubkeys: List[bytes], tweaks: List[bytes], is_xonly: List[bool], msg: bytes, i: int) -> bool:
+    try:
+        partial_sig_verify(psig, pubnonces, pubkeys, tweaks, is_xonly, msg, i)
+        return True
+    except AssertionError:
+        return False
+
 def fromhex_all(l):
     return [bytes.fromhex(l_i) for l_i in l]
 
@@ -342,6 +354,9 @@ def test_sign_vectors():
 
     session_ctx = SessionContext(aggnonce, [pk, X[0], X[1]], [], [], msg)
     assert sign(secnonce, sk, session_ctx) == expected[0]
+    # WARNING: An actual implementation should clear the secnonce after use,
+    # e.g. by setting secnonce = bytes(64) after usage. Reusing the secnonce, as
+    # we do here for testing purposes, can leak the secret key.
 
     session_ctx = SessionContext(aggnonce, [X[0], pk, X[1]], [], [], msg)
     assert sign(secnonce, sk, session_ctx) == expected[1]
@@ -385,6 +400,9 @@ def test_tweak_vectors():
     # A single x-only tweak
     session_ctx = SessionContext(aggnonce, [X[0], X[1], pk], tweaks[:1], [True], msg)
     assert sign(secnonce, sk, session_ctx) == expected[0]
+    # WARNING: An actual implementation should clear the secnonce after use,
+    # e.g. by setting secnonce = bytes(64) after usage. Reusing the secnonce, as
+    # we do here for testing purposes, can leak the secret key.
 
     # A single ordinary tweak
     session_ctx = SessionContext(aggnonce, [X[0], X[1], pk], tweaks[:1], [False], msg)
@@ -432,16 +450,20 @@ def test_sign_and_verify_random(iters):
 
         session_ctx = SessionContext(aggnonce, pubkeys, tweaks, is_xonly, msg)
         psig_1 = sign(secnonce_1, sk_1, session_ctx)
-        assert partial_sig_verify(psig_1, pubnonces, pubkeys, tweaks, is_xonly, msg, 0)
+        # Clear the secnonce after use
+        secnonce_1 = bytes(64)
+        assert partial_sig_verify_test(psig_1, pubnonces, pubkeys, tweaks, is_xonly, msg, 0)
 
         # Wrong signer index
-        assert not partial_sig_verify(psig_1, pubnonces, pubkeys, tweaks, is_xonly, msg, 1)
+        assert not partial_sig_verify_test(psig_1, pubnonces, pubkeys, tweaks, is_xonly, msg, 1)
 
         # Wrong message
-        assert not partial_sig_verify(psig_1, pubnonces, pubkeys, tweaks, is_xonly, secrets.token_bytes(32), 0)
+        assert not partial_sig_verify_test(psig_1, pubnonces, pubkeys, tweaks, is_xonly, secrets.token_bytes(32), 0)
 
         psig_2 = sign(secnonce_2, sk_2, session_ctx)
-        assert partial_sig_verify(psig_2, pubnonces, pubkeys, tweaks, is_xonly, msg, 1)
+        # Clear the secnonce after use
+        secnonce_2 = bytes(64)
+        assert partial_sig_verify_test(psig_2, pubnonces, pubkeys, tweaks, is_xonly, msg, 1)
 
         sig = partial_sig_agg([psig_1, psig_2], session_ctx)
         assert schnorr_verify(msg, aggpk, sig)

doc/musig-spec.mediawiki

@@ -248,7 +248,7 @@ Avoiding reuse also implies that the ''NonceGen'' algorithm must compute unbiase
 There are some vectors in libsecp256k1's [https://github.com/ElementsProject/secp256k1-zkp/blob/master/src/modules/musig/tests_impl.h MuSig test file].
 Search for the ''musig_test_vectors_keyagg'' and ''musig_test_vectors_sign'' functions.
 
-We provide a naive, highly inefficient, and non-constant time [[musig-reference.py|pure Python 3.7 reference implementation of the key aggregation, partial signing, and partial signature verification algorithms]].
+We provide a naive, highly inefficient, and non-constant time [[musig-reference.py|pure Python 3 reference implementation of the key aggregation, partial signing, and partial signature verification algorithms]].
 The reference implementation is for demonstration purposes only and not to be used in production environments.
 
 == Footnotes ==