diff --git a/contracts/colony/Colony.sol b/contracts/colony/Colony.sol index 634f3aabdb..1e6d6c03ed 100755 --- a/contracts/colony/Colony.sol +++ b/contracts/colony/Colony.sol @@ -332,6 +332,9 @@ contract Colony is BasicMetaTransaction, Multicall, ColonyStorage, PatriciaTreeP tokenReputationRates[token] = WAD; tokensWithReputationRatesLinkedList[address(0x00)] = token; nTokensWithReputationRates = 1; + + sig = bytes4(keccak256("setReputationDecayRate(uint256,uint256)")); + colonyAuthority.setRoleCapability(uint8(ColonyRole.Root), address(this), sig, true); } function setTokenReputationRate(address _prevToken, address _token, uint256 _rate) public stoppable { @@ -379,10 +382,6 @@ contract Colony is BasicMetaTransaction, Multicall, ColonyStorage, PatriciaTreeP return tokensWithReputationRatesLinkedList[_token]; } - function setReputationDecayRate(uint256 _numerator, uint256 _denominator) stoppable auth public { - IColonyNetwork(colonyNetworkAddress).setColonyReputationDecayRate(_numerator, _denominator); - } - function getMetatransactionNonce(address _user) override public view returns (uint256 nonce){ return metatransactionNonces[_user]; } diff --git a/contracts/colony/ColonyAuthority.sol b/contracts/colony/ColonyAuthority.sol index ee1ca1aa38..8f635cb0ed 100644 --- a/contracts/colony/ColonyAuthority.sol +++ b/contracts/colony/ColonyAuthority.sol @@ -132,6 +132,7 @@ contract ColonyAuthority is CommonAuthority { // Added in colony v xxxxx addRoleCapability(ROOT_ROLE, "setDomainReputationScaling(uint256,bool,uint256)"); + addRoleCapability(ROOT_ROLE, "setReputationDecayRate(uint256,uint256)"); } diff --git a/contracts/colony/IColony.sol b/contracts/colony/IColony.sol index dd2481147f..da9e01a7e9 100644 --- a/contracts/colony/IColony.sol +++ b/contracts/colony/IColony.sol @@ -1117,7 +1117,6 @@ interface IColony is ColonyDataTypes, IRecovery, IBasicMetaTransaction, IMultica /// @param factor The scale factor to apply, as a WAD function setDomainReputationScaling(uint256 domainId, bool enabled, uint256 factor) external; - /// @notice Call to set the reputation scaling applied to payouts made in a particular token /// @param _prevToken The token before where the token being added (_rate > 0) or removed (_rate ==0) in /// the list of tokens that have reputation scaling applied @@ -1140,9 +1139,4 @@ interface IColony is ColonyDataTypes, IRecovery, IBasicMetaTransaction, IMultica /// @return address The address of the next token. If 0x00, queried token is either not in the /// list or is the last entry in the list. function getNextTokenWithReputationRate(address _token) external view returns (address); - - /// @notice Call to set the rate at which reputation in this colony decays - /// @param _numerator The numerator of the fraction reputation does down by every reputation cycle - /// @param _denominator The denominator of the fraction reputation does down by every reputation cycle - function setReputationDecayRate(uint256 _numerator, uint256 _denominator) external; } diff --git a/contracts/colonyNetwork/ColonyNetwork.sol b/contracts/colonyNetwork/ColonyNetwork.sol index e53551a296..d8fed98ea3 100644 --- a/contracts/colonyNetwork/ColonyNetwork.sol +++ b/contracts/colonyNetwork/ColonyNetwork.sol @@ -332,6 +332,48 @@ contract ColonyNetwork is ColonyDataTypes, BasicMetaTransaction, ColonyNetworkSt return factor; } + function setColonyReputationDecayRate(uint256 _numerator, uint256 _denominator) public calledByColony stoppable { + require(_numerator < 10**15, "colony-network-decay-numerator-too-big"); + require(_numerator <= _denominator, "colony-network-decay-rate-over-1"); + + ColonyDecayRate storage decayRate = colonyDecayRates[msgSender()]; + + if (activeReputationMiningCycle != decayRate.afterMiningCycle) { + // Move the old-next values to current, as they are in effect + decayRate.currentNumerator = decayRate.nextNumerator; + decayRate.currentDenominator = decayRate.nextDenominator; + + // Update afterMiningCycle + decayRate.afterMiningCycle = activeReputationMiningCycle; + } + + // Whether we've updated the current decays rates or not, we update the next values + decayRate.nextNumerator = _numerator; + decayRate.nextDenominator = _denominator; + + emit ColonyReputationDecayRateToChange(msgSender(), activeReputationMiningCycle, _numerator, _denominator); + } + + function getColonyReputationDecayRate(address _colony) public view returns (uint256, uint256) { + uint256 numerator; + uint256 denominator; + + if (activeReputationMiningCycle != colonyDecayRates[_colony].afterMiningCycle) { + // Then the values of interest is whatever's in nextNumerator/nextDenominator + numerator = colonyDecayRates[_colony].nextNumerator; + denominator = colonyDecayRates[_colony].nextDenominator; + } else { + numerator = colonyDecayRates[_colony].currentNumerator; + denominator = colonyDecayRates[_colony].currentDenominator; + } + + if (denominator == 0) { + // Then we return the 'default' decay rate + (numerator, denominator) = IReputationMiningCycle(activeReputationMiningCycle).getDecayConstant(); + } + return (numerator, denominator); + } + function incrementMetatransactionNonce(address _user) override internal { // We need to protect the metatransaction nonce slots, otherwise those with recovery // permissions could replay metatransactions, which would be a disaster. diff --git a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol index d4774de14d..2663e19af2 100755 --- a/contracts/colonyNetwork/ColonyNetworkDataTypes.sol +++ b/contracts/colonyNetwork/ColonyNetworkDataTypes.sol @@ -145,6 +145,14 @@ interface ColonyNetworkDataTypes { /// @param tokenAuthorityAddress The address of the token authority deployed event TokenAuthorityDeployed(address tokenAuthorityAddress); + /// @notice Event logged when a colony sets what its next decay rate is going to be + /// @param colony The colony changing its decay rate + /// @param fromCycleCompleted When this mining cycle is completed, the new rate will be in effect + /// @param numerator The new numerator of the decay rate + /// @param denominator The new denominator of the decay rate + event ColonyReputationDecayRateToChange(address colony, address fromCycleCompleted, uint256 numerator, uint256 denominator); + + struct Skill { // total number of parent skills uint128 nParents; @@ -183,4 +191,12 @@ interface ColonyNetworkDataTypes { uint256 amount; uint256 timestamp; } + + struct ColonyDecayRate { + uint256 currentNumerator; + uint256 currentDenominator; + uint256 nextNumerator; + uint256 nextDenominator; + address afterMiningCycle; + } } diff --git a/contracts/colonyNetwork/ColonyNetworkStorage.sol b/contracts/colonyNetwork/ColonyNetworkStorage.sol index 2c7b36530f..f0f3ee89af 100644 --- a/contracts/colonyNetwork/ColonyNetworkStorage.sol +++ b/contracts/colonyNetwork/ColonyNetworkStorage.sol @@ -106,6 +106,8 @@ contract ColonyNetworkStorage is ColonyNetworkDataTypes, DSMath, CommonStorage { // Mining delegation mapping mapping(address => address) miningDelegators; // Storage slot 42 + mapping(address => ColonyDecayRate) colonyDecayRates; // Storage slot 43 + modifier calledByColony() { require(_isColony[msgSender()], "colony-caller-must-be-colony"); assert(msgSender() == msg.sender); diff --git a/contracts/colonyNetwork/IColonyNetwork.sol b/contracts/colonyNetwork/IColonyNetwork.sol index b988a3d6c7..ac1829bd88 100644 --- a/contracts/colonyNetwork/IColonyNetwork.sol +++ b/contracts/colonyNetwork/IColonyNetwork.sol @@ -476,4 +476,16 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery, IBasicMetaTransac /// If disabling, bool must be false /// @param _factor The scale factor to apply, as a WAD function setDomainReputationScaling(uint256 _domainId, bool _enabled, uint256 _factor) external; + + + /// @notice Called by a colony to set the rate at which reputation in that colony decays + /// @param _numerator The numerator of the fraction reputation does down by every reputation cycle + /// @param _denominator The denominator of the fraction reputation does down by every reputation cycle + function setColonyReputationDecayRate(uint256 _numerator, uint256 _denominator) external; + + /// @notice Called to get the rate at which reputation in a colony decays + /// @param _colony The address of the colony in question + /// @return numerator The numerator of the fraction reputation does down by every reputation cycle + /// @return denominator The denominator of the fraction reputation does down by every reputation cycle + function getColonyReputationDecayRate(address _colony) external view returns (uint256 numerator, uint256 denominator); } diff --git a/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol b/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol index 5c3fa39497..c85df94b88 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol @@ -531,7 +531,11 @@ contract ReputationMiningCycleRespond is ReputationMiningCycleCommon { // We don't care about underflows for the purposes of comparison, but for the calculation we deem 'correct'. // i.e. a reputation can't be negative. if (u[U_DECAY_TRANSITION] == 1) { - require(uint256(_disagreeStateReputationValue) == (uint256(_agreeStateReputationValue)*DECAY_NUMERATOR)/DECAY_DENOMINATOR, "colony-reputation-mining-decay-incorrect"); + uint256 numerator; + uint256 denominator; + + (numerator, denominator) = IColonyNetwork(colonyNetworkAddress).getColonyReputationDecayRate(logEntry.colony); + require(uint256(_disagreeStateReputationValue) == (uint256(_agreeStateReputationValue)*numerator)/denominator, "colony-reputation-mining-decay-incorrect"); } else { if (logEntry.amount >= 0) { // Don't allow reputation to overflow diff --git a/docs/interfaces/icolony.md b/docs/interfaces/icolony.md index 195ea509b2..9ef6ded224 100644 --- a/docs/interfaces/icolony.md +++ b/docs/interfaces/icolony.md @@ -1775,6 +1775,19 @@ Sets the skill on an existing payment. Secured function to authorised members. |_skillId|uint256|Id of the new skill to set +### ▸ `setReputationDecayRate(uint256 _numerator, uint256 _denominator)` + +Call to set the rate at which reputation in this colony decays + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|_numerator|uint256|The numerator of the fraction reputation does down by every reputation cycle +|_denominator|uint256|The denominator of the fraction reputation does down by every reputation cycle + + ### ▸ `setRewardInverse(uint256 _rewardInverse)` Set the reward inverse to pay out from revenue. e.g. if the fee is 1% (or 0.01), set 100. diff --git a/docs/interfaces/icolonynetwork.md b/docs/interfaces/icolonynetwork.md index 515130bf97..572049ec7c 100644 --- a/docs/interfaces/icolonynetwork.md +++ b/docs/interfaces/icolonynetwork.md @@ -379,6 +379,24 @@ Get the number of colonies in the network. |---|---|---| |_count|uint256|The colony count +### ▸ `getColonyReputationDecayRate(address _colony):uint256 numerator, uint256 denominator` + +Called to get the rate at which reputation in a colony decays + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|_colony|address|The address of the colony in question + +**Return Parameters** + +|Name|Type|Description| +|---|---|---| +|numerator|uint256|The numerator of the fraction reputation does down by every reputation cycle +|denominator|uint256|The denominator of the fraction reputation does down by every reputation cycle + ### ▸ `getColonyVersionResolver(uint256 _version):address _resolverAddress` Get the `Resolver` address for Colony contract version `_version`. @@ -885,6 +903,19 @@ Used to track that a user is eligible to claim a reward |_amount|uint256|The amount of CLNY to be awarded +### ▸ `setColonyReputationDecayRate(uint256 _numerator, uint256 _denominator)` + +Called by a colony to set the rate at which reputation in that colony decays + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|_numerator|uint256|The numerator of the fraction reputation does down by every reputation cycle +|_denominator|uint256|The denominator of the fraction reputation does down by every reputation cycle + + ### ▸ `setDomainReputationScaling(uint256 _domainId, bool _enabled, uint256 _factor)` Call to set the reputation scaling applied to reputation earned in a domain. diff --git a/packages/reputation-miner/ReputationMiner.js b/packages/reputation-miner/ReputationMiner.js index ec76812b68..516646becd 100644 --- a/packages/reputation-miner/ReputationMiner.js +++ b/packages/reputation-miner/ReputationMiner.js @@ -248,8 +248,8 @@ class ReputationMiner { const repCycle = await this.getActiveRepCycle(blockNumber); // Update fractions const decayFraction = await repCycle.getDecayConstant({ blockTag: blockNumber }); - this.decayNumerator = decayFraction.numerator; - this.decayDenominator = decayFraction.denominator; + this.defaultDecayNumerator = decayFraction.numerator; + this.defaultDecayDenominator = decayFraction.denominator; // Do updates this.nReputationsBeforeLatestLog = ethers.BigNumber.from(this.nReputations.toString()); @@ -362,8 +362,18 @@ class ReputationMiner { const key = await Object.keys(this.reputations)[updateNumber]; const reputation = ethers.BigNumber.from(`0x${this.reputations[key].slice(2, 66)}`); - const numerator = ethers.BigNumber.from(this.decayNumerator); - const denominator = ethers.BigNumber.from(this.decayDenominator); + let numerator = ethers.BigNumber.from(this.defaultDecayNumerator); + let denominator = ethers.BigNumber.from(this.defaultDecayDenominator); + + try { + const keyElements = ReputationMiner.breakKeyInToElements(key); + const [colonyAddress, ,] = keyElements; + const colonyDecay = await this.colonyNetwork.getColonyReputationDecayRate(colonyAddress, { blockTag: blockNumber }); + numerator = colonyDecay.numerator; + denominator = colonyDecay.denominator; + } catch (err) { + // Update not deployed at that block number, use the default + } const newReputation = reputation.mul(numerator).div(denominator); const reputationChange = newReputation.sub(reputation); diff --git a/scripts/solhint.sh b/scripts/solhint.sh index 26aff7ee30..8dd4e0f9ec 100644 --- a/scripts/solhint.sh +++ b/scripts/solhint.sh @@ -2,9 +2,12 @@ for file in $(git diff --cached --name-only | grep -E '\.sol$') do - git show ":$file" | node_modules/.bin/solhint stdin "$file" # we only want to lint the staged changes, not any un-staged changes + echo $file + git show ":$file" > $file.staged && node_modules/.bin/solhint $file.staged # we only want to lint the staged changes, not any un-staged changes if [ $? -ne 0 ]; then echo "Solhint failed on staged file '$file'." + rm $file.staged exit 1 # exit with failure status fi + rm $file.staged done diff --git a/test-smoke/colony-storage-consistent.js b/test-smoke/colony-storage-consistent.js index 0fcc7d73e6..11351e6f1c 100644 --- a/test-smoke/colony-storage-consistent.js +++ b/test-smoke/colony-storage-consistent.js @@ -149,11 +149,11 @@ contract("Contract Storage", (accounts) => { console.log("miningCycleStateHash:", miningCycleStateHash); console.log("tokenLockingStateHash:", tokenLockingStateHash); - expect(colonyNetworkStateHash).to.equal("0x2d18729c1ac505ad7696122d0bf49080dfc6b1bf646d958fc1f4acb7285c2985"); - expect(colonyStateHash).to.equal("0x926759d6b953cc0fc91637acdea92c42bd94339f78df20cad626d3e3b75d5557"); - expect(metaColonyStateHash).to.equal("0x2d42020b137f88d9ee72f532790059e09fb5a140e1af9002f84be644f7c5c629"); - expect(miningCycleStateHash).to.equal("0xb9d7927e2ed6207a12fd15f4a42b76e28132babcc5e7d77919c998691bbcd1f6"); - expect(tokenLockingStateHash).to.equal("0x13f377fbcf739a58988fb62dec44e0d9d880af7d454678e11f7d88e29db2437f"); + expect(colonyNetworkStateHash).to.equal("0xc8148473449842c2dc31bc7ab0b793cec360fcbee103dcabf871534c68596fe3"); + expect(colonyStateHash).to.equal("0xb720af45b238d03f5a7f3d5c92e8124db57e6609e41f3345dd6ba1568d497135"); + expect(metaColonyStateHash).to.equal("0x11fdffbcb4f398a4ad00450425363ecb146bf9f1b102150990f54fdcda5fb066"); + expect(miningCycleStateHash).to.equal("0x191d67384a6d310742cf57894bcdbc59caa05912dcc795a3dab9f58bc5e58fa2"); + expect(tokenLockingStateHash).to.equal("0x159da4749253532162d9757e5abffc645df23b10e0eb8decef050622f30c794f"); }); }); }); diff --git a/test/contracts-network/colony.js b/test/contracts-network/colony.js index 3e7bdcfe9c..18cfd981dc 100755 --- a/test/contracts-network/colony.js +++ b/test/contracts-network/colony.js @@ -16,7 +16,15 @@ const { WAD, ADDRESS_ZERO, } = require("../../helpers/constants"); -const { getTokenArgs, web3GetBalance, checkErrorRevert, expectNoEvent, expectAllEvents, expectEvent } = require("../../helpers/test-helper"); +const { + getTokenArgs, + web3GetBalance, + checkErrorRevert, + expectNoEvent, + expectAllEvents, + expectEvent, + advanceMiningCycleNoContest, +} = require("../../helpers/test-helper"); const { makeTask, setupRandomColony, getMetaTransactionParameters } = require("../../helpers/test-data-generator"); const { expect } = chai; @@ -456,6 +464,18 @@ contract("Colony", (accounts) => { await checkErrorRevert(colony.setReputationDecayRate(1, 1, { from: USER1 }), "ds-auth-unauthorized"); }); + it("should emit an event when the decay rate is set", async () => { + const tx = await colony.setReputationDecayRate(1, 1, { from: USER0 }); + const activeReputationMiningCycleAddress = await colonyNetwork.getReputationMiningCycle(true); + + await expectEvent(tx, "ColonyReputationDecayRateToChange(address,address,uint256,uint256)", [ + colony.address, + activeReputationMiningCycleAddress, + 1, + 1, + ]); + }); + it("a colony that hasn't had the decay rate explicitly set returns the default decay rate", async () => { const res = await colonyNetwork.getColonyReputationDecayRate(colony.address); @@ -484,9 +504,11 @@ contract("Colony", (accounts) => { }); it("a colony's decay rate is set, it cannot be set to an invalid value", async () => { - await checkErrorRevert(colony.setReputationDecayRate(2, 1, { from: USER0 }), "ds-auth-unauthorized"); - await checkErrorRevert(colony.setReputationDecayRate("1000000000000000000", "1000000000000000000", { from: USER0 }), "ds-auth-unauthorized"); - await checkErrorRevert(colony.setReputationDecayRate(1, 0, { from: USER0 }), "ds-auth-unauthorized"); + await checkErrorRevert(colony.setReputationDecayRate(2, 1, { from: USER0 }), "colony-network-decay-rate-over-1"); + await checkErrorRevert( + colony.setReputationDecayRate("1000000000000000000", "1000000000000000000", { from: USER0 }), + "colony-network-decay-numerator-too-big" + ); }); it("a colony can return to following the default decay rate", async () => { diff --git a/test/reputation-system/happy-paths.js b/test/reputation-system/happy-paths.js index 33208a79bd..53bdc1cd2b 100644 --- a/test/reputation-system/happy-paths.js +++ b/test/reputation-system/happy-paths.js @@ -452,6 +452,46 @@ contract("Reputation Mining - happy paths", (accounts) => { ); }); + it("should calculate reputation decays correctly if the colony is using a custom decay rate", async () => { + await giveUserCLNYTokensAndStake(colonyNetwork, MINER2, DEFAULT_STAKE); + await metaColony.setReputationDecayRate(1, 2); + await advanceMiningCycleNoContest({ colonyNetwork, test: this }); + + const badClient = new MaliciousReputationMinerExtraRep({ loader, realProviderPort, useJsTree, minerAddress: MINER2 }, 1, new BN("10")); + await badClient.initialise(colonyNetwork.address); + + const skillId = GLOBAL_SKILL_ID; + const globalKey = ReputationMinerTestWrapper.getKey(metaColony.address, skillId, ethers.constants.AddressZero); + const userKey = ReputationMinerTestWrapper.getKey(metaColony.address, skillId, MINER1); + + await goodClient.insert(globalKey, INT128_MAX.subn(1), 0); + await goodClient.insert(userKey, INT128_MAX.subn(1), 0); + await badClient.insert(globalKey, INT128_MAX.subn(1), 0); + await badClient.insert(userKey, INT128_MAX.subn(1), 0); + + const rootHash = await goodClient.getRootHash(); + let repCycle = await getActiveRepCycle(colonyNetwork); + await forwardTime(MINING_CYCLE_DURATION + CHALLENGE_RESPONSE_WINDOW_DURATION + 1, this); + await repCycle.submitRootHash(rootHash, 2, "0x00", 10, { from: MINER1 }); + await repCycle.confirmNewHash(0, { from: MINER1 }); + + repCycle = await getActiveRepCycle(colonyNetwork); + await submitAndForwardTimeToDispute([goodClient, badClient], this); + await accommodateChallengeAndInvalidateHash(colonyNetwork, this, goodClient, badClient, { + client2: { respondToChallenge: "colony-reputation-mining-decay-incorrect" }, + }); + await forwardTime(CHALLENGE_RESPONSE_WINDOW_DURATION + 1, this); + await repCycle.confirmNewHash(1, { from: MINER1 }); + + const expectedResult = INT128_MAX.subn(1).muln(1).divn(2); + const decayKey = ReputationMinerTestWrapper.getKey(metaColony.address, skillId, MINER1); + const decimalValueDecay = new BN(goodClient.reputations[decayKey].slice(2, 66), 16); + + expect(expectedResult.toString(16, 64), `Incorrect decay. Actual value is ${decimalValueDecay}`).to.equal( + goodClient.reputations[decayKey].slice(2, 66) + ); + }); + it("should keep reputation updates that occur during one update window for the next window", async () => { // Creates an entry in the reputation log for the worker and manager await fundColonyWithTokens(metaColony, clnyToken);