diff --git a/contracts/broker/contracts/src/Broker.sol b/contracts/broker/contracts/src/Broker.sol index 6cbd9357af..3b3c3632ff 100644 --- a/contracts/broker/contracts/src/Broker.sol +++ b/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 tokenAddress, address validator, bytes memory propertyData) = abi.decode( + data, + (address, address, bytes) + ); - // Decode validator and params from `data` - (address validator, bytes memory propertyData) = abi.decode( - data, - (address, bytes) + while (remainingAmount != 0) { + uint256 tokenId = _cachedTokenIds[cacheIndex]; + cacheIndex++; + + // Validate asset properties + IPropertyValidator(validator).checkBrokerAsset( + tokenId, + propertyData ); - // Execute staticcall - (bool success, bytes memory returnData) = validator.staticcall(abi.encodeWithSelector( - IPropertyValidator(0).checkBrokerAsset.selector, - assetToTransfer, - propertyData - )); - - // Revert with returned data if staticcall is unsuccessful - if (!success) { - assembly { - revert(add(returnData, 32), mload(returnData)) - } - } - // 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. - /// @param order The property-based order to fill. + /// Pays protocol fees using either the ETH supplied by the taker to the transaction or + /// 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 { - revert(add(returnData, 32), mload(returnData)) - } + // Re-throw error + LibRichErrors.rrevert(returnData); } // Transfer maker asset to taker - order.makerAssetData.transferOut(fillResults.makerAssetFilledAmount); + if (!order.makerAssetData.equals(WETH_ASSET_DATA)) { + order.makerAssetData.transferOut(fillResults.makerAssetFilledAmount); + } - // Clear storage - delete _cachedAssetData; - _cacheIndex = 0; - _sender = address(0); + // Refund remaining ETH to msg.sender. + _transferEthRefund(WETH.balanceOf(address(this))); + + _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. - /// @param orders The property-based orders to fill. + /// Pays protocol fees using either the ETH supplied by the taker to the transaction or + /// 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 { - revert(add(returnData, 32), mload(returnData)) - } + // Re-throw error + LibRichErrors.rrevert(returnData); } // Transfer maker assets to taker for (uint256 i = 0; i < orders.length; i++) { - orders[i].makerAssetData.transferOut(fillResults[i].makerAssetFilledAmount); + if (!orders[i].makerAssetData.equals(WETH_ASSET_DATA)) { + orders[i].makerAssetData.transferOut(fillResults[i].makerAssetFilledAmount); + } } - // Clear storage - delete _cachedAssetData; - _cacheIndex = 0; - _sender = address(0); + // Refund remaining ETH to msg.sender. + _transferEthRefund(WETH.balanceOf(address(this))); + + _clearStorage(); return fillResults; } + + function _clearStorage() + private + { + delete _cachedTokenIds; + _cacheIndex = 0; + _sender = address(0); + } } diff --git a/contracts/broker/contracts/src/interfaces/IBroker.sol b/contracts/broker/contracts/src/interfaces/IBroker.sol index ed9e9a77ee..10bf8e7a97 100644 --- a/contracts/broker/contracts/src/interfaces/IBroker.sol +++ b/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. - /// @param order The property-based order to fill. + /// Pays protocol fees using either the ETH supplied by the taker to the transaction or + /// 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. - /// @param orders The property-based orders to fill. + /// Pays protocol fees using either the ETH supplied by the taker to the transaction or + /// 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 diff --git a/contracts/broker/contracts/src/interfaces/IPropertyValidator.sol b/contracts/broker/contracts/src/interfaces/IPropertyValidator.sol index d8e6e07d2d..5316d3934d 100644 --- a/contracts/broker/contracts/src/interfaces/IPropertyValidator.sol +++ b/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 diff --git a/contracts/broker/contracts/src/libs/LibBrokerRichErrors.sol b/contracts/broker/contracts/src/libs/LibBrokerRichErrors.sol index fa2c9322d3..7963fa637c 100644 --- a/contracts/broker/contracts/src/libs/LibBrokerRichErrors.sol +++ b/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 + ); + } } diff --git a/contracts/broker/contracts/src/validators/GodsUnchainedValidator.sol b/contracts/broker/contracts/src/validators/GodsUnchainedValidator.sol index 9f986af2ea..1b4e6e8ddf 100644 --- a/contracts/broker/contracts/src/validators/GodsUnchainedValidator.sol +++ b/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. - address token = assetData.readAddress(16); - require(token == address(GODS_UNCHAINED), "TOKEN_ADDRESS_MISMATCH"); - uint256 tokenId = assetData.readUint256(36); - + // Validate card properties. (uint16 proto, uint8 quality) = GODS_UNCHAINED.getDetails(tokenId); - require(proto == expectedProto, "PROTO_MISMATCH"); - require(quality == expectedQuality, "QUALITY_MISMATCH"); + require(proto == expectedProto, "GodsUnchainedValidator/PROTO_MISMATCH"); + require(quality == expectedQuality, "GodsUnchainedValidator/QUALITY_MISMATCH"); } } diff --git a/contracts/extensions/contracts/src/MixinWethUtils.sol b/contracts/extensions/contracts/src/MixinWethUtils.sol index 66d1983ce6..c7676c6bd8 100644 --- a/contracts/extensions/contracts/src/MixinWethUtils.sol +++ b/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)) { diff --git a/contracts/integrations/test/broker/broker_test.ts b/contracts/integrations/test/broker/broker_test.ts index fe7bc632f1..c66f8f428a 100644 --- a/contracts/integrations/test/broker/broker_test.ts +++ b/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, diff --git a/packages/utils/src/revert_errors/broker/revert_errors.ts b/packages/utils/src/revert_errors/broker/revert_errors.ts index e551454858..1e634813d4 100644 --- a/packages/utils/src/revert_errors/broker/revert_errors.ts +++ b/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.