Return array of FillResults in batch fill methods

contracts/exchange/CHANGELOG.json

@@ -69,6 +69,10 @@
             {
                 "note": "Log an `TransactionExecuted` event when an `executeTransaction` call is successful",
                 "pr": 1832
+            },
+            {
+                "note": "Return a FillResults array for batch fill variants",
+                "pr": 1834
             }
         ]
     },

contracts/exchange/contracts/src/MixinMatchOrders.sol

@@ -57,7 +57,8 @@ contract MixinMatchOrders is
         nonReentrant
         returns (LibFillResults.MatchedFillResults memory matchedFillResults)
     {
-        // We assume that rightOrder.takerAssetData == leftOrder.makerAssetData and rightOrder.makerAssetData == leftOrder.takerAssetData.
+        // We assume that rightOrder.takerAssetData == leftOrder.makerAssetData and rightOrder.makerAssetData == leftOrder.takerAssetData
+        // by pointing these values to the same location in memory. This is cheaper than checking equality.
         // If this assumption isn't true, the match will fail at signature validation.
         rightOrder.makerAssetData = leftOrder.takerAssetData;
         rightOrder.takerAssetData = leftOrder.makerAssetData;

contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol

@@ -55,41 +55,41 @@ contract IWrapperFunctions {
     /// @param orders Array of order specifications.
     /// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders.
     /// @param signatures Proofs that orders have been created by makers.
-    /// @return Amounts filled and fees paid by makers and taker.
+    /// @return Array of amounts filled and fees paid by makers and taker.
     function batchFillOrders(
         LibOrder.Order[] memory orders,
         uint256[] memory takerAssetFillAmounts,
         bytes[] memory signatures
     )
         public
-        returns (LibFillResults.FillResults memory totalFillResults);
+        returns (LibFillResults.FillResults[] memory fillResults);
 
     /// @dev Synchronously executes multiple calls of fillOrKill.
     /// @param orders Array of order specifications.
     /// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders.
     /// @param signatures Proofs that orders have been created by makers.
-    /// @return Amounts filled and fees paid by makers and taker.
+    /// @return Array of amounts filled and fees paid by makers and taker.
     function batchFillOrKillOrders(
         LibOrder.Order[] memory orders,
         uint256[] memory takerAssetFillAmounts,
         bytes[] memory signatures
     )
         public
-        returns (LibFillResults.FillResults memory totalFillResults);
+        returns (LibFillResults.FillResults[] memory fillResults);
 
     /// @dev Fills an order with specified parameters and ECDSA signature.
     ///      Returns false if the transaction would otherwise revert.
     /// @param orders Array of order specifications.
     /// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders.
     /// @param signatures Proofs that orders have been created by makers.
-    /// @return Amounts filled and fees paid by makers and taker.
+    /// @return Array of amounts filled and fees paid by makers and taker.
     function batchFillOrdersNoThrow(
         LibOrder.Order[] memory orders,
         uint256[] memory takerAssetFillAmounts,
         bytes[] memory signatures
     )
         public
-        returns (LibFillResults.FillResults memory totalFillResults);
+        returns (LibFillResults.FillResults[] memory fillResults);
 
     /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerAsset is sold by taker.
     /// @param orders Array of order specifications.
@@ -102,7 +102,7 @@ contract IWrapperFunctions {
         bytes[] memory signatures
     )
         public
-        returns (LibFillResults.FillResults memory totalFillResults);
+        returns (LibFillResults.FillResults memory fillResults);
 
     /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerAsset is sold by taker.
     ///      Returns false if the transaction would otherwise revert.
@@ -116,7 +116,7 @@ contract IWrapperFunctions {
         bytes[] memory signatures
     )
         public
-        returns (LibFillResults.FillResults memory totalFillResults);
+        returns (LibFillResults.FillResults memory fillResults);
 
     /// @dev Synchronously executes multiple calls of fillOrder until total amount of makerAsset is bought by taker.
     /// @param orders Array of order specifications.
@@ -129,7 +129,7 @@ contract IWrapperFunctions {
         bytes[] memory signatures
     )
         public
-        returns (LibFillResults.FillResults memory totalFillResults);
+        returns (LibFillResults.FillResults memory fillResults);
 
     /// @dev Synchronously executes multiple fill orders in a single transaction until total amount is bought by taker.
     ///      Returns false if the transaction would otherwise revert.
@@ -143,7 +143,7 @@ contract IWrapperFunctions {
         bytes[] memory signatures
     )
         public
-        returns (LibFillResults.FillResults memory totalFillResults);
+        returns (LibFillResults.FillResults memory fillResults);
 
     /// @dev Synchronously cancels multiple orders in a single transaction.
     /// @param orders Array of order specifications.

contracts/exchange/test/transactions.ts

@@ -286,11 +286,13 @@ describe('Exchange transactions', () => {
                     const abi = artifacts.Exchange.compilerOutput.abi;
                     const methodAbi = abi.filter(abiItem => (abiItem as MethodAbi).name === fnName)[0] as MethodAbi;
                     const abiEncoder = new AbiEncoder.Method(methodAbi);
+
                     const decodedReturnData = abiEncoder.decodeReturnValues(returnData);
-                    const fillResults: FillResults =
-                        decodedReturnData.fillResults === undefined
-                            ? decodedReturnData.totalFillResults
+                    const fillResults =
+                        exchangeConstants.BATCH_FILL_FN_NAMES.indexOf(fnName) !== -1
+                            ? decodedReturnData.fillResults[0]
                             : decodedReturnData.fillResults;
+
                     expect(fillResults.makerAssetFilledAmount).to.be.bignumber.eq(order.makerAssetAmount);
                     expect(fillResults.takerAssetFilledAmount).to.be.bignumber.eq(order.takerAssetAmount);
                     expect(fillResults.makerFeePaid).to.be.bignumber.eq(order.makerFee);

contracts/exchange/test/utils/exchange_wrapper.ts

@@ -1,14 +1,7 @@
 import { artifacts as erc1155Artifacts } from '@0x/contracts-erc1155';
 import { artifacts as erc20Artifacts } from '@0x/contracts-erc20';
 import { artifacts as erc721Artifacts } from '@0x/contracts-erc721';
-import {
-    FillResults,
-    formatters,
-    LogDecoder,
-    OrderInfo,
-    orderUtils,
-    Web3ProviderEngine,
-} from '@0x/contracts-test-utils';
+import { FillResults, LogDecoder, OrderInfo, orderUtils, Web3ProviderEngine } from '@0x/contracts-test-utils';
 import { SignedOrder, SignedZeroExTransaction } from '@0x/types';
 import { AbiEncoder, BigNumber } from '@0x/utils';
 import { Web3Wrapper } from '@0x/web3-wrapper';
@@ -98,11 +91,12 @@ export class ExchangeWrapper {
         from: string,
         opts: { takerAssetFillAmounts?: BigNumber[] } = {},
     ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = formatters.createBatchFill(orders, opts.takerAssetFillAmounts);
         const txHash = await this._exchange.batchFillOrders.sendTransactionAsync(
-            params.orders,
-            params.takerAssetFillAmounts,
-            params.signatures,
+            orders,
+            opts.takerAssetFillAmounts === undefined
+                ? orders.map(signedOrder => signedOrder.takerAssetAmount)
+                : opts.takerAssetFillAmounts,
+            orders.map(signedOrder => signedOrder.signature),
             { from },
         );
         const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
@@ -113,11 +107,12 @@ export class ExchangeWrapper {
         from: string,
         opts: { takerAssetFillAmounts?: BigNumber[] } = {},
     ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = formatters.createBatchFill(orders, opts.takerAssetFillAmounts);
         const txHash = await this._exchange.batchFillOrKillOrders.sendTransactionAsync(
-            params.orders,
-            params.takerAssetFillAmounts,
-            params.signatures,
+            orders,
+            opts.takerAssetFillAmounts === undefined
+                ? orders.map(signedOrder => signedOrder.takerAssetAmount)
+                : opts.takerAssetFillAmounts,
+            orders.map(signedOrder => signedOrder.signature),
             { from },
         );
         const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
@@ -128,11 +123,12 @@ export class ExchangeWrapper {
         from: string,
         opts: { takerAssetFillAmounts?: BigNumber[]; gas?: number } = {},
     ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = formatters.createBatchFill(orders, opts.takerAssetFillAmounts);
         const txHash = await this._exchange.batchFillOrdersNoThrow.sendTransactionAsync(
-            params.orders,
-            params.takerAssetFillAmounts,
-            params.signatures,
+            orders,
+            opts.takerAssetFillAmounts === undefined
+                ? orders.map(signedOrder => signedOrder.takerAssetAmount)
+                : opts.takerAssetFillAmounts,
+            orders.map(signedOrder => signedOrder.signature),
             { from, gas: opts.gas },
         );
         const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
@@ -143,11 +139,10 @@ export class ExchangeWrapper {
         from: string,
         opts: { takerAssetFillAmount: BigNumber },
     ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = formatters.createMarketSellOrders(orders, opts.takerAssetFillAmount);
         const txHash = await this._exchange.marketSellOrders.sendTransactionAsync(
-            params.orders,
-            params.takerAssetFillAmount,
-            params.signatures,
+            orders,
+            opts.takerAssetFillAmount,
+            orders.map(signedOrder => signedOrder.signature),
             { from },
         );
         const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
@@ -158,11 +153,10 @@ export class ExchangeWrapper {
         from: string,
         opts: { takerAssetFillAmount: BigNumber; gas?: number },
     ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = formatters.createMarketSellOrders(orders, opts.takerAssetFillAmount);
         const txHash = await this._exchange.marketSellOrdersNoThrow.sendTransactionAsync(
-            params.orders,
-            params.takerAssetFillAmount,
-            params.signatures,
+            orders,
+            opts.takerAssetFillAmount,
+            orders.map(signedOrder => signedOrder.signature),
             { from, gas: opts.gas },
         );
         const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
@@ -173,11 +167,10 @@ export class ExchangeWrapper {
         from: string,
         opts: { makerAssetFillAmount: BigNumber },
     ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = formatters.createMarketBuyOrders(orders, opts.makerAssetFillAmount);
         const txHash = await this._exchange.marketBuyOrders.sendTransactionAsync(
-            params.orders,
-            params.makerAssetFillAmount,
-            params.signatures,
+            orders,
+            opts.makerAssetFillAmount,
+            orders.map(signedOrder => signedOrder.signature),
             { from },
         );
         const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
@@ -188,11 +181,10 @@ export class ExchangeWrapper {
         from: string,
         opts: { makerAssetFillAmount: BigNumber; gas?: number },
     ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = formatters.createMarketBuyOrders(orders, opts.makerAssetFillAmount);
         const txHash = await this._exchange.marketBuyOrdersNoThrow.sendTransactionAsync(
-            params.orders,
-            params.makerAssetFillAmount,
-            params.signatures,
+            orders,
+            opts.makerAssetFillAmount,
+            orders.map(signedOrder => signedOrder.signature),
             { from, gas: opts.gas },
         );
         const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
@@ -202,17 +194,15 @@ export class ExchangeWrapper {
         orders: SignedOrder[],
         from: string,
     ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = formatters.createBatchCancel(orders);
-        const txHash = await this._exchange.batchCancelOrders.sendTransactionAsync(params.orders, { from });
+        const txHash = await this._exchange.batchCancelOrders.sendTransactionAsync(orders, { from });
         const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
         return tx;
     }
     public async batchCancelOrdersNoThrowAsync(
         orders: SignedOrder[],
         from: string,
     ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = formatters.createBatchCancel(orders);
-        const txHash = await this._exchange.batchCancelOrdersNoThrow.sendTransactionAsync(params.orders, { from });
+        const txHash = await this._exchange.batchCancelOrdersNoThrow.sendTransactionAsync(orders, { from });
         const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
         return tx;
     }