From abf0f20dedfe3a4bf90e61cf1bc9ea5feae57749 Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Fri, 9 Jun 2023 11:58:57 +0100 Subject: [PATCH] Factor out reputation scaling function --- contracts/colony/Colony.sol | 3 +- contracts/colony/ColonyFunding.sol | 12 ++-- contracts/colony/ColonyStorage.sol | 31 +--------- contracts/colonyNetwork/ColonyNetwork.sol | 28 +-------- contracts/common/ScaleReputation.sol | 54 ++++++++++++++++ contracts/testHelpers/ScaleReputationTest.sol | 26 ++++++++ .../common-scale-reputation.js | 61 +++++++++++++++++++ 7 files changed, 154 insertions(+), 61 deletions(-) create mode 100644 contracts/common/ScaleReputation.sol create mode 100644 contracts/testHelpers/ScaleReputationTest.sol create mode 100644 test/contracts-network/common-scale-reputation.js diff --git a/contracts/colony/Colony.sol b/contracts/colony/Colony.sol index 4026bd2fa5..a172a49231 100755 --- a/contracts/colony/Colony.sol +++ b/contracts/colony/Colony.sol @@ -114,7 +114,8 @@ contract Colony is BasicMetaTransaction, Multicall, ColonyStorage, PatriciaTreeP // After doing all the local storage changes, then do all the external calls for (uint256 i = 0; i < _users.length; i++) { require(ERC20Extended(token).transfer(_users[i], uint256(_amounts[i])), "colony-bootstrap-token-transfer-failed"); - int256 tokenScaledReputationAmount = getTokenScaledReputation(_amounts[i], token); + uint256 scaleFactor = tokenReputationRates[token]; // NB This is a WAD + int256 tokenScaledReputationAmount = scaleReputation(_amounts[i], scaleFactor); IColonyNetwork(colonyNetworkAddress).appendReputationUpdateLog(_users[i], tokenScaledReputationAmount, domains[1].skillId); } diff --git a/contracts/colony/ColonyFunding.sol b/contracts/colony/ColonyFunding.sol index 9cad4b78d7..7efc6b603b 100755 --- a/contracts/colony/ColonyFunding.sol +++ b/contracts/colony/ColonyFunding.sol @@ -180,11 +180,10 @@ contract ColonyFunding is ColonyStorage { // ignore-swc-123 reputation = negative ? reputation + payout : reputation - payout; } + uint256 scaleFactor = tokenReputationRates[tokenAddress]; // NB This is a WAD // We may lose one atom of reputation here :sad: - return getTokenScaledReputation( - int256(reputation / 2) * (negative ? int256(-1) : int256(1)), - tokenAddress - ); + + return scaleReputation(int256(reputation / 2) * (negative ? int256(-1) : int256(1)), scaleFactor); } /// @notice For owners to update payouts with one token and many slots @@ -272,7 +271,7 @@ contract ColonyFunding is ColonyStorage { // ignore-swc-123 // Process reputation updates if relevant for token being paid out if (tokenReputationRates[_token] > 0 && !isExtension(slot.recipient)) { IColonyNetwork colonyNetworkContract = IColonyNetwork(colonyNetworkAddress); - int256 tokenScaledReputationAmount = getTokenScaledReputation(int256(repPayout), _token); + int256 tokenScaledReputationAmount = scaleReputation(int256(repPayout), tokenReputationRates[_token]); colonyNetworkContract.appendReputationUpdateLog(slot.recipient, tokenScaledReputationAmount, domains[expenditure.domainId].skillId); if (slot.skills.length > 0 && slot.skills[0] > 0) { @@ -313,7 +312,8 @@ contract ColonyFunding is ColonyStorage { // ignore-swc-123 if (!isExtension(payment.recipient)) { - int256 tokenScaledReputationAmount = getTokenScaledReputation(int256(fundingPot.payouts[_token]), _token); + uint256 scaleFactor = tokenReputationRates[_token]; // NB This is a WAD + int256 tokenScaledReputationAmount = scaleReputation(int256(fundingPot.payouts[_token]), scaleFactor); // Todo: Is this equality right? if (tokenScaledReputationAmount > 0){ diff --git a/contracts/colony/ColonyStorage.sol b/contracts/colony/ColonyStorage.sol index eb686731d3..43ef36a051 100755 --- a/contracts/colony/ColonyStorage.sol +++ b/contracts/colony/ColonyStorage.sol @@ -19,6 +19,7 @@ pragma solidity 0.8.20; pragma experimental ABIEncoderV2; import "./../../lib/dappsys/math.sol"; +import "./../common/ScaleReputation.sol"; import "./../common/CommonStorage.sol"; import "./../common/ERC20Extended.sol"; import "./../colonyNetwork/IColonyNetwork.sol"; @@ -31,7 +32,7 @@ import "./ColonyDataTypes.sol"; // ignore-file-swc-108 -contract ColonyStorage is ColonyDataTypes, ColonyNetworkDataTypes, DSMath, CommonStorage { +contract ColonyStorage is ColonyDataTypes, ColonyNetworkDataTypes, DSMath, CommonStorage, ScaleReputation { uint256 constant COLONY_NETWORK_SLOT = 6; uint256 constant ROOT_LOCAL_SKILL_SLOT = 36; @@ -360,32 +361,4 @@ contract ColonyStorage is ColonyDataTypes, ColonyNetworkDataTypes, DSMath, Commo success := call(gas(), to, value, add(data, 0x20), mload(data), 0, 0) } } - - - uint256 constant INT128_MAX_AS_UINT256 = uint256(uint128(type(int128).max)); - - function getTokenScaledReputation(int256 _amount, address _token) internal view returns (int256) { - uint256 scaleFactor = tokenReputationRates[_token]; // NB This is a WAD - if (scaleFactor == 0) { return 0; } - - // Check if too large for scaling - int256 amount; - int256 absAmount; - if (_amount == type(int256).min){ - absAmount = type(int256).max; // Off by one, but best we can do - probably gets capped anyway - } else { - absAmount = _amount >= 0 ? _amount : -_amount; - } - - int256 sgnAmount = _amount >= 0 ? int(1) : -1; - - - if (wdiv(INT128_MAX_AS_UINT256, scaleFactor) < uint256(absAmount)){ - return sgnAmount == 1 ? type(int128).max : type(int128).min; - } else { - amount = int256(wmul(scaleFactor, uint256(absAmount))) * sgnAmount; - } - - return amount; - } } diff --git a/contracts/colonyNetwork/ColonyNetwork.sol b/contracts/colonyNetwork/ColonyNetwork.sol index 6a82579e7f..aa06a32b17 100644 --- a/contracts/colonyNetwork/ColonyNetwork.sol +++ b/contracts/colonyNetwork/ColonyNetwork.sol @@ -19,13 +19,14 @@ pragma solidity 0.8.20; pragma experimental "ABIEncoderV2"; import "./../common/BasicMetaTransaction.sol"; +import "./../common/ScaleReputation.sol"; import "./../reputationMiningCycle/IReputationMiningCycle.sol"; import "./ColonyNetworkStorage.sol"; import "./../common/Multicall.sol"; import "./../colony/ColonyDataTypes.sol"; -contract ColonyNetwork is ColonyDataTypes, BasicMetaTransaction, ColonyNetworkStorage, Multicall { +contract ColonyNetwork is ColonyDataTypes, BasicMetaTransaction, ColonyNetworkStorage, Multicall, ScaleReputation { function isColony(address _colony) public view returns (bool) { return _isColony[_colony]; @@ -226,31 +227,8 @@ contract ColonyNetwork is ColonyDataTypes, BasicMetaTransaction, ColonyNetworkSt } uint256 scaleFactor = getSkillReputationScaling(_skillId); - if (scaleFactor == 0){ - // Similarly, if the amount is 0 because of scaling, we short circuit it - return; - } - - // Check if too large for scaling - int256 amount; - int256 absAmount; - if (_amount == type(int256).min){ - absAmount = type(int256).max; // Off by one, but best we can do - probably gets capped anyway - } else { - absAmount = _amount >= 0 ? _amount : -_amount; - } - int256 sgnAmount = _amount >= 0 ? int(1) : -1; - - if (type(uint256).max / scaleFactor < uint256(absAmount)){ - if (sgnAmount == 1){ - amount = type(int128).max; - } else { - amount = type(int128).min; - } - } else { - amount = int256(wmul(scaleFactor, uint256(absAmount))) * sgnAmount; - } + int256 amount = scaleReputation(_amount, scaleFactor); uint128 nParents = skills[_skillId].nParents; // We only update child skill reputation if the update is negative, otherwise just set nChildren to 0 to save gas diff --git a/contracts/common/ScaleReputation.sol b/contracts/common/ScaleReputation.sol new file mode 100644 index 0000000000..190650f61a --- /dev/null +++ b/contracts/common/ScaleReputation.sol @@ -0,0 +1,54 @@ +/* + This file is part of The Colony Network. + + The Colony Network is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + The Colony Network is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with The Colony Network. If not, see . +*/ + +pragma solidity 0.8.20; // ignore-swc-103 +import "../../lib/dappsys/math.sol"; + +contract ScaleReputation is DSMath { + // Note that scaleFactor should be a WAD. + function scaleReputation(int256 reputationAmount, uint256 scaleFactor) internal pure returns (int256) { + if (reputationAmount == 0 || scaleFactor == 0) { + return 0; + } + + int256 scaledReputation; + int256 absAmount; + + if (reputationAmount == type(int256).min){ + absAmount = type(int256).max; // Off by one, but best we can do - probably gets capped anyway + } else { + absAmount = reputationAmount >= 0 ? reputationAmount : -reputationAmount; + } + + int256 sgnAmount = reputationAmount >= 0 ? int(1) : -1; + + // Guard against overflows during calculation with wmul + if (type(uint256).max / scaleFactor < uint256(absAmount)){ + if (sgnAmount == 1){ + scaledReputation = type(int128).max; + } else { + scaledReputation = type(int128).min; + } + } else { + scaledReputation = int256(wmul(scaleFactor, uint256(absAmount))) * sgnAmount; + // Cap inside the range of int128, as we do for all reputations + scaledReputation = imax(type(int128).min, scaledReputation); + scaledReputation = imin(type(int128).max, scaledReputation); + } + return scaledReputation; + } +} diff --git a/contracts/testHelpers/ScaleReputationTest.sol b/contracts/testHelpers/ScaleReputationTest.sol new file mode 100644 index 0000000000..0e6b6bfb55 --- /dev/null +++ b/contracts/testHelpers/ScaleReputationTest.sol @@ -0,0 +1,26 @@ +/* + This file is part of The Colony Network. + + The Colony Network is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + The Colony Network is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with The Colony Network. If not, see . +*/ + +pragma solidity 0.8.20; // ignore-swc-103 +import "../common/ScaleReputation.sol"; + +contract ScaleReputationTest is ScaleReputation { + + function scaleReputationPublic(int256 reputationAmount, uint256 scaleFactor) public pure returns (int256) { + return scaleReputation(reputationAmount, scaleFactor); + } +} diff --git a/test/contracts-network/common-scale-reputation.js b/test/contracts-network/common-scale-reputation.js new file mode 100644 index 0000000000..d7dbf65ea8 --- /dev/null +++ b/test/contracts-network/common-scale-reputation.js @@ -0,0 +1,61 @@ +/* globals artifacts */ + +const chai = require("chai"); +const bnChai = require("bn-chai"); + +const { INT256_MAX, INT256_MIN, INT128_MIN, INT128_MAX, WAD } = require("../../helpers/constants"); + +const { expect } = chai; +chai.use(bnChai(web3.utils.BN)); + +const ScaleReputationTest = artifacts.require("ScaleReputationTest"); + +let scaleReputationTest; + +contract("ScaleReputation", () => { + before(async () => { + scaleReputationTest = await ScaleReputationTest.new(); + }); + + describe("when scaling reputation", () => { + it("should scale reputation up", async () => { + const scaled = await scaleReputationTest.scaleReputationPublic(100, WAD.muln(2)); + expect(scaled).to.eq.BN(200); + }); + + it("should scale reputation down", async () => { + const scaled = await scaleReputationTest.scaleReputationPublic(100, WAD.divn(2)); + expect(scaled).to.eq.BN(50); + }); + + it("should cap negatively", async () => { + const scaled = await scaleReputationTest.scaleReputationPublic(INT128_MAX.subn(10), WAD.muln(2)); + expect(scaled).to.eq.BN(INT128_MAX); + }); + + it("should cap positively", async () => { + const scaled = await scaleReputationTest.scaleReputationPublic(INT128_MIN.addn(10), WAD.muln(2)); + expect(scaled).to.eq.BN(INT128_MIN); + }); + + it("deal with calculations that would arithmetically overflow", async () => { + const scaled = await scaleReputationTest.scaleReputationPublic(INT256_MAX.subn(10), WAD.subn(1)); + expect(scaled).to.eq.BN(INT128_MAX); + }); + + it("deal with calculations that would arithmetically underflow", async () => { + const scaled = await scaleReputationTest.scaleReputationPublic(INT256_MIN.addn(10), WAD.subn(1)); + expect(scaled).to.eq.BN(INT128_MIN); + }); + + it("deal with calculations that exceed our reputation cap during calculation, but not once calculation is complete", async () => { + const scaled = await scaleReputationTest.scaleReputationPublic(INT128_MIN.addn(10), WAD.subn(1)); + expect(scaled).to.be.gt.BN(INT128_MIN); + }); + + it("deal with calculations that exceed our reputation cap during calculation, but not once calculation is complete", async () => { + const scaled = await scaleReputationTest.scaleReputationPublic(INT128_MAX.subn(10), WAD.subn(1)); + expect(scaled).to.be.lt.BN(INT128_MAX); + }); + }); +});