Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Commit

Permalink
@0x/contracts-staking: Reformulate cobb-douglas to be more efficient.
Browse files Browse the repository at this point in the history 
`@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.
  • Loading branch information
@merklejerk
merklejerk committed Sep 4, 2019
1 parent 1af2267 commit 0226c53
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 130 deletions.
64 changes: 32 additions & 32 deletions contracts/staking/contracts/src/fees/MixinExchangeFees.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -212,7 +212,7 @@ contract MixinExchangeFees is
totalStakeDelegatedToPool
.safeSub(stakeHeldByPoolOperator)
.safeMul(REWARD_DELEGATED_STAKE_WEIGHT)
.safeDiv(PPM_ONE)
.safeDiv(PPM_DENOMINATOR)
);

// store pool stats
Expand Down Expand Up @@ -309,44 +309,44 @@ 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) {
return ownerRewards = 0;
}

// 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);
}
}
2 changes: 1 addition & 1 deletion contracts/staking/contracts/src/immutable/MixinConstants.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion contracts/staking/contracts/src/libs/LibFixedMath.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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!)
Expand Down
6 changes: 3 additions & 3 deletions contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
);

Expand Down
61 changes: 5 additions & 56 deletions contracts/staking/test/cobb_douglas.ts
View file
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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 => {
Expand All @@ -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);

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
);
Expand All @@ -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();
Expand Down
23 changes: 2 additions & 21 deletions contracts/staking/test/lib_fixed_math.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
16 changes: 7 additions & 9 deletions contracts/staking/test/pools_test.ts
View file
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
});
Expand Down
Loading

0 comments on commit 0226c53

Please sign in to comment.