fix: check if points are on the curve before addition

heueristik · heueristik · commit 95c413113c31 · 2025-11-03T18:27:06.000+01:00
diff --git a/contracts/src/libs/proving/Delta.sol b/contracts/src/libs/proving/Delta.sol
@@ -20,15 +20,27 @@ library Delta {
         uint256 y;
     }
 
-    /// @notice The constant of the secp256k1 (K-256) elliptic curve.
+    /// @notice The x-coordinate of the curve generator point.
+    uint256 internal constant _GX = 0x79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798;
+
+    /// @notice The y-coordinate of the curve generator point.
+    uint256 internal constant _GY = 0x483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8;
+
+    // @notice The coefficient a of th secp256k1 (K-256) elliptic curve (y² = x³ + ax + b).
     uint256 internal constant _AA = 0;
 
-    /// @notice The modulus of the secp256k1 (K-256) elliptic curve.
+    // @notice The coefficient b of th secp256k1 (K-256) elliptic curve (y² = x³ + ax + b).
+    uint256 internal constant _BB = 7;
+
+    /// @notice The field prime modulus (2^256 - 2^32 - 977) of the secp256k1 (K-256) elliptic curve (y² = x³ + ax + b).
     uint256 internal constant _PP = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F;
 
     /// @notice Thrown if the recovered delta public key doesn't match the delta instance.
     error DeltaMismatch(address expected, address actual);
 
