From 5574c368cd9c79363ff3b7bb457692e2013ac4d4 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 2 Dec 2019 15:03:52 -0800 Subject: [PATCH] Allow different ETh fees to be specified for different feeRecipient addresses --- .../contracts/src/MixinForwarderCore.sol | 43 ++++------ .../contracts/src/MixinWeth.sol | 86 +++++++++++-------- .../src/interfaces/IForwarderCore.sol | 8 +- .../src/libs/LibForwarderRichErrors.sol | 30 ++++--- 4 files changed, 88 insertions(+), 79 deletions(-) diff --git a/contracts/exchange-forwarder/contracts/src/MixinForwarderCore.sol b/contracts/exchange-forwarder/contracts/src/MixinForwarderCore.sol index efe0728277..d9d7669b97 100644 --- a/contracts/exchange-forwarder/contracts/src/MixinForwarderCore.sol +++ b/contracts/exchange-forwarder/contracts/src/MixinForwarderCore.sol @@ -63,14 +63,14 @@ contract MixinForwarderCore is /// as possible, accounting for order and forwarder fees. /// @param orders Array of order specifications used containing desired makerAsset and WETH as takerAsset. /// @param signatures Proofs that orders have been created by makers. - /// @param ethFeeAmount Amount of ETH, denominatoed in Wei, that is payed to feeRecipient. + /// @param ethFeeAmounts Amounts of ETH, denominated in Wei, that are payed 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 marketSellOrdersWithEth( LibOrder.Order[] memory orders, bytes[] memory signatures, - uint256 ethFeeAmount, + uint256[] memory ethFeeAmounts, address payable[] memory feeRecipients ) public @@ -80,30 +80,25 @@ contract MixinForwarderCore is uint256 makerAssetAcquiredAmount ) { - // Convert ETH to WETH. - _convertEthToWeth(); + // Pay ETH affiliate fees to all feeRecipient addresses + uint256 wethRemaining = _transferEthFeesAndWrapRemaining( + ethFeeAmounts, + feeRecipients + ); - // Calculate amount of WETH that won't be spent on the forwarder fee. - uint256 wethSellAmount = msg.value.safeSub(ethFeeAmount); - - // Spends up to wethSellAmount to fill orders, transfers purchased assets to msg.sender, + // Spends up to wethRemaining to fill orders, transfers purchased assets to msg.sender, // and pays WETH order fees. ( wethSpentAmount, makerAssetAcquiredAmount ) = _marketSellWeth( orders, - wethSellAmount, + wethRemaining, signatures ); - // Transfer ethFeeAmount to feeRecipient. // Refund remaining ETH to msg.sender. - _transferEthFeeAndRefund( - wethSpentAmount, - ethFeeAmount, - feeRecipients - ); + _transferEthRefund(wethRemaining, wethSpentAmount); } /// @dev Attempt to buy makerAssetBuyAmount of makerAsset by selling ETH provided with transaction. @@ -113,7 +108,7 @@ contract MixinForwarderCore is /// @param orders Array of order specifications used containing desired makerAsset and WETH as takerAsset. /// @param makerAssetBuyAmount Desired amount of makerAsset to purchase. /// @param signatures Proofs that orders have been created by makers. - /// @param ethFeeAmount Amount of ETH, denominatoed in Wei, that is payed to feeRecipient. + /// @param ethFeeAmounts Amounts of ETH, denominated in Wei, that are payed 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. @@ -121,7 +116,7 @@ contract MixinForwarderCore is LibOrder.Order[] memory orders, uint256 makerAssetBuyAmount, bytes[] memory signatures, - uint256 ethFeeAmount, + uint256[] memory ethFeeAmounts, address payable[] memory feeRecipients ) public @@ -131,8 +126,11 @@ contract MixinForwarderCore is uint256 makerAssetAcquiredAmount ) { - // Convert ETH to WETH. - _convertEthToWeth(); + // Pay ETH affiliate fees to all feeRecipient addresses + uint256 wethRemaining = _transferEthFeesAndWrapRemaining( + ethFeeAmounts, + feeRecipients + ); // Attempts to fill the desired amount of makerAsset and trasnfer purchased assets to msg.sender. ( @@ -144,12 +142,7 @@ contract MixinForwarderCore is signatures ); - // Transfer ethFeeAmount to feeRecipient. // Refund remaining ETH to msg.sender. - _transferEthFeeAndRefund( - wethSpentAmount, - ethFeeAmount, - feeRecipients - ); + _transferEthRefund(wethRemaining, wethSpentAmount); } } diff --git a/contracts/exchange-forwarder/contracts/src/MixinWeth.sol b/contracts/exchange-forwarder/contracts/src/MixinWeth.sol index 51663eb7ad..e0e4758ab0 100644 --- a/contracts/exchange-forwarder/contracts/src/MixinWeth.sol +++ b/contracts/exchange-forwarder/contracts/src/MixinWeth.sol @@ -41,31 +41,61 @@ contract MixinWeth is } } - /// @dev Converts message call's ETH value into WETH. - function _convertEthToWeth() + /// @dev Transfers ETH denominated fees to all feeRecipient addresses + /// @param ethFeeAmounts Amounts of ETH, denominated in Wei, that are payed to corresponding feeRecipients. + /// @param feeRecipients Addresses that will receive ETH when orders are filled. + /// @return ethRemaining msg.value minus the amount of ETH spent on affiliate fees. + function _transferEthFeesAndWrapRemaining( + uint256[] memory ethFeeAmounts, + address payable[] memory feeRecipients + ) internal + returns (uint256 ethRemaining) { - if (msg.value == 0) { - LibRichErrors.rrevert(LibForwarderRichErrors.MsgValueCannotEqualZeroError()); + uint256 feesLen = ethFeeAmounts.length; + // ethFeeAmounts len must equal feeRecipients len + if (feesLen != feeRecipients.length) { + LibRichErrors.rrevert(LibForwarderRichErrors.EthFeeLengthMismatchError( + feesLen, + feeRecipients.length + )); } - ETHER_TOKEN.deposit.value(msg.value)(); + + // This function is always called before any other function, so we assume that + // the ETH remaining is the entire msg.value. + ethRemaining = msg.value; + + for (uint256 i = 0; i != feesLen; i++) { + uint256 ethFeeAmount = ethFeeAmounts[i]; + // Ensure there is enough ETH to pay the fee + if (ethRemaining < ethFeeAmount) { + LibRichErrors.rrevert(LibForwarderRichErrors.InsufficientEthForFeeError( + ethFeeAmount, + ethRemaining + )); + } + // Decrease ethRemaining and transfer fee to corresponding feeRecipient + ethRemaining = ethRemaining.safeSub(ethFeeAmount); + feeRecipients[i].transfer(ethFeeAmount); + } + + // Convert remaining ETH to WETH. + ETHER_TOKEN.deposit.value(ethRemaining)(); + + return ethRemaining; } - /// @dev Transfers feePercentage of WETH spent on primary orders to feeRecipient. - /// Refunds any excess ETH to msg.sender. + /// @dev Refunds any excess ETH to msg.sender. + /// @param initialWethAmount Amount of WETH available after transferring affiliate fees. /// @param wethSpent Amount of WETH spent when filling orders. - /// @param ethFeeAmount Amount of ETH, denominatoed in Wei, that is payed to feeRecipient. - /// @param feeRecipients Addresses that will receive ETH when orders are filled. - /// @return ethFee Amount paid to feeRecipient as a percentage fee on the total WETH sold. - function _transferEthFeeAndRefund( - uint256 wethSpent, - uint256 ethFeeAmount, - address payable[] memory feeRecipients + function _transferEthRefund( + uint256 initialWethAmount, + uint256 wethSpent ) internal { // Ensure that no extra WETH owned by this contract has been spent. - if (wethSpent > msg.value) { + if (wethSpent > initialWethAmount) { LibRichErrors.rrevert(LibForwarderRichErrors.OverspentWethError( wethSpent, msg.value @@ -73,35 +103,15 @@ contract MixinWeth is } // Calculate amount of WETH that hasn't been spent. - uint256 wethRemaining = msg.value.safeSub(wethSpent); - - // Ensure fee is less than amount of WETH remaining. - if (ethFeeAmount > wethRemaining) { - LibRichErrors.rrevert(LibForwarderRichErrors.InsufficientEthForFeeError( - ethFeeAmount, - wethRemaining - )); - } + uint256 wethRemaining = initialWethAmount.safeSub(wethSpent); // Do nothing if no WETH remaining if (wethRemaining > 0) { // Convert remaining WETH to ETH ETHER_TOKEN.withdraw(wethRemaining); - // Pay ETH to feeRecipient - if (ethFeeAmount > 0) { - uint256 feeRecipientsLen = feeRecipients.length; - uint256 ethFeePerRecipient = ethFeeAmount.safeDiv(feeRecipientsLen); - for (uint256 i = 0; i != feeRecipientsLen; i++) { - feeRecipients[i].transfer(ethFeePerRecipient); - } - } - - // Refund remaining ETH to msg.sender. - uint256 ethRefund = wethRemaining.safeSub(ethFeeAmount); - if (ethRefund > 0) { - msg.sender.transfer(ethRefund); - } + // Transfer remaining ETH to sender + msg.sender.transfer(wethRemaining); } } } diff --git a/contracts/exchange-forwarder/contracts/src/interfaces/IForwarderCore.sol b/contracts/exchange-forwarder/contracts/src/interfaces/IForwarderCore.sol index c8759cb119..58a856f6bb 100644 --- a/contracts/exchange-forwarder/contracts/src/interfaces/IForwarderCore.sol +++ b/contracts/exchange-forwarder/contracts/src/interfaces/IForwarderCore.sol @@ -29,14 +29,14 @@ contract IForwarderCore { /// as possible, accounting for order and forwarder fees. /// @param orders Array of order specifications used containing desired makerAsset and WETH as takerAsset. /// @param signatures Proofs that orders have been created by makers. - /// @param ethFeeAmount Amount of ETH, denominatoed in Wei, that is payed to feeRecipient. + /// @param ethFeeAmounts Amounts of ETH, denominated in Wei, that are payed 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 marketSellOrdersWithEth( LibOrder.Order[] memory orders, bytes[] memory signatures, - uint256 ethFeeAmount, + uint256[] memory ethFeeAmounts, address payable[] memory feeRecipients ) public @@ -53,7 +53,7 @@ contract IForwarderCore { /// @param orders Array of order specifications used containing desired makerAsset and WETH as takerAsset. /// @param makerAssetBuyAmount Desired amount of makerAsset to purchase. /// @param signatures Proofs that orders have been created by makers. - /// @param ethFeeAmount Amount of ETH, denominatoed in Wei, that is payed to feeRecipient. + /// @param ethFeeAmounts Amounts of ETH, denominated in Wei, that are payed 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. @@ -61,7 +61,7 @@ contract IForwarderCore { LibOrder.Order[] memory orders, uint256 makerAssetBuyAmount, bytes[] memory signatures, - uint256 ethFeeAmount, + uint256[] memory ethFeeAmounts, address payable[] memory feeRecipients ) public diff --git a/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol b/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol index b0d2599686..75ff5406c5 100644 --- a/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol +++ b/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol @@ -49,13 +49,13 @@ library LibForwarderRichErrors { bytes4 internal constant DEFAULT_FUNCTION_WETH_CONTRACT_ONLY_ERROR_SELECTOR = 0x08b18698; - // bytes4(keccak256("MsgValueCannotEqualZeroError()")) - bytes4 internal constant MSG_VALUE_CANNOT_EQUAL_ZERO_ERROR_SELECTOR = - 0x8c0e562b; - // bytes4(keccak256("Erc721AmountMustEqualOneError(uint256)")) bytes4 internal constant ERC721_AMOUNT_MUST_EQUAL_ONE_ERROR_SELECTOR = 0xbaffa474; + + // bytes4(keccak256("EthFeeLengthMismatchError(uint256,uint256)")) + bytes4 internal constant ETH_FEE_LENGTH_MISMATCH_ERROR_SELECTOR = + 0x3ecb6ceb; // solhint-disable func-name-mixedcase function UnregisteredAssetProxyError() @@ -150,14 +150,6 @@ library LibForwarderRichErrors { ); } - function MsgValueCannotEqualZeroError() - internal - pure - returns (bytes memory) - { - return abi.encodeWithSelector(MSG_VALUE_CANNOT_EQUAL_ZERO_ERROR_SELECTOR); - } - function Erc721AmountMustEqualOneError( uint256 amount ) @@ -171,4 +163,18 @@ library LibForwarderRichErrors { ); } + function EthFeeLengthMismatchError( + uint256 ethFeesLength, + uint256 feeRecipientsLength + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + ETH_FEE_LENGTH_MISMATCH_ERROR_SELECTOR, + ethFeesLength, + feeRecipientsLength + ); + } }