bug(exchange-wrapper): matchOrdersAsync input param mutation

This commit is contained in:
Jacob Evans
2019-01-12 10:22:13 +11:00
parent 943c378309
commit 507c47c42c
2 changed files with 45 additions and 24 deletions

View File

@@ -5,6 +5,9 @@
{ {
"note": "Renamed OrderStatus enum members to PascalCase to conform with tslint enum-naming rule", "note": "Renamed OrderStatus enum members to PascalCase to conform with tslint enum-naming rule",
"pr": 1474 "pr": 1474
},
{
"note": "Prevent Exchange `matchOrdersAsync` input parameters being modified"
} }
] ]
}, },
@@ -70,7 +73,8 @@
"version": "4.1.0", "version": "4.1.0",
"changes": [ "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 "pr": 1292
} }
], ],
@@ -98,15 +102,18 @@
"version": "4.0.0", "version": "4.0.0",
"changes": [ "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 "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 "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 "pr": 1235
} }
], ],
@@ -137,23 +144,28 @@
"pr": 1105 "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 "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 "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 "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 "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 "pr": 1105
}, },
{ {
@@ -189,11 +201,13 @@
"version": "2.0.0", "version": "2.0.0",
"changes": [ "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 "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 "pr": 1080
} }
], ],
@@ -242,15 +256,18 @@
"note": "Add `OrderValidatorWrapper`" "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 "pr": 1044
}, },
{ {
"note": "Export `AssetBalanceAndProxyAllowanceFetcher` and `OrderFilledCancelledFetcher` implementations", "note":
"Export `AssetBalanceAndProxyAllowanceFetcher` and `OrderFilledCancelledFetcher` implementations",
"pr": 1054 "pr": 1054
}, },
{ {
"note": "Add `validateOrderFillableOrThrowAsync` and `validateFillOrderThrowIfInvalidAsync` to ExchangeWrapper", "note":
"Add `validateOrderFillableOrThrowAsync` and `validateFillOrderThrowIfInvalidAsync` to ExchangeWrapper",
"pr": 1054 "pr": 1054
} }
], ],
@@ -269,11 +286,13 @@
"version": "1.0.1-rc.4", "version": "1.0.1-rc.4",
"changes": [ "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 "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 "pr": 924
}, },
{ {

View File

@@ -743,18 +743,20 @@ export class ExchangeWrapper extends ContractWrapper {
rightSignedOrder.takerAssetData !== leftSignedOrder.makerAssetData rightSignedOrder.takerAssetData !== leftSignedOrder.makerAssetData
) { ) {
throw new Error(ExchangeWrapperError.AssetDataMismatch); 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(); const exchangeInstance = await this._getExchangeContractAsync();
if (orderTransactionOpts.shouldValidate) { if (orderTransactionOpts.shouldValidate) {
await exchangeInstance.matchOrders.callAsync( await exchangeInstance.matchOrders.callAsync(
leftSignedOrder, leftSignedOrder,
rightSignedOrder, optimizedRightSignedOrder,
leftSignedOrder.signature, leftSignedOrder.signature,
rightSignedOrder.signature, optimizedRightSignedOrder.signature,
{ {
from: normalizedTakerAddress, from: normalizedTakerAddress,
gas: orderTransactionOpts.gasLimit, gas: orderTransactionOpts.gasLimit,
@@ -765,9 +767,9 @@ export class ExchangeWrapper extends ContractWrapper {
} }
const txHash = await exchangeInstance.matchOrders.sendTransactionAsync( const txHash = await exchangeInstance.matchOrders.sendTransactionAsync(
leftSignedOrder, leftSignedOrder,
rightSignedOrder, optimizedRightSignedOrder,
leftSignedOrder.signature, leftSignedOrder.signature,
rightSignedOrder.signature, optimizedRightSignedOrder.signature,
{ {
from: normalizedTakerAddress, from: normalizedTakerAddress,
gas: orderTransactionOpts.gasLimit, gas: orderTransactionOpts.gasLimit,