From c72a15b488df2e2e702af828c28f933e2f5f7c13 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Sun, 22 Sep 2019 12:03:53 -0400 Subject: [PATCH] `@0x/contracts-staking`: All tests back up and running. --- .../src/libs/LibStakingRichErrors.sol | 29 ------ .../staking_pools/MixinCumulativeRewards.sol | 39 +------- .../staking_pools/MixinStakingPoolRewards.sol | 2 +- .../contracts/src/sys/MixinFinalizer.sol | 8 +- .../staking/contracts/src/sys/MixinParams.sol | 20 +++++ .../contracts/test/TestDelegatorRewards.sol | 15 ++-- .../staking/contracts/test/TestFinalizer.sol | 17 ++-- .../contracts/test/TestProtocolFees.sol | 12 +-- contracts/staking/test/actors/staker_actor.ts | 12 +++ contracts/staking/test/epoch_test.ts | 2 - contracts/staking/test/params.ts | 5 +- contracts/staking/test/rewards_test.ts | 39 +++----- contracts/staking/test/stake_test.ts | 2 - .../test/unit_tests/delegator_reward_test.ts | 8 +- .../staking/test/unit_tests/finalizer_test.ts | 88 +++++++------------ contracts/staking/test/utils/api_wrapper.ts | 4 +- .../cumulative_reward_tracking_simulation.ts | 2 +- .../order-utils/src/staking_revert_errors.ts | 22 ----- 18 files changed, 113 insertions(+), 213 deletions(-) diff --git a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol index 4ff7f2a0ef..7d3fe19e34 100644 --- a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol +++ b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol @@ -57,12 +57,6 @@ library LibStakingRichErrors { PoolIsFull } - enum CumulativeRewardIntervalErrorCode { - BeginEpochMustBeLessThanEndEpoch, - BeginEpochDoesNotHaveReward, - EndEpochDoesNotHaveReward - } - // bytes4(keccak256("MiscalculatedRewardsError(uint256,uint256)")) bytes4 internal constant MISCALCULATED_REWARDS_ERROR_SELECTOR = 0xf7806c4e; @@ -159,10 +153,6 @@ library LibStakingRichErrors { bytes internal constant INVALID_WETH_ASSET_DATA_ERROR = hex"24bf322c"; - // bytes4(keccak256("CumulativeRewardIntervalError(uint8,bytes32,uint256,uint256)")) - bytes4 internal constant CUMULATIVE_REWARD_INTERVAL_ERROR_SELECTOR = - 0x1f806d55; - // bytes4(keccak256("PreviousEpochNotFinalizedError(uint256,uint256)")) bytes4 internal constant PREVIOUS_EPOCH_NOT_FINALIZED_ERROR_SELECTOR = 0x614b800a; @@ -478,25 +468,6 @@ library LibStakingRichErrors { return INVALID_WETH_ASSET_DATA_ERROR; } - function CumulativeRewardIntervalError( - CumulativeRewardIntervalErrorCode errorCode, - bytes32 poolId, - uint256 beginEpoch, - uint256 endEpoch - ) - internal - pure - returns (bytes memory) - { - return abi.encodeWithSelector( - CUMULATIVE_REWARD_INTERVAL_ERROR_SELECTOR, - errorCode, - poolId, - beginEpoch, - endEpoch - ); - } - function PreviousEpochNotFinalizedError( uint256 unfinalizedEpoch, uint256 unfinalizedPoolsRemaining diff --git a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol index 06fd6c5c8f..a7498677a2 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol @@ -247,50 +247,17 @@ contract MixinCumulativeRewards is } // Sanity check interval - if (beginEpoch > endEpoch) { - LibRichErrors.rrevert( - LibStakingRichErrors.CumulativeRewardIntervalError( - LibStakingRichErrors - .CumulativeRewardIntervalErrorCode - .BeginEpochMustBeLessThanEndEpoch, - poolId, - beginEpoch, - endEpoch - ) - ); - } + require(beginEpoch <= endEpoch, "CR_INTERVAL_INVALID"); // Sanity check begin reward IStructs.Fraction memory beginReward = _cumulativeRewardsByPool[poolId][beginEpoch]; - if (!_isCumulativeRewardSet(beginReward)) { - LibRichErrors.rrevert( - LibStakingRichErrors.CumulativeRewardIntervalError( - LibStakingRichErrors - .CumulativeRewardIntervalErrorCode - .BeginEpochDoesNotHaveReward, - poolId, - beginEpoch, - endEpoch - ) - ); - } + require(_isCumulativeRewardSet(beginReward), "CR_INTERVAL_INVALID_BEGIN"); // Sanity check end reward IStructs.Fraction memory endReward = _cumulativeRewardsByPool[poolId][endEpoch]; - if (!_isCumulativeRewardSet(endReward)) { - LibRichErrors.rrevert( - LibStakingRichErrors.CumulativeRewardIntervalError( - LibStakingRichErrors - .CumulativeRewardIntervalErrorCode - .EndEpochDoesNotHaveReward, - poolId, - beginEpoch, - endEpoch - ) - ); - } + require(_isCumulativeRewardSet(endReward), "CR_INTERVAL_INVALID_END"); // Compute reward reward = LibFractions.scaleDifference( diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index d3cb07f36b..21e2f4df33 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -356,7 +356,7 @@ contract MixinStakingPoolRewards is // If the stake has been touched since the last reward epoch, // it has already been claimed. if (unsyncedStake.currentEpoch >= lastRewardEpoch) { - return 0; + return reward; } // From here we know: `unsyncedStake.currentEpoch < currentEpoch > 0`. diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index 1b8980b771..8208141ba2 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -66,19 +66,19 @@ contract MixinFinalizer is returns (uint256 poolsRemaining) { uint256 closingEpoch = currentEpoch; + IStructs.UnfinalizedState memory state = unfinalizedState; // Make sure the previous epoch has been fully finalized. - if (poolsRemaining != 0) { + if (state.poolsRemaining != 0) { LibRichErrors.rrevert( LibStakingRichErrors.PreviousEpochNotFinalizedError( - closingEpoch.safeSub(1), - poolsRemaining + closingEpoch - 1, + state.poolsRemaining ) ); } // Set up unfinalized state. - IStructs.UnfinalizedState memory state; state.rewardsAvailable = _wrapBalanceToWETHAndGetBalance(); state.poolsRemaining = poolsRemaining = numActivePoolsThisEpoch; state.totalFeesCollected = totalFeesCollectedThisEpoch; diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index 91381b6848..c585aae3c8 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -172,6 +172,26 @@ contract MixinParams is } } + /// @dev Rescind the WETH allowance for `oldSpenders` and grant `newSpenders` + /// an unlimited allowance. + /// @param oldSpenders Addresses to remove allowance from. + /// @param newSpenders Addresses to grant allowance to. + function _transferWETHAllownces( + address[2] memory oldSpenders, + address[2] memory newSpenders + ) + internal + { + IEtherToken weth = IEtherToken(_getWETHAddress()); + // Grant new allowances. + for (uint256 i = 0; i < oldSpenders.length; i++) { + // Rescind old allowance. + weth.approve(oldSpenders[i], 0); + // Grant new allowance. + weth.approve(newSpenders[i], uint256(-1)); + } + } + /// @dev Set all configurable parameters at once. /// @param _epochDurationInSeconds Minimum seconds between epochs. /// @param _rewardDelegatedStakeWeight How much delegated stake is weighted vs operator stake, in ppm. diff --git a/contracts/staking/contracts/test/TestDelegatorRewards.sol b/contracts/staking/contracts/test/TestDelegatorRewards.sol index 6bf5dcaad0..88eb9b5ead 100644 --- a/contracts/staking/contracts/test/TestDelegatorRewards.sol +++ b/contracts/staking/contracts/test/TestDelegatorRewards.sol @@ -22,11 +22,11 @@ pragma experimental ABIEncoderV2; import "../src/interfaces/IStructs.sol"; import "../src/interfaces/IStakingPoolRewardVault.sol"; import "../src/interfaces/IEthVault.sol"; -import "./TestStaking.sol"; +import "./TestStakingNoWETH.sol"; contract TestDelegatorRewards is - TestStaking + TestStakingNoWETH { event RecordDepositToEthVault( address owner, @@ -67,9 +67,9 @@ contract TestDelegatorRewards is mapping (uint256 => mapping (bytes32 => UnfinalizedPoolReward)) private unfinalizedPoolRewardsByEpoch; - /// @dev Expose _finalizePool - function internalFinalizePool(bytes32 poolId) external { - _finalizePool(poolId); + /// @dev Expose the original finalizePool + function originalFinalizePool(bytes32 poolId) external { + MixinFinalizer.finalizePool(poolId); } /// @dev Set unfinalized rewards for a pool in the current epoch. @@ -233,10 +233,11 @@ contract TestDelegatorRewards is ); } + // solhint-disable no-simple-event-func-name /// @dev Overridden to realize `unfinalizedPoolRewardsByEpoch` in /// the current epoch and emit a event, - function _finalizePool(bytes32 poolId) - internal + function finalizePool(bytes32 poolId) + public returns ( uint256 operatorReward, uint256 membersReward, diff --git a/contracts/staking/contracts/test/TestFinalizer.sol b/contracts/staking/contracts/test/TestFinalizer.sol index 7b0acac671..2e897e914b 100644 --- a/contracts/staking/contracts/test/TestFinalizer.sol +++ b/contracts/staking/contracts/test/TestFinalizer.sol @@ -21,21 +21,16 @@ pragma experimental ABIEncoderV2; import "../src/interfaces/IStructs.sol"; import "../src/libs/LibCobbDouglas.sol"; -import "./TestStaking.sol"; +import "./TestStakingNoWETH.sol"; contract TestFinalizer is - TestStaking + TestStakingNoWETH { - event RecordStakingPoolRewards( - bytes32 poolId, - uint256 totalReward, - uint256 membersStake - ); - event DepositStakingPoolRewards( - uint256 operatorReward, - uint256 membersReward + bytes32 poolId, + uint256 reward, + uint256 membersStake ); struct UnfinalizedPoolReward { @@ -160,7 +155,7 @@ contract TestFinalizer is _computeSplitStakingPoolRewards(operatorShare, reward, membersStake); address(_operatorRewardsReceiver).transfer(operatorReward); address(_membersRewardsReceiver).transfer(membersReward); - emit DepositStakingPoolRewards(operatorReward, membersReward); + emit DepositStakingPoolRewards(poolId, reward, membersStake); } /// @dev Overriden to just increase the epoch counter. diff --git a/contracts/staking/contracts/test/TestProtocolFees.sol b/contracts/staking/contracts/test/TestProtocolFees.sol index a9d98d5a6e..4fe8756290 100644 --- a/contracts/staking/contracts/test/TestProtocolFees.sol +++ b/contracts/staking/contracts/test/TestProtocolFees.sol @@ -68,18 +68,14 @@ contract TestProtocolFees is currentEpoch += 1; } - function getWethAssetData() external pure returns (bytes memory) { - return WETH_ASSET_DATA; - } - /// @dev Create a test pool. function createTestPool( bytes32 poolId, uint256 operatorStake, uint256 membersStake, - address[] memory makerAddresses + address[] calldata makerAddresses ) - public + external { TestPool storage pool = _testPools[poolId]; pool.operatorStake = operatorStake; @@ -102,6 +98,10 @@ contract TestProtocolFees is emit ERC20ProxyTransferFrom(assetData, from, to, amount); } + function getWethAssetData() external pure returns (bytes memory) { + return WETH_ASSET_DATA; + } + /// @dev Overridden to use test pools. function getStakingPoolIdOfMaker(address makerAddress) public diff --git a/contracts/staking/test/actors/staker_actor.ts b/contracts/staking/test/actors/staker_actor.ts index 07423f5668..cb240e43eb 100644 --- a/contracts/staking/test/actors/staker_actor.ts +++ b/contracts/staking/test/actors/staker_actor.ts @@ -155,6 +155,18 @@ export class StakerActor extends BaseActor { ); } + public async syncDelegatorRewardsAsync(poolId: string, revertError?: RevertError): Promise { + const txReceiptPromise = this._stakingApiWrapper.stakingContract.syncDelegatorRewards.awaitTransactionSuccessAsync( + poolId, + { from: this._owner }, + ); + if (revertError !== undefined) { + await expect(txReceiptPromise, 'expected revert error').to.revertWith(revertError); + return; + } + await txReceiptPromise; + } + public async goToNextEpochAsync(): Promise { // cache balances const initZrxBalanceOfVault = await this._stakingApiWrapper.utils.getZrxTokenBalanceOfZrxVaultAsync(); diff --git a/contracts/staking/test/epoch_test.ts b/contracts/staking/test/epoch_test.ts index 13ca7e6635..ab5d470f34 100644 --- a/contracts/staking/test/epoch_test.ts +++ b/contracts/staking/test/epoch_test.ts @@ -2,8 +2,6 @@ import { ERC20Wrapper } from '@0x/contracts-asset-proxy'; import { blockchainTests, expect } from '@0x/contracts-test-utils'; import * as _ from 'lodash'; -import { artifacts } from '../src'; - import { deployAndConfigureContractsAsync, StakingApiWrapper } from './utils/api_wrapper'; import { constants as stakingConstants } from './utils/constants'; diff --git a/contracts/staking/test/params.ts b/contracts/staking/test/params.ts index 90d27ca819..abab21f8be 100644 --- a/contracts/staking/test/params.ts +++ b/contracts/staking/test/params.ts @@ -30,7 +30,10 @@ blockchainTests('Configurable Parameters unit tests', env => { }); blockchainTests.resets('setParams()', () => { - async function setParamsAndAssertAsync(params: Partial, from?: string): Promise { + async function setParamsAndAssertAsync( + params: Partial, + from?: string, + ): Promise { const _params = { ...stakingConstants.DEFAULT_PARAMS, ...params, diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index 92902aca29..836a7e1d55 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -3,8 +3,6 @@ import { blockchainTests, describe, expect, shortZip } from '@0x/contracts-test- import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; -import { artifacts } from '../src'; - import { FinalizerActor } from './actors/finalizer_actor'; import { PoolOperatorActor } from './actors/pool_operator_actor'; import { StakerActor } from './actors/staker_actor'; @@ -45,7 +43,7 @@ blockchainTests.resets('Testing Rewards', env => { stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper); // set up staking parameters await stakingApiWrapper.utils.setParamsAsync({ - minimumPoolStake: new BigNumber(1), + minimumPoolStake: new BigNumber(2), cobbDouglasAlphaNumerator: new BigNumber(1), cobbDouglasAlphaDenominator: new BigNumber(6), rewardVaultAddress: stakingApiWrapper.rewardVaultContract.address, @@ -60,7 +58,7 @@ blockchainTests.resets('Testing Rewards', env => { poolId = await poolOperator.createStakingPoolAsync(0, true); // Stake something in the pool or else it won't get any rewards. poolOperatorStaker = new StakerActor(poolOperator.getOwner(), stakingApiWrapper); - await poolOperatorStaker.stakeWithPoolAsync(poolId, new BigNumber(1)); + await poolOperatorStaker.stakeWithPoolAsync(poolId, new BigNumber(2)); // set exchange address await stakingApiWrapper.stakingContract.addExchangeAddress.awaitTransactionSuccessAsync(exchangeAddress); // associate operators for tracking in Finalizer @@ -609,18 +607,14 @@ blockchainTests.resets('Testing Rewards', env => { // finalize const reward = toBaseUnitAmount(10); await payProtocolFeeAndFinalize(reward); - // Undelegate 0 stake to move the rewards into the EthVault. - await staker.moveStakeAsync( - new StakeInfo(StakeStatus.Delegated, poolId), - new StakeInfo(StakeStatus.Active), - toBaseUnitAmount(0), - ); + // Sync rewards to move the rewards into the EthVault. + await staker.syncDelegatorRewardsAsync(poolId); await validateEndBalances({ stakerRewardVaultBalance_1: toBaseUnitAmount(0), stakerEthVaultBalance_1: reward, }); }); - it(`should split payout between two delegators when undelegating`, async () => { + it(`should split payout between two delegators when syncing rewards`, async () => { const stakeAmounts = [toBaseUnitAmount(5), toBaseUnitAmount(10)]; const totalStakeAmount = BigNumber.sum(...stakeAmounts); // stake and delegate both @@ -633,13 +627,9 @@ blockchainTests.resets('Testing Rewards', env => { // finalize const reward = toBaseUnitAmount(10); await payProtocolFeeAndFinalize(reward); - // Undelegate 0 stake to move rewards from RewardVault into the EthVault. + // Sync rewards to move rewards from RewardVault into the EthVault. for (const [staker] of _.reverse(stakersAndStake)) { - await staker.moveStakeAsync( - new StakeInfo(StakeStatus.Delegated, poolId), - new StakeInfo(StakeStatus.Active), - toBaseUnitAmount(0), - ); + await staker.syncDelegatorRewardsAsync(poolId); } const expectedStakerRewards = stakeAmounts.map(n => reward.times(n).dividedToIntegerBy(totalStakeAmount)); await validateEndBalances({ @@ -651,7 +641,7 @@ blockchainTests.resets('Testing Rewards', env => { membersRewardVaultBalance: new BigNumber(1), // Rounding error }); }); - it(`delegator should not be credited payout twice by undelegating twice`, async () => { + it(`delegator should not be credited payout twice by syncing rewards twice`, async () => { const stakeAmounts = [toBaseUnitAmount(5), toBaseUnitAmount(10)]; const totalStakeAmount = BigNumber.sum(...stakeAmounts); // stake and delegate both @@ -673,17 +663,10 @@ blockchainTests.resets('Testing Rewards', env => { poolRewardVaultBalance: reward, membersRewardVaultBalance: reward, }); - const undelegateZeroAsync = async (staker: StakerActor) => { - return staker.moveStakeAsync( - new StakeInfo(StakeStatus.Delegated, poolId), - new StakeInfo(StakeStatus.Active), - toBaseUnitAmount(0), - ); - }; - // First staker will undelegate 0 to get rewards transferred to EthVault. + // First staker will sync rewards to get rewards transferred to EthVault. const sneakyStaker = stakers[0]; const sneakyStakerExpectedEthVaultBalance = expectedStakerRewards[0]; - await undelegateZeroAsync(sneakyStaker); + await sneakyStaker.syncDelegatorRewardsAsync(poolId); // Should have been credited the correct amount of rewards. let sneakyStakerEthVaultBalance = await stakingApiWrapper.ethVaultContract.balanceOf.callAsync( sneakyStaker.getOwner(), @@ -692,7 +675,7 @@ blockchainTests.resets('Testing Rewards', env => { sneakyStakerExpectedEthVaultBalance, ); // Now he'll try to do it again to see if he gets credited twice. - await undelegateZeroAsync(sneakyStaker); + await sneakyStaker.syncDelegatorRewardsAsync(poolId); /// The total amount credited should remain the same. sneakyStakerEthVaultBalance = await stakingApiWrapper.ethVaultContract.balanceOf.callAsync( sneakyStaker.getOwner(), diff --git a/contracts/staking/test/stake_test.ts b/contracts/staking/test/stake_test.ts index bdeba00631..a8b0ad975d 100644 --- a/contracts/staking/test/stake_test.ts +++ b/contracts/staking/test/stake_test.ts @@ -4,8 +4,6 @@ import { StakingRevertErrors } from '@0x/order-utils'; import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; -import { artifacts } from '../src'; - import { StakerActor } from './actors/staker_actor'; import { deployAndConfigureContractsAsync, StakingApiWrapper } from './utils/api_wrapper'; import { toBaseUnitAmount } from './utils/number_utils'; diff --git a/contracts/staking/test/unit_tests/delegator_reward_test.ts b/contracts/staking/test/unit_tests/delegator_reward_test.ts index a36a5ab97a..37d62ea8e3 100644 --- a/contracts/staking/test/unit_tests/delegator_reward_test.ts +++ b/contracts/staking/test/unit_tests/delegator_reward_test.ts @@ -223,7 +223,7 @@ blockchainTests.resets('delegator unit rewards', env => { } async function finalizePoolAsync(poolId: string): Promise> { - const receipt = await testContract.internalFinalizePool.awaitTransactionSuccessAsync(poolId); + const receipt = await testContract.originalFinalizePool.awaitTransactionSuccessAsync(poolId); const [ethVaultDeposit, rewardVaultDeposit] = getDepositsFromLogs(receipt.logs, poolId); return { ethVaultDeposit, @@ -379,7 +379,7 @@ blockchainTests.resets('delegator unit rewards', env => { assertRoughlyEquals(delegatorReward, expectedDelegatorRewards); }); - it('has correct reward immediately after unstaking', async () => { + it('has correct reward immediately after undelegating', async () => { const poolId = hexRandom(); const { delegator, stake } = await delegateStakeAsync(poolId); await advanceEpochAsync(); // epoch 1 (stake now active) @@ -392,7 +392,7 @@ blockchainTests.resets('delegator unit rewards', env => { expect(delegatorReward).to.bignumber.eq(0); }); - it('has correct reward immediately after unstaking and restaking', async () => { + it('has correct reward immediately after undelegating and redelegating', async () => { const poolId = hexRandom(); const { delegator, stake } = await delegateStakeAsync(poolId); await advanceEpochAsync(); // epoch 1 (stake now active) @@ -406,7 +406,7 @@ blockchainTests.resets('delegator unit rewards', env => { expect(delegatorReward).to.bignumber.eq(0); }); - it('has correct reward immediately after unstaking, restaking, and rewarding fees', async () => { + it('has correct reward immediately after undelegating, redelegating, and rewarding fees', async () => { const poolId = hexRandom(); const { delegator, stake } = await delegateStakeAsync(poolId); await advanceEpochAsync(); // epoch 1 (stake now active) diff --git a/contracts/staking/test/unit_tests/finalizer_test.ts b/contracts/staking/test/unit_tests/finalizer_test.ts index 581299ed77..45a29c452c 100644 --- a/contracts/staking/test/unit_tests/finalizer_test.ts +++ b/contracts/staking/test/unit_tests/finalizer_test.ts @@ -21,13 +21,8 @@ import { TestFinalizerContract, TestFinalizerDepositStakingPoolRewardsEventArgs as DepositStakingPoolRewardsEventArgs, TestFinalizerEvents, - TestFinalizerRecordStakingPoolRewardsEventArgs as RecordStakingPoolRewardsEventArgs, } from '../../src'; -import { - assertRoughlyEquals as _assertIntegerRoughlyEquals, - getRandomInteger, - toBaseUnitAmount, -} from '../utils/number_utils'; +import { assertIntegerRoughlyEquals, getRandomInteger, toBaseUnitAmount } from '../utils/number_utils'; blockchainTests.resets('finalizer unit tests', env => { const { ZERO_AMOUNT } = constants; @@ -90,7 +85,7 @@ blockchainTests.resets('finalizer unit tests', env => { return _opts; } - interface FinalizationState { + interface UnfinalizedState { rewardsAvailable: Numberish; poolsRemaining: number; totalFeesCollected: Numberish; @@ -98,7 +93,7 @@ blockchainTests.resets('finalizer unit tests', env => { totalRewardsFinalized: Numberish; } - async function getUnfinalizedStateAsync(): Promise { + async function getUnfinalizedStateAsync(): Promise { const r = await testContract.unfinalizedState.callAsync(); return { rewardsAvailable: r[0], @@ -113,12 +108,12 @@ blockchainTests.resets('finalizer unit tests', env => { const logs = [] as LogEntry[]; for (const poolId of poolIds) { const receipt = await testContract.finalizePool.awaitTransactionSuccessAsync(poolId); - logs.splice(logs.length - 1, 0, ...receipt.logs); + logs.splice(logs.length, 0, ...receipt.logs); } return logs; } - async function assertFinalizationStateAsync(expected: Partial): Promise { + async function assertUnfinalizedStateAsync(expected: Partial): Promise { const actual = await getUnfinalizedStateAsync(); assertEqualNumberFields(actual, expected); } @@ -135,10 +130,6 @@ blockchainTests.resets('finalizer unit tests', env => { assertEqualNumberFields(events[0], args); } - function assertRoughlyEquals(actual: Numberish, expected: Numberish): void { - _assertIntegerRoughlyEquals(actual, expected, 5); - } - function assertEqualNumberFields(actual: T, expected: Partial): void { for (const key of Object.keys(actual)) { const a = (actual as any)[key] as BigNumber; @@ -171,42 +162,33 @@ blockchainTests.resets('finalizer unit tests', env => { const reward = poolRewards[i]; const [operatorReward, membersReward] = splitRewards(pool, reward); expect(event.epoch).to.bignumber.eq(currentEpoch); - assertRoughlyEquals(event.operatorReward, operatorReward); - assertRoughlyEquals(event.membersReward, membersReward); - } - - // 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 = poolsWithStake[i]; - const reward = poolRewards[i]; - expect(event.poolId).to.eq(pool.poolId); - assertRoughlyEquals(event.totalReward, reward); - assertRoughlyEquals(event.membersStake, pool.membersStake); + assertIntegerRoughlyEquals(event.operatorReward, operatorReward); + assertIntegerRoughlyEquals(event.membersReward, membersReward); } // Assert the `DepositStakingPoolRewards` logs. - // Make sure they all sum up to the totals. const depositStakingPoolRewardsEvents = getDepositStakingPoolRewardsEvents(finalizationLogs); + expect(depositStakingPoolRewardsEvents.length).to.eq(poolsWithStake.length); + for (const i of _.times(depositStakingPoolRewardsEvents.length)) { + const event = depositStakingPoolRewardsEvents[i]; + const pool = poolsWithStake[i]; + const reward = poolRewards[i]; + expect(event.poolId).to.eq(pool.poolId); + assertIntegerRoughlyEquals(event.reward, reward); + assertIntegerRoughlyEquals(event.membersStake, pool.membersStake); + } + // Make sure they all sum up to the totals. if (depositStakingPoolRewardsEvents.length > 0) { - const totalDepositedOperatorRewards = BigNumber.sum( - ...depositStakingPoolRewardsEvents.map(e => e.operatorReward), - ); - const totalDepositedMembersRewards = BigNumber.sum( - ...depositStakingPoolRewardsEvents.map(e => e.membersReward), - ); - assertRoughlyEquals(totalDepositedOperatorRewards, totalOperatorRewards); - assertRoughlyEquals(totalDepositedMembersRewards, totalMembersRewards); + const totalDepositRewards = BigNumber.sum(...depositStakingPoolRewardsEvents.map(e => e.reward)); + assertIntegerRoughlyEquals(totalDepositRewards, totalRewards); } // Assert the `EpochFinalized` logs. const epochFinalizedEvents = getEpochFinalizedEvents(finalizationLogs); expect(epochFinalizedEvents.length).to.eq(1); expect(epochFinalizedEvents[0].epoch).to.bignumber.eq(currentEpoch - 1); - assertRoughlyEquals(epochFinalizedEvents[0].rewardsPaid, totalRewards); - assertRoughlyEquals(epochFinalizedEvents[0].rewardsRemaining, rewardsRemaining); + assertIntegerRoughlyEquals(epochFinalizedEvents[0].rewardsPaid, totalRewards); + assertIntegerRoughlyEquals(epochFinalizedEvents[0].rewardsRemaining, rewardsRemaining); // Assert the receiver balances. await assertReceiverBalancesAsync(totalOperatorRewards, totalMembersRewards); @@ -214,9 +196,9 @@ blockchainTests.resets('finalizer unit tests', env => { async function assertReceiverBalancesAsync(operatorRewards: Numberish, membersRewards: Numberish): Promise { const operatorRewardsBalance = await getBalanceOfAsync(operatorRewardsReceiver); - assertRoughlyEquals(operatorRewardsBalance, operatorRewards); + assertIntegerRoughlyEquals(operatorRewardsBalance, operatorRewards); const membersRewardsBalance = await getBalanceOfAsync(membersRewardsReceiver); - assertRoughlyEquals(membersRewardsBalance, membersRewards); + assertIntegerRoughlyEquals(membersRewardsBalance, membersRewards); } async function calculatePoolRewardsAsync( @@ -269,13 +251,6 @@ blockchainTests.resets('finalizer unit tests', env => { return filterLogsToArguments(logs, IStakingEventsEvents.EpochFinalized); } - function getRecordStakingPoolRewardsEvents(logs: LogEntry[]): RecordStakingPoolRewardsEventArgs[] { - return filterLogsToArguments( - logs, - TestFinalizerEvents.RecordStakingPoolRewards, - ); - } - function getDepositStakingPoolRewardsEvents(logs: LogEntry[]): DepositStakingPoolRewardsEventArgs[] { return filterLogsToArguments( logs, @@ -338,18 +313,19 @@ blockchainTests.resets('finalizer unit tests', env => { await testContract.endEpoch.awaitTransactionSuccessAsync(); const epoch = await testContract.currentEpoch.callAsync(); expect(epoch).to.bignumber.eq(INITIAL_EPOCH + 1); - return assertFinalizationStateAsync({ - poolsRemaining: 0, - totalFeesCollected: 0, - totalWeightedStake: 0, - }); + const numActivePools = await testContract.numActivePoolsThisEpoch.callAsync(); + const totalFees = await testContract.totalFeesCollectedThisEpoch.callAsync(); + const totalStake = await testContract.totalWeightedStakeThisEpoch.callAsync(); + expect(numActivePools).to.bignumber.eq(0); + expect(totalFees).to.bignumber.eq(0); + expect(totalStake).to.bignumber.eq(0); }); - it('prepares finalization state', async () => { + it('prepares unfinalized state', async () => { // Add a pool so there is state to clear. const pool = await addActivePoolAsync(); await testContract.endEpoch.awaitTransactionSuccessAsync(); - return assertFinalizationStateAsync({ + return assertUnfinalizedStateAsync({ poolsRemaining: 1, rewardsAvailable: INITIAL_BALANCE, totalFeesCollected: pool.feesCollected, @@ -489,7 +465,7 @@ blockchainTests.resets('finalizer unit tests', env => { const finalizeLogs = await finalizePoolsAsync(poolIds); const { rewardsRemaining: rolledOverRewards } = getEpochFinalizedEvents(finalizeLogs)[0]; await Promise.all(poolIds.map(async id => addActivePoolAsync({ poolId: id }))); - const {logs: endEpochLogs } = await testContract.endEpoch.awaitTransactionSuccessAsync(); + const { logs: endEpochLogs } = await testContract.endEpoch.awaitTransactionSuccessAsync(); const { rewardsAvailable } = getEpochEndedEvents(endEpochLogs)[0]; expect(rewardsAvailable).to.bignumber.eq(rolledOverRewards); }); diff --git a/contracts/staking/test/utils/api_wrapper.ts b/contracts/staking/test/utils/api_wrapper.ts index 7b999c411f..0d50fd91aa 100644 --- a/contracts/staking/test/utils/api_wrapper.ts +++ b/contracts/staking/test/utils/api_wrapper.ts @@ -55,9 +55,7 @@ export class StakingApiWrapper { let totalGasUsed = 0; const allLogs = [] as DecodedLogs; for (const poolId of endOfEpochInfo.activePoolIds) { - const receipt = await this.stakingContract.finalizePool.awaitTransactionSuccessAsync( - poolId, - ); + const receipt = await this.stakingContract.finalizePool.awaitTransactionSuccessAsync(poolId); totalGasUsed += receipt.gasUsed; allLogs.splice(allLogs.length, 0, ...(receipt.logs as DecodedLogs)); } diff --git a/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts b/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts index 40a388bb72..7549e709b8 100644 --- a/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts +++ b/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts @@ -175,7 +175,7 @@ export class CumulativeRewardTrackingSimulation { if (receipt !== undefined) { logs = receipt.logs as DecodedLogs; } - combinedLogs.splice(combinedLogs.length - 1, 0, ...logs); + combinedLogs.splice(combinedLogs.length, 0, ...logs); } return combinedLogs; } diff --git a/packages/order-utils/src/staking_revert_errors.ts b/packages/order-utils/src/staking_revert_errors.ts index 0d0f53dff4..bce6f135d3 100644 --- a/packages/order-utils/src/staking_revert_errors.ts +++ b/packages/order-utils/src/staking_revert_errors.ts @@ -40,12 +40,6 @@ export enum InitializationErrorCode { MixinParamsAlreadyInitialized, } -export enum CumulativeRewardIntervalErrorCode { - BeginEpochMustBeLessThanEndEpoch, - BeginEpochDoesNotHaveReward, - EndEpochDoesNotHaveReward, -} - export class MiscalculatedRewardsError extends RevertError { constructor(totalRewardsPaid?: BigNumber | number | string, initialContractBalance?: BigNumber | number | string) { super( @@ -244,21 +238,6 @@ export class ProxyDestinationCannotBeNilError extends RevertError { } } -export class CumulativeRewardIntervalError extends RevertError { - constructor( - errorCode?: CumulativeRewardIntervalErrorCode, - poolId?: string, - beginEpoch?: BigNumber | number | string, - endEpoch?: BigNumber | number | string, - ) { - super( - 'CumulativeRewardIntervalError', - 'CumulativeRewardIntervalError(uint8 errorCode, bytes32 poolId, uint256 beginEpoch, uint256 endEpoch)', - { errorCode, poolId, beginEpoch, endEpoch }, - ); - } -} - export class PreviousEpochNotFinalizedError extends RevertError { constructor(closingEpoch?: BigNumber | number | string, unfinalizedPoolsRemaining?: BigNumber | number | string) { super( @@ -272,7 +251,6 @@ export class PreviousEpochNotFinalizedError extends RevertError { const types = [ AmountExceedsBalanceOfPoolError, BlockTimestampTooLowError, - CumulativeRewardIntervalError, EthVaultNotSetError, ExchangeAddressAlreadyRegisteredError, ExchangeAddressNotRegisteredError,