From 9575ac7b5b783d4da752138b6fed0f73b9b7571e Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Mon, 16 Sep 2024 11:13:33 +0100 Subject: [PATCH] Respect reward pots for domain-level claims; add event --- contracts/colony/ColonyDataTypes.sol | 8 ++++++++ contracts/colony/ColonyFunding.sol | 14 ++++++++++++-- test/contracts-network/colony-funding.js | 19 ++++++++++++++----- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/contracts/colony/ColonyDataTypes.sol b/contracts/colony/ColonyDataTypes.sol index 4b548b7cba..72920eb54d 100755 --- a/contracts/colony/ColonyDataTypes.sol +++ b/contracts/colony/ColonyDataTypes.sol @@ -63,6 +63,14 @@ interface ColonyDataTypes { /// @param payoutRemainder The remaining funds moved to the top-level domain pot event ColonyFundsClaimed(address agent, address token, uint256 fee, uint256 payoutRemainder); + /// @notice Event logged when funds sent directly to a domain are claimed + /// @param agent The address that is responsible for triggering this event + /// @param token The token address + /// @param domainId The domain id + /// @param fee The fee deducted for rewards + /// @param payoutRemainder The remaining funds moved to the domain pot + event DomainFundsClaimed(address agent, address token, uint256 domainId, uint256 fee, uint256 payoutRemainder); + /// @notice Event logged when a new reward payout cycle has started /// @param agent The address that is responsible for triggering this event /// @param rewardPayoutId The reward payout cycle id diff --git a/contracts/colony/ColonyFunding.sol b/contracts/colony/ColonyFunding.sol index 86cdcf15ed..653f0a26d1 100755 --- a/contracts/colony/ColonyFunding.sol +++ b/contracts/colony/ColonyFunding.sol @@ -126,7 +126,15 @@ contract ColonyFunding is thisBalanceBefore = ERC20Extended(_token).balanceOf(address(this)); } - fundingPots[fundingPotId].balance[_token] += claimAmount; + uint256 feeToPay = claimAmount / getRewardInverse(); // ignore-swc-110 . This variable is set when the colony is + // initialised to MAX_UINT, and cannot be set to zero via setRewardInverse, so this is a false positive. It *can* be set + // to 0 via recovery mode, but a) That's not why MythX is balking here and b) There's only so much we can stop people being + // able to do with recovery mode. + uint256 remainder = claimAmount - feeToPay; + nonRewardPotsTotal[_token] += remainder; + + fundingPots[0].balance[_token] += feeToPay; + fundingPots[fundingPotId].balance[_token] += remainder; // Claim funds @@ -136,7 +144,7 @@ contract ColonyFunding is uint256 balanceAfter = getFundingPotBalance(fundingPotId, _token); - assert(balanceAfter - balanceBefore == claimAmount); + assert(balanceAfter - balanceBefore == remainder); uint256 thisBalanceAfter; if (_token == address(0x0)) { @@ -146,6 +154,8 @@ contract ColonyFunding is } assert(thisBalanceAfter - thisBalanceBefore == claimAmount); + + emit DomainFundsClaimed(msgSender(), _token, _domainId, feeToPay, remainder); } function getNonRewardPotsTotal(address _token) public view returns (uint256) { diff --git a/test/contracts-network/colony-funding.js b/test/contracts-network/colony-funding.js index cf29f1a7ff..1f427c2e9f 100755 --- a/test/contracts-network/colony-funding.js +++ b/test/contracts-network/colony-funding.js @@ -17,7 +17,7 @@ const { } = require("../../helpers/constants"); const { fundColonyWithTokens, setupRandomColony, makeExpenditure, setupFundedExpenditure } = require("../../helpers/test-data-generator"); -const { getTokenArgs, checkErrorRevert, web3GetBalance, removeSubdomainLimit } = require("../../helpers/test-helper"); +const { getTokenArgs, checkErrorRevert, web3GetBalance, removeSubdomainLimit, expectEvent } = require("../../helpers/test-helper"); const { setupDomainTokenReceiverResolver } = require("../../helpers/upgradable-contracts"); const { expect } = chai; @@ -563,14 +563,19 @@ contract("Colony Funding", (accounts) => { const domain = await colony.getDomain(2); const domainPotBalanceBefore = await colony.getFundingPotBalance(domain.fundingPotId, ethers.constants.AddressZero); + const nonRewardPotsTotalBefore = await colony.getNonRewardPotsTotal(ethers.constants.AddressZero); // Claim the funds - await colony.claimDomainFunds(ethers.constants.AddressZero, 2); + + const tx = await colony.claimDomainFunds(ethers.constants.AddressZero, 2); + await expectEvent(tx, "DomainFundsClaimed", [MANAGER, ethers.constants.AddressZero, 2, 1, 99]); const domainPotBalanceAfter = await colony.getFundingPotBalance(domain.fundingPotId, ethers.constants.AddressZero); + const nonRewardPotsTotalAfter = await colony.getNonRewardPotsTotal(ethers.constants.AddressZero); // Check the balance of the domain - expect(domainPotBalanceAfter.sub(domainPotBalanceBefore)).to.eq.BN(100); + expect(domainPotBalanceAfter.sub(domainPotBalanceBefore)).to.eq.BN(99); + expect(nonRewardPotsTotalAfter.sub(nonRewardPotsTotalBefore)).to.eq.BN(99); }); it("should allow a token to be directly sent to a domain", async () => { @@ -583,14 +588,18 @@ contract("Colony Funding", (accounts) => { const domain = await colony.getDomain(2); const domainPotBalanceBefore = await colony.getFundingPotBalance(domain.fundingPotId, otherToken.address); + const nonRewardPotsTotalBefore = await colony.getNonRewardPotsTotal(otherToken.address); // Claim the funds - await colony.claimDomainFunds(otherToken.address, 2); + const tx = await colony.claimDomainFunds(otherToken.address, 2); + await expectEvent(tx, "DomainFundsClaimed", [MANAGER, otherToken.address, 2, 1, 99]); const domainPotBalanceAfter = await colony.getFundingPotBalance(domain.fundingPotId, otherToken.address); + const nonRewardPotsTotalAfter = await colony.getNonRewardPotsTotal(otherToken.address); // Check the balance of the domain - expect(domainPotBalanceAfter.sub(domainPotBalanceBefore)).to.eq.BN(100); + expect(domainPotBalanceAfter.sub(domainPotBalanceBefore)).to.eq.BN(99); + expect(nonRewardPotsTotalAfter.sub(nonRewardPotsTotalBefore)).to.eq.BN(99); }); it("should not be able to claim funds for a domain that does not exist", async () => {