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);
+ });
+ });
+});