+    /// @notice Thrown when a provided point is not on the curve.
+    error PointNotOnCurve(CurvePoint point);
+
     /// @notice Returns the elliptic curve point representing the zero delta.
     /// @return zeroDelta The zero delta.
     function zero() internal pure returns (CurvePoint memory zeroDelta) {
@@ -40,9 +52,23 @@ library Delta {
     /// @param p2 The second curve point.
     /// @return sum The resulting curve point.
     function add(CurvePoint memory p1, CurvePoint memory p2) internal pure returns (CurvePoint memory sum) {
+        if (!isZeroDelta(p1) && !EllipticCurve.isOnCurve({_x: p1.x, _y: p1.y, _aa: _AA, _bb: _BB, _pp: _PP})) {
+            revert PointNotOnCurve(p1);
+        }
+        if (!isZeroDelta(p2) && !EllipticCurve.isOnCurve({_x: p2.x, _y: p2.y, _aa: _AA, _bb: _BB, _pp: _PP})) {
+            revert PointNotOnCurve(p2);
+        }
+
         (sum.x, sum.y) = EllipticCurve.ecAdd({_x1: p1.x, _y1: p1.y, _x2: p2.x, _y2: p2.y, _aa: _AA, _pp: _PP});
     }
 
+    /// @notice Returns whether a point is the zero delta or not.
+    /// @param p The point to check.
+    /// @return isZero Whether the point is the zero delta or not.
+    function isZeroDelta(CurvePoint memory p) internal pure returns (bool isZero) {
+        isZero = p.x == 0 && p.y == 0;
+    }
+
     /// @notice Converts an elliptic curve point to an Ethereum account address.
     /// @param delta The elliptic curve point.
     /// @return account The associated account.
diff --git a/contracts/test/libs/EllipticCurve.t.sol b/contracts/test/libs/EllipticCurve.t.sol
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.30;
+
+import {EllipticCurve} from "@elliptic-curve-solidity/contracts/EllipticCurve.sol";
+import {Test} from "forge-std/Test.sol";
+
+import {Delta} from "../../src/libs/proving/Delta.sol";
+
+/**
+ * @title EllipticCurvePropertiesTest
+ * @dev Property-based tests for EllipticCurve.ecAdd using Foundry fuzzing.
+ *
+ * These tests verify GROUP PROPERTIES with points VERIFIED to be on the curve.
+ * Points are generated via scalar multiplication: P = k × G
+ * This guarantees all test points satisfy the curve equation y² = x³ + ax + b
+ *
+ * Run with: forge test -vv
+ * Run with more fuzz runs: forge test --fuzz-runs 10000 -vv
+ */
+contract EllipticCurvePropertiesTest is Test {
+    using EllipticCurve for *;
+
+    /* GROUP PROPERTY TESTS (Points Verified On Curve) */
+
+    /// @notice GROUP PROPERTY: Commutativity - P + Q = Q + P
+    /// @dev Uses secp256k1 with points VERIFIED to be on the curve
+    function testGroupProperty_Commutativity_Secp256k1(uint256 scalar1, uint256 scalar2) public pure {
+        // Generate two points on secp256k1 by scalar multiplication of G
+        scalar1 = bound(scalar1, 1, 20); // Keep small for performance
+        scalar2 = bound(scalar2, 1, 20);
+
+        // P1 = scalar1 * G
+        (uint256 x1, uint256 y1) =
+            EllipticCurve.ecMul({_k: scalar1, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+
+        // P2 = scalar2 * G
+        (uint256 x2, uint256 y2) =
+            EllipticCurve.ecMul({_k: scalar2, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+
+        // Verify both points are on curve
+        assertTrue(
+            EllipticCurve.isOnCurve({_x: x1, _y: y1, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}),
+            "P1 must be on curve"
+        );
+        assertTrue(
+            EllipticCurve.isOnCurve({_x: x2, _y: y2, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}),
+            "P2 must be on curve"
+        );
+
+        // Test commutativity: P1 + P2 = P2 + P1
+        (uint256 resultX1, uint256 resultY1) =
+            EllipticCurve.ecAdd({_x1: x1, _y1: y1, _x2: x2, _y2: y2, _aa: Delta._AA, _pp: Delta._PP});
+        (uint256 resultX2, uint256 resultY2) =
+            EllipticCurve.ecAdd({_x1: x2, _y1: y2, _x2: x1, _y2: y1, _aa: Delta._AA, _pp: Delta._PP});
+
+        assertEq(resultX1, resultX2, "GROUP PROPERTY: P + Q must equal Q + P");
+        assertEq(resultY1, resultY2, "GROUP PROPERTY: P + Q must equal Q + P");
+    }
+
+    /// @notice GROUP PROPERTY: Identity - P + O = P
+    function testGroupProperty_Identity_Secp256k1(uint256 scalar) public pure {
+        scalar = bound(scalar, 1, 20);
+
+        // P = scalar * G (guaranteed on curve)
+        (uint256 x, uint256 y) =
+            EllipticCurve.ecMul({_k: scalar, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+
+        // Verify P is on curve
+        assertTrue(
+            EllipticCurve.isOnCurve({_x: x, _y: y, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}),
+            "P must be on curve"
+        );
+
+        // P + O should equal P
+        (uint256 resultX, uint256 resultY) =
+            EllipticCurve.ecAdd({_x1: x, _y1: y, _x2: 0, _y2: 0, _aa: Delta._AA, _pp: Delta._PP});
+
+        assertEq(resultX, x, "GROUP PROPERTY: P + O must equal P");
+        assertEq(resultY, y, "GROUP PROPERTY: P + O must equal P");
+    }
+
+    /// @notice GROUP PROPERTY: Inverse - P + (-P) = O
+    function testGroupProperty_Inverse_Secp256k1(uint256 scalar) public pure {
+        scalar = bound(scalar, 1, 20);
+
+        // P = scalar * G (guaranteed on curve)
+        (uint256 x, uint256 y) =
+            EllipticCurve.ecMul({_k: scalar, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+
+        // Verify P is on curve
+        assertTrue(
+            EllipticCurve.isOnCurve({_x: x, _y: y, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}),
+            "P must be on curve"
+        );
+
+        // Get -P
+        (uint256 invX, uint256 invY) = EllipticCurve.ecInv({_x: x, _y: y, _pp: Delta._PP});
+
+        // Verify -P is on curve
+        assertTrue(
+            EllipticCurve.isOnCurve({_x: invX, _y: invY, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}),
+            "-P must be on curve"
+        );
+
+        // P + (-P) should equal O
+        (uint256 resultX, uint256 resultY) =
+            EllipticCurve.ecAdd({_x1: x, _y1: y, _x2: invX, _y2: invY, _aa: Delta._AA, _pp: Delta._PP});
+
+        assertTrue(
+            Delta.isZeroDelta(Delta.CurvePoint(resultX, resultY)),
+            "GROUP PROPERTY: P + (-P) must equal point at infinity"
+        );
+    }
+
+    /// @notice GROUP PROPERTY: Associativity - (P + Q) + R = P + (Q + R)
+    function testGroupProperty_Associativity_Secp256k1(uint256 scalar1, uint256 scalar2, uint256 scalar3) public pure {
+        scalar1 = bound(scalar1, 1, 10); // Keep small for performance
+        scalar2 = bound(scalar2, 1, 10);
+        scalar3 = bound(scalar3, 1, 10);
+
+        // Generate three points on curve
+        (uint256 x1, uint256 y1) =
+            EllipticCurve.ecMul({_k: scalar1, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+        (uint256 x2, uint256 y2) =
+            EllipticCurve.ecMul({_k: scalar2, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+        (uint256 x3, uint256 y3) =
+            EllipticCurve.ecMul({_k: scalar3, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+
+        // Verify all points are on curve
+        assertTrue(EllipticCurve.isOnCurve({_x: x1, _y: y1, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}));
+        assertTrue(EllipticCurve.isOnCurve({_x: x2, _y: y2, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}));
+        assertTrue(EllipticCurve.isOnCurve({_x: x3, _y: y3, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}));
+
+        // Compute (P + Q) + R
+        (uint256 pqX, uint256 pqY) =
+            EllipticCurve.ecAdd({_x1: x1, _y1: y1, _x2: x2, _y2: y2, _aa: Delta._AA, _pp: Delta._PP});
+        (uint256 resultX1, uint256 resultY1) =
+            EllipticCurve.ecAdd({_x1: pqX, _y1: pqY, _x2: x3, _y2: y3, _aa: Delta._AA, _pp: Delta._PP});
+
+        // Compute P + (Q + R)
+        (uint256 qrX, uint256 qrY) =
+            EllipticCurve.ecAdd({_x1: x2, _y1: y2, _x2: x3, _y2: y3, _aa: Delta._AA, _pp: Delta._PP});
+
+        (uint256 resultX2, uint256 resultY2) =
+            EllipticCurve.ecAdd({_x1: x1, _y1: y1, _x2: qrX, _y2: qrY, _aa: Delta._AA, _pp: Delta._PP});
+
+        assertEq(resultX1, resultX2, "GROUP PROPERTY: Associativity must hold");
+        assertEq(resultY1, resultY2, "GROUP PROPERTY: Associativity must hold");
+    }
+
+    /// @notice GROUP PROPERTY: Closure - If P, Q on curve, then P+Q on curve or at infinity
+    function testGroupProperty_Closure_Secp256k1(uint256 scalar1, uint256 scalar2) public pure {
+        scalar1 = bound(scalar1, 1, 20);
+        scalar2 = bound(scalar2, 1, 20);
+
+        // Generate two points on curve
+        (uint256 x1, uint256 y1) =
+            EllipticCurve.ecMul({_k: scalar1, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+        (uint256 x2, uint256 y2) =
+            EllipticCurve.ecMul({_k: scalar2, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+
+        // Verify inputs are on curve
+        assertTrue(EllipticCurve.isOnCurve({_x: x1, _y: y1, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}));
+        assertTrue(EllipticCurve.isOnCurve({_x: x2, _y: y2, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}));
+
+        // Add points
+        (uint256 resultX, uint256 resultY) =
+            EllipticCurve.ecAdd({_x1: x1, _y1: y1, _x2: x2, _y2: y2, _aa: Delta._AA, _pp: Delta._PP});
+
+        // Result must be on curve or at infinity
+        bool atInfinity = Delta.isZeroDelta(Delta.CurvePoint(resultX, resultY));
+        bool onCurve =
+            EllipticCurve.isOnCurve({_x: resultX, _y: resultY, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP});
+
+        assertTrue(atInfinity || onCurve, "GROUP PROPERTY: Closure - result must be on curve or at infinity");
+    }
+
+    /* PRECONDITION TESTS (UNREDUCED COORDINATES) */
+
+    /// @notice PRECONDITION TEST: Demonstrates correct behavior with reduced coordinates
+    function test_Precondition_ReducedCoordinates() public pure {
+        // Use a small prime: p = 7, curve parameter a = 2
+        uint256 pp = 7;
+        uint256 aa = 2;
+
+        // Point P: (3, 5), Point Q: (3, 2) where Q is inverse of P since 2 + 5 = 7 ≡ 0 (mod 7)
+        (uint256 rx, uint256 ry) = EllipticCurve.ecAdd({_x1: 3, _y1: 5, _x2: 3, _y2: 2, _aa: aa, _pp: pp});
+
+        // P + (-P) should equal point at infinity
+        assertTrue(
+            Delta.isZeroDelta(Delta.CurvePoint(rx, ry)),
+            "With REDUCED coordinates: (3,5) + (3,2) correctly gives point at infinity"
+        );
+    }
+
+    /* CONCRETE TESTS (SECP256K1) */
+
+    /// @notice Concrete: G + G = 2G
+    function test_Concrete_Doubling_Secp256k1() public pure {
+        (uint256 rx, uint256 ry) = EllipticCurve.ecAdd({
+            _x1: Delta._GX, _y1: Delta._GY, _x2: Delta._GX, _y2: Delta._GY, _aa: Delta._AA, _pp: Delta._PP
+        });
+
+        // Expected 2G
+        assertEq(rx, 0xc6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5);
+        assertEq(ry, 0x1ae168fea63dc339a3c58419466ceaeef7f632653266d0e1236431a950cfe52a);
+
+        // Verify result is on curve
+        assertTrue(EllipticCurve.isOnCurve({_x: rx, _y: ry, _aa: Delta._AA, _bb: Delta._BB, _pp: Delta._PP}));
+    }
+
+    /// @notice Concrete: G + O = G
+    function test_Concrete_Identity_Secp256k1() public pure {
+        (uint256 rx, uint256 ry) =
+            EllipticCurve.ecAdd({_x1: Delta._GX, _y1: Delta._GY, _x2: 0, _y2: 0, _aa: Delta._AA, _pp: Delta._PP});
+
+        assertEq(rx, Delta._GX);
+        assertEq(ry, Delta._GY);
+    }
+
+    /// @notice Concrete: G + (-G) = O
+    function test_Concrete_Inverse_Secp256k1() public pure {
+        (uint256 invX, uint256 invY) = EllipticCurve.ecInv({_x: Delta._GX, _y: Delta._GY, _pp: Delta._PP});
+
+        (uint256 rx, uint256 ry) =
+            EllipticCurve.ecAdd({_x1: Delta._GX, _y1: Delta._GY, _x2: invX, _y2: invY, _aa: Delta._AA, _pp: Delta._PP});
+
+        assertTrue(Delta.isZeroDelta(Delta.CurvePoint(rx, ry)), "G + (-G) should be point at infinity");
+    }
+
+    /// @notice Concrete: (G + 2G) + 3G = G + (2G + 3G) = 6G
+    function test_Concrete_Associativity_Secp256k1() public pure {
+        // Compute 2G, 3G
+
+        (uint256 p2GX, uint256 p2GY) =
+            EllipticCurve.ecMul({_k: 2, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+        (uint256 p3GX, uint256 p3GY) =
+            EllipticCurve.ecMul({_k: 3, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+
+        // Compute (G + 2G) + 3G
+        (uint256 pG2GX, uint256 pG2GY) =
+            EllipticCurve.ecAdd({_x1: Delta._GX, _y1: Delta._GY, _x2: p2GX, _y2: p2GY, _aa: Delta._AA, _pp: Delta._PP});
+        (uint256 result1X, uint256 result1Y) =
+            EllipticCurve.ecAdd({_x1: pG2GX, _y1: pG2GY, _x2: p3GX, _y2: p3GY, _aa: Delta._AA, _pp: Delta._PP});
+
+        // Compute G + (2G + 3G)
+        (uint256 p2G3GX, uint256 p2G3GY) =
+            EllipticCurve.ecAdd({_x1: p2GX, _y1: p2GY, _x2: p3GX, _y2: p3GY, _aa: Delta._AA, _pp: Delta._PP});
+        (uint256 result2X, uint256 result2Y) = EllipticCurve.ecAdd({
+            _x1: Delta._GX, _y1: Delta._GY, _x2: p2G3GX, _y2: p2G3GY, _aa: Delta._AA, _pp: Delta._PP
+        });
+
+        // Both should equal 6G
+        (uint256 g6x, uint256 g6y) =
+            EllipticCurve.ecMul({_k: 6, _x: Delta._GX, _y: Delta._GY, _aa: Delta._AA, _pp: Delta._PP});
+
+        assertEq(result1X, result2X, "Associativity: (G+2G)+3G = G+(2G+3G)");
+
+        assertEq(result1Y, result2Y, "Associativity: (G+2G)+3G = G+(2G+3G)");
+        assertEq(result1X, g6x, "Result should be 6G");
+        assertEq(result1Y, g6y, "Result should be 6G");
+    }
+}
