bug(exchange-wrapper): matchOrdersAsync input param mutation

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,