Address PR feedback and add support for affiliate fees

contracts/broker/contracts/src/Broker.sol

@@ -19,47 +19,66 @@
 pragma solidity ^0.5.9;
 pragma experimental ABIEncoderV2;
 
+import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetData.sol";
+import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol";
+import "@0x/contracts-erc721/contracts/src/interfaces/IERC721Token.sol";
 import "@0x/contracts-exchange/contracts/src/interfaces/IExchange.sol";
-import "@0x/contracts-exchange-libs/contracts/src/LibAssetDataTransfer.sol";
 import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol";
 import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol";
+import "@0x/contracts-extensions/contracts/src/LibAssetDataTransfer.sol";
+import "@0x/contracts-extensions/contracts/src/MixinWethUtils.sol";
 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-utils/contracts/src/Refundable.sol";
 import "./interfaces/IBroker.sol";
 import "./interfaces/IPropertyValidator.sol";
 import "./libs/LibBrokerRichErrors.sol";
 
 
-// solhint-disable space-after-comma
+// solhint-disable space-after-comma, var-name-mixedcase
 contract Broker is
-    Refundable,
+    IBroker,
-    IBroker
+    MixinWethUtils
 {
-    bytes[] internal _cachedAssetData;
+    // Contract addresses
+
+    // Address of the 0x Exchange contract
+    address internal EXCHANGE;
+    // Address of the 0x ERC1155 Asset Proxy contract
+    address internal ERC1155_PROXY;
+
+    // The following storage variables are used to cache data for the duration of the transcation.
+    // They should always cleared at the end of the transaction.
+
+    // Token IDs specified by the taker to be used to fill property-based orders.
+    uint256[] internal _cachedTokenIds;
+    // An index to the above array keeping track of which assets have been transferred.
     uint256 internal _cacheIndex;
+    // The address that called `brokerTrade` or `batchBrokerTrade`. Assets will be transferred to
+    // and from this address as the effectual taker of the orders.
     address internal _sender;
-    address internal _EXCHANGE; // solhint-disable-line var-name-mixedcase
 
     using LibSafeMath for uint256;
     using LibBytes for bytes;
     using LibAssetDataTransfer for bytes;
 
     /// @param exchange Address of the 0x Exchange contract.
-    constructor (address exchange)
+    /// @param exchange Address of the Wrapped Ether contract.
+    /// @param exchange Address of the 0x ERC1155 Asset Proxy contract.
+    constructor (
+        address exchange,
+        address weth
+    )
         public
+        MixinWethUtils(
+            exchange,
+            weth
+        )
     {
-        _EXCHANGE = exchange;
+        EXCHANGE = exchange;
+        ERC1155_PROXY = IExchange(EXCHANGE).getAssetProxy(IAssetData(address(0)).ERC1155Assets.selector);
     }
 
-    /// @dev A payable fallback function that makes this contract "payable". This is necessary to allow
-    ///      this contract to gracefully handle refunds from the Exchange.
-    function ()
-        external
-        payable
-    {} // solhint-disable-line no-empty-blocks
-
     /// @dev The Broker implements the ERC1155 transfer function to be compatible with the ERC1155 asset proxy
     /// @param from Since the Broker serves as the taker of the order, this should equal `address(this)`
     /// @param to This should be the maker of the order.
@@ -74,79 +93,95 @@ contract Broker is
     )
         external
     {
+        // Only the ERC1155 asset proxy contract should be calling this function.
+        if (msg.sender != ERC1155_PROXY) {
+            LibRichErrors.rrevert(LibBrokerRichErrors.OnlyERC1155ProxyError(
+                msg.sender
+            ));
+        }
+        // Only `takerAssetData` should be using Broker assets
         if (from != address(this)) {
             LibRichErrors.rrevert(
                 LibBrokerRichErrors.InvalidFromAddressError(from)
             );
         }
+        // Only one asset amount should be specified.
         if (amounts.length != 1) {
             LibRichErrors.rrevert(
                 LibBrokerRichErrors.AmountsLengthMustEqualOneError(amounts.length)
             );
         }
+
+        uint256 cacheIndex = _cacheIndex;
         uint256 remainingAmount = amounts[0];
-        if (_cachedAssetData.length.safeSub(_cacheIndex) < remainingAmount) {
+
+        // Verify that there are enough broker assets to transfer
+        if (_cachedTokenIds.length.safeSub(cacheIndex) < remainingAmount) {
             LibRichErrors.rrevert(
-                LibBrokerRichErrors.TooFewBrokerAssetsProvidedError(_cachedAssetData.length)
+                LibBrokerRichErrors.TooFewBrokerAssetsProvidedError(_cachedTokenIds.length)
             );
         }
 
-        while (remainingAmount != 0) {
-            bytes memory assetToTransfer = _cachedAssetData[_cacheIndex];
-            _cacheIndex++;
-
         // Decode validator and params from `data`
-            (address validator, bytes memory propertyData) = abi.decode(
+        (address tokenAddress, address validator, bytes memory propertyData) = abi.decode(
             data,
-                (address, bytes)
+            (address, address, bytes)
         );
 
-            // Execute staticcall
+        while (remainingAmount != 0) {
-            (bool success, bytes memory returnData) = validator.staticcall(abi.encodeWithSelector(
+            uint256 tokenId = _cachedTokenIds[cacheIndex];
-                IPropertyValidator(0).checkBrokerAsset.selector,
+            cacheIndex++;
-                assetToTransfer,
-                propertyData
-            ));
 
-            // Revert with returned data if staticcall is unsuccessful
+            // Validate asset properties
-            if (!success) {
+            IPropertyValidator(validator).checkBrokerAsset(
-                assembly {
+                tokenId,
-                    revert(add(returnData, 32), mload(returnData))
+                propertyData
-                }
+            );
-            }
 
             // Perform the transfer
-            assetToTransfer.transferERC721Token(
+            IERC721Token(tokenAddress).transferFrom(
                 _sender,
                 to,
-                1
+                tokenId
             );
 
             remainingAmount--;
         }
+        // Update cache index in storage
+        _cacheIndex = cacheIndex;
     }
 
     /// @dev Fills a single property-based order by the given amount using the given assets.
-    /// @param brokeredAssets Assets specified by the taker to be used to fill the order.
+    ///      Pays protocol fees using either the ETH supplied by the taker to the transaction or
-    /// @param order The property-based order to fill.
+    ///      WETH acquired from the maker during settlement. The final WETH balance is sent to the taker.
+    /// @param brokeredTokenIds Token IDs specified by the taker to be used to fill the orders.
+    /// @param order The property-based order to fill. The format of a property-based order is the
+    ///        same as that of a normal order, except the takerAssetData. Instaed of specifying a
+    ///        specific ERC721 asset, the takerAssetData should be ERC1155 assetData where the
+    ///        underlying tokenAddress is this contract's address and the desired properties are
+    ///        encoded in the extra data field. Also note that takerFees must be denominated in
+    ///        WETH (or zero).
     /// @param takerAssetFillAmount The amount to fill the order by.
     /// @param signature The maker's signature of the given order.
     /// @param fillFunctionSelector The selector for either `fillOrder` or `fillOrKillOrder`.
+    /// @param ethFeeAmounts Amounts of ETH, denominated in Wei, that are paid to corresponding feeRecipients.
+    /// @param feeRecipients Addresses that will receive ETH when orders are filled.
     /// @return fillResults Amounts filled and fees paid by the maker and taker.
     function brokerTrade(
-        bytes[] memory brokeredAssets,
+        uint256[] memory brokeredTokenIds,
         LibOrder.Order memory order,
         uint256 takerAssetFillAmount,
         bytes memory signature,
-        bytes4 fillFunctionSelector
+        bytes4 fillFunctionSelector,
+        uint256[] memory ethFeeAmounts,
+        address payable[] memory feeRecipients
     )
         public
         payable
-        refundFinalBalance
         returns (LibFillResults.FillResults memory fillResults)
     {
         // Cache the taker-supplied asset data
-        _cachedAssetData = brokeredAssets;
+        _cachedTokenIds = brokeredTokenIds;
         // Cache the sender's address
         _sender = msg.sender;
 
@@ -158,6 +193,8 @@ contract Broker is
             LibBrokerRichErrors.InvalidFunctionSelectorError(fillFunctionSelector);
         }
 
+        // Pay ETH affiliate fees to all feeRecipient addresses
+        _transferEthFeesAndWrapRemaining(ethFeeAmounts, feeRecipients);
 
         // Perform the fill
         bytes memory fillCalldata = abi.encodeWithSelector(
@@ -167,47 +204,59 @@ contract Broker is
             signature
         );
         // solhint-disable-next-line avoid-call-value
-        (bool didSucceed, bytes memory returnData) = _EXCHANGE.call.value(msg.value)(fillCalldata);
+        (bool didSucceed, bytes memory returnData) = EXCHANGE.call(fillCalldata);
         if (didSucceed) {
             fillResults = abi.decode(returnData, (LibFillResults.FillResults));
         } else {
-            assembly {
+            // Re-throw error
-                revert(add(returnData, 32), mload(returnData))
+            LibRichErrors.rrevert(returnData);
-            }
         }
 
         // Transfer maker asset to taker
+        if (!order.makerAssetData.equals(WETH_ASSET_DATA)) {
             order.makerAssetData.transferOut(fillResults.makerAssetFilledAmount);
+        }
 
-        // Clear storage
+        // Refund remaining ETH to msg.sender.
-        delete _cachedAssetData;
+        _transferEthRefund(WETH.balanceOf(address(this)));
-        _cacheIndex = 0;
+
-        _sender = address(0);
+        _clearStorage();
 
         return fillResults;
     }
 
     /// @dev Fills multiple property-based orders by the given amounts using the given assets.
-    /// @param brokeredAssets Assets specified by the taker to be used to fill the orders.
+    ///      Pays protocol fees using either the ETH supplied by the taker to the transaction or
-    /// @param orders The property-based orders to fill.
+    ///      WETH acquired from the maker during settlement. The final WETH balance is sent to the taker.
+    /// @param brokeredTokenIds Token IDs specified by the taker to be used to fill the orders.
+    /// @param orders The property-based orders to fill. The format of a property-based order is the
+    ///        same as that of a normal order, except the takerAssetData. Instaed of specifying a
+    ///        specific ERC721 asset, the takerAssetData should be ERC1155 assetData where the
+    ///        underlying tokenAddress is this contract's address and the desired properties are
+    ///        encoded in the extra data field. Also note that takerFees must be denominated in
+    ///        WETH (or zero).
     /// @param takerAssetFillAmounts The amounts to fill the orders by.
     /// @param signatures The makers' signatures for the given orders.
-    /// @param batchFillFunctionSelector The selector for either `batchFillOrders`, `batchFillOrKillOrders`, or `batchFillOrdersNoThrow`.
+    /// @param batchFillFunctionSelector The selector for either `batchFillOrders`,
+    ///        `batchFillOrKillOrders`, or `batchFillOrdersNoThrow`.
+    /// @param ethFeeAmounts Amounts of ETH, denominated in Wei, that are paid to corresponding feeRecipients.
+    /// @param feeRecipients Addresses that will receive ETH when orders are filled.
     /// @return fillResults Amounts filled and fees paid by the makers and taker.
     function batchBrokerTrade(
-        bytes[] memory brokeredAssets,
+        uint256[] memory brokeredTokenIds,
         LibOrder.Order[] memory orders,
         uint256[] memory takerAssetFillAmounts,
         bytes[] memory signatures,
-        bytes4 batchFillFunctionSelector
+        bytes4 batchFillFunctionSelector,
+        uint256[] memory ethFeeAmounts,
+        address payable[] memory feeRecipients
     )
         public
         payable
-        refundFinalBalance
         returns (LibFillResults.FillResults[] memory fillResults)
     {
         // Cache the taker-supplied asset data
-        _cachedAssetData = brokeredAssets;
+        _cachedTokenIds = brokeredTokenIds;
         // Cache the sender's address
         _sender = msg.sender;
 
@@ -220,6 +269,9 @@ contract Broker is
             LibBrokerRichErrors.InvalidFunctionSelectorError(batchFillFunctionSelector);
         }
 
+        // Pay ETH affiliate fees to all feeRecipient addresses
+        _transferEthFeesAndWrapRemaining(ethFeeAmounts, feeRecipients);
+
         // Perform the batch fill
         bytes memory batchFillCalldata = abi.encodeWithSelector(
             batchFillFunctionSelector,
@@ -228,26 +280,35 @@ contract Broker is
             signatures
         );
         // solhint-disable-next-line avoid-call-value
-        (bool didSucceed, bytes memory returnData) = _EXCHANGE.call.value(msg.value)(batchFillCalldata);
+        (bool didSucceed, bytes memory returnData) = EXCHANGE.call(batchFillCalldata);
         if (didSucceed) {
             // solhint-disable-next-line indent
             fillResults = abi.decode(returnData, (LibFillResults.FillResults[]));
         } else {
-            assembly {
+            // Re-throw error
-                revert(add(returnData, 32), mload(returnData))
+            LibRichErrors.rrevert(returnData);
-            }
         }
 
         // Transfer maker assets to taker
         for (uint256 i = 0; i < orders.length; i++) {
+            if (!orders[i].makerAssetData.equals(WETH_ASSET_DATA)) {
                 orders[i].makerAssetData.transferOut(fillResults[i].makerAssetFilledAmount);
             }
+        }
 
-        // Clear storage
+        // Refund remaining ETH to msg.sender.
-        delete _cachedAssetData;
+        _transferEthRefund(WETH.balanceOf(address(this)));
-        _cacheIndex = 0;
+
-        _sender = address(0);
+        _clearStorage();
 
         return fillResults;
     }
+
+    function _clearStorage()
+        private
+    {
+        delete _cachedTokenIds;
+        _cacheIndex = 0;
+        _sender = address(0);
+    }
 }

contracts/broker/contracts/src/interfaces/IBroker.sol

@@ -27,36 +27,59 @@ import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol";
 interface IBroker {
 
     /// @dev Fills a single property-based order by the given amount using the given assets.
-    /// @param brokeredAssets Assets specified by the taker to be used to fill the order.
+    ///      Pays protocol fees using either the ETH supplied by the taker to the transaction or
-    /// @param order The property-based order to fill.
+    ///      WETH acquired from the maker during settlement. The final WETH balance is sent to the taker.
+    /// @param brokeredTokenIds Token IDs specified by the taker to be used to fill the orders.
+    /// @param order The property-based order to fill. The format of a property-based order is the
+    ///        same as that of a normal order, except the takerAssetData. Instaed of specifying a
+    ///        specific ERC721 asset, the takerAssetData should be ERC1155 assetData where the
+    ///        underlying tokenAddress is this contract's address and the desired properties are
+    ///        encoded in the extra data field. Also note that takerFees must be denominated in
+    ///        WETH (or zero).
     /// @param takerAssetFillAmount The amount to fill the order by.
     /// @param signature The maker's signature of the given order.
     /// @param fillFunctionSelector The selector for either `fillOrder` or `fillOrKillOrder`.
+    /// @param ethFeeAmounts Amounts of ETH, denominated in Wei, that are paid to corresponding feeRecipients.
+    /// @param feeRecipients Addresses that will receive ETH when orders are filled.
     /// @return fillResults Amounts filled and fees paid by the maker and taker.
     function brokerTrade(
-        bytes[] calldata brokeredAssets,
+        uint256[] calldata brokeredTokenIds,
         LibOrder.Order calldata order,
         uint256 takerAssetFillAmount,
         bytes calldata signature,
-        bytes4 fillFunctionSelector
+        bytes4 fillFunctionSelector,
+        uint256[] calldata ethFeeAmounts,
+        address payable[] calldata feeRecipients
     )
         external
         payable
         returns (LibFillResults.FillResults memory fillResults);
 
     /// @dev Fills multiple property-based orders by the given amounts using the given assets.
-    /// @param brokeredAssets Assets specified by the taker to be used to fill the orders.
+    ///      Pays protocol fees using either the ETH supplied by the taker to the transaction or
-    /// @param orders The property-based orders to fill.
+    ///      WETH acquired from the maker during settlement. The final WETH balance is sent to the taker.
+    /// @param brokeredTokenIds Token IDs specified by the taker to be used to fill the orders.
+    /// @param orders The property-based orders to fill. The format of a property-based order is the
+    ///        same as that of a normal order, except the takerAssetData. Instaed of specifying a
+    ///        specific ERC721 asset, the takerAssetData should be ERC1155 assetData where the
+    ///        underlying tokenAddress is this contract's address and the desired properties are
+    ///        encoded in the extra data field. Also note that takerFees must be denominated in
+    ///        WETH (or zero).
     /// @param takerAssetFillAmounts The amounts to fill the orders by.
     /// @param signatures The makers' signatures for the given orders.
-    /// @param batchFillFunctionSelector The selector for either `batchFillOrders`, `batchFillOrKillOrders`, or `batchFillOrdersNoThrow`.
+    /// @param batchFillFunctionSelector The selector for either `batchFillOrders`,
+    ///        `batchFillOrKillOrders`, or `batchFillOrdersNoThrow`.
+    /// @param ethFeeAmounts Amounts of ETH, denominated in Wei, that are paid to corresponding feeRecipients.
+    /// @param feeRecipients Addresses that will receive ETH when orders are filled.
     /// @return fillResults Amounts filled and fees paid by the makers and taker.
     function batchBrokerTrade(
-        bytes[] calldata brokeredAssets,
+        uint256[] calldata brokeredTokenIds,
         LibOrder.Order[] calldata orders,
         uint256[] calldata takerAssetFillAmounts,
         bytes[] calldata signatures,
-        bytes4 batchFillFunctionSelector
+        bytes4 batchFillFunctionSelector,
+        uint256[] calldata ethFeeAmounts,
+        address payable[] calldata feeRecipients
     )
         external
         payable

contracts/broker/contracts/src/interfaces/IPropertyValidator.sol

@@ -23,10 +23,11 @@ pragma experimental ABIEncoderV2;
 interface IPropertyValidator {
 
     /// @dev Checks that the given asset data satisfies the properties encoded in `propertyData`.
-    /// @param assetData The encoded asset to check.
+    ///      Should revert if the asset does not satisfy the specified properties.
+    /// @param tokenId The ERC721 tokenId of the asset to check.
     /// @param propertyData Encoded properties or auxiliary data needed to perform the check.
     function checkBrokerAsset(
-        bytes calldata assetData,
+        uint256 tokenId,
         bytes calldata propertyData
     )
         external

contracts/broker/contracts/src/libs/LibBrokerRichErrors.sol

@@ -21,7 +21,6 @@ pragma solidity ^0.5.9;
 
 library LibBrokerRichErrors {
 
-
     // bytes4(keccak256("InvalidFromAddressError(address)"))
     bytes4 internal constant INVALID_FROM_ADDRESS_ERROR_SELECTOR =
         0x906bfb3c;
@@ -38,6 +37,10 @@ library LibBrokerRichErrors {
     bytes4 internal constant INVALID_FUNCTION_SELECTOR_ERROR_SELECTOR =
         0x540943f1;
 
+    // bytes4(keccak256("OnlyERC1155ProxyError(address)"))
+    bytes4 internal constant ONLY_ERC_1155_PROXY_ERROR_SELECTOR =
+        0xccc529af;
+
     // solhint-disable func-name-mixedcase
     function InvalidFromAddressError(
         address from
@@ -90,4 +93,17 @@ library LibBrokerRichErrors {
             selector
         );
     }
+
+    function OnlyERC1155ProxyError(
+        address sender
+    )
+        internal
+        pure
+        returns (bytes memory)
+    {
+        return abi.encodeWithSelector(
+            ONLY_ERC_1155_PROXY_ERROR_SELECTOR,
+            sender
+        );
+    }
 }

contracts/broker/contracts/src/validators/GodsUnchainedValidator.sol

@@ -38,10 +38,11 @@ contract GodsUnchainedValidator is
     }
 
     /// @dev Checks that the given card (encoded as assetData) has the proto and quality encoded in `propertyData`.
-    /// @param assetData The card (encoded as ERC721 assetData) to check.
+    ///      Reverts if the card doesn't match the specified proto and quality.
+    /// @param tokenId The ERC721 tokenId of the card to check.
     /// @param propertyData Encoded proto and quality that the card is expected to have.
     function checkBrokerAsset(
-        bytes calldata assetData,
+        uint256 tokenId,
         bytes calldata propertyData
     )
         external
@@ -52,13 +53,9 @@ contract GodsUnchainedValidator is
             (uint16, uint8)
         );
 
-        // Decode and validate asset data.
+        // Validate card properties.
-        address token = assetData.readAddress(16);
-        require(token == address(GODS_UNCHAINED), "TOKEN_ADDRESS_MISMATCH");
-        uint256 tokenId = assetData.readUint256(36);
-
         (uint16 proto, uint8 quality) = GODS_UNCHAINED.getDetails(tokenId);
-        require(proto == expectedProto, "PROTO_MISMATCH");
+        require(proto == expectedProto, "GodsUnchainedValidator/PROTO_MISMATCH");
-        require(quality == expectedQuality, "QUALITY_MISMATCH");
+        require(quality == expectedQuality, "GodsUnchainedValidator/QUALITY_MISMATCH");
     }
 }

