From 2be8b864bfb634e07f0616e5c9a42b1e178f72ce Mon Sep 17 00:00:00 2001 From: Yondon Fu Date: Fri, 29 Dec 2017 13:57:29 -0500 Subject: [PATCH] Percentage calculation moved to MathUtils library --- contracts/Manager.sol | 3 -- contracts/bonding/BondingManager.sol | 9 ++-- contracts/bonding/libraries/TokenPools.sol | 21 +++----- contracts/jobs/JobsManager.sol | 9 ++-- contracts/libraries/MathUtils.sol | 47 +++++++++++++++++ contracts/rounds/RoundsManager.sol | 6 ++- contracts/token/Minter.sol | 18 +++---- migrations/2_deploy_libraries.js | 14 ++++- test/unit/BondingManager.js | 60 ++++++++++------------ test/unit/TestMathUtils.sol | 29 +++++++++++ 10 files changed, 146 insertions(+), 70 deletions(-) create mode 100644 contracts/libraries/MathUtils.sol create mode 100644 test/unit/TestMathUtils.sol diff --git a/contracts/Manager.sol b/contracts/Manager.sol index 1b6de538..bd9ef7b3 100644 --- a/contracts/Manager.sol +++ b/contracts/Manager.sol @@ -8,9 +8,6 @@ contract Manager is IManager { // Controller that contract is registered with IController public controller; - // Divisor used for representing percentages - uint256 public constant PERC_DIVISOR = 1000000; - // Check if sender is controller modifier onlyController() { require(msg.sender == address(controller)); diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index b9ca2b23..5c69b73a 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -3,6 +3,7 @@ pragma solidity ^0.4.17; import "../ManagerProxyTarget.sol"; import "./IBondingManager.sol"; import "../libraries/SortedDoublyLL.sol"; +import "../libraries/MathUtils.sol"; import "./libraries/TokenPools.sol"; import "../token/ILivepeerToken.sol"; import "../token/IMinter.sol"; @@ -165,9 +166,9 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { // Only callable if current round is not locked require(!roundsManager().currentRoundLocked()); // Block reward cut must be a valid percentage - require(_blockRewardCut <= PERC_DIVISOR); + require(MathUtils.validPerc(_blockRewardCut)); // Fee share must be a valid percentage - require(_feeShare <= PERC_DIVISOR); + require(MathUtils.validPerc(_feeShare)); Transcoder storage t = transcoders[msg.sender]; t.pendingBlockRewardCut = _blockRewardCut; @@ -471,7 +472,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { // Transcoder must be valid require(transcoderStatus(_transcoder) == TranscoderStatus.Registered); - uint256 penalty = delegators[_transcoder].bondedAmount.mul(_slashAmount).div(PERC_DIVISOR); + uint256 penalty = MathUtils.percOf(delegators[_transcoder].bondedAmount, _slashAmount); if (penalty > del.bondedAmount) { penalty = del.bondedAmount; } @@ -503,7 +504,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { if (_finder != address(0)) { // Award finder fee - uint256 finderAmount = penalty.mul(_finderFee).div(PERC_DIVISOR); + uint256 finderAmount = MathUtils.percOf(penalty, _finderFee); minter().transferTokens(_finder, finderAmount); burnAmount = burnAmount.sub(finderAmount); diff --git a/contracts/bonding/libraries/TokenPools.sol b/contracts/bonding/libraries/TokenPools.sol index 4c7b9256..cd0f1d99 100644 --- a/contracts/bonding/libraries/TokenPools.sol +++ b/contracts/bonding/libraries/TokenPools.sol @@ -1,13 +1,13 @@ pragma solidity ^0.4.17; +import "../../libraries/MathUtils.sol"; + import "zeppelin-solidity/contracts/math/SafeMath.sol"; library TokenPools { using SafeMath for uint256; - uint256 public constant PERC_DIVISOR = 1000000; - // Represents rewards and fees to be distributed to delegators struct Data { uint256 rewardPool; // Rewards in the pool @@ -56,12 +56,9 @@ library TokenPools { uint256 delegatorFees = 0; if (tokenPools.claimableStake > 0) { - transcoderFees = tokenPools.feePool.mul(PERC_DIVISOR.sub(tokenPools.transcoderFeeShare)).div(PERC_DIVISOR); - - // Compute delegator's claimable stake percentage - uint256 percPoints = _stake.mul(PERC_DIVISOR).div(tokenPools.claimableStake); - // Compute delegator's fees according to claimable stake percentage - delegatorFees = tokenPools.feePool.sub(transcoderFees).mul(percPoints).div(PERC_DIVISOR); + uint256 delegatorsFees = MathUtils.percOf(tokenPools.feePool, tokenPools.transcoderFeeShare); + transcoderFees = tokenPools.feePool.sub(delegatorsFees); + delegatorFees = MathUtils.percOf(delegatorsFees, _stake, tokenPools.claimableStake); } if (_isTranscoder) { @@ -76,12 +73,8 @@ library TokenPools { uint256 delegatorRewards = 0; if (tokenPools.claimableStake > 0) { - transcoderRewards = tokenPools.rewardPool.mul(tokenPools.transcoderBlockRewardCut).div(PERC_DIVISOR); - - // Compute delegator's claimable stake percentage - uint256 percPoints = _stake.mul(PERC_DIVISOR).div(tokenPools.claimableStake); - // Compute delegator's rewards according to claimable stake percentage - delegatorRewards = tokenPools.rewardPool.sub(transcoderRewards).mul(percPoints).div(PERC_DIVISOR); + transcoderRewards = MathUtils.percOf(tokenPools.rewardPool, tokenPools.transcoderBlockRewardCut); + delegatorRewards = MathUtils.percOf(tokenPools.rewardPool.sub(transcoderRewards), _stake, tokenPools.claimableStake); } if (_isTranscoder) { diff --git a/contracts/jobs/JobsManager.sol b/contracts/jobs/JobsManager.sol index 6c605299..79fa3578 100644 --- a/contracts/jobs/JobsManager.sol +++ b/contracts/jobs/JobsManager.sol @@ -9,6 +9,7 @@ import "../bonding/IBondingManager.sol"; import "../rounds/IRoundsManager.sol"; import "../verification/IVerifiable.sol"; import "../verification/IVerifier.sol"; +import "../libraries/MathUtils.sol"; import "zeppelin-solidity/contracts/math/SafeMath.sol"; @@ -187,7 +188,7 @@ contract JobsManager is ManagerProxyTarget, IVerifiable, IJobsManager { */ function setFailedVerificationSlashAmount(uint256 _failedVerificationSlashAmount) external onlyControllerOwner { // Must be a valid percentage - require(_failedVerificationSlashAmount <= PERC_DIVISOR); + require(MathUtils.validPerc(_failedVerificationSlashAmount)); failedVerificationSlashAmount = _failedVerificationSlashAmount; @@ -200,7 +201,7 @@ contract JobsManager is ManagerProxyTarget, IVerifiable, IJobsManager { */ function setMissedVerificationSlashAmount(uint256 _missedVerificationSlashAmount) external onlyControllerOwner { // Must be a valid percentage - require(_missedVerificationSlashAmount <= PERC_DIVISOR); + require(MathUtils.validPerc(_missedVerificationSlashAmount)); missedVerificationSlashAmount = _missedVerificationSlashAmount; @@ -213,7 +214,7 @@ contract JobsManager is ManagerProxyTarget, IVerifiable, IJobsManager { */ function setDoubleClaimSegmentSlashAmount(uint256 _doubleClaimSegmentSlashAmount) external onlyControllerOwner { // Must be a valid percentage - require(_doubleClaimSegmentSlashAmount <= PERC_DIVISOR); + require(MathUtils.validPerc(_doubleClaimSegmentSlashAmount)); doubleClaimSegmentSlashAmount = _doubleClaimSegmentSlashAmount; @@ -226,7 +227,7 @@ contract JobsManager is ManagerProxyTarget, IVerifiable, IJobsManager { */ function setFinderFee(uint256 _finderFee) external onlyControllerOwner { // Must be a valid percentage - require(_finderFee <= PERC_DIVISOR); + require(MathUtils.validPerc(_finderFee)); finderFee = _finderFee; } diff --git a/contracts/libraries/MathUtils.sol b/contracts/libraries/MathUtils.sol new file mode 100644 index 00000000..dca5c6ad --- /dev/null +++ b/contracts/libraries/MathUtils.sol @@ -0,0 +1,47 @@ +pragma solidity ^0.4.17; + +import "zeppelin-solidity/contracts/math/SafeMath.sol"; + + +library MathUtils { + using SafeMath for uint256; + + // Divisor used for representing percentages + uint256 public constant PERC_DIVISOR = 1000000; + + /* + * @dev Returns whether an amount is a valid percentage out of PERC_DIVISOR + * @param _amount Amount that is supposed to be a percentage + */ + function validPerc(uint256 _amount) internal view returns (bool) { + return _amount <= PERC_DIVISOR; + } + + /* + * @dev Compute percentage of a value with the percentage represented by a fraction + * @param _amount Amount to take the percentage of + * @param _fracNum Numerator of fraction representing the percentage + * @param _fracDenom Denominator of fraction representing the percentage + */ + function percOf(uint256 _amount, uint256 _fracNum, uint256 _fracDenom) internal view returns (uint256) { + return _amount.mul(percPoints(_fracNum, _fracDenom)).div(PERC_DIVISOR); + } + + /* + * @dev Compute percentage of a value with the percentage represented by a fraction over PERC_DIVISOR + * @param _amount Amount to take the percentage of + * @param _fracNum Numerator of fraction representing the percentage with PERC_DIVISOR as the denominator + */ + function percOf(uint256 _amount, uint256 _fracNum) internal view returns (uint256) { + return _amount.mul(_fracNum).div(PERC_DIVISOR); + } + + /* + * @dev Compute percentage representation of a fraction + * @param _fracNum Numerator of fraction represeting the percentage + * @param _fracDenom Denominator of fraction represeting the percentage + */ + function percPoints(uint256 _fracNum, uint256 _fracDenom) internal view returns (uint256) { + return _fracNum.mul(PERC_DIVISOR).div(_fracDenom); + } +} diff --git a/contracts/rounds/RoundsManager.sol b/contracts/rounds/RoundsManager.sol index 15e67b47..47b575f6 100644 --- a/contracts/rounds/RoundsManager.sol +++ b/contracts/rounds/RoundsManager.sol @@ -4,6 +4,7 @@ import "../ManagerProxyTarget.sol"; import "./IRoundsManager.sol"; import "../bonding/IBondingManager.sol"; import "../token/IMinter.sol"; +import "../libraries/MathUtils.sol"; import "zeppelin-solidity/contracts/math/SafeMath.sol"; @@ -31,7 +32,7 @@ contract RoundsManager is ManagerProxyTarget, IRoundsManager { */ function setParameters(uint256 _roundLength, uint256 _roundLockAmount) external onlyControllerOwner { // Must be a valid percentage - require(_roundLockAmount <= PERC_DIVISOR); + require(MathUtils.validPerc(_roundLockAmount)); roundLength = _roundLength; roundLockAmount = _roundLockAmount; @@ -103,7 +104,8 @@ contract RoundsManager is ManagerProxyTarget, IRoundsManager { * @dev Check if we are in the lock period of the current round */ function currentRoundLocked() public view returns (bool) { - return blockNum().sub(currentRoundStartBlock()) >= roundLength.sub(roundLength.mul(roundLockAmount).div(PERC_DIVISOR)); + uint256 lockedBlocks = MathUtils.percOf(roundLength, roundLockAmount); + return blockNum().sub(currentRoundStartBlock()) >= roundLength.sub(lockedBlocks); } /* diff --git a/contracts/token/Minter.sol b/contracts/token/Minter.sol index 706b02c4..6e017722 100644 --- a/contracts/token/Minter.sol +++ b/contracts/token/Minter.sol @@ -5,6 +5,7 @@ import "./IMinter.sol"; import "./ILivepeerToken.sol"; import "../rounds/IRoundsManager.sol"; import "../bonding/IBondingManager.sol"; +import "../libraries/MathUtils.sol"; import "zeppelin-solidity/contracts/math/SafeMath.sol"; @@ -46,11 +47,11 @@ contract Minter is Manager, IMinter { function Minter(address _controller, uint256 _inflation, uint256 _inflationChange, uint256 _targetBondingRate) public Manager(_controller) { // Inflation must be valid percentage - require(_inflation <= PERC_DIVISOR); + require(MathUtils.validPerc(_inflation)); // Inflation change must be valid percentage - require(_inflationChange <= PERC_DIVISOR); + require(MathUtils.validPerc(_inflationChange)); // Target bonding rate must be valid percentage - require(_targetBondingRate <= PERC_DIVISOR); + require(MathUtils.validPerc(_targetBondingRate)); inflation = _inflation; inflationChange = _inflationChange; @@ -59,7 +60,7 @@ contract Minter is Manager, IMinter { function setTargetBondingRate(uint256 _targetBondingRate) external onlyControllerOwner { // Must be valid percentage - require(_targetBondingRate <= PERC_DIVISOR); + require(MathUtils.validPerc(_targetBondingRate)); targetBondingRate = _targetBondingRate; @@ -76,10 +77,8 @@ contract Minter is Manager, IMinter { * @param _fracDenom Denominator of fraction (total active stake) */ function createReward(uint256 _fracNum, uint256 _fracDenom) external onlyBondingManager whenSystemNotPaused returns (uint256) { - uint256 percPoints = _fracNum.mul(PERC_DIVISOR).div(_fracDenom); - // Compute and mint fraction of mintable tokens to include in reward - uint256 mintAmount = currentMintableTokens.mul(percPoints).div(PERC_DIVISOR); + uint256 mintAmount = MathUtils.percOf(currentMintableTokens, _fracNum, _fracDenom); // Update amount of minted tokens for round currentMintedTokens = currentMintedTokens.add(mintAmount); // Minted tokens must not exceed mintable tokens @@ -128,7 +127,8 @@ contract Minter is Manager, IMinter { uint256 totalSupply = livepeerToken().totalSupply(); if (totalSupply > 0) { - currentBondingRate = (bondingManager().getTotalBonded() * PERC_DIVISOR) / totalSupply; + uint256 totalBonded = bondingManager().getTotalBonded(); + currentBondingRate = MathUtils.percPoints(totalBonded, totalSupply); } if (currentBondingRate < targetBondingRate) { @@ -151,7 +151,7 @@ contract Minter is Manager, IMinter { */ function mintedTokensPerRound() internal view returns (uint256) { uint256 currentSupply = livepeerToken().totalSupply(); - return currentSupply.mul(inflation).div(PERC_DIVISOR); + return MathUtils.percOf(currentSupply, inflation); } /* diff --git a/migrations/2_deploy_libraries.js b/migrations/2_deploy_libraries.js index 85f063a5..da9cfb3f 100644 --- a/migrations/2_deploy_libraries.js +++ b/migrations/2_deploy_libraries.js @@ -4,7 +4,9 @@ const MerkleProof = artifacts.require("MerkleProof") const ECRecovery = artifacts.require("ECRecovery") const JobLib = artifacts.require("JobLib") const SafeMath = artifacts.require("SafeMath") +const MathUtils = artifacts.require("MathUtils") +const Minter = artifacts.require("Minter") const JobsManager = artifacts.require("JobsManager") const BondingManager = artifacts.require("BondingManager") const RoundsManager = artifacts.require("RoundsManager") @@ -17,7 +19,17 @@ module.exports = function(deployer) { JobsManager, RoundsManager, OraclizeVerifier, - SortedDoublyLL + SortedDoublyLL, + MathUtils + ]) + + deployer.deploy(MathUtils) + deployer.link(MathUtils, [ + BondingManager, + JobsManager, + RoundsManager, + Minter, + TokenPools ]) deployer.deploy(SortedDoublyLL) diff --git a/test/unit/BondingManager.js b/test/unit/BondingManager.js index 75d70978..06d17b2c 100644 --- a/test/unit/BondingManager.js +++ b/test/unit/BondingManager.js @@ -402,37 +402,29 @@ contract("BondingManager", accounts => { // Transcoder calls reward await bondingManager.reward({from: tAddr}) - // 15 - const delegatorsFeeShare1 = Math.floor((fees * feeShare) / PERC_DIVISOR) - // 7 - const delegatorFeeShare1 = Math.floor((2000 * delegatorsFeeShare1) / transcoderTotalStake) - - // 450 - const delegatorsRewardShare1 = Math.floor((mintedTokens * (PERC_DIVISOR - blockRewardCut)) / PERC_DIVISOR) - // 225 - const delegatorRewardShare1 = Math.floor((2000 * delegatorsRewardShare1) / transcoderTotalStake) - - // 20 - const delegatorsFeeShare2 = Math.floor((fees2 * feeShare) / PERC_DIVISOR) - // 9 - const delegatorFeeShare2 = Math.floor((add(2000, delegatorRewardShare1) * delegatorsFeeShare2) / transcoderTotalStake2) - - // 540 - const delegatorsRewardShare2 = Math.floor((mintedTokens2 * (PERC_DIVISOR - blockRewardCut)) / PERC_DIVISOR) - // 267 - const delegatorRewardShare2 = Math.floor((add(2000, delegatorRewardShare1).toNumber() * delegatorsRewardShare2) / transcoderTotalStake2) - - // 2492 - const expDelegatorStake = add(2000, delegatorRewardShare1, delegatorRewardShare2).toString() - // 18 - const expUnbondedAmount = add(delegatorFeeShare1, delegatorFeeShare2).toString() + const percPoints1 = Math.floor((2000 * PERC_DIVISOR) / transcoderTotalStake) + const transcoderFeeShare1 = Math.floor((fees * (PERC_DIVISOR - feeShare)) / PERC_DIVISOR) + const delegatorsFeeShare1 = fees - transcoderFeeShare1 + const delegatorFeeShare1 = Math.floor((percPoints1 * delegatorsFeeShare1) / PERC_DIVISOR) + const transcoderRewardShare1 = Math.floor((mintedTokens * blockRewardCut) / PERC_DIVISOR) + const delegatorsRewardShare1 = mintedTokens - transcoderRewardShare1 + const delegatorRewardShare1 = Math.floor((percPoints1 * delegatorsRewardShare1) / PERC_DIVISOR) + + const percPoints2 = Math.floor((add(2000, delegatorRewardShare1) * PERC_DIVISOR) / transcoderTotalStake2) + const transcoderFeeShare2 = Math.floor((fees2 * (PERC_DIVISOR - feeShare)) / PERC_DIVISOR) + const delegatorsFeeShare2 = fees2 - transcoderFeeShare2 + const delegatorFeeShare2 = Math.floor((percPoints2 * delegatorsFeeShare2) / PERC_DIVISOR) + const transcoderRewardShare2 = Math.floor((mintedTokens2 * blockRewardCut) / PERC_DIVISOR) + const delegatorsRewardShare2 = mintedTokens2 - transcoderRewardShare2 + const delegatorRewardShare2 = Math.floor((percPoints2 * delegatorsRewardShare2) / PERC_DIVISOR) + + const expDelegatorStake = add(2000, delegatorRewardShare1, delegatorRewardShare2) + const expUnbondedAmount = add(delegatorFeeShare1, delegatorFeeShare2) await bondingManager.claimTokenPoolsShares(8, {from: dAddr}) const dInfo = await bondingManager.getDelegator(dAddr) - const delegatorStake = dInfo[0] - assert.equal(delegatorStake.toString(), expDelegatorStake, "delegator stake incorrect") - const unbondedAmount = dInfo[1] - assert.equal(unbondedAmount.toString(), expUnbondedAmount, "delegator unbonded amount incorrect") + assert.equal(dInfo[0].toString(), expDelegatorStake, "delegator stake incorrect") + assert.equal(dInfo[1].toString(), expUnbondedAmount, "delegator unbonded amount incorrect") }) it("should update delegator's stake and unbonded amount through the end round with a larger portion of rewards and fees after another delegator unbonds before the rewards and fees are released", async () => { @@ -473,11 +465,13 @@ contract("BondingManager", accounts => { // Get Delegator 1 unbonded amount const unbondedAmount = (await bondingManager.getDelegator(dAddr))[1] - const delegatorsFeeShare = Math.floor((fees2 * feeShare) / PERC_DIVISOR) - const delegatorFeeShare = Math.floor((delegatorStake * delegatorsFeeShare) / claimableStake) - - const delegatorsRewardShare = Math.floor((mintedTokens2 * (PERC_DIVISOR - blockRewardCut)) / PERC_DIVISOR) - const delegatorRewardShare = Math.floor((delegatorStake * delegatorsRewardShare) / claimableStake) + const percPoints = Math.floor((delegatorStake * PERC_DIVISOR) / claimableStake) + const transcoderFeeShare = Math.floor((fees2 * (PERC_DIVISOR - feeShare)) / PERC_DIVISOR) + const delegatorsFeeShare = fees2 - transcoderFeeShare + const delegatorFeeShare = Math.floor((percPoints * delegatorsFeeShare) / PERC_DIVISOR) + const transcoderRewardShare = Math.floor((mintedTokens2 * blockRewardCut) / PERC_DIVISOR) + const delegatorsRewardShare = mintedTokens2 - transcoderRewardShare + const delegatorRewardShare = Math.floor((percPoints * delegatorsRewardShare) / PERC_DIVISOR) const expDelegatorStake = add(delegatorStake, delegatorRewardShare).toString() const expUnbondedAmount = add(unbondedAmount, delegatorFeeShare).toString() diff --git a/test/unit/TestMathUtils.sol b/test/unit/TestMathUtils.sol new file mode 100644 index 00000000..29484d05 --- /dev/null +++ b/test/unit/TestMathUtils.sol @@ -0,0 +1,29 @@ +pragma solidity ^0.4.17; + +import "../../contracts/libraries/MathUtils.sol"; +import "truffle/Assert.sol"; + + +contract TestMathUtils { + function test_validPerc() public { + Assert.equal(MathUtils.validPerc(50), true, "50 should be a valid percentage"); + Assert.equal(MathUtils.validPerc(0), true, "0 should be a valid percentage"); + Assert.equal(MathUtils.validPerc(1000000), true, "the max should be a valid percentage"); + Assert.equal(MathUtils.validPerc(1000001), false, "1 more than the max should not be valid percentage"); + } + + function test_percOf1() public { + Assert.equal(MathUtils.percOf(100, 3, 4), 75, "3/4 of 100 should be 75"); + Assert.equal(MathUtils.percOf(100, 7, 9), 77, "7/9 of 100 should be 77"); + } + + function test_percOf2() public { + Assert.equal(MathUtils.percOf(100, 3), 0, ".0003% of 100 is 0"); + Assert.equal(MathUtils.percOf(100, 100000), 10, "10% of 100 is 10"); + } + + function test_percPoints() public { + Assert.equal(MathUtils.percPoints(3, 4), 750000, "3/4 should convert to valid percentage"); + Assert.equal(MathUtils.percPoints(100, 300), 333333, "100/300 should convert to valid percentage"); + } +}