diff --git a/src/CommunityRewardsCalculator.sol b/src/CommunityRewardsCalculator.sol index 8f3e69f..2a9626e 100644 --- a/src/CommunityRewardsCalculator.sol +++ b/src/CommunityRewardsCalculator.sol @@ -11,11 +11,16 @@ import "./ICommunityRewardsCalculator.sol"; */ contract CommunityRewardsCalculator is ICommunityRewardsCalculator { + /** + * @notice Legacy method for backwards compatibility + */ function getRewardsToDistribute( address /*token*/, address[] calldata recipients, - IPurchaseTracker purchaseTracker + IPurchaseTracker purchaseTracker, + uint256[] calldata claimedRewards ) external view returns (uint256[] memory) { + require(recipients.length == claimedRewards.length, "Array lengths must match"); uint256[] memory amounts = new uint256[](recipients.length); // for every purchase or sale made by the recipient, distribute 1 loot token @@ -23,9 +28,41 @@ contract CommunityRewardsCalculator is ICommunityRewardsCalculator { uint256 totalPurchase = purchaseTracker.getPurchaseCount(recipients[i]); uint256 totalSales = purchaseTracker.getSalesCount(recipients[i]); uint256 totalRewards = totalPurchase + totalSales; - amounts[i] = totalRewards; + + // Subtract already claimed rewards to prevent double claiming + if (totalRewards > claimedRewards[i]) { + amounts[i] = totalRewards - claimedRewards[i]; + } else { + amounts[i] = 0; // No new rewards to claim + } } return amounts; } + + /** + * @notice Calculate rewards for a single user based on their purchase/sales activity and checkpoint + * @dev This is the newer, more gas-efficient implementation that works with checkpoints + */ + function calculateUserRewards( + address /*token*/, + address user, + IPurchaseTracker purchaseTracker, + uint256 lastClaimedPurchases, + uint256 lastClaimedSales + ) external view returns (uint256) { + // Get current purchase and sales counts + uint256 currentPurchases = purchaseTracker.getPurchaseCount(user); + uint256 currentSales = purchaseTracker.getSalesCount(user); + + // Calculate new (unclaimed) purchases and sales + uint256 newPurchases = currentPurchases > lastClaimedPurchases ? + currentPurchases - lastClaimedPurchases : 0; + + uint256 newSales = currentSales > lastClaimedSales ? + currentSales - lastClaimedSales : 0; + + // Return the total rewards (1 token per purchase/sale) + return newPurchases + newSales; + } } diff --git a/src/CommunityVault.sol b/src/CommunityVault.sol index 129dedc..641a65c 100644 --- a/src/CommunityVault.sol +++ b/src/CommunityVault.sol @@ -14,8 +14,16 @@ import "./ICommunityRewardsCalculator.sol"; contract CommunityVault is HasSecurityContext { using SafeERC20 for IERC20; - // Keeps a count of already distributed rewards - mapping(address => mapping(address => uint256)) public rewardsDistributed; + // Store the last claim checkpoint for each user/token combination + // Instead of storing accumulated rewards, we store the "checkpoint" values + // This allows us to only distribute rewards that have occurred since the last claim + struct ClaimCheckpoint { + uint256 lastClaimedPurchases; // Last claimed purchase count + uint256 lastClaimedSales; // Last claimed sales count + } + + // Mapping: token => user => checkpoint + mapping(address => mapping(address => ClaimCheckpoint)) public lastClaimCheckpoints; // Governance staking contract address address public governanceVault; @@ -30,7 +38,7 @@ contract CommunityVault is HasSecurityContext { event Deposit(address indexed token, address indexed from, uint256 amount); event Withdraw(address indexed token, address indexed to, uint256 amount); event Distribute(address indexed token, address indexed to, uint256 amount); - event RewardDistributed(address indexed token, address indexed recipient, uint256 amount); + event RewardDistributed(address indexed token, address indexed recipient, uint256 amount, uint256 newPurchaseCheckpoint, uint256 newSalesCheckpoint); /** * @dev Constructor to initialize the security context. @@ -155,15 +163,64 @@ contract CommunityVault is HasSecurityContext { return IERC20(token).balanceOf(address(this)); } - function _distributeRewards(address token, address[] memory recipients) internal { - if (address(rewardsCalculator) != address(0) && address(purchaseTracker) != address(0)) { + /** + * @dev For backward compatibility - returns the total rewards distributed to a recipient + * @param token The token address + * @param recipient The recipient address + * @return The sum of all previous rewards + */ + function rewardsDistributed(address token, address recipient) external view returns (uint256) { + if (address(purchaseTracker) == address(0)) return 0; + + ClaimCheckpoint memory checkpoint = lastClaimCheckpoints[token][recipient]; + return checkpoint.lastClaimedPurchases + checkpoint.lastClaimedSales; + } - //get rewards to distribute - uint256[] memory amounts = rewardsCalculator.getRewardsToDistribute( - token, recipients, IPurchaseTracker(purchaseTracker) + function _distributeRewards(address token, address[] memory recipients) internal { + if (address(rewardsCalculator) == address(0) || address(purchaseTracker) == address(0)) { + return; + } + + for (uint256 i = 0; i < recipients.length; i++) { + address recipient = recipients[i]; + + // Get the current checkpoint for this user/token + ClaimCheckpoint memory checkpoint = lastClaimCheckpoints[token][recipient]; + + // Get current totals from purchase tracker + uint256 currentPurchases = IPurchaseTracker(purchaseTracker).getPurchaseCount(recipient); + uint256 currentSales = IPurchaseTracker(purchaseTracker).getSalesCount(recipient); + + // Calculate reward using the calculator + uint256 totalReward = rewardsCalculator.calculateUserRewards( + token, + recipient, + IPurchaseTracker(purchaseTracker), + checkpoint.lastClaimedPurchases, + checkpoint.lastClaimedSales ); - - _distribute(token, recipients, amounts); + + if (totalReward > 0) { + // Update the checkpoint before distribution to prevent reentrancy + lastClaimCheckpoints[token][recipient] = ClaimCheckpoint( + currentPurchases, + currentSales + ); + + // Distribute rewards + if (token == address(0)) { + // ETH distribution + require(address(this).balance >= totalReward, "Insufficient ETH balance"); + (bool success, ) = recipient.call{value: totalReward}(""); + require(success, "ETH transfer failed"); + } else { + // ERC20 distribution + require(IERC20(token).balanceOf(address(this)) >= totalReward, "Insufficient token balance"); + IERC20(token).safeTransfer(recipient, totalReward); + } + + emit RewardDistributed(token, recipient, totalReward, currentPurchases, currentSales); + } } } @@ -186,9 +243,6 @@ contract CommunityVault is HasSecurityContext { IERC20(token).safeTransfer(recipients[i], amounts[i]); } - // record the distribution - rewardsDistributed[token][recipients[i]] += amounts[i]; - emit Distribute(token, recipients[i], amounts[i]); } } diff --git a/src/ICommunityRewardsCalculator.sol b/src/ICommunityRewardsCalculator.sol index bc7102b..629ff3c 100644 --- a/src/ICommunityRewardsCalculator.sol +++ b/src/ICommunityRewardsCalculator.sol @@ -9,9 +9,31 @@ import "@hamza-escrow/IPurchaseTracker.sol"; * distributed through the CommunityVault. */ interface ICommunityRewardsCalculator { + /** + * @notice Legacy function for backwards compatibility + * @dev This function will be kept for compatibility but not used in newer versions + */ function getRewardsToDistribute( address token, address[] calldata recipients, - IPurchaseTracker purchaseTracker - ) external returns (uint256[] memory); + IPurchaseTracker purchaseTracker, + uint256[] calldata claimedRewards + ) external view returns (uint256[] memory); + + /** + * @notice Calculate rewards for a specific user based on a checkpoint + * @param token The token being distributed + * @param user The user to calculate rewards for + * @param purchaseTracker The tracker for purchase data + * @param lastClaimedPurchases The number of purchases already claimed + * @param lastClaimedSales The number of sales already claimed + * @return The amount of rewards to distribute + */ + function calculateUserRewards( + address token, + address user, + IPurchaseTracker purchaseTracker, + uint256 lastClaimedPurchases, + uint256 lastClaimedSales + ) external view returns (uint256); } diff --git a/test/CommunityVault.t.sol b/test/CommunityVault.t.sol index 119fba0..626d3de 100644 --- a/test/CommunityVault.t.sol +++ b/test/CommunityVault.t.sol @@ -542,6 +542,360 @@ contract TestCommunityVault is DeploymentSetup { assertEq(vault.rewardsDistributed(address(loot), seller), rewardsToDistribute, "Incorrect rewards tracked"); } + function testClaimMultipleRewards() public { + bytes32 paymentId1 = keccak256("payment-reward-test-multiple-1"); + uint256 payAmount = 500; + + address buyer = depositor1; + address seller = recipient1; + + PaymentEscrow payEscrow = PaymentEscrow(payable(escrow)); + + // Fund the vault with multiple token types for rewards + deposit(depositor2, loot, 100_000); + deposit(depositor3, IERC20(address(0)), 100_000); + deposit(depositor2, testToken, 100_000); + + // Buyer makes a purchase with LOOT token + vm.startPrank(buyer); + loot.approve(address(payEscrow), payAmount); + + PaymentInput memory input1 = PaymentInput({ + id: paymentId1, + payer: buyer, + receiver: seller, + currency: address(loot), + amount: payAmount + }); + payEscrow.placePayment(input1); + vm.stopPrank(); + + // Buyer and seller release escrow + vm.prank(buyer); + payEscrow.releaseEscrow(paymentId1); + + if (!autoRelease) { + vm.prank(seller); + payEscrow.releaseEscrow(paymentId1); + } + + // Validate purchase tracking + assertEq(tracker.totalPurchaseCount(buyer), 1, "Incorrect purchase count"); + assertEq(tracker.totalSalesCount(seller), 1, "Incorrect sales count"); + + // Check initial reward balances + uint256 initialBuyerLootBalance = loot.balanceOf(buyer); + uint256 initialBuyerTestTokenBalance = testToken.balanceOf(buyer); + uint256 initialBuyerEthBalance = buyer.balance; + + uint256 expectedLootRewards = tracker.totalPurchaseCount(buyer); + + // First claim: Buyer claims LOOT rewards + vm.prank(buyer); + vault.claimRewards(address(loot)); + + // Verify LOOT rewards were distributed + uint256 afterFirstClaimLootBalance = loot.balanceOf(buyer); + assertEq(afterFirstClaimLootBalance, initialBuyerLootBalance + expectedLootRewards, "Incorrect LOOT reward distribution"); + assertEq(vault.rewardsDistributed(address(loot), buyer), expectedLootRewards, "Incorrect LOOT rewards tracked"); + + // Second claim: Buyer claims LOOT rewards again (should not receive additional rewards) + vm.prank(buyer); + vault.claimRewards(address(loot)); + + // Verify no additional LOOT rewards were distributed + assertEq(loot.balanceOf(buyer), afterFirstClaimLootBalance, "Additional LOOT rewards incorrectly distributed"); + assertEq(vault.rewardsDistributed(address(loot), buyer), expectedLootRewards, "Incorrect LOOT rewards tracking after second claim"); + + // First claim: Buyer claims test token rewards + vm.prank(buyer); + vault.claimRewards(address(testToken)); + + // Verify test token rewards were distributed + uint256 afterFirstClaimTestTokenBalance = testToken.balanceOf(buyer); + assertEq(afterFirstClaimTestTokenBalance, initialBuyerTestTokenBalance + expectedLootRewards, "Incorrect test token reward distribution"); + assertEq(vault.rewardsDistributed(address(testToken), buyer), expectedLootRewards, "Incorrect test token rewards tracked"); + + // Second claim: Buyer claims test token rewards again (should not receive additional rewards) + vm.prank(buyer); + vault.claimRewards(address(testToken)); + + // Verify no additional test token rewards were distributed + assertEq(testToken.balanceOf(buyer), afterFirstClaimTestTokenBalance, "Additional test token rewards incorrectly distributed"); + assertEq(vault.rewardsDistributed(address(testToken), buyer), expectedLootRewards, "Incorrect test token rewards tracking after second claim"); + + // First claim: Buyer claims ETH rewards + vm.prank(buyer); + vault.claimRewards(address(0)); + + // Verify ETH rewards were distributed + uint256 afterFirstClaimEthBalance = buyer.balance; + assertEq(afterFirstClaimEthBalance, initialBuyerEthBalance + expectedLootRewards, "Incorrect ETH reward distribution"); + assertEq(vault.rewardsDistributed(address(0), buyer), expectedLootRewards, "Incorrect ETH rewards tracked"); + + // Second claim: Buyer claims ETH rewards again (should not receive additional rewards) + vm.prank(buyer); + vault.claimRewards(address(0)); + + // Verify no additional ETH rewards were distributed + assertEq(buyer.balance, afterFirstClaimEthBalance, "Additional ETH rewards incorrectly distributed"); + assertEq(vault.rewardsDistributed(address(0), buyer), expectedLootRewards, "Incorrect ETH rewards tracking after second claim"); + + // Repeat the same test for the seller + uint256 initialSellerLootBalance = loot.balanceOf(seller); + uint256 expectedSellerRewards = tracker.totalSalesCount(seller); + + // First claim: Seller claims LOOT rewards + vm.prank(seller); + vault.claimRewards(address(loot)); + + // Verify LOOT rewards were distributed to seller + uint256 afterFirstClaimSellerLootBalance = loot.balanceOf(seller); + assertEq(afterFirstClaimSellerLootBalance, initialSellerLootBalance + expectedSellerRewards, "Incorrect seller LOOT reward distribution"); + assertEq(vault.rewardsDistributed(address(loot), seller), expectedSellerRewards, "Incorrect seller LOOT rewards tracked"); + + // Second claim: Seller claims LOOT rewards again (should not receive additional rewards) + vm.prank(seller); + vault.claimRewards(address(loot)); + + // Verify no additional LOOT rewards were distributed to seller + assertEq(loot.balanceOf(seller), afterFirstClaimSellerLootBalance, "Additional seller LOOT rewards incorrectly distributed"); + assertEq(vault.rewardsDistributed(address(loot), seller), expectedSellerRewards, "Incorrect seller LOOT rewards tracking after second claim"); + + // Try claiming multiple tokens in sequence + // Third claim (LOOT): Should not receive any additional rewards + vm.prank(buyer); + vault.claimRewards(address(loot)); + assertEq(loot.balanceOf(buyer), afterFirstClaimLootBalance, "Additional LOOT rewards incorrectly distributed on third claim"); + + // Third claim (Test Token): Should not receive any additional rewards + vm.prank(buyer); + vault.claimRewards(address(testToken)); + assertEq(testToken.balanceOf(buyer), afterFirstClaimTestTokenBalance, "Additional test token rewards incorrectly distributed on third claim"); + + // Third claim (ETH): Should not receive any additional rewards + vm.prank(buyer); + vault.claimRewards(address(0)); + assertEq(buyer.balance, afterFirstClaimEthBalance, "Additional ETH rewards incorrectly distributed on third claim"); + } + + function testDoubleClaimBugDetection() public { + bytes32 paymentId1 = keccak256("payment-double-claim-test"); + uint256 payAmount = 500; + + address buyer = depositor1; + address seller = recipient1; + + PaymentEscrow payEscrow = PaymentEscrow(payable(escrow)); + + // Fund the vault with tokens for rewards + deposit(depositor2, loot, 100_000); + + // Check initial reward tracker states + uint256 initialBuyerRewardTracker = vault.rewardsDistributed(address(loot), buyer); + + // Make a purchase + vm.startPrank(buyer); + loot.approve(address(payEscrow), payAmount); + PaymentInput memory input = PaymentInput({ + id: paymentId1, + payer: buyer, + receiver: seller, + currency: address(loot), + amount: payAmount + }); + payEscrow.placePayment(input); + vm.stopPrank(); + + // Complete the transaction + vm.prank(buyer); + payEscrow.releaseEscrow(paymentId1); + if (!autoRelease) { + vm.prank(seller); + payEscrow.releaseEscrow(paymentId1); + } + + // Check purchase tracking + assertEq(tracker.totalPurchaseCount(buyer), 1, "Incorrect purchase count"); + + // Get initial balances + uint256 initialBuyerLootBalance = loot.balanceOf(buyer); + + // DEBUG: Log the inputs to getRewardsToDistribute before first claim + address[] memory recipients = new address[](1); + recipients[0] = buyer; + uint256[] memory claimedRewards = new uint256[](1); + claimedRewards[0] = vault.rewardsDistributed(address(loot), buyer); + + console.log("Before first claim - Buyer address:", buyer); + console.log("Before first claim - Total purchases:", tracker.totalPurchaseCount(buyer)); + console.log("Before first claim - Previously claimed rewards:", claimedRewards[0]); + + // First claim + vm.prank(buyer); + vault.claimRewards(address(loot)); + + // Check balances after first claim + uint256 afterFirstClaimLootBalance = loot.balanceOf(buyer); + uint256 firstClaimAmount = afterFirstClaimLootBalance - initialBuyerLootBalance; + uint256 afterFirstClaimRewardTracker = vault.rewardsDistributed(address(loot), buyer); + + console.log("After first claim - Rewards distributed:", afterFirstClaimRewardTracker - initialBuyerRewardTracker); + console.log("After first claim - Balance change:", firstClaimAmount); + + // Verify first claim worked correctly + assertGt(firstClaimAmount, 0, "First claim should give rewards"); + assertEq(afterFirstClaimRewardTracker, initialBuyerRewardTracker + firstClaimAmount, "Rewards tracker should increase by claimed amount"); + + // DEBUG: Log the inputs to getRewardsToDistribute before second claim + claimedRewards[0] = vault.rewardsDistributed(address(loot), buyer); + console.log("Before second claim - Previously claimed rewards:", claimedRewards[0]); + + // Simulate block advancement (which might trigger reward recalculation) + vm.roll(block.number + 10); + vm.warp(block.timestamp + 3600); // Advance 1 hour + + // Second claim attempt - in the real world this seems to be paying out again + vm.prank(buyer); + vault.claimRewards(address(loot)); + + // Check balances after second claim + uint256 afterSecondClaimLootBalance = loot.balanceOf(buyer); + uint256 secondClaimAmount = afterSecondClaimLootBalance - afterFirstClaimLootBalance; + uint256 afterSecondClaimRewardTracker = vault.rewardsDistributed(address(loot), buyer); + + console.log("After second claim - Rewards distributed:", afterSecondClaimRewardTracker - afterFirstClaimRewardTracker); + console.log("After second claim - Balance change:", secondClaimAmount); + + // This assertion is what we expect - but it might be failing in production + assertEq(secondClaimAmount, 0, "Second claim should NOT give additional rewards"); + assertEq(afterSecondClaimRewardTracker, afterFirstClaimRewardTracker, "Rewards tracker should not increase after second claim"); + + // DEBUGGING: Check what the rewards calculator is returning for the second claim + claimedRewards[0] = vault.rewardsDistributed(address(loot), buyer); + + // Debugging helper - directly inspect the rewards calculator if possible (requires additional helper function) + // This will simulate what happens when the second claim is processed + vm.prank(admin); + address[] memory debugRecipients = new address[](1); + debugRecipients[0] = buyer; + + // Call distributeRewards which will give us the debug logs + vault.distributeRewards(address(loot), debugRecipients); + + // Verify final state + uint256 finalLootBalance = loot.balanceOf(buyer); + uint256 finalRewardTracker = vault.rewardsDistributed(address(loot), buyer); + + console.log("Final reward tracker value:", finalRewardTracker); + console.log("Total balance change:", finalLootBalance - initialBuyerLootBalance); + + // This should still be true - buyer should only receive rewards once + assertEq(finalLootBalance, afterFirstClaimLootBalance, "No additional rewards should be given with debug distribution"); + } + + function testClaimMultipleSamePerson() public { + bytes32 paymentId1 = keccak256("payment-self-transaction"); + uint256 payAmount = 500; + + // Same person is both buyer and seller + address buyerAndSeller = depositor1; + + PaymentEscrow payEscrow = PaymentEscrow(payable(escrow)); + + // Fund the vault with multiple token types for rewards + deposit(depositor2, loot, 100_000); + deposit(depositor3, IERC20(address(0)), 100_000); + deposit(depositor2, testToken, 100_000); + + // Person makes a purchase to themselves + vm.startPrank(buyerAndSeller); + loot.approve(address(payEscrow), payAmount); + + PaymentInput memory input1 = PaymentInput({ + id: paymentId1, + payer: buyerAndSeller, + receiver: buyerAndSeller, // Same address as buyer + currency: address(loot), + amount: payAmount + }); + payEscrow.placePayment(input1); + vm.stopPrank(); + + // Complete the transaction (only needs to release once if autoRelease is true) + vm.prank(buyerAndSeller); + payEscrow.releaseEscrow(paymentId1); + + if (!autoRelease) { + vm.prank(buyerAndSeller); + payEscrow.releaseEscrow(paymentId1); + } + + // Validate purchase tracking - should increment both buyer and seller counts + assertEq(tracker.totalPurchaseCount(buyerAndSeller), 1, "Incorrect purchase count"); + assertEq(tracker.totalSalesCount(buyerAndSeller), 1, "Incorrect sales count"); + + // Log debugging info + console.log("Self-transaction - Address:", buyerAndSeller); + console.log("Self-transaction - Total purchases:", tracker.totalPurchaseCount(buyerAndSeller)); + console.log("Self-transaction - Total sales:", tracker.totalSalesCount(buyerAndSeller)); + console.log("Self-transaction - Previously claimed rewards:", vault.rewardsDistributed(address(loot), buyerAndSeller)); + + // Check initial balance + uint256 initialBalance = loot.balanceOf(buyerAndSeller); + + // First claim: Should receive rewards for both buying and selling + vm.prank(buyerAndSeller); + vault.claimRewards(address(loot)); + + // Verify first claim results + uint256 afterFirstClaimBalance = loot.balanceOf(buyerAndSeller); + uint256 firstClaimAmount = afterFirstClaimBalance - initialBalance; + + console.log("After first claim - Balance change:", firstClaimAmount); + console.log("After first claim - Rewards tracker:", vault.rewardsDistributed(address(loot), buyerAndSeller)); + + // The person should get rewards for both buying and selling (total of 2) + // This is the expected behavior, but may be where the bug is happening + assertEq(firstClaimAmount, 2, "Should receive rewards for both buying and selling roles"); + + // Second claim: attempt to claim again + vm.roll(block.number + 10); // Advance blocks to simulate real conditions + vm.warp(block.timestamp + 3600); // Advance time + + vm.prank(buyerAndSeller); + vault.claimRewards(address(loot)); + + // Verify second claim results + uint256 afterSecondClaimBalance = loot.balanceOf(buyerAndSeller); + uint256 secondClaimAmount = afterSecondClaimBalance - afterFirstClaimBalance; + + console.log("After second claim - Balance change:", secondClaimAmount); + console.log("After second claim - Rewards tracker:", vault.rewardsDistributed(address(loot), buyerAndSeller)); + + // Should not receive additional rewards + assertEq(secondClaimAmount, 0, "Should not receive additional rewards on second claim"); + + // Third claim: attempt to claim once more + vm.roll(block.number + 20); // Advance more blocks + vm.warp(block.timestamp + 7200); // Advance more time + + vm.prank(buyerAndSeller); + vault.claimRewards(address(loot)); + + // Verify third claim results + uint256 afterThirdClaimBalance = loot.balanceOf(buyerAndSeller); + uint256 thirdClaimAmount = afterThirdClaimBalance - afterSecondClaimBalance; + + console.log("After third claim - Balance change:", thirdClaimAmount); + console.log("After third claim - Rewards tracker:", vault.rewardsDistributed(address(loot), buyerAndSeller)); + + // Should not receive additional rewards + assertEq(thirdClaimAmount, 0, "Should not receive additional rewards on third claim"); + + // Verify final state + assertEq(vault.rewardsDistributed(address(loot), buyerAndSeller), 2, "Total rewards distributed should be 2"); + } function deposit(address _depositor, IERC20 token, uint256 amount) private { vm.startPrank(_depositor);