contracts/extensions/contracts/src/MixinWethUtils.sol

@@ -32,6 +32,7 @@ contract MixinWethUtils {
 
      // solhint-disable var-name-mixedcase
     IEtherToken internal WETH;
+    bytes internal WETH_ASSET_DATA;
     // solhint-enable var-name-mixedcase
 
     using LibSafeMath for uint256;
@@ -43,6 +44,10 @@ contract MixinWethUtils {
         public
     {
         WETH = IEtherToken(weth);
+        WETH_ASSET_DATA = abi.encodeWithSelector(
+            IAssetData(address(0)).ERC20Token.selector,
+            weth
+        );
 
         address proxyAddress = IExchange(exchange).getAssetProxy(IAssetData(address(0)).ERC20Token.selector);
         if (proxyAddress == address(0)) {

contracts/integrations/test/broker/broker_test.ts

@@ -21,7 +21,7 @@ import { BlockchainBalanceStore } from '../framework/balances/blockchain_balance
 import { LocalBalanceStore } from '../framework/balances/local_balance_store';
 import { DeploymentManager } from '../framework/deployment_manager';
 
-blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
+blockchainTests.resets.only('Broker <> Gods Unchained integration tests', env => {
     let deployment: DeploymentManager;
     let balanceStore: BlockchainBalanceStore;
     let initialBalances: LocalBalanceStore;
@@ -34,7 +34,6 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
     let validator: GodsUnchainedValidatorContract;
 
     let godsUnchainedTokenIds: BigNumber[];
-    let erc721AssetData: string[];
     const makerSpecifiedProto = new BigNumber(1337);
     const makerSpecifiedQuality = new BigNumber(25);
 
@@ -69,10 +68,12 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             env.txDefaults,
             BrokerArtifacts,
             deployment.exchange.address,
+            deployment.tokens.weth.address,
         );
 
         const takerAssetData = godsUnchainedUtils.encodeBrokerAssetData(
             broker.address,
+            godsUnchained.address,
             validator.address,
             makerSpecifiedProto,
             makerSpecifiedQuality,
@@ -102,9 +103,6 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             broker.address,
             5,
         );
-        erc721AssetData = godsUnchainedTokenIds.map(tokenId =>
-            assetDataUtils.encodeERC721AssetData(godsUnchained.address, tokenId),
-        );
 
         const tokenOwners = {
             Maker: maker.address,
@@ -129,7 +127,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
     });
 
     function simulateBrokerFills(
-        brokeredAssets: string[],
+        brokeredAssets: BigNumber[],
         orders: SignedOrder[],
         takerAssetFillAmounts: BigNumber[],
         receipt: TransactionReceiptWithDecodedLogs,
@@ -139,7 +137,8 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
         expectedBalances.burnGas(receipt.from, DeploymentManager.gasPrice.times(receipt.gasUsed));
         // Taker -> Maker
         for (const brokeredAsset of brokeredAssets) {
-            expectedBalances.transferAsset(taker.address, maker.address, new BigNumber(1), brokeredAsset);
+            const erc721AssetData = assetDataUtils.encodeERC721AssetData(godsUnchained.address, brokeredAsset);
+            expectedBalances.transferAsset(taker.address, maker.address, new BigNumber(1), erc721AssetData);
         }
         // Maker -> Taker
         for (const [i, order] of orders.entries()) {
@@ -156,6 +155,11 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             deployment.staking.stakingProxy.address,
             DeploymentManager.protocolFee.times(orders.length),
         );
+        expectedBalances.wrapEth(
+            deployment.staking.stakingProxy.address,
+            deployment.tokens.weth.address,
+            DeploymentManager.protocolFee.times(orders.length),
+        );
         return expectedBalances;
     }
 
@@ -178,11 +182,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with one valid asset`, async () => {
                 const receipt = await broker
                     .brokerTrade(
-                        [erc721AssetData[0]],
+                        [godsUnchainedTokenIds[0]],
                         order,
                         new BigNumber(1),
                         order.signature,
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -190,7 +196,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
                         gasPrice: DeploymentManager.gasPrice,
                     });
                 const expectedBalances = simulateBrokerFills(
-                    [erc721AssetData[0]],
+                    [godsUnchainedTokenIds[0]],
                     [order],
                     [new BigNumber(1)],
                     receipt,
@@ -201,11 +207,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with two valid assets`, async () => {
                 const receipt = await broker
                     .brokerTrade(
-                        [erc721AssetData[0], erc721AssetData[1]],
+                        [godsUnchainedTokenIds[0], godsUnchainedTokenIds[1]],
                         order,
                         new BigNumber(2),
                         order.signature,
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -214,7 +222,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
                     });
 
                 const expectedBalances = simulateBrokerFills(
-                    [erc721AssetData[0], erc721AssetData[1]],
+                    [godsUnchainedTokenIds[0], godsUnchainedTokenIds[1]],
                     [order],
                     [new BigNumber(2)],
                     receipt,
@@ -225,11 +233,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with one invalid asset`, async () => {
                 const tx = broker
                     .brokerTrade(
-                        [erc721AssetData[2]],
+                        [godsUnchainedTokenIds[2]],
                         order,
                         new BigNumber(1),
                         order.signature,
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -241,11 +251,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with one valid asset, one invalid asset`, async () => {
                 const tx = broker
                     .brokerTrade(
-                        [erc721AssetData[0], erc721AssetData[2]], // valid, invalid
+                        [godsUnchainedTokenIds[0], godsUnchainedTokenIds[2]], // valid, invalid
                         order,
                         new BigNumber(2),
                         order.signature,
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -257,11 +269,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with too few assets`, async () => {
                 const tx = broker
                     .brokerTrade(
-                        [erc721AssetData[0]], // One asset provided
+                        [godsUnchainedTokenIds[0]], // One asset provided
                         order,
                         new BigNumber(2), // But two are required for the fill
                         order.signature,
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -273,11 +287,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with same asset twice`, async () => {
                 const tx = broker
                     .brokerTrade(
-                        [erc721AssetData[0], erc721AssetData[0]],
+                        [godsUnchainedTokenIds[0], godsUnchainedTokenIds[0]],
                         order,
                         new BigNumber(2),
                         order.signature,
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -289,11 +305,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with excess assets`, async () => {
                 const receipt = await broker
                     .brokerTrade(
-                        erc721AssetData,
+                        godsUnchainedTokenIds,
                         order,
                         new BigNumber(2),
                         order.signature,
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -302,7 +320,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
                     });
 
                 const expectedBalances = simulateBrokerFills(
-                    [erc721AssetData[0], erc721AssetData[1]], // 3rd card isn't transferred
+                    [godsUnchainedTokenIds[0], godsUnchainedTokenIds[1]], // 3rd card isn't transferred
                     [order],
                     [new BigNumber(2)],
                     receipt,
@@ -314,11 +332,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
         it(`Reverts if insufficient ETH is provided`, async () => {
             const tx = broker
                 .brokerTrade(
-                    [erc721AssetData[0]],
+                    [godsUnchainedTokenIds[0]],
                     order,
                     new BigNumber(1),
                     order.signature,
                     deployment.exchange.getSelector(ExchangeFunctionName.FillOrder),
+                    [],
+                    [],
                 )
                 .awaitTransactionSuccessAsync({
                     from: taker.address,
@@ -330,18 +350,25 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
         it(`Refunds sender if excess ETH is provided`, async () => {
             const receipt = await broker
                 .brokerTrade(
-                    [erc721AssetData[0]],
+                    [godsUnchainedTokenIds[0]],
                     order,
                     new BigNumber(1),
                     order.signature,
                     deployment.exchange.getSelector(ExchangeFunctionName.FillOrder),
+                    [],
+                    [],
                 )
                 .awaitTransactionSuccessAsync({
                     from: taker.address,
                     value: DeploymentManager.protocolFee.plus(1), // 1 wei gets refunded
                     gasPrice: DeploymentManager.gasPrice,
                 });
-            const expectedBalances = simulateBrokerFills([erc721AssetData[0]], [order], [new BigNumber(1)], receipt);
+            const expectedBalances = simulateBrokerFills(
+                [godsUnchainedTokenIds[0]],
+                [order],
+                [new BigNumber(1)],
+                receipt,
+            );
             await balanceStore.updateBalancesAsync();
             balanceStore.assertEquals(expectedBalances);
         });
@@ -376,6 +403,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
                 await maker.signOrderAsync({
                     takerAssetData: godsUnchainedUtils.encodeBrokerAssetData(
                         broker.address,
+                        godsUnchained.address,
                         validator.address,
                         firstOrderProto,
                         firstOrderQuality,
@@ -384,6 +412,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
                 await maker.signOrderAsync({
                     takerAssetData: godsUnchainedUtils.encodeBrokerAssetData(
                         broker.address,
+                        godsUnchained.address,
                         validator.address,
                         secondOrderProto,
                         secondOrderQuality,
@@ -396,11 +425,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with one order, one valid asset`, async () => {
                 const receipt = await broker
                     .batchBrokerTrade(
-                        [erc721AssetData[0]],
+                        [godsUnchainedTokenIds[0]],
                         [orders[0]],
                         [new BigNumber(1)],
                         [orders[0].signature],
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -409,7 +440,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
                     });
 
                 const expectedBalances = simulateBrokerFills(
-                    [erc721AssetData[0]],
+                    [godsUnchainedTokenIds[0]],
                     [orders[0]],
                     [new BigNumber(1)],
                     receipt,
@@ -420,11 +451,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with two orders, one valid asset each`, async () => {
                 const receipt = await broker
                     .batchBrokerTrade(
-                        [erc721AssetData[0], erc721AssetData[2]], // valid for 1st order, valid for 2nd
+                        [godsUnchainedTokenIds[0], godsUnchainedTokenIds[2]], // valid for 1st order, valid for 2nd
                         orders,
                         [new BigNumber(1), new BigNumber(1)],
                         [orders[0].signature, orders[1].signature],
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -433,7 +466,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
                     });
 
                 const expectedBalances = simulateBrokerFills(
-                    [erc721AssetData[0], erc721AssetData[2]],
+                    [godsUnchainedTokenIds[0], godsUnchainedTokenIds[2]],
                     orders,
                     [new BigNumber(1), new BigNumber(1)],
                     receipt,
@@ -444,11 +477,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with two orders, two valid assets each`, async () => {
                 const receipt = await broker
                     .batchBrokerTrade(
-                        erc721AssetData.slice(0, 4),
+                        godsUnchainedTokenIds.slice(0, 4),
                         orders,
                         [new BigNumber(2), new BigNumber(2)],
                         [orders[0].signature, orders[1].signature],
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -457,7 +492,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
                     });
 
                 const expectedBalances = simulateBrokerFills(
-                    erc721AssetData.slice(0, 4),
+                    godsUnchainedTokenIds.slice(0, 4),
                     orders,
                     [new BigNumber(2), new BigNumber(2)],
                     receipt,
@@ -468,11 +503,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
             it(`${fnName} with two orders, two valid assets each + excess asset`, async () => {
                 const receipt = await broker
                     .batchBrokerTrade(
-                        erc721AssetData,
+                        godsUnchainedTokenIds,
                         orders,
                         [new BigNumber(2), new BigNumber(2)],
                         [orders[0].signature, orders[1].signature],
                         deployment.exchange.getSelector(fnName),
+                        [],
+                        [],
                     )
                     .awaitTransactionSuccessAsync({
                         from: taker.address,
@@ -481,7 +518,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
                     });
 
                 const expectedBalances = simulateBrokerFills(
-                    erc721AssetData.slice(0, 4), // 5th card isn't transferred
+                    godsUnchainedTokenIds.slice(0, 4), // 5th card isn't transferred
                     orders,
                     [new BigNumber(2), new BigNumber(2)],
                     receipt,
@@ -493,11 +530,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
         it(`batchFillOrders reverts on invalid asset`, async () => {
             const tx = broker
                 .batchBrokerTrade(
-                    [...erc721AssetData.slice(0, 3), erc721AssetData[4]], // Last card isn't valid for 2nd order
+                    [...godsUnchainedTokenIds.slice(0, 3), godsUnchainedTokenIds[4]], // Last card isn't valid for 2nd order
                     orders,
                     [new BigNumber(2), new BigNumber(2)],
                     [orders[0].signature, orders[1].signature],
                     deployment.exchange.getSelector(ExchangeFunctionName.BatchFillOrders),
+                    [],
+                    [],
                 )
                 .awaitTransactionSuccessAsync({
                     from: taker.address,
@@ -509,11 +548,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
         it(`batchFillOrKillOrders reverts on invalid asset`, async () => {
             const tx = broker
                 .batchBrokerTrade(
-                    [...erc721AssetData.slice(0, 3), erc721AssetData[4]], // Last card isn't valid for 2nd order
+                    [...godsUnchainedTokenIds.slice(0, 3), godsUnchainedTokenIds[4]], // Last card isn't valid for 2nd order
                     orders,
                     [new BigNumber(2), new BigNumber(2)],
                     [orders[0].signature, orders[1].signature],
                     deployment.exchange.getSelector(ExchangeFunctionName.BatchFillOrKillOrders),
+                    [],
+                    [],
                 )
                 .awaitTransactionSuccessAsync({
                     from: taker.address,
@@ -525,11 +566,13 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
         it(`batchFillOrdersNoThrow catches revert on invalid asset`, async () => {
             const receipt = await broker
                 .batchBrokerTrade(
-                    [...erc721AssetData.slice(0, 3), erc721AssetData[4]], // Last card isn't valid for 2nd order
+                    [...godsUnchainedTokenIds.slice(0, 3), godsUnchainedTokenIds[4]], // Last card isn't valid for 2nd order
                     orders,
                     [new BigNumber(2), new BigNumber(2)],
                     [orders[0].signature, orders[1].signature],
                     deployment.exchange.getSelector(ExchangeFunctionName.BatchFillOrdersNoThrow),
+                    [],
+                    [],
                 )
                 .awaitTransactionSuccessAsync({
                     from: taker.address,
@@ -537,7 +580,7 @@ blockchainTests.resets('Broker <> Gods Unchained integration tests', env => {
                     gasPrice: DeploymentManager.gasPrice,
                 });
             const expectedBalances = simulateBrokerFills(
-                erc721AssetData.slice(0, 2), // First order gets filled
+                godsUnchainedTokenIds.slice(0, 2), // First order gets filled
                 [orders[0]],
                 [new BigNumber(2)],
                 receipt,

packages/utils/src/revert_errors/broker/revert_errors.ts

@@ -31,11 +31,18 @@ export class InvalidFunctionSelectorError extends RevertError {
     }
 }
 
+export class OnlyERC1155ProxyError extends RevertError {
+    constructor(sender?: string) {
+        super('OnlyERC1155ProxyError', 'OnlyERC1155ProxyError(address sender)', { sender });
+    }
+}
+
 const types = [
     InvalidFromAddressError,
     AmountsLengthMustEqualOneError,
     TooFewBrokerAssetsProvidedError,
     InvalidFunctionSelectorError,
+    OnlyERC1155ProxyError,
 ];
 
 // Register the types we've defined.