From c0f1e5f17f88ff02cb0f88139793afd0d7eb54d3 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 1 Sep 2019 12:22:00 -0700 Subject: [PATCH] Make fillOrderNoThrow internal only, remove batchFillOrders --- .../contracts/src/MixinWrapperFunctions.sol | 106 +++--- .../src/interfaces/IWrapperFunctions.sol | 29 -- .../test/TestProtocolFeesReceiver.sol | 2 +- .../contracts/test/TestWrapperFunctions.sol | 15 + .../exchange/test/utils/exchange_wrapper.ts | 28 -- contracts/exchange/test/wrapper.ts | 322 ------------------ contracts/exchange/test/wrapper_unit_tests.ts | 88 +---- 7 files changed, 55 insertions(+), 535 deletions(-) diff --git a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol index 7536797a8e..fac761f945 100644 --- a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol +++ b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol @@ -59,71 +59,6 @@ contract MixinWrapperFunctions is return fillResults; } - /// Note: This function only needs `refundFinalBalance` modifier because ether will not - // be returned in the event that the delegatecall fails. This said, there is no - // reason to invoke `disableRefundUntilEnd` because it is cheaper to use this modifier - // and the inner refund will not affect the logic of this call. - /// @dev Fills the input order. - /// Returns a null FillResults instance if the transaction would otherwise revert. - /// @param order Order struct containing order specifications. - /// @param takerAssetFillAmount Desired amount of takerAsset to sell. - /// @param signature Proof that order has been created by maker. - /// @return Amounts filled and fees paid by maker and taker. - function fillOrderNoThrow( - LibOrder.Order memory order, - uint256 takerAssetFillAmount, - bytes memory signature - ) - public - payable - refundFinalBalance - returns (LibFillResults.FillResults memory fillResults) - { - // ABI encode calldata for `fillOrder` - bytes memory fillOrderCalldata = abi.encodeWithSelector( - IExchangeCore(address(0)).fillOrder.selector, - order, - takerAssetFillAmount, - signature - ); - - (bool didSucceed, bytes memory returnData) = address(this).delegatecall(fillOrderCalldata); - if (didSucceed) { - assert(returnData.length == 160); - fillResults = abi.decode(returnData, (LibFillResults.FillResults)); - } - // fillResults values will be 0 by default if call was unsuccessful - 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. @@ -170,7 +105,7 @@ contract MixinWrapperFunctions is uint256 ordersLength = orders.length; fillResults = new LibFillResults.FillResults[](ordersLength); for (uint256 i = 0; i != ordersLength; i++) { - fillResults[i] = fillOrderNoThrow( + fillResults[i] = _fillOrderNoThrow( orders[i], takerAssetFillAmounts[i], signatures[i] @@ -208,7 +143,7 @@ contract MixinWrapperFunctions is orders[i].takerAssetData = takerAssetData; // Attempt to sell the remaining amount of takerAsset - LibFillResults.FillResults memory singleFillResults = fillOrderNoThrow( + LibFillResults.FillResults memory singleFillResults = _fillOrderNoThrow( orders[i], remainingTakerAssetFillAmount, signatures[i] @@ -262,7 +197,7 @@ contract MixinWrapperFunctions is orders[i].makerAssetData = makerAssetData; // Attempt to sell the remaining amount of takerAsset - LibFillResults.FillResults memory singleFillResults = fillOrderNoThrow( + LibFillResults.FillResults memory singleFillResults = _fillOrderNoThrow( orders[i], remainingTakerAssetFillAmount, signatures[i] @@ -367,4 +302,39 @@ contract MixinWrapperFunctions is } return fillResults; } + + /// Note: This function only needs `refundFinalBalance` modifier because ether will not + // be returned in the event that the delegatecall fails. This said, there is no + // reason to invoke `disableRefundUntilEnd` because it is cheaper to use this modifier + // and the inner refund will not affect the logic of this call. + /// @dev Fills the input order. + /// Returns a null FillResults instance if the transaction would otherwise revert. + /// @param order Order struct containing order specifications. + /// @param takerAssetFillAmount Desired amount of takerAsset to sell. + /// @param signature Proof that order has been created by maker. + /// @return Amounts filled and fees paid by maker and taker. + function _fillOrderNoThrow( + LibOrder.Order memory order, + uint256 takerAssetFillAmount, + bytes memory signature + ) + internal + returns (LibFillResults.FillResults memory fillResults) + { + // ABI encode calldata for `fillOrder` + bytes memory fillOrderCalldata = abi.encodeWithSelector( + IExchangeCore(address(0)).fillOrder.selector, + order, + takerAssetFillAmount, + signature + ); + + (bool didSucceed, bytes memory returnData) = address(this).delegatecall(fillOrderCalldata); + if (didSucceed) { + assert(returnData.length == 160); + fillResults = abi.decode(returnData, (LibFillResults.FillResults)); + } + // fillResults values will be 0 by default if call was unsuccessful + return fillResults; + } } diff --git a/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol b/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol index bb68a9ee7a..980c165528 100644 --- a/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol +++ b/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol @@ -38,35 +38,6 @@ contract IWrapperFunctions { payable returns (LibFillResults.FillResults memory fillResults); - /// @dev Fills an order with specified parameters and ECDSA signature. - /// Returns false if the transaction would otherwise revert. - /// @param order LibOrder.Order struct containing order specifications. - /// @param takerAssetFillAmount Desired amount of takerAsset to sell. - /// @param signature Proof that order has been created by maker. - /// @return Amounts filled and fees paid by maker and taker. - function fillOrderNoThrow( - LibOrder.Order memory order, - uint256 takerAssetFillAmount, - bytes memory signature - ) - public - payable - returns (LibFillResults.FillResults memory fillResults); - - /// @dev Synchronously 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 a908f4670f..97cae324e5 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.batchFillOrders.value(msg.value)(orders, takerAssetFilledAmounts, signatures); + testProtocolFees.batchFillOrKillOrders.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/contracts/test/TestWrapperFunctions.sol b/contracts/exchange/contracts/test/TestWrapperFunctions.sol index 7773f289c9..20e0fae825 100644 --- a/contracts/exchange/contracts/test/TestWrapperFunctions.sol +++ b/contracts/exchange/contracts/test/TestWrapperFunctions.sol @@ -65,6 +65,21 @@ contract TestWrapperFunctions is orderInfo.orderHash = order.getTypedDataHash(EIP712_EXCHANGE_DOMAIN_HASH); } + function fillOrderNoThrow( + LibOrder.Order memory order, + uint256 takerAssetFillAmount, + bytes memory signature + ) + public + returns (LibFillResults.FillResults memory fillResults) + { + return _fillOrderNoThrow( + order, + takerAssetFillAmount, + signature + ); + } + /// @dev Overridden to log arguments, be deterministic, and revert with certain inputs. function _fillOrder( LibOrder.Order memory order, diff --git a/contracts/exchange/test/utils/exchange_wrapper.ts b/contracts/exchange/test/utils/exchange_wrapper.ts index 4871bac979..7f5b8939ec 100644 --- a/contracts/exchange/test/utils/exchange_wrapper.ts +++ b/contracts/exchange/test/utils/exchange_wrapper.ts @@ -54,34 +54,6 @@ export class ExchangeWrapper { ); return txReceipt; } - public async fillOrderNoThrowAsync( - signedOrder: SignedOrder, - from: string, - opts: { takerAssetFillAmount?: BigNumber; gas?: number; gasPrice?: BigNumber } = {}, - ): Promise { - const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); - const txReceipt = await this._exchange.fillOrderNoThrow.awaitTransactionSuccessAsync( - params.order, - params.takerAssetFillAmount, - params.signature, - { from, gas: opts.gas, gasPrice: opts.gasPrice }, - ); - 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 d63135daef..3d83787d60 100644 --- a/contracts/exchange/test/wrapper.ts +++ b/contracts/exchange/test/wrapper.ts @@ -217,260 +217,6 @@ blockchainTests.resets('Exchange wrappers', env => { }); }); - describe('fillOrderNoThrow', () => { - it('should transfer the correct amounts', async () => { - const signedOrder = await orderFactory.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), 18), - }); - const takerAssetFillAmount = signedOrder.takerAssetAmount.div(2); - - const fillResults = await exchange.fillOrderNoThrow.callAsync( - signedOrder, - takerAssetFillAmount, - signedOrder.signature, - { from: takerAddress }, - ); - await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress, { - takerAssetFillAmount, - }); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - 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); - - expect(fillResults.makerAssetFilledAmount).to.bignumber.equal(makerAssetFilledAmount); - expect(fillResults.takerAssetFilledAmount).to.bignumber.equal(takerAssetFillAmount); - expect(fillResults.makerFeePaid).to.bignumber.equal(makerFee); - expect(fillResults.takerFeePaid).to.bignumber.equal(takerFee); - - expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( - erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFilledAmount), - ); - expect(newBalances[makerAddress][defaultTakerAssetAddress]).to.be.bignumber.equal( - erc20Balances[makerAddress][defaultTakerAssetAddress].plus(takerAssetFillAmount), - ); - expect(newBalances[makerAddress][feeToken.address]).to.be.bignumber.equal( - erc20Balances[makerAddress][feeToken.address].minus(makerFee), - ); - expect(newBalances[takerAddress][defaultTakerAssetAddress]).to.be.bignumber.equal( - erc20Balances[takerAddress][defaultTakerAssetAddress].minus(takerAssetFillAmount), - ); - expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( - erc20Balances[takerAddress][defaultMakerAssetAddress].plus(makerAssetFilledAmount), - ); - expect(newBalances[takerAddress][feeToken.address]).to.be.bignumber.equal( - erc20Balances[takerAddress][feeToken.address].minus(takerFee), - ); - expect(newBalances[feeRecipientAddress][feeToken.address]).to.be.bignumber.equal( - erc20Balances[feeRecipientAddress][feeToken.address].plus(makerFee.plus(takerFee)), - ); - }); - - it('should not change erc20Balances if maker erc20Balances are too low to fill order', async () => { - const signedOrder = await orderFactory.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18), - }); - - const fillResults = await exchange.fillOrderNoThrow.callAsync( - signedOrder, - signedOrder.takerAssetAmount, - signedOrder.signature, - { from: takerAddress }, - ); - await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - expect(fillResults).to.deep.equal(nullFillResults); - expect(newBalances).to.be.deep.equal(erc20Balances); - }); - - it('should not change erc20Balances if taker erc20Balances are too low to fill order', async () => { - const signedOrder = await orderFactory.newSignedOrderAsync({ - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18), - }); - - const fillResults = await exchange.fillOrderNoThrow.callAsync( - signedOrder, - signedOrder.takerAssetAmount, - signedOrder.signature, - { from: takerAddress }, - ); - await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - expect(fillResults).to.deep.equal(nullFillResults); - expect(newBalances).to.be.deep.equal(erc20Balances); - }); - - it('should not change erc20Balances if maker allowances are too low to fill order', async () => { - const signedOrder = await orderFactory.newSignedOrderAsync(); - - await erc20TokenA.approve.awaitTransactionSuccessAsync(erc20Proxy.address, new BigNumber(0), { - from: makerAddress, - }); - const fillResults = await exchange.fillOrderNoThrow.callAsync( - signedOrder, - signedOrder.takerAssetAmount, - signedOrder.signature, - { from: takerAddress }, - ); - await erc20TokenA.approve.awaitTransactionSuccessAsync( - erc20Proxy.address, - constants.INITIAL_ERC20_ALLOWANCE, - { from: makerAddress }, - ); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - expect(fillResults).to.deep.equal(nullFillResults); - expect(newBalances).to.be.deep.equal(erc20Balances); - }); - - it('should not change erc20Balances if taker allowances are too low to fill order', async () => { - const signedOrder = await orderFactory.newSignedOrderAsync(); - - await erc20TokenB.approve.awaitTransactionSuccessAsync(erc20Proxy.address, new BigNumber(0), { - from: takerAddress, - }); - const fillResults = await exchange.fillOrderNoThrow.callAsync( - signedOrder, - signedOrder.takerAssetAmount, - signedOrder.signature, - { from: takerAddress }, - ); - await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress); - await erc20TokenB.approve.sendTransactionAsync(erc20Proxy.address, constants.INITIAL_ERC20_ALLOWANCE, { - from: takerAddress, - }); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - expect(fillResults).to.deep.equal(nullFillResults); - expect(newBalances).to.be.deep.equal(erc20Balances); - }); - - it('should not change erc20Balances if makerAssetAddress is ZRX, makerAssetAmount + makerFee > maker balance', async () => { - const makerZRXBalance = new BigNumber(erc20Balances[makerAddress][feeToken.address]); - const signedOrder = await orderFactory.newSignedOrderAsync({ - makerAssetAmount: makerZRXBalance, - makerFee: new BigNumber(1), - makerAssetData: assetDataUtils.encodeERC20AssetData(feeToken.address), - }); - - const fillResults = await exchange.fillOrderNoThrow.callAsync( - signedOrder, - signedOrder.takerAssetAmount, - signedOrder.signature, - { from: takerAddress }, - ); - await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - expect(fillResults).to.deep.equal(nullFillResults); - expect(newBalances).to.be.deep.equal(erc20Balances); - }); - - it('should not change erc20Balances if makerAssetAddress is ZRX, makerAssetAmount + makerFee > maker allowance', async () => { - const makerZRXAllowance = await feeToken.allowance.callAsync(makerAddress, erc20Proxy.address); - const signedOrder = await orderFactory.newSignedOrderAsync({ - makerAssetAmount: new BigNumber(makerZRXAllowance), - makerFee: new BigNumber(1), - makerAssetData: assetDataUtils.encodeERC20AssetData(feeToken.address), - }); - await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress); - const newBalances = await erc20Wrapper.getBalancesAsync(); - expect(newBalances).to.be.deep.equal(erc20Balances); - }); - - it('should not change erc20Balances if takerAssetAddress is ZRX, takerAssetAmount + takerFee > taker balance', async () => { - const takerZRXBalance = new BigNumber(erc20Balances[takerAddress][feeToken.address]); - const signedOrder = await orderFactory.newSignedOrderAsync({ - takerAssetAmount: takerZRXBalance, - takerFee: new BigNumber(1), - takerAssetData: assetDataUtils.encodeERC20AssetData(feeToken.address), - }); - - const fillResults = await exchange.fillOrderNoThrow.callAsync( - signedOrder, - signedOrder.takerAssetAmount, - signedOrder.signature, - { from: takerAddress }, - ); - await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - expect(fillResults).to.deep.equal(nullFillResults); - expect(newBalances).to.be.deep.equal(erc20Balances); - }); - - it('should not change erc20Balances if takerAssetAddress is ZRX, takerAssetAmount + takerFee > taker allowance', async () => { - const takerZRXAllowance = await feeToken.allowance.callAsync(takerAddress, erc20Proxy.address); - const signedOrder = await orderFactory.newSignedOrderAsync({ - takerAssetAmount: new BigNumber(takerZRXAllowance), - takerFee: new BigNumber(1), - takerAssetData: assetDataUtils.encodeERC20AssetData(feeToken.address), - }); - - const fillResults = await exchange.fillOrderNoThrow.callAsync( - signedOrder, - signedOrder.takerAssetAmount, - signedOrder.signature, - { from: takerAddress }, - ); - await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - expect(fillResults).to.deep.equal(nullFillResults); - expect(newBalances).to.be.deep.equal(erc20Balances); - }); - - it('should successfully exchange ERC721 tokens', async () => { - // Construct Exchange parameters - const makerAssetId = erc721MakerAssetId; - const takerAssetId = erc721TakerAssetId; - const signedOrder = await orderFactory.newSignedOrderAsync({ - makerAssetAmount: new BigNumber(1), - takerAssetAmount: new BigNumber(1), - makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), - takerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, takerAssetId), - }); - // Verify pre-conditions - const initialOwnerMakerAsset = await erc721Token.ownerOf.callAsync(makerAssetId); - expect(initialOwnerMakerAsset).to.be.bignumber.equal(makerAddress); - const initialOwnerTakerAsset = await erc721Token.ownerOf.callAsync(takerAssetId); - expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress); - // Call Exchange - const takerAssetFillAmount = signedOrder.takerAssetAmount; - - const fillResults = await exchange.fillOrderNoThrow.callAsync( - signedOrder, - takerAssetFillAmount, - signedOrder.signature, - { from: takerAddress }, - ); - await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress, { - takerAssetFillAmount, - }); - - // Verify post-conditions - expect(fillResults.makerAssetFilledAmount).to.bignumber.equal(signedOrder.makerAssetAmount); - expect(fillResults.takerAssetFilledAmount).to.bignumber.equal(signedOrder.takerAssetAmount); - expect(fillResults.makerFeePaid).to.bignumber.equal(signedOrder.makerFee); - expect(fillResults.takerFeePaid).to.bignumber.equal(signedOrder.takerFee); - - const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(makerAssetId); - expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress); - const newOwnerTakerAsset = await erc721Token.ownerOf.callAsync(takerAssetId); - expect(newOwnerTakerAsset).to.be.bignumber.equal(makerAddress); - }); - }); - describe('batch functions', () => { let signedOrders: SignedOrder[]; beforeEach(async () => { @@ -481,74 +227,6 @@ 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, gasPrice: DEFAULT_GAS_PRICE }, - ); - 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 c3391d3d53..ed8c652baa 100644 --- a/contracts/exchange/test/wrapper_unit_tests.ts +++ b/contracts/exchange/test/wrapper_unit_tests.ts @@ -10,7 +10,7 @@ import { } from '@0x/contracts-test-utils'; import { ReferenceFunctions as UtilReferenceFunctions } from '@0x/contracts-utils'; import { ExchangeRevertErrors, orderHashUtils } from '@0x/order-utils'; -import { FillResults, OrderInfo, OrderStatus, OrderWithoutDomain as Order } from '@0x/types'; +import { FillResults, OrderWithoutDomain as Order } from '@0x/types'; import { AnyRevertError, BigNumber, SafeMathRevertErrors, StringRevertError } from '@0x/utils'; import { LogEntry, LogWithDecodedArgs } from 'ethereum-types'; import * as ethjs from 'ethereumjs-util'; @@ -251,92 +251,6 @@ 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(