diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index d05e3e93dd..cdae7bce7d 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -93,7 +93,7 @@ contract MixinExchangeFees is { uint256 amount = msg.value; bytes32 poolId = getStakingPoolIdOfMaker(makerAddress); - if (poolId != 0x0) { + if (poolId != NIL_MAKER_ID) { // There is a pool associated with `makerAddress`. // TODO(dorothy-zbornak): When we have epoch locks on delegating, we could // preclude pools that have no delegated stake, since they will never have @@ -212,7 +212,7 @@ contract MixinExchangeFees is totalStakeDelegatedToPool .safeSub(stakeHeldByPoolOperator) .safeMul(REWARD_DELEGATED_STAKE_WEIGHT) - .safeDiv(PPM_ONE) + .safeDiv(PPM_DENOMINATOR) ); // store pool stats @@ -309,7 +309,6 @@ contract MixinExchangeFees is pure returns (uint256 ownerRewards) { - assert(alphaNumerator <= alphaDenominator); int256 feeRatio = LibFixedMath._toFixed(ownerFees, totalFees); int256 stakeRatio = LibFixedMath._toFixed(ownerStake, totalStake); if (feeRatio == 0 || stakeRatio == 0) { @@ -317,36 +316,37 @@ contract MixinExchangeFees is } // The cobb-doublas function has the form: - // totalRewards * feeRatio ^ alpha * stakeRatio ^ (1-alpha) - // We instead use: - // totalRewards * stakeRatio * e^(alpha * (ln(feeRatio) - ln(stakeRatio))) + // `totalRewards * feeRatio ^ alpha * stakeRatio ^ (1-alpha)` + // This is equivalent to: + // `totalRewards * stakeRatio * e^(alpha * (ln(feeRatio / stakeRatio)))` + // However, because `ln(x)` has the domain of `0 < x < 1` + // and `exp(x)` has the domain of `x < 0`, + // and fixed-point math easily overflows with multiplication, + // we will choose the following if `stakeRatio > feeRatio`: + // `totalRewards * stakeRatio / e^(alpha * (ln(stakeRatio / feeRatio)))` - // Compute e^(alpha * (ln(feeRatio) - ln(stakeRatio))) - int256 logFeeRatio = LibFixedMath._ln(feeRatio); - int256 logStakeRatio = LibFixedMath._ln(stakeRatio); - int256 n; - if (logFeeRatio <= logStakeRatio) { - n = LibFixedMath._exp( - LibFixedMath._mulDiv( - LibFixedMath._sub(logFeeRatio, logStakeRatio), - int256(alphaNumerator), - int256(alphaDenominator) - ) - ); - } else { - n = LibFixedMath._exp( - LibFixedMath._mulDiv( - LibFixedMath._sub(logStakeRatio, logFeeRatio), - int256(alphaNumerator), - int256(alphaDenominator) - ) - ); - n = LibFixedMath._invert(n); - } - // Multiply the above with totalRewards * stakeRatio - ownerRewards = LibFixedMath._uintMul( - LibFixedMath._mul(n, stakeRatio), - totalRewards + // Compute + // `e^(alpha * (ln(feeRatio/stakeRatio)))` if feeRatio <= stakeRatio + // or + // `e^(ln(stakeRatio/feeRatio))` if feeRatio > stakeRatio + int256 n = feeRatio <= stakeRatio ? + LibFixedMath._div(feeRatio, stakeRatio) : + LibFixedMath._div(stakeRatio, feeRatio); + n = LibFixedMath._exp( + LibFixedMath._mulDiv( + LibFixedMath._ln(n), + int256(alphaNumerator), + int256(alphaDenominator) + ) ); + // Compute + // `totalRewards * n` if feeRatio <= stakeRatio + // or + // `totalRewards / n` if stakeRatio > feeRatio + n = feeRatio <= stakeRatio ? + LibFixedMath._mul(stakeRatio, n) : + LibFixedMath._div(stakeRatio, n); + // Multiply the above with totalRewards. + ownerRewards = LibFixedMath._uintMul(n, totalRewards); } } diff --git a/contracts/staking/contracts/src/immutable/MixinConstants.sol b/contracts/staking/contracts/src/immutable/MixinConstants.sol index 5430c6f2cd..faa96e9b13 100644 --- a/contracts/staking/contracts/src/immutable/MixinConstants.sol +++ b/contracts/staking/contracts/src/immutable/MixinConstants.sol @@ -24,7 +24,7 @@ import "./MixinDeploymentConstants.sol"; contract MixinConstants is MixinDeploymentConstants { - uint32 constant internal PPM_ONE = 1000000; + uint32 constant internal PPM_DENOMINATOR = 1000000; // The upper 16 bytes represent the pool id, so this would be pool id 1. See MixinStakinPool for more information. bytes32 constant internal INITIAL_POOL_ID = 0x0000000000000000000000000000000100000000000000000000000000000000; diff --git a/contracts/staking/contracts/src/libs/LibFixedMath.sol b/contracts/staking/contracts/src/libs/LibFixedMath.sol index b8751480b6..567485bd97 100644 --- a/contracts/staking/contracts/src/libs/LibFixedMath.sol +++ b/contracts/staking/contracts/src/libs/LibFixedMath.sol @@ -256,7 +256,7 @@ library LibFixedMath { // Multiply with the taylor series for e^q int256 y; int256 z; - // q = x % 0.125 + // q = x % 0.125 (the residual) z = y = x % 0x0000000000000000000000000000000010000000000000000000000000000000; z = z * y / FIXED_1; r += z * 0x10e1b3be415a0000; // add y^02 * (20! / 02!) z = z * y / FIXED_1; r += z * 0x05a0913f6b1e0000; // add y^03 * (20! / 03!) diff --git a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol index 62741ad9e2..d6d4ef7e0e 100644 --- a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol @@ -160,7 +160,7 @@ contract StakingPoolRewardVault is onlyNotInCatastrophicFailure { // operator share must be a valid fraction - if (poolOperatorShare > PPM_ONE) { + if (poolOperatorShare > PPM_DENOMINATOR) { LibRichErrors.rrevert(LibStakingRichErrors.InvalidPoolOperatorShareError( poolId, poolOperatorShare @@ -228,8 +228,8 @@ contract StakingPoolRewardVault is { // compute portions. One of the two must round down: the operator always receives the leftover from rounding. uint256 operatorPortion = LibMath.getPartialAmountCeil( - uint256(balance.operatorShare), // Operator share out of 100 - PPM_ONE, + uint256(balance.operatorShare), // Operator share out of 1e6 + PPM_DENOMINATOR, amount ); diff --git a/contracts/staking/test/cobb_douglas.ts b/contracts/staking/test/cobb_douglas.ts index fd780702e8..3d1d3c09c3 100644 --- a/contracts/staking/test/cobb_douglas.ts +++ b/contracts/staking/test/cobb_douglas.ts @@ -1,7 +1,6 @@ -import { blockchainTests, constants, expect, filterLogsToArguments, hexRandom } from '@0x/contracts-test-utils'; +import { blockchainTests, constants, expect, filterLogsToArguments } from '@0x/contracts-test-utils'; import { StakingRevertErrors } from '@0x/order-utils'; -import { AnyRevertError, BigNumber, FixedMathRevertErrors, OwnableRevertErrors } from '@0x/utils'; -import { Decimal } from 'decimal.js'; +import { BigNumber, OwnableRevertErrors } from '@0x/utils'; import * as _ from 'lodash'; import { @@ -11,7 +10,7 @@ import { TestCobbDouglasEvents, } from '../src/'; -import { assertRoughlyEquals, Numberish } from './utils/number_utils'; +import { assertRoughlyEquals, getRandomInteger, getRandomPortion, Numberish, toDecimal } from './utils/number_utils'; // tslint:disable: no-unnecessary-type-assertion blockchainTests('Cobb-Douglas', env => { @@ -32,23 +31,6 @@ blockchainTests('Cobb-Douglas', env => { ); }); - function toDecimal(x: Numberish): Decimal { - if (BigNumber.isBigNumber(x)) { - return new Decimal(x.toString(10)); - } - return new Decimal(x); - } - - function getRandomInteger(min: Numberish, max: Numberish): BigNumber { - const range = new BigNumber(max).minus(min); - const random = new BigNumber(hexRandom().substr(2), 16); - return random.mod(range).plus(min); - } - - function getRandomPortion(total: Numberish): BigNumber { - return new BigNumber(total).times(Math.random()).integerValue(); - } - blockchainTests.resets('setCobbDouglasAlpha()', () => { const NEGATIVE_ONE = constants.MAX_UINT256.minus(1); @@ -127,7 +109,7 @@ blockchainTests('Cobb-Douglas', env => { gas?: number; } - const MAX_COBB_DOUGLAS_GAS = 15e3; + const MAX_COBB_DOUGLAS_GAS = 11e3; const TX_GAS_FEE = 21e3; const DEFAULT_COBB_DOUGLAS_PARAMS: CobbDouglasParams = { totalRewards: 100e18, @@ -171,7 +153,7 @@ blockchainTests('Cobb-Douglas', env => { return new BigNumber( feeRatio .pow(alpha) - .times(stakeRatio.pow(new Decimal(1).minus(alpha))) + .times(stakeRatio.pow(toDecimal(1).minus(alpha))) .times(toDecimal(totalRewards)) .toFixed(0, BigNumber.ROUND_FLOOR), ); @@ -196,39 +178,6 @@ blockchainTests('Cobb-Douglas', env => { }; } - it('throws if `alphaNumerator` > `alphaDenominator`', async () => { - return expect( - callCobbDouglasAsync({ - alphaNumerator: 11, - alphaDenominator: 10, - }), - ).to.revertWith(new AnyRevertError()); // Just an assertion failure. - }); - - it('throws if `ownerFees` > `totalFees`', async () => { - const expectedError = new FixedMathRevertErrors.FixedMathSignedValueError( - FixedMathRevertErrors.ValueErrorCodes.TooLarge, - ); - return expect( - callCobbDouglasAsync({ - ownerFees: 11, - totalFees: 10, - }), - ).to.revertWith(expectedError); - }); - - it('throws if `ownerStake` > `totalStake`', async () => { - const expectedError = new FixedMathRevertErrors.FixedMathSignedValueError( - FixedMathRevertErrors.ValueErrorCodes.TooLarge, - ); - return expect( - callCobbDouglasAsync({ - ownerStake: 11, - totalStake: 10, - }), - ).to.revertWith(expectedError); - }); - it('computes the correct reward', async () => { const expected = cobbDouglas(); const r = await callCobbDouglasAsync(); diff --git a/contracts/staking/test/lib_fixed_math.ts b/contracts/staking/test/lib_fixed_math.ts index c7ee069104..a267c5105e 100644 --- a/contracts/staking/test/lib_fixed_math.ts +++ b/contracts/staking/test/lib_fixed_math.ts @@ -5,7 +5,7 @@ import * as _ from 'lodash'; import { artifacts, TestLibFixedMathContract } from '../src/'; -import { assertRoughlyEquals, Numberish } from './utils/number_utils'; +import { assertRoughlyEquals, fromFixed, Numberish, toDecimal, toFixed } from './utils/number_utils'; blockchainTests('LibFixedMath', env => { let testContract: TestLibFixedMathContract; @@ -29,27 +29,8 @@ blockchainTests('LibFixedMath', env => { const MIN_LN_NUMBER = new BigNumber(new Decimal(MIN_EXP_NUMBER.toFixed(128)).exp().toFixed(128)); const FUZZ_COUNT = 1024; - function fromFixed(n: Numberish): BigNumber { - return new BigNumber(n).dividedBy(FIXED_POINT_DIVISOR); - } - - function toFixed(n: Numberish): BigNumber { - return new BigNumber(n).times(FIXED_POINT_DIVISOR).integerValue(); - } - - function numberToFixedToNumber(n: Numberish): BigNumber { - return fromFixed(toFixed(n)); - } - - function toDecimal(x: Numberish): Decimal { - if (BigNumber.isBigNumber(x)) { - return new Decimal(x.toString(10)); - } - return new Decimal(x); - } - function assertFixedEquals(actualFixed: Numberish, expected: Numberish): void { - expect(fromFixed(actualFixed)).to.bignumber.eq(numberToFixedToNumber(expected)); + expect(fromFixed(actualFixed)).to.bignumber.eq(fromFixed(toFixed(expected))); } function assertFixedRoughlyEquals(actualFixed: Numberish, expected: Numberish, precision: number = 18): void { diff --git a/contracts/staking/test/pools_test.ts b/contracts/staking/test/pools_test.ts index 1b0d5a83e9..6773aefb6b 100644 --- a/contracts/staking/test/pools_test.ts +++ b/contracts/staking/test/pools_test.ts @@ -1,8 +1,7 @@ import { ERC20ProxyContract, ERC20Wrapper } from '@0x/contracts-asset-proxy'; import { DummyERC20TokenContract } from '@0x/contracts-erc20'; -import { blockchainTests, expect } from '@0x/contracts-test-utils'; +import { blockchainTests, constants, expect } from '@0x/contracts-test-utils'; import { StakingRevertErrors } from '@0x/order-utils'; -import { BigNumber } from '@0x/utils'; import * as ethUtil from 'ethereumjs-util'; import * as _ from 'lodash'; @@ -14,8 +13,7 @@ import { StakingWrapper } from './utils/staking_wrapper'; // tslint:disable:no-unnecessary-type-assertion blockchainTests('Staking Pool Management', env => { // constants - const ZRX_TOKEN_DECIMALS = new BigNumber(18); - const PPM_ONE = 1e6; + const { DUMMY_TOKEN_DECIMALS, PPM_DENOMINATOR } = constants; // tokens & addresses let accounts: string[]; let owner: string; @@ -35,7 +33,7 @@ blockchainTests('Staking Pool Management', env => { erc20Wrapper = new ERC20Wrapper(env.provider, accounts, owner); erc20ProxyContract = await erc20Wrapper.deployProxyAsync(); // deploy zrx token - [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, ZRX_TOKEN_DECIMALS); + [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, DUMMY_TOKEN_DECIMALS); await erc20Wrapper.setBalancesAndAllowancesAsync(); // deploy staking contracts stakingWrapper = new StakingWrapper(env.provider, owner, erc20ProxyContract, zrxTokenContract, accounts); @@ -45,7 +43,7 @@ blockchainTests('Staking Pool Management', env => { it('Should successfully create a pool', async () => { // test parameters const operatorAddress = users[0]; - const operatorShare = (39 / 100) * PPM_ONE; + const operatorShare = (39 / 100) * PPM_DENOMINATOR; const poolOperator = new PoolOperatorActor(operatorAddress, stakingWrapper); // create pool const poolId = await poolOperator.createStakingPoolAsync(operatorShare); @@ -55,14 +53,14 @@ blockchainTests('Staking Pool Management', env => { const nextPoolId = await stakingWrapper.getNextStakingPoolIdAsync(); expect(nextPoolId).to.be.equal(expectedNextPoolId); }); - it('Should throw if poolOperatorShare is > PPM_ONE', async () => { + it('Should throw if poolOperatorShare is > PPM_DENOMINATOR', async () => { // test parameters const operatorAddress = users[0]; - const operatorShare = PPM_ONE + 1; + const operatorShare = PPM_DENOMINATOR + 1; const poolOperator = new PoolOperatorActor(operatorAddress, stakingWrapper); // create pool const tx = poolOperator.createStakingPoolAsync(operatorShare); - const expectedPoolId = '0x0000000000000000000000000000000100000000000000000000000000000000'; + const expectedPoolId = stakingConstants.INITIAL_POOL_ID; const expectedError = new StakingRevertErrors.InvalidPoolOperatorShareError(expectedPoolId, operatorShare); return expect(tx).to.revertWith(expectedError); }); diff --git a/contracts/staking/test/utils/number_utils.ts b/contracts/staking/test/utils/number_utils.ts index 7aea119e0d..c62dad3c9f 100644 --- a/contracts/staking/test/utils/number_utils.ts +++ b/contracts/staking/test/utils/number_utils.ts @@ -1,24 +1,82 @@ import { expect } from '@0x/contracts-test-utils'; import { BigNumber } from '@0x/utils'; +import * as crypto from 'crypto'; import { Decimal } from 'decimal.js'; Decimal.set({ precision: 80 }); export type Numberish = BigNumber | string | number; +/** + * Convert `x` to a `Decimal` type. + */ +export function toDecimal(x: Numberish): Decimal { + if (BigNumber.isBigNumber(x)) { + return new Decimal(x.toString(10)); + } + return new Decimal(x); +} + +/** + * Generate a random integer between `min` and `max`, inclusive. + */ +export function getRandomInteger(min: Numberish, max: Numberish): BigNumber { + const range = new BigNumber(max).minus(min); + return getRandomPortion(range).plus(min); +} + +/** + * Generate a random integer between `0` and `total`, inclusive. + */ +export function getRandomPortion(total: Numberish): BigNumber { + return new BigNumber(total).times(getRandomFloat(0, 1)).integerValue(BigNumber.ROUND_HALF_UP); +} + +/** + * Generate a random, high-precision decimal between `min` and `max`, inclusive. + */ +export function getRandomFloat(min: Numberish, max: Numberish): BigNumber { + // Generate a really high precision number between [0, 1] + const r = new BigNumber(crypto.randomBytes(32).toString('hex'), 16).dividedBy(new BigNumber(2).pow(256).minus(1)); + return new BigNumber(max) + .minus(min) + .times(r) + .plus(min); +} + +export const FIXED_POINT_BASE = new BigNumber(2).pow(127); + +/** + * Convert `n` to fixed-point integer represenatation. + */ +export function toFixed(n: Numberish): BigNumber { + return new BigNumber(n).times(FIXED_POINT_BASE).integerValue(); +} + +/** + * Convert `n` from fixed-point integer represenatation. + */ +export function fromFixed(n: Numberish): BigNumber { + return new BigNumber(n).dividedBy(FIXED_POINT_BASE); +} + /** * Converts two decimal numbers to integers with `precision` digits, then returns * the absolute difference. */ export function getNumericalDivergence(a: Numberish, b: Numberish, precision: number = 18): number { - const _toInteger = (n: Numberish) => { - const _n = new BigNumber(n); - const integerDigits = _n.integerValue().sd(true); - const base = 10 ** (precision - integerDigits); - return _n.times(base).integerValue(BigNumber.ROUND_DOWN); + const _a = new BigNumber(a); + const _b = new BigNumber(b); + const maxIntegerDigits = Math.max( + _a.integerValue(BigNumber.ROUND_DOWN).sd(true), + _b.integerValue(BigNumber.ROUND_DOWN).sd(true), + ); + const _toInteger = (n: BigNumber) => { + const base = 10 ** (precision - maxIntegerDigits); + return n.times(base).integerValue(BigNumber.ROUND_DOWN); }; - return _toInteger(a) - .minus(_toInteger(b)) + return _toInteger(_a) + .minus(_toInteger(_b)) .abs() .toNumber(); }