Respect reward pots for domain-level claims; add event

Author: area

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

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) {

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 () => {