From b1c2112404946e1ca8c6da2be5db9ec513d81071 Mon Sep 17 00:00:00 2001 From: pascal Date: Sun, 26 Nov 2023 14:09:07 +0100 Subject: [PATCH] ECDSA: Finished property tests --- README.md | 4 ++-- src/signatures/ECDSA.sol | 18 +------------- src/signatures/Schnorr.sol | 2 +- test/signatures/ECDSA.t.sol | 4 +++- test/signatures/ECDSAProperties.t.sol | 31 ++++++++++++++++++++++--- test/signatures/SchnorrProperties.t.sol | 6 ++--- 6 files changed, 38 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 30c829f..b616528 100644 --- 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 index 43a387f..076be0e 100644 --- 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 index a1145b6..c39674a 100644 --- 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 index df2593f..6cfdc98 100644 --- 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 index 96718ac..6f2be62 100644 --- 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 index 9fc1b6f..5da6c46 100644 --- 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 {