fix: constraint on bn254 values

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;

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

docs/index.md

@@ -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
@@ -1059,6 +1067,12 @@ Emitted when a member is removed from the set
 | idCommitment | uint256 | The idCommitment of the member     |
 | index        | uint256 | The index of the member in the set |
 
+### onlyValidIdCommitment
+
+```solidity
+modifier onlyValidIdCommitment(uint256 idCommitment)
+```
+
 ### constructor
 
 ```solidity
@@ -1164,6 +1178,12 @@ NOTE: The variant of Poseidon we use accepts only 1 input, assume n=2, and the s
 | ----- | ------- | ----------------- |
 | input | uint256 | The value to hash |
 
+### isValidCommitment
+
+```solidity
+function isValidCommitment(uint256 idCommitment) public view returns (bool)
+```
+
 ### \_verifyProof
 
 ```solidity

test/RLNApp.t.sol

@@ -10,7 +10,7 @@ import "forge-std/console.sol";
 
 contract RlnApp is RlnBase {
     uint256 public constant allowedIdCommitment =
-        21888242871839275222246405745257275088548364400416034343698204186575808495617;
+        19014214495641488759237505126948346942972912379615652741039992445865937985820;
 
     uint256 private membershipDeposit = 1000000000000000;
     uint256 private depth = 20;
@@ -57,6 +57,7 @@ contract RLNAppTest is Test {
 
     function test__InvalidRegistration(uint256 idCommitment) public {
         vm.assume(idCommitment != rlnApp.allowedIdCommitment());
+        vm.assume(rlnApp.isValidCommitment(idCommitment));
         vm.expectRevert(FailedValidation.selector);
         rlnApp.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
     }

test/Rln.t.sol

@@ -37,12 +37,14 @@ contract RlnTest is Test {
     }
 
     function test__ValidRegistration(uint256 idCommitment) public {
+        vm.assume(rln.isValidCommitment(idCommitment));
         rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
         assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT);
         assertEq(rln.members(idCommitment), 1);
     }
 
     function test__InvalidRegistration__DuplicateCommitment(uint256 idCommitment) public {
+        vm.assume(rln.isValidCommitment(idCommitment));
         rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
         assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT);
         assertEq(rln.members(idCommitment), 1);
@@ -51,13 +53,13 @@ contract RlnTest is Test {
     }
 
     function test__InvalidRegistration__InsufficientDeposit(uint256 idCommitment) public {
+        vm.assume(rln.isValidCommitment(idCommitment));
         uint256 badDepositAmount = MEMBERSHIP_DEPOSIT - 1;
         vm.expectRevert(abi.encodeWithSelector(InsufficientDeposit.selector, MEMBERSHIP_DEPOSIT, badDepositAmount));
         rln.register{value: badDepositAmount}(idCommitment);
     }
 
-    function test__InvalidRegistration__FullSet(uint256 idCommitmentSeed) public {
+    function test__InvalidRegistration__FullSet() public {
-        vm.assume(idCommitmentSeed < 2 ** 255 - SET_SIZE);
         Rln tempRln = new Rln(
             MEMBERSHIP_DEPOSIT,
             2,
@@ -65,12 +67,12 @@ contract RlnTest is Test {
             address(rln.verifier())
         );
         uint256 setSize = tempRln.SET_SIZE() - 1;
-        for (uint256 i = 0; i < setSize; i++) {
+        for (uint256 i = 1; i <= setSize; i++) {
-            tempRln.register{value: MEMBERSHIP_DEPOSIT}(idCommitmentSeed + i);
+            tempRln.register{value: MEMBERSHIP_DEPOSIT}(i);
         }
         assertEq(tempRln.idCommitmentIndex(), 4);
         vm.expectRevert(FullTree.selector);
-        tempRln.register{value: MEMBERSHIP_DEPOSIT}(idCommitmentSeed + setSize);
+        tempRln.register{value: MEMBERSHIP_DEPOSIT}(setSize + 1);
     }
 
     function test__ValidSlash(uint256 idCommitment, address payable to) public {
@@ -79,6 +81,7 @@ contract RlnTest is Test {
         assumePayable(to);
         assumeNotPrecompile(to);
         vm.assume(to != address(0));
+        vm.assume(rln.isValidCommitment(idCommitment));
 
         rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
         assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT);
@@ -110,6 +113,7 @@ contract RlnTest is Test {
     }
 
     function test__InvalidSlash__InvalidIdCommitment(uint256 idCommitment) public {
+        vm.assume(rln.isValidCommitment(idCommitment));
         vm.expectRevert(abi.encodeWithSelector(MemberNotRegistered.selector, idCommitment));
         rln.slash(idCommitment, payable(address(this)), zeroedProof);
     }
@@ -120,6 +124,7 @@ contract RlnTest is Test {
         assumePayable(to);
         assumeNotPrecompile(to);
         vm.assume(to != address(0));
+        vm.assume(rln.isValidCommitment(idCommitment));
 
         rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
         assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT);