refactor: rewrite some checks

StackOverflowExcept1on · Oct 29, 2024 · 01f3073 · 01f3073
1 parent 11df429
commit 01f3073
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 46 deletions.
diff --git a/src/FROST.sol b/src/FROST.sol
@@ -133,8 +133,9 @@ library FROST {
      * @dev Verifies `FROST-secp256k1-KECCAK256` signature by formula $zG - cX = R$.
      *      - Public key ($X$) must be checked with `FROST.isValidPublicKey(publicKeyX, publicKeyY)`.
      *      - Signature R ($R$) must be on curve.
-     *      - Signature Z ($z$).
-     *      - Challenge ($c$) is computed via `FROST.computateChallenge(...)`.
+     *      - Signature Z ($z$) must be in `[1, Secp256k1.N)`.
+     *      - Challenge ($c$) is computed via `FROST.computateChallenge(...)`,
+     *        must be in `[1, Secp256k1.N)`.
      * @param publicKeyX Public key x.
      * @param publicKeyY Public key y.
      * @param signatureRX Signature R x.
@@ -155,18 +156,31 @@ library FROST {
         // https://github.com/ZcashFoundation/frost/blob/2d88edf1623ee29f671a43966aae0bd4ead2ea7a/frost-core/src/traits.rs#L225
         // https://github.com/ZcashFoundation/frost/blob/2d88edf1623ee29f671a43966aae0bd4ead2ea7a/frost-core/src/verifying_key.rs#L54
 
-        if (!Schnorr.isValidSignatureR(signatureRX, signatureRY)) {
+        if (!Secp256k1.isOnCurve(signatureRX, signatureRY)) {
             return false;
         }
 
-        if (!Schnorr.isValidMultiplier(signatureZ)) {
+        // `signatureZ` is always in `[1, Secp256k1.N)` and valid non-zero scalar because:
+        // 1. `signatureZ = 0`  is checked.
+        // 2. `signatureZ >= Secp256k1.N` is checked.
+        // thus `signatureZ % Secp256k1.N != 0`.
+        if (signatureZ == 0) {
+            return false;
+        }
+
+        if (signatureZ >= Secp256k1.N) {
             return false;
         }
 
         (uint256 memPtr, uint256 challenge) =
             computateChallenge(publicKeyX, publicKeyY, signatureRX, signatureRY, messageHash);
 
-        if (!Schnorr.isValidMultiplier(challenge)) {
+        // `challenge` is always in `[1, Secp256k1.N)` and valid non-zero scalar because:
+        // 1. `FROST.computateChallenge(...)` returns `challenge < Secp256k1.N`
+        //    (it uses modular arithmetic).
+        // 2. `challenge = 0` is checked.
+        // thus `challenge % Secp256k1.N != 0`.
+        if (challenge == 0) {
             return false;
         }
 

diff --git a/src/TranspiledFROST.sol b/src/TranspiledFROST.sol
@@ -81,10 +81,11 @@ library TranspiledFROST {
                     var := 0
                     leave
                 }
-                let var_1 := 0
-                let _1 := iszero(mod(var_signatureZ, not(0x014551231950b75fc4402da1732fc9bebe)))
-                var_1 := iszero(_1)
-                if _1 {
+                if iszero(var_signatureZ) {
+                    var := 0
+                    leave
+                }
+                if iszero(lt(var_signatureZ, not(0x014551231950b75fc4402da1732fc9bebe))) {
                     var := 0
                     leave
                 }
@@ -104,25 +105,22 @@ library TranspiledFROST {
                 mstore(add(var_memPtr, 234), 0x300046524f53542d736563703235366b312d4b454343414b3235362d763163)
                 mstore(add(var_memPtr, 266), shl(229, 0x03430b61))
                 let var_value := keccak256(var_memPtr, ret)
-                let _2 := add(var_memPtr, 204)
-                mstore(_2, var_value)
-                let _3 := add(var_memPtr, 236)
-                mstore8(_3, 0x01)
+                let _1 := add(var_memPtr, 204)
+                mstore(_1, var_value)
+                let _2 := add(var_memPtr, 236)
+                mstore8(_2, 0x01)
                 let ret_1 := 0
                 ret_1 := 66
-                let var_value_1 := keccak256(_2, 66)
-                mstore(_2, xor(var_value, var_value_1))
-                mstore8(_3, 0x02)
+                let var_value_1 := keccak256(_1, 66)
+                mstore(_1, xor(var_value, var_value_1))
+                mstore8(_2, 0x02)
                 let expr_1 :=
                     addmod(
                         mulmod(shr(0x40, var_value_1), shl(192, 1), not(0x014551231950b75fc4402da1732fc9bebe)),
-                        or(and(shl(0x80, var_value_1), shl(128, 0xffffffffffffffff)), shr(0x80, keccak256(_2, 66))),
+                        or(and(shl(0x80, var_value_1), shl(128, 0xffffffffffffffff)), shr(0x80, keccak256(_1, 66))),
                         not(0x014551231950b75fc4402da1732fc9bebe)
                     )
-                let var_2 := 0
-                let _4 := iszero(mod(expr_1, not(0x014551231950b75fc4402da1732fc9bebe)))
-                var_2 := iszero(_4)
-                if _4 {
+                if iszero(expr_1) {
                     var := 0
                     leave
                 }

diff --git a/src/utils/cryptography/Schnorr.sol b/src/utils/cryptography/Schnorr.sol
@@ -44,31 +44,12 @@ library Schnorr {
         return publicKeyX < Secp256k1.N && Secp256k1.isOnCurve(publicKeyX, publicKeyY);
     }
 
-    /**
-     * @dev Checks if `signature.R` public key `(x, y)` is on curve.
-     * @param signatureRX Public key x.
-     * @param signatureRY Public key y.
-     * @return isValidSignatureR `true` if `signature.R` public key is on curve, `false` otherwise.
-     */
-    function isValidSignatureR(uint256 signatureRX, uint256 signatureRY) internal pure returns (bool) {
-        return Secp256k1.isOnCurve(signatureRX, signatureRY);
-    }
-
-    /**
-     * @dev Checks if `multiplier % Secp256k1.N != 0`.
-     * @param multiplier Multiplier.
-     * @return isValidMultiplier `true` if `multiplier % Secp256k1.N != 0`, `false` otherwise.
-     */
-    function isValidMultiplier(uint256 multiplier) internal pure returns (bool) {
-        return multiplier % Secp256k1.N != 0;
-    }
-
     /**
      * @dev Verifies Schnorr signature by formula $zG - cX = R$.
      *      - Public key ($X$) must be checked with `Schnorr.isValidPublicKey(publicKeyX, publicKeyY)`.
-     *      - Signature R ($R$) must be checked with `Schnorr.isValidSignatureR(signatureRX, signatureRY)`.
-     *      - Signature Z ($z$) must be checked with `Schnorr.isValidMultiplier(signatureZ)`.
-     *      - Challenge ($c$) must be checked with `Schnorr.isValidMultiplier(challenge)`.
+     *      - Signature R ($R$) must be checked with `Secp256k1.isOnCurve(signatureRX, signatureRY)`.
+     *      - Signature Z ($z$) must be checked with `signatureZ % Secp256k1.N != 0`.
+     *      - Challenge ($c$) must be checked with `challenge % Secp256k1.N != 0`.
      * @param memPtr Memory pointer for writing 128 bytes of input data.
      * @param publicKeyX Public key x.
      * @param publicKeyY Public key y.
@@ -127,9 +108,9 @@ library Schnorr {
         //    ```
         //
         //    but `a % Secp256k1.N != 0` and `b % Secp256k1.N != 0` because it's checked with:
-        //    - `Schnorr.isValidMultiplier(signatureZ)`.
+        //    - `signatureZ % Secp256k1.N != 0`.
         //    - `Schnorr.isValidPublicKey(publicKeyX, publicKeyY)`.
-        //       it also checks `Schnorr.isValidMultiplier(publicKeyX)`
+        //       it also checks `publicKeyX % Secp256k1.N != 0`
         //       (see `isValidPublicKey` implementation).
         //
         //    thus `mulmod(a, b, Secp256k1.N) != 0`.
@@ -199,7 +180,7 @@ library Schnorr {
         // so we also need to convert Signature R to Ethereum address using `Secp256k1.toAddress(signatureRX, signatureRY)`.
 
         // we also previously checked that Signature R is on curve using
-        // `Schnorr.isValidSignatureR(signatureRX, signatureRY)`.
+        // `Secp256k1.isOnCurve(signatureRX, signatureRY)`.
 
         return ECDSA.recover(memPtr, e, v, r, s) == Secp256k1.toAddress(signatureRX, signatureRY);
     }