Skip to content

Commit

Permalink
💥 Revised all verify function signatures
Browse files Browse the repository at this point in the history
- Transitioned rs parameter from memory layout to stack layout across all implementations
- Shifted Q parameter from memory layout to stack layout in basic implementation

This pivotal change, though breaking, was driven by the necessity to reduce
interaction costs with the verify function, the library's sole user-facing
feature.
  • Loading branch information
qd-qd committed Jul 5, 2023
1 parent a79e9d7 commit f2e1f61
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 76 deletions.
13 changes: 7 additions & 6 deletions src/ECDSA256PrecomputeInternal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 11 additions & 9 deletions src/ECDSA256r1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 7 additions & 6 deletions src/ECDSA256r1Precompute.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
54 changes: 28 additions & 26 deletions test/ecdsa256r1.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
}

Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
}
Loading

0 comments on commit f2e1f61

Please sign in to comment.