From 369ffc54efe76de3d33eba52f0ca0a909deb1797 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 4 Sep 2019 00:17:12 -0700 Subject: [PATCH] Rich reverts in tests plus readability --- .../contracts/src/fees/MixinExchangeFees.sol | 4 ++-- .../staking/contracts/test/LibFeeMathTest.sol | 2 +- .../staking/test/actors/finalizer_actor.ts | 8 +++++-- contracts/staking/test/actors/staker_actor.ts | 16 ++++++++------ contracts/staking/test/math_test.ts | 5 +++-- contracts/staking/test/rewards_test.ts | 2 +- contracts/staking/test/stake_test.ts | 21 ++++++++++--------- .../staking/test/utils/staking_wrapper.ts | 18 +++++++--------- 8 files changed, 41 insertions(+), 35 deletions(-) diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 8c8225c369..a5995b48e5 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -217,8 +217,8 @@ contract MixinExchangeFees is initialContractBalance, activePools[i].feesCollected, totalFeesCollected, - totalWeightedStake != 0 ? activePools[i].weightedStake : 1, - totalWeightedStake != 0 ? totalWeightedStake : 1 + totalWeightedStake != 0 ? activePools[i].weightedStake : 1, // only rewards are accounted for if no one has staked + totalWeightedStake != 0 ? totalWeightedStake : 1 // this is to avoid divide-by-zero in cobb douglas ); // record reward in vault diff --git a/contracts/staking/contracts/test/LibFeeMathTest.sol b/contracts/staking/contracts/test/LibFeeMathTest.sol index 4b90e03ee2..b637fd8377 100644 --- a/contracts/staking/contracts/test/LibFeeMathTest.sol +++ b/contracts/staking/contracts/test/LibFeeMathTest.sol @@ -18,7 +18,7 @@ */ -pragma solidity ^0.5.5; +pragma solidity ^0.5.9; contract LibFeeMathTest { diff --git a/contracts/staking/test/actors/finalizer_actor.ts b/contracts/staking/test/actors/finalizer_actor.ts index 535fa4f040..cd170767ee 100644 --- a/contracts/staking/test/actors/finalizer_actor.ts +++ b/contracts/staking/test/actors/finalizer_actor.ts @@ -62,10 +62,14 @@ export class FinalizerActor extends BaseActor { await this._stakingWrapper.skipToNextEpochAsync(); // assert reward vault changes const finalRewardVaultBalanceByPoolId = await this._getRewardVaultBalanceByPoolIdAsync(this._poolIds); - expect(finalRewardVaultBalanceByPoolId).to.be.deep.equal(expectedRewardVaultBalanceByPoolId); + expect(finalRewardVaultBalanceByPoolId, 'final pool balances in reward vault').to.be.deep.equal( + expectedRewardVaultBalanceByPoolId, + ); // assert member balances const finalMemberBalancesByPoolId = await this._getMemberBalancesByPoolIdAsync(this._membersByPoolId); - expect(finalMemberBalancesByPoolId).to.be.deep.equal(expectedMemberBalancesByPoolId); + expect(finalMemberBalancesByPoolId, 'final delegator balances in reward vault').to.be.deep.equal( + expectedMemberBalancesByPoolId, + ); } private async _computeExpectedMemberBalancesByPoolIdAsync( diff --git a/contracts/staking/test/actors/staker_actor.ts b/contracts/staking/test/actors/staker_actor.ts index 81c2b3e7ab..4985d844ab 100644 --- a/contracts/staking/test/actors/staker_actor.ts +++ b/contracts/staking/test/actors/staker_actor.ts @@ -21,7 +21,7 @@ export class StakerActor extends BaseActor { // deposit stake const txReceiptPromise = this._stakingWrapper.stakeAsync(this._owner, amount); if (revertError !== undefined) { - await expect(txReceiptPromise).to.revertWith(revertError); + await expect(txReceiptPromise, 'expected revert error').to.revertWith(revertError); return; } await txReceiptPromise; @@ -35,7 +35,9 @@ export class StakerActor extends BaseActor { await this.assertBalancesAsync(expectedStakerBalances); // check zrx balance of vault const finalZrxBalanceOfVault = await this._stakingWrapper.getZrxTokenBalanceOfZrxVaultAsync(); - expect(finalZrxBalanceOfVault).to.be.bignumber.equal(initZrxBalanceOfVault.plus(amount)); + expect(finalZrxBalanceOfVault, 'final balance of zrx vault').to.be.bignumber.equal( + initZrxBalanceOfVault.plus(amount), + ); } public async unstakeAsync(amount: BigNumber, revertError?: RevertError): Promise { @@ -44,7 +46,7 @@ export class StakerActor extends BaseActor { // deposit stake const txReceiptPromise = this._stakingWrapper.unstakeAsync(this._owner, amount); if (revertError !== undefined) { - await expect(txReceiptPromise).to.revertWith(revertError); + await expect(txReceiptPromise, 'expected revert error').to.revertWith(revertError); return; } await txReceiptPromise; @@ -61,7 +63,9 @@ export class StakerActor extends BaseActor { await this.assertBalancesAsync(expectedStakerBalances); // check zrx balance of vault const finalZrxBalanceOfVault = await this._stakingWrapper.getZrxTokenBalanceOfZrxVaultAsync(); - expect(finalZrxBalanceOfVault).to.be.bignumber.equal(initZrxBalanceOfVault.minus(amount)); + expect(finalZrxBalanceOfVault, 'final balance of zrx vault').to.be.bignumber.equal( + initZrxBalanceOfVault.minus(amount), + ); } public async moveStakeAsync( @@ -134,7 +138,7 @@ export class StakerActor extends BaseActor { await this.assertBalancesAsync(expectedStakerBalances); // check zrx balance of vault const finalZrxBalanceOfVault = await this._stakingWrapper.getZrxTokenBalanceOfZrxVaultAsync(); - expect(finalZrxBalanceOfVault).to.be.bignumber.equal(initZrxBalanceOfVault); + expect(finalZrxBalanceOfVault, 'final balance of zrx vault').to.be.bignumber.equal(initZrxBalanceOfVault); } public async goToNextEpochAsync(): Promise { @@ -148,7 +152,7 @@ export class StakerActor extends BaseActor { await this.assertBalancesAsync(expectedStakerBalances); // check zrx balance of vault const finalZrxBalanceOfVault = await this._stakingWrapper.getZrxTokenBalanceOfZrxVaultAsync(); - expect(finalZrxBalanceOfVault).to.be.bignumber.equal(initZrxBalanceOfVault); + expect(finalZrxBalanceOfVault, 'final balance of zrx vault').to.be.bignumber.equal(initZrxBalanceOfVault); } public getNextEpochBalances(balances: StakeBalances): StakeBalances { diff --git a/contracts/staking/test/math_test.ts b/contracts/staking/test/math_test.ts index c0bd1c73b4..ebd55218b9 100644 --- a/contracts/staking/test/math_test.ts +++ b/contracts/staking/test/math_test.ts @@ -1,6 +1,6 @@ import { ERC20ProxyContract, ERC20Wrapper } from '@0x/contracts-asset-proxy'; import { DummyERC20TokenContract } from '@0x/contracts-erc20'; -import { chaiSetup, provider, web3Wrapper } from '@0x/contracts-test-utils'; +import { blockchainTests, chaiSetup, provider, web3Wrapper } from '@0x/contracts-test-utils'; import { BlockchainLifecycle } from '@0x/dev-utils'; import { BigNumber } from '@0x/utils'; import * as chai from 'chai'; @@ -12,7 +12,8 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); // tslint:disable:no-unnecessary-type-assertion -describe('Math Libraries', () => { +// @TODO: This is deprecated by New Fee Math +blockchainTests.skip('Math Libraries', () => { // constants const ZRX_TOKEN_DECIMALS = new BigNumber(18); // tokens & addresses diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index 7956642217..e0ae8fcbce 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -11,7 +11,7 @@ import { MembersByPoolId, OperatorByPoolId, StakeState } from './utils/types'; // tslint:disable:no-unnecessary-type-assertion // tslint:disable:max-file-line-count -blockchainTests.resets.only('Testing Rewards', () => { +blockchainTests.resets('Testing Rewards', () => { // constants const ZRX_TOKEN_DECIMALS = new BigNumber(18); // tokens & addresses diff --git a/contracts/staking/test/stake_test.ts b/contracts/staking/test/stake_test.ts index 96ea553f6a..8cc77d768e 100644 --- a/contracts/staking/test/stake_test.ts +++ b/contracts/staking/test/stake_test.ts @@ -1,7 +1,8 @@ import { ERC20ProxyContract, ERC20Wrapper } from '@0x/contracts-asset-proxy'; import { DummyERC20TokenContract } from '@0x/contracts-erc20'; import { blockchainTests, describe, provider, web3Wrapper } from '@0x/contracts-test-utils'; -import { BigNumber, StringRevertError } from '@0x/utils'; +import { StakingRevertErrors } from '@0x/order-utils'; +import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; import { StakerActor } from './actors/staker_actor'; @@ -9,7 +10,7 @@ import { StakingWrapper } from './utils/staking_wrapper'; import { StakeState, StakeStateInfo } from './utils/types'; // tslint:disable:no-unnecessary-type-assertion -blockchainTests.resets.only('Stake States', () => { +blockchainTests.resets('Stake States', () => { // constants const ZRX_TOKEN_DECIMALS = new BigNumber(18); const ZERO = new BigNumber(0); @@ -94,7 +95,7 @@ blockchainTests.resets.only('Stake States', () => { { state: StakeState.Active }, { state: StakeState.Inactive }, amount, - new StringRevertError('Insufficient Balance'), + new StakingRevertErrors.InsufficientBalanceError(amount, ZERO), ); }); it('should fail to reassign stake', async () => { @@ -113,7 +114,7 @@ blockchainTests.resets.only('Stake States', () => { { state: StakeState.Inactive }, { state: StakeState.Active }, amount, - new StringRevertError('Insufficient Balance'), + new StakingRevertErrors.InsufficientBalanceError(amount, ZERO), ); }); }); @@ -254,20 +255,20 @@ blockchainTests.resets.only('Stake States', () => { it('should fail to unstake with insufficient balance', async () => { const amount = StakingWrapper.toBaseUnitAmount(10); await staker.stakeAsync(amount); - await staker.unstakeAsync(amount, new StringRevertError('INSUFFICIENT_FUNDS')); + await staker.unstakeAsync(amount, new StakingRevertErrors.InsufficientBalanceError(amount, ZERO)); }); it('should fail to unstake in the same epoch as stake was set to inactive', async () => { const amount = StakingWrapper.toBaseUnitAmount(10); await staker.stakeAsync(amount); await staker.moveStakeAsync({ state: StakeState.Active }, { state: StakeState.Inactive }, amount); - await staker.unstakeAsync(amount, new StringRevertError('INSUFFICIENT_FUNDS')); + await staker.unstakeAsync(amount, new StakingRevertErrors.InsufficientBalanceError(amount, ZERO)); }); it('should fail to unstake after being inactive for <1 epoch', async () => { const amount = StakingWrapper.toBaseUnitAmount(10); await staker.stakeAsync(amount); await staker.moveStakeAsync({ state: StakeState.Active }, { state: StakeState.Inactive }, amount); await staker.goToNextEpochAsync(); - await staker.unstakeAsync(amount, new StringRevertError('INSUFFICIENT_FUNDS')); + await staker.unstakeAsync(amount, new StakingRevertErrors.InsufficientBalanceError(amount, ZERO)); }); it('should fail to unstake in same epoch that inactive/withdrawable stake has been reactivated', async () => { const amount = StakingWrapper.toBaseUnitAmount(10); @@ -276,7 +277,7 @@ blockchainTests.resets.only('Stake States', () => { await staker.goToNextEpochAsync(); // stake is now inactive await staker.goToNextEpochAsync(); // stake is now withdrawable await staker.moveStakeAsync({ state: StakeState.Inactive }, { state: StakeState.Active }, amount); - await staker.unstakeAsync(amount, new StringRevertError('INSUFFICIENT_FUNDS')); + await staker.unstakeAsync(amount, new StakingRevertErrors.InsufficientBalanceError(amount, ZERO)); }); it('should fail to unstake one epoch after inactive/withdrawable stake has been reactivated', async () => { const amount = StakingWrapper.toBaseUnitAmount(10); @@ -286,7 +287,7 @@ blockchainTests.resets.only('Stake States', () => { await staker.goToNextEpochAsync(); // stake is now withdrawable await staker.moveStakeAsync({ state: StakeState.Inactive }, { state: StakeState.Active }, amount); await staker.goToNextEpochAsync(); // stake is active and not withdrawable - await staker.unstakeAsync(amount, new StringRevertError('INSUFFICIENT_FUNDS')); + await staker.unstakeAsync(amount, new StakingRevertErrors.InsufficientBalanceError(amount, ZERO)); }); it('should fail to unstake >1 epoch after inactive/withdrawable stake has been reactivated', async () => { const amount = StakingWrapper.toBaseUnitAmount(10); @@ -297,7 +298,7 @@ blockchainTests.resets.only('Stake States', () => { await staker.moveStakeAsync({ state: StakeState.Inactive }, { state: StakeState.Active }, amount); await staker.goToNextEpochAsync(); // stake is active and not withdrawable await staker.goToNextEpochAsync(); // stake is active and not withdrawable - await staker.unstakeAsync(amount, new StringRevertError('INSUFFICIENT_FUNDS')); + await staker.unstakeAsync(amount, new StakingRevertErrors.InsufficientBalanceError(amount, ZERO)); }); }); describe('Simulations', () => { diff --git a/contracts/staking/test/utils/staking_wrapper.ts b/contracts/staking/test/utils/staking_wrapper.ts index ff93e316aa..362bb25324 100644 --- a/contracts/staking/test/utils/staking_wrapper.ts +++ b/contracts/staking/test/utils/staking_wrapper.ts @@ -207,28 +207,24 @@ export class StakingWrapper { public async moveStakeAsync( owner: string, _fromStatus: { - status: number, - poolId?: string + status: number; + poolId?: string; }, _toStatus: { - status: number, - poolId?: string + status: number; + poolId?: string; }, amount: BigNumber, ): Promise { const fromStatus = { status: _fromStatus.status, - poolId: _fromStatus.poolId !== undefined ? _fromStatus.poolId : constants.NIL_POOL_ID + poolId: _fromStatus.poolId !== undefined ? _fromStatus.poolId : constants.NIL_POOL_ID, }; const toStatus = { status: _toStatus.status, - poolId: _toStatus.poolId !== undefined ? _toStatus.poolId : constants.NIL_POOL_ID + poolId: _toStatus.poolId !== undefined ? _toStatus.poolId : constants.NIL_POOL_ID, }; - const calldata = this.getStakingContract().moveStake.getABIEncodedTransactionData( - fromStatus, - toStatus, - amount, - ); + const calldata = this.getStakingContract().moveStake.getABIEncodedTransactionData(fromStatus, toStatus, amount); const txReceipt = await this._executeTransactionAsync(calldata, owner); return txReceipt; }