Pop id from assetData before dispatching to AssetProxies

4 files changed, 136 insertions, 60 deletions

diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts
index d4fae1601..875d55daa 100644
--- a/packages/contracts/test/asset_proxy/decoder.ts
+++ b/packages/contracts/test/asset_proxy/decoder.ts
@@ -42,33 +42,19 @@ describe('TestAssetDataDecoders', () => {
     });
 
     describe('Asset Data Decoders', () => {
-        it('should correctly decode ERC20 asset data)', async () => {
-            const encodedAssetData = assetProxyUtils.encodeERC20AssetData(testAddress);
-            const expectedDecodedAssetData = assetProxyUtils.decodeERC20AssetData(encodedAssetData);
-            let decodedAssetProxyId: number;
-            let decodedTokenAddress: string;
-            [decodedAssetProxyId, decodedTokenAddress] = await testAssetProxyDecoder.publicDecodeERC20Data.callAsync(
-                encodedAssetData,
-            );
-            expect(decodedAssetProxyId).to.be.equal(expectedDecodedAssetData.assetProxyId);
-            expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress);
-        });
-
         it('should correctly decode ERC721 asset data', async () => {
             const tokenId = generatePseudoRandomSalt();
             const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId);
+            const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
             const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData);
-            let decodedAssetProxyId: number;
             let decodedTokenAddress: string;
             let decodedTokenId: BigNumber;
             let decodedData: string;
             [
-                decodedAssetProxyId,
                 decodedTokenAddress,
                 decodedTokenId,
                 decodedData,
-            ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData);
-            expect(decodedAssetProxyId).to.be.equal(expectedDecodedAssetData.assetProxyId);
+            ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetDataWithoutProxyId);
             expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress);
             expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId);
             expect(decodedData).to.be.equal(expectedDecodedAssetData.receiverData);
@@ -84,17 +70,14 @@ describe('TestAssetDataDecoders', () => {
             const receiverData = receiverDataFirst32Bytes + receiverDataExtraBytes;
             const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId, receiverData);
             const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData);
-            let decodedAssetProxyId: number;
             let decodedTokenAddress: string;
             let decodedTokenId: BigNumber;
             let decodedReceiverData: string;
             [
-                decodedAssetProxyId,
                 decodedTokenAddress,
                 decodedTokenId,
                 decodedReceiverData,
             ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData);
