From 655cf91984fb568c5ff02efd498d093155c4e33d Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Tue, 28 Jan 2025 16:13:24 +0000 Subject: [PATCH] feat: Assume that funds will never go straight from a pool to the receiver - The funds will always go through the router - Rename splitSwap for Swap - Improve tests --- don't change below this line --- ENG-4041 Took 24 minutes Took 23 seconds Took 42 seconds --- foundry/lib/openzeppelin-contracts | 2 +- foundry/lib/permit2 | 2 +- foundry/lib/v2-core | 2 +- foundry/src/TychoRouter.sol | 16 ++--- foundry/test/TychoRouter.t.sol | 95 ++++++++++++++------------- foundry/test/TychoRouterTestSetup.sol | 15 +++-- 6 files changed, 69 insertions(+), 63 deletions(-) diff --git a/foundry/lib/openzeppelin-contracts b/foundry/lib/openzeppelin-contracts index acd4ff7..293a3e2 160000 --- a/foundry/lib/openzeppelin-contracts +++ b/foundry/lib/openzeppelin-contracts @@ -1 +1 @@ -Subproject commit acd4ff74de833399287ed6b31b4debf6b2b35527 +Subproject commit 293a3e264ef2d33526bcc5e8a304887ec4966fdc diff --git a/foundry/lib/permit2 b/foundry/lib/permit2 index cc56ad0..5834a2d 160000 --- a/foundry/lib/permit2 +++ b/foundry/lib/permit2 @@ -1 +1 @@ -Subproject commit cc56ad0f3439c502c246fc5cfcc3db92bb8b7219 +Subproject commit 5834a2d12cc85df5b706eaaea02ef74e89738231 diff --git a/foundry/lib/v2-core b/foundry/lib/v2-core index 4dd5906..ee547b1 160000 --- a/foundry/lib/v2-core +++ b/foundry/lib/v2-core @@ -1 +1 @@ -Subproject commit 4dd59067c76dea4a0e8e4bfdda41877a6b16dedc +Subproject commit ee547b17853e71ed4e0101ccfd52e70d5acded58 diff --git a/foundry/src/TychoRouter.sol b/foundry/src/TychoRouter.sol index 616d5c0..bac2f47 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -143,15 +143,12 @@ contract TychoRouter is ); } - amountOut = _splitSwap(amountIn, nTokens, swaps); + amountOut = _swap(amountIn, nTokens, swaps); if (fee > 0) { uint256 feeAmount = (amountOut * fee) / 10000; amountOut -= feeAmount; IERC20(tokenOut).safeTransfer(feeReceiver, feeAmount); - if (!unwrapEth) { - IERC20(tokenOut).safeTransfer(receiver, amountOut); - } } if (minAmountOut > 0 && amountOut < minAmountOut) { @@ -162,14 +159,15 @@ contract TychoRouter is _unwrapETH(amountOut); // slither-disable-next-line arbitrary-send-eth payable(receiver).transfer(amountOut); + } else { + IERC20(tokenOut).safeTransfer(receiver, amountOut); } } - function _splitSwap( - uint256 amountIn, - uint256 nTokens, - bytes calldata swaps_ - ) internal returns (uint256) { + function _swap(uint256 amountIn, uint256 nTokens, bytes calldata swaps_) + internal + returns (uint256) + { uint256 currentAmountIn; uint256 currentAmountOut; uint8 tokenInIndex; diff --git a/foundry/test/TychoRouter.t.sol b/foundry/test/TychoRouter.t.sol index caad604..d736494 100644 --- a/foundry/test/TychoRouter.t.sol +++ b/foundry/test/TychoRouter.t.sol @@ -72,19 +72,19 @@ contract TychoRouterTest is TychoRouterTestSetup { function testWithdrawNative() public { vm.startPrank(FUND_RESCUER); // Send 100 ether to tychoRouter - assertEq(address(tychoRouter).balance, 0); + assertEq(tychoRouterAddr.balance, 0); assertEq(FUND_RESCUER.balance, 0); - vm.deal(address(tychoRouter), 100 ether); + vm.deal(tychoRouterAddr, 100 ether); vm.expectEmit(); emit Withdrawal(address(0), 100 ether, FUND_RESCUER); tychoRouter.withdrawNative(FUND_RESCUER); - assertEq(address(tychoRouter).balance, 0); + assertEq(tychoRouterAddr.balance, 0); assertEq(FUND_RESCUER.balance, 100 ether); vm.stopPrank(); } function testWithdrawNativeFailures() public { - vm.deal(address(tychoRouter), 100 ether); + vm.deal(tychoRouterAddr, 100 ether); vm.startPrank(FUND_RESCUER); vm.expectRevert(TychoRouter__AddressZero.selector); tychoRouter.withdrawNative(address(0)); @@ -99,7 +99,7 @@ contract TychoRouterTest is TychoRouterTestSetup { function testWithdrawERC20Tokens() public { vm.startPrank(BOB); - mintTokens(100 ether, address(tychoRouter)); + mintTokens(100 ether, tychoRouterAddr); vm.stopPrank(); vm.startPrank(FUND_RESCUER); @@ -112,7 +112,7 @@ contract TychoRouterTest is TychoRouterTestSetup { // Check balances after withdrawing for (uint256 i = 0; i < tokens.length; i++) { // slither-disable-next-line calls-loop - assertEq(tokens[i].balanceOf(address(tychoRouter)), 0); + assertEq(tokens[i].balanceOf(tychoRouterAddr), 0); // slither-disable-next-line calls-loop assertEq(tokens[i].balanceOf(FUND_RESCUER), 100 ether); } @@ -120,7 +120,7 @@ contract TychoRouterTest is TychoRouterTestSetup { } function testWithdrawERC20TokensFailures() public { - mintTokens(100 ether, address(tychoRouter)); + mintTokens(100 ether, tychoRouterAddr); IERC20[] memory tokensArray = new IERC20[](3); tokensArray[0] = IERC20(address(tokens[0])); tokensArray[1] = IERC20(address(tokens[1])); @@ -198,29 +198,30 @@ contract TychoRouterTest is TychoRouterTestSetup { tychoRouter.wrapETH{value: amount}(amount); vm.stopPrank(); - assertEq(address(tychoRouter).balance, 0); - assertEq(IERC20(WETH_ADDR).balanceOf(address(tychoRouter)), amount); + assertEq(tychoRouterAddr.balance, 0); + assertEq(IERC20(WETH_ADDR).balanceOf(tychoRouterAddr), amount); } function testUnwrapETH() public { uint256 amount = 1 ether; - deal(WETH_ADDR, address(tychoRouter), amount); + deal(WETH_ADDR, tychoRouterAddr, amount); tychoRouter.unwrapETH(amount); - assertEq(address(tychoRouter).balance, amount); - assertEq(IERC20(WETH_ADDR).balanceOf(address(tychoRouter)), 0); + assertEq(tychoRouterAddr.balance, amount); + assertEq(IERC20(WETH_ADDR).balanceOf(tychoRouterAddr), 0); } - function testSplitSwapSimple() public { + function testSwapSimple() public { // Trade 1 WETH for DAI with 1 swap on Uniswap V2 // 1 WETH -> DAI // (univ2) uint256 amountIn = 1 ether; - deal(WETH_ADDR, address(tychoRouter), amountIn); + deal(WETH_ADDR, tychoRouterAddr, amountIn); - bytes memory protocolData = - encodeUniswapV2Swap(WETH_ADDR, WETH_DAI_POOL, ALICE, false); + bytes memory protocolData = encodeUniswapV2Swap( + WETH_ADDR, WETH_DAI_POOL, tychoRouterAddr, false + ); bytes memory swap = encodeSwap( uint8(0), @@ -233,19 +234,19 @@ contract TychoRouterTest is TychoRouterTestSetup { bytes[] memory swaps = new bytes[](1); swaps[0] = swap; - tychoRouter.splitSwap(amountIn, 2, pleEncode(swaps)); + tychoRouter.ExposedSwap(amountIn, 2, pleEncode(swaps)); - uint256 daiBalance = IERC20(DAI_ADDR).balanceOf(ALICE); + uint256 daiBalance = IERC20(DAI_ADDR).balanceOf(tychoRouterAddr); assertEq(daiBalance, 2630432278145144658455); - assertEq(IERC20(WETH_ADDR).balanceOf(ALICE), 0); + assertEq(IERC20(WETH_ADDR).balanceOf(tychoRouterAddr), 0); } - function testSplitSwapMultipleHops() public { + function testSwapMultipleHops() public { // Trade 1 WETH for USDC through DAI with 2 swaps on Uniswap V2 // 1 WETH -> DAI -> USDC // (univ2) (univ2) uint256 amountIn = 1 ether; - deal(WETH_ADDR, address(tychoRouter), amountIn); + deal(WETH_ADDR, tychoRouterAddr, amountIn); bytes[] memory swaps = new bytes[](2); // WETH -> DAI @@ -256,7 +257,7 @@ contract TychoRouterTest is TychoRouterTestSetup { address(usv2Executor), bytes4(0), encodeUniswapV2Swap( - WETH_ADDR, WETH_DAI_POOL, address(tychoRouter), false + WETH_ADDR, WETH_DAI_POOL, tychoRouterAddr, false ) ); @@ -267,24 +268,24 @@ contract TychoRouterTest is TychoRouterTestSetup { uint24(0), address(usv2Executor), bytes4(0), - encodeUniswapV2Swap(DAI_ADDR, DAI_USDC_POOL, ALICE, true) + encodeUniswapV2Swap(DAI_ADDR, DAI_USDC_POOL, tychoRouterAddr, true) ); - tychoRouter.splitSwap(amountIn, 3, pleEncode(swaps)); + tychoRouter.ExposedSwap(amountIn, 3, pleEncode(swaps)); - uint256 usdcBalance = IERC20(USDC_ADDR).balanceOf(ALICE); + uint256 usdcBalance = IERC20(USDC_ADDR).balanceOf(tychoRouterAddr); assertEq(usdcBalance, 2610580090); - assertEq(IERC20(WETH_ADDR).balanceOf(ALICE), 0); + assertEq(IERC20(WETH_ADDR).balanceOf(tychoRouterAddr), 0); } - function testSplitSwapSplitHops() public { + function testSwapSplitHops() public { // Trade 1 WETH for USDC through DAI and WBTC with 4 swaps on Uniswap V2 // -> DAI -> // 1 WETH USDC // -> WBTC -> // (univ2) (univ2) uint256 amountIn = 1 ether; - deal(WETH_ADDR, address(tychoRouter), amountIn); + deal(WETH_ADDR, tychoRouterAddr, amountIn); bytes[] memory swaps = new bytes[](4); // WETH -> WBTC (60%) @@ -295,7 +296,7 @@ contract TychoRouterTest is TychoRouterTestSetup { address(usv2Executor), bytes4(0), encodeUniswapV2Swap( - WETH_ADDR, WETH_WBTC_POOL, address(tychoRouter), false + WETH_ADDR, WETH_WBTC_POOL, tychoRouterAddr, false ) ); // WBTC -> USDC @@ -305,7 +306,9 @@ contract TychoRouterTest is TychoRouterTestSetup { uint24(0), address(usv2Executor), bytes4(0), - encodeUniswapV2Swap(WBTC_ADDR, USDC_WBTC_POOL, ALICE, true) + encodeUniswapV2Swap( + WBTC_ADDR, USDC_WBTC_POOL, tychoRouterAddr, true + ) ); // WETH -> DAI swaps[2] = encodeSwap( @@ -315,7 +318,7 @@ contract TychoRouterTest is TychoRouterTestSetup { address(usv2Executor), bytes4(0), encodeUniswapV2Swap( - WETH_ADDR, WETH_DAI_POOL, address(tychoRouter), false + WETH_ADDR, WETH_DAI_POOL, tychoRouterAddr, false ) ); @@ -326,14 +329,14 @@ contract TychoRouterTest is TychoRouterTestSetup { uint24(0), address(usv2Executor), bytes4(0), - encodeUniswapV2Swap(DAI_ADDR, DAI_USDC_POOL, ALICE, true) + encodeUniswapV2Swap(DAI_ADDR, DAI_USDC_POOL, tychoRouterAddr, true) ); - tychoRouter.splitSwap(amountIn, 4, pleEncode(swaps)); + tychoRouter.ExposedSwap(amountIn, 4, pleEncode(swaps)); - uint256 usdcBalance = IERC20(USDC_ADDR).balanceOf(ALICE); + uint256 usdcBalance = IERC20(USDC_ADDR).balanceOf(tychoRouterAddr); assertEq(usdcBalance, 2581503157); - assertEq(IERC20(WETH_ADDR).balanceOf(ALICE), 0); + assertEq(IERC20(WETH_ADDR).balanceOf(tychoRouterAddr), 0); } function testSwapChecked() public { @@ -350,8 +353,9 @@ contract TychoRouterTest is TychoRouterTestSetup { bytes memory signature ) = handlePermit2Approval(WETH_ADDR, amountIn); - bytes memory protocolData = - encodeUniswapV2Swap(WETH_ADDR, WETH_DAI_POOL, ALICE, false); + bytes memory protocolData = encodeUniswapV2Swap( + WETH_ADDR, WETH_DAI_POOL, tychoRouterAddr, false + ); bytes memory swap = encodeSwap( uint8(0), @@ -402,8 +406,9 @@ contract TychoRouterTest is TychoRouterTestSetup { bytes memory signature ) = handlePermit2Approval(WETH_ADDR, amountIn); - bytes memory protocolData = - encodeUniswapV2Swap(WETH_ADDR, WETH_DAI_POOL, ALICE, false); + bytes memory protocolData = encodeUniswapV2Swap( + WETH_ADDR, WETH_DAI_POOL, tychoRouterAddr, false + ); bytes memory swap = encodeSwap( uint8(0), @@ -461,7 +466,7 @@ contract TychoRouterTest is TychoRouterTestSetup { ) = handlePermit2Approval(WETH_ADDR, amountIn); bytes memory protocolData = encodeUniswapV2Swap( - WETH_ADDR, WETH_DAI_POOL, address(tychoRouter), false + WETH_ADDR, WETH_DAI_POOL, tychoRouterAddr, false ); bytes memory swap = encodeSwap( @@ -517,8 +522,9 @@ contract TychoRouterTest is TychoRouterTestSetup { spender: address(0), sigDeadline: 0 }); - bytes memory protocolData = - encodeUniswapV2Swap(WETH_ADDR, WETH_DAI_POOL, ALICE, false); + bytes memory protocolData = encodeUniswapV2Swap( + WETH_ADDR, WETH_DAI_POOL, tychoRouterAddr, false + ); bytes memory swap = encodeSwap( uint8(0), @@ -566,9 +572,8 @@ contract TychoRouterTest is TychoRouterTestSetup { bytes memory signature ) = handlePermit2Approval(DAI_ADDR, amountIn); - bytes memory protocolData = encodeUniswapV2Swap( - DAI_ADDR, WETH_DAI_POOL, address(tychoRouter), true - ); + bytes memory protocolData = + encodeUniswapV2Swap(DAI_ADDR, WETH_DAI_POOL, tychoRouterAddr, true); bytes memory swap = encodeSwap( uint8(0), diff --git a/foundry/test/TychoRouterTestSetup.sol b/foundry/test/TychoRouterTestSetup.sol index f80ea11..91edbf1 100644 --- a/foundry/test/TychoRouterTestSetup.sol +++ b/foundry/test/TychoRouterTestSetup.sol @@ -18,16 +18,18 @@ contract TychoRouterExposed is TychoRouter { return _unwrapETH(amount); } - function splitSwap(uint256 amountIn, uint256 nTokens, bytes calldata swaps) - external - returns (uint256) - { - return _splitSwap(amountIn, nTokens, swaps); + function ExposedSwap( + uint256 amountIn, + uint256 nTokens, + bytes calldata swaps + ) external returns (uint256) { + return _swap(amountIn, nTokens, swaps); } } contract TychoRouterTestSetup is Test, Constants { TychoRouterExposed tychoRouter; + address tychoRouterAddr; address permit2Address = address(0x000000000022D473030F116dDEE9F6B43aC78BA3); UniswapV2Executor public usv2Executor; MockERC20[] tokens; @@ -38,6 +40,7 @@ contract TychoRouterTestSetup is Test, Constants { vm.startPrank(ADMIN); tychoRouter = new TychoRouterExposed(permit2Address, WETH_ADDR); + tychoRouterAddr = address(tychoRouter); tychoRouter.grantRole(keccak256("FUND_RESCUER_ROLE"), FUND_RESCUER); tychoRouter.grantRole(keccak256("FEE_SETTER_ROLE"), FEE_SETTER); tychoRouter.grantRole(keccak256("PAUSER_ROLE"), PAUSER); @@ -98,7 +101,7 @@ contract TychoRouterTestSetup is Test, Constants { expiration: uint48(block.timestamp + 1 days), nonce: 0 }), - spender: address(tychoRouter), + spender: tychoRouterAddr, sigDeadline: block.timestamp + 1 days });