@0x/contracts-staking: Add some extra finalizer tests and light refactorings.

`@0x/contracts-staking`: Add finalization-related protocol fees unit tests.

contracts/staking/contracts/test/TestFinalizer.sol

@@ -107,6 +107,13 @@ contract TestFinalizer is
             reward.membersStake) = _finalizePool(poolId);
     }
 
+    /// @dev Drain the balance of this contract.
+    function drainBalance()
+        external
+    {
+        address(0).transfer(address(this).balance);
+    }
+
     /// @dev Get finalization-related state variables.
     function getFinalizationState()
         external

contracts/staking/contracts/test/TestProtocolFees.sol

@@ -53,6 +53,12 @@ contract TestProtocolFees is
         poolJoinedByMakerAddress[makerAddress].confirmed = true;
     }
 
+    function advanceEpoch()
+        external
+    {
+        currentEpoch += 1;
+    }
+
     function getWethAssetData() external pure returns (bytes memory) {
         return WETH_ASSET_DATA;
     }

contracts/staking/test/protocol_fees.ts

@@ -14,12 +14,14 @@ import * as _ from 'lodash';
 
 import {
     artifacts,
+    IStakingEventsEvents,
+    IStakingEventsStakingPoolActivatedEventArgs,
     TestProtocolFeesContract,
     TestProtocolFeesERC20ProxyContract,
     TestProtocolFeesERC20ProxyTransferFromCalledEventArgs,
 } from '../src';
 
