@0x/contracts-staking: Reformulate cobb-douglas to be more efficient.

`@0x/contracts-staking`: Remove some unecessary asserts.
`@0x/contracts-staking`: Fix some broken test assertions.
`@0x/contracts-staking`: Generate better random values in tests.
`@0x/contracts-staking`: Rename `PPM_ONE` constant to `PPM_DENOMINATOR`.
`@0x/contracts-staking`: Minor solidity code improvements.
`@0x/contracts-staking`: Use more constants from `@0x/contracts-test-utils` in tests.

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)
+        // `totalRewards * feeRatio ^ alpha * stakeRatio ^ (1-alpha)`
-        // We instead use:
+        // This is equivalent to:
-        // totalRewards * stakeRatio * e^(alpha * (ln(feeRatio) - ln(stakeRatio)))
+        // `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)))
+        // Compute
-        int256 logFeeRatio = LibFixedMath._ln(feeRatio);
+        // `e^(alpha * (ln(feeRatio/stakeRatio)))` if feeRatio <= stakeRatio
-        int256 logStakeRatio = LibFixedMath._ln(stakeRatio);
+        // or
-        int256 n;
+        // `e^(ln(stakeRatio/feeRatio))` if feeRatio > stakeRatio
-        if (logFeeRatio <= logStakeRatio) {
+        int256 n = feeRatio <= stakeRatio ?
-            n = LibFixedMath._exp(
+            LibFixedMath._div(feeRatio, stakeRatio) :
-                LibFixedMath._mulDiv(
+            LibFixedMath._div(stakeRatio, feeRatio);
-                    LibFixedMath._sub(logFeeRatio, logStakeRatio),
+        n = LibFixedMath._exp(
-                    int256(alphaNumerator),
+            LibFixedMath._mulDiv(
-                    int256(alphaDenominator)
+                LibFixedMath._ln(n),
-                )
+                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
+        // `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);
     }
 }

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;

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!)

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
+            uint256(balance.operatorShare),  // Operator share out of 1e6
-            PPM_ONE,
+            PPM_DENOMINATOR,
             amount
         );
 

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 { BigNumber, OwnableRevertErrors } from '@0x/utils';
-import { Decimal } from 'decimal.js';
 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();

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 {

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 { DUMMY_TOKEN_DECIMALS, PPM_DENOMINATOR } = constants;
-    const PPM_ONE = 1e6;
     // 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);
         });

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 _a = new BigNumber(a);
-        const _n = new BigNumber(n);
+    const _b = new BigNumber(b);
-        const integerDigits = _n.integerValue().sd(true);
+    const maxIntegerDigits = Math.max(
-        const base = 10 ** (precision - integerDigits);
+        _a.integerValue(BigNumber.ROUND_DOWN).sd(true),
-        return _n.times(base).integerValue(BigNumber.ROUND_DOWN);
+        _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)
+    return _toInteger(_a)
-        .minus(_toInteger(b))
+        .minus(_toInteger(_b))
         .abs()
         .toNumber();
 }