diff --git a/src/Membership.sol b/src/Membership.sol index f6a797f..2eee821 100644 --- a/src/Membership.sol +++ b/src/Membership.sol @@ -57,8 +57,8 @@ abstract contract MembershipUpgradeable is Initializable { /// @notice The index in the membership set for the next membership to be registered uint32 public nextFreeIndex; - /// @notice indices of expired memberships that can be erased (and maybe overwritten) - uint32[] public availableExpiredMembershipsIndices; + /// @notice indices of memberships (expired or grace-period marked for erasure) that can be reused + uint32[] public reusableMembershipsIndices; struct MembershipInfo { /// @notice deposit amount (in tokens) to register this membership @@ -81,7 +81,13 @@ abstract contract MembershipUpgradeable is Initializable { /// @param idCommitment the idCommitment of the membership /// @param membershipRateLimit the rate limit of this membership /// @param index the index of the membership in the membership set - event MembershipExpired(uint256 idCommitment, uint32 membershipRateLimit, uint32 index); + event MembershipExpiredAndErased(uint256 idCommitment, uint32 membershipRateLimit, uint32 index); + + /// @notice Emitted when a membership is erased by its holder during grace period + /// @param idCommitment the idCommitment of the membership + /// @param membershipRateLimit the rate limit of this membership + /// @param index the index of the membership in the membership set + event MembershipErasedByHolder(uint256 idCommitment, uint32 membershipRateLimit, uint32 index); /// @notice Emitted when a membership in its grace period is extended (i.e., is back to Active state) /// @param idCommitment the idCommitment of the membership @@ -228,10 +234,10 @@ abstract contract MembershipUpgradeable is Initializable { /// @return indexReused indicates whether index comes form reusing a slot of an expired membership function _getFreeIndex() internal returns (uint32 index, bool indexReused) { // Reuse available expired memberships - uint256 arrLen = availableExpiredMembershipsIndices.length; + uint256 arrLen = reusableMembershipsIndices.length; if (arrLen != 0) { - index = availableExpiredMembershipsIndices[arrLen - 1]; - availableExpiredMembershipsIndices.pop(); + index = reusableMembershipsIndices[arrLen - 1]; + reusableMembershipsIndices.pop(); indexReused = true; } else { index = nextFreeIndex; @@ -323,12 +329,17 @@ abstract contract MembershipUpgradeable is Initializable { // Deduct the expired membership rate limit totalRateLimit -= _membership.rateLimit; - // Note: the Merkle tree data will be erased lazily when the index is reused - availableExpiredMembershipsIndices.push(_membership.index); + // Note: not all memberships here are expired; some are erased from grace period by their holder + reusableMembershipsIndices.push(_membership.index); + // Note: the Merkle tree data will be erased when the index is reused delete memberships[_idCommitment]; - emit MembershipExpired(_idCommitment, _membership.rateLimit, _membership.index); + if (membershipExpired) { + emit MembershipExpiredAndErased(_idCommitment, _membership.rateLimit, _membership.index); + } else if (membershipIsInGracePeriodAndHeld) { + emit MembershipErasedByHolder(_idCommitment, _membership.rateLimit, _membership.index); + } } /// @dev Withdraw any available deposit balance in tokens after a membership is erased. diff --git a/test/WakuRlnV2.t.sol b/test/WakuRlnV2.t.sol index 24dd49f..5334d32 100644 --- a/test/WakuRlnV2.t.sol +++ b/test/WakuRlnV2.t.sol @@ -339,11 +339,11 @@ contract WakuRlnV2Test is Test { // Attempt to expire 3 commitments that can be erased commitmentsToErase[2] = 4; vm.expectEmit(true, false, false, false); - emit MembershipUpgradeable.MembershipExpired(1, 0, 0); + emit MembershipUpgradeable.MembershipExpiredAndErased(1, 0, 0); vm.expectEmit(true, false, false, false); - emit MembershipUpgradeable.MembershipExpired(2, 0, 0); + emit MembershipUpgradeable.MembershipExpiredAndErased(2, 0, 0); vm.expectEmit(true, false, false, false); - emit MembershipUpgradeable.MembershipExpired(4, 0, 0); + emit MembershipUpgradeable.MembershipExpiredAndErased(4, 0, 0); w.register(6, 60, commitmentsToErase); // Ensure that the chosen memberships were erased and others unaffected @@ -433,28 +433,28 @@ contract WakuRlnV2Test is Test { // Verify that expired indices match what we expect for (uint32 i = 0; i < idCommitmentsLength; i++) { - assertEq(i, w.availableExpiredMembershipsIndices(i)); + assertEq(i, w.reusableMembershipsIndices(i)); } uint32 currnextCommitmentIndex = w.nextFreeIndex(); for (uint256 i = 1; i <= idCommitmentsLength; i++) { uint256 idCommitment = i + 10; uint256 expectedindexReusedPos = idCommitmentsLength - i; - uint32 expectedIndex = w.availableExpiredMembershipsIndices(expectedindexReusedPos); + uint32 expectedIndex = w.reusableMembershipsIndices(expectedindexReusedPos); token.approve(address(w), price); w.register(idCommitment, 20); (,,,, index,,) = w.memberships(idCommitment); assertEq(expectedIndex, index); // Should have been removed from the list vm.expectRevert(); - w.availableExpiredMembershipsIndices(expectedindexReusedPos); + w.reusableMembershipsIndices(expectedindexReusedPos); // Should not have been affected assertEq(currnextCommitmentIndex, w.nextFreeIndex()); } // No indexes should be available for reuse vm.expectRevert(); - w.availableExpiredMembershipsIndices(0); + w.reusableMembershipsIndices(0); // Should use a new index since we got rid of all available indexes token.approve(address(w), price); @@ -498,9 +498,9 @@ contract WakuRlnV2Test is Test { commitmentsToErase[1] = idCommitment + 2; vm.expectEmit(true, false, false, false); // only check the first parameter of the event (the idCommitment) - emit MembershipUpgradeable.MembershipExpired(commitmentsToErase[0], 0, 0); + emit MembershipUpgradeable.MembershipExpiredAndErased(commitmentsToErase[0], 0, 0); vm.expectEmit(true, false, false, false); // only check the first parameter of the event (the idCommitment) - emit MembershipUpgradeable.MembershipExpired(commitmentsToErase[0], 0, 0); + emit MembershipUpgradeable.MembershipExpiredAndErased(commitmentsToErase[0], 0, 0); w.eraseMemberships(commitmentsToErase); address holder; @@ -546,7 +546,7 @@ contract WakuRlnV2Test is Test { 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 MembershipUpgradeable.MembershipExpired(i + 1, 0, 0); + emit MembershipUpgradeable.MembershipExpiredAndErased(i + 1, 0, 0); } w.eraseMemberships(commitmentsToErase);