-import { getRandomPortion } from './utils/number_utils';
+import { getRandomInteger } from './utils/number_utils';
 
 blockchainTests('Protocol Fee Unit Tests', env => {
     let ownerAddress: string;
@@ -27,6 +29,7 @@ blockchainTests('Protocol Fee Unit Tests', env => {
     let notExchangeAddress: string;
     let testContract: TestProtocolFeesContract;
     let wethAssetData: string;
+    let minimumStake: BigNumber;
 
     before(async () => {
         [ownerAddress, exchangeAddress, notExchangeAddress] = await env.web3Wrapper.getAvailableAddressesAsync();
@@ -53,29 +56,31 @@ blockchainTests('Protocol Fee Unit Tests', env => {
         );
 
         wethAssetData = await testContract.getWethAssetData.callAsync();
+        minimumStake = (await testContract.getParams.callAsync())[2];
     });
 
-    interface CreatePoolOpts {
+    interface CreateTestPoolOpts {
+        poolId: string;
         operatorStake: Numberish;
         membersStake: Numberish;
         makers: string[];
     }
 
-    async function createTestPoolAsync(opts: Partial<CreatePoolOpts>): Promise<string> {
+    async function createTestPoolAsync(opts?: Partial<CreateTestPoolOpts>): Promise<CreateTestPoolOpts> {
         const _opts = {
-            operatorStake: 0,
+            poolId: hexRandom(),
-            membersStake: 0,
+            operatorStake: getRandomInteger(minimumStake, '100e18'),
-            makers: [],
+            membersStake: getRandomInteger(minimumStake, '100e18'),
+            makers: _.times(2, () => randomAddress()),
             ...opts,
         };
-        const poolId = hexRandom();
         await testContract.createTestPool.awaitTransactionSuccessAsync(
-            poolId,
+            _opts.poolId,
             new BigNumber(_opts.operatorStake),
             new BigNumber(_opts.membersStake),
             _opts.makers,
         );
-        return poolId;
+        return _opts;
     }
 
     blockchainTests.resets('payProtocolFee()', () => {
@@ -83,11 +88,6 @@ blockchainTests('Protocol Fee Unit Tests', env => {
         const { ZERO_AMOUNT } = constants;
         const makerAddress = randomAddress();
         const payerAddress = randomAddress();
-        let minimumStake: BigNumber;
-
-        before(async () => {
-            minimumStake = (await testContract.getParams.callAsync())[2];
-        });
 
         describe('forbidden actions', () => {
             it('should revert if called by a non-exchange', async () => {
@@ -187,7 +187,7 @@ blockchainTests('Protocol Fee Unit Tests', env => {
             });
 
             it('should credit pool if the maker is in a pool', async () => {
-                const poolId = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
+                const { poolId } = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
                 const receipt = await testContract.payProtocolFee.awaitTransactionSuccessAsync(
                     makerAddress,
                     payerAddress,
@@ -200,7 +200,7 @@ blockchainTests('Protocol Fee Unit Tests', env => {
             });
 
             it('should not credit the pool if maker is not in a pool', async () => {
-                const poolId = await createTestPoolAsync({ operatorStake: minimumStake });
+                const { poolId } = await createTestPoolAsync({ operatorStake: minimumStake });
                 const receipt = await testContract.payProtocolFee.awaitTransactionSuccessAsync(
                     makerAddress,
                     payerAddress,
@@ -213,7 +213,7 @@ blockchainTests('Protocol Fee Unit Tests', env => {
             });
 
             it('fees paid to the same maker should go to the same pool', async () => {
-                const poolId = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
+                const { poolId } = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
                 const payAsync = async () => {
                     const receipt = await testContract.payProtocolFee.awaitTransactionSuccessAsync(
                         makerAddress,
@@ -258,7 +258,7 @@ blockchainTests('Protocol Fee Unit Tests', env => {
             });
 
             it('should update `protocolFeesThisEpochByPool` if the maker is in a pool', async () => {
-                const poolId = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
+                const { poolId } = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
                 const receipt = await testContract.payProtocolFee.awaitTransactionSuccessAsync(
                     makerAddress,
                     payerAddress,
@@ -271,7 +271,7 @@ blockchainTests('Protocol Fee Unit Tests', env => {
             });
 
             it('should not update `protocolFeesThisEpochByPool` if maker is not in a pool', async () => {
-                const poolId = await createTestPoolAsync({ operatorStake: minimumStake });
+                const { poolId } = await createTestPoolAsync({ operatorStake: minimumStake });
                 const receipt = await testContract.payProtocolFee.awaitTransactionSuccessAsync(
                     makerAddress,
                     payerAddress,
@@ -284,7 +284,7 @@ blockchainTests('Protocol Fee Unit Tests', env => {
             });
 
             it('fees paid to the same maker should go to the same pool', async () => {
-                const poolId = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
+                const { poolId } = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
                 const payAsync = async () => {
                     const receipt = await testContract.payProtocolFee.awaitTransactionSuccessAsync(
                         makerAddress,
@@ -302,7 +302,7 @@ blockchainTests('Protocol Fee Unit Tests', env => {
             });
 
             it('fees paid to the same maker in WETH then ETH should go to the same pool', async () => {
-                const poolId = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
+                const { poolId } = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
                 const payAsync = async (inWETH: boolean) => {
                     await testContract.payProtocolFee.awaitTransactionSuccessAsync(
                         makerAddress,
@@ -322,60 +322,11 @@ blockchainTests('Protocol Fee Unit Tests', env => {
             });
         });
 
-        describe('Multiple makers', () => {
-            it('fees paid to different makers in the same pool go to that pool', async () => {
-                const otherMakerAddress = randomAddress();
-                const poolId = await createTestPoolAsync({
-                    operatorStake: minimumStake,
-                    makers: [makerAddress, otherMakerAddress],
-                });
-                const payAsync = async (_makerAddress: string) => {
-                    await testContract.payProtocolFee.awaitTransactionSuccessAsync(
-                        _makerAddress,
-                        payerAddress,
-                        DEFAULT_PROTOCOL_FEE_PAID,
-                        { from: exchangeAddress, value: DEFAULT_PROTOCOL_FEE_PAID },
-                    );
-                };
-                await payAsync(makerAddress);
-                await payAsync(otherMakerAddress);
-                const expectedTotalFees = DEFAULT_PROTOCOL_FEE_PAID.times(2);
-                const poolFees = await getProtocolFeesAsync(poolId);
-                expect(poolFees).to.bignumber.eq(expectedTotalFees);
-            });
-
-            it('fees paid to makers in different pools go to their respective pools', async () => {
-                const [fee, otherFee] = _.times(2, () => getRandomPortion(DEFAULT_PROTOCOL_FEE_PAID));
-                const otherMakerAddress = randomAddress();
-                const poolId = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
-                const otherPoolId = await createTestPoolAsync({
-                    operatorStake: minimumStake,
-                    makers: [otherMakerAddress],
-                });
-                const payAsync = async (_poolId: string, _makerAddress: string, _fee: BigNumber) => {
-                    // prettier-ignore
-                    await testContract.payProtocolFee.awaitTransactionSuccessAsync(
-                        _makerAddress,
-                        payerAddress,
-                        _fee,
-                        { from: exchangeAddress, value: _fee },
-                    );
-                };
-                await payAsync(poolId, makerAddress, fee);
-                await payAsync(otherPoolId, otherMakerAddress, otherFee);
-                const [poolFees, otherPoolFees] = await Promise.all([
-                    getProtocolFeesAsync(poolId),
-                    getProtocolFeesAsync(otherPoolId),
-                ]);
-                expect(poolFees).to.bignumber.eq(fee);
-                expect(otherPoolFees).to.bignumber.eq(otherFee);
-            });
-        });
-
         describe('Dust stake', () => {
             it('credits pools with stake > minimum', async () => {
-                const poolId = await createTestPoolAsync({
+                const { poolId } = await createTestPoolAsync({
                     operatorStake: minimumStake.plus(1),
+                    membersStake: 0,
                     makers: [makerAddress],
                 });
                 await testContract.payProtocolFee.awaitTransactionSuccessAsync(
@@ -389,7 +340,11 @@ blockchainTests('Protocol Fee Unit Tests', env => {
             });
 
             it('credits pools with stake == minimum', async () => {
-                const poolId = await createTestPoolAsync({ operatorStake: minimumStake, makers: [makerAddress] });
+                const { poolId } = await createTestPoolAsync({
+                    operatorStake: minimumStake,
+                    membersStake: 0,
+                    makers: [makerAddress],
+                });
                 await testContract.payProtocolFee.awaitTransactionSuccessAsync(
                     makerAddress,
                     constants.NULL_ADDRESS,
@@ -401,8 +356,9 @@ blockchainTests('Protocol Fee Unit Tests', env => {
             });
 
             it('does not credit pools with stake < minimum', async () => {
-                const poolId = await createTestPoolAsync({
+                const { poolId } = await createTestPoolAsync({
                     operatorStake: minimumStake.minus(1),
+                    membersStake: 0,
                     makers: [makerAddress],
                 });
                 await testContract.payProtocolFee.awaitTransactionSuccessAsync(
@@ -415,5 +371,184 @@ blockchainTests('Protocol Fee Unit Tests', env => {
                 expect(feesCredited).to.bignumber.eq(0);
             });
         });
+
+        blockchainTests.resets('Finalization', () => {
+            let membersStakeWeight: number;
+
+            before(async () => {
+                membersStakeWeight = (await testContract.getParams.callAsync())[1];
+            });
+
+            interface FinalizationState {
+                numActivePools: BigNumber;
+                totalFeesCollected: BigNumber;
+                totalWeightedStake: BigNumber;
+            }
+
+            async function getFinalizationStateAsync(): Promise<FinalizationState> {
+                return {
+                    numActivePools: await testContract.numActivePoolsThisEpoch.callAsync(),
+                    totalFeesCollected: await testContract.totalFeesCollectedThisEpoch.callAsync(),
+                    totalWeightedStake: await testContract.totalWeightedStakeThisEpoch.callAsync(),
+                };
+            }
+
+            interface PayToMakerResult {
+                poolActivatedEvents: IStakingEventsStakingPoolActivatedEventArgs[];
+                fee: BigNumber;
+            }
+
+            async function payToMakerAsync(poolMaker: string, fee?: Numberish): Promise<PayToMakerResult> {
+                const _fee = fee === undefined ? getRandomInteger(1, '1e18') : fee;
+                const receipt = await testContract.payProtocolFee.awaitTransactionSuccessAsync(
+                    poolMaker,
+                    payerAddress,
+                    new BigNumber(_fee),
+                    { from: exchangeAddress, value: _fee },
+                );
+                const events = filterLogsToArguments<IStakingEventsStakingPoolActivatedEventArgs>(
+                    receipt.logs,
+                    IStakingEventsEvents.StakingPoolActivated,
+                );
+                return {
+                    fee: new BigNumber(_fee),
+                    poolActivatedEvents: events,
+                };
+            }
+
+            function toWeightedStake(operatorStake: Numberish, membersStake: Numberish): BigNumber {
+                return new BigNumber(membersStake)
+                    .times(membersStakeWeight)
+                    .dividedToIntegerBy(constants.PPM_DENOMINATOR)
+                    .plus(operatorStake);
+            }
+
+            it('no active pools to start', async () => {
+                const state = await getFinalizationStateAsync();
+                expect(state.numActivePools).to.bignumber.eq(0);
+                expect(state.totalFeesCollected).to.bignumber.eq(0);
+                expect(state.totalWeightedStake).to.bignumber.eq(0);
+            });
+
+            it('pool is not registered to start', async () => {
+                const { poolId } = await createTestPoolAsync();
+                const pool = await testContract.getActiveStakingPoolThisEpoch.callAsync(poolId);
+                expect(pool.feesCollected).to.bignumber.eq(0);
+                expect(pool.membersStake).to.bignumber.eq(0);
+                expect(pool.weightedStake).to.bignumber.eq(0);
+            });
+
+            it('activates a active pool the first time it earns a fee', async () => {
+                const pool = await createTestPoolAsync();
+                const {
+                    poolId,
+                    makers: [poolMaker],
+                } = pool;
+                const { fee, poolActivatedEvents } = await payToMakerAsync(poolMaker);
+                expect(poolActivatedEvents.length).to.eq(1);
+                expect(poolActivatedEvents[0].poolId).to.eq(poolId);
+                const actualPool = await testContract.getActiveStakingPoolThisEpoch.callAsync(poolId);
+                const expectedWeightedStake = toWeightedStake(pool.operatorStake, pool.membersStake);
+                expect(actualPool.feesCollected).to.bignumber.eq(fee);
+                expect(actualPool.membersStake).to.bignumber.eq(pool.membersStake);
+                expect(actualPool.weightedStake).to.bignumber.eq(expectedWeightedStake);
+                const state = await getFinalizationStateAsync();
+                expect(state.numActivePools).to.bignumber.eq(1);
+                expect(state.totalFeesCollected).to.bignumber.eq(fee);
+                expect(state.totalWeightedStake).to.bignumber.eq(expectedWeightedStake);
+            });
+
+            it('only adds to the already activated pool in the same epoch', async () => {
+                const pool = await createTestPoolAsync();
+                const {
+                    poolId,
+                    makers: [poolMaker],
+                } = pool;
+                const { fee: fee1 } = await payToMakerAsync(poolMaker);
+                const { fee: fee2, poolActivatedEvents } = await payToMakerAsync(poolMaker);
+                expect(poolActivatedEvents).to.deep.eq([]);
+                const actualPool = await testContract.getActiveStakingPoolThisEpoch.callAsync(poolId);
+                const expectedWeightedStake = toWeightedStake(pool.operatorStake, pool.membersStake);
+                const fees = BigNumber.sum(fee1, fee2);
+                expect(actualPool.feesCollected).to.bignumber.eq(fees);
+                expect(actualPool.membersStake).to.bignumber.eq(pool.membersStake);
+                expect(actualPool.weightedStake).to.bignumber.eq(expectedWeightedStake);
+                const state = await getFinalizationStateAsync();
+                expect(state.numActivePools).to.bignumber.eq(1);
+                expect(state.totalFeesCollected).to.bignumber.eq(fees);
+                expect(state.totalWeightedStake).to.bignumber.eq(expectedWeightedStake);
+            });
+
+            it('can activate multiple pools in the same epoch', async () => {
+                const pools = await Promise.all(_.times(3, async () => createTestPoolAsync()));
+                let totalFees = new BigNumber(0);
+                let totalWeightedStake = new BigNumber(0);
+                for (const pool of pools) {
+                    const {
+                        poolId,
+                        makers: [poolMaker],
+                    } = pool;
+                    const { fee, poolActivatedEvents } = await payToMakerAsync(poolMaker);
+                    expect(poolActivatedEvents.length).to.eq(1);
+                    expect(poolActivatedEvents[0].poolId).to.eq(poolId);
+                    const actualPool = await testContract.getActiveStakingPoolThisEpoch.callAsync(poolId);
+                    const expectedWeightedStake = toWeightedStake(pool.operatorStake, pool.membersStake);
+                    expect(actualPool.feesCollected).to.bignumber.eq(fee);
+                    expect(actualPool.membersStake).to.bignumber.eq(pool.membersStake);
+                    expect(actualPool.weightedStake).to.bignumber.eq(expectedWeightedStake);
+                    totalFees = totalFees.plus(fee);
+                    totalWeightedStake = totalWeightedStake.plus(expectedWeightedStake);
+                }
+                const state = await getFinalizationStateAsync();
+                expect(state.numActivePools).to.bignumber.eq(pools.length);
+                expect(state.totalFeesCollected).to.bignumber.eq(totalFees);
+                expect(state.totalWeightedStake).to.bignumber.eq(totalWeightedStake);
+            });
+
+            it('resets the pool after the epoch advances', async () => {
+                const pool = await createTestPoolAsync();
+                const {
+                    poolId,
+                    makers: [poolMaker],
+                } = pool;
+                await payToMakerAsync(poolMaker);
+                await testContract.advanceEpoch.awaitTransactionSuccessAsync();
+                const actualPool = await testContract.getActiveStakingPoolThisEpoch.callAsync(poolId);
+                expect(actualPool.feesCollected).to.bignumber.eq(0);
+                expect(actualPool.membersStake).to.bignumber.eq(0);
+                expect(actualPool.weightedStake).to.bignumber.eq(0);
+            });
+
+            describe('Multiple makers', () => {
+                it('fees paid to different makers in the same pool go to that pool', async () => {
+                    const { poolId, makers } = await createTestPoolAsync();
+                    const { fee: fee1 } = await payToMakerAsync(makers[0]);
+                    const { fee: fee2 } = await payToMakerAsync(makers[1]);
+                    const expectedTotalFees = BigNumber.sum(fee1, fee2);
+                    const poolFees = await getProtocolFeesAsync(poolId);
+                    expect(poolFees).to.bignumber.eq(expectedTotalFees);
+                });
+
+                it('fees paid to makers in different pools go to their respective pools', async () => {
+                    const {
+                        poolId: poolId1,
+                        makers: [maker1],
+                    } = await createTestPoolAsync();
+                    const {
+                        poolId: poolId2,
+                        makers: [maker2],
+                    } = await createTestPoolAsync();
+                    const { fee: fee1 } = await payToMakerAsync(maker1);
+                    const { fee: fee2 } = await payToMakerAsync(maker2);
+                    const [poolFees, otherPoolFees] = await Promise.all([
+                        getProtocolFeesAsync(poolId1),
+                        getProtocolFeesAsync(poolId2),
+                    ]);
+                    expect(poolFees).to.bignumber.eq(fee1);
+                    expect(otherPoolFees).to.bignumber.eq(fee2);
+                });
+            });
+        });
     });
 });
+// tslint:disable: max-file-line-count

contracts/staking/test/unit_tests/finalizer_test.ts

@@ -157,20 +157,19 @@ blockchainTests.resets('finalizer unit tests', env => {
     ): Promise<void> {
         const currentEpoch = await getCurrentEpochAsync();
         // Compute the expected rewards for each pool.
-        const poolRewards = await calculatePoolRewardsAsync(rewardsAvailable, activePools);
+        const poolsWithStake = activePools.filter(p => !new BigNumber(p.weightedStake).isZero());
+        const poolRewards = await calculatePoolRewardsAsync(rewardsAvailable, poolsWithStake);
         const totalRewards = BigNumber.sum(...poolRewards);
         const rewardsRemaining = new BigNumber(rewardsAvailable).minus(totalRewards);
-        const nonZeroPoolRewards = poolRewards.filter(r => !r.isZero());
-        const poolsWithNonZeroRewards = _.filter(activePools, (p, i) => !poolRewards[i].isZero());
         const [totalOperatorRewards, totalMembersRewards] = getTotalSplitRewards(activePools, poolRewards);
 
         // Assert the `RewardsPaid` logs.
         const rewardsPaidEvents = getRewardsPaidEvents(finalizationLogs);
-        expect(rewardsPaidEvents.length).to.eq(poolsWithNonZeroRewards.length);
+        expect(rewardsPaidEvents.length).to.eq(poolsWithStake.length);
         for (const i of _.times(rewardsPaidEvents.length)) {
             const event = rewardsPaidEvents[i];
-            const pool = poolsWithNonZeroRewards[i];
+            const pool = poolsWithStake[i];
-            const reward = nonZeroPoolRewards[i];
+            const reward = poolRewards[i];
             const [operatorReward, membersReward] = splitRewards(pool, reward);
             expect(event.epoch).to.bignumber.eq(currentEpoch);
             assertRoughlyEquals(event.operatorReward, operatorReward);
@@ -179,10 +178,11 @@ blockchainTests.resets('finalizer unit tests', env => {
 
         // Assert the `RecordStakingPoolRewards` logs.
         const recordStakingPoolRewardsEvents = getRecordStakingPoolRewardsEvents(finalizationLogs);
+        expect(recordStakingPoolRewardsEvents.length).to.eq(poolsWithStake.length);
         for (const i of _.times(recordStakingPoolRewardsEvents.length)) {
             const event = recordStakingPoolRewardsEvents[i];
-            const pool = poolsWithNonZeroRewards[i];
+            const pool = poolsWithStake[i];
-            const reward = nonZeroPoolRewards[i];
+            const reward = poolRewards[i];
             expect(event.poolId).to.eq(pool.poolId);
             assertRoughlyEquals(event.totalReward, reward);
             assertRoughlyEquals(event.membersStake, pool.membersStake);
@@ -191,7 +191,7 @@ blockchainTests.resets('finalizer unit tests', env => {
         // Assert the `DepositStakingPoolRewards` logs.
         // Make sure they all sum up to the totals.
         const depositStakingPoolRewardsEvents = getDepositStakingPoolRewardsEvents(finalizationLogs);
-        {
+        if (depositStakingPoolRewardsEvents.length > 0) {
             const totalDepositedOperatorRewards = BigNumber.sum(
                 ...depositStakingPoolRewardsEvents.map(e => e.operatorReward),
             );
@@ -406,6 +406,17 @@ blockchainTests.resets('finalizer unit tests', env => {
             return assertFinalizationLogsAndBalancesAsync(INITIAL_BALANCE, pools, allLogs);
         });
 
+        it('can finalize with no rewards', async () => {
+            await testContract.drainBalance.awaitTransactionSuccessAsync();
+            const pools = await Promise.all(_.times(2, async () => addActivePoolAsync()));
+            await testContract.endEpoch.awaitTransactionSuccessAsync();
+            const receipts = await Promise.all(
+                pools.map(pool => testContract.finalizePools.awaitTransactionSuccessAsync([pool.poolId])),
+            );
+            const allLogs = _.flatten(receipts.map(r => r.logs));
+            return assertFinalizationLogsAndBalancesAsync(0, pools, allLogs);
+        });
+
         it('ignores a non-active pool', async () => {
             const pools = await Promise.all(_.times(3, async () => addActivePoolAsync()));
             const nonActivePoolId = hexRandom();