address comments and tests

contracts/exchange-forwarder/contracts/src/MixinAssets.sol

@@ -23,6 +23,7 @@ import "@0x/contracts-utils/contracts/src/LibRichErrors.sol";
 import "@0x/contracts-utils/contracts/src/Ownable.sol";
 import "@0x/contracts-erc20/contracts/src/LibERC20Token.sol";
 import "@0x/contracts-erc721/contracts/src/interfaces/IERC721Token.sol";
+import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetData.sol";
 import "./libs/LibConstants.sol";
 import "./libs/LibForwarderRichErrors.sol";
 import "./interfaces/IAssets.sol";
@@ -59,9 +60,11 @@ contract MixinAssets is
         external
     {
         bytes4 proxyId = assetData.readBytes4(0);
+        bytes4 erc20ProxyId = IAssetData(address(0)).ERC20Token.selector;
+
         // For now we only care about ERC20, since percentage fees on ERC721 tokens are invalid.
-        if (proxyId == ERC20_PROXY_ID) {
+        if (proxyId == erc20ProxyId) {
-            address proxyAddress = EXCHANGE.getAssetProxy(ERC20_PROXY_ID);
+            address proxyAddress = EXCHANGE.getAssetProxy(erc20ProxyId);
             if (proxyAddress == address(0)) {
                 LibRichErrors.rrevert(LibForwarderRichErrors.UnregisteredAssetProxyError());
             }
@@ -81,12 +84,13 @@ contract MixinAssets is
     {
         bytes4 proxyId = assetData.readBytes4(0);
 
-        if (proxyId == ERC20_PROXY_ID) {
+        if (
+            proxyId == IAssetData(address(0)).ERC20Token.selector ||
+            proxyId == IAssetData(address(0)).ERC20Bridge.selector
+        ) {
             _transferERC20Token(assetData, amount);
-        } else if (proxyId == ERC721_PROXY_ID) {
+        } else if (proxyId == IAssetData(address(0)).ERC721Token.selector) {
             _transferERC721Token(assetData, amount);
-        } else if (proxyId == ERC20_BRIDGE_PROXY_ID) {
-            _transferERC20BridgeAsset(assetData, amount);
         } else {
             LibRichErrors.rrevert(LibForwarderRichErrors.UnsupportedAssetProxyError(
                 proxyId
@@ -94,7 +98,7 @@ contract MixinAssets is
         }
     }
 
-    /// @dev Decodes ERC20 assetData and transfers given amount to sender.
+    /// @dev Decodes ERC20 or ERC20Bridge assetData and transfers given amount to sender.
     /// @param assetData Byte array encoded for the respective asset proxy.
     /// @param amount Amount of asset to transfer to sender.
     function _transferERC20Token(
@@ -133,18 +137,4 @@ contract MixinAssets is
             tokenId
         );
     }
-
-    /// @dev Decodes ERC20Bridge assetData and transfers given amount to sender.
-    /// @param assetData Byte array encoded for the respective asset proxy.
-    /// @param amount Amount of asset to transfer to sender.
-    function _transferERC20BridgeAsset(
-        bytes memory assetData,
-        uint256 amount
-    )
-        internal
-    {
-        address token = assetData.readAddress(16);
-        // Transfer tokens.
-        LibERC20Token.transfer(token, msg.sender, amount);
-    }
 }

contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol

@@ -19,12 +19,15 @@
 pragma solidity ^0.5.9;
 pragma experimental ABIEncoderV2;
 
+import "@0x/contracts-utils/contracts/src/LibBytes.sol";
 import "@0x/contracts-utils/contracts/src/LibRichErrors.sol";
 import "@0x/contracts-utils/contracts/src/LibSafeMath.sol";
 import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol";
 import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol";
 import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol";
 import "@0x/contracts-exchange/contracts/src/interfaces/IExchange.sol";
+import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetData.sol";
+import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol";
 import "./libs/LibConstants.sol";
 import "./libs/LibForwarderRichErrors.sol";
 import "./MixinAssets.sol";
@@ -34,6 +37,7 @@ contract MixinExchangeWrapper is
     LibConstants,
     MixinAssets
 {
+    using LibBytes for bytes;
     using LibSafeMath for uint256;
 
     /// @dev Fills the input order.
@@ -88,7 +92,10 @@ contract MixinExchangeWrapper is
         )
     {
         // No taker fee or percentage fee
-        if (order.takerFee == 0 || _isPercentageFee(order.takerFeeAssetData, order.makerAssetData)) {
+        if (
+            order.takerFee == 0 ||
+            _areUnderlyingAssetsEqual(order.takerFeeAssetData, order.makerAssetData)
+        ) {
             // Attempt to sell the remaining amount of WETH
             LibFillResults.FillResults memory singleFillResults = _fillOrderNoThrow(
                 order,
@@ -103,7 +110,7 @@ contract MixinExchangeWrapper is
             makerAssetAcquiredAmount = singleFillResults.makerAssetFilledAmount
                 .safeSub(singleFillResults.takerFeePaid);
         // WETH fee
-        } else if (order.takerFeeAssetData.equals(order.takerAssetData)) {
+        } else if (_areUnderlyingAssetsEqual(order.takerFeeAssetData, order.takerAssetData)) {
 
             // We will first sell WETH as the takerAsset, then use it to pay the takerFee.
             // This ensures that we reserve enough to pay the taker and protocol fees.
@@ -150,9 +157,10 @@ contract MixinExchangeWrapper is
             uint256 totalMakerAssetAcquiredAmount
         )
     {
-        uint256 ordersLength = orders.length;
         uint256 protocolFee = tx.gasprice.safeMul(EXCHANGE.protocolFeeMultiplier());
+        bytes4 erc20BridgeProxyId = IAssetData(address(0)).ERC20Bridge.selector;
 
+        uint256 ordersLength = orders.length;
         for (uint256 i = 0; i != ordersLength; i++) {
             // Preemptively skip to avoid division by zero in _marketSellSingleOrder
             if (orders[i].makerAssetAmount == 0 || orders[i].takerAssetAmount == 0) {
@@ -164,6 +172,15 @@ contract MixinExchangeWrapper is
                 .safeSub(totalWethSpentAmount)
                 .safeSub(protocolFee);
 
+            // If the maker asset is ERC20Bridge, take a snapshot of the Forwarder contract's balance.
+            bytes4 makerAssetProxyId = orders[i].makerAssetData.readBytes4(0);
+            address tokenAddress;
+            uint256 balanceBefore;
+            if (makerAssetProxyId == erc20BridgeProxyId) {
+                tokenAddress = orders[i].makerAssetData.readAddress(16);
+                balanceBefore = IERC20Token(tokenAddress).balanceOf(address(this));
+            }
+
             (
                 uint256 wethSpentAmount,
                 uint256 makerAssetAcquiredAmount
@@ -173,6 +190,15 @@ contract MixinExchangeWrapper is
                 remainingTakerAssetFillAmount
             );
 
+            // Account for the ERC20Bridge transfering more of the maker asset than expected.
+            if (makerAssetProxyId == erc20BridgeProxyId) {
+                uint256 balanceAfter = IERC20Token(tokenAddress).balanceOf(address(this));
+                makerAssetAcquiredAmount = LibSafeMath.max256(
+                    balanceAfter.safeSub(balanceBefore),
+                    makerAssetAcquiredAmount
+                );
+            }
+
             _transferAssetToSender(orders[i].makerAssetData, makerAssetAcquiredAmount);
 
             totalWethSpentAmount = totalWethSpentAmount
@@ -206,7 +232,10 @@ contract MixinExchangeWrapper is
         )
     {
         // No taker fee or WETH fee
-        if (order.takerFee == 0 || order.takerFeeAssetData.equals(order.takerAssetData)) {
+        if (
+            order.takerFee == 0 ||
+            _areUnderlyingAssetsEqual(order.takerFeeAssetData, order.takerAssetData)
+        ) {
             // Calculate the remaining amount of takerAsset to sell
             uint256 remainingTakerAssetFillAmount = LibMath.getPartialAmountCeil(
                 order.takerAssetAmount,
@@ -228,7 +257,7 @@ contract MixinExchangeWrapper is
 
             makerAssetAcquiredAmount = singleFillResults.makerAssetFilledAmount;
         // Percentage fee
-        } else if (_isPercentageFee(order.takerFeeAssetData, order.makerAssetData)) {
+        } else if (_areUnderlyingAssetsEqual(order.takerFeeAssetData, order.makerAssetData)) {
             // Calculate the remaining amount of takerAsset to sell
             uint256 remainingTakerAssetFillAmount = LibMath.getPartialAmountCeil(
                 order.takerAssetAmount,
@@ -277,6 +306,8 @@ contract MixinExchangeWrapper is
             uint256 totalMakerAssetAcquiredAmount
         )
     {
+        bytes4 erc20BridgeProxyId = IAssetData(address(0)).ERC20Bridge.selector;
+
         uint256 ordersLength = orders.length;
         for (uint256 i = 0; i != ordersLength; i++) {
             // Preemptively skip to avoid division by zero in _marketBuySingleOrder
@@ -287,6 +318,15 @@ contract MixinExchangeWrapper is
             uint256 remainingMakerAssetFillAmount = makerAssetBuyAmount
                 .safeSub(totalMakerAssetAcquiredAmount);
 
+            // If the maker asset is ERC20Bridge, take a snapshot of the Forwarder contract's balance.
+            bytes4 makerAssetProxyId = orders[i].makerAssetData.readBytes4(0);
+            address tokenAddress;
+            uint256 balanceBefore;
+            if (makerAssetProxyId == erc20BridgeProxyId) {
+                tokenAddress = orders[i].makerAssetData.readAddress(16);
+                balanceBefore = IERC20Token(tokenAddress).balanceOf(address(this));
+            }
+
             (
                 uint256 wethSpentAmount,
                 uint256 makerAssetAcquiredAmount
@@ -296,6 +336,15 @@ contract MixinExchangeWrapper is
                 remainingMakerAssetFillAmount
             );
 
+            // Account for the ERC20Bridge transfering more of the maker asset than expected.
+            if (makerAssetProxyId == erc20BridgeProxyId) {
+                uint256 balanceAfter = IERC20Token(tokenAddress).balanceOf(address(this));
+                makerAssetAcquiredAmount = LibSafeMath.max256(
+                    balanceAfter.safeSub(balanceBefore),
+                    makerAssetAcquiredAmount
+                );
+            }
+
             _transferAssetToSender(orders[i].makerAssetData, makerAssetAcquiredAmount);
 
             totalWethSpentAmount = totalWethSpentAmount
@@ -317,38 +366,34 @@ contract MixinExchangeWrapper is
         }
     }
 
-    /// @dev Checks whether an order's taker fee is effectively denominated in the maker asset.
+    /// @dev Checks whether one asset is effectively equal to another asset.
-    ///      This is the case if they have the same ERC20Proxy asset data, or if the makerAssetData
+    ///      This is the case if they have the same ERC20Proxy/ERC20BridgeProxy asset data, or if
-    ///      is the ERC20Bridge equivalent of the takerFeeAssetData.
+    ///      one is the ERC20Bridge equivalent of the other.
-    /// @param takerFeeAssetData Byte array encoded for the takerFee asset proxy.
+    /// @param assetData1 Byte array encoded for the takerFee asset proxy.
-    /// @param makerAssetData Byte array encoded for the maker asset proxy.
+    /// @param assetData2 Byte array encoded for the maker asset proxy.
-    /// @return isPercentageFee Whether or not the taker fee asset matches the maker asset.
+    /// @return areEqual Whether or not the underlying assets are equal.
-    function _isPercentageFee(
+    function _areUnderlyingAssetsEqual(
-        bytes memory takerFeeAssetData,
+        bytes memory assetData1,
-        bytes memory makerAssetData
+        bytes memory assetData2
     )
         internal
         pure
         returns (bool)
     {
-        bytes4 takerFeeAssetProxyId = takerFeeAssetData.readBytes4(0);
+        bytes4 assetProxyId1 = assetData1.readBytes4(0);
-        // If the takerFee asset is not ERC20, it cannot be a percentage fee (and will revert with
+        bytes4 assetProxyId2 = assetData2.readBytes4(0);
-        // an UnsupportedFeeError in the calling function).
+        bytes4 erc20ProxyId = IAssetData(address(0)).ERC20Token.selector;
-        if (takerFeeAssetProxyId != ERC20_PROXY_ID) {
+        bytes4 erc20BridgeProxyId = IAssetData(address(0)).ERC20Bridge.selector;
-            return false;
-        }
 
-        bytes4 makerAssetProxyId = makerAssetData.readBytes4(0);
+        if (
-        if (makerAssetProxyId == ERC20_PROXY_ID) {
+            (assetProxyId1 == erc20ProxyId || assetProxyId1 == erc20BridgeProxyId) &&
-            // If the maker asset is ERC20, we can directly compare the asset data.
+            (assetProxyId2 == erc20ProxyId || assetProxyId2 == erc20BridgeProxyId)
-            return takerFeeAssetData.equals(makerAssetData);
+        ) {
-        } else if (makerAssetProxyId == ERC20_BRIDGE_PROXY_ID) {
+            // Compare the underlying token addresses.
-            // If the maker asset is from an ERC20Bridge, compare the underlying token addresses.
+            address token1 = assetData1.readAddress(16);
-            address takerFeeToken = takerFeeAssetData.readAddress(16);
+            address token2 = assetData2.readAddress(16);
-            address makerToken = makerAssetData.readAddress(16);
+            return (token1 == token2);
-            return (takerFeeToken == makerToken);
         } else {
-            // If the maker asset is of any other type, the taker fee cannot be a percentage fee.
             return false;
         }
     }

contracts/exchange-forwarder/contracts/src/MixinForwarderCore.sol

@@ -24,6 +24,7 @@ import "@0x/contracts-utils/contracts/src/LibRichErrors.sol";
 import "@0x/contracts-utils/contracts/src/LibSafeMath.sol";
 import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol";
 import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol";
+import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetData.sol";
 import "./libs/LibConstants.sol";
 import "./libs/LibForwarderRichErrors.sol";
 import "./interfaces/IAssets.sol";
@@ -46,7 +47,7 @@ contract MixinForwarderCore is
     constructor ()
         public
     {
-        address proxyAddress = EXCHANGE.getAssetProxy(ERC20_PROXY_ID);
+        address proxyAddress = EXCHANGE.getAssetProxy(IAssetData(address(0)).ERC20Token.selector);
         if (proxyAddress == address(0)) {
             LibRichErrors.rrevert(LibForwarderRichErrors.UnregisteredAssetProxyError());
         }

contracts/exchange-forwarder/contracts/src/libs/LibConstants.sol

@@ -27,10 +27,6 @@ contract LibConstants {
 
     using LibBytes for bytes;
 
-    bytes4 constant internal ERC20_PROXY_ID = bytes4(keccak256("ERC20Token(address)"));
-    bytes4 constant internal ERC721_PROXY_ID = bytes4(keccak256("ERC721Token(address,uint256)"));
-    bytes4 constant internal ERC20_BRIDGE_PROXY_ID = bytes4(keccak256("ERC20Bridge(address,address,bytes)"));
-
     uint256 constant internal MAX_UINT = 2**256 - 1;
     uint256 constant internal PERCENTAGE_DENOMINATOR = 10**18;
     uint256 constant internal MAX_FEE_PERCENTAGE = 5 * PERCENTAGE_DENOMINATOR / 100;         // 5%

contracts/exchange-forwarder/contracts/test/TestForwarder.sol

@@ -36,16 +36,16 @@ contract TestForwarder is
         )
     {}
 
-    function isPercentageFee(
+    function areUnderlyingAssetsEqual(
-        bytes memory takerFeeAssetData,
+        bytes memory assetData1,
-        bytes memory makerAssetData
+        bytes memory assetData2
     )
         public
         returns (bool)
     {
-        return _isPercentageFee(
+        return _areUnderlyingAssetsEqual(
-            takerFeeAssetData,
+            assetData1,
-            makerAssetData
+            assetData2
         );
     }
 

contracts/exchange-forwarder/test/asset_test.ts

@@ -84,33 +84,43 @@ blockchainTests('Supported asset type unit tests', env => {
             .getABIEncodedTransactionData();
     });
 
-    describe('_isPercentageFee', () => {
+    describe('_areUnderlyingAssetsEqual', () => {
-        it('returns true if takerFeeAssetData == makerAssetData are ERC20', async () => {
+        it('returns true if assetData1 == assetData2 are ERC20', async () => {
-            const result = await forwarder.isPercentageFee(erc20AssetData, erc20AssetData).callAsync();
+            const result = await forwarder.areUnderlyingAssetsEqual(erc20AssetData, erc20AssetData).callAsync();
             expect(result).to.be.true();
         });
-        it('returns true if makerAssetData is the ERC20Bridge equivalent of takerFeeAssetData', async () => {
+        it('returns true if assetData1 == assetData2 are ERC20Bridge', async () => {
-            const result = await forwarder.isPercentageFee(erc20AssetData, erc20BridgeAssetData).callAsync();
+            const result = await forwarder
+                .areUnderlyingAssetsEqual(erc20BridgeAssetData, erc20BridgeAssetData)
+                .callAsync();
             expect(result).to.be.true();
         });
-        it('returns false if takerFeeAssetData != makerAssetData are ERC20', async () => {
+        it('returns true if assetData2 is the ERC20Bridge equivalent of assetData1', async () => {
+            const result = await forwarder.areUnderlyingAssetsEqual(erc20AssetData, erc20BridgeAssetData).callAsync();
+            expect(result).to.be.true();
+        });
+        it('returns false if assetData1 != assetData2 are ERC20', async () => {
             const differentERC20AssetData = assetDataEncoder.ERC20Token(randomAddress()).getABIEncodedTransactionData();
-            const result = await forwarder.isPercentageFee(erc20AssetData, differentERC20AssetData).callAsync();
+            const result = await forwarder
+                .areUnderlyingAssetsEqual(erc20AssetData, differentERC20AssetData)
+                .callAsync();
             expect(result).to.be.false();
         });
-        it('returns false if takerFeeAssetData is ERC20 and makerAssetData is ERC721', async () => {
+        it('returns false if assetData1 is ERC20 and assetData2 is ERC721', async () => {
-            const result = await forwarder.isPercentageFee(erc20AssetData, erc721AssetData).callAsync();
+            const result = await forwarder.areUnderlyingAssetsEqual(erc20AssetData, erc721AssetData).callAsync();
             expect(result).to.be.false();
         });
-        it('returns false if makerAssetData us ERC20Bridge, but for a different token than takerFeeAssetData', async () => {
+        it('returns false if assetData2 is ERC20Bridge, but for a different token than assetData1', async () => {
             const mismatchedErc20BridgeAssetData = assetDataEncoder
                 .ERC20Bridge(randomAddress(), bridgeAddress, bridgeData)
                 .getABIEncodedTransactionData();
-            const result = await forwarder.isPercentageFee(erc20AssetData, mismatchedErc20BridgeAssetData).callAsync();
+            const result = await forwarder
+                .areUnderlyingAssetsEqual(erc20AssetData, mismatchedErc20BridgeAssetData)
+                .callAsync();
             expect(result).to.be.false();
         });
-        it('returns false if takerFeeAssetData is not ERC20', async () => {
+        it('returns false if assetData1 == assetData2 are ERC721', async () => {
-            const result = await forwarder.isPercentageFee(erc721AssetData, erc721AssetData).callAsync();
+            const result = await forwarder.areUnderlyingAssetsEqual(erc721AssetData, erc721AssetData).callAsync();
             expect(result).to.be.false();
         });
     });

contracts/integrations/contracts/test/TestEth2Dai.sol

@@ -26,6 +26,14 @@ import "@0x/contracts-erc20/contracts/test/DummyERC20Token.sol";
 contract TestEth2Dai is
     IEth2Dai
 {
+    uint256 private _excessBuyAmount;
+
+    function setExcessBuyAmount(uint256 amount)
+        external
+    {
+        _excessBuyAmount = amount;
+    }
+
     function sellAllAmount(
         address sellTokenAddress,
         uint256 sellTokenAmount,
@@ -41,11 +49,11 @@ contract TestEth2Dai is
             sellTokenAmount
         );
         DummyERC20Token buyToken = DummyERC20Token(buyTokenAddress);
-        buyToken.mint(minimumFillAmount);
+        buyToken.mint(minimumFillAmount + _excessBuyAmount);
         buyToken.transfer(
             msg.sender,
-            minimumFillAmount
+            minimumFillAmount + _excessBuyAmount
         );
-        return minimumFillAmount;
+        return minimumFillAmount + _excessBuyAmount;
     }
 }

contracts/integrations/contracts/test/TestUniswapExchange.sol

@@ -27,6 +27,7 @@ contract TestUniswapExchange is
     IUniswapExchange
 {
     DummyERC20Token public token;
+    uint256 private _excessBuyAmount;
 
     constructor(address _tokenAddress) public {
         token = DummyERC20Token(_tokenAddress);
@@ -39,6 +40,12 @@ contract TestUniswapExchange is
         payable
     {}
 
+    function setExcessBuyAmount(uint256 amount)
+        external
+    {
+        _excessBuyAmount = amount;
+    }
+
     function ethToTokenTransferInput(
         uint256 minTokensBought,
         uint256, /* deadline */
@@ -48,9 +55,9 @@ contract TestUniswapExchange is
         payable
         returns (uint256 tokensBought)
     {
-        token.mint(minTokensBought);
+        token.mint(minTokensBought + _excessBuyAmount);
-        token.transfer(recipient, minTokensBought);
+        token.transfer(recipient, minTokensBought + _excessBuyAmount);
-        return minTokensBought;
+        return minTokensBought + _excessBuyAmount;
     }
 
     function tokenToEthSwapInput(
@@ -66,8 +73,8 @@ contract TestUniswapExchange is
             address(this),
             tokensSold
         );
-        msg.sender.transfer(minEthBought);
+        msg.sender.transfer(minEthBought + _excessBuyAmount);
-        return minEthBought;
+        return minEthBought + _excessBuyAmount;
     }
 
     function tokenToTokenTransferInput(
@@ -87,9 +94,9 @@ contract TestUniswapExchange is
             tokensSold
         );
         DummyERC20Token toToken = DummyERC20Token(toTokenAddress);
-        toToken.mint(minTokensBought);
+        toToken.mint(minTokensBought + _excessBuyAmount);
-        toToken.transfer(recipient, minTokensBought);
+        toToken.transfer(recipient, minTokensBought + _excessBuyAmount);
-        return minTokensBought;
+        return minTokensBought + _excessBuyAmount;
     }
 
     function toTokenAddress()

contracts/integrations/test/forwarder/bridge_test.ts

@@ -21,17 +21,21 @@ import { Taker } from '../framework/actors/taker';
 import { actorAddressesByName } from '../framework/actors/utils';
 import { BlockchainBalanceStore } from '../framework/balances/blockchain_balance_store';
 import { DeploymentManager } from '../framework/deployment_manager';
+import { TestEth2DaiContract, TestUniswapExchangeContract } from '../wrappers';
 
 import { deployForwarderAsync } from './deploy_forwarder';
 import { ForwarderTestFactory } from './forwarder_test_factory';
 
-blockchainTests.resets('Forwarder <> ERC20Bridge integration tests', env => {
+blockchainTests.resets.only('Forwarder <> ERC20Bridge integration tests', env => {
     let deployment: DeploymentManager;
-    let forwarder: ForwarderContract;
-    let assetDataEncoder: IAssetDataContract;
     let balanceStore: BlockchainBalanceStore;
     let testFactory: ForwarderTestFactory;
 
+    let forwarder: ForwarderContract;
+    let assetDataEncoder: IAssetDataContract;
+    let eth2Dai: TestEth2DaiContract;
+    let uniswapExchange: TestUniswapExchangeContract;
+
     let erc721Token: DummyERC721TokenContract;
     let nftId: BigNumber;
     let makerTokenAssetData: string;
@@ -58,10 +62,12 @@ blockchainTests.resets('Forwarder <> ERC20Bridge integration tests', env => {
         [erc721Token] = deployment.tokens.erc721;
 
         forwarder = await deployForwarderAsync(deployment, env);
-        const [eth2DaiBridge] = await deployEth2DaiBridgeAsync(deployment, env);
+        const eth2DaiContracts = await deployEth2DaiBridgeAsync(deployment, env);
-        const [uniswapBridge, [uniswapMakerTokenExchange]] = await deployUniswapBridgeAsync(deployment, env, [
+        const [eth2DaiBridge] = eth2DaiContracts;
-            makerToken.address,
+        [, eth2Dai] = eth2DaiContracts;
-        ]);
+        const uniswapContracts = await deployUniswapBridgeAsync(deployment, env, [makerToken.address]);
+        const [uniswapBridge] = uniswapContracts;
+        [, [uniswapExchange]] = uniswapContracts;
 
         makerTokenAssetData = assetDataEncoder.ERC20Token(makerToken.address).getABIEncodedTransactionData();
         makerFeeTokenAssetData = assetDataEncoder.ERC20Token(makerFeeToken.address).getABIEncodedTransactionData();
@@ -129,7 +135,7 @@ blockchainTests.resets('Forwarder <> ERC20Bridge integration tests', env => {
         [nftId] = await maker.configureERC721TokenAsync(erc721Token);
 
         // We need to top up the TestUniswapExchange with some ETH so that it can perform tokenToEthSwapInput
-        await uniswapMakerTokenExchange.topUpEth().awaitTransactionSuccessAsync({
+        await uniswapExchange.topUpEth().awaitTransactionSuccessAsync({
             from: forwarderFeeRecipient.address,
             value: constants.ONE_ETHER.times(10),
         });
@@ -160,6 +166,11 @@ blockchainTests.resets('Forwarder <> ERC20Bridge integration tests', env => {
         it('should partially fill a single Eth2DaiBridge order without a taker fee', async () => {
             await testFactory.marketSellTestAsync([eth2DaiBridgeOrder], 0.34);
         });
+        it('should correctly handle excess maker asset acquired from Eth2Dai', async () => {
+            const bridgeExcessBuyAmount = new BigNumber(1);
+            await eth2Dai.setExcessBuyAmount(bridgeExcessBuyAmount).awaitTransactionSuccessAsync();
+            await testFactory.marketSellTestAsync([eth2DaiBridgeOrder], 0.34, { bridgeExcessBuyAmount });
+        });
         it('should fill a single Eth2DaiBridge order with a WETH taker fee', async () => {
             const order = {
                 ...eth2DaiBridgeOrder,
@@ -196,6 +207,11 @@ blockchainTests.resets('Forwarder <> ERC20Bridge integration tests', env => {
         it('should partially fill a single UniswapBridge order without a taker fee', async () => {
             await testFactory.marketSellTestAsync([uniswapBridgeOrder], 0.34);
         });
+        it('should correctly handle excess maker asset acquired from Uniswap', async () => {
+            const bridgeExcessBuyAmount = new BigNumber(1);
+            await uniswapExchange.setExcessBuyAmount(bridgeExcessBuyAmount).awaitTransactionSuccessAsync();
+            await testFactory.marketSellTestAsync([uniswapBridgeOrder], 0.34, { bridgeExcessBuyAmount });
+        });
         it('should fill a single UniswapBridge order with a WETH taker fee', async () => {
             const order = {
                 ...uniswapBridgeOrder,
@@ -249,6 +265,11 @@ blockchainTests.resets('Forwarder <> ERC20Bridge integration tests', env => {
         it('should return excess ETH', async () => {
             await testFactory.marketBuyTestAsync([eth2DaiBridgeOrder], 1, { ethValueAdjustment: 1 });
         });
+        it('should correctly handle excess maker asset acquired from Eth2Dai', async () => {
+            const bridgeExcessBuyAmount = new BigNumber(1);
+            await eth2Dai.setExcessBuyAmount(bridgeExcessBuyAmount).awaitTransactionSuccessAsync();
+            await testFactory.marketBuyTestAsync([eth2DaiBridgeOrder], 0.34, { bridgeExcessBuyAmount });
+        });
         it('should fill a single Eth2DaiBridge order with a WETH taker fee', async () => {
             const order = {
                 ...eth2DaiBridgeOrder,
@@ -295,6 +316,11 @@ blockchainTests.resets('Forwarder <> ERC20Bridge integration tests', env => {
         it('should partially fill a single UniswapBridge order without a taker fee', async () => {
             await testFactory.marketBuyTestAsync([uniswapBridgeOrder], 0.34);
         });
+        it('should correctly handle excess maker asset acquired from Uniswap', async () => {
+            const bridgeExcessBuyAmount = new BigNumber(1);
+            await uniswapExchange.setExcessBuyAmount(bridgeExcessBuyAmount).awaitTransactionSuccessAsync();
+            await testFactory.marketBuyTestAsync([uniswapBridgeOrder], 0.34, { bridgeExcessBuyAmount });
+        });
         it('should fill a single UniswapBridge order with a WETH taker fee', async () => {
             const order = {
                 ...uniswapBridgeOrder,

contracts/integrations/test/forwarder/forwarder_test_factory.ts

@@ -29,28 +29,32 @@ interface ForwarderFillState {
 interface MarketSellOptions {
     forwarderFeePercentage: Numberish;
     revertError: RevertError;
+    bridgeExcessBuyAmount: BigNumber;
 }
 
 interface MarketBuyOptions extends MarketSellOptions {
     ethValueAdjustment: number; // Used to provided insufficient/excess ETH
 }
 
-function isPercentageFee(takerFeeAssetData: string, makerAssetData: string): boolean {
+function areUnderlyingAssetsEqual(assetData1: string, assetData2: string): boolean {
-    const makerAssetProxyId = hexSlice(makerAssetData, 0, 4);
+    const assetProxyId1 = hexSlice(assetData1, 0, 4);
-    const takerFeeAssetProxyId = hexSlice(takerFeeAssetData, 0, 4);
+    const assetProxyId2 = hexSlice(assetData2, 0, 4);
-    if (makerAssetProxyId === AssetProxyId.ERC20Bridge && takerFeeAssetProxyId === AssetProxyId.ERC20) {
+    if (
+        (assetProxyId1 === AssetProxyId.ERC20 || assetProxyId1 === AssetProxyId.ERC20Bridge) &&
+        (assetProxyId2 === AssetProxyId.ERC20 || assetProxyId2 === AssetProxyId.ERC20Bridge)
+    ) {
         const assetDataDecoder = new IAssetDataContract(constants.NULL_ADDRESS, provider);
-        const [makerTokenAddress] = assetDataDecoder.getABIDecodedTransactionData<string>(
+        const tokenAddress1 =
-            'ERC20Bridge',
+            assetProxyId1 === AssetProxyId.ERC20
-            makerAssetData,
+                ? assetDataDecoder.getABIDecodedTransactionData<string>('ERC20Token', assetData1)
-        );
+                : assetDataDecoder.getABIDecodedTransactionData<[string]>('ERC20Bridge', assetData1)[0];
-        const takerFeeTokenAddress = assetDataDecoder.getABIDecodedTransactionData<string>(
+        const tokenAddress2 =
-            'ERC20Token',
+            assetProxyId2 === AssetProxyId.ERC20
-            takerFeeAssetData,
+                ? assetDataDecoder.getABIDecodedTransactionData<string>('ERC20Token', assetData2)
-        );
+                : assetDataDecoder.getABIDecodedTransactionData<[string]>('ERC20Bridge', assetData2)[0];
-        return makerTokenAddress === takerFeeTokenAddress;
+        return tokenAddress2 === tokenAddress1;
     } else {
-        return makerAssetData === takerFeeAssetData;
+        return false;
     }
 }
 
@@ -92,7 +96,7 @@ export class ForwarderTestFactory {
         const tx = this._forwarder
             .marketBuyOrdersWithEth(
                 orders,
-                makerAssetAcquiredAmount,
+                makerAssetAcquiredAmount.minus(options.bridgeExcessBuyAmount || 0),
                 orders.map(signedOrder => signedOrder.signature),
                 feePercentage,
                 this._forwarderFeeRecipient.address,
@@ -208,6 +212,7 @@ export class ForwarderTestFactory {
                 order,
                 ordersInfoBefore[i].orderTakerAssetFilledAmount,
                 Math.min(remainingOrdersToFill, 1),
+                options.bridgeExcessBuyAmount || constants.ZERO_AMOUNT,
             );
             remainingOrdersToFill = Math.max(remainingOrdersToFill - 1, 0);
 
@@ -232,6 +237,7 @@ export class ForwarderTestFactory {
         order: SignedOrder,
         takerAssetFilled: BigNumber,
         fillFraction: number,
+        bridgeExcessBuyAmount: BigNumber,
     ): Promise<ForwarderFillState> {
         let { makerAssetAmount, takerAssetAmount, makerFee, takerFee } = order;
         makerAssetAmount = makerAssetAmount.times(fillFraction).integerValue(BigNumber.ROUND_CEIL);
@@ -251,9 +257,10 @@ export class ForwarderTestFactory {
         const takerFeeFilled = takerAssetFilled.times(order.takerFee).dividedToIntegerBy(order.takerAssetAmount);
         takerFee = BigNumber.max(takerFee.minus(takerFeeFilled), 0);
 
+        makerAssetAmount = makerAssetAmount.plus(bridgeExcessBuyAmount);
         let wethSpentAmount = takerAssetAmount.plus(DeploymentManager.protocolFee);
         let makerAssetAcquiredAmount = makerAssetAmount;
-        if (isPercentageFee(order.takerFeeAssetData, order.makerAssetData)) {
+        if (areUnderlyingAssetsEqual(order.takerFeeAssetData, order.makerAssetData)) {
             makerAssetAcquiredAmount = makerAssetAcquiredAmount.minus(takerFee);
         } else if (order.takerFeeAssetData === order.takerAssetData) {
             wethSpentAmount = wethSpentAmount.plus(takerFee);