finalize error types

Makefile

@@ -37,8 +37,8 @@ coverage: ## Update coverage report and print summary
 # Note that ripgrep instead of grep is used.
 # See https://github.com/BurntSushi/ripgrep.
 .PHONY: todos
-todos: ## Grep TODO's in src/ and test/
-	@rg -rn "TODO" src/ test/
+todos: ## Grep TODO's in src/, test/, offchain/ and unsafe/
+	@rg -rn "TODO" src/ test/ offchain/ unsafe/
 
 .PHONY: examples
 examples: ## Run examples

src/Errors.sol

@@ -11,32 +11,36 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 pragma solidity ^0.8.16;
 
-// Scalars
-error CRYSOL_ScalarNotAFelt();
+/// @dev Prime field error thrown when scalar is invalid for attempted
+///      operation.
 error CRYSOL_ScalarInvalid();
-error CRYSOL_ScalarMalleable();
 
-// Math
+/// @dev Prime field error thrown if division by zero is attempted.
 error CRYSOL_DivByZero();
+
+/// @dev Prime field error thrown if computing the inversion of zero is attempted.
 error CRYSOL_InvOfZero();
 
-// (De)Encoding
+/// @dev De/Serialization error thrown if input's length invalid.
 error CRYSOL_LengthInvalid();
+
+/// @dev De/Serialization error thrown if input's prefix invalid.
 error CRYSOL_PrefixInvalid();
 
-// Point
+/// @dev Point error thrown if point invalid.
 error CRYSOL_PointInvalid();
-error CRYSOL_PointNotOnCurve();
 
-// SecretKey
+/// @dev Cryptographic error thrown if secret key invalid.
 error CRYSOL_SecretKeyInvalid();
 
-// PublicKey
+/// @dev Cryptographic error thrown if public key invalid.
 error CRYSOL_PublicKeyInvalid();
 
-// ECDSA
+/// @dev ECDSA signature error thrown if signer is zero address.
 error CRYSOL_SignerZeroAddress();
+
+/// @dev ECDSA signature error thrown if signature is malleable.
 error CRYSOL_SignatureMalleable();
 
-// Schnorr
+/// @dev Schnorr signature error thrown if Schnorr signature insane.
 error CRYSOL_SignatureInsane();

src/arithmetic/Fp.sol

