From 8ed3d59f969c2f07e34739c5a08c69de583cef88 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Mon, 30 Jul 2018 22:14:07 -0700 Subject: Add more assertions --- .../contract-wrappers/src/contract_wrappers.ts | 7 ++- .../src/contract_wrappers/forwarder_wrapper.ts | 41 ++++++++++++++- packages/contract-wrappers/src/utils/assert.ts | 60 ++++++++++++++++++++-- 3 files changed, 102 insertions(+), 6 deletions(-) diff --git a/packages/contract-wrappers/src/contract_wrappers.ts b/packages/contract-wrappers/src/contract_wrappers.ts index 76aefbdcf..4277a0746 100644 --- a/packages/contract-wrappers/src/contract_wrappers.ts +++ b/packages/contract-wrappers/src/contract_wrappers.ts @@ -110,7 +110,12 @@ export class ContractWrappers { config.zrxContractAddress, blockPollingIntervalMs, ); - this.forwarder = new ForwarderWrapper(this._web3Wrapper, config.networkId, config.forwarderContractAddress); + this.forwarder = new ForwarderWrapper( + this._web3Wrapper, + config.networkId, + config.forwarderContractAddress, + config.zrxContractAddress, + ); } /** * Sets a new web3 provider for 0x.js. Updating the provider will stop all diff --git a/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts index db1e5390a..beb2d1c81 100644 --- a/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts +++ b/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts @@ -22,9 +22,16 @@ export class ForwarderWrapper extends ContractWrapper { public abi: ContractAbi = artifacts.Forwarder.compilerOutput.abi; private _forwarderContractIfExists?: ForwarderContract; private _contractAddressIfExists?: string; - constructor(web3Wrapper: Web3Wrapper, networkId: number, contractAddressIfExists?: string) { + private _zrxContractAddressIfExists?: string; + constructor( + web3Wrapper: Web3Wrapper, + networkId: number, + contractAddressIfExists?: string, + zrxContractAddressIfExists?: string, + ) { super(web3Wrapper, networkId); this._contractAddressIfExists = contractAddressIfExists; + this._zrxContractAddressIfExists = zrxContractAddressIfExists; } /** * Purchases as much of orders' makerAssets as possible by selling up to 95% of transaction's ETH value. @@ -53,6 +60,7 @@ export class ForwarderWrapper extends ContractWrapper { feeRecipientAddress: string = constants.NULL_ADDRESS, txOpts: TransactionOpts = {}, ): Promise { + // type assertions assert.doesConformToSchema('signedOrders', signedOrders, schemas.signedOrdersSchema); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); assert.isBigNumber('ethAmount', ethAmount); @@ -60,6 +68,13 @@ export class ForwarderWrapper extends ContractWrapper { assert.isBigNumber('feePercentage', feePercentage); assert.isETHAddressHex('feeRecipientAddress', feeRecipientAddress); assert.doesConformToSchema('txOpts', txOpts, txOptsSchema); + // other assertions + assert.ordersCanBeUsedForForwarderContract(signedOrders, this.getEtherTokenAddress()); + assert.feeOrdersCanBeUsedForForwarderContract( + signedFeeOrders, + this.getZRXTokenAddress(), + this.getEtherTokenAddress(), + ); const normalizedTakerAddress = takerAddress.toLowerCase(); const normalizedFeeRecipientAddress = feeRecipientAddress.toLowerCase(); const forwarderContractInstance = await this._getForwarderContractAsync(); @@ -107,6 +122,7 @@ export class ForwarderWrapper extends ContractWrapper { feeRecipientAddress: string = constants.NULL_ADDRESS, txOpts: TransactionOpts = {}, ): Promise { + // type assertions assert.doesConformToSchema('signedOrders', signedOrders, schemas.signedOrdersSchema); assert.isBigNumber('makerAssetFillAmount', makerAssetFillAmount); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); @@ -115,6 +131,13 @@ export class ForwarderWrapper extends ContractWrapper { assert.isBigNumber('feePercentage', feePercentage); assert.isETHAddressHex('feeRecipientAddress', feeRecipientAddress); assert.doesConformToSchema('txOpts', txOpts, txOptsSchema); + // other assertions + assert.ordersCanBeUsedForForwarderContract(signedOrders, this.getEtherTokenAddress()); + assert.feeOrdersCanBeUsedForForwarderContract( + signedFeeOrders, + this.getZRXTokenAddress(), + this.getEtherTokenAddress(), + ); const normalizedTakerAddress = takerAddress.toLowerCase(); const normalizedFeeRecipientAddress = feeRecipientAddress.toLowerCase(); const forwarderContractInstance = await this._getForwarderContractAsync(); @@ -144,6 +167,22 @@ export class ForwarderWrapper extends ContractWrapper { const contractAddress = this._getContractAddress(artifacts.Forwarder, this._contractAddressIfExists); return contractAddress; } + /** + * Returns the ZRX token address used by the forwarder contract. + * @return Address of ZRX token + */ + public getZRXTokenAddress(): string { + const contractAddress = this._getContractAddress(artifacts.ZRXToken, this._zrxContractAddressIfExists); + return contractAddress; + } + /** + * Returns the Ether token address used by the forwarder contract. + * @return Address of Ether token + */ + public getEtherTokenAddress(): string { + const contractAddress = this._getContractAddress(artifacts.EtherToken); + return contractAddress; + } // HACK: We don't want this method to be visible to the other units within that package but not to the end user. // TS doesn't give that possibility and therefore we make it private and access it over an any cast. Because of that tslint sees it as unused. // tslint:disable-next-line:no-unused-variable diff --git a/packages/contract-wrappers/src/utils/assert.ts b/packages/contract-wrappers/src/utils/assert.ts index 842b16fa0..183642170 100644 --- a/packages/contract-wrappers/src/utils/assert.ts +++ b/packages/contract-wrappers/src/utils/assert.ts @@ -1,11 +1,14 @@ import { assert as sharedAssert } from '@0xproject/assert'; // HACK: We need those two unused imports because they're actually used by sharedAssert which gets injected here import { Schema } from '@0xproject/json-schemas'; // tslint:disable-line:no-unused-variable -import { isValidSignatureAsync } from '@0xproject/order-utils'; -import { ECSignature } from '@0xproject/types'; // tslint:disable-line:no-unused-variable +import { assetDataUtils, isValidSignatureAsync } from '@0xproject/order-utils'; +import { ECSignature, Order } from '@0xproject/types'; // tslint:disable-line:no-unused-variable import { BigNumber } from '@0xproject/utils'; // tslint:disable-line:no-unused-variable import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { Provider } from 'ethereum-types'; +import * as _ from 'lodash'; + +import { constants } from './constants'; export const assert = { ...sharedAssert, @@ -16,12 +19,12 @@ export const assert = { signerAddress: string, ): Promise { const isValid = await isValidSignatureAsync(provider, orderHash, signature, signerAddress); - this.assert(isValid, `Expected order with hash '${orderHash}' to have a valid signature`); + sharedAssert.assert(isValid, `Expected order with hash '${orderHash}' to have a valid signature`); }, isValidSubscriptionToken(variableName: string, subscriptionToken: string): void { const uuidRegex = new RegExp('^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$'); const isValid = uuidRegex.test(subscriptionToken); - this.assert(isValid, `Expected ${variableName} to be a valid subscription token`); + sharedAssert.assert(isValid, `Expected ${variableName} to be a valid subscription token`); }, async isSenderAddressAsync( variableName: string, @@ -35,4 +38,53 @@ export const assert = { `Specified ${variableName} ${senderAddressHex} isn't available through the supplied web3 provider`, ); }, + ordersCanBeUsedForForwarderContract(orders: Order[], etherTokenAddress: string): void { + sharedAssert.assert(!_.isEmpty(orders), 'Expected at least 1 signed order. Found no orders'); + assert.ordersHaveAtMostOneUniqueValueForProperty(orders, 'makerAssetData'); + assert.allTakerAssetDatasAreErc20Token(orders, etherTokenAddress); + assert.allTakerAddressesAreNull(orders); + }, + feeOrdersCanBeUsedForForwarderContract(orders: Order[], zrxTokenAddress: string, etherTokenAddress: string): void { + if (!_.isEmpty(orders)) { + assert.allMakerAssetDatasAreErc20Token(orders, zrxTokenAddress); + assert.allTakerAssetDatasAreErc20Token(orders, etherTokenAddress); + } + }, + allTakerAddressesAreNull(orders: Order[]): void { + assert.ordersHaveAtMostOneUniqueValueForProperty(orders, 'takerAddress', constants.NULL_ADDRESS); + }, + allMakerAssetDatasAreErc20Token(orders: Order[], tokenAddress: string): void { + assert.ordersHaveAtMostOneUniqueValueForProperty( + orders, + 'makerAssetData', + assetDataUtils.encodeERC20AssetData(tokenAddress), + ); + }, + allTakerAssetDatasAreErc20Token(orders: Order[], tokenAddress: string): void { + assert.ordersHaveAtMostOneUniqueValueForProperty( + orders, + 'takerAssetData', + assetDataUtils.encodeERC20AssetData(tokenAddress), + ); + }, + /* + * Asserts that all the orders have the same value for the provided propertyName + * If the value parameter is provided, this asserts that all orders have the prope + */ + ordersHaveAtMostOneUniqueValueForProperty(orders: Order[], propertyName: string, value?: any): void { + const allValues = _.map(orders, order => _.get(order, propertyName)); + sharedAssert.hasAtMostOneUniqueValue( + allValues, + `Expected all orders to have the same ${propertyName} field. Found the following ${propertyName} values: ${JSON.stringify( + allValues, + )}`, + ); + if (!_.isUndefined(value)) { + const firstValue = _.head(allValues); + sharedAssert.assert( + firstValue === value, + `Expected all orders to have a ${propertyName} field with value: ${value}. Found: ${firstValue}`, + ); + } + }, }; -- cgit