diff --git a/src/Membership.sol b/src/Membership.sol index ce04c48..ea2769c 100644 --- a/src/Membership.sol +++ b/src/Membership.sol @@ -58,7 +58,7 @@ abstract contract MembershipUpgradeable is Initializable { uint32 public nextFreeIndex; /// @notice indices of memberships (expired or grace-period marked for erasure) that can be reused - uint32[] public reusableIndicesOfErasableMemberships; + uint32[] public reusableIndicesOfErasedMemberships; struct MembershipInfo { /// @notice deposit amount (in tokens) to register this membership @@ -234,10 +234,10 @@ abstract contract MembershipUpgradeable is Initializable { /// @return indexReused indicates whether index comes form reusing a slot of an erased membership function _getFreeIndex() internal returns (uint32 index, bool indexReused) { // Reuse the last membership marked as reusable - uint256 numIndices = reusableIndicesOfErasableMemberships.length; + uint256 numIndices = reusableIndicesOfErasedMemberships.length; if (numIndices != 0) { - index = reusableIndicesOfErasableMemberships[numIndices - 1]; - reusableIndicesOfErasableMemberships.pop(); + index = reusableIndicesOfErasedMemberships[numIndices - 1]; + reusableIndicesOfErasedMemberships.pop(); indexReused = true; } else { index = nextFreeIndex; @@ -315,7 +315,13 @@ abstract contract MembershipUpgradeable is Initializable { /// @dev Erase expired memberships or owned grace-period memberships. /// @param _sender address of the sender of transaction (will be used to check memberships in grace period) /// @param _idCommitment idCommitment of the membership to erase - function _eraseMembership(address _sender, uint256 _idCommitment, MembershipInfo memory _membership) internal { + function _eraseMembershipAndSaveSlotToReuse( + address _sender, + uint256 _idCommitment, + MembershipInfo memory _membership + ) + internal + { bool membershipExpired = _isExpired(_membership.gracePeriodStartTimestamp, _membership.gracePeriodDuration); bool membershipIsInGracePeriodAndHeld = _isInGracePeriod( _membership.gracePeriodStartTimestamp, _membership.gracePeriodDuration @@ -330,7 +336,7 @@ abstract contract MembershipUpgradeable is Initializable { totalRateLimit -= _membership.rateLimit; // Mark this membership as reusable - reusableIndicesOfErasableMemberships.push(_membership.index); + reusableIndicesOfErasedMemberships.push(_membership.index); // Erase this membership from the memberships mapping // Note: the Merkle tree data will be erased when the index is reused diff --git a/src/WakuRlnV2.sol b/src/WakuRlnV2.sol index f8cdef9..70eace6 100644 --- a/src/WakuRlnV2.sol +++ b/src/WakuRlnV2.sol @@ -164,7 +164,7 @@ contract WakuRlnV2 is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, M uint256 idCommitmentToErase = idCommitmentsToErase[i]; MembershipInfo memory membershipToErase = memberships[idCommitmentToErase]; if (membershipToErase.rateLimit == 0) revert InvalidIdCommitment(idCommitmentToErase); - _eraseMembership(_msgSender(), idCommitmentToErase, membershipToErase); + _eraseMembershipAndSaveSlotToReuse(_msgSender(), idCommitmentToErase, membershipToErase); LazyIMT.update(merkleTree, 0, membershipToErase.index); } @@ -251,7 +251,7 @@ contract WakuRlnV2 is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, M uint256 idCommitmentToErase = idCommitments[i]; MembershipInfo memory membershipToErase = memberships[idCommitmentToErase]; if (membershipToErase.rateLimit == 0) revert InvalidIdCommitment(idCommitmentToErase); - _eraseMembership(_msgSender(), idCommitmentToErase, membershipToErase); + _eraseMembershipAndSaveSlotToReuse(_msgSender(), idCommitmentToErase, membershipToErase); LazyIMT.update(merkleTree, 0, membershipToErase.index); } } diff --git a/test/WakuRlnV2.t.sol b/test/WakuRlnV2.t.sol index 34f577a..71b205e 100644 --- a/test/WakuRlnV2.t.sol +++ b/test/WakuRlnV2.t.sol @@ -59,7 +59,8 @@ contract WakuRlnV2Test is Test { w.root(), 13_801_897_483_540_040_307_162_267_952_866_411_686_127_372_014_953_358_983_481_592_640_000_001_877_295 ); - (uint32 fetchedMembershipRateLimit2, uint32 index2, uint256 rateCommitment2) = w.getMembershipInfo(idCommitment); + (uint32 fetchedMembershipRateLimit2, uint32 index2, uint256 rateCommitment2) = + w.getMembershipInfo(idCommitment); assertEq(fetchedMembershipRateLimit2, membershipRateLimit); assertEq(index2, 0); assertEq(rateCommitment2, rateCommitment); @@ -433,28 +434,28 @@ contract WakuRlnV2Test is Test { // Verify that expired indices match what we expect for (uint32 i = 0; i < idCommitmentsLength; i++) { - assertEq(i, w.reusableIndicesOfErasableMemberships(i)); + assertEq(i, w.reusableIndicesOfErasedMemberships(i)); } uint32 currnextCommitmentIndex = w.nextFreeIndex(); for (uint256 i = 1; i <= idCommitmentsLength; i++) { uint256 idCommitment = i + 10; uint256 expectedindexReusedPos = idCommitmentsLength - i; - uint32 expectedIndex = w.reusableIndicesOfErasableMemberships(expectedindexReusedPos); + uint32 expectedIndex = w.reusableIndicesOfErasedMemberships(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.reusableIndicesOfErasableMemberships(expectedindexReusedPos); + w.reusableIndicesOfErasedMemberships(expectedindexReusedPos); // Should not have been affected assertEq(currnextCommitmentIndex, w.nextFreeIndex()); } // No indexes should be available for reuse vm.expectRevert(); - w.reusableIndicesOfErasableMemberships(0); + w.reusableIndicesOfErasedMemberships(0); // Should use a new index since we got rid of all available indexes token.approve(address(w), price);