diff --git a/contracts/dev-utils/contracts/src/LibTransactionDecoder.sol b/contracts/dev-utils/contracts/src/LibTransactionDecoder.sol index df5158e9ee..476cf1c4c5 100644 --- a/contracts/dev-utils/contracts/src/LibTransactionDecoder.sol +++ b/contracts/dev-utils/contracts/src/LibTransactionDecoder.sol @@ -48,6 +48,8 @@ contract LibTransactionDecoder { if (functionSelector == IExchange(address(0)).batchCancelOrders.selector) { functionName = "batchCancelOrders"; + } else if (functionSelector == IExchange(address(0)).batchFillOrders.selector) { + functionName = "batchFillOrders"; } else if (functionSelector == IExchange(address(0)).batchFillOrdersNoThrow.selector) { functionName = "batchFillOrdersNoThrow"; } else if (functionSelector == IExchange(address(0)).batchFillOrKillOrders.selector) { @@ -84,6 +86,7 @@ contract LibTransactionDecoder { signatures = new bytes[](0); } else if ( functionSelector == IExchange(address(0)).batchFillOrKillOrders.selector || + functionSelector == IExchange(address(0)).batchFillOrders.selector || functionSelector == IExchange(address(0)).batchFillOrdersNoThrow.selector ) { (orders, takerAssetFillAmounts, signatures) = _makeReturnValuesForBatchFill(transactionData); diff --git a/contracts/dev-utils/test/lib_transaction_decoder.ts b/contracts/dev-utils/test/lib_transaction_decoder.ts index bdf93a1306..7c4ea38894 100644 --- a/contracts/dev-utils/test/lib_transaction_decoder.ts +++ b/contracts/dev-utils/test/lib_transaction_decoder.ts @@ -56,7 +56,7 @@ describe('LibTransactionDecoder', () => { ]); }); - for (const func of ['batchFillOrdersNoThrow', 'batchFillOrKillOrders']) { + for (const func of ['batchFillOrders', 'batchFillOrdersNoThrow', 'batchFillOrKillOrders']) { const input = (exchangeInterface as any)[func].getABIEncodedTransactionData( [order, order], [takerAssetFillAmount, takerAssetFillAmount], diff --git a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol index fac761f945..8df4fbc847 100644 --- a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol +++ b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol @@ -59,6 +59,34 @@ contract MixinWrapperFunctions is return fillResults; } + /// @dev Executes multiple calls of fillOrder. + /// @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 Array of amounts filled and fees paid by makers and taker. + function batchFillOrders( + LibOrder.Order[] memory orders, + uint256[] memory takerAssetFillAmounts, + bytes[] memory signatures + ) + public + payable + nonReentrant + refundFinalBalance + returns (LibFillResults.FillResults[] memory fillResults) + { + uint256 ordersLength = orders.length; + fillResults = new LibFillResults.FillResults[](ordersLength); + for (uint256 i = 0; i != ordersLength; i++) { + fillResults[i] = _fillOrder( + orders[i], + takerAssetFillAmounts[i], + signatures[i] + ); + } + return fillResults; + } + /// @dev Executes multiple calls of fillOrKill. /// @param orders Array of order specifications. /// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders. diff --git a/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol b/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol index 980c165528..28d35236b4 100644 --- a/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol +++ b/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol @@ -38,6 +38,20 @@ contract IWrapperFunctions { payable returns (LibFillResults.FillResults memory fillResults); + /// @dev Executes multiple calls of fillOrder. + /// @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 Array of amounts filled and fees paid by makers and taker. + function batchFillOrders( + LibOrder.Order[] memory orders, + uint256[] memory takerAssetFillAmounts, + bytes[] memory signatures + ) + public + payable + 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. diff --git a/contracts/exchange/contracts/test/TestProtocolFeesReceiver.sol b/contracts/exchange/contracts/test/TestProtocolFeesReceiver.sol index 97cae324e5..a908f4670f 100644 --- a/contracts/exchange/contracts/test/TestProtocolFeesReceiver.sol +++ b/contracts/exchange/contracts/test/TestProtocolFeesReceiver.sol @@ -84,7 +84,7 @@ contract TestProtocolFeesReceiver { } // Forward all of the value sent to the contract to `batchFillOrders()`. - testProtocolFees.batchFillOrKillOrders.value(msg.value)(orders, takerAssetFilledAmounts, signatures); + testProtocolFees.batchFillOrders.value(msg.value)(orders, takerAssetFilledAmounts, signatures); // If the `protocolFeeCollector` was set, ensure that the protocol fees were paid correctly. // Otherwise, the protocol fees should not have been paid. diff --git a/contracts/exchange/test/utils/constants.ts b/contracts/exchange/test/utils/constants.ts index 78c1f7c1a9..b060b83d18 100644 --- a/contracts/exchange/test/utils/constants.ts +++ b/contracts/exchange/test/utils/constants.ts @@ -13,7 +13,11 @@ export const constants = { ExchangeFunctionName.SetProtocolFeeCollectorAddress, ], SINGLE_FILL_FN_NAMES: [ExchangeFunctionName.FillOrder, ExchangeFunctionName.FillOrKillOrder], - BATCH_FILL_FN_NAMES: [ExchangeFunctionName.BatchFillOrKillOrders, ExchangeFunctionName.BatchFillOrdersNoThrow], + BATCH_FILL_FN_NAMES: [ + ExchangeFunctionName.BatchFillOrders, + ExchangeFunctionName.BatchFillOrKillOrders, + ExchangeFunctionName.BatchFillOrdersNoThrow, + ], MARKET_FILL_FN_NAMES: [ ExchangeFunctionName.MarketBuyOrdersFillOrKill, ExchangeFunctionName.MarketSellOrdersFillOrKill, diff --git a/contracts/exchange/test/utils/exchange_wrapper.ts b/contracts/exchange/test/utils/exchange_wrapper.ts index 7f5b8939ec..69b64da172 100644 --- a/contracts/exchange/test/utils/exchange_wrapper.ts +++ b/contracts/exchange/test/utils/exchange_wrapper.ts @@ -54,6 +54,20 @@ export class ExchangeWrapper { ); return txReceipt; } + public async batchFillOrdersAsync( + orders: SignedOrder[], + from: string, + opts: { takerAssetFillAmounts?: BigNumber[]; gasPrice?: BigNumber } = {}, + ): Promise { + return this._exchange.batchFillOrders.awaitTransactionSuccessAsync( + orders, + opts.takerAssetFillAmounts === undefined + ? orders.map(signedOrder => signedOrder.takerAssetAmount) + : opts.takerAssetFillAmounts, + orders.map(signedOrder => signedOrder.signature), + { from, gasPrice: opts.gasPrice }, + ); + } public async batchFillOrKillOrdersAsync( orders: SignedOrder[], from: string, diff --git a/contracts/exchange/test/wrapper.ts b/contracts/exchange/test/wrapper.ts index 47456c08dd..399186ad50 100644 --- a/contracts/exchange/test/wrapper.ts +++ b/contracts/exchange/test/wrapper.ts @@ -216,6 +216,74 @@ blockchainTests.resets('Exchange wrappers', env => { ]; }); + describe('batchFillOrders', () => { + it('should transfer the correct amounts', async () => { + const makerAssetAddress = erc20TokenA.address; + const takerAssetAddress = erc20TokenB.address; + + const takerAssetFillAmounts: BigNumber[] = []; + const expectedFillResults: FillResults[] = []; + + _.forEach(signedOrders, signedOrder => { + const takerAssetFillAmount = signedOrder.takerAssetAmount.div(2); + const makerAssetFilledAmount = takerAssetFillAmount + .times(signedOrder.makerAssetAmount) + .dividedToIntegerBy(signedOrder.takerAssetAmount); + const makerFee = signedOrder.makerFee + .times(makerAssetFilledAmount) + .dividedToIntegerBy(signedOrder.makerAssetAmount); + const takerFee = signedOrder.takerFee + .times(makerAssetFilledAmount) + .dividedToIntegerBy(signedOrder.makerAssetAmount); + + takerAssetFillAmounts.push(takerAssetFillAmount); + expectedFillResults.push({ + takerAssetFilledAmount: takerAssetFillAmount, + makerAssetFilledAmount, + makerFeePaid: makerFee, + takerFeePaid: takerFee, + protocolFeePaid: constants.ZERO_AMOUNT, + }); + + erc20Balances[makerAddress][makerAssetAddress] = erc20Balances[makerAddress][ + makerAssetAddress + ].minus(makerAssetFilledAmount); + erc20Balances[makerAddress][takerAssetAddress] = erc20Balances[makerAddress][ + takerAssetAddress + ].plus(takerAssetFillAmount); + erc20Balances[makerAddress][feeToken.address] = erc20Balances[makerAddress][feeToken.address].minus( + makerFee, + ); + erc20Balances[takerAddress][makerAssetAddress] = erc20Balances[takerAddress][ + makerAssetAddress + ].plus(makerAssetFilledAmount); + erc20Balances[takerAddress][takerAssetAddress] = erc20Balances[takerAddress][ + takerAssetAddress + ].minus(takerAssetFillAmount); + erc20Balances[takerAddress][feeToken.address] = erc20Balances[takerAddress][feeToken.address].minus( + takerFee, + ); + erc20Balances[feeRecipientAddress][feeToken.address] = erc20Balances[feeRecipientAddress][ + feeToken.address + ].plus(makerFee.plus(takerFee)); + }); + + const fillResults = await exchange.batchFillOrders.callAsync( + signedOrders, + takerAssetFillAmounts, + signedOrders.map(signedOrder => signedOrder.signature), + { from: takerAddress }, + ); + await exchangeWrapper.batchFillOrdersAsync(signedOrders, takerAddress, { + takerAssetFillAmounts, + }); + const newBalances = await erc20Wrapper.getBalancesAsync(); + + expect(fillResults).to.deep.equal(expectedFillResults); + expect(newBalances).to.be.deep.equal(erc20Balances); + }); + }); + describe('batchFillOrKillOrders', () => { it('should transfer the correct amounts', async () => { const makerAssetAddress = erc20TokenA.address; diff --git a/contracts/exchange/test/wrapper_unit_tests.ts b/contracts/exchange/test/wrapper_unit_tests.ts index ed8c652baa..b070e0bd78 100644 --- a/contracts/exchange/test/wrapper_unit_tests.ts +++ b/contracts/exchange/test/wrapper_unit_tests.ts @@ -251,6 +251,92 @@ blockchainTests('Exchange wrapper functions unit tests.', env => { }); }); + describe('batchFillOrders', () => { + it('works with no fills', async () => { + const [actualResult, receipt] = await txHelper.getResultAndReceiptAsync( + testContract.batchFillOrders, + [], + [], + [], + ); + expect(actualResult).to.deep.eq([]); + assertFillOrderCallsFromLogs(receipt.logs, []); + }); + + it('works with one fill', async () => { + const COUNT = 1; + const orders = _.times(COUNT, () => randomOrder()); + const fillAmounts = _.times(COUNT, i => orders[i].takerAssetAmount); + const signatures = _.times(COUNT, i => createOrderSignature(orders[i])); + const expectedResult = _.times(COUNT, i => getExpectedFillResults(orders[i], signatures[i])); + const expectedCalls = _.zip(orders, fillAmounts, signatures); + const [actualResult, receipt] = await txHelper.getResultAndReceiptAsync( + testContract.batchFillOrders, + orders, + fillAmounts, + signatures, + ); + expect(actualResult).to.deep.eq(expectedResult); + assertFillOrderCallsFromLogs(receipt.logs, expectedCalls as any); + }); + + it('works with many fills', async () => { + const COUNT = 8; + const orders = _.times(COUNT, () => randomOrder()); + const fillAmounts = _.times(COUNT, i => orders[i].takerAssetAmount); + const signatures = _.times(COUNT, i => createOrderSignature(orders[i])); + const expectedResult = _.times(COUNT, i => getExpectedFillResults(orders[i], signatures[i])); + const expectedCalls = _.zip(orders, fillAmounts, signatures); + const [actualResult, receipt] = await txHelper.getResultAndReceiptAsync( + testContract.batchFillOrders, + orders, + fillAmounts, + signatures, + ); + expect(actualResult).to.deep.eq(expectedResult); + assertFillOrderCallsFromLogs(receipt.logs, expectedCalls as any); + }); + + it('works with duplicate orders', async () => { + const NUM_UNIQUE_ORDERS = 2; + const COUNT = 4; + const uniqueOrders = _.times(NUM_UNIQUE_ORDERS, () => randomOrder()); + const orders = _.shuffle(_.flatten(_.times(COUNT / NUM_UNIQUE_ORDERS, () => uniqueOrders))); + const fillAmounts = _.times(COUNT, i => orders[i].takerAssetAmount.dividedToIntegerBy(COUNT)); + const signatures = _.times(COUNT, i => createOrderSignature(orders[i])); + const expectedResult = _.times(COUNT, i => getExpectedFillResults(orders[i], signatures[i])); + const expectedCalls = _.zip(orders, fillAmounts, signatures); + const [actualResult, receipt] = await txHelper.getResultAndReceiptAsync( + testContract.batchFillOrders, + orders, + fillAmounts, + signatures, + ); + expect(actualResult).to.deep.eq(expectedResult); + assertFillOrderCallsFromLogs(receipt.logs, expectedCalls as any); + }); + + it('reverts if there are more orders than fill amounts', async () => { + const COUNT = 8; + const orders = _.times(COUNT, () => randomOrder()); + const fillAmounts = _.times(COUNT - 1, i => orders[i].takerAssetAmount); + const signatures = _.times(COUNT, i => createOrderSignature(orders[i])); + const expectedError = new AnyRevertError(); // Just a generic revert. + const tx = txHelper.getResultAndReceiptAsync(testContract.batchFillOrders, orders, fillAmounts, signatures); + return expect(tx).to.revertWith(expectedError); + }); + + it('reverts if there are more orders than signatures', async () => { + const COUNT = 8; + const orders = _.times(COUNT, () => randomOrder()); + const fillAmounts = _.times(COUNT, i => orders[i].takerAssetAmount); + const signatures = _.times(COUNT - 1, i => createOrderSignature(orders[i])); + const expectedError = new AnyRevertError(); // Just a generic revert. + const tx = txHelper.getResultAndReceiptAsync(testContract.batchFillOrders, orders, fillAmounts, signatures); + return expect(tx).to.revertWith(expectedError); + }); + }); + describe('batchFillOrKillOrders', () => { it('works with no fills', async () => { const [actualResult, receipt] = await txHelper.getResultAndReceiptAsync(