Forwarder Market sell specified amount or throw (#2521)

* Forwarder Market sell specified amount or throw

* Address feedback comments

* Break if we have only protocol fee remaining

* Lint

* Update deployed addresses

* Updated artifacts and wrappers

* [asset-swapper] Forwarder throws on market sell if amount not sold (#2534)
This commit is contained in:
Jacob Evans
2020-03-31 16:25:34 +10:00
committed by GitHub
parent 350feed993
commit 424cbd4831
13 changed files with 527 additions and 54 deletions

View File

@@ -151,6 +151,72 @@ contract Forwarder is
_unwrapAndTransferEth(wethRemaining);
}
/// @dev Purchases as much of orders' makerAssets as possible by selling the specified amount of ETH
/// accounting for order and forwarder fees. This functions throws if ethSellAmount was not reached.
/// @param orders Array of order specifications used containing desired makerAsset and WETH as takerAsset.
/// @param ethSellAmount Desired amount of ETH to sell.
/// @param signatures Proofs that orders have been created by makers.
/// @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 wethSpentAmount Amount of WETH spent on the given set of orders.
/// @return makerAssetAcquiredAmount Amount of maker asset acquired from the given set of orders.
function marketSellAmountWithEth(
LibOrder.Order[] memory orders,
uint256 ethSellAmount,
bytes[] memory signatures,
uint256[] memory ethFeeAmounts,
address payable[] memory feeRecipients
)
public
payable
returns (
uint256 wethSpentAmount,
uint256 makerAssetAcquiredAmount
)
{
if (ethSellAmount > msg.value) {
LibRichErrors.rrevert(LibForwarderRichErrors.CompleteSellFailedError(
ethSellAmount,
msg.value
));
}
// Pay ETH affiliate fees to all feeRecipient addresses
uint256 wethRemaining = _transferEthFeesAndWrapRemaining(
ethFeeAmounts,
feeRecipients
);
// Need enough remaining to ensure we can sell ethSellAmount
if (wethRemaining < ethSellAmount) {
LibRichErrors.rrevert(LibForwarderRichErrors.OverspentWethError(
wethRemaining,
ethSellAmount
));
}
// Spends up to ethSellAmount to fill orders, transfers purchased assets to msg.sender,
// and pays WETH order fees.
(
wethSpentAmount,
makerAssetAcquiredAmount
) = _marketSellExactAmountNoThrow(
orders,
ethSellAmount,
signatures
);
// Ensure we sold the specified amount (note: wethSpentAmount includes fees)
if (wethSpentAmount < ethSellAmount) {
LibRichErrors.rrevert(LibForwarderRichErrors.CompleteSellFailedError(
ethSellAmount,
wethSpentAmount
));
}
// Calculate amount of WETH that hasn't been spent.
wethRemaining = wethRemaining.safeSub(wethSpentAmount);
// Refund remaining ETH to msg.sender.
_unwrapAndTransferEth(wethRemaining);
}
/// @dev Attempt to buy makerAssetBuyAmount of makerAsset by selling ETH provided with transaction.
/// The Forwarder may *fill* more than makerAssetBuyAmount of the makerAsset so that it can
/// pay takerFees where takerFeeAssetData == makerAssetData (i.e. percentage fees).

View File

@@ -53,6 +53,7 @@ contract MixinExchangeWrapper {
// ")"
// )));
bytes4 constant public EXCHANGE_V2_ORDER_ID = 0x770501f8;
bytes4 constant internal ERC20_BRIDGE_PROXY_ID = 0xdc1600f3;
// solhint-disable var-name-mixedcase
IExchange internal EXCHANGE;
@@ -73,6 +74,12 @@ contract MixinExchangeWrapper {
EXCHANGE_V2 = IExchangeV2(_exchangeV2);
}
struct SellFillResults {
uint256 wethSpentAmount;
uint256 makerAssetAcquiredAmount;
uint256 protocolFeePaid;
}
/// @dev Fills the input order.
/// Returns false if the transaction would otherwise revert.
/// @param order Order struct containing order specifications.
@@ -115,11 +122,16 @@ contract MixinExchangeWrapper {
uint256 remainingTakerAssetFillAmount
)
internal
returns (
uint256 wethSpentAmount,
uint256 makerAssetAcquiredAmount
)
returns (SellFillResults memory sellFillResults)
{
// If the maker asset is ERC20Bridge, take a snapshot of the Forwarder contract's balance.
bytes4 makerAssetProxyId = order.makerAssetData.readBytes4(0);
address tokenAddress;
uint256 balanceBefore;
if (makerAssetProxyId == ERC20_BRIDGE_PROXY_ID) {
tokenAddress = order.makerAssetData.readAddress(16);
balanceBefore = IERC20Token(tokenAddress).balanceOf(address(this));
}
// No taker fee or percentage fee
if (
order.takerFee == 0 ||
@@ -132,11 +144,11 @@ contract MixinExchangeWrapper {
signature
);
wethSpentAmount = singleFillResults.takerAssetFilledAmount
.safeAdd(singleFillResults.protocolFeePaid);
sellFillResults.wethSpentAmount = singleFillResults.takerAssetFilledAmount;
sellFillResults.protocolFeePaid = singleFillResults.protocolFeePaid;
// Subtract fee from makerAssetFilledAmount for the net amount acquired.
makerAssetAcquiredAmount = singleFillResults.makerAssetFilledAmount
sellFillResults.makerAssetAcquiredAmount = singleFillResults.makerAssetFilledAmount
.safeSub(singleFillResults.takerFeePaid);
// WETH fee
@@ -157,18 +169,27 @@ contract MixinExchangeWrapper {
);
// WETH is also spent on the taker fee, so we add it here.
wethSpentAmount = singleFillResults.takerAssetFilledAmount
.safeAdd(singleFillResults.takerFeePaid)
.safeAdd(singleFillResults.protocolFeePaid);
makerAssetAcquiredAmount = singleFillResults.makerAssetFilledAmount;
sellFillResults.wethSpentAmount = singleFillResults.takerAssetFilledAmount
.safeAdd(singleFillResults.takerFeePaid);
sellFillResults.makerAssetAcquiredAmount = singleFillResults.makerAssetFilledAmount;
sellFillResults.protocolFeePaid = singleFillResults.protocolFeePaid;
// Unsupported fee
} else {
LibRichErrors.rrevert(LibForwarderRichErrors.UnsupportedFeeError(order.takerFeeAssetData));
}
return (wethSpentAmount, makerAssetAcquiredAmount);
// Account for the ERC20Bridge transfering more of the maker asset than expected.
if (makerAssetProxyId == ERC20_BRIDGE_PROXY_ID) {
uint256 balanceAfter = IERC20Token(tokenAddress).balanceOf(address(this));
sellFillResults.makerAssetAcquiredAmount = LibSafeMath.max256(
balanceAfter.safeSub(balanceBefore),
sellFillResults.makerAssetAcquiredAmount
);
}
order.makerAssetData.transferOut(sellFillResults.makerAssetAcquiredAmount);
return sellFillResults;
}
/// @dev Synchronously executes multiple calls of fillOrder until total amount of WETH has been sold by taker.
@@ -189,7 +210,6 @@ contract MixinExchangeWrapper {
)
{
uint256 protocolFee = tx.gasprice.safeMul(EXCHANGE.protocolFeeMultiplier());
bytes4 erc20BridgeProxyId = IAssetData(address(0)).ERC20Bridge.selector;
for (uint256 i = 0; i != orders.length; i++) {
// Preemptively skip to avoid division by zero in _marketSellSingleOrder
@@ -199,42 +219,27 @@ contract MixinExchangeWrapper {
// The remaining amount of WETH to sell
uint256 remainingTakerAssetFillAmount = wethSellAmount
.safeSub(totalWethSpentAmount)
.safeSub(_isV2Order(orders[i]) ? 0 : protocolFee);
// If the maker asset is ERC20Bridge, take a snapshot of the Forwarder contract's balance.
bytes4 makerAssetProxyId = orders[i].makerAssetData.readBytes4(0);
address tokenAddress;
uint256 balanceBefore;
if (makerAssetProxyId == erc20BridgeProxyId) {
tokenAddress = orders[i].makerAssetData.readAddress(16);
balanceBefore = IERC20Token(tokenAddress).balanceOf(address(this));
.safeSub(totalWethSpentAmount);
uint256 currentProtocolFee = _isV2Order(orders[i]) ? 0 : protocolFee;
if (remainingTakerAssetFillAmount > currentProtocolFee) {
// Do not count the protocol fee as part of the fill amount.
remainingTakerAssetFillAmount = remainingTakerAssetFillAmount.safeSub(currentProtocolFee);
} else {
// Stop if we don't have at least enough ETH to pay another protocol fee.
break;
}
(
uint256 wethSpentAmount,
uint256 makerAssetAcquiredAmount
) = _marketSellSingleOrder(
SellFillResults memory sellFillResults = _marketSellSingleOrder(
orders[i],
signatures[i],
remainingTakerAssetFillAmount
);
// Account for the ERC20Bridge transfering more of the maker asset than expected.
if (makerAssetProxyId == erc20BridgeProxyId) {
uint256 balanceAfter = IERC20Token(tokenAddress).balanceOf(address(this));
makerAssetAcquiredAmount = LibSafeMath.max256(
balanceAfter.safeSub(balanceBefore),
makerAssetAcquiredAmount
);
}
orders[i].makerAssetData.transferOut(makerAssetAcquiredAmount);
totalWethSpentAmount = totalWethSpentAmount
.safeAdd(wethSpentAmount);
.safeAdd(sellFillResults.wethSpentAmount)
.safeAdd(sellFillResults.protocolFeePaid);
totalMakerAssetAcquiredAmount = totalMakerAssetAcquiredAmount
.safeAdd(makerAssetAcquiredAmount);
.safeAdd(sellFillResults.makerAssetAcquiredAmount);
// Stop execution if the entire amount of WETH has been sold
if (totalWethSpentAmount >= wethSellAmount) {
@@ -243,6 +248,56 @@ contract MixinExchangeWrapper {
}
}
/// @dev Synchronously executes multiple calls of fillOrder until total amount of WETH (exclusive of protocol fee)
/// has been sold by taker.
/// @param orders Array of order specifications.
/// @param wethSellAmount Desired amount of WETH to sell.
/// @param signatures Proofs that orders have been signed by makers.
/// @return totalWethSpentAmount Total amount of WETH spent on the given orders.
/// @return totalMakerAssetAcquiredAmount Total amount of maker asset acquired from the given orders.
function _marketSellExactAmountNoThrow(
LibOrder.Order[] memory orders,
uint256 wethSellAmount,
bytes[] memory signatures
)
internal
returns (
uint256 totalWethSpentAmount,
uint256 totalMakerAssetAcquiredAmount
)
{
uint256 totalProtocolFeePaid;
for (uint256 i = 0; i != orders.length; i++) {
// Preemptively skip to avoid division by zero in _marketSellSingleOrder
if (orders[i].makerAssetAmount == 0 || orders[i].takerAssetAmount == 0) {
continue;
}
// The remaining amount of WETH to sell
uint256 remainingTakerAssetFillAmount = wethSellAmount
.safeSub(totalWethSpentAmount);
SellFillResults memory sellFillResults = _marketSellSingleOrder(
orders[i],
signatures[i],
remainingTakerAssetFillAmount
);
totalWethSpentAmount = totalWethSpentAmount
.safeAdd(sellFillResults.wethSpentAmount);
totalMakerAssetAcquiredAmount = totalMakerAssetAcquiredAmount
.safeAdd(sellFillResults.makerAssetAcquiredAmount);
totalProtocolFeePaid = totalProtocolFeePaid.safeAdd(sellFillResults.protocolFeePaid);
// Stop execution if the entire amount of WETH has been sold
if (totalWethSpentAmount >= wethSellAmount) {
break;
}
}
totalWethSpentAmount = totalWethSpentAmount.safeAdd(totalProtocolFeePaid);
}
/// @dev Executes a single call of fillOrder according to the makerAssetBuyAmount and
/// the amount already bought.
/// @param order A single order specification.
@@ -338,8 +393,6 @@ contract MixinExchangeWrapper {
uint256 totalMakerAssetAcquiredAmount
)
{
bytes4 erc20BridgeProxyId = IAssetData(address(0)).ERC20Bridge.selector;
uint256 ordersLength = orders.length;
for (uint256 i = 0; i != ordersLength; i++) {
// Preemptively skip to avoid division by zero in _marketBuySingleOrder
@@ -354,7 +407,7 @@ contract MixinExchangeWrapper {
bytes4 makerAssetProxyId = orders[i].makerAssetData.readBytes4(0);
address tokenAddress;
uint256 balanceBefore;
if (makerAssetProxyId == erc20BridgeProxyId) {
if (makerAssetProxyId == ERC20_BRIDGE_PROXY_ID) {
tokenAddress = orders[i].makerAssetData.readAddress(16);
balanceBefore = IERC20Token(tokenAddress).balanceOf(address(this));
}
@@ -369,7 +422,7 @@ contract MixinExchangeWrapper {
);
// Account for the ERC20Bridge transfering more of the maker asset than expected.
if (makerAssetProxyId == erc20BridgeProxyId) {
if (makerAssetProxyId == ERC20_BRIDGE_PROXY_ID) {
uint256 balanceAfter = IERC20Token(tokenAddress).balanceOf(address(this));
makerAssetAcquiredAmount = LibSafeMath.max256(
balanceAfter.safeSub(balanceBefore),

View File

@@ -29,6 +29,10 @@ library LibForwarderRichErrors {
bytes4 internal constant COMPLETE_BUY_FAILED_ERROR_SELECTOR =
0x91353a0c;
// bytes4(keccak256("CompleteSellFailedError(uint256,uint256)"))
bytes4 internal constant COMPLETE_SELL_FAILED_ERROR_SELECTOR =
0x450a0219;
// bytes4(keccak256("UnsupportedFeeError(bytes)"))
bytes4 internal constant UNSUPPORTED_FEE_ERROR_SELECTOR =
0x31360af1;
@@ -61,6 +65,21 @@ library LibForwarderRichErrors {
);
}
function CompleteSellFailedError(
uint256 expectedAssetSellAmount,
uint256 actualAssetSellAmount
)
internal
pure
returns (bytes memory)
{
return abi.encodeWithSelector(
COMPLETE_SELL_FAILED_ERROR_SELECTOR,
expectedAssetSellAmount,
actualAssetSellAmount
);
}
function UnsupportedFeeError(
bytes memory takerFeeAssetData
)

View File

@@ -448,6 +448,24 @@ blockchainTests('Forwarder integration tests', env => {
});
});
});
blockchainTests.resets('marketSellAmountWithEth', () => {
it('should fail if the supplied amount is not sold', async () => {
const order = await maker.signOrderAsync();
const ethSellAmount = order.takerAssetAmount;
const revertError = new ExchangeForwarderRevertErrors.CompleteSellFailedError(
ethSellAmount,
order.takerAssetAmount.times(0.5).plus(DeploymentManager.protocolFee),
);
await testFactory.marketSellAmountTestAsync([order], ethSellAmount, 0.5, {
revertError,
});
});
it('should sell the supplied amount', async () => {
const order = await maker.signOrderAsync();
const ethSellAmount = order.takerAssetAmount;
await testFactory.marketSellAmountTestAsync([order], ethSellAmount, 1);
});
});
blockchainTests.resets('marketBuyOrdersWithEth without extra fees', () => {
it('should buy the exact amount of makerAsset in a single order', async () => {
const order = await maker.signOrderAsync({

View File

@@ -146,6 +146,51 @@ export class ForwarderTestFactory {
await this._checkResultsAsync(txReceipt, orders, expectedOrderStatuses, expectedBalances);
}
}
public async marketSellAmountTestAsync(
orders: SignedOrder[],
ethSellAmount: BigNumber,
fractionalNumberOfOrdersToFill: number,
options: Partial<MarketSellOptions> = {},
): Promise<void> {
const orderInfoBefore = await Promise.all(
orders.map(order => this._deployment.exchange.getOrderInfo(order).callAsync()),
);
const expectedOrderStatuses = orderInfoBefore.map((orderInfo, i) =>
fractionalNumberOfOrdersToFill >= i + 1 && !(options.noopOrders || []).includes(i)
? OrderStatus.FullyFilled
: orderInfo.orderStatus,
);
const { balances: expectedBalances, wethSpentAmount } = await this._simulateForwarderFillAsync(
orders,
orderInfoBefore,
fractionalNumberOfOrdersToFill,
options,
);
const forwarderFeeAmounts = options.forwarderFeeAmounts || [];
const forwarderFeeRecipientAddresses = options.forwarderFeeRecipientAddresses || [];
const tx = this._forwarder
.marketSellAmountWithEth(
orders,
ethSellAmount,
orders.map(signedOrder => signedOrder.signature),
forwarderFeeAmounts,
forwarderFeeRecipientAddresses,
)
.awaitTransactionSuccessAsync({
value: wethSpentAmount.plus(BigNumber.sum(0, ...forwarderFeeAmounts)),
from: this._taker.address,
});
if (options.revertError !== undefined) {
await expect(tx).to.revertWith(options.revertError);
} else {
const txReceipt = await tx;
await this._checkResultsAsync(txReceipt, orders, expectedOrderStatuses, expectedBalances);
}
}
private async _checkResultsAsync(
txReceipt: TransactionReceiptWithDecodedLogs,