crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make marketBuy functions revert if entire amount not filled

Browse Source
This commit is contained in:
Amir Bandeali
2018-08-09 11:20:06 -07:00
parent b60a74c8bc
commit b9d8d2d5e3
6 changed files with 44 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol vendored
View File

@@ -139,7 +139,7 @@ contract MixinExchangeWrapper is
/// @param makerAssetFillAmount Desired amount of makerAsset to buy. /// @param makerAssetFillAmount Desired amount of makerAsset to buy.
/// @param signatures Proofs that orders have been signed by makers. /// @param signatures Proofs that orders have been signed by makers.
/// @return Amounts filled and fees paid by makers and taker. /// @return Amounts filled and fees paid by makers and taker.
function marketBuyWithWeth( function marketBuyExactAmountWithWeth(
LibOrder.Order[] memory orders, LibOrder.Order[] memory orders,
uint256 makerAssetFillAmount, uint256 makerAssetFillAmount,
bytes[] memory signatures bytes[] memory signatures
@@ -180,10 +180,16 @@ contract MixinExchangeWrapper is
addFillResults(totalFillResults, singleFillResults); addFillResults(totalFillResults, singleFillResults);
// Stop execution if the entire amount of makerAsset has been bought // Stop execution if the entire amount of makerAsset has been bought
if (totalFillResults.makerAssetFilledAmount >= makerAssetFillAmount) { uint256 makerAssetFilledAmount = totalFillResults.makerAssetFilledAmount;
if (makerAssetFilledAmount >= makerAssetFillAmount) {
break; break;
} }
} }
require(
makerAssetFilledAmount >= makerAssetFillAmount,
"COMPLETE_FILL_FAILED"
);
return totalFillResults; return totalFillResults;
} }
@@ -196,7 +202,7 @@ contract MixinExchangeWrapper is
/// @param zrxBuyAmount Desired amount of ZRX to buy. /// @param zrxBuyAmount Desired amount of ZRX to buy.
/// @param signatures Proofs that orders have been created by makers. /// @param signatures Proofs that orders have been created by makers.
/// @return totalFillResults Amounts filled and fees paid by maker and taker. /// @return totalFillResults Amounts filled and fees paid by maker and taker.
function marketBuyZrxWithWeth( function marketBuyExactZrxWithWeth(
LibOrder.Order[] memory orders, LibOrder.Order[] memory orders,
uint256 zrxBuyAmount, uint256 zrxBuyAmount,
bytes[] memory signatures bytes[] memory signatures
@@ -248,6 +254,10 @@ contract MixinExchangeWrapper is
} }
} }
require(
zrxPurchased >= zrxBuyAmount,
"COMPLETE_FILL_FAILED"
);
return totalFillResults; return totalFillResults;
} }
} }

53
packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol vendored
View File

