diff options
Diffstat (limited to 'packages')
8 files changed, 484 insertions, 412 deletions
diff --git a/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol b/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol index d80e06350..e230767b7 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol @@ -100,12 +100,15 @@ contract MixinExchangeWrapper is internal returns (FillResults memory totalFillResults) { + bytes memory makerAssetData = orders[0].makerAssetData; bytes memory wethAssetData = WETH_ASSET_DATA; uint256 ordersLength = orders.length; for (uint256 i = 0; i < ordersLength; i++) { + // We assume that asset being bought by taker is the same for each order. // We assume that asset being sold by taker is WETH for each order. + orders[i].makerAssetData = makerAssetData; orders[i].takerAssetData = wethAssetData; // Calculate the remaining amount of WETH to sell @@ -231,7 +234,7 @@ contract MixinExchangeWrapper is // Attempt to sell the remaining amount of WETH. FillResults memory singleFillResult = fillOrderNoThrow( orders[i], - safeAdd(remainingWethSellAmount, 1), + safeAdd(remainingWethSellAmount, 1), // we add 1 wei to the fill amount to make up for rounding errors signatures[i] ); diff --git a/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol b/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol index a50863a59..8ba236e7f 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol @@ -87,7 +87,7 @@ contract MixinWeth is // Ensure fee is less than amount of WETH remaining. require( ethFee <= wethRemaining, - "MAX_FEE_EXCEEDED" + "INSUFFICIENT_ETH_REMAINING" ); // Do nothing if no WETH remaining diff --git a/packages/contracts/src/2.0.0/forwarder/libs/LibConstants.sol b/packages/contracts/src/2.0.0/forwarder/libs/LibConstants.sol index 8abe1e42d..c26d7902c 100644 --- a/packages/contracts/src/2.0.0/forwarder/libs/LibConstants.sol +++ b/packages/contracts/src/2.0.0/forwarder/libs/LibConstants.sol @@ -26,7 +26,7 @@ import "../../tokens/ERC20Token/IERC20Token.sol"; contract LibConstants { bytes4 constant internal ERC20_DATA_ID = bytes4(keccak256("ERC20Token(address)")); - bytes4 constant internal ERC721_DATA_ID = bytes4(keccak256("ERC721Token(address,uint256,bytes)")); + bytes4 constant internal ERC721_DATA_ID = bytes4(keccak256("ERC721Token(address,uint256)")); 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% diff --git a/packages/contracts/src/2.0.0/forwarder/libs/LibForwarderErrors.sol b/packages/contracts/src/2.0.0/forwarder/libs/LibForwarderErrors.sol index 69108738a..cdfb77a0b 100644 --- a/packages/contracts/src/2.0.0/forwarder/libs/LibForwarderErrors.sol +++ b/packages/contracts/src/2.0.0/forwarder/libs/LibForwarderErrors.sol @@ -23,7 +23,7 @@ pragma solidity 0.4.24; /// This contract is intended to serve as a reference, but is not actually used for efficiency reasons. contract LibForwarderErrors { string constant FEE_PERCENTAGE_TOO_LARGE = "FEE_PROPORTION_TOO_LARGE"; // Provided fee percentage greater than 5%. - string constant MAX_FEE_EXCEEDED = "MAX_FEE_EXCEEDED"; // Not enough ETH remaining to pay feeRecipient. + string constant INSUFFICIENT_ETH_REMAINING = "INSUFFICIENT_ETH_REMAINING"; // Not enough ETH remaining to pay feeRecipient. string constant OVERSOLD_WETH = "OVERSOLD_WETH"; // More WETH sold than provided with current message call. string constant COMPLETE_FILL_FAILED = "COMPLETE_FILL_FAILED"; // Desired purchase amount not completely filled (required for ZRX fees only). string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Asset transfer failed. diff --git a/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol b/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol index 190989181..63a2a085f 100644 --- a/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol +++ b/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol @@ -34,7 +34,7 @@ contract SafeMath { { require( b <= a, - "UINT256_OVERFLOW" + "UINT256_UNDERFLOW" ); return a - b; } diff --git a/packages/contracts/test/forwarder/forwarder.ts b/packages/contracts/test/forwarder/forwarder.ts index f10f22556..b6d81df58 100644 --- a/packages/contracts/test/forwarder/forwarder.ts +++ b/packages/contracts/test/forwarder/forwarder.ts @@ -27,8 +27,9 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); const DECIMALS_DEFAULT = 18; +const MAX_WETH_FILL_PERCENTAGE = 95; -describe(ContractName.Forwarder, () => { +describe.only(ContractName.Forwarder, () => { let makerAddress: string; let owner: string; let takerAddress: string; @@ -45,11 +46,8 @@ describe(ContractName.Forwarder, () => { let exchangeWrapper: ExchangeWrapper; let orderWithoutFee: SignedOrder; - let ordersWithoutFee: SignedOrder[]; let orderWithFee: SignedOrder; - let ordersWithFee: SignedOrder[]; let feeOrder: SignedOrder; - let feeOrders: SignedOrder[]; let orderFactory: OrderFactory; let erc20Wrapper: ERC20Wrapper; let erc20Balances: ERC20BalancesByOwner; @@ -60,8 +58,6 @@ describe(ContractName.Forwarder, () => { let feePercentage: BigNumber; let gasPrice: BigNumber; - const MAX_WETH_FILL_PERCENTAGE = 95; - before(async () => { await blockchainLifecycle.startAsync(); const accounts = await web3Wrapper.getAvailableAddressesAsync(); @@ -168,9 +164,9 @@ describe(ContractName.Forwarder, () => { }); describe('marketSellOrdersWithEth without extra fees', () => { - it('should fill the order', async () => { - ordersWithoutFee = [orderWithoutFee]; - feeOrders = []; + it('should fill a single order', async () => { + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; const ethValue = orderWithoutFee.takerAssetAmount.dividedToIntegerBy(2); tx = await forwarderWrapper.marketSellOrdersWithEthAsync(ordersWithoutFee, feeOrders, { @@ -206,9 +202,54 @@ describe(ContractName.Forwarder, () => { ); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); - it('should fill the order and pay ZRX fees from feeOrders', async () => { - ordersWithFee = [orderWithFee]; - feeOrders = [feeOrder]; + it('should fill multiple orders', async () => { + const secondOrderWithoutFee = await orderFactory.newSignedOrderAsync(); + const ordersWithoutFee = [orderWithoutFee, secondOrderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const ethValue = ordersWithoutFee[0].takerAssetAmount.plus( + ordersWithoutFee[1].takerAssetAmount.dividedToIntegerBy(2), + ); + + tx = await forwarderWrapper.marketSellOrdersWithEthAsync(ordersWithoutFee, feeOrders, { + value: ethValue, + from: takerAddress, + }); + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + + const primaryTakerAssetFillAmount = ForwarderWrapper.getPercentageOfValue( + ethValue, + MAX_WETH_FILL_PERCENTAGE, + ); + const firstTakerAssetFillAmount = ordersWithoutFee[0].takerAssetAmount; + const secondTakerAssetFillAmount = primaryTakerAssetFillAmount.minus(firstTakerAssetFillAmount); + + const makerAssetFillAmount = ordersWithoutFee[0].makerAssetAmount.plus( + ordersWithoutFee[1].makerAssetAmount + .times(secondTakerAssetFillAmount) + .dividedToIntegerBy(ordersWithoutFee[1].takerAssetAmount), + ); + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultMakerAssetAddress].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('should fill the order and pay ZRX fees from a single feeOrder', async () => { + const ordersWithFee = [orderWithFee]; + const feeOrders = [feeOrder]; const ethValue = orderWithFee.takerAssetAmount.dividedToIntegerBy(2); tx = await forwarderWrapper.marketSellOrdersWithEthAsync(ordersWithFee, feeOrders, { @@ -251,13 +292,71 @@ describe(ContractName.Forwarder, () => { ); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); + it('should fill the orders and pay ZRX from multiple feeOrders', async () => { + const ordersWithFee = [orderWithFee]; + const ethValue = orderWithFee.takerAssetAmount; + const makerAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); + const makerAssetAmount = orderWithFee.takerFee.dividedToIntegerBy(2); + const takerAssetAmount = feeOrder.takerAssetAmount + .times(makerAssetAmount) + .dividedToIntegerBy(feeOrder.makerAssetAmount); + + const firstFeeOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData, + makerAssetAmount, + takerAssetAmount, + }); + const secondFeeOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData, + makerAssetAmount, + takerAssetAmount, + }); + const feeOrders = [firstFeeOrder, secondFeeOrder]; + + tx = await forwarderWrapper.marketSellOrdersWithEthAsync(ordersWithFee, feeOrders, { + value: ethValue, + from: takerAddress, + }); + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + + const primaryTakerAssetFillAmount = ForwarderWrapper.getPercentageOfValue( + ethValue, + MAX_WETH_FILL_PERCENTAGE, + ); + const makerAssetFillAmount = primaryTakerAssetFillAmount + .times(orderWithoutFee.makerAssetAmount) + .dividedToIntegerBy(orderWithoutFee.takerAssetAmount); + const feeAmount = ForwarderWrapper.getPercentageOfValue(orderWithFee.takerFee, MAX_WETH_FILL_PERCENTAGE); + const wethSpentOnFeeOrders = ForwarderWrapper.getWethForFeeOrders(feeAmount, feeOrders); + const totalEthSpent = primaryTakerAssetFillAmount + .plus(wethSpentOnFeeOrders) + .plus(gasPrice.times(tx.gasUsed)); + + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultMakerAssetAddress].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount).plus(wethSpentOnFeeOrders), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); it('should fill the order when token is ZRX with fees', async () => { orderWithFee = await orderFactory.newSignedOrderAsync({ makerAssetData: assetDataUtils.encodeERC20AssetData(zrxToken.address), takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), DECIMALS_DEFAULT), }); - ordersWithFee = [orderWithFee]; - feeOrders = []; + const ordersWithFee = [orderWithFee]; + const feeOrders: SignedOrder[] = []; const ethValue = orderWithFee.takerAssetAmount.dividedToIntegerBy(2); tx = await forwarderWrapper.marketSellOrdersWithEthAsync(ordersWithFee, feeOrders, { @@ -290,7 +389,8 @@ describe(ContractName.Forwarder, () => { expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); it('should refund remaining ETH if amount is greater than takerAssetAmount', async () => { - ordersWithoutFee = [orderWithoutFee]; + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; const ethValue = orderWithoutFee.takerAssetAmount.times(2); tx = await forwarderWrapper.marketSellOrdersWithEthAsync(ordersWithoutFee, feeOrders, { @@ -306,12 +406,12 @@ describe(ContractName.Forwarder, () => { orderWithFee = await orderFactory.newSignedOrderAsync({ takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), DECIMALS_DEFAULT), }); - ordersWithFee = [orderWithFee]; + const ordersWithFee = [orderWithFee]; feeOrder = await orderFactory.newSignedOrderAsync({ makerAssetData: assetDataUtils.encodeERC20AssetData(zrxToken.address), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), DECIMALS_DEFAULT), }); - feeOrders = [feeOrder]; + const feeOrders = [feeOrder]; const ethValue = orderWithFee.takerAssetAmount; return expectTransactionFailedAsync( forwarderWrapper.marketSellOrdersWithEthAsync(ordersWithFee, feeOrders, { @@ -328,7 +428,8 @@ describe(ContractName.Forwarder, () => { makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), }); const erc20SignedOrder = await orderFactory.newSignedOrderAsync(); - ordersWithoutFee = [erc20SignedOrder, erc721SignedOrder]; + const ordersWithoutFee = [erc20SignedOrder, erc721SignedOrder]; + const feeOrders: SignedOrder[] = []; const ethValue = erc20SignedOrder.takerAssetAmount.plus(erc721SignedOrder.takerAssetAmount); tx = await forwarderWrapper.marketSellOrdersWithEthAsync(ordersWithoutFee, feeOrders, { @@ -343,8 +444,8 @@ describe(ContractName.Forwarder, () => { }); describe('marketSellOrdersWithEth with extra fees', () => { it('should fill the order and send fee to feeRecipient', async () => { - ordersWithoutFee = [orderWithoutFee]; - feeOrders = []; + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; const ethValue = orderWithoutFee.takerAssetAmount.div(2); const baseFeePercentage = 2; @@ -392,11 +493,11 @@ describe(ContractName.Forwarder, () => { expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); it('should fail if the fee is set too high', async () => { - const feeRecipientEthBalanceBefore = await web3Wrapper.getBalanceInWeiAsync(feeRecipientAddress); const ethValue = orderWithoutFee.takerAssetAmount.div(2); const baseFeePercentage = 6; feePercentage = ForwarderWrapper.getPercentageOfValue(ethValue, baseFeePercentage); - feeOrders = []; + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; await expectTransactionFailedAsync( forwarderWrapper.marketSellOrdersWithEthAsync( ordersWithoutFee, @@ -406,14 +507,28 @@ describe(ContractName.Forwarder, () => { ), RevertReason.FeePercentageTooLarge, ); - const feeRecipientEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(feeRecipientAddress); - expect(feeRecipientEthBalanceAfter).to.be.bignumber.equal(feeRecipientEthBalanceBefore); + }); + it('should fail if there is not enough ETH remaining to pay the fee', async () => { + const ethValue = orderWithoutFee.takerAssetAmount.div(2); + const baseFeePercentage = 5; + feePercentage = ForwarderWrapper.getPercentageOfValue(constants.PERCENTAGE_DENOMINATOR, baseFeePercentage); + const ordersWithFee = [orderWithFee]; + const feeOrders = [feeOrder]; + await expectTransactionFailedAsync( + forwarderWrapper.marketSellOrdersWithEthAsync( + ordersWithFee, + feeOrders, + { from: takerAddress, value: ethValue, gasPrice }, + { feePercentage, feeRecipient: feeRecipientAddress }, + ), + RevertReason.InsufficientEthRemaining, + ); }); }); describe('marketBuyOrdersWithEth without extra fees', () => { - it('should buy the exact amount of assets', async () => { - ordersWithoutFee = [orderWithoutFee]; - feeOrders = []; + it('should buy the exact amount of makerAsset in a single order', async () => { + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; const makerAssetFillAmount = orderWithoutFee.makerAssetAmount.dividedToIntegerBy(2); const ethValue = orderWithoutFee.takerAssetAmount.dividedToIntegerBy(2); @@ -444,9 +559,47 @@ describe(ContractName.Forwarder, () => { ); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); - it('should buy the exact amount of assets and return excess ETH', async () => { - ordersWithoutFee = [orderWithoutFee]; - feeOrders = []; + it('should buy the exact amount of makerAsset in multiple orders', async () => { + const secondOrderWithoutFee = await orderFactory.newSignedOrderAsync(); + const ordersWithoutFee = [orderWithoutFee, secondOrderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const makerAssetFillAmount = ordersWithoutFee[0].makerAssetAmount.plus( + ordersWithoutFee[1].makerAssetAmount.dividedToIntegerBy(2), + ); + const ethValue = ordersWithoutFee[0].takerAssetAmount.plus( + ordersWithoutFee[1].takerAssetAmount.dividedToIntegerBy(2), + ); + + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { + value: ethValue, + from: takerAddress, + }); + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultMakerAssetAddress].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('should buy the exact amount of makerAsset and return excess ETH', async () => { + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; const makerAssetFillAmount = orderWithoutFee.makerAssetAmount.dividedToIntegerBy(2); const ethValue = orderWithoutFee.takerAssetAmount; @@ -477,11 +630,11 @@ describe(ContractName.Forwarder, () => { ); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); - it('should buy the exact amount of assets with fee abstraction', async () => { - ordersWithFee = [orderWithFee]; - feeOrders = [feeOrder]; + it('should buy the exact amount of makerAsset and pay ZRX from feeOrders', async () => { + const ordersWithFee = [orderWithFee]; + const feeOrders = [feeOrder]; const makerAssetFillAmount = orderWithFee.makerAssetAmount.dividedToIntegerBy(2); - const ethValue = orderWithoutFee.takerAssetAmount; + const ethValue = orderWithFee.takerAssetAmount; tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithFee, feeOrders, makerAssetFillAmount, { value: ethValue, @@ -514,13 +667,13 @@ describe(ContractName.Forwarder, () => { ); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); - it('should buy the exact amount of assets when buying ZRX with fee abstraction', async () => { + it('should buy slightly greater than makerAssetAmount when buying ZRX', async () => { orderWithFee = await orderFactory.newSignedOrderAsync({ makerAssetData: assetDataUtils.encodeERC20AssetData(zrxToken.address), takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), DECIMALS_DEFAULT), }); - ordersWithFee = [orderWithFee]; - feeOrders = []; + const ordersWithFee = [orderWithFee]; + const feeOrders: SignedOrder[] = []; const makerAssetFillAmount = orderWithFee.makerAssetAmount.dividedToIntegerBy(2); const ethValue = orderWithFee.takerAssetAmount; tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithFee, feeOrders, makerAssetFillAmount, { @@ -545,13 +698,20 @@ describe(ContractName.Forwarder, () => { const makerFeePaid = orderWithFee.makerFee .times(primaryTakerAssetFillAmount) .dividedToIntegerBy(orderWithFee.takerAssetAmount); + const totalZrxPurchased = makerAssetFilledAmount.minus(takerFeePaid); + // Up to 1 wei worth of ZRX will be overbought per order + const maxOverboughtZrx = new BigNumber(1) + .times(orderWithFee.makerAssetAmount) + .dividedToIntegerBy(orderWithFee.takerAssetAmount); + expect(totalZrxPurchased).to.be.bignumber.gte(makerAssetFillAmount); + expect(totalZrxPurchased).to.be.bignumber.lte(makerAssetFillAmount.plus(maxOverboughtZrx)); expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( erc20Balances[makerAddress][zrxToken.address].minus(makerAssetFilledAmount).minus(makerFeePaid), ); expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( - erc20Balances[takerAddress][zrxToken.address].plus(makerAssetFilledAmount).minus(takerFeePaid), + erc20Balances[takerAddress][zrxToken.address].plus(totalZrxPurchased), ); expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), @@ -562,373 +722,282 @@ describe(ContractName.Forwarder, () => { ); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); - // it('throws if fees are higher than 5% when buying zrx', async () => { - // const highFeeZRXOrder = orderFactory.newSignedOrder({ - // makerAssetData: assetDataUtils.encodeERC20AssetData(zrxToken.address), - // makerAssetAmount: orderWithoutFee.makerAssetAmount, - // takerFee: orderWithoutFee.makerAssetAmount.times(0.06), - // }); - // ordersWithFee = [highFeeZRXOrder]; - // feeOrders = []; - // const makerAssetAmount = orderWithoutFee.makerAssetAmount.div(2); - // const fillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // ordersWithFee, - // feeOrders, - // feePercentage, - // makerAssetAmount, - // ); - // return expectTransactionFailedAsync( - // forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithFee, feeOrders, makerAssetAmount, { - // from: takerAddress, - // value: fillAmountWei, - // }), - // RevertReason.UnacceptableThreshold, - // ); - // }); - // it('throws if fees are higher than 5% when buying erc20', async () => { - // const highFeeERC20Order = orderFactory.newSignedOrder({ - // takerFee: orderWithoutFee.makerAssetAmount.times(0.06), - // }); - // ordersWithFee = [highFeeERC20Order]; - // feeOrders = [feeOrder]; - // const makerAssetAmount = orderWithoutFee.makerAssetAmount.div(2); - // const fillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // ordersWithFee, - // feeOrders, - // feePercentage, - // makerAssetAmount, - // ); - // return expectTransactionFailedAsync( - // forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithFee, feeOrders, makerAssetAmount, { - // from: takerAddress, - // value: fillAmountWei, - // }), - // RevertReason.UnacceptableThreshold as any, - // ); - // }); - // it('throws if makerAssetAmount is 0', async () => { - // const makerAssetAmount = new BigNumber(0); - // const fillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // ordersWithFee, - // feeOrders, - // feePercentage, - // makerAssetAmount, - // ); - // return expectTransactionFailedAsync( - // forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithFee, feeOrders, makerAssetAmount, { - // from: takerAddress, - // value: fillAmountWei, - // }), - // RevertReason.ValueGreaterThanZero as any, - // ); - // }); - // it('throws if the amount of ETH sent in is less than the takerAssetFilledAmount', async () => { - // const makerAssetAmount = orderWithoutFee.makerAssetAmount; - // const primaryTakerAssetFillAmount = orderWithoutFee.takerAssetAmount.div(2); - // const zero = new BigNumber(0); - // // Deposit enough taker balance to fill the order - // const wethDepositTxHash = await wethContract.deposit.sendTransactionAsync({ - // from: takerAddress, - // value: orderWithoutFee.takerAssetAmount, - // }); - // await web3Wrapper.awaitTransactionSuccessAsync(wethDepositTxHash); - // // Transfer all of this WETH to the forwarding contract - // const wethTransferTxHash = await wethContract.transfer.sendTransactionAsync( - // forwarderContract.address, - // orderWithoutFee.takerAssetAmount, - // { from: takerAddress }, - // ); - // await web3Wrapper.awaitTransactionSuccessAsync(wethTransferTxHash); - // // We use the contract directly to get around wrapper validations and calculations - // const formattedOrders = formatters.createMarketSellOrders(signedOrders, zero); - // const formattedFeeOrders = formatters.createMarketSellOrders(feeOrders, zero); - // return expectTransactionFailedAsync( - // forwarderContract.marketBuyOrdersWithEth.sendTransactionAsync( - // formattedOrders.orders, - // formattedOrders.signatures, - // formattedFeeOrders.orders, - // formattedFeeOrders.signatures, - // makerAssetAmount, - // zero, - // constants.NULL_ADDRESS, - // { value: primaryTakerAssetFillAmount, from: takerAddress }, - // ), - // RevertReason.InvalidMsgValue, - // ); - // }); + it('should not change balances if the amount of ETH sent is too low to fill the makerAssetAmount', async () => { + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const makerAssetFillAmount = orderWithoutFee.makerAssetAmount.dividedToIntegerBy(2); + const ethValue = orderWithoutFee.takerAssetAmount.dividedToIntegerBy(4); + + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { + value: ethValue, + from: takerAddress, + }); + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + + const totalEthSpent = gasPrice.times(tx.gasUsed); + + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances).to.deep.equal(erc20Balances); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('should buy an ERC721 asset from a single order', async () => { + const makerAssetId = erc721MakerAssetIds[0]; + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber(1), + makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const makerAssetFillAmount = new BigNumber(1); + const ethValue = orderWithFee.takerAssetAmount; + + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { + from: takerAddress, + value: ethValue, + }); + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newOwner = await erc721Token.ownerOf.callAsync(makerAssetId); + const newBalances = await erc20Wrapper.getBalancesAsync(); + + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + expect(newOwner).to.be.bignumber.equal(takerAddress); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('should buy an ERC721 asset and ignore later orders with different makerAssetData', async () => { + const makerAssetId = erc721MakerAssetIds[0]; + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber(1), + makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), + }); + const differentMakerAssetDataOrder = await orderFactory.newSignedOrderAsync(); + const ordersWithoutFee = [orderWithoutFee, differentMakerAssetDataOrder]; + const feeOrders: SignedOrder[] = []; + const makerAssetFillAmount = new BigNumber(1).plus(differentMakerAssetDataOrder.makerAssetAmount); + const ethValue = orderWithFee.takerAssetAmount; + + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { + from: takerAddress, + value: ethValue, + }); + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newOwner = await erc721Token.ownerOf.callAsync(makerAssetId); + const newBalances = await erc20Wrapper.getBalancesAsync(); + + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + expect(newOwner).to.be.bignumber.equal(takerAddress); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultMakerAssetAddress], + ); + expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultMakerAssetAddress], + ); + }); + it('should buy an ERC721 asset and pay ZRX fees from a single fee order', async () => { + const makerAssetId = erc721MakerAssetIds[0]; + orderWithFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber(1), + makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), + takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), DECIMALS_DEFAULT), + }); + const ordersWithFee = [orderWithFee]; + const feeOrders = [feeOrder]; + const makerAssetFillAmount = orderWithFee.makerAssetAmount; + const primaryTakerAssetFillAmount = orderWithFee.takerAssetAmount; + const feeAmount = orderWithFee.takerFee; + const wethSpentOnFeeOrders = ForwarderWrapper.getWethForFeeOrders(feeAmount, feeOrders); + const ethValue = primaryTakerAssetFillAmount.plus(wethSpentOnFeeOrders); + + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithFee, feeOrders, makerAssetFillAmount, { + value: ethValue, + from: takerAddress, + }); + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newOwner = await erc721Token.ownerOf.callAsync(makerAssetId); + const newBalances = await erc20Wrapper.getBalancesAsync(); + + const totalEthSpent = ethValue.plus(gasPrice.times(tx.gasUsed)); + + expect(newOwner).to.be.bignumber.equal(takerAddress); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount).plus(wethSpentOnFeeOrders), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('should buy an ERC721 asset and pay ZRX fees from multiple fee orders', async () => { + const makerAssetId = erc721MakerAssetIds[0]; + orderWithFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber(1), + makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), + takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), DECIMALS_DEFAULT), + }); + const ordersWithFee = [orderWithFee]; + const makerAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); + const makerAssetAmount = orderWithFee.takerFee.dividedToIntegerBy(2); + const takerAssetAmount = feeOrder.takerAssetAmount + .times(makerAssetAmount) + .dividedToIntegerBy(feeOrder.makerAssetAmount); + + const firstFeeOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData, + makerAssetAmount, + takerAssetAmount, + }); + const secondFeeOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData, + makerAssetAmount, + takerAssetAmount, + }); + const feeOrders = [firstFeeOrder, secondFeeOrder]; + + const makerAssetFillAmount = orderWithFee.makerAssetAmount; + const primaryTakerAssetFillAmount = orderWithFee.takerAssetAmount; + const feeAmount = orderWithFee.takerFee; + const wethSpentOnFeeOrders = ForwarderWrapper.getWethForFeeOrders(feeAmount, feeOrders); + const ethValue = primaryTakerAssetFillAmount.plus(wethSpentOnFeeOrders); + + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithFee, feeOrders, makerAssetFillAmount, { + value: ethValue, + from: takerAddress, + }); + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newOwner = await erc721Token.ownerOf.callAsync(makerAssetId); + const newBalances = await erc20Wrapper.getBalancesAsync(); + + const totalEthSpent = ethValue.plus(gasPrice.times(tx.gasUsed)); + + expect(newOwner).to.be.bignumber.equal(takerAddress); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount).plus(wethSpentOnFeeOrders), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + }); + describe('marketBuyOrdersWithEth with extra fees', () => { + it('should buy an asset and send fee to feeRecipient', async () => { + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const makerAssetFillAmount = orderWithoutFee.makerAssetAmount.dividedToIntegerBy(2); + const ethValue = orderWithoutFee.takerAssetAmount; + + const baseFeePercentage = 2; + feePercentage = ForwarderWrapper.getPercentageOfValue(constants.PERCENTAGE_DENOMINATOR, baseFeePercentage); + const feeRecipientEthBalanceBefore = await web3Wrapper.getBalanceInWeiAsync(feeRecipientAddress); + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + makerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + { feePercentage, feeRecipient: feeRecipientAddress }, + ); + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const feeRecipientEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(feeRecipientAddress); + const newBalances = await erc20Wrapper.getBalancesAsync(); + + const primaryTakerAssetFillAmount = orderWithoutFee.takerAssetAmount.dividedToIntegerBy(2); + const ethSpentOnFee = ForwarderWrapper.getPercentageOfValue(primaryTakerAssetFillAmount, baseFeePercentage); + const totalEthSpent = primaryTakerAssetFillAmount.plus(ethSpentOnFee).plus(gasPrice.times(tx.gasUsed)); + + expect(feeRecipientEthBalanceAfter).to.be.bignumber.equal(feeRecipientEthBalanceBefore.plus(ethSpentOnFee)); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultMakerAssetAddress].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('should fail if the fee is set too high', async () => { + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const makerAssetFillAmount = orderWithoutFee.makerAssetAmount.dividedToIntegerBy(2); + const ethValue = orderWithoutFee.takerAssetAmount; + + const baseFeePercentage = 6; + feePercentage = ForwarderWrapper.getPercentageOfValue(constants.PERCENTAGE_DENOMINATOR, baseFeePercentage); + await expectTransactionFailedAsync( + forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + makerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + { feePercentage, feeRecipient: feeRecipientAddress }, + ), + RevertReason.FeePercentageTooLarge, + ); + }); + it('should fail if there is not enough ETH remaining to pay the fee', async () => { + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const makerAssetFillAmount = orderWithoutFee.makerAssetAmount.dividedToIntegerBy(2); + const ethValue = orderWithoutFee.takerAssetAmount.dividedToIntegerBy(2); + + const baseFeePercentage = 2; + feePercentage = ForwarderWrapper.getPercentageOfValue(constants.PERCENTAGE_DENOMINATOR, baseFeePercentage); + await expectTransactionFailedAsync( + forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + makerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + { feePercentage, feeRecipient: feeRecipientAddress }, + ), + RevertReason.InsufficientEthRemaining, + ); + }); }); - // describe('marketBuyOrdersWithEth - ERC721', async () => { - // it('buys ERC721 assets', async () => { - // const makerAssetId = erc721MakerAssetIds[0]; - // orderWithoutFee = orderFactory.newSignedOrder({ - // makerAssetAmount: new BigNumber(1), - // makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), - // }); - // feeOrders = []; - // signedOrders = [orderWithoutFee]; - // const makerAssetAmount = new BigNumber(signedOrders.length); - // const fillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // signedOrders, - // feeOrders, - // feePercentage, - // makerAssetAmount, - // ); - // tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(signedOrders, feeOrders, makerAssetAmount, { - // from: takerAddress, - // value: fillAmountWei, - // }); - // const newOwnerTakerAsset = await erc721Token.ownerOf.callAsync(makerAssetId); - // expect(newOwnerTakerAsset).to.be.bignumber.equal(takerAddress); - // }); - // it('buys ERC721 assets with fee abstraction', async () => { - // const makerAssetId = erc721MakerAssetIds[0]; - // orderWithoutFee = orderFactory.newSignedOrder({ - // makerAssetAmount: new BigNumber(1), - // takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), DECIMALS_DEFAULT), - // makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), - // }); - // signedOrders = [orderWithoutFee]; - // const makerAssetAmount = new BigNumber(signedOrders.length); - // const fillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // signedOrders, - // feeOrders, - // feePercentage, - // makerAssetAmount, - // ); - // tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(signedOrders, feeOrders, makerAssetAmount, { - // from: takerAddress, - // value: fillAmountWei, - // }); - // const newOwnerTakerAsset = await erc721Token.ownerOf.callAsync(makerAssetId); - // expect(newOwnerTakerAsset).to.be.bignumber.equal(takerAddress); - // }); - // it('buys ERC721 assets with fee abstraction and pays fee to fee recipient', async () => { - // const makerAssetId = erc721MakerAssetIds[0]; - // orderWithoutFee = orderFactory.newSignedOrder({ - // makerAssetAmount: new BigNumber(1), - // takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), DECIMALS_DEFAULT), - // makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), - // }); - // signedOrders = [orderWithoutFee]; - // feePercentage = 100; - // const initTakerBalanceWei = await web3Wrapper.getBalanceInWeiAsync(takerAddress); - // const initFeeRecipientBalanceWei = await web3Wrapper.getBalanceInWeiAsync(feeRecipientAddress); - // const makerAssetAmount = new BigNumber(signedOrders.length); - // const fillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // signedOrders, - // feeOrders, - // feePercentage, - // makerAssetAmount, - // ); - // tx = await forwarderWrapper.marketBuyOrdersWithEthAsync( - // signedOrders, - // feeOrders, - // makerAssetAmount, - // { - // from: takerAddress, - // value: fillAmountWei, - // gasPrice: gasPrice, - // }, - // { - // feePercentage, - // feeRecipient: feeRecipientAddress, - // }, - // ); - // const afterFeeRecipientEthBalance = await web3Wrapper.getBalanceInWeiAsync(feeRecipientAddress); - // const afterTakerBalanceWei = await web3Wrapper.getBalanceInWeiAsync(takerAddress); - // const takerFilledAmount = initTakerBalanceWei.minus(afterTakerBalanceWei).plus(tx.gasUsed); - // const newOwnerTakerAsset = await erc721Token.ownerOf.callAsync(makerAssetId); - // expect(newOwnerTakerAsset).to.be.bignumber.equal(takerAddress); - // const balanceDiff = afterFeeRecipientEthBalance.minus(initFeeRecipientBalanceWei); - // expect(takerFilledAmount.dividedToIntegerBy(balanceDiff)).to.be.bignumber.equal(101); - // expect(takerFilledAmount.minus(balanceDiff).dividedToIntegerBy(balanceDiff)).to.be.bignumber.equal(100); - // }); - // it('buys multiple ERC721 assets with fee abstraction and pays fee to fee recipient', async () => { - // const makerAssetId1 = erc721MakerAssetIds[0]; - // const makerAssetId2 = erc721MakerAssetIds[1]; - // const signedOrder1 = orderFactory.newSignedOrder({ - // makerAssetAmount: new BigNumber(1), - // takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), DECIMALS_DEFAULT), - // makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId1), - // }); - // const signedOrder2 = orderFactory.newSignedOrder({ - // makerAssetAmount: new BigNumber(1), - // takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), DECIMALS_DEFAULT), - // makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId2), - // }); - // signedOrders = [signedOrder1, signedOrder2]; - // feePercentage = 10; - // const makerAssetAmount = new BigNumber(signedOrders.length); - // const fillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // signedOrders, - // feeOrders, - // feePercentage, - // makerAssetAmount, - // ); - // tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(signedOrders, feeOrders, makerAssetAmount, { - // from: takerAddress, - // value: fillAmountWei, - // }); - // const newOwnerTakerAsset1 = await erc721Token.ownerOf.callAsync(makerAssetId1); - // expect(newOwnerTakerAsset1).to.be.bignumber.equal(takerAddress); - // const newOwnerTakerAsset2 = await erc721Token.ownerOf.callAsync(makerAssetId2); - // expect(newOwnerTakerAsset2).to.be.bignumber.equal(takerAddress); - // }); - // it('buys ERC721 assets with fee abstraction and handles fee orders filled and excess eth', async () => { - // const makerAssetId = erc721MakerAssetIds[0]; - // feePercentage = 0; - // // In this scenario a total of 6 ZRX fees need to be paid. - // // There are two fee orders, but the first fee order is partially filled while - // // the Forwarding contract tx is in the mempool. - // const erc721MakerAssetAmount = new BigNumber(1); - // orderWithoutFee = orderFactory.newSignedOrder({ - // makerAssetAmount: erc721MakerAssetAmount, - // takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), DECIMALS_DEFAULT), - // takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(6), DECIMALS_DEFAULT), - // makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), - // }); - // signedOrders = [orderWithoutFee]; - // const firstFeeOrder = orderFactory.newSignedOrder({ - // makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(8), DECIMALS_DEFAULT), - // takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(0.1), DECIMALS_DEFAULT), - // makerAssetData: assetDataUtils.encodeERC20AssetData(zrxToken.address), - // takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), DECIMALS_DEFAULT), - // }); - // const secondFeeOrder = orderFactory.newSignedOrder({ - // makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(8), DECIMALS_DEFAULT), - // takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(0.12), DECIMALS_DEFAULT), - // makerAssetData: assetDataUtils.encodeERC20AssetData(zrxToken.address), - // takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), DECIMALS_DEFAULT), - // }); - // feeOrders = [firstFeeOrder, secondFeeOrder]; - // const makerAssetAmount = new BigNumber(signedOrders.length); - // const fillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // signedOrders, - // feeOrders, - // feePercentage, - // erc721MakerAssetAmount, - // ); - // // Simulate another otherAddress user partially filling firstFeeOrder - // const firstFeeOrderFillAmount = firstFeeOrder.makerAssetAmount.div(2); - // tx = await forwarderWrapper.marketBuyOrdersWithEthAsync([firstFeeOrder], [], firstFeeOrderFillAmount, { - // from: otherAddress, - // value: fillAmountWei, - // }); - // // For tests we calculate how much this should've cost given that firstFeeOrder was filled - // const expectedFillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // signedOrders, - // feeOrders, - // feePercentage, - // erc721MakerAssetAmount, - // ); - // // With 4 ZRX remaining in firstFeeOrder, the secondFeeOrder will need to be filled to make up - // // the total amount of fees required (6) - // // Since the fee orders can be filled while the transaction is pending the user safely sends in - // // extra ether to cover any slippage - // const initEthBalance = await web3Wrapper.getBalanceInWeiAsync(takerAddress); - // const slippageFillAmountWei = fillAmountWei.times(2); - // tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(signedOrders, feeOrders, makerAssetAmount, { - // from: takerAddress, - // value: slippageFillAmountWei, - // gasPrice: gasPrice, - // }); - // const afterEthBalance = await web3Wrapper.getBalanceInWeiAsync(takerAddress); - // const expectedEthBalanceAfterGasCosts = initEthBalance.minus(expectedFillAmountWei).minus(tx.gasUsed); - // const newOwnerTakerAsset = await erc721Token.ownerOf.callAsync(makerAssetId); - // expect(newOwnerTakerAsset).to.be.bignumber.equal(takerAddress); - // expect(afterEthBalance).to.be.bignumber.equal(expectedEthBalanceAfterGasCosts); - // }); - // it('buys ERC721 assets with fee abstraction and handles fee orders filled', async () => { - // const makerAssetId = erc721MakerAssetIds[0]; - // feePercentage = 0; - // // In this scenario a total of 6 ZRX fees need to be paid. - // // There are two fee orders, but the first fee order is partially filled while - // // the Forwarding contract tx is in the mempool. - // const erc721MakerAssetAmount = new BigNumber(1); - // orderWithoutFee = orderFactory.newSignedOrder({ - // makerAssetAmount: erc721MakerAssetAmount, - // takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), DECIMALS_DEFAULT), - // takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(6), DECIMALS_DEFAULT), - // makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), - // }); - // const zrxMakerAssetAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(8), DECIMALS_DEFAULT); - // signedOrders = [orderWithoutFee]; - // const firstFeeOrder = orderFactory.newSignedOrder({ - // makerAssetAmount: zrxMakerAssetAmount, - // takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(0.1), DECIMALS_DEFAULT), - // makerAssetData: assetDataUtils.encodeERC20AssetData(zrxToken.address), - // takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), DECIMALS_DEFAULT), - // }); - // const secondFeeOrder = orderFactory.newSignedOrder({ - // makerAssetAmount: zrxMakerAssetAmount, - // takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(0.12), DECIMALS_DEFAULT), - // makerAssetData: assetDataUtils.encodeERC20AssetData(zrxToken.address), - // takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), DECIMALS_DEFAULT), - // }); - // feeOrders = [firstFeeOrder, secondFeeOrder]; - // const makerAssetAmount = new BigNumber(signedOrders.length); - // const fillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // signedOrders, - // feeOrders, - // feePercentage, - // erc721MakerAssetAmount, - // ); - // // Simulate another otherAddress user partially filling firstFeeOrder - // const firstFeeOrderFillAmount = firstFeeOrder.makerAssetAmount.div(2); - // tx = await forwarderWrapper.marketBuyOrdersWithEthAsync([firstFeeOrder], [], firstFeeOrderFillAmount, { - // from: otherAddress, - // value: fillAmountWei, - // }); - // const expectedFillAmountWei = await forwarderWrapper.calculateMarketBuyFillAmountWeiAsync( - // signedOrders, - // feeOrders, - // feePercentage, - // erc721MakerAssetAmount, - // ); - // tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(signedOrders, feeOrders, makerAssetAmount, { - // from: takerAddress, - // value: expectedFillAmountWei, - // }); - // const newOwnerTakerAsset = await erc721Token.ownerOf.callAsync(makerAssetId); - // expect(newOwnerTakerAsset).to.be.bignumber.equal(takerAddress); - // }); - // it('throws when mixed ERC721 and ERC20 assets', async () => { - // const makerAssetId = erc721MakerAssetIds[0]; - // const erc721SignedOrder = orderFactory.newSignedOrder({ - // makerAssetAmount: new BigNumber(1), - // makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), - // }); - // const erc20SignedOrder = orderFactory.newSignedOrder(); - // signedOrders = [erc721SignedOrder, erc20SignedOrder]; - // const makerAssetAmount = new BigNumber(signedOrders.length); - // const fillAmountWei = erc20SignedOrder.takerAssetAmount.plus(erc721SignedOrder.takerAssetAmount); - // return expectTransactionFailedAsync( - // forwarderWrapper.marketBuyOrdersWithEthAsync(signedOrders, feeOrders, makerAssetAmount, { - // from: takerAddress, - // value: fillAmountWei, - // }), - // RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, - // ); - // }); - // it('throws when mixed ERC721 and ERC20 assets with ERC20 first', async () => { - // const makerAssetId = erc721MakerAssetIds[0]; - // const erc721SignedOrder = orderFactory.newSignedOrder({ - // makerAssetAmount: new BigNumber(1), - // makerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), - // }); - // const erc20SignedOrder = orderFactory.newSignedOrder(); - // signedOrders = [erc20SignedOrder, erc721SignedOrder]; - // const makerAssetAmount = new BigNumber(signedOrders.length); - // const fillAmountWei = erc20SignedOrder.takerAssetAmount.plus(erc721SignedOrder.takerAssetAmount); - // return expectTransactionFailedAsync( - // forwarderWrapper.marketBuyOrdersWithEthAsync(signedOrders, feeOrders, makerAssetAmount, { - // from: takerAddress, - // value: fillAmountWei, - // }), - // RevertReason.InvalidTakerAmount, - // ); - // }); - // }); }); // tslint:disable:max-file-line-count // tslint:enable:no-unnecessary-type-assertion diff --git a/packages/contracts/test/utils/forwarder_wrapper.ts b/packages/contracts/test/utils/forwarder_wrapper.ts index 773ddf897..ef7476e36 100644 --- a/packages/contracts/test/utils/forwarder_wrapper.ts +++ b/packages/contracts/test/utils/forwarder_wrapper.ts @@ -25,13 +25,13 @@ export class ForwarderWrapper { let remainingFeeAmount = feeAmount; _.forEach(feeOrders, feeOrder => { const feeAvailable = feeOrder.makerAssetAmount.minus(feeOrder.takerFee); - if (!remainingFeeAmount.isZero() && feeAvailable.gte(remainingFeeAmount)) { + if (!remainingFeeAmount.isZero() && feeAvailable.gt(remainingFeeAmount)) { wethAmount = wethAmount .plus(feeOrder.takerAssetAmount.times(remainingFeeAmount).dividedToIntegerBy(feeAvailable)) .plus(1); remainingFeeAmount = new BigNumber(0); } else if (!remainingFeeAmount.isZero()) { - wethAmount = wethAmount.plus(feeOrder.takerAssetAmount).plus(1); + wethAmount = wethAmount.plus(feeOrder.takerAssetAmount); remainingFeeAmount = remainingFeeAmount.minus(feeAvailable); } }); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index b96bdb182..555cd7dec 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -215,10 +215,10 @@ export enum RevertReason { LibBytesGreaterOrEqualToSourceBytesLengthRequired = 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', Erc20InsufficientBalance = 'ERC20_INSUFFICIENT_BALANCE', Erc20InsufficientAllowance = 'ERC20_INSUFFICIENT_ALLOWANCE', - UnacceptableThreshold = 'UNACCEPTABLE_THRESHOLD', FeePercentageTooLarge = 'FEE_PERCENTAGE_TOO_LARGE', ValueGreaterThanZero = 'VALUE_GREATER_THAN_ZERO', InvalidMsgValue = 'INVALID_MSG_VALUE', + InsufficientEthRemaining = 'INSUFFICIENT_ETH_REMAINING', } export enum StatusCodes { |