code review

This commit is contained in:
Richard Ramos 2024-10-07 16:13:15 -04:00
parent 6db38e367f
commit 66a1a6c48c
No known key found for this signature in database
GPG Key ID: 1CE87DB518195760
3 changed files with 12 additions and 13 deletions

View File

@ -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 =

View File

@ -57,7 +57,6 @@ contract WakuRlnV2 is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, M
_;
}
constructor() {
_disableInitializers();
}

View File

@ -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);
}