From 18783ba67ee51f70ac5d297da279dbbcef579cae Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Tue, 23 May 2023 12:24:58 +0530 Subject: [PATCH] fix: remove batching ops, include index in removal --- contracts/Rln.sol | 67 +++++------------- docs/index.md | 81 +++++---------------- test/RLN.t.sol | 174 +++------------------------------------------- 3 files changed, 40 insertions(+), 282 deletions(-) diff --git a/contracts/Rln.sol b/contracts/Rln.sol index 1b2b07a..793954c 100644 --- a/contracts/Rln.sol +++ b/contracts/Rln.sol @@ -4,25 +4,17 @@ pragma solidity 0.8.15; import {IPoseidonHasher} from "./PoseidonHasher.sol"; +/// The tree is full +error FullTree(); + /// Invalid deposit amount /// @param required The required deposit amount /// @param provided The provided deposit amount error InsufficientDeposit(uint256 required, uint256 provided); -/// Provided Batch is empty -error EmptyBatch(); - -/// Batch is full, during batch operations -error FullBatch(); - /// Member is already registered error DuplicateIdCommitment(); -/// Batch size mismatch, when the length of secrets and receivers are not equal -/// @param givenSecretsLen The length of the secrets array -/// @param givenReceiversLen The length of the receivers array -error MismatchedBatchSize(uint256 givenSecretsLen, uint256 givenReceiversLen); - /// Invalid receiver address, when the receiver is the contract itself or 0x0 error InvalidReceiverAddress(address to); @@ -49,13 +41,15 @@ contract RLN { uint256 public immutable SET_SIZE; /// @notice The index of the next member to be registered - uint256 public idCommitmentIndex; + uint256 public idCommitmentIndex = 1; /// @notice The amount of eth staked by each member + /// maps from idCommitment to the amount staked mapping(uint256 => uint256) public stakedAmounts; /// @notice The membership status of each member - mapping(uint256 => bool) public members; + /// maps from idCommitment to their index in the set + mapping(uint256 => uint256) public members; /// @notice The balance of each user that can be withdrawn mapping(address => uint256) public withdrawalBalance; @@ -70,7 +64,8 @@ contract RLN { /// Emitted when a member is removed from the set /// @param idCommitment The idCommitment of the member - event MemberWithdrawn(uint256 idCommitment); + /// @param index The index of the member in the set + event MemberWithdrawn(uint256 idCommitment, uint256 index); constructor( uint256 membershipDeposit, @@ -91,51 +86,20 @@ contract RLN { _register(idCommitment, msg.value); } - /// Allows batch registration of members - /// @param idCommitments array of idCommitments - function registerBatch(uint256[] calldata idCommitments) external payable { - uint256 idCommitmentlen = idCommitments.length; - if (idCommitmentlen == 0) revert EmptyBatch(); - if (idCommitmentIndex + idCommitmentlen >= SET_SIZE) revert FullBatch(); - uint256 requiredDeposit = MEMBERSHIP_DEPOSIT * idCommitmentlen; - if (msg.value != requiredDeposit) - revert InsufficientDeposit(requiredDeposit, msg.value); - - for (uint256 i = 0; i < idCommitmentlen; i++) { - _register(idCommitments[i], MEMBERSHIP_DEPOSIT); - } - } - /// Registers a member /// @param idCommitment The idCommitment of the member /// @param stake The amount of eth staked by the member function _register(uint256 idCommitment, uint256 stake) internal { - if (members[idCommitment]) revert DuplicateIdCommitment(); - if (idCommitmentIndex >= SET_SIZE) revert FullBatch(); + if (members[idCommitment] != 0) revert DuplicateIdCommitment(); + if (idCommitmentIndex >= SET_SIZE) revert FullTree(); - members[idCommitment] = true; + members[idCommitment] = idCommitmentIndex; stakedAmounts[idCommitment] = stake; emit MemberRegistered(idCommitment, idCommitmentIndex); idCommitmentIndex += 1; } - /// Allows a user to slash a batch of members - /// @param secrets array of idSecretHashes - /// @param receivers array of addresses to receive the funds - function slashBatch( - uint256[] calldata secrets, - address payable[] calldata receivers - ) external { - uint256 batchSize = secrets.length; - if (batchSize == 0) revert EmptyBatch(); - if (batchSize != receivers.length) - revert MismatchedBatchSize(secrets.length, receivers.length); - for (uint256 i = 0; i < batchSize; i++) { - _slash(secrets[i], receivers[i]); - } - } - /// Allows a user to slash a member /// @param secret The idSecretHash of the member function slash(uint256 secret, address payable receiver) external { @@ -153,20 +117,21 @@ contract RLN { // derive idCommitment uint256 idCommitment = hash(secret); // check if member is registered - if (!members[idCommitment]) revert MemberNotRegistered(idCommitment); + if (members[idCommitment] == 0) revert MemberNotRegistered(idCommitment); if (stakedAmounts[idCommitment] == 0) revert MemberHasNoStake(idCommitment); uint256 amountToTransfer = stakedAmounts[idCommitment]; // delete member - members[idCommitment] = false; + uint256 index = members[idCommitment]; + members[idCommitment] = 0; stakedAmounts[idCommitment] = 0; // refund deposit withdrawalBalance[receiver] += amountToTransfer; - emit MemberWithdrawn(idCommitment); + emit MemberWithdrawn(idCommitment, index); } /// Allows a user to withdraw funds allocated to them upon slashing a member diff --git a/docs/index.md b/docs/index.md index a8566af..639c2a4 100644 --- a/docs/index.md +++ b/docs/index.md @@ -836,6 +836,14 @@ Hashes the input using the Poseidon hash function, n = 2, second input is the co function _hash(uint256 input) internal pure returns (uint256 result) ``` +## FullTree + +```solidity +error FullTree() +``` + +The tree is full + ## InsufficientDeposit ```solidity @@ -851,22 +859,6 @@ Invalid deposit amount | required | uint256 | The required deposit amount | | provided | uint256 | The provided deposit amount | -## EmptyBatch - -```solidity -error EmptyBatch() -``` - -Provided Batch is empty - -## FullBatch - -```solidity -error FullBatch() -``` - -Batch is full, during batch operations - ## DuplicateIdCommitment ```solidity @@ -875,21 +867,6 @@ error DuplicateIdCommitment() Member is already registered -## MismatchedBatchSize - -```solidity -error MismatchedBatchSize(uint256 givenSecretsLen, uint256 givenReceiversLen) -``` - -Batch size mismatch, when the length of secrets and receivers are not equal - -### Parameters - -| Name | Type | Description | -| ----------------- | ------- | --------------------------------- | -| givenSecretsLen | uint256 | The length of the secrets array | -| givenReceiversLen | uint256 | The length of the receivers array | - ## InvalidReceiverAddress ```solidity @@ -971,14 +948,16 @@ mapping(uint256 => uint256) stakedAmounts ``` The amount of eth staked by each member +maps from idCommitment to the amount staked ### members ```solidity -mapping(uint256 => bool) members +mapping(uint256 => uint256) members ``` The membership status of each member +maps from idCommitment to their index in the set ### withdrawalBalance @@ -1014,16 +993,17 @@ Emitted when a new member is added to the set ### MemberWithdrawn ```solidity -event MemberWithdrawn(uint256 idCommitment) +event MemberWithdrawn(uint256 idCommitment, uint256 index) ``` Emitted when a member is removed from the set #### Parameters -| Name | Type | Description | -| ------------ | ------- | ------------------------------ | -| idCommitment | uint256 | The idCommitment of the member | +| Name | Type | Description | +| ------------ | ------- | ---------------------------------- | +| idCommitment | uint256 | The idCommitment of the member | +| index | uint256 | The index of the member in the set | ### constructor @@ -1045,20 +1025,6 @@ Allows a user to register as a member | ------------ | ------- | ------------------------------ | | idCommitment | uint256 | The idCommitment of the member | -### registerBatch - -```solidity -function registerBatch(uint256[] idCommitments) external payable -``` - -Allows batch registration of members - -#### Parameters - -| Name | Type | Description | -| ------------- | --------- | ---------------------- | -| idCommitments | uint256[] | array of idCommitments | - ### \_register ```solidity @@ -1074,21 +1040,6 @@ Registers a member | idCommitment | uint256 | The idCommitment of the member | | stake | uint256 | The amount of eth staked by the member | -### slashBatch - -```solidity -function slashBatch(uint256[] secrets, address payable[] receivers) external -``` - -Allows a user to slash a batch of members - -#### Parameters - -| Name | Type | Description | -| --------- | ----------------- | --------------------------------------- | -| secrets | uint256[] | array of idSecretHashes | -| receivers | address payable[] | array of addresses to receive the funds | - ### slash ```solidity diff --git a/test/RLN.t.sol b/test/RLN.t.sol index b56e8eb..2e9f406 100644 --- a/test/RLN.t.sol +++ b/test/RLN.t.sol @@ -7,31 +7,6 @@ import "forge-std/Test.sol"; import "forge-std/StdCheats.sol"; import "forge-std/console.sol"; -contract ArrayUnique { - mapping(uint256 => bool) seen; - - constructor(uint256[] memory arr) { - for (uint256 i = 0; i < arr.length; i++) { - require(!seen[arr[i]], "ArrayUnique: duplicate value"); - seen[arr[i]] = true; - } - } - - // contract in construction goes around the assumePayable() check - receive() external payable {} -} - -function repeatElementIntoArray( - uint256 length, - address payable element -) pure returns (address payable[] memory) { - address payable[] memory arr = new address payable[](length); - for (uint256 i = 0; i < length; i++) { - arr[i] = element; - } - return arr; -} - contract RLNTest is Test { using stdStorage for StdStorage; @@ -48,14 +23,6 @@ contract RLNTest is Test { rln = new RLN(MEMBERSHIP_DEPOSIT, DEPTH, address(poseidon)); } - function isUniqueArray(uint256[] memory arr) internal returns (bool) { - try new ArrayUnique(arr) { - return true; - } catch { - return false; - } - } - /// @dev Ensure that you can hash a value. function test__Constants() public { assertEq(rln.MEMBERSHIP_DEPOSIT(), MEMBERSHIP_DEPOSIT); @@ -66,7 +33,7 @@ contract RLNTest is Test { function test__ValidRegistration(uint256 idCommitment) public { rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment); assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT); - assertEq(rln.members(idCommitment), true); + assertEq(rln.members(idCommitment), 1); } function test__InvalidRegistration__DuplicateCommitment( @@ -74,7 +41,7 @@ contract RLNTest is Test { ) public { rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment); assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT); - assertEq(rln.members(idCommitment), true); + assertEq(rln.members(idCommitment), 1); vm.expectRevert(DuplicateIdCommitment.selector); rln.register{value: MEMBERSHIP_DEPOSIT}(idCommitment); } @@ -102,71 +69,15 @@ contract RLNTest is Test { 2, address(rln.poseidonHasher()) ); - uint256 setSize = tempRln.SET_SIZE(); + uint256 setSize = tempRln.SET_SIZE() - 1; for (uint256 i = 0; i < setSize; i++) { tempRln.register{value: MEMBERSHIP_DEPOSIT}(idCommitmentSeed + i); } assertEq(tempRln.idCommitmentIndex(), 4); - vm.expectRevert(FullBatch.selector); + vm.expectRevert(FullTree.selector); tempRln.register{value: MEMBERSHIP_DEPOSIT}(idCommitmentSeed + setSize); } - function test__ValidBatchRegistration( - uint256[] calldata idCommitments - ) public { - // assume that the array is unique, otherwise it triggers - // a revert that has already been tested - vm.assume(isUniqueArray(idCommitments) && idCommitments.length > 0); - uint256 idCommitmentlen = idCommitments.length; - rln.registerBatch{value: MEMBERSHIP_DEPOSIT * idCommitmentlen}( - idCommitments - ); - for (uint256 i = 0; i < idCommitmentlen; i++) { - assertEq(rln.stakedAmounts(idCommitments[i]), MEMBERSHIP_DEPOSIT); - assertEq(rln.members(idCommitments[i]), true); - } - } - - function test__InvalidBatchRegistration__FullSet( - uint256 idCommitmentSeed - ) public { - vm.assume(idCommitmentSeed < 2 ** 255 - SET_SIZE); - RLN tempRln = new RLN(MEMBERSHIP_DEPOSIT, 2, address(poseidon)); - uint256 setSize = tempRln.SET_SIZE(); - for (uint256 i = 0; i < setSize; i++) { - tempRln.register{value: MEMBERSHIP_DEPOSIT}(idCommitmentSeed + i); - } - assertEq(tempRln.idCommitmentIndex(), 4); - uint256[] memory idCommitments = new uint256[](1); - idCommitments[0] = idCommitmentSeed + setSize; - vm.expectRevert(FullBatch.selector); - tempRln.registerBatch{value: MEMBERSHIP_DEPOSIT}(idCommitments); - } - - function test__InvalidBatchRegistration__EmptyBatch() public { - uint256[] memory idCommitments = new uint256[](0); - vm.expectRevert(EmptyBatch.selector); - rln.registerBatch{value: MEMBERSHIP_DEPOSIT}(idCommitments); - } - - function test__InvalidBatchRegistration__InsufficientDeposit( - uint256[] calldata idCommitments - ) public { - vm.assume(isUniqueArray(idCommitments) && idCommitments.length > 0); - uint256 idCommitmentlen = idCommitments.length; - uint256 goodDepositAmount = MEMBERSHIP_DEPOSIT * idCommitmentlen; - uint256 badDepositAmount = goodDepositAmount - 1; - - vm.expectRevert( - abi.encodeWithSelector( - InsufficientDeposit.selector, - goodDepositAmount, - badDepositAmount - ) - ); - rln.registerBatch{value: badDepositAmount}(idCommitments); - } - function test__ValidSlash(uint256 idSecretHash, address payable to) public { // avoid precompiles, etc // TODO: wrap both of these in a single function @@ -183,7 +94,7 @@ contract RLNTest is Test { vm.prank(to); rln.withdraw(); assertEq(rln.stakedAmounts(idCommitment), 0); - assertEq(rln.members(idCommitment), false); + assertEq(rln.members(idCommitment), 0); assertEq(to.balance, balanceBefore + MEMBERSHIP_DEPOSIT); } @@ -238,7 +149,7 @@ contract RLNTest is Test { rln.slash(idSecretHash, to); assertEq(rln.stakedAmounts(idCommitment), 0); - assertEq(rln.members(idCommitment), false); + assertEq(rln.members(idCommitment), 0); // manually set members[idCommitment] to true using vm stdstore @@ -254,75 +165,6 @@ contract RLNTest is Test { rln.slash(idSecretHash, to); } - function test__ValidBatchSlash( - uint256[] calldata idSecretHashes, - address payable to - ) public { - // avoid precompiles, etc - assumePayable(to); - assumeNoPrecompiles(to); - vm.assume(isUniqueArray(idSecretHashes) && idSecretHashes.length > 0); - vm.assume(to != address(0)); - uint256 idCommitmentlen = idSecretHashes.length; - uint256[] memory idCommitments = new uint256[](idCommitmentlen); - for (uint256 i = 0; i < idCommitmentlen; i++) { - idCommitments[i] = poseidon.hash(idSecretHashes[i]); - } - - rln.registerBatch{value: MEMBERSHIP_DEPOSIT * idCommitmentlen}( - idCommitments - ); - for (uint256 i = 0; i < idCommitmentlen; i++) { - assertEq(rln.stakedAmounts(idCommitments[i]), MEMBERSHIP_DEPOSIT); - } - - uint256 balanceBefore = to.balance; - rln.slashBatch( - idSecretHashes, - repeatElementIntoArray(idSecretHashes.length, to) - ); - for (uint256 i = 0; i < idCommitmentlen; i++) { - assertEq(rln.stakedAmounts(idCommitments[i]), 0); - assertEq(rln.members(idCommitments[i]), false); - } - vm.prank(to); - rln.withdraw(); - assertEq( - to.balance, - balanceBefore + MEMBERSHIP_DEPOSIT * idCommitmentlen - ); - } - - function test__InvalidBatchSlash__EmptyBatch() public { - uint256[] memory idSecretHashes = new uint256[](0); - address payable[] memory to = new address payable[](0); - vm.expectRevert(EmptyBatch.selector); - rln.slashBatch(idSecretHashes, to); - } - - function test__InvalidBatchSlash__MismatchInputSize( - uint256[] calldata idSecretHashes, - address payable to - ) public { - assumePayable(to); - assumeNoPrecompiles(to); - vm.assume(isUniqueArray(idSecretHashes) && idSecretHashes.length > 0); - vm.assume(to != address(0)); - - uint256 numberOfReceivers = idSecretHashes.length + 1; - vm.expectRevert( - abi.encodeWithSelector( - MismatchedBatchSize.selector, - idSecretHashes.length, - numberOfReceivers - ) - ); - rln.slashBatch( - idSecretHashes, - repeatElementIntoArray(numberOfReceivers, to) - ); - } - function test__InvalidWithdraw__InsufficientWithdrawalBalance() public { vm.expectRevert(InsufficientWithdrawalBalance.selector); rln.withdraw(); @@ -335,7 +177,7 @@ contract RLNTest is Test { assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT); rln.slash(idSecretHash, payable(address(this))); assertEq(rln.stakedAmounts(idCommitment), 0); - assertEq(rln.members(idCommitment), false); + assertEq(rln.members(idCommitment), 0); vm.deal(address(rln), 0); vm.expectRevert(InsufficientContractBalance.selector); @@ -353,7 +195,7 @@ contract RLNTest is Test { assertEq(rln.stakedAmounts(idCommitment), MEMBERSHIP_DEPOSIT); rln.slash(idSecretHash, to); assertEq(rln.stakedAmounts(idCommitment), 0); - assertEq(rln.members(idCommitment), false); + assertEq(rln.members(idCommitment), 0); vm.prank(to); rln.withdraw();