-            expect(decodedAssetProxyId).to.be.equal(expectedDecodedAssetData.assetProxyId);
             expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress);
             expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId);
             expect(decodedReceiverData).to.be.equal(expectedDecodedAssetData.receiverData);
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index 08376ccfb..9760d3b9c 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -96,12 +96,13 @@ describe('Asset Transfer Proxies', () => {
             it('should successfully transfer tokens', async () => {
                 // Construct ERC20 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Perform a transfer from makerAddress to takerAddress
                 const erc20Balances = await erc20Wrapper.getBalancesAsync();
                 const amount = new BigNumber(10);
                 await web3Wrapper.awaitTransactionSuccessAsync(
                     await erc20Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
+                        encodedAssetDataWithoutProxyId,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -122,12 +123,13 @@ describe('Asset Transfer Proxies', () => {
             it('should do nothing if transferring 0 amount of a token', async () => {
                 // Construct ERC20 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Perform a transfer from makerAddress to takerAddress
                 const erc20Balances = await erc20Wrapper.getBalancesAsync();
                 const amount = new BigNumber(0);
                 await web3Wrapper.awaitTransactionSuccessAsync(
                     await erc20Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
+                        encodedAssetDataWithoutProxyId,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -172,12 +174,19 @@ describe('Asset Transfer Proxies', () => {
             it('should throw if requesting address is not authorized', async () => {
                 // Construct ERC20 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(10);
                 return expectRevertOrAlwaysFailingTransactionAsync(
-                    erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
-                        from: notAuthorized,
-                    }),
+                    erc20Proxy.transferFrom.sendTransactionAsync(
+                        encodedAssetDataWithoutProxyId,
+                        makerAddress,
+                        takerAddress,
+                        amount,
+                        {
+                            from: notAuthorized,
+                        },
+                    ),
                 );
             });
         });
@@ -187,9 +196,10 @@ describe('Asset Transfer Proxies', () => {
                 const erc20Balances = await erc20Wrapper.getBalancesAsync();
 
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 const amount = new BigNumber(10);
                 const numTransfers = 2;
-                const assetData = _.times(numTransfers, () => encodedAssetData);
+                const assetData = _.times(numTransfers, () => encodedAssetDataWithoutProxyId);
                 const fromAddresses = _.times(numTransfers, () => makerAddress);
                 const toAddresses = _.times(numTransfers, () => takerAddress);
                 const amounts = _.times(numTransfers, () => amount);
@@ -218,9 +228,10 @@ describe('Asset Transfer Proxies', () => {
 
             it('should throw if not called by an authorized address', async () => {
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 const amount = new BigNumber(10);
                 const numTransfers = 2;
-                const assetData = _.times(numTransfers, () => encodedAssetData);
+                const assetData = _.times(numTransfers, () => encodedAssetDataWithoutProxyId);
                 const fromAddresses = _.times(numTransfers, () => makerAddress);
                 const toAddresses = _.times(numTransfers, () => takerAddress);
                 const amounts = _.times(numTransfers, () => amount);
@@ -244,6 +255,7 @@ describe('Asset Transfer Proxies', () => {
             it('should successfully transfer tokens', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -251,7 +263,7 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(1);
                 await web3Wrapper.awaitTransactionSuccessAsync(
                     await erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
+                        encodedAssetDataWithoutProxyId,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -267,13 +279,14 @@ describe('Asset Transfer Proxies', () => {
             it('should not call onERC721Received when transferring to a smart contract without receiver data', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(1);
                 const txHash = await erc721Proxy.transferFrom.sendTransactionAsync(
-                    encodedAssetData,
+                    encodedAssetDataWithoutProxyId,
                     makerAddress,
                     erc721Receiver.address,
                     amount,
@@ -298,13 +311,14 @@ describe('Asset Transfer Proxies', () => {
                     erc721MakerTokenId,
                     receiverData,
                 );
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(1);
                 const txHash = await erc721Proxy.transferFrom.sendTransactionAsync(
-                    encodedAssetData,
+                    encodedAssetDataWithoutProxyId,
                     makerAddress,
                     erc721Receiver.address,
                     amount,
@@ -333,6 +347,7 @@ describe('Asset Transfer Proxies', () => {
                     erc721MakerTokenId,
                     receiverData,
                 );
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -340,7 +355,7 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(1);
                 return expectRevertOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
+                        encodedAssetDataWithoutProxyId,
                         makerAddress,
                         erc20Proxy.address, // the ERC20 proxy does not have an ERC721 receiver
                         amount,
@@ -352,6 +367,7 @@ describe('Asset Transfer Proxies', () => {
             it('should throw if transferring 0 amount of a token', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -359,7 +375,7 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(0);
                 return expectRevertOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
+                        encodedAssetDataWithoutProxyId,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -371,6 +387,7 @@ describe('Asset Transfer Proxies', () => {
             it('should throw if transferring > 1 amount of a token', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -378,7 +395,7 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(500);
                 return expectRevertOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
+                        encodedAssetDataWithoutProxyId,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -390,6 +407,7 @@ describe('Asset Transfer Proxies', () => {
             it('should throw if allowances are too low', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Remove transfer approval for makerAddress.
                 await web3Wrapper.awaitTransactionSuccessAsync(
                     await erc721Token.setApprovalForAll.sendTransactionAsync(erc721Proxy.address, false, {
@@ -400,20 +418,27 @@ describe('Asset Transfer Proxies', () => {
                 // Perform a transfer; expect this to fail.
                 const amount = new BigNumber(1);
                 return expectRevertOrAlwaysFailingTransactionAsync(
-                    erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
-                        from: notAuthorized,
-                    }),
+                    erc20Proxy.transferFrom.sendTransactionAsync(
+                        encodedAssetDataWithoutProxyId,
+                        makerAddress,
+                        takerAddress,
+                        amount,
+                        {
+                            from: notAuthorized,
+                        },
+                    ),
                 );
             });
 
             it('should throw if requesting address is not authorized', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
+                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(1);
                 return expectRevertOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
+                        encodedAssetDataWithoutProxyId,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -430,8 +455,8 @@ describe('Asset Transfer Proxies', () => {
 
                 const numTransfers = 2;
                 const assetData = [
-                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA),
-                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB),
+                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA).slice(0, -2),
+                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB).slice(0, -2),
                 ];
                 const fromAddresses = _.times(numTransfers, () => makerAddress);
                 const toAddresses = _.times(numTransfers, () => takerAddress);
@@ -462,8 +487,8 @@ describe('Asset Transfer Proxies', () => {
 
                 const numTransfers = 2;
                 const assetData = [
-                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA),
-                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB),
+                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA).slice(0, -2),
+                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB).slice(0, -2),
                 ];
                 const fromAddresses = _.times(numTransfers, () => makerAddress);
                 const toAddresses = _.times(numTransfers, () => takerAddress);
@@ -484,3 +509,4 @@ describe('Asset Transfer Proxies', () => {
     });
 });
 // tslint:enable:no-unnecessary-type-assertion
+// tslint:disable:max-file-line-count
diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts
index 9e113e47d..abbfd7ac7 100644
--- a/packages/contracts/test/exchange/dispatcher.ts
+++ b/packages/contracts/test/exchange/dispatcher.ts
@@ -276,12 +276,14 @@ describe('AssetProxyDispatcher', () => {
             );
             // Construct metadata for ERC20 proxy
             const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
+            const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
             // Perform a transfer from makerAddress to takerAddress
             const erc20Balances = await erc20Wrapper.getBalancesAsync();
             const amount = new BigNumber(10);
             await web3Wrapper.awaitTransactionSuccessAsync(
                 await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
-                    encodedAssetData,
+                    encodedAssetDataWithoutProxyId,
+                    AssetProxyId.ERC20,
                     makerAddress,
                     takerAddress,
                     amount,
@@ -302,11 +304,13 @@ describe('AssetProxyDispatcher', () => {
         it('should throw if dispatching to unregistered proxy', async () => {
             // Construct metadata for ERC20 proxy
             const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
+            const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
             // Perform a transfer from makerAddress to takerAddress
             const amount = new BigNumber(10);
             return expectRevertOrAlwaysFailingTransactionAsync(
                 assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
-                    encodedAssetData,
+                    encodedAssetDataWithoutProxyId,
+                    AssetProxyId.ERC20,
                     makerAddress,
                     takerAddress,
                     amount,
diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts
index b66cff90a..ad0704e3a 100644
--- a/packages/contracts/test/exchange/wrapper.ts
+++ b/packages/contracts/test/exchange/wrapper.ts
@@ -781,20 +781,49 @@ describe('Exchange wrappers', () => {
                 expect(newBalances).to.be.deep.equal(erc20Balances);
             });
 
-            it('should throw when a signedOrder does not use the same takerAssetAddress', async () => {
+            it('should not fill a signedOrder that does not use the same takerAssetAddress', async () => {
                 signedOrders = [
                     orderFactory.newSignedOrder(),
+                    orderFactory.newSignedOrder(),
                     orderFactory.newSignedOrder({
                         takerAssetData: assetProxyUtils.encodeERC20AssetData(zrxToken.address),
                     }),
-                    orderFactory.newSignedOrder(),
                 ];
+                const takerAssetFillAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18);
+                const filledSignedOrders = signedOrders.slice(0, -1);
+                _.forEach(filledSignedOrders, signedOrder => {
+                    erc20Balances[makerAddress][defaultMakerAssetAddress] = erc20Balances[makerAddress][
+                        defaultMakerAssetAddress
+                    ].minus(signedOrder.makerAssetAmount);
+                    erc20Balances[makerAddress][defaultTakerAssetAddress] = erc20Balances[makerAddress][
+                        defaultTakerAssetAddress
+                    ].add(signedOrder.takerAssetAmount);
+                    erc20Balances[makerAddress][zrxToken.address] = erc20Balances[makerAddress][zrxToken.address].minus(
+                        signedOrder.makerFee,
+                    );
+                    erc20Balances[takerAddress][defaultMakerAssetAddress] = erc20Balances[takerAddress][
+                        defaultMakerAssetAddress
+                    ].add(signedOrder.makerAssetAmount);
+                    erc20Balances[takerAddress][defaultTakerAssetAddress] = erc20Balances[takerAddress][
+                        defaultTakerAssetAddress
+                    ].minus(signedOrder.takerAssetAmount);
+                    erc20Balances[takerAddress][zrxToken.address] = erc20Balances[takerAddress][zrxToken.address].minus(
+                        signedOrder.takerFee,
+                    );
+                    erc20Balances[feeRecipientAddress][zrxToken.address] = erc20Balances[feeRecipientAddress][
+                        zrxToken.address
+                    ].add(signedOrder.makerFee.add(signedOrder.takerFee));
+                });
+                await exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, {
+                    takerAssetFillAmount,
+                    // HACK(albrow): We need to hardcode the gas estimate here because
+                    // the Geth gas estimator doesn't work with the way we use
+                    // delegatecall and swallow errors.
+                    gas: 600000,
+                });
 
-                return expectRevertOrAlwaysFailingTransactionAsync(
-                    exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, {
-                        takerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18),
-                    }),
-                );
+                const newBalances = await erc20Wrapper.getBalancesAsync();
+                expect(newBalances).to.be.deep.equal(erc20Balances);
             });
         });
 
@@ -894,6 +923,10 @@ describe('Exchange wrappers', () => {
                 );
                 await exchangeWrapper.marketBuyOrdersNoThrowAsync(signedOrders, takerAddress, {
                     makerAssetFillAmount,
+                    // HACK(albrow): We need to hardcode the gas estimate here because
+                    // the Geth gas estimator doesn't work with the way we use
+                    // delegatecall and swallow errors.
+                    gas: 600000,
                 });
 
                 const newBalances = await erc20Wrapper.getBalancesAsync();
@@ -926,8 +959,8 @@ describe('Exchange wrappers', () => {
                 );
             });
 
-            it('should fill all signedOrders if cannot fill entire takerAssetFillAmount', async () => {
-                const takerAssetFillAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18);
+            it('should fill all signedOrders if cannot fill entire makerAssetFillAmount', async () => {
+                const makerAssetFillAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18);
                 _.forEach(signedOrders, signedOrder => {
                     erc20Balances[makerAddress][defaultMakerAssetAddress] = erc20Balances[makerAddress][
                         defaultMakerAssetAddress
@@ -951,8 +984,8 @@ describe('Exchange wrappers', () => {
                         zrxToken.address
                     ].add(signedOrder.makerFee.add(signedOrder.takerFee));
                 });
-                await exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, {
-                    takerAssetFillAmount,
+                await exchangeWrapper.marketBuyOrdersNoThrowAsync(signedOrders, takerAddress, {
+                    makerAssetFillAmount,
                     // HACK(albrow): We need to hardcode the gas estimate here because
                     // the Geth gas estimator doesn't work with the way we use
                     // delegatecall and swallow errors.
@@ -963,20 +996,50 @@ describe('Exchange wrappers', () => {
                 expect(newBalances).to.be.deep.equal(erc20Balances);
             });
 
-            it('should throw when a signedOrder does not use the same makerAssetAddress', async () => {
+            it('should not fill a signedOrder that does not use the same makerAssetAddress', async () => {
                 signedOrders = [
                     orderFactory.newSignedOrder(),
+                    orderFactory.newSignedOrder(),
                     orderFactory.newSignedOrder({
                         makerAssetData: assetProxyUtils.encodeERC20AssetData(zrxToken.address),
                     }),
-                    orderFactory.newSignedOrder(),
                 ];
 
-                return expectRevertOrAlwaysFailingTransactionAsync(
-                    exchangeWrapper.marketBuyOrdersNoThrowAsync(signedOrders, takerAddress, {
-                        makerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18),
-                    }),
-                );
+                const makerAssetFillAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18);
+                const filledSignedOrders = signedOrders.slice(0, -1);
+                _.forEach(filledSignedOrders, signedOrder => {
+                    erc20Balances[makerAddress][defaultMakerAssetAddress] = erc20Balances[makerAddress][
+                        defaultMakerAssetAddress
+                    ].minus(signedOrder.makerAssetAmount);
+                    erc20Balances[makerAddress][defaultTakerAssetAddress] = erc20Balances[makerAddress][
+                        defaultTakerAssetAddress
+                    ].add(signedOrder.takerAssetAmount);
+                    erc20Balances[makerAddress][zrxToken.address] = erc20Balances[makerAddress][zrxToken.address].minus(
+                        signedOrder.makerFee,
+                    );
+                    erc20Balances[takerAddress][defaultMakerAssetAddress] = erc20Balances[takerAddress][
+                        defaultMakerAssetAddress
+                    ].add(signedOrder.makerAssetAmount);
+                    erc20Balances[takerAddress][defaultTakerAssetAddress] = erc20Balances[takerAddress][
+                        defaultTakerAssetAddress
+                    ].minus(signedOrder.takerAssetAmount);
+                    erc20Balances[takerAddress][zrxToken.address] = erc20Balances[takerAddress][zrxToken.address].minus(
+                        signedOrder.takerFee,
+                    );
+                    erc20Balances[feeRecipientAddress][zrxToken.address] = erc20Balances[feeRecipientAddress][
+                        zrxToken.address
+                    ].add(signedOrder.makerFee.add(signedOrder.takerFee));
+                });
+                await exchangeWrapper.marketBuyOrdersNoThrowAsync(signedOrders, takerAddress, {
+                    makerAssetFillAmount,
+                    // HACK(albrow): We need to hardcode the gas estimate here because
+                    // the Geth gas estimator doesn't work with the way we use
+                    // delegatecall and swallow errors.
+                    gas: 600000,
+                });
+
+                const newBalances = await erc20Wrapper.getBalancesAsync();
+                expect(newBalances).to.be.deep.equal(erc20Balances);
             });
         });