logos-messaging/logos-messaging-rlnv2-contract
1
0
Fork 0
mirror of https://github.com/logos-messaging/logos-messaging-rlnv2-contract.git synced 2026-04-21 19:13:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: off-by-one: end index can't be equal to next free index

Browse Source
This commit is contained in:
Sergei Tikhomirov 2024-10-02 17:13:43 +02:00
parent 2e5db4f8db
commit 0d2ee0a403
No known key found for this signature in database
GPG Key ID: 6A1F8ED9D6538027
2 changed files with 21 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/WakuRlnV2.sol
View File

@ -37,21 +37,21 @@ contract WakuRlnV2 is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, M
/// @notice The Merkle tree that stores rate commitments of memberships
LazyIMTData public merkleTree;
/// @notice the modifier to check if the idCommitment is valid
/// @notice Сheck if the idCommitment is valid
/// @param idCommitment The idCommitment of the membership
modifier onlyValidIdCommitment(uint256 idCommitment) {
if (!isValidIdCommitment(idCommitment)) revert InvalidIdCommitment(idCommitment);
_;
}
/// @notice the modifier to check that the membership with this idCommitment doesn't already exist
/// @notice Сheck that the membership with this idCommitment is not already in the membership set
/// @param idCommitment The idCommitment of the membership
modifier noDuplicateMembership(uint256 idCommitment) {
require(!isInMembershipSet(idCommitment), "Duplicate idCommitment: membership already exists");
_;
}
/// @notice the modifier to check that the membership set is not full
/// @notice Check that the membership set is not full
modifier membershipSetNotFull() {
require(nextFreeIndex < MAX_MEMBERSHIP_SET_SIZE, "Membership set is full");
_;
@ -61,7 +61,7 @@ contract WakuRlnV2 is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, M
_disableInitializers();
}
/// @dev contract initializer
/// @dev Contract initializer
/// @param _priceCalculator Address of an instance of IPriceCalculator
/// @param _maxTotalRateLimit Maximum total rate limit of all memberships in the membership set
/// @param _minMembershipRateLimit Minimum rate limit of one membership
@ -98,11 +98,11 @@ contract WakuRlnV2 is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, M
function _authorizeUpgrade(address newImplementation) internal override onlyOwner { } // solhint-disable-line
/// @notice Checks if an idCommitment is valid
/// @notice Checks if an idCommitment is valid (between 0 and Q, both exclusive)
/// @param idCommitment The idCommitment of the membership
/// @return true if the idCommitment is valid, false otherwise
function isValidIdCommitment(uint256 idCommitment) public pure returns (bool) {
return idCommitment != 0 && idCommitment < Q;
return 0 < idCommitment && idCommitment < Q;
}
/// @notice Checks if a membership is in the membership set
@ -129,9 +129,16 @@ contract WakuRlnV2 is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, M
/// @param startIndex The start index of the range (inclusive)
/// @param endIndex The end index of the range (inclusive)
/// @return The rateCommitments of the memberships
function getRateCommitmentsInRange(uint32 startIndex, uint32 endIndex) public view returns (uint256[] memory) {
function getRateCommitmentsInRangeBoundsInclusive(
uint32 startIndex,
uint32 endIndex
)
public
view
returns (uint256[] memory)
{
if (startIndex > endIndex) revert InvalidPaginationQuery(startIndex, endIndex);
if (endIndex > nextFreeIndex) revert InvalidPaginationQuery(startIndex, endIndex); //FIXME: should it be >=?
if (endIndex >= nextFreeIndex) revert InvalidPaginationQuery(startIndex, endIndex);
uint256[] memory rateCommitments = new uint256[](endIndex - startIndex + 1);
for (uint32 i = startIndex; i <= endIndex; i++) {

12
test/WakuRlnV2.t.sol
View File

@ -659,12 +659,12 @@ contract WakuRlnV2Test is Test {
function test__InvalidPaginationQuery__StartIndexGTEndIndex() external {
vm.expectRevert(abi.encodeWithSelector(InvalidPaginationQuery.selector, 1, 0));
w.getRateCommitmentsInRange(1, 0);
w.getRateCommitmentsInRangeBoundsInclusive(1, 0);
}
function test__InvalidPaginationQuery__EndIndexGTNextFreeIndex() external {
vm.expectRevert(abi.encodeWithSelector(InvalidPaginationQuery.selector, 0, 2));
w.getRateCommitmentsInRange(0, 2); // FIXME: > or >=?
w.getRateCommitmentsInRangeBoundsInclusive(0, 2);
}
function test__ValidPaginationQuery__OneElement() external {
@ -676,7 +676,7 @@ contract WakuRlnV2Test is Test {
token.approve(address(w), price);
w.register(idCommitment, membershipRateLimit);
uint256[] memory commitments = w.getRateCommitmentsInRange(0, 0);
uint256[] memory commitments = w.getRateCommitmentsInRangeBoundsInclusive(0, 0);
assertEq(commitments.length, 1);
uint256 rateCommitment = PoseidonT3.hash([idCommitment, membershipRateLimit]);
assertEq(commitments[0], rateCommitment);
@ -688,14 +688,14 @@ contract WakuRlnV2Test is Test {
uint32 membershipRateLimit = w.minMembershipRateLimit();
(, uint256 price) = w.priceCalculator().calculate(membershipRateLimit);
for (uint256 i = 0; i < idCommitmentsLength; i++) {
for (uint256 i = 0; i <= idCommitmentsLength; i++) {
token.approve(address(w), price);
w.register(i + 1, membershipRateLimit);
}
vm.resumeGasMetering();
uint256[] memory rateCommitments = w.getRateCommitmentsInRange(0, idCommitmentsLength);
assertEq(rateCommitments.length, idCommitmentsLength + 1); // FIXME: check for off-by-one error here
uint256[] memory rateCommitments = w.getRateCommitmentsInRangeBoundsInclusive(0, idCommitmentsLength - 1);
assertEq(rateCommitments.length, idCommitmentsLength);
for (uint256 i = 0; i < idCommitmentsLength; i++) {
uint256 rateCommitment = PoseidonT3.hash([i + 1, membershipRateLimit]);
assertEq(rateCommitments[i], rateCommitment);