From 90b826da61ec51b1a6c898bc587f6de5b807b945 Mon Sep 17 00:00:00 2001 From: abls <112491550+abls@users.noreply.github.com> Date: Fri, 10 Feb 2023 13:04:20 -0800 Subject: [PATCH] add multiplexMultiHopSellTokenForToken to metatransactions, with related msgSender changes --- .../src/features/MetaTransactionsFeature.sol | 43 ++++++++++++- .../src/features/UniswapV3Feature.sol | 2 +- .../features/interfaces/IMultiplexFeature.sol | 19 ++++++ .../features/multiplex/MultiplexFeature.sol | 60 +++++++++++++------ .../features/multiplex/MultiplexUniswapV3.sol | 11 ++-- 5 files changed, 109 insertions(+), 26 deletions(-) diff --git a/contracts/zero-ex/contracts/src/features/MetaTransactionsFeature.sol b/contracts/zero-ex/contracts/src/features/MetaTransactionsFeature.sol index c33cd96f71..06e35eb87f 100644 --- a/contracts/zero-ex/contracts/src/features/MetaTransactionsFeature.sol +++ b/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) { diff --git a/contracts/zero-ex/contracts/src/features/UniswapV3Feature.sol b/contracts/zero-ex/contracts/src/features/UniswapV3Feature.sol index 483555cebb..0e24ea8b94 100644 --- a/contracts/zero-ex/contracts/src/features/UniswapV3Feature.sol +++ b/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); } diff --git a/contracts/zero-ex/contracts/src/features/interfaces/IMultiplexFeature.sol b/contracts/zero-ex/contracts/src/features/interfaces/IMultiplexFeature.sol index 8c03e246da..4a23032891 100644 --- a/contracts/zero-ex/contracts/src/features/interfaces/IMultiplexFeature.sol +++ b/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); } diff --git a/contracts/zero-ex/contracts/src/features/multiplex/MultiplexFeature.sol b/contracts/zero-ex/contracts/src/features/multiplex/MultiplexFeature.sol index 5e265ef160..f6c889798f 100644 --- a/contracts/zero-ex/contracts/src/features/multiplex/MultiplexFeature.sol +++ b/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) { - _transferERC20TokensFrom(IERC20TokenV06(params.tokens[0]), msg.sender, state.from, params.sellAmount); + if (!params.useSelfBalance && state.from != params.msgSender) { + _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); } diff --git a/contracts/zero-ex/contracts/src/features/multiplex/MultiplexUniswapV3.sol b/contracts/zero-ex/contracts/src/features/multiplex/MultiplexUniswapV3.sol index 72ffca05ee..b735a07907 100644 --- a/contracts/zero-ex/contracts/src/features/multiplex/MultiplexUniswapV3.sol +++ b/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 - // `sellTokenForTokenToUniswapV3`, which pulls the input token - // from `msg.sender`. + // Otherwise, we self-delegatecall `_sellTokenForTokenToUniswapV3`, + // which pulls the input token from `msgSender`. (success, resultData) = address(this).delegatecall( abi.encodeWithSelector( - IUniswapV3Feature.sellTokenForTokenToUniswapV3.selector, + IUniswapV3Feature._sellTokenForTokenToUniswapV3.selector, wrappedCallData, state.outputTokenAmount, 0, - state.to + state.to, + params.msgSender ) ); }