From e3ac394d27b8adaeb0aa16ffffb286fc31486ef1 Mon Sep 17 00:00:00 2001 From: TAMARA LIPOWSKI Date: Tue, 8 Apr 2025 16:40:01 -0400 Subject: [PATCH] feat: Proper USV3Executor transfer decoding + tests - Properly decode, update tests with proper decoding - Added test case for each transfer method - Also fully tested permit2 transferFrom and it works perfectly. NOTE: UniswapV3 doesn't support NONE as a transfer method. TODO: - Fix integration tests once encoding is implemented. --- .../src/executors/ExecutorTransferMethods.sol | 2 +- foundry/src/executors/UniswapV3Executor.sol | 4 +- foundry/test/TychoRouterTestSetup.sol | 8 +- .../test/executors/UniswapV3Executor.t.sol | 113 ++++++++++++++++-- 4 files changed, 115 insertions(+), 12 deletions(-) diff --git a/foundry/src/executors/ExecutorTransferMethods.sol b/foundry/src/executors/ExecutorTransferMethods.sol index 0e9772e..6eb73a8 100644 --- a/foundry/src/executors/ExecutorTransferMethods.sol +++ b/foundry/src/executors/ExecutorTransferMethods.sol @@ -40,7 +40,7 @@ contract ExecutorTransferMethods { if (method == TransferMethod.TRANSFER) { tokenIn.safeTransfer(receiver, amount); } else if (method == TransferMethod.TRANSFERFROM) { - tokenIn.safeTransferFrom(msg.sender, receiver, amount); + tokenIn.safeTransferFrom(sender, receiver, amount); } else if (method == TransferMethod.TRANSFERPERMIT2) { // Permit2.permit is already called from the TychoRouter permit2.transferFrom( diff --git a/foundry/src/executors/UniswapV3Executor.sol b/foundry/src/executors/UniswapV3Executor.sol index 09521a9..49b95d0 100644 --- a/foundry/src/executors/UniswapV3Executor.sol +++ b/foundry/src/executors/UniswapV3Executor.sol @@ -149,7 +149,7 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods { TransferMethod method ) { - if (data.length != 84) { + if (data.length != 85) { revert UniswapV3Executor__InvalidDataLength(); } tokenIn = address(bytes20(data[0:20])); @@ -158,7 +158,7 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods { receiver = address(bytes20(data[43:63])); target = address(bytes20(data[63:83])); zeroForOne = uint8(data[83]) > 0; - method = TransferMethod.TRANSFER; + method = TransferMethod(uint8(data[84])); } function _makeV3CallbackData( diff --git a/foundry/test/TychoRouterTestSetup.sol b/foundry/test/TychoRouterTestSetup.sol index e9bc79e..df7cc69 100644 --- a/foundry/test/TychoRouterTestSetup.sol +++ b/foundry/test/TychoRouterTestSetup.sol @@ -198,7 +198,13 @@ contract TychoRouterTestSetup is Constants, Permit2TestHelper { ) internal view returns (bytes memory) { IUniswapV3Pool pool = IUniswapV3Pool(target); return abi.encodePacked( - tokenIn, tokenOut, pool.fee(), receiver, target, zero2one + tokenIn, + tokenOut, + pool.fee(), + receiver, + target, + zero2one, + ExecutorTransferMethods.TransferMethod.TRANSFER ); } } diff --git a/foundry/test/executors/UniswapV3Executor.t.sol b/foundry/test/executors/UniswapV3Executor.t.sol index 4628754..8d35c0c 100644 --- a/foundry/test/executors/UniswapV3Executor.t.sol +++ b/foundry/test/executors/UniswapV3Executor.t.sol @@ -2,8 +2,10 @@ pragma solidity ^0.8.26; import "@src/executors/UniswapV3Executor.sol"; +import "@permit2/src/interfaces/IAllowanceTransfer.sol"; import {Test} from "../../lib/forge-std/src/Test.sol"; import {Constants} from "../Constants.sol"; +import {Permit2TestHelper} from "../Permit2TestHelper.sol"; contract UniswapV3ExecutorExposed is UniswapV3Executor { constructor(address _factory, bytes32 _initCode, address _permit2) @@ -36,13 +38,14 @@ contract UniswapV3ExecutorExposed is UniswapV3Executor { } } -contract UniswapV3ExecutorTest is Test, Constants { +contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper { using SafeERC20 for IERC20; UniswapV3ExecutorExposed uniswapV3Exposed; UniswapV3ExecutorExposed pancakeV3Exposed; IERC20 WETH = IERC20(WETH_ADDR); IERC20 DAI = IERC20(DAI_ADDR); + IAllowanceTransfer permit2; function setUp() public { uint256 forkBlock = 17323404; @@ -56,12 +59,19 @@ contract UniswapV3ExecutorTest is Test, Constants { PANCAKEV3_POOL_CODE_INIT_HASH, PERMIT2_ADDRESS ); + permit2 = IAllowanceTransfer(PERMIT2_ADDRESS); } function testDecodeParams() public view { uint24 expectedPoolFee = 500; bytes memory data = abi.encodePacked( - WETH_ADDR, DAI_ADDR, expectedPoolFee, address(2), address(3), false + WETH_ADDR, + DAI_ADDR, + expectedPoolFee, + address(2), + address(3), + false, + ExecutorTransferMethods.TransferMethod.TRANSFER ); ( @@ -113,8 +123,12 @@ contract UniswapV3ExecutorTest is Test, Constants { uint256 initialPoolReserve = IERC20(WETH_ADDR).balanceOf(DAI_WETH_USV3); vm.startPrank(DAI_WETH_USV3); - bytes memory protocolData = - abi.encodePacked(WETH_ADDR, DAI_ADDR, poolFee); + bytes memory protocolData = abi.encodePacked( + WETH_ADDR, + DAI_ADDR, + poolFee, + ExecutorTransferMethods.TransferMethod.TRANSFER + ); uint256 dataOffset = 3; // some offset uint256 dataLength = protocolData.length; @@ -125,7 +139,6 @@ contract UniswapV3ExecutorTest is Test, Constants { dataOffset, dataLength, protocolData, - uint8(ExecutorTransferMethods.TransferMethod.TRANSFER), address(uniswapV3Exposed) // transferFrom sender (irrelevant in this case) ); uniswapV3Exposed.handleCallback(callbackData); @@ -135,6 +148,88 @@ contract UniswapV3ExecutorTest is Test, Constants { assertEq(finalPoolReserve - initialPoolReserve, amountOwed); } + function testSwapWithTransfer() public { + uint256 amountIn = 10 ** 18; + deal(WETH_ADDR, address(uniswapV3Exposed), amountIn); + + uint256 expAmountOut = 1205_128428842122129186; //Swap 1 WETH for 1205.12 DAI + bool zeroForOne = false; + + bytes memory data = encodeUniswapV3Swap( + WETH_ADDR, + DAI_ADDR, + address(this), + DAI_WETH_USV3, + zeroForOne, + ExecutorTransferMethods.TransferMethod.TRANSFER + ); + + uint256 amountOut = uniswapV3Exposed.swap(amountIn, data); + + assertGe(amountOut, expAmountOut); + assertEq(IERC20(WETH_ADDR).balanceOf(address(uniswapV3Exposed)), 0); + assertGe(IERC20(DAI_ADDR).balanceOf(address(this)), expAmountOut); + } + + function testSwapWithTransferFrom() public { + uint256 amountIn = 10 ** 18; + deal(WETH_ADDR, address(this), amountIn); + IERC20(WETH_ADDR).approve(address(uniswapV3Exposed), amountIn); + + uint256 expAmountOut = 1205_128428842122129186; //Swap 1 WETH for 1205.12 DAI + bool zeroForOne = false; + + bytes memory data = encodeUniswapV3Swap( + WETH_ADDR, + DAI_ADDR, + address(this), + DAI_WETH_USV3, + zeroForOne, + ExecutorTransferMethods.TransferMethod.TRANSFERFROM + ); + + uint256 amountOut = uniswapV3Exposed.swap(amountIn, data); + + assertGe(amountOut, expAmountOut); + assertEq(IERC20(WETH_ADDR).balanceOf(address(uniswapV3Exposed)), 0); + assertGe(IERC20(DAI_ADDR).balanceOf(address(this)), expAmountOut); + } + + function testSwapWithPermit2TransferFrom() public { + uint256 amountIn = 10 ** 18; + + uint256 expAmountOut = 1205_128428842122129186; //Swap 1 WETH for 1205.12 DAI + bool zeroForOne = false; + + bytes memory data = encodeUniswapV3Swap( + WETH_ADDR, + DAI_ADDR, + address(this), + DAI_WETH_USV3, + zeroForOne, + ExecutorTransferMethods.TransferMethod.TRANSFERPERMIT2 + ); + + deal(WETH_ADDR, ALICE, amountIn); + vm.startPrank(ALICE); + ( + IAllowanceTransfer.PermitSingle memory permitSingle, + bytes memory signature + ) = handlePermit2Approval( + WETH_ADDR, address(uniswapV3Exposed), amountIn + ); + + // Assume the permit2.approve method will be called from the TychoRouter + // Replicate this secnario in this test. + permit2.permit(ALICE, permitSingle, signature); + uint256 amountOut = uniswapV3Exposed.swap(amountIn, data); + vm.stopPrank(); + + assertGe(amountOut, expAmountOut); + assertEq(IERC20(WETH_ADDR).balanceOf(address(uniswapV3Exposed)), 0); + assertGe(IERC20(DAI_ADDR).balanceOf(address(this)), expAmountOut); + } + function testSwapFailureInvalidTarget() public { uint256 amountIn = 10 ** 18; deal(WETH_ADDR, address(uniswapV3Exposed), amountIn); @@ -147,7 +242,8 @@ contract UniswapV3ExecutorTest is Test, Constants { uint24(3000), address(this), fakePool, - zeroForOne + zeroForOne, + ExecutorTransferMethods.TransferMethod.TRANSFER ); vm.expectRevert(UniswapV3Executor__InvalidTarget.selector); @@ -159,11 +255,12 @@ contract UniswapV3ExecutorTest is Test, Constants { address tokenOut, address receiver, address target, - bool zero2one + bool zero2one, + ExecutorTransferMethods.TransferMethod method ) internal view returns (bytes memory) { IUniswapV3Pool pool = IUniswapV3Pool(target); return abi.encodePacked( - tokenIn, tokenOut, pool.fee(), receiver, target, zero2one + tokenIn, tokenOut, pool.fee(), receiver, target, zero2one, method ); } }