From d1f7f6dde129fd366e3f10aae1c23424343baa4a Mon Sep 17 00:00:00 2001 From: royvardhan Date: Wed, 26 Feb 2025 18:05:29 +0530 Subject: [PATCH 1/3] refactor: centralize swap logic to reduce duplication between swap and swapPermit2 --- foundry/src/TychoRouter.sol | 71 +++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/foundry/src/TychoRouter.sol b/foundry/src/TychoRouter.sol index c032a97..0896279 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -151,37 +151,16 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { tokenIn = address(_weth); } - amountOut = _swap(amountIn, nTokens, swaps); - - if (fee > 0) { - uint256 feeAmount = (amountOut * fee) / 10000; - amountOut -= feeAmount; - IERC20(tokenOut).safeTransfer(feeReceiver, feeAmount); - } - - if (minAmountOut > 0 && amountOut < minAmountOut) { - revert TychoRouter__NegativeSlippage(amountOut, minAmountOut); - } - - uint256 leftoverAmountIn; - if (tokenIn == address(0)) { - leftoverAmountIn = address(this).balance; - } else { - leftoverAmountIn = IERC20(tokenIn).balanceOf(address(this)); - } - - if (leftoverAmountIn > 0) { - revert TychoRouter__AmountInNotFullySpent(leftoverAmountIn); - } - - if (unwrapEth) { - _unwrapETH(amountOut); - } - if (tokenOut == address(0)) { - Address.sendValue(payable(receiver), amountOut); - } else { - IERC20(tokenOut).safeTransfer(receiver, amountOut); - } + return _executeSwap( + amountIn, + tokenIn, + tokenOut, + minAmountOut, + unwrapEth, + nTokens, + receiver, + swaps + ); } /** @@ -242,6 +221,36 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { ); } + return _executeSwap( + amountIn, + tokenIn, + tokenOut, + minAmountOut, + unwrapEth, + nTokens, + receiver, + swaps + ); + } + + /** + * @notice Internal implementation of the core swap logic shared between swap() and swapPermit2(). + * + * @notice This function centralizes the swap execution logic. + * @notice For detailed documentation on parameters and behavior, see the documentation for + * swap() and swapPermit2() functions. + * + */ + function _executeSwap( + uint256 amountIn, + address tokenIn, + address tokenOut, + uint256 minAmountOut, + bool unwrapEth, + uint256 nTokens, + address receiver, + bytes calldata swaps + ) internal returns (uint256 amountOut) { amountOut = _swap(amountIn, nTokens, swaps); if (fee > 0) { From ff83693b2872743f1673091fd0906299e8dd1579 Mon Sep 17 00:00:00 2001 From: royvardhan Date: Wed, 26 Feb 2025 20:44:29 +0530 Subject: [PATCH 2/3] refactor: move wrap operation and receiver check to _executeSwap --- foundry/src/TychoRouter.sol | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/foundry/src/TychoRouter.sol b/foundry/src/TychoRouter.sol index 0896279..ac50628 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -141,21 +141,12 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { address receiver, bytes calldata swaps ) external payable whenNotPaused nonReentrant returns (uint256 amountOut) { - if (receiver == address(0)) { - revert TychoRouter__AddressZero(); - } - - // Assume funds already in our router. - if (wrapEth) { - _wrapETH(amountIn); - tokenIn = address(_weth); - } - return _executeSwap( amountIn, tokenIn, tokenOut, minAmountOut, + wrapEth, unwrapEth, nTokens, receiver, @@ -203,15 +194,8 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { bytes calldata signature, bytes calldata swaps ) external payable whenNotPaused nonReentrant returns (uint256 amountOut) { - if (receiver == address(0)) { - revert TychoRouter__AddressZero(); - } - - // For native ETH, assume funds already in our router. Else, transfer and handle approval. - if (wrapEth) { - _wrapETH(amountIn); - tokenIn = address(_weth); - } else if (tokenIn != address(0)) { + // For non native ETH, transfer and handle approval. + if (tokenIn != address(0)) { permit2.permit(msg.sender, permitSingle, signature); permit2.transferFrom( msg.sender, @@ -226,6 +210,7 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { tokenIn, tokenOut, minAmountOut, + wrapEth, unwrapEth, nTokens, receiver, @@ -246,11 +231,21 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { address tokenIn, address tokenOut, uint256 minAmountOut, + bool wrapEth, bool unwrapEth, uint256 nTokens, address receiver, bytes calldata swaps ) internal returns (uint256 amountOut) { + if (receiver == address(0)) { + revert TychoRouter__AddressZero(); + } + // For native ETH, assume funds are already in the router. + if (wrapEth) { + _wrapETH(amountIn); + tokenIn = address(_weth); + } + amountOut = _swap(amountIn, nTokens, swaps); if (fee > 0) { From 07987a3584a38311c770c6007021ca1751879d73 Mon Sep 17 00:00:00 2001 From: royvardhan Date: Wed, 26 Feb 2025 21:31:14 +0530 Subject: [PATCH 3/3] refactor: rm _executeSwap and move core logic back to swap, make swapPermit2 use swap --- foundry/src/TychoRouter.sol | 120 +++++++++++++----------------------- 1 file changed, 44 insertions(+), 76 deletions(-) diff --git a/foundry/src/TychoRouter.sol b/foundry/src/TychoRouter.sol index ac50628..83303e9 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -140,18 +140,47 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { uint256 nTokens, address receiver, bytes calldata swaps - ) external payable whenNotPaused nonReentrant returns (uint256 amountOut) { - return _executeSwap( - amountIn, - tokenIn, - tokenOut, - minAmountOut, - wrapEth, - unwrapEth, - nTokens, - receiver, - swaps - ); + ) public payable whenNotPaused nonReentrant returns (uint256 amountOut) { + if (receiver == address(0)) { + revert TychoRouter__AddressZero(); + } + // Assume funds are already in the router. + if (wrapEth) { + _wrapETH(amountIn); + tokenIn = address(_weth); + } + + amountOut = _swap(amountIn, nTokens, swaps); + + if (fee > 0) { + uint256 feeAmount = (amountOut * fee) / 10000; + amountOut -= feeAmount; + IERC20(tokenOut).safeTransfer(feeReceiver, feeAmount); + } + + if (minAmountOut > 0 && amountOut < minAmountOut) { + revert TychoRouter__NegativeSlippage(amountOut, minAmountOut); + } + + uint256 leftoverAmountIn; + if (tokenIn == address(0)) { + leftoverAmountIn = address(this).balance; + } else { + leftoverAmountIn = IERC20(tokenIn).balanceOf(address(this)); + } + + if (leftoverAmountIn > 0) { + revert TychoRouter__AmountInNotFullySpent(leftoverAmountIn); + } + + if (unwrapEth) { + _unwrapETH(amountOut); + } + if (tokenOut == address(0)) { + Address.sendValue(payable(receiver), amountOut); + } else { + IERC20(tokenOut).safeTransfer(receiver, amountOut); + } } /** @@ -193,8 +222,8 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { IAllowanceTransfer.PermitSingle calldata permitSingle, bytes calldata signature, bytes calldata swaps - ) external payable whenNotPaused nonReentrant returns (uint256 amountOut) { - // For non native ETH, transfer and handle approval. + ) external payable whenNotPaused returns (uint256 amountOut) { + // For native ETH, assume funds already in our router. Else, transfer and handle approval. if (tokenIn != address(0)) { permit2.permit(msg.sender, permitSingle, signature); permit2.transferFrom( @@ -205,7 +234,7 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { ); } - return _executeSwap( + return swap( amountIn, tokenIn, tokenOut, @@ -218,67 +247,6 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { ); } - /** - * @notice Internal implementation of the core swap logic shared between swap() and swapPermit2(). - * - * @notice This function centralizes the swap execution logic. - * @notice For detailed documentation on parameters and behavior, see the documentation for - * swap() and swapPermit2() functions. - * - */ - function _executeSwap( - uint256 amountIn, - address tokenIn, - address tokenOut, - uint256 minAmountOut, - bool wrapEth, - bool unwrapEth, - uint256 nTokens, - address receiver, - bytes calldata swaps - ) internal returns (uint256 amountOut) { - if (receiver == address(0)) { - revert TychoRouter__AddressZero(); - } - // For native ETH, assume funds are already in the router. - if (wrapEth) { - _wrapETH(amountIn); - tokenIn = address(_weth); - } - - amountOut = _swap(amountIn, nTokens, swaps); - - if (fee > 0) { - uint256 feeAmount = (amountOut * fee) / 10000; - amountOut -= feeAmount; - IERC20(tokenOut).safeTransfer(feeReceiver, feeAmount); - } - - if (minAmountOut > 0 && amountOut < minAmountOut) { - revert TychoRouter__NegativeSlippage(amountOut, minAmountOut); - } - - uint256 leftoverAmountIn; - if (tokenIn == address(0)) { - leftoverAmountIn = address(this).balance; - } else { - leftoverAmountIn = IERC20(tokenIn).balanceOf(address(this)); - } - - if (leftoverAmountIn > 0) { - revert TychoRouter__AmountInNotFullySpent(leftoverAmountIn); - } - - if (unwrapEth) { - _unwrapETH(amountOut); - } - if (tokenOut == address(0)) { - Address.sendValue(payable(receiver), amountOut); - } else { - IERC20(tokenOut).safeTransfer(receiver, amountOut); - } - } - /** * @dev Executes sequential swaps as defined by the provided swap graph. *