@@ -95,7 +95,7 @@ library Fp {
     function fromUint(uint scalar) internal pure returns (Felt) {
         (Felt felt, bool ok) = tryFromUint(scalar);
         if (!ok) {
-            revert Errors.CRYSOL_ScalarNotAFelt();
+            revert Errors.CRYSOL_ScalarInvalid();
         }
 
         return felt;

src/arithmetic/Points.sol

@@ -257,10 +257,12 @@ library Points {
             && point.y.asUint() == other.y.asUint();
     }
 
-    // TODO: Misses revert documentation?
     /// @dev Returns the product of point `point` and scalar `scalar` as
     ///      address.
     ///
+    /// @dev Reverts if:
+    ///         Scalar malleable
+    ///
     /// @dev Note that this function is substantially cheaper than
     ///      `mul(ProjectivePoint,uint)(ProjectivePoint)` with the caveat that
     ///      only the point's address is returned instead of the point itself.
@@ -270,7 +272,7 @@ library Points {
         returns (address)
     {
         if (scalar >= Q) {
-            revert Errors.CRYSOL_ScalarMalleable();
+            revert Errors.CRYSOL_ScalarInvalid();
         }
 
         if (scalar == 0 || point.isIdentity()) {
@@ -419,18 +421,20 @@ library Points {
         );
     }
 
-    // TODO: Misses revert documentation?
     /// @dev Returns the product of projective point `point` and scalar `scalar`
     ///      as projective point.
     ///
+    /// @dev Reverts if:
+    ///         Scalar malleable
+    ///
     /// @dev Uses the repeated add-and-double algorithm.
     function mul(ProjectivePoint memory point, uint scalar)
         internal
         pure
         returns (ProjectivePoint memory)
     {
         if (scalar >= Q) {
-            revert Errors.CRYSOL_ScalarMalleable();
+            revert Errors.CRYSOL_ScalarInvalid();
         }
 
         if (scalar == 0) {
@@ -440,9 +444,6 @@ library Points {
         ProjectivePoint memory copy = point;
         ProjectivePoint memory result = ProjectiveIdentity();
 
-        // TODO: Can endomorphism be used?
-        //       See Faster Point Multiplication on Elliptic Curves with
-        //       Efficient Endomorphism from GLV.
         while (scalar != 0) {
             if (scalar & 1 == 1) {
                 result = result.add(copy);
@@ -612,7 +613,6 @@ library Points {
         // Revert if identity not 1 byte encoded.
         // TODO: Not explicitly tested.
         if (point.isIdentity()) {
-            // TODO: Need different error type.
             revert Errors.CRYSOL_PointInvalid();
         }
 
@@ -642,7 +642,7 @@ library Points {
         returns (bytes memory blob)
     {
         if (!point.isOnCurve()) {
-            revert Errors.CRYSOL_PointNotOnCurve();
+            revert Errors.CRYSOL_PointInvalid();
         }
 
         // Note to catch special encoding for identity.
@@ -702,20 +702,19 @@ library Points {
         }
 
         // Revert if identity not 1 byte encoded.
-        // TODO: Should have own error for identity not 1 byte encoded?
         //
         // Note that identity is explicitly enforced to be 1 byte encoded,
         // eventhough for x = 0 the resulting point is not on the curve anyway.
         if (xRaw == 0) {
-            revert Errors.CRYSOL_PointNotOnCurve();
+            revert Errors.CRYSOL_PointInvalid();
         }
 
         // Construct x coordinate as felt.
         bool ok;
         Felt x;
         (x, ok) = Fp.tryFromUint(xRaw);
         if (!ok) {
-            revert Errors.CRYSOL_PointNotOnCurve();
+            revert Errors.CRYSOL_PointInvalid();
         }
 
         // Compute α = x³ + ax + b (mod p).
@@ -749,7 +748,7 @@ library Points {
         // Revert if point not on curve.
         // TODO: Find vectors for x coordinates not on the curve.
         if (!point.isOnCurve()) {
-            revert Errors.CRYSOL_PointNotOnCurve();
+            revert Errors.CRYSOL_PointInvalid();
         }
 
         return point;
@@ -773,7 +772,7 @@ library Points {
         returns (bytes memory blob)
     {
         if (!point.isOnCurve()) {
-            revert Errors.CRYSOL_PointNotOnCurve();
+            revert Errors.CRYSOL_PointInvalid();
         }
 
         // Note to catch special encoding for identity.

test/arithmetic/Fp.t.sol

@@ -71,7 +71,7 @@ contract FpTest is Test {
     function testFuzz_feltFromUint_RevertsIf_ScalarNotAFelt(uint seed) public {
         uint scalar = _bound(seed, Secp256k1.P, type(uint).max);
 
-        vm.expectRevert(Errors.CRYSOL_ScalarNotAFelt.selector);
+        vm.expectRevert(Errors.CRYSOL_ScalarInvalid.selector);
         wrapper.feltFromUint(scalar);
     }
 

test/arithmetic/Points.t.sol

@@ -448,7 +448,7 @@ contract PointsTest is Test {
     ) public {
         vm.assume(scalar >= Points.Q);
 
-        vm.expectRevert(Errors.CRYSOL_ScalarMalleable.selector);
+        vm.expectRevert(Errors.CRYSOL_ScalarInvalid.selector);
         wrapper.mulToAddress(point, scalar);
     }
 
@@ -665,7 +665,7 @@ contract PointsTest is Test {
     ) public {
         vm.assume(scalar >= Points.Q);
 
-        vm.expectRevert(Errors.CRYSOL_ScalarMalleable.selector);
+        vm.expectRevert(Errors.CRYSOL_ScalarInvalid.selector);
         wrapper.mul(point, scalar);
     }
 
@@ -709,8 +709,11 @@ contract PointsTest is Test {
 
         // Compute [a+b]G.
         uint scalar = addmod(a.asUint(), b.asUint(), Secp256k1.Q);
-        Point memory want =
-            Secp256k1.secretKeyFromUint(scalar).toPublicKey().intoPoint();
+        vm.assume(scalar != 0);
+        // forgefmt: disable-next-item
+        Point memory want = Secp256k1.secretKeyFromUint(scalar)
+                                     .toPublicKey()
+                                     .intoPoint();
 
         // Compute [a]G + [b]G via ProjectivePoints.
         // forgefmt: disable-next-item
@@ -739,8 +742,11 @@ contract PointsTest is Test {
 
         // Compute [a+b]G.
         uint scalar = addmod(a.asUint(), b.asUint(), Secp256k1.Q);
-        Point memory want =
-            Secp256k1.secretKeyFromUint(scalar).toPublicKey().intoPoint();
+        vm.assume(scalar != 0);
+        // forgefmt: disable-next-item
+        Point memory want = Secp256k1.secretKeyFromUint(scalar)
+                                     .toPublicKey()
+                                     .intoPoint();
 
         // Compute [a]G + [b]G via ProjectivePoints.
         // forgefmt: disable-next-item
@@ -862,7 +868,7 @@ contract PointsTest is Test {
     ) public {
         vm.assume(!point.isOnCurve());
 
-        vm.expectRevert(Errors.CRYSOL_PointNotOnCurve.selector);
+        vm.expectRevert(Errors.CRYSOL_PointInvalid.selector);
         wrapper.toEncoded(point);
     }
 
@@ -935,12 +941,12 @@ contract PointsTest is Test {
 
         // Using 0x02 prefix.
         blob = abi.encodePacked(bytes1(0x02), uint(0));
-        vm.expectRevert(Errors.CRYSOL_PointNotOnCurve.selector);
+        vm.expectRevert(Errors.CRYSOL_PointInvalid.selector);
         wrapper.pointFromCompressedEncoded(blob);
 
         // Using 0x03 prefix.
         blob = abi.encodePacked(bytes1(0x03), uint(0));
-        vm.expectRevert(Errors.CRYSOL_PointNotOnCurve.selector);
+        vm.expectRevert(Errors.CRYSOL_PointInvalid.selector);
         wrapper.pointFromCompressedEncoded(blob);
     }
 
@@ -953,12 +959,12 @@ contract PointsTest is Test {
 
         // Using 0x02 prefix.
         blob = abi.encodePacked(bytes1(0x02), uint(0));
-        vm.expectRevert(Errors.CRYSOL_PointNotOnCurve.selector);
+        vm.expectRevert(Errors.CRYSOL_PointInvalid.selector);
         wrapper.pointFromCompressedEncoded(blob);
 
         // Using 0x03 prefix.
         blob = abi.encodePacked(bytes1(0x03), uint(0));
-        vm.expectRevert(Errors.CRYSOL_PointNotOnCurve.selector);
+        vm.expectRevert(Errors.CRYSOL_PointInvalid.selector);
         wrapper.pointFromCompressedEncoded(blob);
     }
 
@@ -1011,7 +1017,7 @@ contract PointsTest is Test {
     ) public {
         vm.assume(!point.isOnCurve());
 
-        vm.expectRevert(Errors.CRYSOL_PointNotOnCurve.selector);
+        vm.expectRevert(Errors.CRYSOL_PointInvalid.selector);
         wrapper.toCompressedEncoded(point);
     }
 }