ECDSA: Finished property tests

verklegarden · Nov 26, 2023 · b1c2112 · b1c2112
1 parent c5a3069
commit b1c2112
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 27 deletions.
diff --git a/README.md b/README.md
@@ -60,8 +60,8 @@ $ forge fmt [--check]
 | ---------------------------- | -------------- | ------------------------ | ----------------------------- |
 | `curves/Secp256k1`           | ✅              | ❌                        | ❌                             |
 | `curves/Secp256k1Arithmetic` | ✅              | ❌                        | ❌                             |
-| `signatures/ECDSA`           | ✅              | ❌                        | ❌                             |
-| `signatures/Schnorr`         | ❌              | ❌                        | ❌                             |
+| `signatures/ECDSA`           | ✅              | ✅                        | ❌                             |
+| `signatures/Schnorr`         | ✅              | ✅                        | ❌                             |
 | `signatures/utils/Nonce`     | ✅              | ❌                        | ❌                             |
 | `Random`                     | ✅              | ❌                        | ❌                             |
 | `Message`                    | ✅              | ❌                        | ✅                             |

diff --git a/src/signatures/ECDSA.sol b/src/signatures/ECDSA.sol
@@ -383,22 +383,7 @@ library ECDSA {
         pure
         returns (bytes memory)
     {
-        bytes memory blob;
-
-        // TODO: Use direct access in assembly.
-        uint8 v = sig.v; // TODO: Does this use one word or a single byte?
-        bytes32 r = sig.r;
-        bytes32 s = sig.s;
-        assembly ("memory-safe") {
-            // Signature consists of two words and one byte.
-            mstore(blob, 0x41)
-
-            mstore(add(blob, 0x20), r)
-            mstore(add(blob, 0x40), s)
-            // Note to shift v to highest-order byte.
-            mstore(add(blob, 0x60), shl(248, v))
-        }
-        return blob;
+        return abi.encodePacked(sig.r, sig.s, sig.v);
     }
 
     /// @dev Returns Signature from bytes `blob`.
@@ -441,7 +426,6 @@ library ECDSA {
     {
         bytes memory blob;
 
-        // TODO: Use direct access in assembly.
         uint8 v = sig.v;
         bytes32 r = sig.r;
         bytes32 s = sig.s;

diff --git a/src/signatures/Schnorr.sol b/src/signatures/Schnorr.sol
@@ -198,7 +198,7 @@ library Schnorr {
 
         // Derive deterministic nonce ∊ [1, Q).
         uint nonce = privKey.deriveNonce(digest) % Secp256k1.Q;
-        assert(nonce != 0); // TODO: Revisit once nonce derived via RFC 6979.
+        // assert(nonce != 0); // TODO: Revisit once nonce derived via RFC 6979.
 
         // Compute nonce's public key.
         PublicKey memory noncePubKey =

diff --git a/test/signatures/ECDSA.t.sol b/test/signatures/ECDSA.t.sol
@@ -264,7 +264,9 @@ contract ECDSATest is Test {
         assertEq(got, want);
     }
 
-    function testFuzz_signatureFromBytes(uint8 v, bytes32 r, bytes32 s) public {
+    function testFuzz_signatureFromBytes(uint8 v, bytes32 r, bytes32 s)
+        public
+    {
         bytes memory blob = abi.encodePacked(r, s, v);
 
         Signature memory got = wrapper.signatureFromBytes(blob);

diff --git a/test/signatures/ECDSAProperties.t.sol b/test/signatures/ECDSAProperties.t.sol
@@ -21,7 +21,7 @@ contract ECDSAPropertiesTest is Test {
     //--------------------------------------------------------------------------
     // Properties: Signature
 
-    function testProperty_CreatedSignaturesAreVerifiable(
+    function testProperty_sign_CreatesVerifiableSignatures(
         PrivateKey privKey,
         bytes memory message
     ) public {
@@ -32,7 +32,7 @@ contract ECDSAPropertiesTest is Test {
         assertTrue(pubKey.verify(message, privKey.sign(message)));
     }
 
-    function testProperty_CreatedSignaturesAreDeterministic(
+    function testProperty_sign_CreatesDeterministicSignatures(
         PrivateKey privKey,
         bytes memory message
     ) public {
@@ -46,7 +46,7 @@ contract ECDSAPropertiesTest is Test {
         assertEq(sig1.s, sig2.s);
     }
 
-    function testProperty_CreatedSignaturesAreNonMalleable(
+    function testProperty_sign_CreatesNonMalleableSignatures(
         PrivateKey privKey,
         bytes memory message
     ) public {
@@ -57,4 +57,29 @@ contract ECDSAPropertiesTest is Test {
 
     //--------------------------------------------------------------------------
     // Properties: (De)Serialization
+
+    function testProperty_Bytes_SerializationLoop(Signature memory sig)
+        public
+    {
+        Signature memory got = ECDSA.signatureFromBytes(sig.toBytes());
+
+        assertEq(got.v, sig.v);
+        assertEq(got.r, sig.r);
+        assertEq(got.s, sig.s);
+    }
+
+    function testProperty_CompactBytes_SerializationLoop(
+        PrivateKey privKey,
+        bytes memory message
+    ) public {
+        vm.assume(privKey.isValid());
+
+        Signature memory want = privKey.sign(message);
+        Signature memory got =
+            ECDSA.signatureFromCompactBytes(want.toCompactBytes());
+
+        assertEq(got.v, want.v);
+        assertEq(got.r, want.r);
+        assertEq(got.s, want.s);
+    }
 }
diff --git a/test/signatures/SchnorrProperties.t.sol b/test/signatures/SchnorrProperties.t.sol
@@ -21,7 +21,7 @@ contract SchnorrPropertiesTest is Test {
     //--------------------------------------------------------------------------
     // Properties: Signature
 
-    function testProperty_CreatedSignaturesAreVerifiable(
+    function testProperty_sign_CreatesVerifiableSignatures(
         PrivateKey privKey,
         bytes memory message
     ) public {
@@ -32,7 +32,7 @@ contract SchnorrPropertiesTest is Test {
         assertTrue(pubKey.verify(message, privKey.sign(message)));
     }
 
-    function testProperty_CreatedSignaturesAreDeterministic(
+    function testProperty_sign_CreatesDeterministicSignatures(
         PrivateKey privKey,
         bytes memory message
     ) public {
@@ -45,7 +45,7 @@ contract SchnorrPropertiesTest is Test {
         assertEq(sig1.commitment, sig2.commitment);
     }
 
-    function testProperty_CreatedSignaturesAreNonMalleable(
+    function testProperty_sign_CreatesNonMalleableSignatures(
         PrivateKey privKey,
         bytes memory message
     ) public {