Merge pull request #992 from JoinColony/feat/no-extension-reputation

Change how extensions receive tokens and reputation

contracts/colony/ColonyFunding.sol

@@ -23,13 +23,13 @@ import "./ColonyStorage.sol";
 
 
 contract ColonyFunding is ColonyStorage, PatriciaTreeProofs { // ignore-swc-123
-  function lockToken() public stoppable onlyExtension returns (uint256) {
+  function lockToken() public stoppable onlyOwnExtension returns (uint256) {
     uint256 lockId = ITokenLocking(tokenLockingAddress).lockToken(token);
     tokenLocks[msg.sender][lockId] = true;
     return lockId;
   }
 
-  function unlockTokenForUser(address _user, uint256 _lockId) public stoppable onlyExtension {
+  function unlockTokenForUser(address _user, uint256 _lockId) public stoppable onlyOwnExtension {
     require(tokenLocks[msg.sender][_lockId], "colony-bad-lock-id");
     ITokenLocking(tokenLockingAddress).unlockTokenForUser(token, _user, _lockId);
   }
@@ -144,7 +144,7 @@ contract ColonyFunding is ColonyStorage, PatriciaTreeProofs { // ignore-swc-123
     }
 
     // Process reputation updates if internal token
-    if (_token == token) {
+    if (_token == token && !isExtension(slot.recipient)) {
       IColonyNetwork colonyNetworkContract = IColonyNetwork(colonyNetworkAddress);
       colonyNetworkContract.appendReputationUpdateLog(slot.recipient, int256(repPayout), domains[expenditure.domainId].skillId);
       if (slot.skills.length > 0 && slot.skills[0] > 0) {
@@ -623,7 +623,7 @@ contract ColonyFunding is ColonyStorage, PatriciaTreeProofs { // ignore-swc-123
     fundingPots[_fundingPotId].balance[_token] = sub(fundingPots[_fundingPotId].balance[_token], _payout);
     nonRewardPotsTotal[_token] = sub(nonRewardPotsTotal[_token], _payout);
 
-    uint fee = calculateNetworkFeeForPayout(_payout);
+    uint fee = isOwnExtension(_user) ? 0 : calculateNetworkFeeForPayout(_payout);
     uint remainder = sub(_payout, fee);
     fundingPots[_fundingPotId].payouts[_token] = sub(fundingPots[_fundingPotId].payouts[_token], _payout);
 

contracts/colony/ColonyPayment.sol

@@ -79,15 +79,17 @@ contract ColonyPayment is ColonyStorage {
     Payment storage payment = payments[_id];
     payment.finalized = true;
 
-    FundingPot storage fundingPot = fundingPots[payment.fundingPotId];
-
-    IColonyNetwork colonyNetworkContract = IColonyNetwork(colonyNetworkAddress);
-    // All payments in Colony's home token earn domain reputation and if skill was set, earn skill reputation
-    colonyNetworkContract.appendReputationUpdateLog(payment.recipient, int(fundingPot.payouts[token]), domains[payment.domainId].skillId);
-    if (payment.skills[0] > 0) {
-      // Currently we support at most one skill per Payment, similarly to Task model.
-      // This may change in future to allow multiple skills to be set on both Tasks and Payments
-      colonyNetworkContract.appendReputationUpdateLog(payment.recipient, int(fundingPot.payouts[token]), payment.skills[0]);
+    if (!isExtension(payment.recipient)) {
+      FundingPot storage fundingPot = fundingPots[payment.fundingPotId];
+
+      IColonyNetwork colonyNetworkContract = IColonyNetwork(colonyNetworkAddress);
+      // All payments in Colony's home token earn domain reputation and if skill was set, earn skill reputation
+      colonyNetworkContract.appendReputationUpdateLog(payment.recipient, int(fundingPot.payouts[token]), domains[payment.domainId].skillId);
+      if (payment.skills[0] > 0) {
+        // Currently we support at most one skill per Payment, similarly to Task model.
+        // This may change in future to allow multiple skills to be set on both Tasks and Payments
+        colonyNetworkContract.appendReputationUpdateLog(payment.recipient, int(fundingPot.payouts[token]), payment.skills[0]);
+      }
     }
 
     emit PaymentFinalized(msg.sender, _id);

contracts/colony/ColonyStorage.sol

@@ -238,20 +238,8 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes
     _;
   }
 
-  modifier onlyExtension() {
-    // Ensure msg.sender is a contract
-    require(isContract(msg.sender), "colony-sender-must-be-contract");
-
-    // Ensure msg.sender is an extension
-    // slither-disable-next-line unused-return
-    try ColonyExtension(msg.sender).identifier() returns (bytes32 extensionId) {
-      require(
-        IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == msg.sender,
-        "colony-must-be-extension"
-      );
-    } catch {
-      require(false, "colony-must-be-extension");
-    }
+  modifier onlyOwnExtension() {
+    require(isOwnExtension(msg.sender), "colony-must-be-own-extension");
     _;
   }
 
@@ -300,6 +288,38 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes
     return size > 0;
   }
 
+  function isOwnExtension(address addr) internal returns (bool) {
+    if (!isContract(addr)) {
+      return false;
+    }
+
+    // slither-disable-next-line unused-return
+    try ColonyExtension(addr).identifier() returns (bytes32 extensionId) {
+      return IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == addr;
+    } catch {
+      return false;
+    }
+  }
+
+  function isExtension(address addr) internal returns (bool) {
+    if (!isContract(addr)) {
+      return false;
+    }
+
+    // slither-disable-next-line unused-return
+    try ColonyExtension(addr).identifier() returns (bytes32 extensionId) {
+      // slither-disable-next-line unused-return
+      try ColonyExtension(addr).getColony() returns (address claimedAssociatedColony) {
+        return IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, claimedAssociatedColony) == addr;
+      } catch {
+        return false;
+      }
+    } catch {
+      return false;
+    }
+  }
+
+
   function domainExists(uint256 domainId) internal view returns (bool) {
     return domainId > 0 && domainId <= domainCount;
   }

contracts/colony/ColonyTask.sol

@@ -419,7 +419,9 @@ contract ColonyTask is ColonyStorage {
     task.status = TaskStatus.Finalized;
 
     for (uint8 roleId = 0; roleId <= 2; roleId++) {
-      updateReputation(TaskRole(roleId), task);
+      if (!isExtension(task.roles[roleId].user)) {
+        updateReputation(TaskRole(roleId), task);
+      }
     }
 
     emit TaskFinalized(msg.sender, _id);

test/contracts-network/colony-expenditure.js

@@ -3,10 +3,12 @@ import chai from "chai";
 import bnChai from "bn-chai";
 import { BN } from "bn.js";
 import { ethers } from "ethers";
+import { soliditySha3 } from "web3-utils";
 
 import { UINT256_MAX, INT128_MAX, WAD, SECONDS_PER_DAY, MAX_PAYOUT, GLOBAL_SKILL_ID, IPFS_HASH } from "../../helpers/constants";
 import { checkErrorRevert, expectEvent, getTokenArgs, forwardTime, getBlockTime, bn2bytes32 } from "../../helpers/test-helper";
 import { fundColonyWithTokens, setupRandomColony } from "../../helpers/test-data-generator";
+import { setupEtherRouter } from "../../helpers/upgradable-contracts";
 
 const { expect } = chai;
 chai.use(bnChai(web3.utils.BN));
@@ -16,6 +18,8 @@ const IColonyNetwork = artifacts.require("IColonyNetwork");
 const IReputationMiningCycle = artifacts.require("IReputationMiningCycle");
 const IMetaColony = artifacts.require("IMetaColony");
 const Token = artifacts.require("Token");
+const TestExtension0 = artifacts.require("TestExtension0");
+const Resolver = artifacts.require("Resolver");
 
 contract("Colony Expenditure", (accounts) => {
   const SLOT0 = 0;
@@ -711,6 +715,106 @@ contract("Colony Expenditure", (accounts) => {
     });
   });
 
+  describe("when claiming expenditures for extension contracts", () => {
+    let expenditureId;
+    let extensionAddress;
+    const TEST_EXTENSION = soliditySha3("TestExtension");
+    const extensionVersion = 0;
+
+    before(async () => {
+      // Install an extension
+
+      const extensionImplementation = await TestExtension0.new();
+      const resolver = await Resolver.new();
+      await setupEtherRouter("TestExtension0", { TestExtension0: extensionImplementation.address }, resolver);
+
+      await metaColony.addExtensionToNetwork(TEST_EXTENSION, resolver.address);
+
+      await colony.installExtension(TEST_EXTENSION, extensionVersion);
+      extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address);
+    });
+
+    beforeEach(async () => {
+      await colony.makeExpenditure(1, UINT256_MAX, 1, { from: ADMIN });
+      expenditureId = await colony.getExpenditureCount();
+    });
+
+    it("if recipient is own extension, should not award reputation or pay network fee", async () => {
+      await colony.setExpenditureRecipient(expenditureId, SLOT0, extensionAddress, { from: ADMIN });
+      await colony.setExpenditurePayout(expenditureId, SLOT0, token.address, WAD, { from: ADMIN });
+      await colony.setExpenditureSkill(expenditureId, SLOT0, GLOBAL_SKILL_ID, { from: ADMIN });
+
+      const expenditure = await colony.getExpenditure(expenditureId);
+      await colony.moveFundsBetweenPots(
+        1,
+        UINT256_MAX,
+        1,
+        UINT256_MAX,
+        UINT256_MAX,
+        domain1.fundingPotId,
+        expenditure.fundingPotId,
+        WAD,
+        token.address
+      );
+      await colony.finalizeExpenditure(expenditureId, { from: ADMIN });
+      await colony.claimExpenditurePayout(expenditureId, SLOT0, token.address);
+
+      const addr = await colonyNetwork.getReputationMiningCycle(false);
+      const repCycle = await IReputationMiningCycle.at(addr);
+      const numEntries = await repCycle.getReputationUpdateLogLength();
+
+      // No entry in the log should be for this address
+      for (let i = new BN(0); i.lt(numEntries); i = i.addn(1)) {
+        const skillEntry = await repCycle.getReputationUpdateLogEntry(i);
+        expect(skillEntry.user).to.not.equal(extensionAddress);
+      }
+
+      // Balance should be whole payout
+      const balance = await token.balanceOf(extensionAddress);
+      expect(balance).to.eq.BN(WAD);
+    });
+
+    it("if recipient is an extension for another colony, should not award reputation but should pay fee", async () => {
+      const { colony: otherColony } = await setupRandomColony(colonyNetwork);
+
+      await otherColony.installExtension(TEST_EXTENSION, 0);
+      const otherExtensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, otherColony.address);
+
+      await colony.setExpenditureRecipient(expenditureId, SLOT0, otherExtensionAddress, { from: ADMIN });
+      await colony.setExpenditurePayout(expenditureId, SLOT0, token.address, WAD, { from: ADMIN });
+      await colony.setExpenditureSkill(expenditureId, SLOT0, GLOBAL_SKILL_ID, { from: ADMIN });
+
+      const expenditure = await colony.getExpenditure(expenditureId);
+      await colony.moveFundsBetweenPots(
+        1,
+        UINT256_MAX,
+        1,
+        UINT256_MAX,
+        UINT256_MAX,
+        domain1.fundingPotId,
+        expenditure.fundingPotId,
+        WAD,
+        token.address
+      );
+      await colony.finalizeExpenditure(expenditureId, { from: ADMIN });
+      await colony.claimExpenditurePayout(expenditureId, SLOT0, token.address);
+
+      const addr = await colonyNetwork.getReputationMiningCycle(false);
+      const repCycle = await IReputationMiningCycle.at(addr);
+      const numEntries = await repCycle.getReputationUpdateLogLength();
+
+      // No entry in the log should be for this address
+      for (let i = new BN(0); i.lt(numEntries); i = i.addn(1)) {
+        const skillEntry = await repCycle.getReputationUpdateLogEntry(i);
+        expect(skillEntry.user).to.not.equal(otherExtensionAddress);
+      }
+
+      // But the balance should have the fee deducted
+      const balance = await token.balanceOf(otherExtensionAddress);
+      expect(balance).to.be.lt.BN(WAD);
+    });
+  });
+
   describe("when cancelling expenditures", () => {
     let expenditureId;
 

test/contracts-network/colony-network-extensions.js

@@ -324,13 +324,13 @@ contract("Colony Network Extensions", (accounts) => {
     it("does not allow non network-managed extensions to lock and unlock tokens", async () => {
       const testVotingToken = await TestVotingToken.new();
       await testVotingToken.install(colony.address);
-      await checkErrorRevert(testVotingToken.lockToken(), "colony-must-be-extension");
-      await checkErrorRevert(testVotingToken.unlockTokenForUser(ROOT, 0), "colony-must-be-extension");
+      await checkErrorRevert(testVotingToken.lockToken(), "colony-must-be-own-extension");
+      await checkErrorRevert(testVotingToken.unlockTokenForUser(ROOT, 0), "colony-must-be-own-extension");
     });
 
     it("does not allow users to lock and unlock tokens", async () => {
-      await checkErrorRevert(colony.lockToken(), "colony-sender-must-be-contract");
-      await checkErrorRevert(colony.unlockTokenForUser(ROOT, 0), "colony-sender-must-be-contract");
+      await checkErrorRevert(colony.lockToken(), "colony-must-be-own-extension");
+      await checkErrorRevert(colony.unlockTokenForUser(ROOT, 0), "colony-must-be-own-extension");
     });
 
     it("does not allow a colony to unlock a lock placed by another colony", async () => {

test/contracts-network/colony-payment.js

@@ -3,18 +3,23 @@ import chai from "chai";
 import bnChai from "bn-chai";
 import BN from "bn.js";
 import { ethers } from "ethers";
+import { soliditySha3 } from "web3-utils";
 
 import { UINT256_MAX, WAD, MAX_PAYOUT } from "../../helpers/constants";
 import { checkErrorRevert, getTokenArgs, expectEvent } from "../../helpers/test-helper";
 import { fundColonyWithTokens, setupRandomColony } from "../../helpers/test-data-generator";
+import { setupEtherRouter } from "../../helpers/upgradable-contracts";
 
 const { expect } = chai;
 chai.use(bnChai(web3.utils.BN));
 
 const EtherRouter = artifacts.require("EtherRouter");
 const IColonyNetwork = artifacts.require("IColonyNetwork");
 const IMetaColony = artifacts.require("IMetaColony");
+const IReputationMiningCycle = artifacts.require("IReputationMiningCycle");
 const Token = artifacts.require("Token");
+const TestExtension0 = artifacts.require("TestExtension0");
+const Resolver = artifacts.require("Resolver");
 
 contract("Colony Payment", (accounts) => {
   const RECIPIENT = accounts[3];
@@ -373,4 +378,75 @@ contract("Colony Payment", (accounts) => {
       expect(networkBalanceAfter2.sub(networkBalanceBefore2)).to.eq.BN(new BN("2"));
     });
   });
+
+  describe("when claiming payments on behalf of extensions", () => {
+    let extensionAddress;
+    const TEST_EXTENSION = soliditySha3("TestExtension");
+    const extensionVersion = 0;
+
+    before(async () => {
+      // Install an extension
+
+      const extensionImplementation = await TestExtension0.new();
+      const resolver = await Resolver.new();
+      await setupEtherRouter("TestExtension0", { TestExtension0: extensionImplementation.address }, resolver);
+
+      await metaColony.addExtensionToNetwork(TEST_EXTENSION, resolver.address);
+
+      await colony.installExtension(TEST_EXTENSION, extensionVersion);
+      extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address);
+    });
+
+    it("if recipient is own extension, should not award reputation or pay network fee", async () => {
+      await colony.addPayment(1, UINT256_MAX, extensionAddress, token.address, WAD, 1, 0);
+      const paymentId = await colony.getPaymentCount();
+      const payment = await colony.getPayment(paymentId);
+
+      await colony.moveFundsBetweenPots(1, UINT256_MAX, 1, UINT256_MAX, UINT256_MAX, 1, payment.fundingPotId, WAD.add(WAD.divn(10)), token.address);
+      await colony.finalizePayment(1, UINT256_MAX, paymentId);
+      await colony.claimPayment(paymentId, token.address);
+
+      const addr = await colonyNetwork.getReputationMiningCycle(false);
+      const repCycle = await IReputationMiningCycle.at(addr);
+      const numEntries = await repCycle.getReputationUpdateLogLength();
+
+      // No entry in the log should be for this address
+      for (let i = new BN(0); i.lt(numEntries); i = i.addn(1)) {
+        const skillEntry = await repCycle.getReputationUpdateLogEntry(i);
+        expect(skillEntry.user).to.not.equal(extensionAddress);
+      }
+
+      // Balance should be whole payout
+      const balance = await token.balanceOf(extensionAddress);
+      expect(balance).to.eq.BN(WAD);
+    });
+
+    it("if recipient is an extension for another colony, should not award reputation but should pay fee", async () => {
+      const { colony: otherColony } = await setupRandomColony(colonyNetwork);
+
+      await otherColony.installExtension(TEST_EXTENSION, extensionVersion);
+      const otherExtensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, otherColony.address);
+
+      await colony.addPayment(1, UINT256_MAX, otherExtensionAddress, token.address, WAD, 1, 0);
+      const paymentId = await colony.getPaymentCount();
+      const payment = await colony.getPayment(paymentId);
+
+      await colony.moveFundsBetweenPots(1, UINT256_MAX, 1, UINT256_MAX, UINT256_MAX, 1, payment.fundingPotId, WAD.add(WAD.divn(10)), token.address);
+      await colony.finalizePayment(1, UINT256_MAX, paymentId);
+
+      const addr = await colonyNetwork.getReputationMiningCycle(false);
+      const repCycle = await IReputationMiningCycle.at(addr);
+      const numEntries = await repCycle.getReputationUpdateLogLength();
+
+      // No entry in the log should be for this address
+      for (let i = new BN(0); i.lt(numEntries); i = i.addn(1)) {
+        const skillEntry = await repCycle.getReputationUpdateLogEntry(i);
+        expect(skillEntry.user).to.not.equal(otherExtensionAddress);
+      }
+
+      // But the balance should have the fee deducted
+      const balance = await token.balanceOf(otherExtensionAddress);
+      expect(balance).to.be.lt.BN(WAD);
+    });
+  });
 });