Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scep256k1: Updates identity integration #21

Merged
merged 2 commits into from
Jun 27, 2024
Merged

scep256k1: Updates identity integration #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
019815f
scep256k1: Updates identity integration
pmerkleplant Jun 26, 2024
66b5bcb
fix: Failing isIdentity fuzz test
pmerkleplant Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions src/onchain/secp256k1/Secp256k1.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,11 @@ library Secp256k1 {

/// @dev Returns whether public key `pk` is a valid secp256k1 public key.
///
/// @dev Note that a public key is valid if its either on the curve or the
/// identity (aka point at infinity) point.
/// @dev Note that the identity point is not a valid public key.
function isValid(PublicKey memory pk) internal pure returns (bool) {
// TODO: Should identity be a valid public key?
// Point memory p = pk.intoPoint();
// return p.isOnCurve() && !p.isIdentity();
return pk.intoPoint().isOnCurve();
Point memory p = pk.intoPoint();

return p.isOnCurve() && !p.isIdentity();
}

/// @dev Returns the y parity of public key `pk`.
Expand Down
44 changes: 12 additions & 32 deletions src/onchain/secp256k1/Secp256k1Arithmetic.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pragma solidity ^0.8.16;
* @notice Point is a secp256k1 point in affine coordinates
*
* @dev The identity, aka point at infinity, is represented via:
* x = y = type(uint).max
* x = y = 0
*/
struct Point {
uint x;
Expand Down Expand Up @@ -75,7 +75,7 @@ library Secp256k1Arithmetic {

/// @dev Used as substitute for `Identity().intoPublicKey().toAddress()`.
address private constant IDENTITY_ADDRESS =
0x2dCC482901728b6df477f4fB2F192733A005d396;
0x3f17f1962B36e491b30A40b2405849e597Ba5FB5;

//--------------------------------------------------------------------------
// Secp256k1 Constants
Expand Down Expand Up @@ -111,28 +111,18 @@ library Secp256k1Arithmetic {
//--------------------------------------------------------------------------
// Point

/// @dev Returns the zero point.
///
/// @dev Note that the zero point is invalid and this function only provided
/// for convenience.
function ZeroPoint() internal pure returns (Point memory) {
return Point(0, 0);
}

/// @dev Returns whether point `point` is the zero point.
function isZeroPoint(Point memory point) internal pure returns (bool) {
return (point.x | point.y) == 0;
}

// TODO: Use (0, 0) as infinity? Follows general convention...
/// @dev Returns the additive identity.
///
/// @dev Note that the identity is represented via:
/// point.x = point.y = type(uint).max
/// point.x = point.y = 0
///
/// @dev Note that the identity is a valid point on the curve to enable
/// arithmetic functionality. However, the identity is not a valid
/// PublicKey and MUST NOT be used as cryptographic object.
///
/// @dev Note that the identity is also called point at infinity.
function Identity() internal pure returns (Point memory) {
return Point(type(uint).max, type(uint).max);
return Point(0, 0);
}

/// @dev Returns whether point `point` is the identity.
Expand All @@ -142,7 +132,7 @@ library Secp256k1Arithmetic {
///
/// @dev Note that the identity is also called point at infinity.
function isIdentity(Point memory point) internal pure returns (bool) {
return (point.x & point.y) == type(uint).max;
return (point.x | point.y) == 0;
}

/// @dev Returns whether point `point` is on the curve.
Expand All @@ -152,18 +142,8 @@ library Secp256k1Arithmetic {
/// a = 0
/// b = 7
///
/// @dev Note that the identity is also on the curve.
/// @dev Note that the identity is on the curve.
function isOnCurve(Point memory point) internal pure returns (bool) {
// TODO: Issue with whether identity is on curve:
//
// isOnCurve should return true to enable arithmetic functionality.
// However, publicKey.isValid should return false! Identity is only
// valid inside Arithmetic package. For crypto package it is invalid.
//
// See also go's issue: https://github.com/golang/go/issues/37294.
//
// Important: It MUST NOT be possible to de-/serialize identity to
// normal public key.
if (point.isIdentity()) {
return true;
}
Expand Down Expand Up @@ -396,8 +376,8 @@ library Secp256k1Arithmetic {
assembly ("memory-safe") {
p := point
}
p.x = type(uint).max;
p.y = type(uint).max;
p.x = 0;
p.y = 0;
return p;
}

Expand Down
10 changes: 2 additions & 8 deletions test/onchain/secp256k1/Secp256k1.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,21 +165,15 @@ contract Secp256k1Test is Test {
assertTrue(wrapper.isValid(sk.toPublicKey()));
}

function test_PublicKey_isValid_If_Identity() public {
function test_PublicKey_isValid_FailsIf_Identity() public {
PublicKey memory pk = Secp256k1Arithmetic.Identity().intoPublicKey();

assertTrue(pk.isValid());
assertFalse(wrapper.isValid(pk));
}

function test_PublicKey_isValid_FailsIf_PointNotOnCurve() public {
PublicKey memory pk;

// Zero point not on curve.
pk.x = 0;
pk.y = 0;
assertFalse(wrapper.isValid(pk));

// Some other point.
pk.x = 1;
pk.x = 3;
assertFalse(wrapper.isValid(pk));
Expand Down
26 changes: 1 addition & 25 deletions test/onchain/secp256k1/Secp256k1Arithmetic.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,22 +61,6 @@ contract Secp256k1ArithmeticTest is Test {
//--------------------------------------------------------------------------
// Test: Point

// -- ZeroPoint

function test_ZeroPoint() public {
assertTrue(wrapper.ZeroPoint().isZeroPoint());
}

// -- isZeroPoint

function testFuzz_Point_isZeroPoint(Point memory point) public {
if (point.x == 0 && point.y == 0) {
assertTrue(wrapper.isZeroPoint(point));
} else {
assertFalse(wrapper.isZeroPoint(point));
}
}

// -- Identity

function test_Identity() public {
Expand All @@ -86,7 +70,7 @@ contract Secp256k1ArithmeticTest is Test {
// -- isIdentity

function testFuzz_Point_isIdentity(Point memory point) public {
if (point.x == type(uint).max && point.y == type(uint).max) {
if (point.x == 0 && point.y == 0) {
assertTrue(wrapper.isIdentity(point));
} else {
assertFalse(wrapper.isIdentity(point));
Expand Down Expand Up @@ -569,14 +553,6 @@ contract Secp256k1ArithmeticWrapper {
//--------------------------------------------------------------------------
// Point

function ZeroPoint() public pure returns (Point memory) {
return Secp256k1Arithmetic.ZeroPoint();
}

function isZeroPoint(Point memory point) public pure returns (bool) {
return point.isZeroPoint();
}

function Identity() public pure returns (Point memory) {
return Secp256k1Arithmetic.Identity();
}
Expand Down
19 changes: 1 addition & 18 deletions test/onchain/secp256k1/Secp256k1ArithmeticProperties.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ contract Secp256k1ArithmeticPropertiesTest is Test {

Point memory sum = p1.add(p2).intoPoint();

assertFalse(sum.isZeroPoint());
assertFalse(sum.x == 0 && sum.y == 0);
}

function testProperty_ProjectivePoint_add_ResultIsOnCurve(
Expand All @@ -64,29 +64,12 @@ contract Secp256k1ArithmeticPropertiesTest is Test {

// -- mul

function testProperty_ProjectivePoint_mul_NeverReturnsZeroPoint(
SecretKey sk,
uint scalar
) public {
vm.assume(sk.isValid());

// TODO: Make Felt type to circumvent manual bounding?
vm.assume(scalar < Secp256k1Arithmetic.Q);

ProjectivePoint memory p = sk.toPublicKey().toProjectivePoint();

Point memory product = p.mul(scalar).intoPoint();

assertFalse(product.isZeroPoint());
}

function testProperty_ProjectivePoint_mul_ResultIsOnCurve(
SecretKey sk,
uint scalar
) public {
vm.assume(sk.isValid());

// TODO: Make Felt type to circumvent manual bounding?
vm.assume(scalar < Secp256k1Arithmetic.Q);

ProjectivePoint memory p = sk.toPublicKey().toProjectivePoint();
Expand Down
Loading