code review

waku-org · Oct 7, 2024 · 66a1a6c · 66a1a6c
1 parent 6db38e3
commit 66a1a6c
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 13 deletions.
diff --git a/src/Membership.sol b/src/Membership.sol
@@ -11,13 +11,13 @@ error InvalidMembershipRateLimit();
 
 // Cannot acquire the rate limit for a new membership due to exceeding the expected limits
 // even after attempting to erase expired memberships
-error ExceededMaxTotalRateLimit();
+error CannotExceedMaxTotalRateLimit();
 
 // This membership is not in its grace period
-error NotInGracePeriod(uint256 idCommitment);
+error CannotExtendActiveMembership(uint256 idCommitment);
 
 // The sender is not the holder of this membership
-error AttemptedExtensionByNonHolder(uint256 idCommitment);
+error NonHolderCannotExtend(uint256 idCommitment);
 
 // This membership cannot be erased
 error CannotEraseMembership(uint256 idCommitment);
@@ -161,7 +161,7 @@ abstract contract MembershipUpgradeable is Initializable {
         gracePeriodDurationForNewMemberships = _gracePeriodDurationForNewMemberships;
     }
 
-    /// @dev acquire a membership and trasnfer the deposit to the contract
+    /// @dev acquire a membership and transfer the deposit to the contract
     /// @param _sender the address of the transaction sender
     /// @param _idCommitment the idCommitment of the new membership
     /// @param _rateLimit the membership rate limit
@@ -184,7 +184,7 @@ abstract contract MembershipUpgradeable is Initializable {
 
         // Determine if we exceed the total rate limit
         if (currentTotalRateLimit > maxTotalRateLimit) {
-            revert ExceededMaxTotalRateLimit();
+            revert CannotExceedMaxTotalRateLimit();
         }
 
         (address token, uint256 depositAmount) = priceCalculator.calculate(_rateLimit);
@@ -235,10 +235,10 @@ abstract contract MembershipUpgradeable is Initializable {
         MembershipInfo storage membership = memberships[_idCommitment];
 
         if (!_isInPeriod(membership.gracePeriodStartTimestamp, membership.gracePeriodDuration)) {
-            revert NotInGracePeriod(_idCommitment);
+            revert CannotExtendActiveMembership(_idCommitment);
         }
 
-        if (_sender != membership.holder) revert AttemptedExtensionByNonHolder(_idCommitment);
+        if (_sender != membership.holder) revert NonHolderCannotExtend(_idCommitment);
 
         // Note: we add the new active period to the end of the ongoing grace period
         uint256 newGracePeriodStartTimestamp =

diff --git a/src/WakuRlnV2.sol b/src/WakuRlnV2.sol
@@ -57,7 +57,6 @@ contract WakuRlnV2 is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, M
         _;
     }
 
-
     constructor() {
         _disableInitializers();
     }

diff --git a/test/WakuRlnV2.t.sol b/test/WakuRlnV2.t.sol
@@ -213,7 +213,7 @@ contract WakuRlnV2Test is Test {
         // Attempt to extend the membership (but it is not owned by us)
         address randomAddress = vm.addr(block.timestamp);
         vm.prank(randomAddress);
-        vm.expectRevert(abi.encodeWithSelector(AttemptedExtensionByNonHolder.selector, commitmentsToExtend[0]));
+        vm.expectRevert(abi.encodeWithSelector(NonHolderCannotExtend.selector, commitmentsToExtend[0]));
         w.extendMemberships(commitmentsToExtend);
 
         // Attempt to extend the membership (but now we are the owner)
@@ -238,7 +238,7 @@ contract WakuRlnV2Test is Test {
         token.approve(address(w), price);
         w.register(idCommitment + 1, membershipRateLimit);
         commitmentsToExtend[0] = idCommitment + 1;
-        vm.expectRevert(abi.encodeWithSelector(NotInGracePeriod.selector, commitmentsToExtend[0]));
+        vm.expectRevert(abi.encodeWithSelector(CannotExtendActiveMembership.selector, commitmentsToExtend[0]));
         w.extendMemberships(commitmentsToExtend);
     }
 
@@ -367,7 +367,7 @@ contract WakuRlnV2Test is Test {
         token.approve(address(w), priceB);
 
         // Should fail. There's not enough free rate limit
-        vm.expectRevert(abi.encodeWithSelector(ExceededMaxTotalRateLimit.selector));
+        vm.expectRevert(abi.encodeWithSelector(CannotExceedMaxTotalRateLimit.selector));
         w.register(6, 60);
 
         // Attempt to erase 3 memberships including one that can't be erased (the last one)
@@ -438,7 +438,7 @@ contract WakuRlnV2Test is Test {
         membershipRateLimit = 2;
         (, price) = w.priceCalculator().calculate(membershipRateLimit);
         token.approve(address(w), price);
-        vm.expectRevert(abi.encodeWithSelector(ExceededMaxTotalRateLimit.selector));
+        vm.expectRevert(abi.encodeWithSelector(CannotExceedMaxTotalRateLimit.selector));
         w.register(3, membershipRateLimit);
 
         // Should register succesfully
@@ -451,7 +451,7 @@ contract WakuRlnV2Test is Test {
         membershipRateLimit = 1;
         (, price) = w.priceCalculator().calculate(membershipRateLimit);
         token.approve(address(w), price);
-        vm.expectRevert(abi.encodeWithSelector(ExceededMaxTotalRateLimit.selector));
+        vm.expectRevert(abi.encodeWithSelector(CannotExceedMaxTotalRateLimit.selector));
         w.register(4, membershipRateLimit);
     }
-Original file line number
+Diff line change
@@ Expand Up @@
             _;
         }
         constructor() {
             _disableInitializers();
         }
@@ Expand Down @@