add multiplexMultiHopSellTokenForToken to metatransactions, with related msgSender changes

contracts/zero-ex/contracts/src/features/MetaTransactionsFeature.sol

@@ -248,6 +248,8 @@ contract MetaTransactionsFeature is
             returnResult = _executeFillRfqOrderCall(state);
         } else if (state.selector == IMultiplexFeature.multiplexBatchSellTokenForToken.selector) {
             returnResult = _executeMultiplexBatchSellTokenForTokenCall(state);
+        } else if (state.selector == IMultiplexFeature.multiplexMultiHopSellTokenForToken.selector) {
+            returnResult = _executeMultiplexMultiHopSellTokenForTokenCall(state);
         } else {
             LibMetaTransactionsRichErrors.MetaTransactionUnsupportedFunctionError(state.hash, state.selector).rrevert();
         }
@@ -429,7 +431,7 @@ contract MetaTransactionsFeature is
 
     /// @dev Execute a `INativeOrdersFeature.fillRfqOrder()` meta-transaction call
     ///      by decoding the call args and translating the call to the internal
-    ///      `INativeOrdersFeature._fillRfqOrder()` variant, where we can overrideunimpleme
+    ///      `INativeOrdersFeature._fillRfqOrder()` variant, where we can override
     ///      the taker address.
     function _executeFillRfqOrderCall(ExecuteState memory state) private returns (bytes memory returnResult) {
         LibNativeOrder.RfqOrder memory order;
@@ -458,6 +460,10 @@ contract MetaTransactionsFeature is
             );
     }
 
+    /// @dev Execute a `IMultiplexFeature.multiplexBatchSellTokenForToken()` meta-transaction
+    ///      call by decoding the call args and translating the call to the internal
+    ///      `IMultiplexFeature._multiplexBatchSell()` variant, where we can override the
+    ///      msgSender address.
     function _executeMultiplexBatchSellTokenForTokenCall(ExecuteState memory state) private returns (bytes memory returnResult) {
         IERC20TokenV06 inputToken;
         IERC20TokenV06 outputToken;
@@ -491,6 +497,41 @@ contract MetaTransactionsFeature is
             );
     }
 
+    /// @dev Execute a `IMultiplexFeature.multiplexMultiHopSellTokenForToken()` meta-transaction
+    ///      call by decoding the call args and translating the call to the internal
+    ///      `IMultiplexFeature._multiplexMultiHopSell()` variant, where we can override the
+    ///      msgSender address.
+    function _executeMultiplexMultiHopSellTokenForTokenCall(ExecuteState memory state) private returns (bytes memory returnResult) {
+        address[] memory tokens;
+        IMultiplexFeature.MultiHopSellSubcall[] memory calls;
+        uint256 sellAmount;
+        uint256 minBuyAmount;
+
+        bytes memory args = _extractArgumentsFromCallData(state.mtx.callData);
+        (tokens, calls, sellAmount, minBuyAmount) = abi.decode(
+            args,
+            (address[], IMultiplexFeature.MultiHopSellSubcall[], uint256, uint256)
+        );
+
+        return
+            _callSelf(
+                state.hash,
+                abi.encodeWithSelector(
+                    IMultiplexFeature._multiplexMultiHopSell.selector,
+                    IMultiplexFeature.MultiHopSellParams({
+                        tokens: tokens,
+                        sellAmount: sellAmount,
+                        calls: calls,
+                        useSelfBalance: false,
+                        recipient: state.mtx.signer,
+                        msgSender: state.mtx.signer
+                    }),
+                    minBuyAmount
+                ),
+                state.mtx.value
+            );
+    }
+
     /// @dev Make an arbitrary internal, meta-transaction call.
     ///      Warning: Do not let unadulterated `callData` into this function.
     function _callSelf(bytes32 hash, bytes memory callData, uint256 value) private returns (bytes memory returnResult) {

contracts/zero-ex/contracts/src/features/UniswapV3Feature.sol

@@ -356,7 +356,7 @@ contract UniswapV3Feature is IFeature, IUniswapV3Feature, FixinCommon, FixinToke
     }
 
     // Convert null address values to fallback.
