From e9e6452890716bcb311eabdb4719427a249a23e1 Mon Sep 17 00:00:00 2001 From: Michael Zhu Date: Wed, 30 Oct 2019 13:33:22 -0700 Subject: [PATCH 1/4] add tslint.json to package and update actor mixins --- contracts/integrations/test/actors/fee_recipient.ts | 11 ++++++++++- contracts/integrations/test/actors/keeper.ts | 7 ++++++- contracts/integrations/test/actors/maker.ts | 10 +++++++++- contracts/integrations/test/actors/pool_operator.ts | 11 ++++++++++- contracts/integrations/test/actors/staker.ts | 9 ++++++++- contracts/integrations/test/actors/taker.ts | 10 +++++++++- contracts/integrations/tslint.json | 9 +++++++++ 7 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 contracts/integrations/tslint.json diff --git a/contracts/integrations/test/actors/fee_recipient.ts b/contracts/integrations/test/actors/fee_recipient.ts index a560a81816..22b15d55e3 100644 --- a/contracts/integrations/test/actors/fee_recipient.ts +++ b/contracts/integrations/test/actors/fee_recipient.ts @@ -8,7 +8,16 @@ export interface FeeRecipientConfig extends ActorConfig { verifyingContract?: BaseContract; } -export function FeeRecipientMixin(Base: TBase) { +export interface FeeRecipientInterface { + approvalFactory?: ApprovalFactory; + signCoordinatorApproval: ( + transaction: SignedZeroExTransaction, + txOrigin: string, + signatureType?: SignatureType, + ) => SignedCoordinatorApproval; +} + +export function FeeRecipientMixin(Base: TBase): TBase & Constructor { return class extends Base { public readonly actor: Actor; public readonly approvalFactory?: ApprovalFactory; diff --git a/contracts/integrations/test/actors/keeper.ts b/contracts/integrations/test/actors/keeper.ts index 11bb03078d..ddd778305c 100644 --- a/contracts/integrations/test/actors/keeper.ts +++ b/contracts/integrations/test/actors/keeper.ts @@ -5,7 +5,12 @@ import { BlockParamLiteral, TransactionReceiptWithDecodedLogs } from 'ethereum-t import { Actor, Constructor } from './base'; -export function KeeperMixin(Base: TBase) { +export interface KeeperInterface { + endEpochAsync: (shouldFastForward?: boolean) => Promise; + finalizePoolsAsync: (poolIds?: string[]) => Promise; +} + +export function KeeperMixin(Base: TBase): TBase & Constructor { return class extends Base { public readonly actor: Actor; diff --git a/contracts/integrations/test/actors/maker.ts b/contracts/integrations/test/actors/maker.ts index 3106614f70..4a5dae8082 100644 --- a/contracts/integrations/test/actors/maker.ts +++ b/contracts/integrations/test/actors/maker.ts @@ -8,7 +8,15 @@ export interface MakerConfig extends ActorConfig { orderConfig: Partial; } -export function MakerMixin(Base: TBase) { +export interface MakerInterface { + makerPoolId?: string; + orderFactory: OrderFactory; + signOrderAsync: (customOrderParams?: Partial) => Promise; + cancelOrderAsync: (order: SignedOrder) => Promise; + joinStakingPoolAsync: (poolId: string) => Promise; +} + +export function MakerMixin(Base: TBase): TBase & Constructor { return class extends Base { public makerPoolId?: string; public readonly actor: Actor; diff --git a/contracts/integrations/test/actors/pool_operator.ts b/contracts/integrations/test/actors/pool_operator.ts index 37aa400cc6..7302c776bc 100644 --- a/contracts/integrations/test/actors/pool_operator.ts +++ b/contracts/integrations/test/actors/pool_operator.ts @@ -6,7 +6,16 @@ export interface OperatorShareByPoolId { [poolId: string]: number; } -export function PoolOperatorMixin(Base: TBase) { +export interface PoolOperatorInterface { + operatorShares: OperatorShareByPoolId; + createStakingPoolAsync: (operatorShare: number, addOperatorAsMaker?: boolean) => Promise; + decreaseOperatorShareAsync: ( + poolId: string, + newOperatorShare: number, + ) => Promise; +} + +export function PoolOperatorMixin(Base: TBase): TBase & Constructor { return class extends Base { public readonly operatorShares: OperatorShareByPoolId = {}; public readonly actor: Actor; diff --git a/contracts/integrations/test/actors/staker.ts b/contracts/integrations/test/actors/staker.ts index 60d80cf34c..0fef5e1713 100644 --- a/contracts/integrations/test/actors/staker.ts +++ b/contracts/integrations/test/actors/staker.ts @@ -3,7 +3,14 @@ import { BigNumber } from '@0x/utils'; import { Actor, Constructor } from './base'; -export function StakerMixin(Base: TBase) { +export interface StakerInterface { + stakeAsync: ( + amount: BigNumber, + poolId?: string, + ) => Promise; +} + +export function StakerMixin(Base: TBase): TBase & Constructor { return class extends Base { public readonly actor: Actor; diff --git a/contracts/integrations/test/actors/taker.ts b/contracts/integrations/test/actors/taker.ts index 6a9e13ec26..d616ff7b10 100644 --- a/contracts/integrations/test/actors/taker.ts +++ b/contracts/integrations/test/actors/taker.ts @@ -5,7 +5,15 @@ import { TransactionReceiptWithDecodedLogs, TxData } from 'ethereum-types'; import { Actor, Constructor } from './base'; import { DeploymentManager } from '../utils/deployment_manager'; -export function TakerMixin(Base: TBase) { +export interface TakerInterface { + fillOrderAsync: ( + order: SignedOrder, + fillAmount: BigNumber, + txData?: Partial, + ) => Promise; +} + +export function TakerMixin(Base: TBase): TBase & Constructor { return class extends Base { public readonly actor: Actor; diff --git a/contracts/integrations/tslint.json b/contracts/integrations/tslint.json new file mode 100644 index 0000000000..2a304eb78b --- /dev/null +++ b/contracts/integrations/tslint.json @@ -0,0 +1,9 @@ +{ + "extends": ["@0x/tslint-config"], + "rules": { + "custom-no-magic-numbers": false + }, + "linterOptions": { + "exclude": ["src/artifacts.ts"] + } +} From 43e32f6a1a9e965c24947b512cca13266ff53b7f Mon Sep 17 00:00:00 2001 From: Michael Zhu Date: Wed, 30 Oct 2019 14:43:25 -0700 Subject: [PATCH 2/4] fix other lint errors --- contracts/integrations/test/actors/base.ts | 6 +- .../integrations/test/actors/fee_recipient.ts | 7 +- contracts/integrations/test/actors/hybrids.ts | 2 +- contracts/integrations/test/actors/keeper.ts | 14 ++-- contracts/integrations/test/actors/maker.ts | 7 +- .../integrations/test/actors/pool_operator.ts | 7 +- contracts/integrations/test/actors/staker.ts | 10 +-- contracts/integrations/test/actors/taker.ts | 8 ++- .../integrations/test/actors/tslint.json | 6 ++ contracts/integrations/test/actors/utils.ts | 4 ++ .../test/coordinator/coordinator_test.ts | 22 +++--- .../test/coordinator/deploy_coordinator.ts | 5 +- .../test/forwarder/deploy_forwarder.ts | 5 +- .../test/forwarder/forwarder_test.ts | 12 ++-- .../function_assertion_test.ts | 47 +++++-------- .../fillorder_test.ts | 16 ++--- .../test/utils/deployment_manager.ts | 67 +++++++++---------- .../test/utils/function_assertions.ts | 2 +- 18 files changed, 138 insertions(+), 109 deletions(-) create mode 100644 contracts/integrations/test/actors/tslint.json diff --git a/contracts/integrations/test/actors/base.ts b/contracts/integrations/test/actors/base.ts index 643dbd6188..a763e02f8d 100644 --- a/contracts/integrations/test/actors/base.ts +++ b/contracts/integrations/test/actors/base.ts @@ -21,7 +21,7 @@ export class Actor { public readonly name: string; public readonly privateKey: Buffer; public readonly deployment: DeploymentManager; - protected readonly transactionFactory: TransactionFactory; + protected readonly _transactionFactory: TransactionFactory; constructor(config: ActorConfig) { Actor.count++; @@ -29,7 +29,7 @@ export class Actor { this.name = config.name || this.address; this.deployment = config.deployment; this.privateKey = constants.TESTRPC_PRIVATE_KEYS[config.deployment.accounts.indexOf(this.address)]; - this.transactionFactory = new TransactionFactory( + this._transactionFactory = new TransactionFactory( this.privateKey, config.deployment.exchange.address, config.deployment.chainId, @@ -99,6 +99,6 @@ export class Actor { customTransactionParams: Partial, signatureType: SignatureType = SignatureType.EthSign, ): Promise { - return this.transactionFactory.newSignedTransactionAsync(customTransactionParams, signatureType); + return this._transactionFactory.newSignedTransactionAsync(customTransactionParams, signatureType); } } diff --git a/contracts/integrations/test/actors/fee_recipient.ts b/contracts/integrations/test/actors/fee_recipient.ts index 22b15d55e3..94d813c06a 100644 --- a/contracts/integrations/test/actors/fee_recipient.ts +++ b/contracts/integrations/test/actors/fee_recipient.ts @@ -4,7 +4,7 @@ import { SignatureType, SignedZeroExTransaction } from '@0x/types'; import { Actor, ActorConfig, Constructor } from './base'; -export interface FeeRecipientConfig extends ActorConfig { +interface FeeRecipientConfig extends ActorConfig { verifyingContract?: BaseContract; } @@ -17,6 +17,10 @@ export interface FeeRecipientInterface { ) => SignedCoordinatorApproval; } +/** + * This mixin encapsulates functionaltiy associated with fee recipients within the 0x ecosystem. + * As of writing, the only extra functionality provided is signing Coordinator approvals. + */ export function FeeRecipientMixin(Base: TBase): TBase & Constructor { return class extends Base { public readonly actor: Actor; @@ -28,6 +32,7 @@ export function FeeRecipientMixin(Base: TBase): TBase * base class). */ constructor(...args: any[]) { + // tslint:disable-next-line:no-inferred-empty-object-type super(...args); this.actor = (this as any) as Actor; diff --git a/contracts/integrations/test/actors/hybrids.ts b/contracts/integrations/test/actors/hybrids.ts index c45660a785..7265b0fb93 100644 --- a/contracts/integrations/test/actors/hybrids.ts +++ b/contracts/integrations/test/actors/hybrids.ts @@ -1,8 +1,8 @@ import { Actor } from './base'; +import { KeeperMixin } from './keeper'; import { MakerMixin } from './maker'; import { PoolOperatorMixin } from './pool_operator'; import { StakerMixin } from './staker'; -import { KeeperMixin } from './keeper'; export class OperatorMaker extends PoolOperatorMixin(MakerMixin(Actor)) {} export class StakerMaker extends StakerMixin(MakerMixin(Actor)) {} diff --git a/contracts/integrations/test/actors/keeper.ts b/contracts/integrations/test/actors/keeper.ts index ddd778305c..9389d136aa 100644 --- a/contracts/integrations/test/actors/keeper.ts +++ b/contracts/integrations/test/actors/keeper.ts @@ -10,6 +10,10 @@ export interface KeeperInterface { finalizePoolsAsync: (poolIds?: string[]) => Promise; } +/** + * This mixin encapsulates functionaltiy associated with keepers within the 0x ecosystem. + * This includes ending epochs sand finalizing pools in the staking system. + */ export function KeeperMixin(Base: TBase): TBase & Constructor { return class extends Base { public readonly actor: Actor; @@ -20,6 +24,7 @@ export function KeeperMixin(Base: TBase): TBase & Con * class). */ constructor(...args: any[]) { + // tslint:disable-next-line:no-inferred-empty-object-type super(...args); this.actor = (this as any) as Actor; } @@ -63,11 +68,10 @@ export function KeeperMixin(Base: TBase): TBase & Con } return Promise.all( - poolIds.map( - async poolId => - await stakingWrapper.finalizePool.awaitTransactionSuccessAsync(poolId, { - from: this.actor.address, - }), + poolIds.map(async poolId => + stakingWrapper.finalizePool.awaitTransactionSuccessAsync(poolId, { + from: this.actor.address, + }), ), ); } diff --git a/contracts/integrations/test/actors/maker.ts b/contracts/integrations/test/actors/maker.ts index 4a5dae8082..8e9bcb9f79 100644 --- a/contracts/integrations/test/actors/maker.ts +++ b/contracts/integrations/test/actors/maker.ts @@ -4,7 +4,7 @@ import { TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import { Actor, ActorConfig, Constructor } from './base'; -export interface MakerConfig extends ActorConfig { +interface MakerConfig extends ActorConfig { orderConfig: Partial; } @@ -16,6 +16,10 @@ export interface MakerInterface { joinStakingPoolAsync: (poolId: string) => Promise; } +/** + * This mixin encapsulates functionaltiy associated with makers within the 0x ecosystem. + * This includes signing and canceling orders, as well as joining a staking pool as a maker. + */ export function MakerMixin(Base: TBase): TBase & Constructor { return class extends Base { public makerPoolId?: string; @@ -28,6 +32,7 @@ export function MakerMixin(Base: TBase): TBase & Cons * class). */ constructor(...args: any[]) { + // tslint:disable-next-line:no-inferred-empty-object-type super(...args); this.actor = (this as any) as Actor; diff --git a/contracts/integrations/test/actors/pool_operator.ts b/contracts/integrations/test/actors/pool_operator.ts index 7302c776bc..22763f76ba 100644 --- a/contracts/integrations/test/actors/pool_operator.ts +++ b/contracts/integrations/test/actors/pool_operator.ts @@ -2,7 +2,7 @@ import { TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import { Actor, Constructor } from './base'; -export interface OperatorShareByPoolId { +interface OperatorShareByPoolId { [poolId: string]: number; } @@ -15,6 +15,10 @@ export interface PoolOperatorInterface { ) => Promise; } +/** + * This mixin encapsulates functionaltiy associated with pool operators within the 0x ecosystem. + * This includes creating staking pools and decreasing the operator share of a pool. + */ export function PoolOperatorMixin(Base: TBase): TBase & Constructor { return class extends Base { public readonly operatorShares: OperatorShareByPoolId = {}; @@ -26,6 +30,7 @@ export function PoolOperatorMixin(Base: TBase): TBase * base class). */ constructor(...args: any[]) { + // tslint:disable-next-line:no-inferred-empty-object-type super(...args); this.actor = (this as any) as Actor; } diff --git a/contracts/integrations/test/actors/staker.ts b/contracts/integrations/test/actors/staker.ts index 0fef5e1713..0e2c48e8c6 100644 --- a/contracts/integrations/test/actors/staker.ts +++ b/contracts/integrations/test/actors/staker.ts @@ -4,12 +4,13 @@ import { BigNumber } from '@0x/utils'; import { Actor, Constructor } from './base'; export interface StakerInterface { - stakeAsync: ( - amount: BigNumber, - poolId?: string, - ) => Promise; + stakeAsync: (amount: BigNumber, poolId?: string) => Promise; } +/** + * This mixin encapsulates functionaltiy associated with stakers within the 0x ecosystem. + * This includes staking ZRX (and optionally delegating it to a specific pool). + */ export function StakerMixin(Base: TBase): TBase & Constructor { return class extends Base { public readonly actor: Actor; @@ -20,6 +21,7 @@ export function StakerMixin(Base: TBase): TBase & Con * class). */ constructor(...args: any[]) { + // tslint:disable-next-line:no-inferred-empty-object-type super(...args); this.actor = (this as any) as Actor; } diff --git a/contracts/integrations/test/actors/taker.ts b/contracts/integrations/test/actors/taker.ts index d616ff7b10..a63311b139 100644 --- a/contracts/integrations/test/actors/taker.ts +++ b/contracts/integrations/test/actors/taker.ts @@ -2,9 +2,10 @@ import { SignedOrder } from '@0x/types'; import { BigNumber } from '@0x/utils'; import { TransactionReceiptWithDecodedLogs, TxData } from 'ethereum-types'; -import { Actor, Constructor } from './base'; import { DeploymentManager } from '../utils/deployment_manager'; +import { Actor, Constructor } from './base'; + export interface TakerInterface { fillOrderAsync: ( order: SignedOrder, @@ -13,6 +14,10 @@ export interface TakerInterface { ) => Promise; } +/** + * This mixin encapsulates functionaltiy associated with takers within the 0x ecosystem. + * As of writing, the only extra functionality provided is a utility wrapper around `fillOrder`, + */ export function TakerMixin(Base: TBase): TBase & Constructor { return class extends Base { public readonly actor: Actor; @@ -23,6 +28,7 @@ export function TakerMixin(Base: TBase): TBase & Cons * class). */ constructor(...args: any[]) { + // tslint:disable-next-line:no-inferred-empty-object-type super(...args); this.actor = (this as any) as Actor; } diff --git a/contracts/integrations/test/actors/tslint.json b/contracts/integrations/test/actors/tslint.json new file mode 100644 index 0000000000..afa3246052 --- /dev/null +++ b/contracts/integrations/test/actors/tslint.json @@ -0,0 +1,6 @@ +{ + "extends": ["@0x/tslint-config"], + "rules": { + "max-classes-per-file": false + } +} diff --git a/contracts/integrations/test/actors/utils.ts b/contracts/integrations/test/actors/utils.ts index 0bbc6e07c8..9311e170b6 100644 --- a/contracts/integrations/test/actors/utils.ts +++ b/contracts/integrations/test/actors/utils.ts @@ -3,6 +3,10 @@ import * as _ from 'lodash'; import { Actor } from './base'; +/** + * Utility function to convert Actors into an object mapping readable names to addresses. + * Useful for BalanceStore. + */ export function actorAddressesByName(actors: Actor[]): TokenOwnersByName { return _.zipObject(actors.map(actor => actor.name), actors.map(actor => actor.address)); } diff --git a/contracts/integrations/test/coordinator/coordinator_test.ts b/contracts/integrations/test/coordinator/coordinator_test.ts index 1c48774b0e..17f6c3ca58 100644 --- a/contracts/integrations/test/coordinator/coordinator_test.ts +++ b/contracts/integrations/test/coordinator/coordinator_test.ts @@ -1,7 +1,6 @@ import { CoordinatorContract, SignedCoordinatorApproval } from '@0x/contracts-coordinator'; import { BlockchainBalanceStore, - LocalBalanceStore, constants as exchangeConstants, ExchangeCancelEventArgs, ExchangeCancelUpToEventArgs, @@ -9,6 +8,7 @@ import { ExchangeEvents, ExchangeFillEventArgs, ExchangeFunctionName, + LocalBalanceStore, } from '@0x/contracts-exchange'; import { blockchainTests, expect, hexConcat, hexSlice, verifyEvents } from '@0x/contracts-test-utils'; import { assetDataUtils, CoordinatorRevertErrors, orderHashUtils, transactionHashUtils } from '@0x/order-utils'; @@ -17,9 +17,10 @@ import { BigNumber } from '@0x/utils'; import { TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import { Actor, actorAddressesByName, FeeRecipient, Maker } from '../actors'; -import { deployCoordinatorAsync } from './deploy_coordinator'; import { DeploymentManager } from '../utils/deployment_manager'; +import { deployCoordinatorAsync } from './deploy_coordinator'; + // tslint:disable:no-unnecessary-type-assertion blockchainTests.resets('Coordinator integration tests', env => { let deployment: DeploymentManager; @@ -59,11 +60,11 @@ blockchainTests.resets('Coordinator integration tests', env => { }, }); - taker.configureERC20TokenAsync(takerToken); - taker.configureERC20TokenAsync(takerFeeToken); - taker.configureERC20TokenAsync(deployment.tokens.weth, deployment.staking.stakingProxy.address); - maker.configureERC20TokenAsync(makerToken); - maker.configureERC20TokenAsync(makerFeeToken); + await taker.configureERC20TokenAsync(takerToken); + await taker.configureERC20TokenAsync(takerFeeToken); + await taker.configureERC20TokenAsync(deployment.tokens.weth, deployment.staking.stakingProxy.address); + await maker.configureERC20TokenAsync(makerToken); + await maker.configureERC20TokenAsync(makerFeeToken); balanceStore = new BlockchainBalanceStore( { @@ -79,8 +80,9 @@ blockchainTests.resets('Coordinator integration tests', env => { function simulateFills( orders: SignedOrder[], txReceipt: TransactionReceiptWithDecodedLogs, - msgValue: BigNumber = new BigNumber(0), + msgValue?: BigNumber, ): LocalBalanceStore { + let remainingValue = msgValue || new BigNumber(0); const localBalanceStore = LocalBalanceStore.create(balanceStore); // Transaction gas cost localBalanceStore.burnGas(txReceipt.from, DeploymentManager.gasPrice.times(txReceipt.gasUsed)); @@ -106,13 +108,13 @@ blockchainTests.resets('Coordinator integration tests', env => { ); // Protocol fee - if (msgValue.isGreaterThanOrEqualTo(DeploymentManager.protocolFee)) { + if (remainingValue.isGreaterThanOrEqualTo(DeploymentManager.protocolFee)) { localBalanceStore.sendEth( txReceipt.from, deployment.staking.stakingProxy.address, DeploymentManager.protocolFee, ); - msgValue = msgValue.minus(DeploymentManager.protocolFee); + remainingValue = remainingValue.minus(DeploymentManager.protocolFee); } else { localBalanceStore.transferAsset( taker.address, diff --git a/contracts/integrations/test/coordinator/deploy_coordinator.ts b/contracts/integrations/test/coordinator/deploy_coordinator.ts index 031af5177c..f5b1c93842 100644 --- a/contracts/integrations/test/coordinator/deploy_coordinator.ts +++ b/contracts/integrations/test/coordinator/deploy_coordinator.ts @@ -5,11 +5,14 @@ import { BigNumber } from '@0x/utils'; import { DeploymentManager } from '../utils/deployment_manager'; +/** + * Deploys a Coordinator contract configured to work alongside the provided `deployment`. + */ export async function deployCoordinatorAsync( deployment: DeploymentManager, environment: BlockchainTestsEnvironment, ): Promise { - return await CoordinatorContract.deployFrom0xArtifactAsync( + return CoordinatorContract.deployFrom0xArtifactAsync( artifacts.Coordinator, environment.provider, deployment.txDefaults, diff --git a/contracts/integrations/test/forwarder/deploy_forwarder.ts b/contracts/integrations/test/forwarder/deploy_forwarder.ts index 43b651f5c8..88056fb423 100644 --- a/contracts/integrations/test/forwarder/deploy_forwarder.ts +++ b/contracts/integrations/test/forwarder/deploy_forwarder.ts @@ -5,11 +5,14 @@ import { assetDataUtils } from '@0x/order-utils'; import { DeploymentManager } from '../utils/deployment_manager'; +/** + * Deploys a Forwarder contract configured to work alongside the provided `deployment`. + */ export async function deployForwarderAsync( deployment: DeploymentManager, environment: BlockchainTestsEnvironment, ): Promise { - return await ForwarderContract.deployFrom0xArtifactAsync( + return ForwarderContract.deployFrom0xArtifactAsync( artifacts.Forwarder, environment.provider, deployment.txDefaults, diff --git a/contracts/integrations/test/forwarder/forwarder_test.ts b/contracts/integrations/test/forwarder/forwarder_test.ts index c7246fe72a..53937393df 100644 --- a/contracts/integrations/test/forwarder/forwarder_test.ts +++ b/contracts/integrations/test/forwarder/forwarder_test.ts @@ -19,9 +19,10 @@ import { assetDataUtils, ForwarderRevertErrors } from '@0x/order-utils'; import { BigNumber } from '@0x/utils'; import { Actor, actorAddressesByName, FeeRecipient, Maker } from '../actors'; +import { DeploymentManager } from '../utils/deployment_manager'; + import { deployForwarderAsync } from './deploy_forwarder'; import { ForwarderTestFactory } from './forwarder_test_factory'; -import { DeploymentManager } from '../utils/deployment_manager'; blockchainTests('Forwarder integration tests', env => { let deployment: DeploymentManager; @@ -71,7 +72,7 @@ blockchainTests('Forwarder integration tests', env => { feeRecipientAddress: orderFeeRecipient.address, makerAssetAmount: toBaseUnitAmount(2), takerAssetAmount: toBaseUnitAmount(1), - makerAssetData: makerAssetData, + makerAssetData, takerAssetData: wethAssetData, takerFee: constants.ZERO_AMOUNT, makerFeeAssetData: assetDataUtils.encodeERC20AssetData(makerFeeToken.address), @@ -79,9 +80,9 @@ blockchainTests('Forwarder integration tests', env => { }, }); - maker.configureERC20TokenAsync(makerToken); - maker.configureERC20TokenAsync(makerFeeToken); - maker.configureERC20TokenAsync(anotherErc20Token); + await maker.configureERC20TokenAsync(makerToken); + await maker.configureERC20TokenAsync(makerFeeToken); + await maker.configureERC20TokenAsync(anotherErc20Token); await forwarder.approveMakerAssetProxy.awaitTransactionSuccessAsync(makerAssetData); [nftId] = await maker.configureERC721TokenAsync(erc721Token); @@ -111,7 +112,6 @@ blockchainTests('Forwarder integration tests', env => { blockchainTests.resets('constructor', () => { it('should revert if assetProxy is unregistered', async () => { const chainId = await env.getChainIdAsync(); - const wethAssetData = assetDataUtils.encodeERC20AssetData(deployment.tokens.weth.address); const exchange = await ExchangeContract.deployFrom0xArtifactAsync( exchangeArtifacts.Exchange, env.provider, diff --git a/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts b/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts index b354ef87a6..93f6fcb911 100644 --- a/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts +++ b/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts @@ -1,11 +1,4 @@ -import { - blockchainTests, - constants, - expect, - filterLogsToArguments, - getRandomInteger, - hexRandom, -} from '@0x/contracts-test-utils'; +import { blockchainTests, constants, expect, filterLogsToArguments, getRandomInteger } from '@0x/contracts-test-utils'; import { BigNumber, StringRevertError } from '@0x/utils'; import { TransactionReceiptWithDecodedLogs } from 'ethereum-types'; @@ -30,10 +23,10 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { it('should call the before function with the provided arguments', async () => { let sideEffectTarget = ZERO_AMOUNT; const assertion = new FunctionAssertion(exampleContract.returnInteger, { - before: async (input: BigNumber) => { + before: async (_input: BigNumber) => { sideEffectTarget = randomInput; }, - after: async (beforeInfo: any, result: Result, input: BigNumber) => {}, + after: async (_beforeInfo: any, _result: Result, _input: BigNumber) => null, }); const randomInput = getRandomInteger(ZERO_AMOUNT, MAX_UINT256); await assertion.executeAsync(randomInput); @@ -43,8 +36,8 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { it('should call the after function with the provided arguments', async () => { let sideEffectTarget = ZERO_AMOUNT; const assertion = new FunctionAssertion(exampleContract.returnInteger, { - before: async (input: BigNumber) => {}, - after: async (beforeInfo: any, result: Result, input: BigNumber) => { + before: async (_input: BigNumber) => null, + after: async (_beforeInfo: any, _result: Result, input: BigNumber) => { sideEffectTarget = input; }, }); @@ -55,8 +48,8 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { it('should not fail immediately if the wrapped function fails', async () => { const assertion = new FunctionAssertion(exampleContract.emptyRevert, { - before: async () => {}, - after: async (beforeInfo: any, result: Result) => {}, + before: async () => null, + after: async (_beforeInfo: any, _result: Result) => null, }); await assertion.executeAsync(); }); @@ -65,10 +58,10 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { const randomInput = getRandomInteger(ZERO_AMOUNT, MAX_UINT256); let sideEffectTarget = ZERO_AMOUNT; const assertion = new FunctionAssertion(exampleContract.returnInteger, { - before: async (input: BigNumber) => { + before: async (_input: BigNumber) => { return randomInput; }, - after: async (beforeInfo: any, result: Result, input: BigNumber) => { + after: async (beforeInfo: any, _result: Result, _input: BigNumber) => { sideEffectTarget = beforeInfo; }, }); @@ -79,8 +72,8 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { it('should pass the result from the function call to "after"', async () => { let sideEffectTarget = ZERO_AMOUNT; const assertion = new FunctionAssertion(exampleContract.returnInteger, { - before: async (input: BigNumber) => {}, - after: async (beforeInfo: any, result: Result, input: BigNumber) => { + before: async (_input: BigNumber) => null, + after: async (_beforeInfo: any, result: Result, _input: BigNumber) => { sideEffectTarget = result.data; }, }); @@ -90,12 +83,10 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { }); it('should pass the receipt from the function call to "after"', async () => { - let sideEffectTarget = {} as TransactionReceiptWithDecodedLogs; + let sideEffectTarget: TransactionReceiptWithDecodedLogs; const assertion = new FunctionAssertion(exampleContract.emitEvent, { - before: async (input: string) => { - return {}; - }, - after: async (beforeInfo: {}, result: Result, input: string) => { + before: async (_input: string) => null, + after: async (_beforeInfo: any, result: Result, _input: string) => { if (result.receipt) { sideEffectTarget = result.receipt; } @@ -107,7 +98,7 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { // Ensure that the correct events were emitted. const [event] = filterLogsToArguments( - sideEffectTarget.logs, + sideEffectTarget!.logs, // tslint:disable-line:no-non-null-assertion TestFrameworkEvents.Event, ); expect(event).to.be.deep.eq({ input }); @@ -116,10 +107,8 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { it('should pass the error to "after" if the function call fails', async () => { let sideEffectTarget: Error; const assertion = new FunctionAssertion(exampleContract.stringRevert, { - before: async string => { - return {}; - }, - after: async (any, result: Result, string) => { + before: async _string => null, + after: async (_beforeInfo: any, result: Result, _input: string) => { sideEffectTarget = result.data; }, }); @@ -128,7 +117,7 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { const expectedError = new StringRevertError(message); return expect( - new Promise((resolve, reject) => { + new Promise((_resolve, reject) => { reject(sideEffectTarget); }), ).to.revertWith(expectedError); diff --git a/contracts/integrations/test/internal-integration-tests/fillorder_test.ts b/contracts/integrations/test/internal-integration-tests/fillorder_test.ts index 0405538bcd..ea873b35db 100644 --- a/contracts/integrations/test/internal-integration-tests/fillorder_test.ts +++ b/contracts/integrations/test/internal-integration-tests/fillorder_test.ts @@ -1,4 +1,3 @@ -import { blockchainTests, constants, expect } from '@0x/contracts-test-utils'; import { IERC20TokenEvents, IERC20TokenTransferEventArgs } from '@0x/contracts-erc20'; import { BlockchainBalanceStore, @@ -14,13 +13,13 @@ import { IStakingEventsRewardsPaidEventArgs, IStakingEventsStakingPoolEarnedRewardsInEpochEventArgs, } from '@0x/contracts-staking'; -import { SignedOrder } from '@0x/types'; +import { blockchainTests, constants, expect, toBaseUnitAmount, verifyEvents } from '@0x/contracts-test-utils'; import { assetDataUtils, orderHashUtils } from '@0x/order-utils'; -import { toBaseUnitAmount, verifyEvents } from '@0x/contracts-test-utils'; +import { SignedOrder } from '@0x/types'; import { BigNumber } from '@0x/utils'; import { TransactionReceiptWithDecodedLogs } from 'ethereum-types'; -import { Taker, actorAddressesByName, FeeRecipient, Maker, OperatorStakerMaker, StakerKeeper } from '../actors'; +import { actorAddressesByName, FeeRecipient, Maker, OperatorStakerMaker, StakerKeeper, Taker } from '../actors'; import { DeploymentManager } from '../utils/deployment_manager'; blockchainTests.resets('fillOrder integration tests', env => { @@ -78,7 +77,7 @@ blockchainTests.resets('fillOrder integration tests', env => { await delegator.configureERC20TokenAsync(deployment.tokens.zrx); // Create a staking pool with the operator as a maker. - poolId = await operator.createStakingPoolAsync(0.95 * stakingConstants.PPM, true); + poolId = await operator.createStakingPoolAsync(stakingConstants.PPM * 0.95, true); // A vanilla maker joins the pool as well. await maker.joinStakingPoolAsync(poolId); @@ -97,8 +96,9 @@ blockchainTests.resets('fillOrder integration tests', env => { function simulateFill( order: SignedOrder, txReceipt: TransactionReceiptWithDecodedLogs, - msgValue: BigNumber = DeploymentManager.protocolFee, + msgValue?: BigNumber, ): LocalBalanceStore { + let remainingValue = msgValue !== undefined ? msgValue : DeploymentManager.protocolFee; const localBalanceStore = LocalBalanceStore.create(balanceStore); // Transaction gas cost localBalanceStore.burnGas(txReceipt.from, DeploymentManager.gasPrice.times(txReceipt.gasUsed)); @@ -109,13 +109,13 @@ blockchainTests.resets('fillOrder integration tests', env => { localBalanceStore.transferAsset(maker.address, taker.address, order.makerAssetAmount, order.makerAssetData); // Protocol fee - if (msgValue.isGreaterThanOrEqualTo(DeploymentManager.protocolFee)) { + if (remainingValue.isGreaterThanOrEqualTo(DeploymentManager.protocolFee)) { localBalanceStore.sendEth( txReceipt.from, deployment.staking.stakingProxy.address, DeploymentManager.protocolFee, ); - msgValue = msgValue.minus(DeploymentManager.protocolFee); + remainingValue = remainingValue.minus(DeploymentManager.protocolFee); } else { localBalanceStore.transferAsset( taker.address, diff --git a/contracts/integrations/test/utils/deployment_manager.ts b/contracts/integrations/test/utils/deployment_manager.ts index 876a574032..b67008cbf1 100644 --- a/contracts/integrations/test/utils/deployment_manager.ts +++ b/contracts/integrations/test/utils/deployment_manager.ts @@ -7,7 +7,7 @@ import { StaticCallProxyContract, } from '@0x/contracts-asset-proxy'; import { artifacts as ERC1155Artifacts, ERC1155MintableContract } from '@0x/contracts-erc1155'; -import { DummyERC20TokenContract, artifacts as ERC20Artifacts, WETH9Contract } from '@0x/contracts-erc20'; +import { artifacts as ERC20Artifacts, DummyERC20TokenContract, WETH9Contract } from '@0x/contracts-erc20'; import { artifacts as ERC721Artifacts, DummyERC721TokenContract } from '@0x/contracts-erc721'; import { artifacts as exchangeArtifacts, @@ -256,8 +256,8 @@ export class DeploymentManager { /** * Configures an exchange contract with staking contracts - * @param exchange - * @param staking + * @param exchange The Exchange contract. + * @param staking The Staking contracts. * @param owner An owner address to use when configuring the asset proxies. */ protected static async _configureExchangeWithStakingAsync( @@ -413,45 +413,39 @@ export class DeploymentManager { : constants.NUM_DUMMY_ERC1155_CONTRACTS_TO_DEPLOY; const erc20 = await Promise.all( - _.times( - numErc20TokensToDeploy, - async () => - await DummyERC20TokenContract.deployFrom0xArtifactAsync( - ERC20Artifacts.DummyERC20Token, - environment.provider, - txDefaults, - ERC20Artifacts, - constants.DUMMY_TOKEN_NAME, - constants.DUMMY_TOKEN_SYMBOL, - constants.DUMMY_TOKEN_DECIMALS, - constants.DUMMY_TOKEN_TOTAL_SUPPLY, - ), + _.times(numErc20TokensToDeploy, async () => + DummyERC20TokenContract.deployFrom0xArtifactAsync( + ERC20Artifacts.DummyERC20Token, + environment.provider, + txDefaults, + ERC20Artifacts, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + constants.DUMMY_TOKEN_DECIMALS, + constants.DUMMY_TOKEN_TOTAL_SUPPLY, + ), ), ); const erc721 = await Promise.all( - _.times( - numErc721TokensToDeploy, - async () => - await DummyERC721TokenContract.deployFrom0xArtifactAsync( - ERC721Artifacts.DummyERC721Token, - environment.provider, - txDefaults, - ERC721Artifacts, - constants.DUMMY_TOKEN_NAME, - constants.DUMMY_TOKEN_SYMBOL, - ), + _.times(numErc721TokensToDeploy, async () => + DummyERC721TokenContract.deployFrom0xArtifactAsync( + ERC721Artifacts.DummyERC721Token, + environment.provider, + txDefaults, + ERC721Artifacts, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + ), ), ); const erc1155 = await Promise.all( - _.times( - numErc1155TokensToDeploy, - async () => - await ERC1155MintableContract.deployFrom0xArtifactAsync( - ERC1155Artifacts.ERC1155Mintable, - environment.provider, - txDefaults, - ERC1155Artifacts, - ), + _.times(numErc1155TokensToDeploy, async () => + ERC1155MintableContract.deployFrom0xArtifactAsync( + ERC1155Artifacts.ERC1155Mintable, + environment.provider, + txDefaults, + ERC1155Artifacts, + ), ), ); @@ -492,3 +486,4 @@ export class DeploymentManager { public txDefaults: Partial, ) {} } +// tslint:disable:max-file-line-count diff --git a/contracts/integrations/test/utils/function_assertions.ts b/contracts/integrations/test/utils/function_assertions.ts index fd0f261db8..6a7d7252bb 100644 --- a/contracts/integrations/test/utils/function_assertions.ts +++ b/contracts/integrations/test/utils/function_assertions.ts @@ -74,7 +74,7 @@ export class FunctionAssertion implements Assertion { const beforeInfo = await this.condition.before(...args); // Initialize the callResult so that the default success value is true. - let callResult: Result = { success: true }; + const callResult: Result = { success: true }; // Try to make the call to the function. If it is successful, pass the // result and receipt to the after condition. From af0de72bc37ab6051cb4d3bcebf81be2ea183fee Mon Sep 17 00:00:00 2001 From: Michael Zhu Date: Thu, 31 Oct 2019 15:13:51 -0700 Subject: [PATCH 3/4] address comments --- contracts/integrations/test/coordinator/coordinator_test.ts | 4 ++-- .../test/framework-unit-tests/function_assertion_test.ts | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/contracts/integrations/test/coordinator/coordinator_test.ts b/contracts/integrations/test/coordinator/coordinator_test.ts index 17f6c3ca58..076066c332 100644 --- a/contracts/integrations/test/coordinator/coordinator_test.ts +++ b/contracts/integrations/test/coordinator/coordinator_test.ts @@ -10,7 +10,7 @@ import { ExchangeFunctionName, LocalBalanceStore, } from '@0x/contracts-exchange'; -import { blockchainTests, expect, hexConcat, hexSlice, verifyEvents } from '@0x/contracts-test-utils'; +import { blockchainTests, constants, expect, hexConcat, hexSlice, verifyEvents } from '@0x/contracts-test-utils'; import { assetDataUtils, CoordinatorRevertErrors, orderHashUtils, transactionHashUtils } from '@0x/order-utils'; import { SignedOrder, SignedZeroExTransaction } from '@0x/types'; import { BigNumber } from '@0x/utils'; @@ -82,7 +82,7 @@ blockchainTests.resets('Coordinator integration tests', env => { txReceipt: TransactionReceiptWithDecodedLogs, msgValue?: BigNumber, ): LocalBalanceStore { - let remainingValue = msgValue || new BigNumber(0); + let remainingValue = msgValue || constants.ZERO_AMOUNT; const localBalanceStore = LocalBalanceStore.create(balanceStore); // Transaction gas cost localBalanceStore.burnGas(txReceipt.from, DeploymentManager.gasPrice.times(txReceipt.gasUsed)); diff --git a/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts b/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts index 93f6fcb911..d67aff7979 100644 --- a/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts +++ b/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts @@ -116,11 +116,7 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { await assertion.executeAsync(message); const expectedError = new StringRevertError(message); - return expect( - new Promise((_resolve, reject) => { - reject(sideEffectTarget); - }), - ).to.revertWith(expectedError); + return expect(Promise.reject(sideEffectTarget!)).to.revertWith(expectedError); // tslint:disable-line }); }); }); From 09d13b2bfa1a9b69d2d335e7b4e539ca64382ad0 Mon Sep 17 00:00:00 2001 From: Michael Zhu Date: Thu, 31 Oct 2019 15:49:12 -0700 Subject: [PATCH 4/4] default before/after in FunctionAssertion --- .../function_assertion_test.ts | 10 +-------- .../test/utils/function_assertions.ts | 21 ++++++++++++------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts b/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts index d67aff7979..f10ec1efb7 100644 --- a/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts +++ b/contracts/integrations/test/framework-unit-tests/function_assertion_test.ts @@ -26,7 +26,6 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { before: async (_input: BigNumber) => { sideEffectTarget = randomInput; }, - after: async (_beforeInfo: any, _result: Result, _input: BigNumber) => null, }); const randomInput = getRandomInteger(ZERO_AMOUNT, MAX_UINT256); await assertion.executeAsync(randomInput); @@ -36,7 +35,6 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { it('should call the after function with the provided arguments', async () => { let sideEffectTarget = ZERO_AMOUNT; const assertion = new FunctionAssertion(exampleContract.returnInteger, { - before: async (_input: BigNumber) => null, after: async (_beforeInfo: any, _result: Result, input: BigNumber) => { sideEffectTarget = input; }, @@ -47,10 +45,7 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { }); it('should not fail immediately if the wrapped function fails', async () => { - const assertion = new FunctionAssertion(exampleContract.emptyRevert, { - before: async () => null, - after: async (_beforeInfo: any, _result: Result) => null, - }); + const assertion = new FunctionAssertion<{}>(exampleContract.emptyRevert); await assertion.executeAsync(); }); @@ -72,7 +67,6 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { it('should pass the result from the function call to "after"', async () => { let sideEffectTarget = ZERO_AMOUNT; const assertion = new FunctionAssertion(exampleContract.returnInteger, { - before: async (_input: BigNumber) => null, after: async (_beforeInfo: any, result: Result, _input: BigNumber) => { sideEffectTarget = result.data; }, @@ -85,7 +79,6 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { it('should pass the receipt from the function call to "after"', async () => { let sideEffectTarget: TransactionReceiptWithDecodedLogs; const assertion = new FunctionAssertion(exampleContract.emitEvent, { - before: async (_input: string) => null, after: async (_beforeInfo: any, result: Result, _input: string) => { if (result.receipt) { sideEffectTarget = result.receipt; @@ -107,7 +100,6 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => { it('should pass the error to "after" if the function call fails', async () => { let sideEffectTarget: Error; const assertion = new FunctionAssertion(exampleContract.stringRevert, { - before: async _string => null, after: async (_beforeInfo: any, result: Result, _input: string) => { sideEffectTarget = result.data; }, diff --git a/contracts/integrations/test/utils/function_assertions.ts b/contracts/integrations/test/utils/function_assertions.ts index 6a7d7252bb..37ebceb2cc 100644 --- a/contracts/integrations/test/utils/function_assertions.ts +++ b/contracts/integrations/test/utils/function_assertions.ts @@ -1,9 +1,9 @@ import { PromiseWithTransactionHash } from '@0x/base-contract'; -import { BlockchainTestsEnvironment } from '@0x/contracts-test-utils'; -import { decodeThrownErrorAsRevertError } from '@0x/utils'; -import { BlockParam, CallData, TransactionReceiptWithDecodedLogs, TxData } from 'ethereum-types'; +import { TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; +// tslint:disable:max-classes-per-file + export interface ContractGetterFunction { callAsync: (...args: any[]) => Promise; } @@ -60,8 +60,12 @@ export class FunctionAssertion implements Assertion { // The wrapper function that will be wrapped in assertions. public wrapperFunction: ContractWrapperFunction; - constructor(wrapperFunction: ContractWrapperFunction, condition: Condition) { - this.condition = condition; + constructor(wrapperFunction: ContractWrapperFunction, condition: Partial> = {}) { + this.condition = { + before: _.noop.bind(this), + after: _.noop.bind(this), + ...condition, + }; this.wrapperFunction = wrapperFunction; } @@ -123,10 +127,11 @@ class MetaAssertion implements Assertion { ) {} public async executeAsync(): Promise { - let idx: number; - while ((idx = this.indexGenerator()) > 0) { + let idx = this.indexGenerator(); + while (idx > 0) { const args = await this.assertionGenerators[idx].generator(); - this.assertionGenerators[idx].assertion.executeAsync(...args); + await this.assertionGenerators[idx].assertion.executeAsync(...args); + idx = this.indexGenerator(); } } }