@@ -23,7 +23,7 @@ import "./libs/LibConstants.sol";
import "./mixins/MWeth.sol"; import "./mixins/MWeth.sol";
import "./mixins/MAssets.sol"; import "./mixins/MAssets.sol";
import "./mixins/MExchangeWrapper.sol"; import "./mixins/MExchangeWrapper.sol";
import "./mixins/MForwarderCore.sol"; import "./interfaces/IForwarderCore.sol";
import "../utils/LibBytes/LibBytes.sol"; import "../utils/LibBytes/LibBytes.sol";
import "../protocol/Exchange/libs/LibOrder.sol"; import "../protocol/Exchange/libs/LibOrder.sol";
import "../protocol/Exchange/libs/LibFillResults.sol"; import "../protocol/Exchange/libs/LibFillResults.sol";
@@ -37,7 +37,7 @@ contract MixinForwarderCore is
MWeth, MWeth,
MAssets, MAssets,
MExchangeWrapper, MExchangeWrapper,
MForwarderCore IForwarderCore
{ {
using LibBytes for bytes; using LibBytes for bytes;
@@ -117,7 +117,7 @@ contract MixinForwarderCore is
); );
// Buy back all ZRX spent on fees. // Buy back all ZRX spent on fees.
zrxBuyAmount = orderFillResults.takerFeePaid; zrxBuyAmount = orderFillResults.takerFeePaid;
feeOrderFillResults = marketBuyZrxWithWeth( feeOrderFillResults = marketBuyExactZrxWithWeth(
feeOrders, feeOrders,
zrxBuyAmount, zrxBuyAmount,
feeSignatures feeSignatures
@@ -125,13 +125,6 @@ contract MixinForwarderCore is
makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount; makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount;
} }
// Ensure that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold.
assertValidFillResults(
orderFillResults,
feeOrderFillResults,
zrxBuyAmount
);
// Transfer feePercentage of total ETH spent on primary orders to feeRecipient. // Transfer feePercentage of total ETH spent on primary orders to feeRecipient.
// Refund remaining ETH to msg.sender. // Refund remaining ETH to msg.sender.
transferEthFeeAndRefund( transferEthFeeAndRefund(
@@ -180,7 +173,7 @@ contract MixinForwarderCore is
if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) { if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) {
// If the makerAsset is ZRX, it is not necessary to pay fees out of this // If the makerAsset is ZRX, it is not necessary to pay fees out of this
// contracts's ZRX balance because fees are factored into the price of the order. // contracts's ZRX balance because fees are factored into the price of the order.
orderFillResults = marketBuyZrxWithWeth( orderFillResults = marketBuyExactZrxWithWeth(
orders, orders,
makerAssetFillAmount, makerAssetFillAmount,
signatures signatures
@@ -190,14 +183,14 @@ contract MixinForwarderCore is
} else { } else {
// Attemp to purchase desired amount of makerAsset. // Attemp to purchase desired amount of makerAsset.
// ZRX fees are payed with this contract's balance. // ZRX fees are payed with this contract's balance.
orderFillResults = marketBuyWithWeth( orderFillResults = marketBuyExactAmountWithWeth(
orders, orders,
makerAssetFillAmount, makerAssetFillAmount,
signatures signatures
); );
// Buy back all ZRX spent on fees. // Buy back all ZRX spent on fees.
zrxBuyAmount = orderFillResults.takerFeePaid; zrxBuyAmount = orderFillResults.takerFeePaid;
feeOrderFillResults = marketBuyZrxWithWeth( feeOrderFillResults = marketBuyExactZrxWithWeth(
feeOrders, feeOrders,
zrxBuyAmount, zrxBuyAmount,
feeSignatures feeSignatures
@@ -205,13 +198,6 @@ contract MixinForwarderCore is
makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount; makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount;
} }
// Ensure that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold.
assertValidFillResults(
orderFillResults,
feeOrderFillResults,
zrxBuyAmount
);
// Transfer feePercentage of total ETH spent on primary orders to feeRecipient. // Transfer feePercentage of total ETH spent on primary orders to feeRecipient.
// Refund remaining ETH to msg.sender. // Refund remaining ETH to msg.sender.
transferEthFeeAndRefund( transferEthFeeAndRefund(
@@ -224,31 +210,4 @@ contract MixinForwarderCore is
// Transfer purchased assets to msg.sender. // Transfer purchased assets to msg.sender.
transferPurchasedAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased); transferPurchasedAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased);
} }
/// @dev Ensures that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold.
/// @param orderFillResults Amounts filled and fees paid for primary orders.
/// @param feeOrderFillResults Amounts filled and fees paid for fee orders.
/// @param zrxBuyAmount The amount of ZRX that needed to be repurchased after filling primary orders.
function assertValidFillResults(
FillResults memory orderFillResults,
FillResults memory feeOrderFillResults,
uint256 zrxBuyAmount
)
internal
view
{
// Ensure that all ZRX spent while filling primary orders has been repurchased.
uint256 zrxPurchased = safeSub(feeOrderFillResults.makerAssetFilledAmount, feeOrderFillResults.takerFeePaid);
require(
zrxPurchased >= zrxBuyAmount,
"COMPLETE_FILL_FAILED"
);
// Ensure that no extra WETH owned by this contract has been sold.
uint256 wethSold = safeAdd(orderFillResults.takerAssetFilledAmount, feeOrderFillResults.takerAssetFilledAmount);
require(
wethSold <= msg.value,
"OVERSOLD_WETH"
);
}
} }

12
packages/contracts/src/2.0.0/forwarder/MixinWeth.sol vendored
View File

