diff --git a/contracts/exchange-libs/src/reference_functions.ts b/contracts/exchange-libs/src/reference_functions.ts index 9fbecedb32..ccbaace911 100644 --- a/contracts/exchange-libs/src/reference_functions.ts +++ b/contracts/exchange-libs/src/reference_functions.ts @@ -132,9 +132,8 @@ export const LibFractions = { maxValue: BigNumber = new BigNumber(2 ** 127), ): [BigNumber, BigNumber] => { if (numerator.isGreaterThan(maxValue) || denominator.isGreaterThan(maxValue)) { - const rescaleBase = numerator.isGreaterThanOrEqualTo(denominator) - ? safeDiv(numerator, maxValue) - : safeDiv(denominator, maxValue); + let rescaleBase = numerator.isGreaterThanOrEqualTo(denominator) ? numerator : denominator; + rescaleBase = safeDiv(rescaleBase, maxValue); return [safeDiv(numerator, rescaleBase), safeDiv(denominator, rescaleBase)]; } else { return [numerator, denominator]; diff --git a/contracts/integrations/test/framework/actors/keeper.ts b/contracts/integrations/test/framework/actors/keeper.ts index a7c1b108e9..441055d6be 100644 --- a/contracts/integrations/test/framework/actors/keeper.ts +++ b/contracts/integrations/test/framework/actors/keeper.ts @@ -89,6 +89,7 @@ export function KeeperMixin(Base: TBase): TBase & Con const { stakingPools } = this.actor.simulationEnvironment!; const assertion = validFinalizePoolAssertion(this.actor.deployment, this.actor.simulationEnvironment!); while (true) { + // Finalize a random pool, or do nothing if there are no pools in the simulation yet. const poolId = Pseudorandom.sample(Object.keys(stakingPools)); if (poolId === undefined) { yield; diff --git a/contracts/integrations/test/framework/actors/staker.ts b/contracts/integrations/test/framework/actors/staker.ts index 3e426735f4..6263408339 100644 --- a/contracts/integrations/test/framework/actors/staker.ts +++ b/contracts/integrations/test/framework/actors/staker.ts @@ -107,22 +107,29 @@ export function StakerMixin(Base: TBase): TBase & Con while (true) { const { currentEpoch } = this.actor.simulationEnvironment!; + // Pick a random pool that this staker has delegated to (undefined if no such pools exist) const fromPoolId = Pseudorandom.sample( Object.keys(_.omit(this.stake[StakeStatus.Delegated], ['total'])), ); + // The `from` status must be Undelegated if the staker isn't delegated to any pools + // at the moment, or if the chosen pool is unfinalized const fromStatus = fromPoolId === undefined || stakingPools[fromPoolId].lastFinalized.isLessThan(currentEpoch.minus(1)) ? StakeStatus.Undelegated : (Pseudorandom.sample([StakeStatus.Undelegated, StakeStatus.Delegated]) as StakeStatus); const from = new StakeInfo(fromStatus, fromPoolId); + // Pick a random pool to move the stake to const toPoolId = Pseudorandom.sample(Object.keys(stakingPools)); + // The `from` status must be Undelegated if no pools exist in the simulation yet, + // or if the chosen pool is unfinalized const toStatus = toPoolId === undefined || stakingPools[toPoolId].lastFinalized.isLessThan(currentEpoch.minus(1)) ? StakeStatus.Undelegated : (Pseudorandom.sample([StakeStatus.Undelegated, StakeStatus.Delegated]) as StakeStatus); const to = new StakeInfo(toStatus, toPoolId); + // The next epoch balance of the `from` stake is the amount that can be moved const moveableStake = from.status === StakeStatus.Undelegated ? this.stake[StakeStatus.Undelegated].nextEpochBalance @@ -141,6 +148,7 @@ export function StakerMixin(Base: TBase): TBase & Con ); while (true) { const prevEpoch = this.actor.simulationEnvironment!.currentEpoch.minus(1); + // Pick a finalized pool const poolId = Pseudorandom.sample( Object.keys(stakingPools).filter(id => stakingPools[id].lastFinalized.isGreaterThanOrEqualTo(prevEpoch), diff --git a/contracts/integrations/test/framework/actors/taker.ts b/contracts/integrations/test/framework/actors/taker.ts index 0bbeeb4d16..5527891844 100644 --- a/contracts/integrations/test/framework/actors/taker.ts +++ b/contracts/integrations/test/framework/actors/taker.ts @@ -68,37 +68,36 @@ export function TakerMixin(Base: TBase): TBase & Cons const { actors, balanceStore } = this.actor.simulationEnvironment!; const assertion = validFillOrderAssertion(this.actor.deployment, this.actor.simulationEnvironment!); while (true) { + // Choose a maker to be the other side of the order const maker = Pseudorandom.sample(filterActorsByRole(actors, Maker)); if (maker === undefined) { yield; } else { await balanceStore.updateErc20BalancesAsync(); + // Choose the assets for the order const [makerToken, makerFeeToken, takerToken, takerFeeToken] = Pseudorandom.sampleSize( this.actor.deployment.tokens.erc20, 4, // tslint:disable-line:custom-no-magic-numbers ); - const configureOrderAssetAsync = async ( - owner: Actor, - token: DummyERC20TokenContract, - ): Promise => { - let balance = balanceStore.balances.erc20[owner.address][token.address]; - await owner.configureERC20TokenAsync(token); - balance = balanceStore.balances.erc20[owner.address][token.address] = - constants.INITIAL_ERC20_BALANCE; - return Pseudorandom.integer(balance.dividedToIntegerBy(2)); - }; - + // Maker and taker set balances/allowances to guarantee that the fill succeeds. + // Amounts are chosen to be within each actor's balance (divided by 2, in case + // e.g. makerAsset = makerFeeAsset) const [makerAssetAmount, makerFee, takerAssetAmount, takerFee] = await Promise.all( [ [maker, makerToken], [maker, makerFeeToken], [this.actor, takerToken], [this.actor, takerFeeToken], - ].map(async ([owner, token]) => - configureOrderAssetAsync(owner as Actor, token as DummyERC20TokenContract), - ), + ].map(async ([owner, token]) => { + let balance = balanceStore.balances.erc20[owner.address][token.address]; + await (owner as Actor).configureERC20TokenAsync(token as DummyERC20TokenContract); + balance = balanceStore.balances.erc20[owner.address][token.address] = + constants.INITIAL_ERC20_BALANCE; + return Pseudorandom.integer(balance.dividedToIntegerBy(2)); + }), ); + // Encode asset data const [makerAssetData, makerFeeAssetData, takerAssetData, takerFeeAssetData] = [ makerToken, makerFeeToken, @@ -108,6 +107,7 @@ export function TakerMixin(Base: TBase): TBase & Cons this.actor.deployment.assetDataEncoder.ERC20Token(token.address).getABIEncodedTransactionData(), ); + // Maker signs the order const order = await maker.signOrderAsync({ makerAssetData, takerAssetData, @@ -120,7 +120,10 @@ export function TakerMixin(Base: TBase): TBase & Cons feeRecipientAddress: Pseudorandom.sample(actors)!.address, }); + // Taker fills the order by a random amount (up to the order's takerAssetAmount) const fillAmount = Pseudorandom.integer(order.takerAssetAmount); + // Taker executes the fill with a random msg.value, so that sometimes the + // protocol fee is paid in ETH and other times it's paid in WETH. yield assertion.executeAsync([order, fillAmount, order.signature], { from: this.actor.address, value: Pseudorandom.integer(DeploymentManager.protocolFee.times(2)), diff --git a/contracts/integrations/test/framework/assertions/endEpoch.ts b/contracts/integrations/test/framework/assertions/endEpoch.ts index 61d70fd95e..db82b4ef9a 100644 --- a/contracts/integrations/test/framework/assertions/endEpoch.ts +++ b/contracts/integrations/test/framework/assertions/endEpoch.ts @@ -21,11 +21,10 @@ interface EndEpochBeforeInfo { } /** - * Returns a FunctionAssertion for `stake` which assumes valid input is provided. The - * FunctionAssertion checks that the staker and zrxVault's balances of ZRX decrease and increase, - * respectively, by the input amount. + * Returns a FunctionAssertion for `endEpoch` which assumes valid input is provided. It checks + * that the staking proxy contract wrapped its ETH balance, aggregated stats were updated, and + * EpochFinalized/EpochEnded events were emitted. */ -/* tslint:disable:no-unnecessary-type-assertion */ export function validEndEpochAssertion( deployment: DeploymentManager, simulationEnvironment: SimulationEnvironment, @@ -47,7 +46,7 @@ export function validEndEpochAssertion( expect(result.success, `Error: ${result.data}`).to.be.true(); const { currentEpoch } = simulationEnvironment; - const logs = result.receipt!.logs; // tslint:disable-line:no-non-null-assertion + const logs = result.receipt!.logs; // tslint:disable-line // Check WETH deposit event const previousEthBalance = balanceStore.balances.eth[stakingWrapper.address] || constants.ZERO_AMOUNT; @@ -61,6 +60,7 @@ export function validEndEpochAssertion( : []; verifyEventsFromLogs(logs, expectedDepositEvents, WETH9Events.Deposit); + // Check that the aggregated stats were updated await balanceStore.updateErc20BalancesAsync(); const { wethReservedForPoolRewards, aggregatedStatsBefore } = beforeInfo; const expectedAggregatedStats = { @@ -71,12 +71,12 @@ export function validEndEpochAssertion( constants.ZERO_AMOUNT, ).minus(wethReservedForPoolRewards), }; - const aggregatedStatsAfter = AggregatedStats.fromArray( await stakingWrapper.aggregatedStatsByEpoch(currentEpoch).callAsync(), ); expect(aggregatedStatsAfter).to.deep.equal(expectedAggregatedStats); + // Check that an EpochEnded event was emitted verifyEventsFromLogs( logs, [ @@ -91,6 +91,7 @@ export function validEndEpochAssertion( StakingEvents.EpochEnded, ); + // If there are no more pools to finalize, an EpochFinalized event should've been emitted const expectedEpochFinalizedEvents = aggregatedStatsAfter.numPoolsToFinalize.isZero() ? [ { @@ -106,12 +107,13 @@ export function validEndEpochAssertion( StakingEvents.EpochFinalized, ); + // The function returns the remaining number of unfinalized pools for the epoch expect(result.data, 'endEpoch should return the number of unfinalized pools').to.bignumber.equal( aggregatedStatsAfter.numPoolsToFinalize, ); + // Update currentEpoch locally simulationEnvironment.currentEpoch = currentEpoch.plus(1); }, }); } -/* tslint:enable:no-unnecessary-type-assertion */ diff --git a/contracts/integrations/test/framework/assertions/fillOrder.ts b/contracts/integrations/test/framework/assertions/fillOrder.ts index 54ab4782e9..cb319aa453 100644 --- a/contracts/integrations/test/framework/assertions/fillOrder.ts +++ b/contracts/integrations/test/framework/assertions/fillOrder.ts @@ -155,6 +155,7 @@ export function validFillOrderAssertion( // Ensure that the correct events were emitted. verifyFillEvents(txData, order, result.receipt!, deployment, fillAmount); + // If the maker is not in a staking pool, there's nothing to check if (beforeInfo === undefined) { return; } @@ -163,6 +164,7 @@ export function validFillOrderAssertion( const expectedAggregatedStats = { ...beforeInfo.aggregatedStats }; const expectedEvents = []; + // Refer to `payProtocolFee` if (beforeInfo.poolStake.isGreaterThanOrEqualTo(stakingConstants.DEFAULT_PARAMS.minimumPoolStake)) { if (beforeInfo.poolStats.feesCollected.isZero()) { const membersStakeInPool = beforeInfo.poolStake.minus(beforeInfo.operatorStake); @@ -181,20 +183,23 @@ export function validFillOrderAssertion( expectedAggregatedStats.numPoolsToFinalize = beforeInfo.aggregatedStats.numPoolsToFinalize.plus( 1, ); + // StakingPoolEarnedRewardsInEpoch event emitted expectedEvents.push({ epoch: currentEpoch, poolId: beforeInfo.poolId, }); } - + // Credit a protocol fee to the maker's staking pool expectedPoolStats.feesCollected = beforeInfo.poolStats.feesCollected.plus( DeploymentManager.protocolFee, ); + // Update aggregated stats expectedAggregatedStats.totalFeesCollected = beforeInfo.aggregatedStats.totalFeesCollected.plus( DeploymentManager.protocolFee, ); } + // Check for updated stats and event const poolStats = PoolStats.fromArray( await stakingWrapper.poolStatsByEpoch(beforeInfo.poolId, currentEpoch).callAsync(), ); diff --git a/contracts/integrations/test/framework/assertions/finalizePool.ts b/contracts/integrations/test/framework/assertions/finalizePool.ts index 47c037b8a8..6a5ff99334 100644 --- a/contracts/integrations/test/framework/assertions/finalizePool.ts +++ b/contracts/integrations/test/framework/assertions/finalizePool.ts @@ -60,7 +60,7 @@ interface FinalizePoolBeforeInfo { * Returns a FunctionAssertion for `finalizePool` which assumes valid input is provided. The `after` * callback below is annotated with the solidity source of `finalizePool`. */ - /* tslint:disable:no-unnecessary-type-assertion */ +/* tslint:disable:no-unnecessary-type-assertion */ export function validFinalizePoolAssertion( deployment: DeploymentManager, simulationEnvironment: SimulationEnvironment, diff --git a/contracts/integrations/test/framework/assertions/joinStakingPool.ts b/contracts/integrations/test/framework/assertions/joinStakingPool.ts index cafa5ac29c..363683ef3c 100644 --- a/contracts/integrations/test/framework/assertions/joinStakingPool.ts +++ b/contracts/integrations/test/framework/assertions/joinStakingPool.ts @@ -11,16 +11,17 @@ import { FunctionAssertion, FunctionResult } from './function_assertion'; */ /* tslint:disable:no-unnecessary-type-assertion */ /* tslint:disable:no-non-null-assertion */ -export function validJoinStakingPoolAssertion(deployment: DeploymentManager): FunctionAssertion<[string], {}, void> { +export function validJoinStakingPoolAssertion(deployment: DeploymentManager): FunctionAssertion<[string], void, void> { const { stakingWrapper } = deployment.staking; - return new FunctionAssertion<[string], {}, void>(stakingWrapper, 'joinStakingPoolAsMaker', { - after: async (_beforeInfo, result: FunctionResult, args: [string], txData: Partial) => { + return new FunctionAssertion<[string], void, void>(stakingWrapper, 'joinStakingPoolAsMaker', { + after: async (_beforeInfo: void, result: FunctionResult, args: [string], txData: Partial) => { // Ensure that the tx succeeded. expect(result.success, `Error: ${result.data}`).to.be.true(); const [poolId] = args; + // Verify a MakerStakingPoolSet event was emitted const logs = result.receipt!.logs; const logArgs = filterLogsToArguments( logs, @@ -32,6 +33,7 @@ export function validJoinStakingPoolAssertion(deployment: DeploymentManager): Fu poolId, }, ]); + // Verify that the maker's pool id has been updated in storage const joinedPoolId = await deployment.staking.stakingWrapper.poolIdByMaker(txData.from!).callAsync(); expect(joinedPoolId).to.be.eq(poolId); }, diff --git a/contracts/integrations/test/framework/assertions/moveStake.ts b/contracts/integrations/test/framework/assertions/moveStake.ts index 5434efe1a2..e29dbe3ccf 100644 --- a/contracts/integrations/test/framework/assertions/moveStake.ts +++ b/contracts/integrations/test/framework/assertions/moveStake.ts @@ -75,8 +75,8 @@ function updateNextEpochBalances( return updatedPools; } /** - * Returns a FunctionAssertion for `moveStake` which assumes valid input is provided. The - * FunctionAssertion checks that the staker's + * Returns a FunctionAssertion for `moveStake` which assumes valid input is provided. Checks that + * the owner's stake and global stake by status get updated correctly. */ /* tslint:disable:no-unnecessary-type-assertion */ export function validMoveStakeAssertion( diff --git a/contracts/integrations/test/framework/assertions/stake.ts b/contracts/integrations/test/framework/assertions/stake.ts index db900381a0..e4921eea24 100644 --- a/contracts/integrations/test/framework/assertions/stake.ts +++ b/contracts/integrations/test/framework/assertions/stake.ts @@ -53,10 +53,7 @@ export function validStakeAssertion( await balanceStore.updateErc20BalancesAsync(); balanceStore.assertEquals(expectedBalances); - // Checks that the owner's undelegated stake has increased by the stake amount - const ownerUndelegatedStake = await stakingWrapper - .getOwnerStakeByStatus(txData.from as string, StakeStatus.Undelegated) - .callAsync(); + // _increaseCurrentAndNextBalance loadCurrentBalance(ownerStake[StakeStatus.Undelegated], currentEpoch, true); ownerStake[StakeStatus.Undelegated].currentEpochBalance = ownerStake[ StakeStatus.Undelegated @@ -64,6 +61,11 @@ export function validStakeAssertion( ownerStake[StakeStatus.Undelegated].nextEpochBalance = ownerStake[ StakeStatus.Undelegated ].nextEpochBalance.plus(amount); + + // Checks that the owner's undelegated stake has increased by the stake amount + const ownerUndelegatedStake = await stakingWrapper + .getOwnerStakeByStatus(txData.from as string, StakeStatus.Undelegated) + .callAsync(); expect(ownerUndelegatedStake, 'Owner undelegated stake').to.deep.equal(ownerStake[StakeStatus.Undelegated]); }, }); diff --git a/contracts/integrations/test/framework/assertions/unstake.ts b/contracts/integrations/test/framework/assertions/unstake.ts index 72236bfdd3..fc69f991a8 100644 --- a/contracts/integrations/test/framework/assertions/unstake.ts +++ b/contracts/integrations/test/framework/assertions/unstake.ts @@ -53,10 +53,7 @@ export function validUnstakeAssertion( await balanceStore.updateErc20BalancesAsync(); balanceStore.assertEquals(expectedBalances); - // Checks that the owner's undelegated stake has decreased by the stake amount - const ownerUndelegatedStake = await stakingWrapper - .getOwnerStakeByStatus(txData.from as string, StakeStatus.Undelegated) - .callAsync(); + // _decreaseCurrentAndNextBalance loadCurrentBalance(ownerStake[StakeStatus.Undelegated], currentEpoch, true); ownerStake[StakeStatus.Undelegated].currentEpochBalance = ownerStake[ StakeStatus.Undelegated @@ -64,6 +61,11 @@ export function validUnstakeAssertion( ownerStake[StakeStatus.Undelegated].nextEpochBalance = ownerStake[ StakeStatus.Undelegated ].nextEpochBalance.minus(amount); + + // Checks that the owner's undelegated stake has decreased by the stake amount + const ownerUndelegatedStake = await stakingWrapper + .getOwnerStakeByStatus(txData.from as string, StakeStatus.Undelegated) + .callAsync(); expect(ownerUndelegatedStake, 'Owner undelegated stake').to.deep.equal(ownerStake[StakeStatus.Undelegated]); }, }); diff --git a/contracts/integrations/test/framework/assertions/withdrawDelegatorRewards.ts b/contracts/integrations/test/framework/assertions/withdrawDelegatorRewards.ts index 673757a133..76feff70af 100644 --- a/contracts/integrations/test/framework/assertions/withdrawDelegatorRewards.ts +++ b/contracts/integrations/test/framework/assertions/withdrawDelegatorRewards.ts @@ -16,9 +16,9 @@ interface WithdrawDelegatorRewardsBeforeInfo { } /** - * Returns a FunctionAssertion for `stake` which assumes valid input is provided. The - * FunctionAssertion checks that the staker and zrxVault's balances of ZRX decrease and increase, - * respectively, by the input amount. + * Returns a FunctionAssertion for `withdrawDelegatorRewards` which assumes valid input is provided. + * It checks that the delegator's stake gets synced and pool rewards are updated to reflect the + * amount withdrawn. */ /* tslint:disable:no-unnecessary-type-assertion */ export function validWithdrawDelegatorRewardsAssertion( @@ -50,12 +50,14 @@ export function validWithdrawDelegatorRewardsAssertion( const [poolId] = args; const { currentEpoch } = simulationEnvironment; + // Check that delegator stake has been synced const expectedDelegatorStake = loadCurrentBalance(beforeInfo.delegatorStake, currentEpoch); const delegatorStake = await stakingWrapper .getStakeDelegatedToPoolByOwner(txData.from as string, poolId) .callAsync(); expect(delegatorStake).to.deep.equal(expectedDelegatorStake); + // Check that pool rewards have been updated to reflect the amount withdrawn. const transferEvents = filterLogsToArguments( result.receipt!.logs, // tslint:disable-line:no-non-null-assertion WETH9Events.Transfer, diff --git a/contracts/integrations/test/fuzz_tests/staking_rewards_test.ts b/contracts/integrations/test/fuzz_tests/staking_rewards_test.ts index ea3cbaa192..9cf1c6ae53 100644 --- a/contracts/integrations/test/fuzz_tests/staking_rewards_test.ts +++ b/contracts/integrations/test/fuzz_tests/staking_rewards_test.ts @@ -58,12 +58,15 @@ blockchainTests('Staking rewards fuzz test', env => { }); it('fuzz', async () => { + // Deploy contracts const deployment = await DeploymentManager.deployAsync(env, { numErc20TokensToDeploy: 4, numErc721TokensToDeploy: 0, numErc1155TokensToDeploy: 0, }); const [ERC20TokenA, ERC20TokenB, ERC20TokenC, ERC20TokenD] = deployment.tokens.erc20; + + // Set up balance store const balanceStore = new BlockchainBalanceStore( { StakingProxy: deployment.staking.stakingProxy.address, @@ -80,8 +83,11 @@ blockchainTests('Staking rewards fuzz test', env => { }, }, ); + + // Set up simulation environment const simulationEnvironment = new SimulationEnvironment(deployment, balanceStore); + // Spin up actors const actors = [ new Maker({ deployment, simulationEnvironment, name: 'Maker 1' }), new Maker({ deployment, simulationEnvironment, name: 'Maker 2' }), @@ -98,14 +104,17 @@ blockchainTests('Staking rewards fuzz test', env => { new OperatorStakerMaker({ deployment, simulationEnvironment, name: 'Operator/Staker/Maker' }), ]; + // Takers need to set a WETH allowance for the staking proxy in case they pay the protocol fee in WETH const takers = filterActorsByRole(actors, Taker); for (const taker of takers) { await taker.configureERC20TokenAsync(deployment.tokens.weth, deployment.staking.stakingProxy.address); } + // Stakers need to set a ZRX allowance to deposit their ZRX into the zrxVault const stakers = filterActorsByRole(actors, Staker); for (const staker of stakers) { await staker.configureERC20TokenAsync(deployment.tokens.zrx); } + // Register each actor in the balance store for (const actor of actors) { balanceStore.registerTokenOwner(actor.address, actor.name); } diff --git a/contracts/staking/contracts/test/TestStaking.sol b/contracts/staking/contracts/test/TestStaking.sol index 4a60105d81..ac6cd979f1 100644 --- a/contracts/staking/contracts/test/TestStaking.sol +++ b/contracts/staking/contracts/test/TestStaking.sol @@ -58,6 +58,7 @@ contract TestStaking is testZrxVaultAddress = zrxVaultAddress; } + // @dev Gets the most recent cumulative reward for a pool, and the epoch it was stored. function getMostRecentCumulativeReward(bytes32 poolId) external view diff --git a/contracts/staking/src/types.ts b/contracts/staking/src/types.ts index 771edf7722..45f0719c8e 100644 --- a/contracts/staking/src/types.ts +++ b/contracts/staking/src/types.ts @@ -67,6 +67,10 @@ export class StoredBalance { ) {} } +/** + * Simulates _loadCurrentBalance. `shouldMutate` flag specifies whether or not to update the given + * StoredBalance instance. + */ export function loadCurrentBalance( balance: StoredBalance, epoch: BigNumber, @@ -85,11 +89,17 @@ export function loadCurrentBalance( return loadedBalance; } +/** + * Simulates _incrementNextEpochBalance + */ export function incrementNextEpochBalance(balance: StoredBalance, amount: Numberish, epoch: BigNumber): void { loadCurrentBalance(balance, epoch, true); balance.nextEpochBalance = balance.nextEpochBalance.plus(amount); } +/** + * Simulates _decrementNextEpochBalance + */ export function decrementNextEpochBalance(balance: StoredBalance, amount: Numberish, epoch: BigNumber): void { loadCurrentBalance(balance, epoch, true); balance.nextEpochBalance = balance.nextEpochBalance.minus(amount);