-    function _normalizeRecipient(address recipient, address alternative) private view returns (address payable normalizedRecipient) {
+    function _normalizeRecipient(address recipient, address alternative) private pure returns (address payable normalizedRecipient) {
         return recipient == address(0) ? payable(alternative) : payable(recipient);
     }
 

contracts/zero-ex/contracts/src/features/interfaces/IMultiplexFeature.sol

@@ -77,6 +77,8 @@ interface IMultiplexFeature {
         bool useSelfBalance;
         // The recipient of the bought output tokens.
         address recipient;
+        // The sender of the transaction.
+        address msgSender;
     }
 
     // Represents a constituent call of a multi-hop sell.
@@ -155,6 +157,12 @@ interface IMultiplexFeature {
         uint256 minBuyAmount
     ) external returns (uint256 boughtAmount);
 
+    /// @dev Executes a multiplex BatchSell using the given
+    ///      parameters. Internal only.
+    /// @param params The parameters for the BatchSell.
+    /// @param minBuyAmount The minimum amount of `params.outputToken`
+    ///        that must be bought for this function to not revert.
+    /// @return boughtAmount The amount of `params.outputToken` bought.
     function _multiplexBatchSell(
         BatchSellParams memory params,
         uint256 minBuyAmount
@@ -211,4 +219,15 @@ interface IMultiplexFeature {
         uint256 sellAmount,
         uint256 minBuyAmount
     ) external returns (uint256 boughtAmount);
+
+    /// @dev Executes a multiplex MultiHopSell using the given
+    ///      parameters. Internal only.
+    /// @param params The parameters for the MultiHopSell.
+    /// @param minBuyAmount The minimum amount of the output token
+    ///        that must be bought for this function to not revert.
+    /// @return boughtAmount The amount of the output token bought.
+    function _multiplexMultiHopSell(
+        MultiHopSellParams memory params,
+        uint256 minBuyAmount
+    ) external returns (uint256 boughtAmount);
 }

contracts/zero-ex/contracts/src/features/multiplex/MultiplexFeature.sol

@@ -80,10 +80,11 @@ contract MultiplexFeature is
         _registerFeatureFunction(this.multiplexBatchSellEthForToken.selector);
         _registerFeatureFunction(this.multiplexBatchSellTokenForEth.selector);
         _registerFeatureFunction(this.multiplexBatchSellTokenForToken.selector);
+        _registerFeatureFunction(this._multiplexBatchSell.selector);
         _registerFeatureFunction(this.multiplexMultiHopSellEthForToken.selector);
         _registerFeatureFunction(this.multiplexMultiHopSellTokenForEth.selector);
         _registerFeatureFunction(this.multiplexMultiHopSellTokenForToken.selector);
-        _registerFeatureFunction(this._multiplexBatchSell.selector);
+        _registerFeatureFunction(this._multiplexMultiHopSell.selector);
         return LibMigrate.MIGRATE_SUCCESS;
     }
 
@@ -244,13 +245,14 @@ contract MultiplexFeature is
         // WETH is now held by this contract,
         // so `useSelfBalance` is true.
         return
-            _multiplexMultiHopSell(
+            _multiplexMultiHopSellPrivate(
                 MultiHopSellParams({
                     tokens: tokens,
                     sellAmount: msg.value,
                     calls: calls,
                     useSelfBalance: true,
-                    recipient: msg.sender
+                    recipient: msg.sender,
+                    msgSender: msg.sender
                 }),
                 minBuyAmount
             );
@@ -280,13 +282,14 @@ contract MultiplexFeature is
         );
         // The `recipient of the WETH is set to  this contract, since
         // we must unwrap the WETH and transfer the resulting ETH.