@@ -71,12 +71,16 @@ contract MixinWeth is
"FEE_PERCENTAGE_TOO_LARGE" "FEE_PERCENTAGE_TOO_LARGE"
); );
// Calculate amount of WETH that hasn't been sold. // Ensure that no extra WETH owned by this contract has been sold.
uint256 wethRemaining = safeSub( uint256 wethSold = safeAdd(wethSoldExcludingFeeOrders, wethSoldForZrx);
msg.value, require(
safeAdd(wethSoldExcludingFeeOrders, wethSoldForZrx) wethSold <= msg.value,
"OVERSOLD_WETH"
); );
// Calculate amount of WETH that hasn't been sold.
uint256 wethRemaining = safeSub(msg.value, wethSold);
// Calculate ETH fee to pay to feeRecipient. // Calculate ETH fee to pay to feeRecipient.
uint256 ethFee = getPartialAmount( uint256 ethFee = getPartialAmount(
feePercentage, feePercentage,

4
packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol vendored
View File

@@ -60,7 +60,7 @@ contract MExchangeWrapper {
/// @param makerAssetFillAmount Desired amount of makerAsset to buy. /// @param makerAssetFillAmount Desired amount of makerAsset to buy.
/// @param signatures Proofs that orders have been signed by makers. /// @param signatures Proofs that orders have been signed by makers.
/// @return Amounts filled and fees paid by makers and taker. /// @return Amounts filled and fees paid by makers and taker.
function marketBuyWithWeth( function marketBuyExactAmountWithWeth(
LibOrder.Order[] memory orders, LibOrder.Order[] memory orders,
uint256 makerAssetFillAmount, uint256 makerAssetFillAmount,
bytes[] memory signatures bytes[] memory signatures
@@ -77,7 +77,7 @@ contract MExchangeWrapper {
/// @param zrxBuyAmount Desired amount of ZRX to buy. /// @param zrxBuyAmount Desired amount of ZRX to buy.
/// @param signatures Proofs that orders have been created by makers. /// @param signatures Proofs that orders have been created by makers.
/// @return totalFillResults Amounts filled and fees paid by maker and taker. /// @return totalFillResults Amounts filled and fees paid by maker and taker.
function marketBuyZrxWithWeth( function marketBuyExactZrxWithWeth(
LibOrder.Order[] memory orders, LibOrder.Order[] memory orders,
uint256 zrxBuyAmount, uint256 zrxBuyAmount,
bytes[] memory signatures bytes[] memory signatures

42
packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol vendored
View File

@@ -1,42 +0,0 @@
/*
Copyright 2018 ZeroEx Intl.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../../protocol/Exchange/libs/LibOrder.sol";
import "../../protocol/Exchange/libs/LibFillResults.sol";
import "../interfaces/IForwarderCore.sol";
contract MForwarderCore is
IForwarderCore
{
/// @dev Ensures that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold.
/// @param orderFillResults Amounts filled and fees paid for primary orders.
/// @param feeOrderFillResults Amounts filled and fees paid for fee orders.
/// @param zrxBuyAmount The amount of ZRX that needed to be repurchased after filling primary orders.
function assertValidFillResults(
LibFillResults.FillResults memory orderFillResults,
LibFillResults.FillResults memory feeOrderFillResults,
uint256 zrxBuyAmount
)
internal
view;
}

58
packages/contracts/test/forwarder/forwarder.ts
View File

@@ -722,25 +722,18 @@ describe(ContractName.Forwarder, () => {
); );
expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT);
}); });
it('should not change balances if the amount of ETH sent is too low to fill the makerAssetAmount', async () => { it('should revert if the amount of ETH sent is too low to fill the makerAssetAmount', async () => {
const ordersWithoutFee = [orderWithoutFee]; const ordersWithoutFee = [orderWithoutFee];
const feeOrders: SignedOrder[] = []; const feeOrders: SignedOrder[] = [];
const makerAssetFillAmount = orderWithoutFee.makerAssetAmount.dividedToIntegerBy(2); const makerAssetFillAmount = orderWithoutFee.makerAssetAmount.dividedToIntegerBy(2);
const ethValue = orderWithoutFee.takerAssetAmount.dividedToIntegerBy(4); const ethValue = orderWithoutFee.takerAssetAmount.dividedToIntegerBy(4);
return expectTransactionFailedAsync(
tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, {
value: ethValue, value: ethValue,
from: takerAddress, from: takerAddress,
}); }),
const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); RevertReason.CompleteFillFailed,
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 () => { it('should buy an ERC721 asset from a single order', async () => {
const makerAssetId = erc721MakerAssetIds[0]; const makerAssetId = erc721MakerAssetIds[0];
@@ -775,7 +768,7 @@ describe(ContractName.Forwarder, () => {
); );
expect(forwarderEthBalance).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 () => { it('should revert if buying an ERC721 asset when later orders contain different makerAssetData', async () => {
const makerAssetId = erc721MakerAssetIds[0]; const makerAssetId = erc721MakerAssetIds[0];
orderWithoutFee = await orderFactory.newSignedOrderAsync({ orderWithoutFee = await orderFactory.newSignedOrderAsync({
makerAssetAmount: new BigNumber(1), makerAssetAmount: new BigNumber(1),
@@ -786,33 +779,12 @@ describe(ContractName.Forwarder, () => {
const feeOrders: SignedOrder[] = []; const feeOrders: SignedOrder[] = [];
const makerAssetFillAmount = new BigNumber(1).plus(differentMakerAssetDataOrder.makerAssetAmount); const makerAssetFillAmount = new BigNumber(1).plus(differentMakerAssetDataOrder.makerAssetAmount);
const ethValue = orderWithFee.takerAssetAmount; const ethValue = orderWithFee.takerAssetAmount;
return expectTransactionFailedAsync(
tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, {
from: takerAddress, value: ethValue,
value: ethValue, from: takerAddress,
}); }),
const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); RevertReason.CompleteFillFailed,
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 () => { it('should buy an ERC721 asset and pay ZRX fees from a single fee order', async () => {