From 507c47c42cdc0cd2164b22bf84d14bfd2e59b1ea Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Sat, 12 Jan 2019 10:22:13 +1100 Subject: bug(exchange-wrapper): matchOrdersAsync input param mutation --- packages/contract-wrappers/CHANGELOG.json | 51 +++++++++++++++------- .../src/contract_wrappers/exchange_wrapper.ts | 18 ++++---- 2 files changed, 45 insertions(+), 24 deletions(-) diff --git a/packages/contract-wrappers/CHANGELOG.json b/packages/contract-wrappers/CHANGELOG.json index 9a225726a..c06f55465 100644 --- a/packages/contract-wrappers/CHANGELOG.json +++ b/packages/contract-wrappers/CHANGELOG.json @@ -5,6 +5,9 @@ { "note": "Renamed OrderStatus enum members to PascalCase to conform with tslint enum-naming rule", "pr": 1474 + }, + { + "note": "Prevent Exchange `matchOrdersAsync` input parameters being modified" } ] }, @@ -70,7 +73,8 @@ "version": "4.1.0", "changes": [ { - "note": "Add a `nonce` field for `TxOpts` so that it's now possible to re-broadcast stuck transactions with a higher gas amount", + "note": + "Add a `nonce` field for `TxOpts` so that it's now possible to re-broadcast stuck transactions with a higher gas amount", "pr": 1292 } ], @@ -98,15 +102,18 @@ "version": "4.0.0", "changes": [ { - "note": "Add signature validation, regular cancellation and `cancelledUpTo` checks to `validateOrderFillableOrThrowAsync`", + "note": + "Add signature validation, regular cancellation and `cancelledUpTo` checks to `validateOrderFillableOrThrowAsync`", "pr": 1235 }, { - "note": "Improved the errors thrown by `validateOrderFillableOrThrowAsync` by making them more descriptive", + "note": + "Improved the errors thrown by `validateOrderFillableOrThrowAsync` by making them more descriptive", "pr": 1235 }, { - "note": "Throw previously swallowed network errors when calling `validateOrderFillableOrThrowAsync` (see issue: #1218)", + "note": + "Throw previously swallowed network errors when calling `validateOrderFillableOrThrowAsync` (see issue: #1218)", "pr": 1235 } ], @@ -137,23 +144,28 @@ "pr": 1105 }, { - "note": "Default contract addresses are no longer stored in artifacts and are instead loaded from the `@0xproject/contract-addresses` package.", + "note": + "Default contract addresses are no longer stored in artifacts and are instead loaded from the `@0xproject/contract-addresses` package.", "pr": 1105 }, { - "note": "Most contract addresses are now defined at instantiation time and are available as properties (e.g., `exchangeWrapper.address`) instead of methods (e.g., `exchangeWrapper.getContractAddress()`).", + "note": + "Most contract addresses are now defined at instantiation time and are available as properties (e.g., `exchangeWrapper.address`) instead of methods (e.g., `exchangeWrapper.getContractAddress()`).", "pr": 1105 }, { - "note": "Removed `setProvider` method in top-level `ContractWrapper` class and added new `unsubscribeAll` method.", + "note": + "Removed `setProvider` method in top-level `ContractWrapper` class and added new `unsubscribeAll` method.", "pr": 1105 }, { - "note": "Some properties and methods have been renamed. For example, some methods that previously could throw no longer can, and so their names have been updated accordingly.", + "note": + "Some properties and methods have been renamed. For example, some methods that previously could throw no longer can, and so their names have been updated accordingly.", "pr": 1105 }, { - "note": "Removed ContractNotFound errors. Checking for this error was somewhat ineffecient. Relevant methods/functions now return the default error from web3-wrapper, which we feel provides enough information.", + "note": + "Removed ContractNotFound errors. Checking for this error was somewhat ineffecient. Relevant methods/functions now return the default error from web3-wrapper, which we feel provides enough information.", "pr": 1105 }, { @@ -189,11 +201,13 @@ "version": "2.0.0", "changes": [ { - "note": "Fixes dropped events in subscriptions by fetching logs by blockHash instead of blockNumber. Support for fetching by blockHash was added in Geth > v1.8.13 and Parity > v2.1.0. Infura works too.", + "note": + "Fixes dropped events in subscriptions by fetching logs by blockHash instead of blockNumber. Support for fetching by blockHash was added in Geth > v1.8.13 and Parity > v2.1.0. Infura works too.", "pr": 1080 }, { - "note": "Fix misunderstanding about blockstream interface callbacks and pass the raw JSON RPC responses to it", + "note": + "Fix misunderstanding about blockstream interface callbacks and pass the raw JSON RPC responses to it", "pr": 1080 } ], @@ -242,15 +256,18 @@ "note": "Add `OrderValidatorWrapper`" }, { - "note": "Fix bug where contracts not deployed on a network showed an `EXCHANGE_CONTRACT_DOES_NOT_EXIST` error instead of `CONTRACT_NOT_DEPLOYED_ON_NETWORK`", + "note": + "Fix bug where contracts not deployed on a network showed an `EXCHANGE_CONTRACT_DOES_NOT_EXIST` error instead of `CONTRACT_NOT_DEPLOYED_ON_NETWORK`", "pr": 1044 }, { - "note": "Export `AssetBalanceAndProxyAllowanceFetcher` and `OrderFilledCancelledFetcher` implementations", + "note": + "Export `AssetBalanceAndProxyAllowanceFetcher` and `OrderFilledCancelledFetcher` implementations", "pr": 1054 }, { - "note": "Add `validateOrderFillableOrThrowAsync` and `validateFillOrderThrowIfInvalidAsync` to ExchangeWrapper", + "note": + "Add `validateOrderFillableOrThrowAsync` and `validateFillOrderThrowIfInvalidAsync` to ExchangeWrapper", "pr": 1054 } ], @@ -269,11 +286,13 @@ "version": "1.0.1-rc.4", "changes": [ { - "note": "Export missing types: `TransactionEncoder`, `ContractAbi`, `JSONRPCRequestPayload`, `JSONRPCResponsePayload`, `JSONRPCErrorCallback`, `AbiDefinition`, `FunctionAbi`, `EventAbi`, `EventParameter`, `DecodedLogArgs`, `MethodAbi`, `ConstructorAbi`, `FallbackAbi`, `DataItem`, `ConstructorStateMutability`, `StateMutability` & `ExchangeSignatureValidatorApprovalEventArgs`", + "note": + "Export missing types: `TransactionEncoder`, `ContractAbi`, `JSONRPCRequestPayload`, `JSONRPCResponsePayload`, `JSONRPCErrorCallback`, `AbiDefinition`, `FunctionAbi`, `EventAbi`, `EventParameter`, `DecodedLogArgs`, `MethodAbi`, `ConstructorAbi`, `FallbackAbi`, `DataItem`, `ConstructorStateMutability`, `StateMutability` & `ExchangeSignatureValidatorApprovalEventArgs`", "pr": 924 }, { - "note": "Remove superfluous exported types: `ContractEvent`, `Token`, `OrderFillRequest`, `ContractEventArgs`, `LogEvent`, `OnOrderStateChangeCallback`, `ECSignature`, `OrderStateValid`, `OrderStateInvalid`, `OrderState`, `FilterObject`, `TransactionReceipt` & `TransactionReceiptWithDecodedLogs`", + "note": + "Remove superfluous exported types: `ContractEvent`, `Token`, `OrderFillRequest`, `ContractEventArgs`, `LogEvent`, `OnOrderStateChangeCallback`, `ECSignature`, `OrderStateValid`, `OrderStateInvalid`, `OrderState`, `FilterObject`, `TransactionReceipt` & `TransactionReceiptWithDecodedLogs`", "pr": 924 }, { diff --git a/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts index c9556971a..e61f5bbdc 100644 --- a/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts +++ b/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts @@ -743,18 +743,20 @@ export class ExchangeWrapper extends ContractWrapper { rightSignedOrder.takerAssetData !== leftSignedOrder.makerAssetData ) { throw new Error(ExchangeWrapperError.AssetDataMismatch); - } else { - // Smart contracts assigns the asset data from the left order to the right one so we can save gas on reducing the size of call data - rightSignedOrder.makerAssetData = '0x'; - rightSignedOrder.takerAssetData = '0x'; } + // Smart contracts assigns the asset data from the left order to the right one so we can save gas on reducing the size of call data + const optimizedRightSignedOrder = { + ...rightSignedOrder, + makerAssetData: '0x', + takerAssetData: '0x', + }; const exchangeInstance = await this._getExchangeContractAsync(); if (orderTransactionOpts.shouldValidate) { await exchangeInstance.matchOrders.callAsync( leftSignedOrder, - rightSignedOrder, + optimizedRightSignedOrder, leftSignedOrder.signature, - rightSignedOrder.signature, + optimizedRightSignedOrder.signature, { from: normalizedTakerAddress, gas: orderTransactionOpts.gasLimit, @@ -765,9 +767,9 @@ export class ExchangeWrapper extends ContractWrapper { } const txHash = await exchangeInstance.matchOrders.sendTransactionAsync( leftSignedOrder, - rightSignedOrder, + optimizedRightSignedOrder, leftSignedOrder.signature, - rightSignedOrder.signature, + optimizedRightSignedOrder.signature, { from: normalizedTakerAddress, gas: orderTransactionOpts.gasLimit, -- cgit v1.2.3