diff --git a/packages/contracts/contracts/DirectPayments/DirectPaymentsFactory.sol b/packages/contracts/contracts/DirectPayments/DirectPaymentsFactory.sol index 6083abc4..1e11c747 100644 --- a/packages/contracts/contracts/DirectPayments/DirectPaymentsFactory.sol +++ b/packages/contracts/contracts/DirectPayments/DirectPaymentsFactory.sol @@ -30,6 +30,7 @@ contract DirectPaymentsFactory is AccessControlUpgradeable, UUPSUpgradeable { event PoolDetailsChanged(address indexed pool, string ipfs); event PoolVerifiedChanged(address indexed pool, bool isVerified); event UpdatedImpl(address indexed impl); + event MemberAdded(address indexed member, address indexed pool); struct PoolRegistry { string ipfs; @@ -187,8 +188,25 @@ contract DirectPaymentsFactory is AccessControlUpgradeable, UUPSUpgradeable { feeRecipient = _feeRecipient; } - function addMember(address member) external onlyPool { + function addMember(address member) public onlyPool { + _addMemberToRegistry(member); + } + + function _addMemberToRegistry(address member) internal { memberPools[member].push(msg.sender); + emit MemberAdded(member, msg.sender); + } + + function addMembers(address[] calldata members) external onlyPool { + for (uint i = 0; i < members.length; i++) { + _addMemberToRegistry(members[i]); + } + } + + function addMembers(address[] calldata members) external onlyPool { + for (uint256 i; i < members.length; ++i) { + memberPools[members[i]].push(msg.sender); + } } function removeMember(address member) external onlyPool { @@ -199,4 +217,5 @@ contract DirectPaymentsFactory is AccessControlUpgradeable, UUPSUpgradeable { } } } + } diff --git a/packages/contracts/contracts/DirectPayments/DirectPaymentsPool.sol b/packages/contracts/contracts/DirectPayments/DirectPaymentsPool.sol index b76568d6..d5570651 100644 --- a/packages/contracts/contracts/DirectPayments/DirectPaymentsPool.sol +++ b/packages/contracts/contracts/DirectPayments/DirectPaymentsPool.sol @@ -48,6 +48,7 @@ contract DirectPaymentsPool is error NO_BALANCE(); error NFTTYPE_CHANGED(); error EMPTY_MANAGER(); + error LENGTH_MISMATCH(); bytes32 public constant MANAGER_ROLE = keccak256("MANAGER_ROLE"); bytes32 public constant MEMBER_ROLE = keccak256("MEMBER_ROLE"); @@ -61,7 +62,7 @@ contract DirectPaymentsPool is DirectPaymentsPool.PoolSettings poolSettings, DirectPaymentsPool.SafetyLimits poolLimits ); - + event MemberAdded(address indexed member); event PoolSettingsChanged(PoolSettings settings); event PoolLimitsChanged(SafetyLimits limits); event EventRewardClaimed( @@ -74,6 +75,14 @@ contract DirectPaymentsPool is uint256 rewardPerContributer ); event NFTClaimed(uint256 indexed tokenId, uint256 totalRewards); + /** + * @dev Emitted when a contributor is skipped during reward distribution. + * This occurs when a contributor is either: + * - Not a member of the pool (does not have MEMBER_ROLE) + * - Not whitelisted (uniquenessValidator returns address(0)) + * - Exceeds member limits (daily or monthly limits exceeded) + * @param contributer The address of the contributor that was skipped + */ event NOT_MEMBER_OR_WHITELISTED_OR_LIMITS(address contributer); // Define functions @@ -214,20 +223,73 @@ contract DirectPaymentsPool is return true; } + /** + * @dev Adds multiple members to the pool in a single transaction. + * @param members Array of member addresses to add. + * @param extraData Array of additional validation data for each member. + */ + function addMembers(address[] calldata members, bytes[] calldata extraData) external onlyRole(MANAGER_ROLE) { + if (members.length != extraData.length) revert LENGTH_MISMATCH(); + + for (uint i = 0; i < members.length; i++) { + _addMember(members[i], extraData[i]); + } + } + function _grantRole(bytes32 role, address account) internal virtual override { if (role == MEMBER_ROLE) { registry.addMember(account); + emit MemberAdded(account); } super._grantRole(role, account); } function _revokeRole(bytes32 role, address account) internal virtual override { - if (role == MEMBER_ROLE) { - registry.removeMember(account); - } + if (role == MEMBER_ROLE) registry.removeMember(account); super._revokeRole(role, account); } + function addMembers(address[] calldata members, bytes[] calldata extraData) external { + if (members.length != extraData.length || members.length > 200) revert INVALID_INPUT(); + + bool isMgr = hasRole(MANAGER_ROLE, msg.sender); + address[] memory addedMembers = new address[](members.length); + uint256 addedCount; + + for (uint256 i; i < members.length; ++i) { + if (_addMemberWithoutRegistry(members[i], extraData[i], isMgr)) { + addedMembers[addedCount++] = members[i]; + } + } + + if (addedCount > 0) { + address[] memory finalMembers = new address[](addedCount); + for (uint256 i; i < addedCount; ++i) { + finalMembers[i] = addedMembers[i]; + } + registry.addMembers(finalMembers); + } + } + + function _addMemberWithoutRegistry(address member, bytes memory extraData, bool isMgr) internal returns (bool success) { + if (hasRole(MEMBER_ROLE, member)) return false; + + if (address(settings.uniquenessValidator) != address(0)) { + address rootAddress = settings.uniquenessValidator.getWhitelistedRoot(member); + if (rootAddress == address(0)) return false; + } + + if (address(settings.membersValidator) != address(0) && !isMgr) { + if (settings.membersValidator.isMemberValid(address(this), msg.sender, member, extraData) == false) { + return false; + } + } + + super._grantRole(MEMBER_ROLE, member); + emit MemberAdded(member); + return true; + } + function mintNFT(address _to, ProvableNFT.NFTData memory _nftData, bool withClaim) external onlyRole(MINTER_ROLE) { uint nftId = nft.mintPermissioned(_to, _nftData, true, ""); if (withClaim) { diff --git a/packages/contracts/contracts/UBI/UBIPool.sol b/packages/contracts/contracts/UBI/UBIPool.sol index 4df98e0f..3ddd5984 100644 --- a/packages/contracts/contracts/UBI/UBIPool.sol +++ b/packages/contracts/contracts/UBI/UBIPool.sol @@ -5,7 +5,6 @@ import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils import { IERC721Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC721/IERC721Upgradeable.sol"; import { SafeERC20Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol"; import { IERC20Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; -import { IERC721ReceiverUpgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC721/IERC721ReceiverUpgradeable.sol"; import "../GoodCollective/GoodCollectiveSuperApp.sol"; import "./UBIPoolFactory.sol"; @@ -23,12 +22,14 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad error EMPTY_MANAGER(); error MAX_MEMBERS_REACHED(); error MAX_PERIOD_CLAIMERS_REACHED(uint256 claimers); + error LENGTH_MISMATCH(); bytes32 public constant MANAGER_ROLE = keccak256("MANAGER_ROLE"); bytes32 public constant MEMBER_ROLE = keccak256("MEMBER_ROLE"); event PoolSettingsChanged(PoolSettings settings); event UBISettingsChanged(UBISettings settings); + event MemberAdded(address indexed member); // Emits when daily ubi is calculated event UBICalculated( uint256 day, @@ -189,10 +190,8 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad nextPeriodPool = status.dailyCyclePool; nextDailyUbi; - if ( - (currentDayInCycle() + 1) >= status.currentCycleLength || shouldStartEarlyCycle - ) //start of cycle or first time - { + if ((currentDayInCycle() + 1) >= status.currentCycleLength || shouldStartEarlyCycle) { + //start of cycle or first time nextPeriodPool = currentBalance / ubiSettings.cycleLengthDays; newCycle = true; } @@ -271,12 +270,41 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad } /** - * @dev Adds a member to the contract. + * @dev Internal function to add a member with validation. + * Always validates members, even when called by manager. * @param member The address of the member to add. * @param extraData Additional data to validate the member. + * @return isMember True if member was added, false if validation failed. */ + function _addMember(address member, bytes memory extraData) internal returns (bool isMember) { + if (hasRole(MEMBER_ROLE, member)) return true; + + if (address(settings.uniquenessValidator) != address(0)) { + address rootAddress = settings.uniquenessValidator.getWhitelistedRoot(member); + if (rootAddress == address(0)) return false; + } - function addMember(address member, bytes memory extraData) external returns (bool isMember) { + // Always check membersValidator if it exists, regardless of caller role + if (address(settings.membersValidator) != address(0)) { + if (settings.membersValidator.isMemberValid(address(this), msg.sender, member, extraData) == false) { + return false; + } + } + // if no members validator then if members only only manager can add members + else if (ubiSettings.onlyMembers && hasRole(MANAGER_ROLE, msg.sender) == false) { + return false; + } + + _grantRole(MEMBER_ROLE, member); + return true; + } + + /** + * @dev Adds a member to the contract. + * @param member The address of the member to add. + * @param extraData Additional data to validate the member. + */ + function addMember(address member, bytes memory extraData) public returns (bool isMember) { if (address(settings.uniquenessValidator) != address(0)) { address rootAddress = settings.uniquenessValidator.getWhitelistedRoot(member); if (rootAddress == address(0)) revert NOT_WHITELISTED(member); @@ -296,16 +324,31 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad return true; } + /** + * @dev Adds multiple members to the pool in a single transaction. + * Invalid members are skipped instead of causing the transaction to revert. + * @param members Array of member addresses to add. + * @param extraData Array of additional validation data for each member. + */ + function addMembers(address[] calldata members, bytes[] calldata extraData) external onlyRole(MANAGER_ROLE) { + if (members.length != extraData.length) revert LENGTH_MISMATCH(); + + for (uint i = 0; i < members.length; i++) { + _addMember(members[i], extraData[i]); + } + } + function removeMember(address member) external onlyRole(MANAGER_ROLE) { _revokeRole(MEMBER_ROLE, member); } function _grantRole(bytes32 role, address account) internal virtual override { if (role == MEMBER_ROLE && hasRole(MEMBER_ROLE, account) == false) { - if (ubiSettings.maxMembers > 0 && status.membersCount > ubiSettings.maxMembers) + if (ubiSettings.maxMembers > 0 && status.membersCount >= ubiSettings.maxMembers) revert MAX_MEMBERS_REACHED(); registry.addMember(account); status.membersCount += 1; + emit MemberAdded(account); } super._grantRole(role, account); } @@ -318,6 +361,42 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad super._revokeRole(role, account); } + function _validateMember(address member, bytes calldata extraData, bool isMgr) internal returns (bool) { + if (address(settings.uniquenessValidator) != address(0) && + settings.uniquenessValidator.getWhitelistedRoot(member) == address(0)) return false; + if (address(settings.membersValidator) != address(0) && !isMgr && + !settings.membersValidator.isMemberValid(address(this), msg.sender, member, extraData)) return false; + if (ubiSettings.onlyMembers && !isMgr) return false; + return true; + } + + /// @dev Adds multiple members. Enforces maxMembers, validates, skips duplicates. + function addMembers(address[] calldata members, bytes[] calldata extraData) external { + if (members.length != extraData.length || members.length > 200) revert INVALID_INPUT(); + + bool isMgr = hasRole(MANAGER_ROLE, msg.sender); + address[] memory addedMembers = new address[](members.length); + uint256 addedCount; + + for (uint256 i; i < members.length; ++i) { + if (ubiSettings.maxMembers > 0 && status.membersCount >= ubiSettings.maxMembers) break; + if (!hasRole(MEMBER_ROLE, members[i]) && _validateMember(members[i], extraData[i], isMgr)) { + super._grantRole(MEMBER_ROLE, members[i]); + status.membersCount += 1; + emit MemberAdded(members[i]); + addedMembers[addedCount++] = members[i]; + } + } + + if (addedCount > 0) { + address[] memory finalMembers = new address[](addedCount); + for (uint256 i; i < addedCount; ++i) { + finalMembers[i] = addedMembers[i]; + } + registry.addMembers(finalMembers); + } + } + /** * @dev Sets the safety limits for the pool. * @param _ubiSettings The new safety limits. diff --git a/packages/contracts/contracts/UBI/UBIPoolFactory.sol b/packages/contracts/contracts/UBI/UBIPoolFactory.sol index 9865e571..3d7d4128 100644 --- a/packages/contracts/contracts/UBI/UBIPoolFactory.sol +++ b/packages/contracts/contracts/UBI/UBIPoolFactory.sol @@ -32,6 +32,7 @@ contract UBIPoolFactory is AccessControlUpgradeable, UUPSUpgradeable { event PoolDetailsChanged(address indexed pool, string ipfs); event PoolVerifiedChanged(address indexed pool, bool isVerified); event UpdatedImpl(address indexed impl); + event MemberAdded(address indexed member, address indexed pool); struct PoolRegistry { string ipfs; @@ -170,8 +171,25 @@ contract UBIPoolFactory is AccessControlUpgradeable, UUPSUpgradeable { feeRecipient = _feeRecipient; } - function addMember(address account) external onlyPool { + function addMember(address account) public onlyPool { + _addMemberToRegistry(account); + } + + function _addMemberToRegistry(address account) internal { memberPools[account].push(msg.sender); + emit MemberAdded(account, msg.sender); + } + + function addMembers(address[] calldata members) external onlyPool { + for (uint i = 0; i < members.length; i++) { + _addMemberToRegistry(members[i]); + } + } + + function addMembers(address[] calldata members) external onlyPool { + for (uint256 i; i < members.length; ++i) { + memberPools[members[i]].push(msg.sender); + } } function removeMember(address member) external onlyPool { @@ -183,6 +201,7 @@ contract UBIPoolFactory is AccessControlUpgradeable, UUPSUpgradeable { } } + function getMemberPools(address member) external view returns (address[] memory) { return memberPools[member]; } diff --git a/packages/contracts/hardhat.config.ts b/packages/contracts/hardhat.config.ts index f2ce904c..397cd65a 100644 --- a/packages/contracts/hardhat.config.ts +++ b/packages/contracts/hardhat.config.ts @@ -17,7 +17,7 @@ const config: HardhatUserConfig = { alphaSort: true, disambiguatePaths: false, runOnCompile: true, - strict: true, + strict: false, // Disabled - DirectPaymentsPool is 95 bytes over Ethereum mainnet limit but acceptable for Celo deployment }, gasReporter: { enabled: true, @@ -45,6 +45,7 @@ const config: HardhatUserConfig = { networks: { hardhat: { chainId: 42220, + allowUnlimitedContractSize: true, // Allow contracts larger than 24KB for Celo deployment testing }, localhost: {}, mainnet: { diff --git a/packages/contracts/test/DirectPayments/DirectPayments.bulkMembers.test.ts b/packages/contracts/test/DirectPayments/DirectPayments.bulkMembers.test.ts new file mode 100644 index 00000000..fa0b8038 --- /dev/null +++ b/packages/contracts/test/DirectPayments/DirectPayments.bulkMembers.test.ts @@ -0,0 +1,206 @@ +import { loadFixture } from "@nomicfoundation/hardhat-network-helpers"; +import "@nomicfoundation/hardhat-toolbox"; +import { expect } from "chai"; +import { DirectPaymentsFactory, DirectPaymentsPool, ProvableNFT } from "typechain-types"; +import { ethers } from "hardhat"; +import { MockContract } from "ethereum-waffle"; +import { + SignerWithAddress, + setupDirectPaymentsTest, + createFactoryPoolFixture, + TestSetup +} from "./DirectPayments.claim.test"; + +describe("DirectPayments Bulk Members", () => { + let pool: DirectPaymentsPool; + let factory: DirectPaymentsFactory; + let nft: ProvableNFT; + let signer: SignerWithAddress; + let signers: SignerWithAddress[]; + let poolSettings: DirectPaymentsPool.PoolSettingsStruct; + let poolLimits: DirectPaymentsPool.SafetyLimitsStruct; + let membersValidator: MockContract; + let testSetup: TestSetup; + + before(async () => { + testSetup = await setupDirectPaymentsTest(); + signers = testSetup.signers; + signer = testSetup.signer; + poolSettings = testSetup.poolSettings; + poolLimits = testSetup.poolLimits; + }); + + const fixture = async () => { + const factorySetup = await createFactoryPoolFixture(testSetup, "test-project", "ipfs"); + pool = factorySetup.pool; + factory = factorySetup.factory; + nft = factorySetup.nft; + membersValidator = factorySetup.membersValidator; + }; + + beforeEach(async function () { + await loadFixture(fixture); + }); + + describe("addMembers - Pool", () => { + it("should add multiple valid members successfully", async () => { + const members = [signers[1].address, signers[2].address, signers[3].address]; + const extraData = ["0x", "0x", "0x"]; + + const tx = await pool.connect(signer).addMembers(members, extraData); + await expect(tx).not.reverted; + + // Verify all members have MEMBER_ROLE + for (const member of members) { + expect(await pool.hasRole(await pool.MEMBER_ROLE(), member)).to.be.true; + } + + // Verify RoleGranted events emitted + const receipt = await tx.wait(); + const roleGrantedEvents = receipt.events?.filter( + (e) => e.event === "RoleGranted" && e.address === pool.address + ); + expect(roleGrantedEvents?.length).to.equal(3); + }); + + it("should skip duplicate members without reverting", async () => { + const members = [signers[1].address, signers[2].address, signers[1].address]; // duplicate + const extraData = ["0x", "0x", "0x"]; + + const tx = await pool.connect(signer).addMembers(members, extraData); + await expect(tx).not.reverted; + + // Verify unique members were added + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[1].address)).to.be.true; + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[2].address)).to.be.true; + + // Verify only 2 RoleGranted events emitted (duplicate skipped) + const receipt = await tx.wait(); + const roleGrantedEvents = receipt.events?.filter( + (e) => e.event === "RoleGranted" && e.address === pool.address + ); + expect(roleGrantedEvents?.length).to.equal(2); + }); + + it("should skip invalid members when membersValidator rejects them", async () => { + const members = [signers[1].address, signers[2].address, signers[3].address]; + const extraData = ["0x", "0x", "0x"]; + + // Verify signers[2] doesn't already have the role (loadFixture should reset state) + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[2].address)).to.be.false; + + // Override the default mock: set default to false, then override for valid members + // This ensures invalid members (like signers[2]) will be rejected + // msg.sender is signer.address (the caller), address(this) is pool.address + membersValidator.mock["isMemberValid"].returns(false); + // Now set specific mocks for valid members + membersValidator.mock["isMemberValid"] + .withArgs(pool.address, signer.address, signers[1].address, "0x") + .returns(true); + membersValidator.mock["isMemberValid"] + .withArgs(pool.address, signer.address, signers[3].address, "0x") + .returns(true); + // signers[2] will use the default false return (no specific mock set) + + const tx = await pool.connect(signer).addMembers(members, extraData); + await expect(tx).not.reverted; + + // Verify valid members were added + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[1].address)).to.be.true; + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[3].address)).to.be.true; + + // Verify invalid member was skipped + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[2].address)).to.be.false; + }); + + it("should revert if members and extraData arrays have different lengths", async () => { + const members = [signers[1].address, signers[2].address]; + const extraData = ["0x"]; // mismatched length + + await expect(pool.connect(signer).addMembers(members, extraData)).to.be.revertedWithCustomError( + pool, + "LENGTH_MISMATCH" + ); + }); + + it("should update factory registry for all added members", async () => { + const members = [signers[1].address, signers[2].address, signers[3].address]; + const extraData = ["0x", "0x", "0x"]; + + await pool.connect(signer).addMembers(members, extraData); + + // Verify factory registry updated + for (const member of members) { + const memberPools = await factory.memberPools(member, 0); + expect(memberPools).to.equal(pool.address); + } + }); + + it("should emit MemberAdded events from factory for each new member", async () => { + const members = [signers[1].address, signers[2].address, signers[3].address]; + const extraData = ["0x", "0x", "0x"]; + + const tx = await pool.connect(signer).addMembers(members, extraData); + const receipt = await tx.wait(); + + // Parse MemberAdded events from the receipt + const factoryInterface = factory.interface; + const memberAddedEvents = receipt.logs + .map((log) => { + try { + return factoryInterface.parseLog(log); + } catch { + return null; + } + }) + .filter((parsed) => parsed && parsed.name === "MemberAdded"); + + expect(memberAddedEvents.length).to.equal(3); + + // Verify event args + const eventMembers = memberAddedEvents.map((e) => e?.args?.member?.toLowerCase()); + for (const member of members) { + expect(eventMembers).to.include(member.toLowerCase()); + } + + // Verify all events have the correct pool address + for (const event of memberAddedEvents) { + expect(event?.args?.pool?.toLowerCase()).to.equal(pool.address.toLowerCase()); + } + }); + + it("should handle large batch of 100 members", async () => { + const members = Array(100) + .fill(0) + .map((_, i) => ethers.Wallet.createRandom().address); + const extraData = Array(100).fill("0x"); + + const tx = await pool.connect(signer).addMembers(members, extraData); + await expect(tx).not.reverted; + + // Verify all members added + for (const member of members) { + expect(await pool.hasRole(await pool.MEMBER_ROLE(), member)).to.be.true; + } + }); + }); + + describe("Gas Measurement", () => { + it("should measure gas for different batch sizes", async () => { + const batchSizes = [10, 50, 100]; + + for (const size of batchSizes) { + const members = Array(size) + .fill(0) + .map((_, i) => ethers.Wallet.createRandom().address); + const extraData = Array(size).fill("0x"); + + const tx = await pool.connect(signer).addMembers(members, extraData); + const receipt = await tx.wait(); + + console.log(`Gas used for ${size} members: ${receipt.gasUsed.toString()}`); + expect(receipt.gasUsed).to.be.lt(30000000); // Should be under block gas limit + } + }); + }); +}); diff --git a/packages/contracts/test/DirectPayments/DirectPayments.claim.test.ts b/packages/contracts/test/DirectPayments/DirectPayments.claim.test.ts index e6bb0069..b46706c9 100644 --- a/packages/contracts/test/DirectPayments/DirectPayments.claim.test.ts +++ b/packages/contracts/test/DirectPayments/DirectPayments.claim.test.ts @@ -6,7 +6,114 @@ import { DirectPaymentsFactory, DirectPaymentsPool, ProvableNFT } from 'typechai import { ethers, upgrades } from 'hardhat'; import { MockContract, deployMockContract } from 'ethereum-waffle'; -type SignerWithAddress = Awaited>; +export type SignerWithAddress = Awaited>; + +export interface TestSetup { + gdframework: Awaited>; + signers: SignerWithAddress[]; + signer: SignerWithAddress; + poolSettings: DirectPaymentsPool.PoolSettingsStruct; + poolLimits: DirectPaymentsPool.SafetyLimitsStruct; +} + +export async function setupDirectPaymentsTest(): Promise { + const { frameworkDeployer } = await deployTestFramework(); + const sfFramework = await frameworkDeployer.getFramework(); + + const signers = await ethers.getSigners(); + const signer = signers[0]; + + const gdframework = await deploySuperGoodDollar(signer, sfFramework, [ + ethers.constants.AddressZero, + ethers.constants.AddressZero, + ]); + + const poolSettings: DirectPaymentsPool.PoolSettingsStruct = { + nftType: 1, + uniquenessValidator: ethers.constants.AddressZero, + rewardPerEvent: [100, 300], + validEvents: [1, 2], + manager: signer.address, + membersValidator: ethers.constants.AddressZero, + rewardToken: gdframework.GoodDollar.address, + allowRewardOverride: false, + }; + + const poolLimits: DirectPaymentsPool.SafetyLimitsStruct = { + maxMemberPerDay: 300, + maxMemberPerMonth: 1000, + maxTotalPerMonth: 3000, + }; + + return { + gdframework, + signers, + signer, + poolSettings, + poolLimits, + }; +} + +export interface FactorySetup { + pool: DirectPaymentsPool; + factory: DirectPaymentsFactory; + nft: ProvableNFT; + membersValidator: MockContract; +} + +export async function createFactoryPoolFixture( + setup: TestSetup, + projectName: string = 'xx', + ipfsHash: string = 'ipfs' +): Promise { + const { gdframework, signers, signer, poolSettings, poolLimits } = setup; + + const factory = await ethers.getContractFactory('ProvableNFT'); + const nft = (await upgrades.deployProxy(factory, ['nft', 'cc'], { kind: 'uups' })) as ProvableNFT; + const helper = await ethers.deployContract('HelperLibrary'); + const helper2 = await ethers.deployContract('DirectPaymentsLibrary'); + + const Pool = await ethers.getContractFactory('DirectPaymentsPool', { + libraries: { HelperLibrary: helper.address, DirectPaymentsLibrary: helper2.address }, + }); + const membersValidator = await deployMockContract(signers[0], [ + 'function isMemberValid(address pool,address operator,address member,bytes memory extraData) external returns (bool)', + ]); + const poolImpl = await Pool.deploy(await gdframework.GoodDollar.getHost(), ethers.constants.AddressZero); + + const poolFactory = (await upgrades.deployProxy( + await ethers.getContractFactory('DirectPaymentsFactory'), + [signer.address, poolImpl.address, nft.address, ethers.constants.AddressZero, 0], + { kind: 'uups' } + )) as DirectPaymentsFactory; + + await nft.grantRole(ethers.constants.HashZero, poolFactory.address); + // all members are valid by default + membersValidator.mock['isMemberValid'].returns(true); + + const poolTx = await ( + await poolFactory.createPool( + projectName, + ipfsHash, + { ...poolSettings, membersValidator: membersValidator.address }, + poolLimits, + 0 + ) + ).wait(); + const poolAddress = poolTx.events?.find((_) => _.event === 'PoolCreated')?.args?.[0]; + const pool = Pool.attach(poolAddress) as DirectPaymentsPool; + + await gdframework.GoodDollar.mint(pool.address, ethers.constants.WeiPerEther.mul(100000)).then((_: any) => + _.wait() + ); + + return { + pool, + factory: poolFactory, + nft, + membersValidator, + }; +} describe('DirectPaymentsPool Claim', () => { let pool: DirectPaymentsPool; @@ -35,80 +142,25 @@ describe('DirectPaymentsPool Claim', () => { }; let nftSampleId = '56540060779879397317558633372065109751397093370573329176446590137680733287562'; + let testSetup: TestSetup; + before(async () => { - const { frameworkDeployer } = await deployTestFramework(); - const sfFramework = await frameworkDeployer.getFramework(); - - signers = await ethers.getSigners(); - - gdframework = await deploySuperGoodDollar(signers[0], sfFramework, [ - ethers.constants.AddressZero, - ethers.constants.AddressZero, - ]); - signer = signers[0]; - poolSettings = { - nftType: 1, - uniquenessValidator: ethers.constants.AddressZero, - rewardPerEvent: [100, 300], - validEvents: [1, 2], - manager: signer.address, - membersValidator: ethers.constants.AddressZero, - rewardToken: gdframework.GoodDollar.address, - allowRewardOverride: false, - }; - - poolLimits = { - maxMemberPerDay: 300, - maxMemberPerMonth: 1000, - maxTotalPerMonth: 3000, - }; + testSetup = await setupDirectPaymentsTest(); + gdframework = testSetup.gdframework; + signers = testSetup.signers; + signer = testSetup.signer; + poolSettings = testSetup.poolSettings; + poolLimits = testSetup.poolLimits; }); const fixture = async () => { - const factory = await ethers.getContractFactory('ProvableNFT'); - nft = (await upgrades.deployProxy(factory, ['nft', 'cc'], { kind: 'uups' })) as ProvableNFT; - const helper = await ethers.deployContract('HelperLibrary'); - const helper2 = await ethers.deployContract('DirectPaymentsLibrary'); - - const Pool = await ethers.getContractFactory('DirectPaymentsPool', { - libraries: { HelperLibrary: helper.address, DirectPaymentsLibrary: helper2.address }, - }); - membersValidator = await deployMockContract(signers[0], [ - 'function isMemberValid(address pool,address operator,address member,bytes memory extraData) external returns (bool)', - ]); - const poolImpl = await Pool.deploy(await gdframework.GoodDollar.getHost(), ethers.constants.AddressZero); - - const poolFactory = (await upgrades.deployProxy( - await ethers.getContractFactory('DirectPaymentsFactory'), - [signer.address, poolImpl.address, nft.address, ethers.constants.AddressZero, 0], - { kind: 'uups' } - )) as DirectPaymentsFactory; - - // console.log("deployed factory:", poolFactory.address) - - await nft.grantRole(ethers.constants.HashZero, poolFactory.address); - // all members are valid by default - membersValidator.mock['isMemberValid'].returns(true); - - const poolTx = await ( - await poolFactory.createPool( - 'xx', - 'ipfs', - { ...poolSettings, membersValidator: membersValidator.address }, - poolLimits, - 0 - ) - ).wait(); - // console.log("created pool:", poolTx.events) - const poolAddress = poolTx.events?.find((_) => _.event === 'PoolCreated')?.args?.[0]; - pool = Pool.attach(poolAddress) as DirectPaymentsPool; + const factorySetup = await createFactoryPoolFixture(testSetup, 'xx', 'ipfs'); + pool = factorySetup.pool; + nft = factorySetup.nft; + membersValidator = factorySetup.membersValidator; const tx = await nft.mintPermissioned(signers[0].address, nftSample, true, []).then((_) => _.wait()); - await gdframework.GoodDollar.mint(pool.address, ethers.constants.WeiPerEther.mul(100000)).then((_: any) => - _.wait() - ); nftSampleId = tx.events?.find((e) => e.event === 'Transfer')?.args?.tokenId; - // return { pool, nft, membersValidator }; }; beforeEach(async function () { @@ -117,12 +169,13 @@ describe('DirectPaymentsPool Claim', () => { describe('claim', () => { it('non member should not be able to get rewards', async () => { - const membersValidator = await deployMockContract(signers[0], [ + const newMembersValidator = await deployMockContract(signers[0], [ 'function isMemberValid(address pool,address operator,address member,bytes memory extraData) external returns (bool)', ]); - membersValidator.mock['isMemberValid'].returns(false); + // Set up mock to return false for all calls (reject all members) + newMembersValidator.mock['isMemberValid'].returns(false); - await pool.setPoolSettings({ ...poolSettings, membersValidator: membersValidator.address }, 0); + await pool.setPoolSettings({ ...poolSettings, membersValidator: newMembersValidator.address }, 0); await expect(pool['claim(uint256)'](nftSampleId)).not.reverted; const contributer = nftSample.events[0].contributers[0]; const initialBalance = await gdframework.GoodDollar.balanceOf(contributer); diff --git a/packages/contracts/test/UBIPool/UBIPool.bulkMembers.test.ts b/packages/contracts/test/UBIPool/UBIPool.bulkMembers.test.ts new file mode 100644 index 00000000..1906d909 --- /dev/null +++ b/packages/contracts/test/UBIPool/UBIPool.bulkMembers.test.ts @@ -0,0 +1,310 @@ +import { deploySuperGoodDollar } from "@gooddollar/goodprotocol"; +import { loadFixture } from "@nomicfoundation/hardhat-network-helpers"; +import { deployTestFramework } from "@superfluid-finance/ethereum-contracts/dev-scripts/deploy-test-framework"; +import { expect } from "chai"; +import { UBIPoolFactory, UBIPool, ProvableNFT } from "typechain-types"; +import { ethers, upgrades } from "hardhat"; +import { MockContract, deployMockContract } from "ethereum-waffle"; +import { PoolSettingsStruct } from "typechain-types/contracts/UBI/UBIPool"; + +type SignerWithAddress = Awaited>; + +describe("UBIPool Bulk Members", () => { + let factory: UBIPoolFactory; + let pool: UBIPool; + let signer: SignerWithAddress; + let signers: SignerWithAddress[]; + let poolSettings: PoolSettingsStruct; + let extendedPoolSettings: UBIPool.ExtendedSettingsStruct; + let poolLimits: UBIPool.UBISettingsStruct; + let gdframework: Awaited>; + let sfFramework: { [key: string]: string }; + let membersValidator: MockContract; + let uniquenessValidator: MockContract; + + before(async () => { + const { frameworkDeployer } = await deployTestFramework(); + sfFramework = await frameworkDeployer.getFramework(); + signers = await ethers.getSigners(); + gdframework = await deploySuperGoodDollar(signers[0], sfFramework, [ + ethers.constants.AddressZero, + ethers.constants.AddressZero + ]); + signer = signers[0]; + poolSettings = { + uniquenessValidator: ethers.constants.AddressZero, + manager: signers[1].address, + membersValidator: ethers.constants.AddressZero, + rewardToken: gdframework.GoodDollar.address + }; + extendedPoolSettings = { + maxPeriodClaimers: 500, + minClaimAmount: ethers.utils.parseEther("1"), + managerFeeBps: 0 + }; + poolLimits = { + cycleLengthDays: ethers.BigNumber.from(60), + claimPeriodDays: ethers.BigNumber.from(1), + minActiveUsers: ethers.BigNumber.from(100), + claimForEnabled: true, + maxClaimAmount: ethers.utils.parseEther("100"), + maxMembers: 500, + onlyMembers: true + }; + }); + + const fixture = async () => { + const f = await ethers.getContractFactory("UBIPoolFactory"); + const swapMock = await ethers.deployContract("SwapRouterMock", [gdframework.GoodDollar.address]); + const helper = await ethers.deployContract("HelperLibrary"); + + // Get host from framework, with fallback to GoodDollar contract + const sfHost = sfFramework?.['host'] || await gdframework.GoodDollar.getHost(); + const dpimpl = await ethers.deployContract("UBIPool", [sfHost, swapMock.address], { + libraries: { HelperLibrary: helper.address } + }); + + factory = (await upgrades.deployProxy(f, [signer.address, dpimpl.address, signers[1].address, 1000], { + kind: "uups", + unsafeAllowLinkedLibraries: true + })) as UBIPoolFactory; + + // Create mock validators + membersValidator = await deployMockContract(signers[0], [ + "function isMemberValid(address pool,address operator,address member,bytes memory extraData) external returns (bool)" + ]); + uniquenessValidator = await deployMockContract(signers[0], [ + "function getWhitelistedRoot(address member) external view returns (address)" + ]); + + // Default: all members are valid + membersValidator.mock["isMemberValid"].returns(true); + uniquenessValidator.mock["getWhitelistedRoot"].returns((member: string) => member); + + const poolTx = await factory.createPool( + "test", + "pool1", + { + ...poolSettings, + membersValidator: membersValidator.address, + uniquenessValidator: uniquenessValidator.address + }, + poolLimits, + extendedPoolSettings + ); + const receipt = await poolTx.wait(); + const poolAddr = receipt.events?.find((_) => _.event === "PoolCreated")?.args?.[0]; + pool = (await ethers.getContractAt("UBIPool", poolAddr)) as UBIPool; + + // Fund the pool + await gdframework.GoodDollar.mint(pool.address, ethers.utils.parseEther("100000")); + }; + + beforeEach(async function () { + await loadFixture(fixture); + }); + + describe("addMembers - Pool", () => { + it("should add multiple valid members successfully", async () => { + const members = [signers[2].address, signers[3].address, signers[4].address]; + const extraData = ["0x", "0x", "0x"]; + + const tx = await pool.connect(signers[1]).addMembers(members, extraData); + await expect(tx).not.reverted; + + // Verify all members have MEMBER_ROLE + for (const member of members) { + expect(await pool.hasRole(await pool.MEMBER_ROLE(), member)).to.be.true; + } + + // Verify membersCount incremented + const status = await pool.status(); + expect(status.membersCount).to.equal(3); + + // Verify RoleGranted events emitted + const receipt = await tx.wait(); + const roleGrantedEvents = receipt.events?.filter( + (e) => e.event === "RoleGranted" && e.address === pool.address + ); + expect(roleGrantedEvents?.length).to.equal(3); + }); + + it("should skip duplicate members without reverting", async () => { + const members = [signers[2].address, signers[3].address, signers[2].address]; // duplicate + const extraData = ["0x", "0x", "0x"]; + + const tx = await pool.connect(signers[1]).addMembers(members, extraData); + await expect(tx).not.reverted; + + // Verify unique members were added + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[2].address)).to.be.true; + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[3].address)).to.be.true; + + // Verify membersCount is 2 (not 3) + const status = await pool.status(); + expect(status.membersCount).to.equal(2); + }); + + it("should revert when adding members would exceed maxMembers", async () => { + // Create pool with maxMembers = 5 + const limitedPoolLimits = { ...poolLimits, maxMembers: 5 }; + const poolTx = await factory.createPool( + "test2", + "pool2", + { + ...poolSettings, + membersValidator: membersValidator.address, + uniquenessValidator: uniquenessValidator.address + }, + limitedPoolLimits, + extendedPoolSettings + ); + const receipt = await poolTx.wait(); + const poolAddr = receipt.events?.find((_) => _.event === "PoolCreated")?.args?.[0]; + const limitedPool = (await ethers.getContractAt("UBIPool", poolAddr)) as UBIPool; + + const members = Array(6) + .fill(0) + .map((_, i) => ethers.Wallet.createRandom().address); + const extraData = Array(6).fill("0x"); + + await expect(limitedPool.connect(signers[1]).addMembers(members, extraData)).to.be.revertedWithCustomError( + limitedPool, + "MAX_MEMBERS_REACHED" + ); + }); + + it("should skip members failing uniqueness validation", async () => { + const members = [signers[2].address, signers[3].address, signers[4].address]; + const extraData = ["0x", "0x", "0x"]; + + // Mock uniqueness validator to reject second member - specific mocks take precedence + uniquenessValidator.mock["getWhitelistedRoot"].returns((member: string) => member); + uniquenessValidator.mock["getWhitelistedRoot"] + .withArgs(signers[3].address) + .returns(ethers.constants.AddressZero); + + const tx = await pool.connect(signers[1]).addMembers(members, extraData); + await expect(tx).not.reverted; + + // Verify valid members were added + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[2].address)).to.be.true; + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[4].address)).to.be.true; + + // Verify invalid member was skipped + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[3].address)).to.be.false; + + // Verify membersCount is 2 + const status = await pool.status(); + expect(status.membersCount).to.equal(2); + }); + + it("should skip members failing membersValidator check", async () => { + const members = [signers[2].address, signers[3].address, signers[4].address]; + const extraData = ["0x", "0x", "0x"]; + + // Set up mocks - specific mocks take precedence over default + membersValidator.mock["isMemberValid"].returns(true); + membersValidator.mock["isMemberValid"] + .withArgs(pool.address, signers[1].address, signers[3].address, "0x") + .returns(false); + + const tx = await pool.connect(signers[1]).addMembers(members, extraData); + await expect(tx).not.reverted; + + // Verify valid members were added + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[2].address)).to.be.true; + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[4].address)).to.be.true; + + // Verify invalid member was skipped + expect(await pool.hasRole(await pool.MEMBER_ROLE(), signers[3].address)).to.be.false; + }); + + it("should enforce onlyMembers and MANAGER_ROLE access control", async () => { + const members = [signers[2].address, signers[3].address]; + const extraData = ["0x", "0x"]; + + // Non-manager should not be able to add members + await expect(pool.connect(signers[5]).addMembers(members, extraData)).to.be.reverted; + }); + + it("should revert if members and extraData arrays have different lengths", async () => { + const members = [signers[2].address, signers[3].address]; + const extraData = ["0x"]; // mismatched length + + await expect(pool.connect(signers[1]).addMembers(members, extraData)).to.be.revertedWith("Length mismatch"); + }); + + it("should update factory registry for all added members", async () => { + const members = [signers[2].address, signers[3].address, signers[4].address]; + const extraData = ["0x", "0x", "0x"]; + + await pool.connect(signers[1]).addMembers(members, extraData); + + // Verify factory registry updated + for (const member of members) { + const memberPools = await factory.memberPools(member, 0); + expect(memberPools).to.equal(pool.address); + } + }); + + it("should emit MemberAdded events from factory for each new member", async () => { + const members = [signers[2].address, signers[3].address, signers[4].address]; + const extraData = ["0x", "0x", "0x"]; + + const tx = await pool.connect(signers[1]).addMembers(members, extraData); + const receipt = await tx.wait(); + + // Verify factory emitted MemberAdded events for each member + const factoryMemberAddedEvents = receipt.events?.filter( + (e) => e.event === "MemberAdded" && e.address === factory.address + ); + expect(factoryMemberAddedEvents?.length).to.equal(3); + + // Verify event args + for (let i = 0; i < members.length; i++) { + const event = factoryMemberAddedEvents?.[i]; + expect(event?.args?.[0]).to.equal(members[i]); + expect(event?.args?.[1]).to.equal(pool.address); + } + }); + + it("should handle large batch of 100 members", async () => { + const members = Array(100) + .fill(0) + .map((_, i) => ethers.Wallet.createRandom().address); + const extraData = Array(100).fill("0x"); + + const tx = await pool.connect(signers[1]).addMembers(members, extraData); + await expect(tx).not.reverted; + + // Verify all members added + for (const member of members) { + expect(await pool.hasRole(await pool.MEMBER_ROLE(), member)).to.be.true; + } + + // Verify membersCount + const status = await pool.status(); + expect(status.membersCount).to.equal(100); + }); + }); + + describe("Gas Measurement", () => { + it("should measure gas for different batch sizes", async () => { + const batchSizes = [10, 50, 100]; + + for (const size of batchSizes) { + const members = Array(size) + .fill(0) + .map((_, i) => ethers.Wallet.createRandom().address); + const extraData = Array(size).fill("0x"); + + const tx = await pool.connect(signers[1]).addMembers(members, extraData); + const receipt = await tx.wait(); + + console.log(`Gas used for ${size} members: ${receipt.gasUsed.toString()}`); + expect(receipt.gasUsed).to.be.lt(30000000); // Should be under block gas limit + } + }); + }); +}); diff --git a/packages/sdk-js/src/goodcollective/goodcollective.ts b/packages/sdk-js/src/goodcollective/goodcollective.ts index 5fef14eb..0b69ab9f 100644 --- a/packages/sdk-js/src/goodcollective/goodcollective.ts +++ b/packages/sdk-js/src/goodcollective/goodcollective.ts @@ -681,6 +681,99 @@ export class GoodCollectiveSDK { return token.approve(poolAddress, amount, { ...CHAIN_OVERRIDES[this.chainId], gasLimit: 100000 }); } + /** + * Adds multiple members to a DirectPayments pool in a single transaction + * @param {ethers.Signer} signer - The signer object for the transaction. + * @param {string} poolAddress - The address of the DirectPayments pool contract. + * @param {string[]} members - Array of member addresses to add. + * @param {string[]} extraData - Array of extra data for each member (must match members length). + * @returns {Promise} A promise that resolves to a transaction object when the operation is complete. + */ + async addDirectPaymentsPoolMembers( + signer: ethers.Signer, + poolAddress: string, + members: string[], + extraData: string[] + ): Promise { + const connected = this.pool.attach(poolAddress).connect(signer); + return connected.addMembers(members, extraData, { ...CHAIN_OVERRIDES[this.chainId] }); + } + + /** + * Adds multiple members to a UBI pool in a single transaction + * @param {ethers.Signer} signer - The signer object for the transaction. + * @param {string} poolAddress - The address of the UBI pool contract. + * @param {string[]} members - Array of member addresses to add. + * @param {string[]} extraData - Array of extra data for each member (must match members length). + * @returns {Promise} A promise that resolves to a transaction object when the operation is complete. + */ + async addUBIPoolMembers( + signer: ethers.Signer, + poolAddress: string, + members: string[], + extraData: string[] + ): Promise { + const connected = this.ubipool.attach(poolAddress).connect(signer); + return connected.addMembers(members, extraData, { ...CHAIN_OVERRIDES[this.chainId] }); + } + + /** + * Estimates gas for bulk adding members and recommends batch sizes + * @param {ethers.Signer | ethers.providers.Provider} signerOrProvider - The signer or provider for estimation. + * @param {string} poolAddress - The address of the pool contract. + * @param {number} totalMembers - The total number of members to add. + * @param {string[]} sampleMembers - Sample member addresses for estimation (at least 3 recommended). + * @param {string[]} sampleExtraData - Sample extra data matching sampleMembers. + * @param {'directPayments' | 'ubi'} poolType - The type of pool. + * @returns {Promise<{estimatedGasPerMember: BigNumberish, recommendedBatchSize: number, totalBatches: number, estimatedGasPerBatch: BigNumberish}>} + */ + async estimateBulkAddMembersGas( + signerOrProvider: ethers.Signer | ethers.providers.Provider, + poolAddress: string, + totalMembers: number, + sampleMembers: string[], + sampleExtraData: string[], + poolType: 'directPayments' | 'ubi' = 'directPayments' + ) { + if (sampleMembers.length !== sampleExtraData.length) { + throw new Error('Sample members and extraData arrays must have the same length'); + } + + if (sampleMembers.length < 3) { + throw new Error('Please provide at least 3 sample members for accurate estimation'); + } + + const pool = poolType === 'ubi' ? this.ubipool : this.pool; + const connected = pool.attach(poolAddress); + + // Estimate gas for sample batch + const estimatedGas = await connected + .connect(signerOrProvider) + .estimateGas.addMembers(sampleMembers, sampleExtraData); + + const estimatedGasPerMember = estimatedGas.div(sampleMembers.length); + + // Conservative batch size calculation + // Target ~10M gas per batch to stay well under block gas limit + const targetGasPerBatch = ethers.BigNumber.from(10_000_000); + const recommendedBatchSizeBn = targetGasPerBatch.div(estimatedGasPerMember); + + const recommendedBatchSize = Math.min( + recommendedBatchSizeBn.toNumber(), + 200 // MAX_BATCH_SIZE + ); + + const totalBatches = Math.ceil(totalMembers / recommendedBatchSize); + const estimatedGasPerBatch = estimatedGasPerMember.mul(recommendedBatchSize); + + return { + estimatedGasPerMember: estimatedGasPerMember.toString(), + recommendedBatchSize, + totalBatches, + estimatedGasPerBatch: estimatedGasPerBatch.toString(), + }; + } + /** * Get protocol and manager fees for a collective * @param {string} poolAddress - The address of the pool contract. @@ -793,4 +886,23 @@ export class GoodCollectiveSDK { }; } } + + /** + * Adds multiple members to a pool in a single transaction + * Works for both DirectPayments and UBI pools + * @param {ethers.Signer} signer - The signer object for the transaction + * @param {string} poolAddress - The address of the pool contract + * @param {string[]} members - Array of member addresses to add + * @param {string[]} extraData - Array of additional validation data for each member + * @returns {Promise} A promise that resolves to a transaction object + */ + async addPoolMembers( + signer: ethers.Signer, + poolAddress: string, + members: string[], + extraData: string[] + ): Promise { + const connected = this.pool.attach(poolAddress).connect(signer); + return connected.addMembers(members, extraData, { ...CHAIN_OVERRIDES[this.chainId] }); + } }