-        boughtAmount = _multiplexMultiHopSell(
+        boughtAmount = _multiplexMultiHopSellPrivate(
             MultiHopSellParams({
                 tokens: tokens,
                 sellAmount: sellAmount,
                 calls: calls,
                 useSelfBalance: false,
-                recipient: address(this)
+                recipient: address(this),
+                msgSender: msg.sender
             }),
             minBuyAmount
         );
@@ -315,25 +318,40 @@ contract MultiplexFeature is
         uint256 minBuyAmount
     ) public override returns (uint256 boughtAmount) {
         return
-            _multiplexMultiHopSell(
+            _multiplexMultiHopSellPrivate(
                 MultiHopSellParams({
                     tokens: tokens,
                     sellAmount: sellAmount,
                     calls: calls,
                     useSelfBalance: false,
-                    recipient: msg.sender
+                    recipient: msg.sender,
+                    msgSender: msg.sender
                 }),
                 minBuyAmount
             );
     }
 
+    /// @dev Executes a multi-hop sell and checks that at least
+    ///      `minBuyAmount` of output tokens were bought. Internal
+    ///      variant.
+    /// @param params Multi-hop sell parameters.
+    /// @param minBuyAmount The minimum amount of output tokens that
+    ///        must be bought for this function to not revert.
+    /// @return boughtAmount The amount of output tokens bought.
+    function _multiplexMultiHopSell(
+        MultiHopSellParams memory params,
+        uint256 minBuyAmount
+    ) public override onlySelf returns (uint256 boughtAmount) {
+        return _multiplexMultiHopSellPrivate(params, minBuyAmount);
+    }
+
     /// @dev Executes a multi-hop sell and checks that at least
     ///      `minBuyAmount` of output tokens were bought.
     /// @param params Multi-hop sell parameters.
     /// @param minBuyAmount The minimum amount of output tokens that
     ///        must be bought for this function to not revert.
     /// @return boughtAmount The amount of output tokens bought.
-    function _multiplexMultiHopSell(
+    function _multiplexMultiHopSellPrivate(
         MultiHopSellParams memory params,
         uint256 minBuyAmount
     ) private returns (uint256 boughtAmount) {
@@ -405,14 +423,14 @@ contract MultiplexFeature is
         // amount of the multi-hop fill.
         state.outputTokenAmount = params.sellAmount;
         // The first call may expect the input tokens to be held by
-        // `msg.sender`, `address(this)`, or some other address.
+        // `msgSender`, `address(this)`, or some other address.
         // Compute the expected address and transfer the input tokens
         // there if necessary.
         state.from = _computeHopTarget(params, 0);
-        // If the input tokens are currently held by `msg.sender` but
+        // If the input tokens are currently held by `msgSender` but
         // the first hop expects them elsewhere, perform a `transferFrom`.
-        if (!params.useSelfBalance && state.from != msg.sender) {
+        if (!params.useSelfBalance && state.from != params.msgSender) {
-            _transferERC20TokensFrom(IERC20TokenV06(params.tokens[0]), msg.sender, state.from, params.sellAmount);
+            _transferERC20TokensFrom(IERC20TokenV06(params.tokens[0]), params.msgSender, state.from, params.sellAmount);
         }
         // If the input tokens are currently held by `address(this)` but
         // the first hop expects them elsewhere, perform a `transfer`.
@@ -429,7 +447,7 @@ contract MultiplexFeature is
             if (subcall.id == MultiplexSubcall.UniswapV2) {
                 _multiHopSellUniswapV2(state, params, subcall.data);
             } else if (subcall.id == MultiplexSubcall.UniswapV3) {
-                _multiHopSellUniswapV3(state, subcall.data);
+                _multiHopSellUniswapV3(state, params, subcall.data);
             } else if (subcall.id == MultiplexSubcall.LiquidityProvider) {
                 _multiHopSellLiquidityProvider(state, params, subcall.data);
             } else if (subcall.id == MultiplexSubcall.BatchSell) {
@@ -461,6 +479,8 @@ contract MultiplexFeature is
         // Likewise, the recipient of the multi-hop sell is
         // equal to the recipient of its containing batch sell.
         multiHopParams.recipient = params.recipient;
+        // The msgSender is the same too.
+        multiHopParams.msgSender = params.msgSender;
         // Execute the nested multi-hop sell.
         uint256 outputTokenAmount = _executeMultiHopSell(multiHopParams).outputTokenAmount;
         // Increment the sold and bought amounts.
@@ -487,7 +507,7 @@ contract MultiplexFeature is
         // If the nested batch sell is the first hop
         // and `useSelfBalance` for the containing multi-
         // hop sell is false, the nested batch sell should
-        // pull tokens from `msg.sender` (so  `batchSellParams.useSelfBalance`
+        // pull tokens from `msgSender` (so  `batchSellParams.useSelfBalance`
         // should be false). Otherwise `batchSellParams.useSelfBalance`
         // should be true.
         batchSellParams.useSelfBalance = state.hopIndex > 0 || params.useSelfBalance;
@@ -495,6 +515,8 @@ contract MultiplexFeature is
         // that should receive the output tokens of the
         // batch sell.
         batchSellParams.recipient = state.to;
+        // msgSender shound be the same too.
+        batchSellParams.msgSender = params.msgSender;
         // Execute the nested batch sell.
         state.outputTokenAmount = _executeBatchSell(batchSellParams).boughtAmount;
     }
@@ -527,25 +549,25 @@ contract MultiplexFeature is
                 // UniswapV3 uses a callback to pull in the tokens being
                 // sold to it. The callback implemented in `UniswapV3Feature`
                 // can either:
-                // - call `transferFrom` to move tokens from `msg.sender` to the
+                // - call `transferFrom` to move tokens from `msgSender` to the
                 //   UniswapV3 pool, or
                 // - call `transfer` to move tokens from `address(this)` to the
                 //   UniswapV3 pool.
                 // A nested batch sell is similar, in that it can either:
-                // - use tokens from `msg.sender`, or
+                // - use tokens from `msgSender`, or
                 // - use tokens held by `address(this)`.
 
                 // Suppose UniswapV3/BatchSell is the first call in the multi-hop
-                // path. The input tokens are either held by `msg.sender`,
+                // path. The input tokens are either held by `msgSender`,
                 // or in the case of `multiplexMultiHopSellEthForToken` WETH is
                 // held by `address(this)`. The target is set accordingly.
 
                 // If this is _not_ the first call in the multi-hop path, we
                 // are dealing with an "intermediate" token in the multi-hop path,
-                // which `msg.sender` may not have an allowance set for. Thus
+                // which `msgSender` may not have an allowance set for. Thus
                 // target must be set to `address(this)` for `i > 0`.
                 if (i == 0 && !params.useSelfBalance) {
-                    target = msg.sender;
+                    target = params.msgSender;
                 } else {
                     target = address(this);
                 }

contracts/zero-ex/contracts/src/features/multiplex/MultiplexUniswapV3.sol

@@ -70,6 +70,7 @@ abstract contract MultiplexUniswapV3 is FixinTokenSpender {
 
     function _multiHopSellUniswapV3(
         IMultiplexFeature.MultiHopSellState memory state,
+        IMultiplexFeature.MultiHopSellParams memory params,
         bytes memory wrappedCallData
     ) internal {
         bool success;
@@ -88,16 +89,16 @@ abstract contract MultiplexUniswapV3 is FixinTokenSpender {
                 )
             );
         } else {
-            // Otherwise, we self-delegatecall the normal variant
+            // Otherwise, we self-delegatecall `_sellTokenForTokenToUniswapV3`,
-            // `sellTokenForTokenToUniswapV3`, which pulls the input token
+            // which pulls the input token from `msgSender`.
-            // from `msg.sender`.
             (success, resultData) = address(this).delegatecall(
                 abi.encodeWithSelector(
-                    IUniswapV3Feature.sellTokenForTokenToUniswapV3.selector,
+                    IUniswapV3Feature._sellTokenForTokenToUniswapV3.selector,
                     wrappedCallData,
                     state.outputTokenAmount,
                     0,
-                    state.to
+                    state.to,
+                    params.msgSender
                 )
             );
         }