fix: constraint on bn254 values (#26)

* fix: constraint on bn254 values * fix: test sig * fix: test invalid id commitment
vacp2p · Aug 16, 2023 · d899a0c · d899a0c
1 parent b0e78f2
commit d899a0c
Show file tree

Hide file tree

Showing 5 changed files with 124 additions and 75 deletions.
diff --git a/contracts/PoseidonHasher.sol b/contracts/PoseidonHasher.sol
@@ -11,7 +11,7 @@ interface IPoseidonHasher {
 }
 
 contract PoseidonHasher is IPoseidonHasher {
-    uint256 constant Q = 21888242871839275222246405745257275088548364400416034343698204186575808495617;
+    uint256 public constant Q = 21888242871839275222246405745257275088548364400416034343698204186575808495617;
     uint256 constant C0 = 4417881134626180770308697923359573201005643519861877412381846989312604493735;
     uint256 constant C1 = 5433650512959517612316327474713065966758808864213826738576266661723522780033;
     uint256 constant C2 = 13641176377184356099764086973022553863760045607496549923679278773208775739952;

diff --git a/contracts/RlnBase.sol b/contracts/RlnBase.sol
@@ -2,7 +2,7 @@
 
 pragma solidity 0.8.15;
 
-import {IPoseidonHasher} from "./PoseidonHasher.sol";
+import {PoseidonHasher} from "./PoseidonHasher.sol";
 import {IVerifier} from "./IVerifier.sol";
 
 /// The tree is full
@@ -19,6 +19,9 @@ error DuplicateIdCommitment();
 /// Failed validation on registration/slashing
 error FailedValidation();
 
+/// Invalid idCommitment
+error InvalidIdCommitment(uint256 idCommitment);
+
 /// Invalid receiver address, when the receiver is the contract itself or 0x0
 error InvalidReceiverAddress(address to);
 
@@ -62,7 +65,7 @@ abstract contract RlnBase {
     mapping(address => uint256) public withdrawalBalance;
 
     /// @notice The Poseidon hasher contract
-    IPoseidonHasher public immutable poseidonHasher;
+    PoseidonHasher public immutable poseidonHasher;
 
     /// @notice The groth16 verifier contract
     IVerifier public immutable verifier;
@@ -77,17 +80,22 @@ abstract contract RlnBase {
     /// @param index The index of the member in the set
     event MemberWithdrawn(uint256 idCommitment, uint256 index);
 
+    modifier onlyValidIdCommitment(uint256 idCommitment) {
+        if (!isValidCommitment(idCommitment)) revert InvalidIdCommitment(idCommitment);
+        _;
+    }
+
     constructor(uint256 membershipDeposit, uint256 depth, address _poseidonHasher, address _verifier) {
         MEMBERSHIP_DEPOSIT = membershipDeposit;
         DEPTH = depth;
         SET_SIZE = 1 << depth;
-        poseidonHasher = IPoseidonHasher(_poseidonHasher);
+        poseidonHasher = PoseidonHasher(_poseidonHasher);
         verifier = IVerifier(_verifier);
     }
 
     /// Allows a user to register as a member
     /// @param idCommitment The idCommitment of the member
-    function register(uint256 idCommitment) external payable virtual {
+    function register(uint256 idCommitment) external payable virtual onlyValidIdCommitment(idCommitment) {
         if (msg.value != MEMBERSHIP_DEPOSIT) {
             revert InsufficientDeposit(MEMBERSHIP_DEPOSIT, msg.value);
         }
@@ -114,7 +122,11 @@ abstract contract RlnBase {
 
     /// @dev Allows a user to slash a member
     /// @param idCommitment The idCommitment of the member
-    function slash(uint256 idCommitment, address payable receiver, uint256[8] calldata proof) external virtual {
+    function slash(uint256 idCommitment, address payable receiver, uint256[8] calldata proof)
+        external
+        virtual
+        onlyValidIdCommitment(idCommitment)
+    {
         _validateSlash(idCommitment, receiver, proof);
         _slash(idCommitment, receiver, proof);
     }
@@ -177,6 +189,10 @@ abstract contract RlnBase {
         return poseidonHasher.hash(input);
     }
 
+    function isValidCommitment(uint256 idCommitment) public view returns (bool) {
+        return idCommitment != 0 && idCommitment < poseidonHasher.Q();
+    }
+
     /// @dev Groth16 proof verification
     function _verifyProof(uint256 idCommitment, address receiver, uint256[8] calldata proof)
         internal

diff --git a/docs/index.md b/docs/index.md
@@ -20,8 +20,8 @@ Hashes the input using the Poseidon hash function, n = 2, second input is the co
 
 #### Parameters
 
-| Name  | Type    | Description       |
-| ----- | ------- | ----------------- |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
 | input | uint256 | The input to hash |
 
 ## PoseidonHasher
@@ -834,11 +834,11 @@ Hashes the input using the Poseidon hash function, n = 2, second input is the co
 
 #### Parameters
 
-| Name  | Type    | Description       |
-| ----- | ------- | ----------------- |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
 | input | uint256 | The input to hash |
 
-### \_hash
+### _hash
 
 ```solidity
 function _hash(uint256 input) internal pure returns (uint256 result)
@@ -852,15 +852,15 @@ function _hash(uint256 input) internal pure returns (uint256 result)
 constructor(uint256 membershipDeposit, uint256 depth, address _poseidonHasher, address _verifier) public
 ```
 
-### \_validateRegistration
+### _validateRegistration
 
 ```solidity
 function _validateRegistration(uint256 idCommitment) internal pure
 ```
 
 _Inheriting contracts MUST override this function_
 
-### \_validateSlash
+### _validateSlash
 
 ```solidity
 function _validateSlash(uint256 idCommitment, address payable receiver, uint256[8] proof) internal pure
@@ -884,8 +884,8 @@ Invalid deposit amount
 
 ### Parameters
 
-| Name     | Type    | Description                 |
-| -------- | ------- | --------------------------- |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
 | required | uint256 | The required deposit amount |
 | provided | uint256 | The provided deposit amount |
 
@@ -905,6 +905,14 @@ error FailedValidation()
 
 Failed validation on registration/slashing
 
+## InvalidIdCommitment
+
+```solidity
+error InvalidIdCommitment(uint256 idCommitment)
+```
+
+Invalid idCommitment
+
 ## InvalidReceiverAddress
 
 ```solidity
@@ -1016,7 +1024,7 @@ The balance of each user that can be withdrawn
 ### poseidonHasher
 
 ```solidity
-contract IPoseidonHasher poseidonHasher
+contract PoseidonHasher poseidonHasher
 ```
 
 The Poseidon hasher contract
@@ -1039,10 +1047,10 @@ Emitted when a new member is added to the set
 
 #### Parameters
 
-| Name         | Type    | Description                        |
-| ------------ | ------- | ---------------------------------- |
-| idCommitment | uint256 | The idCommitment of the member     |
-| index        | uint256 | The index of the member in the set |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| idCommitment | uint256 | The idCommitment of the member |
+| index | uint256 | The index of the member in the set |
 
 ### MemberWithdrawn
 
@@ -1054,10 +1062,16 @@ Emitted when a member is removed from the set
 
 #### Parameters
 
-| Name         | Type    | Description                        |
-| ------------ | ------- | ---------------------------------- |
-| idCommitment | uint256 | The idCommitment of the member     |
-| index        | uint256 | The index of the member in the set |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| idCommitment | uint256 | The idCommitment of the member |
+| index | uint256 | The index of the member in the set |
+
+### onlyValidIdCommitment
+
+```solidity
+modifier onlyValidIdCommitment(uint256 idCommitment)
+```
 
 ### constructor
 
@@ -1075,11 +1089,11 @@ Allows a user to register as a member
 
 #### Parameters
 
-| Name         | Type    | Description                    |
-| ------------ | ------- | ------------------------------ |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
 | idCommitment | uint256 | The idCommitment of the member |
 
-### \_register
+### _register
 
 ```solidity
 function _register(uint256 idCommitment, uint256 stake) internal virtual
@@ -1089,12 +1103,12 @@ Registers a member
 
 #### Parameters
 
-| Name         | Type    | Description                            |
-| ------------ | ------- | -------------------------------------- |
-| idCommitment | uint256 | The idCommitment of the member         |
-| stake        | uint256 | The amount of eth staked by the member |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| idCommitment | uint256 | The idCommitment of the member |
+| stake | uint256 | The amount of eth staked by the member |
 
-### \_validateRegistration
+### _validateRegistration
 
 ```solidity
 function _validateRegistration(uint256 idCommitment) internal view virtual
@@ -1112,13 +1126,13 @@ _Allows a user to slash a member_
 
 #### Parameters
 
-| Name         | Type            | Description                    |
-| ------------ | --------------- | ------------------------------ |
-| idCommitment | uint256         | The idCommitment of the member |
-| receiver     | address payable |                                |
-| proof        | uint256[8]      |                                |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| idCommitment | uint256 | The idCommitment of the member |
+| receiver | address payable |  |
+| proof | uint256[8] |  |
 
-### \_slash
+### _slash
 
 ```solidity
 function _slash(uint256 idCommitment, address payable receiver, uint256[8] proof) internal virtual
@@ -1129,13 +1143,13 @@ stake to the receiver's available withdrawal balance_
 
 #### Parameters
 
-| Name         | Type            | Description                      |
-| ------------ | --------------- | -------------------------------- |
-| idCommitment | uint256         | The idCommitment of the member   |
-| receiver     | address payable | The address to receive the funds |
-| proof        | uint256[8]      |                                  |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| idCommitment | uint256 | The idCommitment of the member |
+| receiver | address payable | The address to receive the funds |
+| proof | uint256[8] |  |
 
-### \_validateSlash
+### _validateSlash
 
 ```solidity
 function _validateSlash(uint256 idCommitment, address payable receiver, uint256[8] proof) internal view virtual
@@ -1160,11 +1174,17 @@ NOTE: The variant of Poseidon we use accepts only 1 input, assume n=2, and the s
 
 #### Parameters
 
-| Name  | Type    | Description       |
-| ----- | ------- | ----------------- |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
 | input | uint256 | The value to hash |
 
-### \_verifyProof
+### isValidCommitment
+
+```solidity
+function isValidCommitment(uint256 idCommitment) public view returns (bool)
+```
+
+### _verifyProof
 
 ```solidity
 function _verifyProof(uint256 idCommitment, address receiver, uint256[8] proof) internal view virtual returns (bool)
@@ -1200,9 +1220,9 @@ function P1() internal pure returns (struct Pairing.G1Point)
 
 #### Return Values
 
-| Name | Type                   | Description         |
-| ---- | ---------------------- | ------------------- |
-| [0]  | struct Pairing.G1Point | the generator of G1 |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| [0] | struct Pairing.G1Point | the generator of G1 |
 
 ### P2
 
@@ -1212,9 +1232,9 @@ function P2() internal pure returns (struct Pairing.G2Point)
 
 #### Return Values
 
-| Name | Type                   | Description         |
-| ---- | ---------------------- | ------------------- |
-| [0]  | struct Pairing.G2Point | the generator of G2 |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| [0] | struct Pairing.G2Point | the generator of G2 |
 
 ### negate
 
@@ -1224,9 +1244,9 @@ function negate(struct Pairing.G1Point p) internal pure returns (struct Pairing.
 
 #### Return Values
 
-| Name | Type                   | Description                                                    |
-| ---- | ---------------------- | -------------------------------------------------------------- |
-| r    | struct Pairing.G1Point | the negation of p, i.e. p.addition(p.negate()) should be zero. |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| r | struct Pairing.G1Point | the negation of p, i.e. p.addition(p.negate()) should be zero. |
 
 ### addition
 
@@ -1236,9 +1256,9 @@ function addition(struct Pairing.G1Point p1, struct Pairing.G1Point p2) internal
 
 #### Return Values
 
-| Name | Type                   | Description                 |
-| ---- | ---------------------- | --------------------------- |
-| r    | struct Pairing.G1Point | the sum of two points of G1 |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| r | struct Pairing.G1Point | the sum of two points of G1 |
 
 ### scalar_mul
 
@@ -1248,9 +1268,9 @@ function scalar_mul(struct Pairing.G1Point p, uint256 s) internal view returns (
 
 #### Return Values
 
-| Name | Type                   | Description                                                                                                                 |
-| ---- | ---------------------- | --------------------------------------------------------------------------------------------------------------------------- |
-| r    | struct Pairing.G1Point | the product of a point on G1 and a scalar, i.e. p == p.scalar_mul(1) and p.addition(p) == p.scalar_mul(2) for all points p. |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| r | struct Pairing.G1Point | the product of a point on G1 and a scalar, i.e. p == p.scalar_mul(1) and p.addition(p) == p.scalar_mul(2) for all points p. |
 
 ### pairing
 
@@ -1260,9 +1280,9 @@ function pairing(struct Pairing.G1Point[] p1, struct Pairing.G2Point[] p2) inter
 
 #### Return Values
 
-| Name | Type | Description                                                                                                                                                          |
-| ---- | ---- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| [0]  | bool | the result of computing the pairing check e(p1[0], p2[0]) _ .... _ e(p1[n], p2[n]) == 1 For example pairing([P1(), P1().negate()], [P2(), P2()]) should return true. |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| [0] | bool | the result of computing the pairing check e(p1[0], p2[0]) *  .... * e(p1[n], p2[n]) == 1 For example pairing([P1(), P1().negate()], [P2(), P2()]) should return true. |
 
 ### pairingProd2
 
@@ -1332,6 +1352,7 @@ function verifyProof(uint256[2] a, uint256[2][2] b, uint256[2] c, uint256[2] inp
 
 #### Return Values
 
-| Name | Type | Description                 |
-| ---- | ---- | --------------------------- |
-| r    | bool | bool true if proof is valid |
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| r | bool | bool true if proof is valid |
+