Address review comments

ACINQ · Apr 17, 2024 · 2732f67 · 2732f67
1 parent e532ad8
commit 2732f67
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 15 deletions.
diff --git a/src/commonMain/kotlin/fr/acinq/secp256k1/Secp256k1.kt b/src/commonMain/kotlin/fr/acinq/secp256k1/Secp256k1.kt
@@ -218,12 +218,12 @@ public interface Secp256k1 {
     public fun musigNonceProcess(aggnonce: ByteArray, msg32: ByteArray, keyaggCache: ByteArray): ByteArray
 
     /**
-     * Validate a musig2 secret nonce
-     * @param secretnonce secret nonce
-     * @param pubkey public key that was passed to the nonce generation method
-     * @return false if the secret nonce does not match the public key
+     * Check that a secret nonce was generated with a public key that matches the private key used for signing.
+     * @param secretnonce secret nonce.
+     * @param pubkey public key for the private key that will be used, with the secret nonce, to generate a partial signature.
+     * @return false if the secret nonce does not match the public key.
      */
-    public fun musigNoncevalidate(secretnonce: ByteArray, pubkey: ByteArray): Boolean {
+    public fun musigNonceValidate(secretnonce: ByteArray, pubkey: ByteArray): Boolean {
         if (secretnonce.size != MUSIG2_SECRET_NONCE_SIZE) return false
         if (pubkey.size != 33 && pubkey.size != 65) return false
         val pk = Secp256k1.pubkeyParse(pubkey)

diff --git a/src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt b/src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt
@@ -81,7 +81,7 @@ public object Secp256k1Native : Secp256k1 {
         return serialized.readBytes(Secp256k1.MUSIG2_PUBLIC_NONCE_SIZE)
     }
 
-    private fun DeferScope.toNat(bytes: ByteArray): CPointer<UByteVar>  {
+    private fun DeferScope.toNat(bytes: ByteArray): CPointer<UByteVar> {
         val ubytes = bytes.asUByteArray()
         val pinned = ubytes.pin()
         this.defer { pinned.unpin() }
@@ -112,7 +112,7 @@ public object Secp256k1Native : Secp256k1 {
     }
 
     public override fun signatureNormalize(sig: ByteArray): Pair<ByteArray, Boolean> {
-        require(sig.size >= 64){ "invalid signature ${Hex.encode(sig)}" }
+        require(sig.size >= 64) { "invalid signature ${Hex.encode(sig)}" }
         memScoped {
             val nSig = allocSignature(sig)
             val isHighS = secp256k1_ecdsa_signature_normalize(ctx, nSig.ptr, nSig.ptr)
@@ -307,7 +307,16 @@ public object Secp256k1Native : Secp256k1 {
                 memcpy(n.ptr, toNat(it), Secp256k1.MUSIG2_PUBLIC_KEYAGG_CACHE_SIZE.toULong())
                 n
             }
-            secp256k1_musig_nonce_gen(ctx, secnonce.ptr, pubnonce.ptr, toNat(sessionId32), privkey?.let { toNat(it) }, nPubkey.ptr, msg32?.let { toNat(it) },nKeyAggCache?.ptr, extraInput32?.let { toNat(it) }).requireSuccess("secp256k1_musig_nonce_gen() failed")
+            secp256k1_musig_nonce_gen(
+                ctx,
+                secnonce.ptr,
+                pubnonce.ptr,
+                toNat(sessionId32),
+                privkey?.let { toNat(it) },
+                nPubkey.ptr,
+                msg32?.let { toNat(it) },
+                nKeyAggCache?.ptr,
+                extraInput32?.let { toNat(it) }).requireSuccess("secp256k1_musig_nonce_gen() failed")
             val nPubnonce = allocArray<UByteVar>(Secp256k1.MUSIG2_PUBLIC_NONCE_SIZE)
             secp256k1_musig_pubnonce_serialize(ctx, nPubnonce, pubnonce.ptr).requireSuccess("secp256k1_musig_pubnonce_serialize failed")
             secnonce.ptr.readBytes(Secp256k1.MUSIG2_SECRET_NONCE_SIZE) + nPubnonce.readBytes(Secp256k1.MUSIG2_PUBLIC_NONCE_SIZE)
@@ -339,7 +348,7 @@ public object Secp256k1Native : Secp256k1 {
                 n
             }
             secp256k1_musig_pubkey_agg(ctx, combined.ptr, nKeyAggCache?.ptr, nPubkeys.toCValues(), pubkeys.size.convert()).requireSuccess("secp256k1_musig_nonce_agg() failed")
-            val agg =  serializeXonlyPubkey(combined)
+            val agg = serializeXonlyPubkey(combined)
             keyaggCache?.let { blob -> nKeyAggCache?.let { memcpy(toNat(blob), it.ptr, Secp256k1.MUSIG2_PUBLIC_KEYAGG_CACHE_SIZE.toULong()) } }
             return agg
         }
@@ -386,14 +395,14 @@ public object Secp256k1Native : Secp256k1 {
             memcpy(toNat(session), nSession.ptr, Secp256k1.MUSIG2_PUBLIC_SESSION_SIZE.toULong())
             return session
         }
-     }
+    }
 
     override fun musigPartialSign(secnonce: ByteArray, privkey: ByteArray, keyaggCache: ByteArray, session: ByteArray): ByteArray {
         require(secnonce.size == Secp256k1.MUSIG2_SECRET_NONCE_SIZE)
         require(privkey.size == 32)
         require(keyaggCache.size == Secp256k1.MUSIG2_PUBLIC_KEYAGG_CACHE_SIZE)
         require(session.size == Secp256k1.MUSIG2_PUBLIC_SESSION_SIZE)
-        require(musigNoncevalidate(secnonce, pubkeyCreate(privkey)))
+        require(musigNonceValidate(secnonce, pubkeyCreate(privkey)))
 
         memScoped {
             val nSecnonce = alloc<secp256k1_musig_secnonce>()

diff --git a/tests/src/commonTest/kotlin/fr/acinq/secp256k1/Secp256k1Test.kt b/tests/src/commonTest/kotlin/fr/acinq/secp256k1/Secp256k1Test.kt
@@ -459,13 +459,13 @@ class Secp256k1Test {
         val agg3 = Secp256k1.musigPubkeyXonlyTweakAdd(cache, Hex.decode("7468697320636f756c64206265206120746170726f6f7420747765616b2e2e00"))
         assertEquals("04537a081a8d32ff700ca86aaa77a423e9b8d1480938076b645c68ee39d263c93948026928799b2d942cb5851db397015b26b1759de1b9ab2c691ced64a2eef836", Hex.encode(agg3))
     }
-    
+
     @Test
     fun testMusig2SigningSession() {
         val privkeys = listOf(randomBytes(32), randomBytes(32))
-        val pubkeys = privkeys.map { Secp256k1.pubkeyCreate(it) }
-
         val sessionId = randomBytes(32)
+        val msg32 = randomBytes(32)
+        val pubkeys = privkeys.map { Secp256k1.pubkeyCreate(it) }
         val nonces = pubkeys.map { Secp256k1.musigNonceGen(sessionId, null, it, null, null, null) }
         val secnonces = nonces.map { it.copyOfRange(0, 132) }
         val pubnonces = nonces.map { it.copyOfRange(132, 132 + 66) }
@@ -476,7 +476,6 @@ class Secp256k1Test {
         assertContentEquals(aggpubkey, Secp256k1.musigPubkeyAgg(pubkeys.toTypedArray(), keyaggCaches[1]))
         assertContentEquals(keyaggCaches[0], keyaggCaches[1])
 
-        val msg32 = randomBytes(32)
         val sessions = (0 until 2).map { Secp256k1.musigNonceProcess(aggnonce, msg32, keyaggCaches[it]) }
         val psigs = (0 until 2).map {
             val psig = Secp256k1.musigPartialSign(secnonces[it], privkeys[it], keyaggCaches[it], sessions[it])
@@ -485,13 +484,26 @@ class Secp256k1Test {
             psig
         }
 
+        fun printTestData() {
+            println("private keys")
+            privkeys.forEach { println(Hex.encode(it)) }
+            println("sessionId ${Hex.encode(sessionId)}")
+            println("msg32 ${Hex.encode(msg32)}")
+            println("nonces")
+            nonces.forEach { println(Hex.encode(it)) }
+        }
+
         // signing fails if the secret nonce does not match the private key's public key
         assertFails {
             Secp256k1.musigPartialSign(secnonces[1], privkeys[0], keyaggCaches[0], sessions[0])
+            println("musigPartialSign should have failed !")
+            printTestData()
         }
 
         assertFails {
             Secp256k1.musigPartialSign(secnonces[0], privkeys[1], keyaggCaches[1], sessions[1])
+            println("musigPartialSign should have failed !")
+            printTestData()
         }
 
         val sig = Secp256k1.musigPartialSigAgg(sessions[0], psigs.toTypedArray())