diff --git a/src/ECDSA256PrecomputeInternal.sol b/src/ECDSA256PrecomputeInternal.sol index 856d903..4423696 100644 --- a/src/ECDSA256PrecomputeInternal.sol +++ b/src/ECDSA256PrecomputeInternal.sol @@ -143,22 +143,23 @@ library ECDSA256r1PrecomputeInternal { /// @notice Verifies an ECDSA signature using a precomputed table of multiples of P and Q stored in the bytecode of /// the contrat that uses this library /// @param message The message that was signed. - /// @param rs uint256[2] The r and s values of the ECDSA signature. + /// @param r uint256 The r value of the ECDSA signature. + /// @param s uint256 The s value of the ECDSA signature. /// @param precomputedOffset The **offset** where the precomputed points starts in the bytecode /// @return True if the signature is valid, false otherwise. /// @dev Note the required interactions with the precompled contract can revert the transaction - function verify(bytes32 message, uint256[2] calldata rs, uint256 precomputedOffset) external returns (bool) { + function verify(bytes32 message, uint256 r, uint256 s, uint256 precomputedOffset) external returns (bool) { // check the validity of the signature - if (rs[0] == 0 || rs[0] >= n || rs[1] == 0) { + if (r == 0 || r >= n || s == 0) { return false; } // preform the Shamir's trick in order to calculate the x coordinate of the point - uint256 sInv = rs[1].nModInv(); - uint256 X = mulmuladd(mulmod(uint256(message), sInv, n), mulmod(rs[0], sInv, n), precomputedOffset); + uint256 sInv = s.nModInv(); + uint256 X = mulmuladd(mulmod(uint256(message), sInv, n), mulmod(r, sInv, n), precomputedOffset); assembly { - X := addmod(X, sub(n, calldataload(rs)), n) + X := addmod(X, sub(n, r), n) } return X == 0; diff --git a/src/ECDSA256r1.sol b/src/ECDSA256r1.sol index 8d61a17..447c8d8 100644 --- a/src/ECDSA256r1.sol +++ b/src/ECDSA256r1.sol @@ -192,30 +192,32 @@ library ECDSA256r1 { /// @notice Verifies an ECDSA signature on the secp256r1 curve given the message, signature, and public key. /// This function is the only one exposed by the library /// @param message The original message that was signed - /// @param rs uint256[2] The r and s values of the ECDSA signature. - /// @param Q The public key used for the signature, in the format [Qx, Qy] + /// @param r uint256 The r value of the ECDSA signature. + /// @param s uint256 The s value of the ECDSA signature. + /// @param qx The x value of the public key used for the signature + /// @param qy The y value of the public key used for the signature /// @return bool True if the signature is valid, false otherwise /// @dev Note the required interactions with the precompled contract can revert the transaction - function verify(bytes32 message, uint256[2] calldata rs, uint256[2] calldata Q) external returns (bool) { + function verify(bytes32 message, uint256 r, uint256 s, uint256 qx, uint256 qy) external returns (bool) { // check the validity of the signature - if (rs[0] == 0 || rs[0] >= n || rs[1] == 0 || rs[1] >= n) { + if (r == 0 || r >= n || s == 0 || s >= n) { return false; } // check the public key is on the curve - if (!ECDSA.affIsOnCurve(Q[0], Q[1])) { + if (!ECDSA.affIsOnCurve(qx, qy)) { return false; } // calculate the scalars used for the multiplication of the point - uint256 sInv = rs[1].nModInv(); + uint256 sInv = s.nModInv(); uint256 scalar_u = mulmod(uint256(message), sInv, n); - uint256 scalar_v = mulmod(rs[0], sInv, n); + uint256 scalar_v = mulmod(r, sInv, n); - uint256 x1 = mulmuladd(Q[0], Q[1], scalar_u, scalar_v); + uint256 x1 = mulmuladd(qx, qy, scalar_u, scalar_v); assembly { - x1 := addmod(x1, sub(n, calldataload(rs)), n) + x1 := addmod(x1, sub(n, r), n) } return x1 == 0; diff --git a/src/ECDSA256r1Precompute.sol b/src/ECDSA256r1Precompute.sol index 414d12c..11e8624 100644 --- a/src/ECDSA256r1Precompute.sol +++ b/src/ECDSA256r1Precompute.sol @@ -181,24 +181,25 @@ library ECDSA256r1Precompute { /// @notice Verifies an ECDSA signature using a precomputed table of multiples of P and Q stored in an external /// contract /// @param message The message to verify - /// @param rs tuple [r, s] representing the ECDSA signature + /// @param r uint256 The r value of the ECDSA signature. + /// @param s uint256 The s value of the ECDSA signature. /// @param precomputedTable The address of the external contract containing the precomputations for Shamir's trick. /// It is expected the contract store the precomputations as its bytecode. The contract is not supposed /// to be functional. /// @return True if the signature is valid, false otherwise. /// @dev Note the required interactions with the precompled contract can revert the transaction - function verify(bytes32 message, uint256[2] calldata rs, address precomputedTable) external returns (bool) { + function verify(bytes32 message, uint256 r, uint256 s, address precomputedTable) external returns (bool) { // check the validity of the signature - if (rs[0] == 0 || rs[0] >= n || rs[1] == 0 || rs[1] >= n) { + if (r == 0 || r >= n || s == 0 || s >= n) { return false; } // perform the Shamir's trick in order to calculate the x coordinate of the point - uint256 sInv = rs[1].nModInv(); - uint256 X = mulmuladd(mulmod(uint256(message), sInv, n), mulmod(rs[0], sInv, n), precomputedTable); + uint256 sInv = s.nModInv(); + uint256 X = mulmuladd(mulmod(uint256(message), sInv, n), mulmod(r, sInv, n), precomputedTable); assembly { - X := addmod(X, sub(n, calldataload(rs)), n) + X := addmod(X, sub(n, r), n) } return X == 0; diff --git a/test/ecdsa256r1.t.sol b/test/ecdsa256r1.t.sol index 380bc12..60e8201 100644 --- a/test/ecdsa256r1.t.sol +++ b/test/ecdsa256r1.t.sol @@ -6,14 +6,18 @@ import { ECDSA, p, gx, gy, n } from "../src/utils/ECDSA.sol"; import { ECDSA256r1 } from "../src/ECDSA256r1.sol"; struct TestVectors { - uint256[2] pubKey; + // public key + uint256 qx; + uint256 qy; + // number of tests uint256 numtests; + // JSON string containing the test vectors string fixtures; } contract ImplementationECDSA256r1 { - function verify(bytes32 message, uint256[2] memory rs, uint256[2] memory pubKey) external returns (bool) { - return ECDSA256r1.verify(message, rs, pubKey); + function verify(bytes32 message, uint256 r, uint256 s, uint256 qx, uint256 qy) external returns (bool) { + return ECDSA256r1.verify(message, r, s, qx, qy); } function mulmuladd(uint256 q0, uint256 q1, uint256 n0, uint256 n1) external returns (uint256) { @@ -50,31 +54,29 @@ contract Ecdsa256r1Test is PRBTest { /// @return testvectors The test vectors function _loadFixtures(bool flag) internal returns (TestVectors memory testvectors) { // all the content of the JSON file - string memory fixtures = + testvectors.fixtures = flag == true ? vm.readFile(VALID_VECTOR_FILE_PATH) : vm.readFile(INVALID_VECTOR_FILE_PATH); - uint256[2] memory pubKey; - pubKey[0] = vm.parseJsonUint(fixtures, ".keyx"); - pubKey[1] = vm.parseJsonUint(fixtures, ".keyy"); - uint256 numtests = vm.parseJsonUint(fixtures, ".NumberOfTests"); - - return TestVectors(pubKey, numtests, fixtures); + testvectors.qx = vm.parseJsonUint(testvectors.fixtures, ".keyx"); + testvectors.qy = vm.parseJsonUint(testvectors.fixtures, ".keyy"); + testvectors.numtests = vm.parseJsonUint(testvectors.fixtures, ".NumberOfTests"); } /// @notice get the test vector n from the JSON string /// @param fixtures The JSON string containing the test vectors /// @param id The test vector number to get - /// @return rs The signature (r, s) of the test vector + /// @return r uint256 The r value of the ECDSA signature. + /// @return s uint256 The s value of the ECDSA signature. /// @return message The message of the test vector function _getTestVector( string memory fixtures, string memory id ) internal - returns (uint256[2] memory rs, bytes32 message) + returns (uint256 r, uint256 s, bytes32 message) { - rs[0] = vm.parseJsonUint(fixtures, string.concat(".sigx_", id)); - rs[1] = vm.parseJsonUint(fixtures, string.concat(".sigy_", id)); + r = vm.parseJsonUint(fixtures, string.concat(".sigx_", id)); + s = vm.parseJsonUint(fixtures, string.concat(".sigy_", id)); message = vm.parseJsonBytes32(fixtures, string.concat(".msg_", id)); } @@ -86,9 +88,9 @@ contract Ecdsa256r1Test is PRBTest { for (uint256 i = 1; i <= testVectors.numtests; i++) { // get the test vector (message, signature) - (uint256[2] memory rs, bytes32 message) = _getTestVector(testVectors.fixtures, vm.toString(i)); + (uint256 r, uint256 s, bytes32 message) = _getTestVector(testVectors.fixtures, vm.toString(i)); // run the verification function with the test vector - bool isStandardValid = implementation.verify(message, rs, testVectors.pubKey); + bool isStandardValid = implementation.verify(message, r, s, testVectors.qx, testVectors.qy); // ensure the result is the expected one assertEq(isStandardValid, flag); } @@ -136,43 +138,43 @@ contract Ecdsa256r1Test is PRBTest { // test invalid vectors, all assert shall be false function test_VerifySignatureValidity() public { // expect to fail because rs[0] == 0 - bool isValid = implementation.verify(bytes32("hello"), [uint256(0), uint256(1)], [uint256(1), uint256(1)]); + bool isValid = implementation.verify(bytes32("hello"), uint256(0), uint256(1), uint256(1), uint256(1)); assertFalse(isValid); // expect to fail because rs[1] == 0 - isValid = implementation.verify(bytes32("hello"), [uint256(1), uint256(0)], [uint256(1), uint256(1)]); + isValid = implementation.verify(bytes32("hello"), uint256(1), uint256(0), uint256(1), uint256(1)); assertFalse(isValid); // expect to fail because rs[0] > n - isValid = implementation.verify(bytes32("hello"), [n + 1, uint256(1)], [uint256(1), uint256(1)]); + isValid = implementation.verify(bytes32("hello"), n + 1, uint256(1), uint256(1), uint256(1)); assertFalse(isValid); // expect to fail because rs[0] == n - isValid = implementation.verify(bytes32("hello"), [n, uint256(1)], [uint256(1), uint256(1)]); + isValid = implementation.verify(bytes32("hello"), n, uint256(1), uint256(1), uint256(1)); assertFalse(isValid); // expect to fail because rs[1] > n - isValid = implementation.verify(bytes32("hello"), [uint256(1), n + 1], [uint256(1), uint256(1)]); + isValid = implementation.verify(bytes32("hello"), uint256(1), n + 1, uint256(1), uint256(1)); assertFalse(isValid); // expect to fail because rs[1] == n - isValid = implementation.verify(bytes32("hello"), [uint256(1), n], [uint256(1), uint256(1)]); + isValid = implementation.verify(bytes32("hello"), uint256(1), n, uint256(1), uint256(1)); assertFalse(isValid); // expect to fail because q[0] == 0 (affine coordinates not on the curve) - isValid = implementation.verify(bytes32("hello"), [uint256(1), uint256(1)], [0, uint256(1)]); + isValid = implementation.verify(bytes32("hello"), uint256(1), uint256(1), 0, uint256(1)); assertFalse(isValid); // expect to fail because q[1] == 0 (affine coordinates not on the curve) - isValid = implementation.verify(bytes32("hello"), [uint256(1), uint256(1)], [uint256(1), 0]); + isValid = implementation.verify(bytes32("hello"), uint256(1), uint256(1), uint256(1), 0); assertFalse(isValid); // expect to fail because q[0] == p (affine coordinates not on the curve) - isValid = implementation.verify(bytes32("hello"), [uint256(1), uint256(1)], [p, uint256(1)]); + isValid = implementation.verify(bytes32("hello"), uint256(1), uint256(1), p, uint256(1)); assertFalse(isValid); // expect to fail because q[1] == p (affine coordinates not on the curve) - isValid = implementation.verify(bytes32("hello"), [uint256(1), uint256(1)], [uint256(1), p]); + isValid = implementation.verify(bytes32("hello"), uint256(1), uint256(1), uint256(1), p); assertFalse(isValid); } } diff --git a/test/ecdsa256r1Precomput.t.sol b/test/ecdsa256r1Precomput.t.sol index 45dcfa4..2d9ec58 100644 --- a/test/ecdsa256r1Precomput.t.sol +++ b/test/ecdsa256r1Precomput.t.sol @@ -7,14 +7,18 @@ import { ECDSA, Curve, n } from "../src/utils/ECDSA.sol"; import { ECDSA256r1Precompute } from "../src/ECDSA256r1Precompute.sol"; struct TestVectors { - uint256[2] pubKey; + // public key + uint256 qx; + uint256 qy; + // number of tests uint256 numtests; + // JSON string containing the test vectors string fixtures; } contract ImplementationECDSA256r1Precompute { - function verify(bytes32 message, uint256[2] calldata rs, address precomputedTable) external returns (bool) { - return ECDSA256r1Precompute.verify(message, rs, precomputedTable); + function verify(bytes32 message, uint256 r, uint256 s, address precomputedTable) external returns (bool) { + return ECDSA256r1Precompute.verify(message, r, s, precomputedTable); } function mulmuladd(uint256 scalar_u, uint256 scalar_v, address precomputedTable) external returns (uint256) { @@ -64,31 +68,29 @@ contract Ecdsa256r1PrecomputTest is StdUtils, PRBTest { /// @return testvectors The test vectors function _loadFixtures(bool flag) internal returns (TestVectors memory testvectors) { // all the content of the JSON file - string memory fixtures = + testvectors.fixtures = flag == true ? vm.readFile(VALID_VECTOR_FILE_PATH) : vm.readFile(INVALID_VECTOR_FILE_PATH); - uint256[2] memory pubKey; - pubKey[0] = vm.parseJsonUint(fixtures, ".keyx"); - pubKey[1] = vm.parseJsonUint(fixtures, ".keyy"); - uint256 numtests = vm.parseJsonUint(fixtures, ".NumberOfTests"); - - return TestVectors(pubKey, numtests, fixtures); + testvectors.qx = vm.parseJsonUint(testvectors.fixtures, ".keyx"); + testvectors.qy = vm.parseJsonUint(testvectors.fixtures, ".keyy"); + testvectors.numtests = vm.parseJsonUint(testvectors.fixtures, ".NumberOfTests"); } /// @notice get the test vector n from the JSON string /// @param fixtures The JSON string containing the test vectors /// @param id The test vector number to get - /// @return rs The signature (r, s) of the test vector + /// @return r uint256 The r value of the ECDSA signature. + /// @return s uint256 The s value of the ECDSA signature. /// @return message The message of the test vector function _getTestVector( string memory fixtures, string memory id ) internal - returns (uint256[2] memory rs, bytes32 message) + returns (uint256 r, uint256 s, bytes32 message) { - rs[0] = vm.parseJsonUint(fixtures, string.concat(".sigx_", id)); - rs[1] = vm.parseJsonUint(fixtures, string.concat(".sigy_", id)); + r = vm.parseJsonUint(fixtures, string.concat(".sigx_", id)); + s = vm.parseJsonUint(fixtures, string.concat(".sigy_", id)); message = vm.parseJsonBytes32(fixtures, string.concat(".msg_", id)); } @@ -112,7 +114,7 @@ contract Ecdsa256r1PrecomputTest is StdUtils, PRBTest { /// @dev Uses `_precomputeShamirTable` function to generate the precomputed table modifier _preparePrecomputeTable() { // generate the precomputed table - bytes memory precompute = _precomputeShamirTable(validVectors.pubKey[0], validVectors.pubKey[1]); + bytes memory precompute = _precomputeShamirTable(validVectors.qx, validVectors.qy); // set the precomputed points as the bytecode of the target contract vm.etch(precomputeAddress, precompute); @@ -155,9 +157,9 @@ contract Ecdsa256r1PrecomputTest is StdUtils, PRBTest { for (uint256 i = 1; i <= testVectors.numtests; i++) { // get the test vector (message, signature) - (uint256[2] memory rs, bytes32 message) = _getTestVector(testVectors.fixtures, vm.toString(i)); + (uint256 r, uint256 s, bytes32 message) = _getTestVector(testVectors.fixtures, vm.toString(i)); // run the verification function with the test vector - bool isValid = implementation.verify(message, rs, precomputeAddress); + bool isValid = implementation.verify(message, r, s, precomputeAddress); // ensure the result is the expected one assertTrue(isValid); } @@ -171,9 +173,9 @@ contract Ecdsa256r1PrecomputTest is StdUtils, PRBTest { for (uint256 i = 1; i <= testVectors.numtests; i++) { // get the test vector (message, signature) - (uint256[2] memory rs, bytes32 message) = _getTestVector(testVectors.fixtures, vm.toString(i)); + (uint256 r, uint256 s, bytes32 message) = _getTestVector(testVectors.fixtures, vm.toString(i)); // run the verification function with the test vector - bool isValid = implementation.verify(message, rs, precomputeAddress); + bool isValid = implementation.verify(message, r, s, precomputeAddress); // ensure the result is the expected one assertFalse(isValid); } @@ -183,27 +185,27 @@ contract Ecdsa256r1PrecomputTest is StdUtils, PRBTest { /// @dev Ensures that incorrect signatures are not valid function test_VerifyIncorrectSignatureFail() external _preparePrecomputeTable { // expect to fail because rs[0] == 0 - bool isValid = implementation.verify(bytes32("hello"), [uint256(0), uint256(1)], precomputeAddress); + bool isValid = implementation.verify(bytes32("hello"), uint256(0), uint256(1), precomputeAddress); assertFalse(isValid); // expect to fail because rs[1] == 0 - isValid = implementation.verify(bytes32("hello"), [uint256(1), uint256(0)], precomputeAddress); + isValid = implementation.verify(bytes32("hello"), uint256(1), uint256(0), precomputeAddress); assertFalse(isValid); // expect to fail because rs[0] > n - isValid = implementation.verify(bytes32("hello"), [n + 1, uint256(1)], precomputeAddress); + isValid = implementation.verify(bytes32("hello"), n + 1, uint256(1), precomputeAddress); assertFalse(isValid); // expect to fail because rs[0] == n - isValid = implementation.verify(bytes32("hello"), [n, uint256(1)], precomputeAddress); + isValid = implementation.verify(bytes32("hello"), n, uint256(1), precomputeAddress); assertFalse(isValid); // expect to fail because rs[1] > n - isValid = implementation.verify(bytes32("hello"), [uint256(1), n + 1], precomputeAddress); + isValid = implementation.verify(bytes32("hello"), uint256(1), n + 1, precomputeAddress); assertFalse(isValid); // expect to fail because rs[1] == n - isValid = implementation.verify(bytes32("hello"), [uint256(1), n], precomputeAddress); + isValid = implementation.verify(bytes32("hello"), uint256(1), n, precomputeAddress); assertFalse(isValid); } @@ -211,11 +213,11 @@ contract Ecdsa256r1PrecomputTest is StdUtils, PRBTest { /// @dev Ensures that incorrect addresses fail verification (0x00 + precompile addresses + some extra empty ones) function test_VerifyIncorectAddressesFail() external _preparePrecomputeTable { // get a valid test vector (message, signature) - (uint256[2] memory rs, bytes32 message) = _getTestVector(validVectors.fixtures, "1"); + (uint256 r, uint256 s, bytes32 message) = _getTestVector(validVectors.fixtures, "1"); for (uint160 addr = 0; addr <= 20; addr++) { // run the verification function with the correct test vector BUT an invalid address - bool isValid = implementation.verify(message, rs, address(addr)); + bool isValid = implementation.verify(message, r, s, address(addr)); assertFalse(isValid); } } @@ -229,7 +231,7 @@ contract Ecdsa256r1PrecomputTest is StdUtils, PRBTest { TestVectors memory testVectors = invalidVectors; // get the test vector (message, signature) - (uint256[2] memory rs, bytes32 message) = _getTestVector(testVectors.fixtures, "1"); + (uint256 r,, bytes32 message) = _getTestVector(testVectors.fixtures, "1"); // minimal fixtures uint256 sInv = @@ -239,7 +241,7 @@ contract Ecdsa256r1PrecomputTest is StdUtils, PRBTest { // run the verification function with the test vector and the fixtures uint256 result = - implementation.mulmuladd(mulmod(uint256(message), sInv, n), mulmod(rs[0], sInv, n), precomputeAddress); + implementation.mulmuladd(mulmod(uint256(message), sInv, n), mulmod(r, sInv, n), precomputeAddress); assertEq(result, expected); }