Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Commit

Permalink
Forwarder Market sell specified amount or throw (#2521)
Browse files Browse the repository at this point in the history 
* 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)
  • Loading branch information
@dekz
dekz committed Mar 31, 2020
1 parent 350feed commit 424cbd4
Show file tree
Hide file tree
Showing 13 changed files with 524 additions and 51 deletions.
66 changes: 66 additions & 0 deletions contracts/exchange-forwarder/contracts/src/Forwarder.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
137 changes: 95 additions & 42 deletions contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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 ||
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -199,48 +219,83 @@ 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
);
totalWethSpentAmount = totalWethSpentAmount
.safeAdd(sellFillResults.wethSpentAmount)
.safeAdd(sellFillResults.protocolFeePaid);
totalMakerAssetAcquiredAmount = totalMakerAssetAcquiredAmount
.safeAdd(sellFillResults.makerAssetAcquiredAmount);

// Stop execution if the entire amount of WETH has been sold
if (totalWethSpentAmount >= wethSellAmount) {
break;
}
}
}

orders[i].makerAssetData.transferOut(makerAssetAcquiredAmount);
/// @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(wethSpentAmount);
.safeAdd(sellFillResults.wethSpentAmount);
totalMakerAssetAcquiredAmount = totalMakerAssetAcquiredAmount
.safeAdd(makerAssetAcquiredAmount);
.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
Expand Down Expand Up @@ -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
Expand All @@ -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));
}
Expand All @@ -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),
Expand Down
19 changes: 19 additions & 0 deletions contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
)
Expand Down
18 changes: 18 additions & 0 deletions contracts/integrations/test/forwarder/forwarder_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
Loading

0 comments on commit 424cbd4

Please sign in to comment.