Skip to content

Commit

Permalink
chore: more test units and fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@richard-ramos
richard-ramos committed Sep 6, 2024
1 parent fe01893 commit 5ddfc35
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 18 deletions.
21 changes: 12 additions & 9 deletions src/Membership.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";
import "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";

Check warning on line 6 in src/Membership.sol

View workflow job for this annotation

GitHub Actions / lint 

global import of path openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol is not allowed. Specify names to import individually or bind all exports of the module into a name (import "path" as Name)
import "openzeppelin-contracts/contracts/utils/Context.sol";

Check warning on line 7 in src/Membership.sol

View workflow job for this annotation

GitHub Actions / lint 

global import of path openzeppelin-contracts/contracts/utils/Context.sol is not allowed. Specify names to import individually or bind all exports of the module into a name (import "path" as Name)

import "forge-std/console.sol";

// The number of periods should be greater than zero
error NumberOfPeriodsCantBeZero();

Expand Down Expand Up @@ -226,19 +224,22 @@ contract Membership {
// Deduct the expired membership rate limit
totalRateLimitPerEpoch -= oldestMembership.userMessageLimit;

// Remove the element from the list
delete members[head];

// Promote the next oldest membership to oldest
uint256 nextOldest = oldestMembership.next;
head = nextOldest;
if (nextOldest != 0) {
members[nextOldest].prev = 0;
} else {
tail = 0;
}

// Move balance from expired membership to holder balance
balancesToWithdraw[oldestMembership.holder][oldestMembership.token] += oldestMembership.amount;

availableExpiredIndices.push(oldestMembership.index);

delete members[head];
} else {
revert ExceedAvailableMaxRateLimitPerEpoch();
}
Expand Down Expand Up @@ -275,7 +276,7 @@ contract Membership {
token: _token,
amount: _amount,
userMessageLimit: _rateLimit,
next: 0, // It's the last value, so point to nowhere
next: 0, // It's the newest value, so point to nowhere
prev: prev,
index: index
});
Expand All @@ -296,7 +297,7 @@ contract Membership {

if (_sender != mdetails.holder) revert NotHolder(_idCommitment);

uint256 newExpirationDate = block.timestamp + (uint256(billingPeriod) * uint256(mdetails.numberOfPeriods));
uint256 newGracePeriodStartDate = block.timestamp + (uint256(billingPeriod) * uint256(mdetails.numberOfPeriods));

Check failure on line 300 in src/Membership.sol

View workflow job for this annotation

GitHub Actions / lint 

Line length must be no more than 120 but current length is 121

uint256 mdetailsNext = mdetails.next;
uint256 mdetailsPrev = mdetails.prev;
Expand All @@ -317,13 +318,13 @@ contract Membership {
// Move membership to the end (since it will be the newest)
mdetails.next = 0;
mdetails.prev = tail;
mdetails.gracePeriodStartDate = newExpirationDate;
mdetails.gracePeriodStartDate = newGracePeriodStartDate;
mdetails.gracePeriod = gracePeriod;

members[tail].next = _idCommitment;
tail = _idCommitment;

emit MemberExtended(_idCommitment, mdetails.userMessageLimit, mdetails.index, newExpirationDate);
emit MemberExtended(_idCommitment, mdetails.userMessageLimit, mdetails.index, newGracePeriodStartDate);
}

/// @dev Determine whether a timestamp is considered to be expired or not after exceeding the grace period
Expand All @@ -349,9 +350,11 @@ contract Membership {
return _isExpired(m.gracePeriodStartDate, m.gracePeriod, m.numberOfPeriods);
}

/// @notice Returns the timestamp on which a membership can be considered expired
/// @param _idCommitment the idCommitment of the membership
function expirationDate(uint256 _idCommitment) public view returns (uint256) {
MembershipInfo memory m = members[_idCommitment];
return m.gracePeriodStartDate + (uint256(m.gracePeriod) * uint256(m.numberOfPeriods));
return m.gracePeriodStartDate + (uint256(m.gracePeriod) * uint256(m.numberOfPeriods)) + 1;
}

/// @dev Determine whether a timestamp is considered to be in grace period or not
Expand Down
91 changes: 82 additions & 9 deletions test/WakuRlnV2.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@ contract WakuRlnV2Test is Test {
w.extend(commitmentsToExtend);

// Attempt to extend the membership (but now we are the owner)
vm.expectEmit(true, false, false, false); // only check the first parameter of the event (the idCommitment)
emit Membership.MemberExtended(idCommitment, 0, 0, 0);
w.extend(commitmentsToExtend);

(,,,, uint256 newGracePeriodStartDate,,,,,) = w.members(idCommitment);
Expand Down Expand Up @@ -343,12 +345,12 @@ contract WakuRlnV2Test is Test {
w.members(idCommitment);

uint256 expectedExpirationDate =
fetchedGracePeriodStartDate + (uint256(fetchedGracePeriod) * uint256(fetchedNumberOfPeriods));
fetchedGracePeriodStartDate + (uint256(fetchedGracePeriod) * uint256(fetchedNumberOfPeriods)) + 1;
uint256 expirationDate = w.expirationDate(idCommitment);

assertEq(expectedExpirationDate, expirationDate);

vm.warp(expirationDate + 1);
vm.warp(expirationDate);

assertFalse(w.isGracePeriod(idCommitment));
assertTrue(w.isExpired(idCommitment));
Expand Down Expand Up @@ -404,20 +406,83 @@ contract WakuRlnV2Test is Test {
}

function test__RegistrationWhenMaxRateLimitIsReachedAndSingleExpiredMemberAvailable() external {
// TODO: implement
// TODO: validate elements are chained correctly
// TODO: validate reuse of index
// TODO: validate that expired event is emitted
// TODO: validate balance
vm.pauseGasMetering();
vm.startPrank(w.owner());
w.setMinRateLimitPerMembership(1);
w.setMaxRateLimitPerMembership(5);
w.setMaxTotalRateLimitPerEpoch(5);
vm.stopPrank();
vm.resumeGasMetering();

uint32 userMessageLimitA = 2;
uint32 totalUserMessageLimit = userMessageLimitA;
(, uint256 priceA) = w.priceCalculator().calculate(userMessageLimitA, 1);
w.register{ value: priceA }(1, userMessageLimitA, 1);

(,,,, uint256 gracePeriodStartDate,,, uint32 indexA,,) = w.members(1);
vm.warp(gracePeriodStartDate + 1);

// Exceeds the rate limit, but if the first were expired, it should register
// It is in grace period so can't be erased
assertTrue(w.isGracePeriod(1));
assertFalse(w.isExpired(1));
uint32 userMessageLimitB = 4;
(, uint256 priceB) = w.priceCalculator().calculate(userMessageLimitB, 1);
(, priceB) = w.priceCalculator().calculate(userMessageLimitB, 1);
vm.expectRevert(abi.encodeWithSelector(ExceedAvailableMaxRateLimitPerEpoch.selector));
w.register{ value: priceB }(2, userMessageLimitB, 1);

// FFW until the membership is expired so we can get rid of it
uint256 expirationDate = w.expirationDate(1);
vm.warp(expirationDate);
assertTrue(w.isExpired(1));

// It should succeed now
vm.expectEmit();
emit Membership.MemberExpired(1, userMessageLimitA, indexA);
w.register{ value: priceB }(2, userMessageLimitB, 1);

// The previous expired membership should have been erased
(,,,,,,,, address holder,) = w.members(1);
assertEq(holder, address(0));

uint32 expectedUserMessageLimit = totalUserMessageLimit - userMessageLimitA + userMessageLimitB;
assertEq(expectedUserMessageLimit, w.totalRateLimitPerEpoch());

// The new commitment should be the only element in the list
assertEq(w.head(), 2);
assertEq(w.tail(), 2);
(uint256 prev, uint256 next,,,,,, uint32 indexB,,) = w.members(2);
assertEq(prev, 0);
assertEq(next, 0);

// Index should have been reused
assertEq(indexA, indexB);

// The balance available for withdrawal should match the amount of the expired membership
uint256 availableBalance = w.balancesToWithdraw(address(this), address(0));
assertEq(availableBalance, priceA);
}

function test__RegistrationWhenMaxRateLimitIsReachedAndMultipleExpiredMembersAvailable() external {
// TODO: implement
// TODO: validate elements are chained correctly
// TODO: validate reuse of index
// TODO: validate balance

// TODO: check that the expired memberships are gone
// TODO: check the usermessage limits (total)
// TODO: check that it reused the index of the one gone
// TODO: check head and tail are correct and next and prev
// TODO: validate that expired event is emitted
// TODO: validate balance
}

function test__RegistrationWhenMaxRateLimitIsReachedAndMultipleExpiredMembersAvailableButTheyDontHaveEnoughRateLimit(

Check failure on line 481 in test/WakuRlnV2.t.sol

View workflow job for this annotation

GitHub Actions / lint 

Line length must be no more than 120 but current length is 121
)
external
{ }

function test__RemoveExpiredMemberships(uint32 userMessageLimit, uint32 numberOfPeriods) external {
vm.pauseGasMetering();
uint256 idCommitment = 2;
Expand All @@ -438,7 +503,7 @@ contract WakuRlnV2Test is Test {

// Expiring the first 3
uint256 expirationDate = w.expirationDate(idCommitment + 2);
vm.warp(expirationDate + 1);
vm.warp(expirationDate);
for (uint256 i = 0; i < 5; i++) {
if (i <= 2) {
assertTrue(w.isExpired(idCommitment + i));
Expand All @@ -450,6 +515,11 @@ contract WakuRlnV2Test is Test {
uint256[] memory commitmentsToErase = new uint256[](2);
commitmentsToErase[0] = idCommitment + 1;
commitmentsToErase[1] = idCommitment + 2;

vm.expectEmit(true, false, false, false); // only check the first parameter of the event (the idCommitment)
emit Membership.MemberExpired(commitmentsToErase[0], 0, 0);
vm.expectEmit(true, false, false, false); // only check the first parameter of the event (the idCommitment)
emit Membership.MemberExpired(commitmentsToErase[0], 0, 0);
w.eraseMemberships(commitmentsToErase);

address holder;
Expand Down Expand Up @@ -501,15 +571,18 @@ contract WakuRlnV2Test is Test {
}

uint256 expirationDate = w.expirationDate(idCommitmentsLength);
vm.warp(expirationDate + 1);
vm.warp(expirationDate);
for (uint256 i = 1; i <= 5; i++) {
assertTrue(w.isExpired(i));
}

uint256[] memory commitmentsToErase = new uint256[](idCommitmentsLength);
for (uint256 i = 0; i < idCommitmentsLength; i++) {
commitmentsToErase[i] = i + 1;
vm.expectEmit(true, false, false, false); // only check the first parameter of the event (the idCommitment)
emit Membership.MemberExpired(i + 1, 0, 0);
}

w.eraseMemberships(commitmentsToErase);

// No memberships registered
Expand Down

0 comments on commit 5ddfc35

Please sign in to comment.