From 22abd1dfcf0ca1f7566c6ab5e0392097cf973bff Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Fri, 12 Oct 2018 13:16:36 -0700 Subject: feat(contract-wrappers): add optional validation to the forwarder wrapper Similar to the approach taken in exchange wrapper, make a call to an rpc node in order to simulate the transaction before actually sending the transaction. The decorator will parse revert reasons and other types of errors into canonical errors that a consumer of the library expects when interacting with a contract wrapper. --- packages/contract-wrappers/CHANGELOG.json | 8 ++ .../src/contract_wrappers/forwarder_wrapper.ts | 122 ++++++++++++++------- .../test/forwarder_wrapper_test.ts | 48 ++++++++ 3 files changed, 140 insertions(+), 38 deletions(-) diff --git a/packages/contract-wrappers/CHANGELOG.json b/packages/contract-wrappers/CHANGELOG.json index a96cb3a59..0770b6c0d 100644 --- a/packages/contract-wrappers/CHANGELOG.json +++ b/packages/contract-wrappers/CHANGELOG.json @@ -1,4 +1,12 @@ [ + { + "version": "2.1.0", + "changes": [ + { + "note": "Add optional validation to the forwarder wrapper methods" + } + ] + }, { "version": "2.0.2", "changes": [ diff --git a/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts index 906222731..c19edf188 100644 --- a/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts +++ b/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts @@ -8,10 +8,11 @@ import * as _ from 'lodash'; import { artifacts } from '../artifacts'; import { orderTxOptsSchema } from '../schemas/order_tx_opts_schema'; import { txOptsSchema } from '../schemas/tx_opts_schema'; -import { TransactionOpts } from '../types'; +import { OrderTransactionOpts } from '../types'; import { assert } from '../utils/assert'; import { calldataOptimizationUtils } from '../utils/calldata_optimization_utils'; import { constants } from '../utils/constants'; +import { decorators } from '../utils/decorators'; import { utils } from '../utils/utils'; import { ContractWrapper } from './contract_wrapper'; @@ -40,19 +41,20 @@ export class ForwarderWrapper extends ContractWrapper { * Any ZRX required to pay fees for primary orders will automatically be purchased by this contract. * 5% of ETH value is reserved for paying fees to order feeRecipients (in ZRX) and forwarding contract feeRecipient (in ETH). * Any ETH not spent will be refunded to sender. - * @param signedOrders An array of objects that conform to the SignedOrder interface. All orders must specify the same makerAsset. - * All orders must specify WETH as the takerAsset - * @param takerAddress The user Ethereum address who would like to fill this order. Must be available via the supplied - * Provider provided at instantiation. - * @param ethAmount The amount of eth to send with the transaction (in wei). - * @param signedFeeOrders An array of objects that conform to the SignedOrder interface. All orders must specify ZRX as makerAsset and WETH as takerAsset. - * Used to purchase ZRX for primary order fees. - * @param feePercentage The percentage of WETH sold that will payed as fee to forwarding contract feeRecipient. - * Defaults to 0. - * @param feeRecipientAddress The address that will receive ETH when signedFeeOrders are filled. - * @param txOpts Transaction parameters. + * @param signedOrders An array of objects that conform to the SignedOrder interface. All orders must specify the same makerAsset. + * All orders must specify WETH as the takerAsset + * @param takerAddress The user Ethereum address who would like to fill this order. Must be available via the supplied + * Provider provided at instantiation. + * @param ethAmount The amount of eth to send with the transaction (in wei). + * @param signedFeeOrders An array of objects that conform to the SignedOrder interface. All orders must specify ZRX as makerAsset and WETH as takerAsset. + * Used to purchase ZRX for primary order fees. + * @param feePercentage The percentage of WETH sold that will payed as fee to forwarding contract feeRecipient. + * Defaults to 0. + * @param feeRecipientAddress The address that will receive ETH when signedFeeOrders are filled. + * @param orderTransactionOpts Transaction parameters. * @return Transaction hash. */ + @decorators.asyncZeroExErrorHandler public async marketSellOrdersWithEthAsync( signedOrders: SignedOrder[], takerAddress: string, @@ -60,7 +62,7 @@ export class ForwarderWrapper extends ContractWrapper { signedFeeOrders: SignedOrder[] = [], feePercentage: number = 0, feeRecipientAddress: string = constants.NULL_ADDRESS, - txOpts: TransactionOpts = {}, + orderTransactionOpts: OrderTransactionOpts = { shouldValidate: true }, ): Promise { // type assertions assert.doesConformToSchema('signedOrders', signedOrders, schemas.signedOrdersSchema); @@ -69,7 +71,7 @@ export class ForwarderWrapper extends ContractWrapper { assert.doesConformToSchema('signedFeeOrders', signedFeeOrders, schemas.signedOrdersSchema); assert.isNumber('feePercentage', feePercentage); assert.isETHAddressHex('feeRecipientAddress', feeRecipientAddress); - assert.doesConformToSchema('txOpts', txOpts, txOptsSchema); + assert.doesConformToSchema('orderTransactionOpts', orderTransactionOpts, orderTxOptsSchema, [txOptsSchema]); // other assertions assert.ordersCanBeUsedForForwarderContract(signedOrders, this.getEtherTokenAddress()); assert.feeOrdersCanBeUsedForForwarderContract( @@ -85,20 +87,41 @@ export class ForwarderWrapper extends ContractWrapper { // optimize orders const optimizedMarketOrders = calldataOptimizationUtils.optimizeForwarderOrders(signedOrders); const optimizedFeeOrders = calldataOptimizationUtils.optimizeForwarderFeeOrders(signedFeeOrders); - // send transaction + // compile signatures + const signatures = _.map(optimizedMarketOrders, order => order.signature); + const feeSignatures = _.map(optimizedFeeOrders, order => order.signature); + // get contract const forwarderContractInstance = await this._getForwarderContractAsync(); + // validate transaction + if (orderTransactionOpts.shouldValidate) { + await forwarderContractInstance.marketSellOrdersWithEth.callAsync( + optimizedMarketOrders, + signatures, + optimizedFeeOrders, + feeSignatures, + formattedFeePercentage, + feeRecipientAddress, + { + value: ethAmount, + from: normalizedTakerAddress, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, + }, + ); + } + // send transaction const txHash = await forwarderContractInstance.marketSellOrdersWithEth.sendTransactionAsync( optimizedMarketOrders, - _.map(optimizedMarketOrders, order => order.signature), + signatures, optimizedFeeOrders, - _.map(optimizedFeeOrders, order => order.signature), + feeSignatures, formattedFeePercentage, feeRecipientAddress, { value: ethAmount, from: normalizedTakerAddress, - gas: txOpts.gasLimit, - gasPrice: txOpts.gasPrice, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, }, ); return txHash; @@ -107,20 +130,21 @@ export class ForwarderWrapper extends ContractWrapper { * Attempt to purchase makerAssetFillAmount of makerAsset by selling ethAmount provided with transaction. * Any ZRX required to pay fees for primary orders will automatically be purchased by the contract. * Any ETH not spent will be refunded to sender. - * @param signedOrders An array of objects that conform to the SignedOrder interface. All orders must specify the same makerAsset. - * All orders must specify WETH as the takerAsset - * @param makerAssetFillAmount The amount of the order (in taker asset baseUnits) that you wish to fill. - * @param takerAddress The user Ethereum address who would like to fill this order. Must be available via the supplied - * Provider provided at instantiation. - * @param ethAmount The amount of eth to send with the transaction (in wei). - * @param signedFeeOrders An array of objects that conform to the SignedOrder interface. All orders must specify ZRX as makerAsset and WETH as takerAsset. - * Used to purchase ZRX for primary order fees. - * @param feePercentage The percentage of WETH sold that will payed as fee to forwarding contract feeRecipient. - * Defaults to 0. - * @param feeRecipientAddress The address that will receive ETH when signedFeeOrders are filled. - * @param txOpts Transaction parameters. + * @param signedOrders An array of objects that conform to the SignedOrder interface. All orders must specify the same makerAsset. + * All orders must specify WETH as the takerAsset + * @param makerAssetFillAmount The amount of the order (in taker asset baseUnits) that you wish to fill. + * @param takerAddress The user Ethereum address who would like to fill this order. Must be available via the supplied + * Provider provided at instantiation. + * @param ethAmount The amount of eth to send with the transaction (in wei). + * @param signedFeeOrders An array of objects that conform to the SignedOrder interface. All orders must specify ZRX as makerAsset and WETH as takerAsset. + * Used to purchase ZRX for primary order fees. + * @param feePercentage The percentage of WETH sold that will payed as fee to forwarding contract feeRecipient. + * Defaults to 0. + * @param feeRecipientAddress The address that will receive ETH when signedFeeOrders are filled. + * @param orderTransactionOpts Transaction parameters. * @return Transaction hash. */ + @decorators.asyncZeroExErrorHandler public async marketBuyOrdersWithEthAsync( signedOrders: SignedOrder[], makerAssetFillAmount: BigNumber, @@ -129,7 +153,7 @@ export class ForwarderWrapper extends ContractWrapper { signedFeeOrders: SignedOrder[] = [], feePercentage: number = 0, feeRecipientAddress: string = constants.NULL_ADDRESS, - txOpts: TransactionOpts = {}, + orderTransactionOpts: OrderTransactionOpts = { shouldValidate: true }, ): Promise { // type assertions assert.doesConformToSchema('signedOrders', signedOrders, schemas.signedOrdersSchema); @@ -139,7 +163,7 @@ export class ForwarderWrapper extends ContractWrapper { assert.doesConformToSchema('signedFeeOrders', signedFeeOrders, schemas.signedOrdersSchema); assert.isNumber('feePercentage', feePercentage); assert.isETHAddressHex('feeRecipientAddress', feeRecipientAddress); - assert.doesConformToSchema('txOpts', txOpts, txOptsSchema); + assert.doesConformToSchema('orderTransactionOpts', orderTransactionOpts, orderTxOptsSchema, [txOptsSchema]); // other assertions assert.ordersCanBeUsedForForwarderContract(signedOrders, this.getEtherTokenAddress()); assert.feeOrdersCanBeUsedForForwarderContract( @@ -155,21 +179,43 @@ export class ForwarderWrapper extends ContractWrapper { // optimize orders const optimizedMarketOrders = calldataOptimizationUtils.optimizeForwarderOrders(signedOrders); const optimizedFeeOrders = calldataOptimizationUtils.optimizeForwarderFeeOrders(signedFeeOrders); - // send transaction + // compile signatures + const signatures = _.map(optimizedMarketOrders, order => order.signature); + const feeSignatures = _.map(optimizedFeeOrders, order => order.signature); + // get contract const forwarderContractInstance = await this._getForwarderContractAsync(); + // validate transaction + if (orderTransactionOpts.shouldValidate) { + await forwarderContractInstance.marketBuyOrdersWithEth.callAsync( + optimizedMarketOrders, + makerAssetFillAmount, + signatures, + optimizedFeeOrders, + feeSignatures, + formattedFeePercentage, + feeRecipientAddress, + { + value: ethAmount, + from: normalizedTakerAddress, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, + }, + ); + } + // send transaction const txHash = await forwarderContractInstance.marketBuyOrdersWithEth.sendTransactionAsync( optimizedMarketOrders, makerAssetFillAmount, - _.map(optimizedMarketOrders, order => order.signature), + signatures, optimizedFeeOrders, - _.map(optimizedFeeOrders, order => order.signature), + feeSignatures, formattedFeePercentage, feeRecipientAddress, { value: ethAmount, from: normalizedTakerAddress, - gas: txOpts.gasLimit, - gasPrice: txOpts.gasPrice, + gas: orderTransactionOpts.gasLimit, + gasPrice: orderTransactionOpts.gasPrice, }, ); return txHash; diff --git a/packages/contract-wrappers/test/forwarder_wrapper_test.ts b/packages/contract-wrappers/test/forwarder_wrapper_test.ts index f77b47337..4329e8770 100644 --- a/packages/contract-wrappers/test/forwarder_wrapper_test.ts +++ b/packages/contract-wrappers/test/forwarder_wrapper_test.ts @@ -17,6 +17,7 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); +// tslint:disable:custom-no-magic-numbers describe('ForwarderWrapper', () => { const contractWrappersConfig = { networkId: constants.TESTRPC_NETWORK_ID, @@ -99,6 +100,25 @@ describe('ForwarderWrapper', () => { expect(ordersInfo[0].orderStatus).to.be.equal(OrderStatus.FULLY_FILLED); expect(ordersInfo[1].orderStatus).to.be.equal(OrderStatus.FULLY_FILLED); }); + it('should throw when invalid transaction and shouldValidate is true', async () => { + const signedOrders = [signedOrder]; + // request more makerAsset than what is available + const makerAssetFillAmount = signedOrder.makerAssetAmount.plus(100); + return expect( + contractWrappers.forwarder.marketBuyOrdersWithEthAsync( + signedOrders, + makerAssetFillAmount, + takerAddress, + makerAssetFillAmount, + [], + 0, + constants.NULL_ADDRESS, + { + shouldValidate: true, + }, + ), + ).to.be.rejectedWith('COMPLETE_FILL_FAILED'); + }); }); describe('#marketSellOrdersWithEthAsync', () => { it('should market sell orders with eth', async () => { @@ -115,5 +135,33 @@ describe('ForwarderWrapper', () => { expect(ordersInfo[1].orderStatus).to.be.equal(OrderStatus.FILLABLE); expect(ordersInfo[1].orderTakerAssetFilledAmount).to.be.bignumber.equal(new BigNumber(4)); // only 95% of ETH is sold }); + it('should throw when invalid transaction and shouldValidate is true', async () => { + // create an order with fees, we try to fill it but we do not provide enough ETH to cover the fees + const signedOrderWithFee = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerAssetData, + takerAssetData, + constants.ZERO_AMOUNT, + new BigNumber(100), + makerAddress, + constants.NULL_ADDRESS, + fillableAmount, + constants.NULL_ADDRESS, + ); + const signedOrders = [signedOrderWithFee]; + const makerAssetFillAmount = signedOrder.makerAssetAmount; + return expect( + contractWrappers.forwarder.marketSellOrdersWithEthAsync( + signedOrders, + takerAddress, + makerAssetFillAmount, + [], + 0, + constants.NULL_ADDRESS, + { + shouldValidate: true, + }, + ), + ).to.be.rejectedWith('COMPLETE_FILL_FAILED'); + }); }); }); -- cgit v1.2.3