Merge pull request #19 from vacp2p/custom-errors

feat: custom errors

contracts/Rln.sol

@@ -4,6 +4,34 @@ pragma solidity 0.8.15;
 
 import {IPoseidonHasher} from "./PoseidonHasher.sol";
 
+/// Invalid deposit amount
+/// @param required The required deposit amount
+/// @param provided The provided deposit amount
+error InsufficientDeposit(uint256 required, uint256 provided);
+
+/// Provided Batch is empty
+error EmptyBatch();
+
+/// Batch is full, during batch operations
+error FullBatch();
+
+/// Member is already registered
+error DuplicateIdCommitment();
+
+/// Batch size mismatch, when the length of secrets and receivers are not equal
+/// @param givenSecretsLen The length of the secrets array
+/// @param givenReceiversLen The length of the receivers array
+error MismatchedBatchSize(uint256 givenSecretsLen, uint256 givenReceiversLen);
+
+/// Invalid withdrawal address, when the receiver is the contract itself or 0x0
+error InvalidWithdrawalAddress(address to);
+
+/// Member is not registered
+error MemberNotRegistered(uint256 idCommitment);
+
+/// Member has no stake
+error MemberHasNoStake(uint256 idCommitment);
+
 contract RLN {
     /// @notice The deposit amount required to register as a member
     uint256 public immutable MEMBERSHIP_DEPOSIT;
@@ -49,10 +77,8 @@ contract RLN {
     /// Allows a user to register as a member
     /// @param idCommitment The idCommitment of the member
     function register(uint256 idCommitment) external payable {
-        require(
+        if (msg.value != MEMBERSHIP_DEPOSIT)
-            msg.value == MEMBERSHIP_DEPOSIT,
+            revert InsufficientDeposit(MEMBERSHIP_DEPOSIT, msg.value);
-            "RLN, register: membership deposit is not satisfied"
-        );
         _register(idCommitment, msg.value);
     }
 
@@ -60,15 +86,11 @@ contract RLN {
     /// @param idCommitments array of idCommitments
     function registerBatch(uint256[] calldata idCommitments) external payable {
         uint256 idCommitmentlen = idCommitments.length;
-        require(idCommitmentlen > 0, "RLN, registerBatch: batch size zero");
+        if (idCommitmentlen == 0) revert EmptyBatch();
-        require(
+        if (idCommitmentIndex + idCommitmentlen >= SET_SIZE) revert FullBatch();
-            idCommitmentIndex + idCommitmentlen <= SET_SIZE,
+        uint256 requiredDeposit = MEMBERSHIP_DEPOSIT * idCommitmentlen;
-            "RLN, registerBatch: set is full"
+        if (msg.value != requiredDeposit)
-        );
+            revert InsufficientDeposit(requiredDeposit, msg.value);
-        require(
-            msg.value == MEMBERSHIP_DEPOSIT * idCommitmentlen,
-            "RLN, registerBatch: membership deposit is not satisfied"
-        );
         for (uint256 i = 0; i < idCommitmentlen; i++) {
             _register(idCommitments[i], msg.value / idCommitmentlen);
         }
@@ -78,11 +100,8 @@ contract RLN {
     /// @param idCommitment The idCommitment of the member
     /// @param stake The amount of eth staked by the member
     function _register(uint256 idCommitment, uint256 stake) internal {
-        require(
+        if (members[idCommitment]) revert DuplicateIdCommitment();
-            !members[idCommitment],
+        if (idCommitmentIndex >= SET_SIZE) revert FullBatch();
-            "RLN, _register: member already registered"
-        );
-        require(idCommitmentIndex < SET_SIZE, "RLN, register: set is full");
 
         members[idCommitment] = true;
         stakedAmounts[idCommitment] = stake;
@@ -99,11 +118,9 @@ contract RLN {
         address payable[] calldata receivers
     ) external {
         uint256 batchSize = secrets.length;
-        require(batchSize != 0, "RLN, withdrawBatch: batch size zero");
+        if (batchSize == 0) revert EmptyBatch();
-        require(
+        if (batchSize != receivers.length)
-            batchSize == receivers.length,
+            revert MismatchedBatchSize(secrets.length, receivers.length);
-            "RLN, withdrawBatch: batch size mismatch receivers"
-        );
         for (uint256 i = 0; i < batchSize; i++) {
             _withdraw(secrets[i], receivers[i]);
         }
@@ -120,26 +137,15 @@ contract RLN {
     /// @param secret The idSecretHash of the member
     /// @param receiver The address to receive the funds
     function _withdraw(uint256 secret, address payable receiver) internal {
-        require(
+        if (receiver == address(this) || receiver == address(0))
-            receiver != address(0),
+            revert InvalidWithdrawalAddress(receiver);
-            "RLN, _withdraw: empty receiver address"
-        );
-
-        require(
-            receiver != address(this),
-            "RLN, _withdraw: cannot withdraw to RLN"
-        );
 
         // derive idCommitment
         uint256 idCommitment = hash(secret);
         // check if member is registered
-        require(members[idCommitment], "RLN, _withdraw: member not registered");
+        if (!members[idCommitment]) revert MemberNotRegistered(idCommitment);
-
+        if (stakedAmounts[idCommitment] == 0)
-        // check if member has stake
+            revert MemberHasNoStake(idCommitment);
-        require(
-            stakedAmounts[idCommitment] != 0,
-            "RLN, _withdraw: member has no stake"
-        );
 
         uint256 amountToTransfer = stakedAmounts[idCommitment];
 

docs/index.md

@@ -836,6 +836,84 @@ Hashes the input using the Poseidon hash function, n = 2, second input is the co
 function _hash(uint256 input) internal pure returns (uint256 result)
 ```
 
+## InsufficientDeposit
+
+```solidity
+error InsufficientDeposit(uint256 required, uint256 provided)
+```
+
+Invalid deposit amount
+
+### Parameters
+
+| Name     | Type    | Description                 |
+| -------- | ------- | --------------------------- |
+| required | uint256 | The required deposit amount |
+| provided | uint256 | The provided deposit amount |
+
+## EmptyBatch
+
+```solidity
+error EmptyBatch()
+```
+
+Provided Batch is empty
+
+## FullBatch
+
+```solidity
+error FullBatch()
+```
+
+Batch is full, during batch operations
+
+## DuplicateIdCommitment
+
+```solidity
+error DuplicateIdCommitment()
+```
+
+Member is already registered
+
+## MismatchedBatchSize
+
+```solidity
+error MismatchedBatchSize(uint256 givenSecretsLen, uint256 givenReceiversLen)
+```
+
+Batch size mismatch, when the length of secrets and receivers are not equal
+
+### Parameters
+
+| Name              | Type    | Description                       |
+| ----------------- | ------- | --------------------------------- |
+| givenSecretsLen   | uint256 | The length of the secrets array   |
+| givenReceiversLen | uint256 | The length of the receivers array |
+
+## InvalidWithdrawalAddress
+
+```solidity
+error InvalidWithdrawalAddress(address to)
+```
+
+Invalid withdrawal address, when the receiver is the contract itself or 0x0
+
+## MemberNotRegistered
+
+```solidity
+error MemberNotRegistered(uint256 idCommitment)
+```
+
+Member is not registered
+
+## MemberHasNoStake
+
+```solidity
+error MemberHasNoStake(uint256 idCommitment)
+```
+
+Member has no stake
+
 ## RLN
 
 ### MEMBERSHIP_DEPOSIT

test/RLN.t.sol

@@ -75,18 +75,22 @@ contract RLNTest is Test {
         rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
         assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT);
         assertEq(rln.members(idCommitment), true);
-        // TODO: use custom errors instead of revert strings
+        vm.expectRevert(DuplicateIdCommitment.selector);
-        vm.expectRevert(bytes("RLN, _register: member already registered"));
         rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
     }
 
     function test__InvalidRegistration__InsufficientDeposit(
         uint256 idCommitment
     ) public {
+        uint256 badDepositAmount = MEMBERSHIP_DEPOSIT - 1;
         vm.expectRevert(
-            bytes("RLN, register: membership deposit is not satisfied")
+            abi.encodeWithSelector(
+                InsufficientDeposit.selector,
+                MEMBERSHIP_DEPOSIT,
+                badDepositAmount
+            )
         );
-        rln.register{value: MEMBERSHIP_DEPOSIT - 1}(idCommitment);
+        rln.register{value: badDepositAmount}(idCommitment);
     }
 
     function test__InvalidRegistration__FullSet(
@@ -103,7 +107,7 @@ contract RLNTest is Test {
             tempRln.register{value: MEMBERSHIP_DEPOSIT}(idCommitmentSeed + i);
         }
         assertEq(tempRln.idCommitmentIndex(), 4);
-        vm.expectRevert(bytes("RLN, register: set is full"));
+        vm.expectRevert(FullBatch.selector);
         tempRln.register{value: MEMBERSHIP_DEPOSIT}(idCommitmentSeed + setSize);
     }
 
@@ -135,13 +139,13 @@ contract RLNTest is Test {
         assertEq(tempRln.idCommitmentIndex(), 4);
         uint256[] memory idCommitments = new uint256[](1);
         idCommitments[0] = idCommitmentSeed + setSize;
-        vm.expectRevert(bytes("RLN, registerBatch: set is full"));
+        vm.expectRevert(FullBatch.selector);
         tempRln.registerBatch{value: MEMBERSHIP_DEPOSIT}(idCommitments);
     }
 
     function test__InvalidBatchRegistration__EmptyBatch() public {
         uint256[] memory idCommitments = new uint256[](0);
-        vm.expectRevert(bytes("RLN, registerBatch: batch size zero"));
+        vm.expectRevert(EmptyBatch.selector);
         rln.registerBatch{value: MEMBERSHIP_DEPOSIT}(idCommitments);
     }
 
@@ -150,12 +154,17 @@ contract RLNTest is Test {
     ) public {
         vm.assume(isUniqueArray(idCommitments) && idCommitments.length > 0);
         uint256 idCommitmentlen = idCommitments.length;
+        uint256 goodDepositAmount = MEMBERSHIP_DEPOSIT * idCommitmentlen;
+        uint256 badDepositAmount = goodDepositAmount - 1;
+
         vm.expectRevert(
-            bytes("RLN, registerBatch: membership deposit is not satisfied")
+            abi.encodeWithSelector(
-        );
+                InsufficientDeposit.selector,
-        rln.registerBatch{value: MEMBERSHIP_DEPOSIT * idCommitmentlen - 1}(
+                goodDepositAmount,
-            idCommitments
+                badDepositAmount
+            )
         );
+        rln.registerBatch{value: badDepositAmount}(idCommitments);
     }
 
     function test__ValidWithdraw(
@@ -184,7 +193,12 @@ contract RLNTest is Test {
         uint256 idCommitment = poseidon.hash(idSecretHash);
         rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
         assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT);
-        vm.expectRevert(bytes("RLN, _withdraw: empty receiver address"));
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                InvalidWithdrawalAddress.selector,
+                address(0)
+            )
+        );
         rln.withdraw(idSecretHash, payable(address(0)));
     }
 
@@ -193,15 +207,23 @@ contract RLNTest is Test {
         uint256 idCommitment = poseidon.hash(idSecretHash);
         rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment);
         assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT);
-        vm.expectRevert(bytes("RLN, _withdraw: cannot withdraw to RLN"));
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                InvalidWithdrawalAddress.selector,
+                address(rln)
+            )
+        );
         rln.withdraw(idSecretHash, payable(address(rln)));
     }
 
     function test__InvalidWithdraw__InvalidIdCommitment(
-        uint256 idCommitment
+        uint256 idSecretHash
     ) public {
-        vm.expectRevert(bytes("RLN, _withdraw: member not registered"));
+        uint256 idCommitment = poseidon.hash(idSecretHash);
-        rln.withdraw(idCommitment, payable(address(this)));
+        vm.expectRevert(
+            abi.encodeWithSelector(MemberNotRegistered.selector, idCommitment)
+        );
+        rln.withdraw(idSecretHash, payable(address(this)));
     }
 
     // this shouldn't be possible, but just in case
@@ -230,7 +252,9 @@ contract RLNTest is Test {
             .depth(0)
             .checked_write(true);
 
-        vm.expectRevert(bytes("RLN, _withdraw: member has no stake"));
+        vm.expectRevert(
+            abi.encodeWithSelector(MemberHasNoStake.selector, idCommitment)
+        );
         rln.withdraw(idSecretHash, to);
     }
 
@@ -274,7 +298,7 @@ contract RLNTest is Test {
     function test__InvalidBatchWithdraw__EmptyBatch() public {
         uint256[] memory idSecretHashes = new uint256[](0);
         address payable[] memory to = new address payable[](0);
-        vm.expectRevert(bytes("RLN, withdrawBatch: batch size zero"));
+        vm.expectRevert(EmptyBatch.selector);
         rln.withdrawBatch(idSecretHashes, to);
     }
 
@@ -287,12 +311,17 @@ contract RLNTest is Test {
         vm.assume(isUniqueArray(idSecretHashes) && idSecretHashes.length > 0);
         vm.assume(to != address(0));
 
+        uint256 numberOfReceivers = idSecretHashes.length + 1;
         vm.expectRevert(
-            bytes("RLN, withdrawBatch: batch size mismatch receivers")
+            abi.encodeWithSelector(
+                MismatchedBatchSize.selector,
+                idSecretHashes.length,
+                numberOfReceivers
+            )
         );
         rln.withdrawBatch(
             idSecretHashes,
-            repeatElementIntoArray(idSecretHashes.length + 1, to)
+            repeatElementIntoArray(numberOfReceivers, to)
         );
     }
 }

test/rln.test.ts

@@ -82,8 +82,6 @@ describe("RLN", () => {
       value: price,
     });
 
-    await expect(registerTx2).to.be.revertedWith(
+    await expect(registerTx2).to.be.revertedWith("DuplicateIdCommitment()");
-      "RLN, _register: member already registered"
-    );
   });
 });