fix(rln): use zero based indexing for commitments (#28)

vacp2p · Aug 30, 2023 · 35f2182 · 35f2182
1 parent bd8403a
commit 35f2182
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 7 deletions.
diff --git a/contracts/RlnBase.sol b/contracts/RlnBase.sol
@@ -51,7 +51,7 @@ abstract contract RlnBase {
     uint256 public immutable SET_SIZE;
 
     /// @notice The index of the next member to be registered
-    uint256 public idCommitmentIndex = 1;
+    uint256 public idCommitmentIndex = 0;
 
     /// @notice The amount of eth staked by each member
     /// maps from idCommitment to the amount staked
@@ -61,6 +61,8 @@ abstract contract RlnBase {
     /// maps from idCommitment to their index in the set
     mapping(uint256 => uint256) public members;
 
+    mapping(uint256 => bool) public memberExists;
+
     /// @notice The balance of each user that can be withdrawn
     mapping(address => uint256) public withdrawalBalance;
 
@@ -111,10 +113,11 @@ abstract contract RlnBase {
     /// @param idCommitment The idCommitment of the member
     /// @param stake The amount of eth staked by the member
     function _register(uint256 idCommitment, uint256 stake) internal virtual {
-        if (members[idCommitment] != 0) revert DuplicateIdCommitment();
+        if (memberExists[idCommitment]) revert DuplicateIdCommitment();
         if (idCommitmentIndex >= SET_SIZE) revert FullTree();
 
         members[idCommitment] = idCommitmentIndex;
+        memberExists[idCommitment] = true;
         stakedAmounts[idCommitment] = stake;
 
         emit MemberRegistered(idCommitment, idCommitmentIndex);
@@ -144,7 +147,7 @@ abstract contract RlnBase {
             revert InvalidReceiverAddress(receiver);
         }
 
-        if (members[idCommitment] == 0) revert MemberNotRegistered(idCommitment);
+        if (memberExists[idCommitment] == false) revert MemberNotRegistered(idCommitment);
         // check if member is registered
         if (stakedAmounts[idCommitment] == 0) {
             revert MemberHasNoStake(idCommitment);
@@ -159,6 +162,7 @@ abstract contract RlnBase {
         // delete member
         uint256 index = members[idCommitment];
         members[idCommitment] = 0;
+        memberExists[idCommitment] = false;
         stakedAmounts[idCommitment] = 0;
 
         // refund deposit

diff --git a/test/Rln.t.sol b/test/Rln.t.sol
@@ -41,14 +41,16 @@ contract RlnTest is Test {
         vm.assume(rln.isValidCommitment(idCommitment));
         rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
         assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT);
-        assertEq(rln.members(idCommitment), 1);
+        assertEq(rln.memberExists(idCommitment), true);
+        assertEq(rln.members(idCommitment), 0);
     }
 
     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);
+        assertEq(rln.memberExists(idCommitment), true);
+        assertEq(rln.members(idCommitment), 0);
         vm.expectRevert(DuplicateIdCommitment.selector);
         rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
     }
@@ -73,7 +75,7 @@ contract RlnTest is Test {
             address(rln.poseidonHasher()),
             address(rln.verifier())
         );
-        uint256 setSize = tempRln.SET_SIZE() - 1;
+        uint256 setSize = tempRln.SET_SIZE();
         for (uint256 i = 1; i <= setSize; i++) {
             tempRln.register{value: MEMBERSHIP_DEPOSIT}(i);
         }
@@ -141,7 +143,7 @@ contract RlnTest is Test {
         assertEq(rln.members(idCommitment), 0);
 
         // manually set members[idCommitment] to true using vm
-        stdstore.target(address(rln)).sig("members(uint256)").with_key(idCommitment).depth(0).checked_write(true);
+        stdstore.target(address(rln)).sig("memberExists(uint256)").with_key(idCommitment).depth(0).checked_write(true);
 
         vm.expectRevert(abi.encodeWithSelector(MemberHasNoStake.selector, idCommitment));
         rln.slash(idCommitment, to, zeroedProof);