From 5627a1902b74ace7eccce9888b4505f77b827d43 Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 23 Jan 2025 17:04:08 +0000 Subject: [PATCH 1/6] feat: UniswapV2 SwapExecutor --- don't change below this line --- ENG-4033 Took 52 minutes Took 3 minutes Took 5 minutes Took 36 seconds Took 2 minutes Took 30 seconds --- .gitmodules | 3 + README.md | 2 +- foundry/README.md | 14 +-- foundry/lib/v2-core | 1 + foundry/remappings.txt | 3 +- .../src/executors/Uniswapv2SwapExecutor.sol | 73 ++++++++++++ foundry/src/interfaces/ISwapExecutor.sol | 37 +++++++ foundry/test/Constants.sol | 1 + .../executors/UniswapV2SwapExecutor.t.sol | 104 ++++++++++++++++++ 9 files changed, 226 insertions(+), 12 deletions(-) create mode 160000 foundry/lib/v2-core create mode 100644 foundry/src/executors/Uniswapv2SwapExecutor.sol create mode 100644 foundry/src/interfaces/ISwapExecutor.sol create mode 100644 foundry/test/executors/UniswapV2SwapExecutor.t.sol diff --git a/.gitmodules b/.gitmodules index b61da93..b573165 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,3 +4,6 @@ [submodule "foundry/lib/permit2"] path = foundry/lib/permit2 url = https://github.com/Uniswap/permit2 +[submodule "foundry/lib/v2-core"] + path = foundry/lib/v2-core + url = https://github.com/uniswap/v2-core diff --git a/README.md b/README.md index db429f9..49b15fe 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ To run locally, simply install Slither in your conda env and run it inside the f conda create --name tycho-execution python=3.10 conda activate tycho-execution -python3 -m pip install slither-analyzer` +pip install slither-analyzer cd foundry slither . ``` \ No newline at end of file diff --git a/foundry/README.md b/foundry/README.md index 9265b45..ffbfb13 100644 --- a/foundry/README.md +++ b/foundry/README.md @@ -4,10 +4,10 @@ Foundry consists of: -- **Forge**: Ethereum testing framework (like Truffle, Hardhat and DappTools). -- **Cast**: Swiss army knife for interacting with EVM smart contracts, sending transactions and getting chain data. -- **Anvil**: Local Ethereum node, akin to Ganache, Hardhat Network. -- **Chisel**: Fast, utilitarian, and verbose solidity REPL. +- **Forge**: Ethereum testing framework (like Truffle, Hardhat and DappTools). +- **Cast**: Swiss army knife for interacting with EVM smart contracts, sending transactions and getting chain data. +- **Anvil**: Local Ethereum node, akin to Ganache, Hardhat Network. +- **Chisel**: Fast, utilitarian, and verbose solidity REPL. ## Documentation @@ -45,12 +45,6 @@ $ forge snapshot $ anvil ``` -### Deploy - -```shell -$ forge script script/Counter.s.sol:CounterScript --rpc-url --private-key -``` - ### Cast ```shell diff --git a/foundry/lib/v2-core b/foundry/lib/v2-core new file mode 160000 index 0000000..4dd5906 --- /dev/null +++ b/foundry/lib/v2-core @@ -0,0 +1 @@ +Subproject commit 4dd59067c76dea4a0e8e4bfdda41877a6b16dedc diff --git a/foundry/remappings.txt b/foundry/remappings.txt index 0ab4175..f4b9a08 100644 --- a/foundry/remappings.txt +++ b/foundry/remappings.txt @@ -1,4 +1,5 @@ @openzeppelin/=lib/openzeppelin-contracts/ @interfaces/=interfaces/ @permit2/=lib/permit2/ -@src/=src/ \ No newline at end of file +@src/=src/ +@uniswap-v2/=lib/v2-core/ \ No newline at end of file diff --git a/foundry/src/executors/Uniswapv2SwapExecutor.sol b/foundry/src/executors/Uniswapv2SwapExecutor.sol new file mode 100644 index 0000000..c194c80 --- /dev/null +++ b/foundry/src/executors/Uniswapv2SwapExecutor.sol @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.28; + +import "@uniswap-v2/contracts/interfaces/IUniswapV2Pair.sol"; +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import {ISwapExecutor} from "../interfaces/ISwapExecutor.sol"; + +contract UniswapV2SwapExecutor is ISwapExecutor { + using SafeERC20 for IERC20; + + function swap(uint256 givenAmount, bytes calldata data) + external + returns (uint256 calculatedAmount) + { + address target; + address receiver; + bool zeroForOne; + bool exactOut; + IERC20 tokenIn; + + (tokenIn, target, receiver, zeroForOne, exactOut) = _decodeData(data); + calculatedAmount = _getAmountOut(target, givenAmount, zeroForOne); + tokenIn.safeTransfer(target, givenAmount); + + IUniswapV2Pair pool = IUniswapV2Pair(target); + if (zeroForOne) { + pool.swap(0, calculatedAmount, receiver, ""); + } else { + pool.swap(calculatedAmount, 0, receiver, ""); + } + } + + function _decodeData(bytes calldata data) + internal + pure + returns ( + IERC20 inToken, + address target, + address receiver, + bool zeroForOne, + bool exactOut + ) + { + inToken = IERC20(address(bytes20(data[0:20]))); + target = address(bytes20(data[20:40])); + receiver = address(bytes20(data[40:60])); + zeroForOne = uint8(data[60]) > 0; + exactOut = uint8(data[61]) > 0; + } + + function _getAmountOut(address target, uint256 amountIn, bool zeroForOne) + internal + view + returns (uint256 amount) + { + IUniswapV2Pair pair = IUniswapV2Pair(target); + uint112 reserveIn; + uint112 reserveOut; + if (zeroForOne) { + // slither-disable-next-line unused-return + (reserveIn, reserveOut,) = pair.getReserves(); + } else { + // slither-disable-next-line unused-return + (reserveOut, reserveIn,) = pair.getReserves(); + } + + require(reserveIn > 0 && reserveOut > 0, "L"); + uint256 amountInWithFee = amountIn * 997; + uint256 numerator = amountInWithFee * uint256(reserveOut); + uint256 denominator = (uint256(reserveIn) * 1000) + amountInWithFee; + amount = numerator / denominator; + } +} diff --git a/foundry/src/interfaces/ISwapExecutor.sol b/foundry/src/interfaces/ISwapExecutor.sol new file mode 100644 index 0000000..d8bf82c --- /dev/null +++ b/foundry/src/interfaces/ISwapExecutor.sol @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.28; + +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +pragma abicoder v2; + +interface ISwapExecutor { + /** + * @notice Performs a swap on a liquidity pool. + * @dev This method can either take the amount of the input token or the amount + * of the output token that we would like to swap. If called with the amount of + * the input token, the amount of the output token will be returned, and vice + * versa. Whether it is the input or output that is given, is encoded in the data + * parameter. + * + * Note Part of the informal interface is that the executor supports sending the received + * tokens to a receiver address. If the underlying smart contract does not provide this + * functionality consider adding an additional transfer in the implementation. + * + * This function is marked as `payable` to accommodate delegatecalls, which can forward + * a potential `msg.value` to it. + * + * @param givenAmount The amount of either the input token or output token to swap. + * @param data Data that holds information necessary to perform the swap. + * @return calculatedAmount The amount of either the input token or output token + * swapped, depending on the givenAmount inputted. + */ + function swap(uint256 givenAmount, bytes calldata data) + external + returns (uint256 calculatedAmount); +} + +interface ISwapExecutorErrors { + error InvalidParameterLength(uint256); + error UnknownCurveType(uint8); +} diff --git a/foundry/test/Constants.sol b/foundry/test/Constants.sol index dfd2c1b..058a27a 100644 --- a/foundry/test/Constants.sol +++ b/foundry/test/Constants.sol @@ -14,6 +14,7 @@ contract Constants is Test { address DUMMY = makeAddr("dummy"); address WETH_ADDR = address(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2); + address DAI_ADDR = address(0x6B175474E89094C44Da98b954EedeAC495271d0F); /** * @dev Deploys a dummy contract with non-empty bytecode diff --git a/foundry/test/executors/UniswapV2SwapExecutor.t.sol b/foundry/test/executors/UniswapV2SwapExecutor.t.sol new file mode 100644 index 0000000..4b9587a --- /dev/null +++ b/foundry/test/executors/UniswapV2SwapExecutor.t.sol @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.28; + +import "@src/executors/Uniswapv2SwapExecutor.sol"; +import {Test} from "../../lib/forge-std/src/Test.sol"; +import {Constants} from "../Constants.sol"; + +contract UniswapV2SwapExecutorExposed is UniswapV2SwapExecutor { + function decodeParams(bytes calldata data) + external + pure + returns ( + IERC20 inToken, + address target, + address receiver, + bool zeroForOne, + bool exactOut + ) + { + return _decodeData(data); + } + + function getAmountOut(address target, uint256 amountIn, bool zeroForOne) + external + view + returns (uint256 amount) + { + return _getAmountOut(target, amountIn, zeroForOne); + } +} + +contract UniswapV2SwapExecutorTest is + UniswapV2SwapExecutorExposed, + Test, + Constants +{ + using SafeERC20 for IERC20; + + UniswapV2SwapExecutorExposed uniswapV2Exposed; + IERC20 WETH = IERC20(WETH_ADDR); + IERC20 DAI = IERC20(DAI_ADDR); + address WETH_DAI_POOL = 0xA478c2975Ab1Ea89e8196811F51A7B7Ade33eB11; + + function setUp() public { + uint256 forkBlock = 17323404; + vm.createSelectFork(vm.rpcUrl("mainnet"), forkBlock); + uniswapV2Exposed = new UniswapV2SwapExecutorExposed(); + } + + function testDecodeParams() public view { + bytes memory params = + abi.encodePacked(WETH_ADDR, address(2), address(3), false, true); + + ( + IERC20 tokenIn, + address target, + address receiver, + bool zeroForOne, + bool exactOut + ) = uniswapV2Exposed.decodeParams(params); + + assertEq(address(tokenIn), WETH_ADDR); + assertEq(target, address(2)); + assertEq(receiver, address(3)); + assertEq(zeroForOne, false); + assertEq(exactOut, true); + } + + function testAmountOut() public view { + uint256 amountOut = + uniswapV2Exposed.getAmountOut(WETH_DAI_POOL, 10 ** 18, false); + uint256 expAmountOut = 1847751195973566072891; + assertEq(amountOut, expAmountOut); + } + + // triggers a uint112 overflow on purpose + function testAmountOutInt112Overflow() public view { + address target = 0x0B9f5cEf1EE41f8CCCaA8c3b4c922Ab406c980CC; + uint256 amountIn = 83638098812630667483959471576; + + uint256 amountOut = + uniswapV2Exposed.getAmountOut(target, amountIn, true); + + assertGe(amountOut, 0); + } + + function testSwap() public { + uint256 amountIn = 10 ** 18; + uint256 amountOut = 1847751195973566072891; + bool zeroForOne = false; + bool exactOut = true; + bytes memory protocolData = abi.encodePacked( + WETH_ADDR, WETH_DAI_POOL, BOB, zeroForOne, exactOut + ); + + vm.startPrank(ADMIN); + deal(WETH_ADDR, address(uniswapV2Exposed), amountIn); + uniswapV2Exposed.swap(amountIn, protocolData); + vm.stopPrank(); + + uint256 finalBalance = DAI.balanceOf(BOB); + assertGe(finalBalance, amountOut); + } +} From b9f445176924e7f52d5e130f96038cfe8c44ea18 Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Fri, 24 Jan 2025 09:39:06 +0000 Subject: [PATCH 2/6] fix: Remove exactOut logic from Uniswapv2SwapExecutor We don't support it now --- don't change below this line --- ENG-4033 Took 9 minutes --- .../src/executors/Uniswapv2SwapExecutor.sol | 7 ++----- .../executors/UniswapV2SwapExecutor.t.sol | 21 ++++++------------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/foundry/src/executors/Uniswapv2SwapExecutor.sol b/foundry/src/executors/Uniswapv2SwapExecutor.sol index c194c80..a6f8517 100644 --- a/foundry/src/executors/Uniswapv2SwapExecutor.sol +++ b/foundry/src/executors/Uniswapv2SwapExecutor.sol @@ -15,10 +15,9 @@ contract UniswapV2SwapExecutor is ISwapExecutor { address target; address receiver; bool zeroForOne; - bool exactOut; IERC20 tokenIn; - (tokenIn, target, receiver, zeroForOne, exactOut) = _decodeData(data); + (tokenIn, target, receiver, zeroForOne) = _decodeData(data); calculatedAmount = _getAmountOut(target, givenAmount, zeroForOne); tokenIn.safeTransfer(target, givenAmount); @@ -37,15 +36,13 @@ contract UniswapV2SwapExecutor is ISwapExecutor { IERC20 inToken, address target, address receiver, - bool zeroForOne, - bool exactOut + bool zeroForOne ) { inToken = IERC20(address(bytes20(data[0:20]))); target = address(bytes20(data[20:40])); receiver = address(bytes20(data[40:60])); zeroForOne = uint8(data[60]) > 0; - exactOut = uint8(data[61]) > 0; } function _getAmountOut(address target, uint256 amountIn, bool zeroForOne) diff --git a/foundry/test/executors/UniswapV2SwapExecutor.t.sol b/foundry/test/executors/UniswapV2SwapExecutor.t.sol index 4b9587a..88f99ea 100644 --- a/foundry/test/executors/UniswapV2SwapExecutor.t.sol +++ b/foundry/test/executors/UniswapV2SwapExecutor.t.sol @@ -13,8 +13,7 @@ contract UniswapV2SwapExecutorExposed is UniswapV2SwapExecutor { IERC20 inToken, address target, address receiver, - bool zeroForOne, - bool exactOut + bool zeroForOne ) { return _decodeData(data); @@ -49,21 +48,15 @@ contract UniswapV2SwapExecutorTest is function testDecodeParams() public view { bytes memory params = - abi.encodePacked(WETH_ADDR, address(2), address(3), false, true); + abi.encodePacked(WETH_ADDR, address(2), address(3), false); - ( - IERC20 tokenIn, - address target, - address receiver, - bool zeroForOne, - bool exactOut - ) = uniswapV2Exposed.decodeParams(params); + (IERC20 tokenIn, address target, address receiver, bool zeroForOne) = + uniswapV2Exposed.decodeParams(params); assertEq(address(tokenIn), WETH_ADDR); assertEq(target, address(2)); assertEq(receiver, address(3)); assertEq(zeroForOne, false); - assertEq(exactOut, true); } function testAmountOut() public view { @@ -88,10 +81,8 @@ contract UniswapV2SwapExecutorTest is uint256 amountIn = 10 ** 18; uint256 amountOut = 1847751195973566072891; bool zeroForOne = false; - bool exactOut = true; - bytes memory protocolData = abi.encodePacked( - WETH_ADDR, WETH_DAI_POOL, BOB, zeroForOne, exactOut - ); + bytes memory protocolData = + abi.encodePacked(WETH_ADDR, WETH_DAI_POOL, BOB, zeroForOne); vm.startPrank(ADMIN); deal(WETH_ADDR, address(uniswapV2Exposed), amountIn); From ed44f4e993f3856dbeb14cae04acffec72c25524 Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Fri, 24 Jan 2025 16:43:22 +0000 Subject: [PATCH 3/6] fix: Add input validation size in Uniswapv2SwapExecutor --- don't change below this line --- ENG-4033 Took 12 minutes --- foundry/src/executors/Uniswapv2SwapExecutor.sol | 5 +++++ foundry/test/executors/UniswapV2SwapExecutor.t.sol | 10 ++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/foundry/src/executors/Uniswapv2SwapExecutor.sol b/foundry/src/executors/Uniswapv2SwapExecutor.sol index a6f8517..c54e726 100644 --- a/foundry/src/executors/Uniswapv2SwapExecutor.sol +++ b/foundry/src/executors/Uniswapv2SwapExecutor.sol @@ -5,6 +5,8 @@ import "@uniswap-v2/contracts/interfaces/IUniswapV2Pair.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {ISwapExecutor} from "../interfaces/ISwapExecutor.sol"; +error UniswapV2Executor__InvalidDataLength(); + contract UniswapV2SwapExecutor is ISwapExecutor { using SafeERC20 for IERC20; @@ -39,6 +41,9 @@ contract UniswapV2SwapExecutor is ISwapExecutor { bool zeroForOne ) { + if (data.length != 61) { + revert UniswapV2Executor__InvalidDataLength(); + } inToken = IERC20(address(bytes20(data[0:20]))); target = address(bytes20(data[20:40])); receiver = address(bytes20(data[40:60])); diff --git a/foundry/test/executors/UniswapV2SwapExecutor.t.sol b/foundry/test/executors/UniswapV2SwapExecutor.t.sol index 88f99ea..c2011f1 100644 --- a/foundry/test/executors/UniswapV2SwapExecutor.t.sol +++ b/foundry/test/executors/UniswapV2SwapExecutor.t.sol @@ -59,6 +59,14 @@ contract UniswapV2SwapExecutorTest is assertEq(zeroForOne, false); } + function testDecodeParamsInvalidDataLength() public { + bytes memory invalidParams = + abi.encodePacked(WETH_ADDR, address(2), address(3)); + + vm.expectRevert(UniswapV2Executor__InvalidDataLength.selector); + uniswapV2Exposed.decodeParams(invalidParams); + } + function testAmountOut() public view { uint256 amountOut = uniswapV2Exposed.getAmountOut(WETH_DAI_POOL, 10 ** 18, false); @@ -84,10 +92,8 @@ contract UniswapV2SwapExecutorTest is bytes memory protocolData = abi.encodePacked(WETH_ADDR, WETH_DAI_POOL, BOB, zeroForOne); - vm.startPrank(ADMIN); deal(WETH_ADDR, address(uniswapV2Exposed), amountIn); uniswapV2Exposed.swap(amountIn, protocolData); - vm.stopPrank(); uint256 finalBalance = DAI.balanceOf(BOB); assertGe(finalBalance, amountOut); From b3241804c6b5c72dd968fbcc4521912452fd1f7c Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Fri, 24 Jan 2025 16:46:56 +0000 Subject: [PATCH 4/6] chore: Rename Uniswapv2SwapExecutor to UniswapV2Executor --- don't change below this line --- ENG-4033 Took 4 minutes --- ...wapv2SwapExecutor.sol => UniswapV2Executor.sol} | 4 ++-- .../{ISwapExecutor.sol => IExecutor.sol} | 4 ++-- ...2SwapExecutor.t.sol => UniswapV2Executor.t.sol} | 14 +++++--------- 3 files changed, 9 insertions(+), 13 deletions(-) rename foundry/src/executors/{Uniswapv2SwapExecutor.sol => UniswapV2Executor.sol} (95%) rename foundry/src/interfaces/{ISwapExecutor.sol => IExecutor.sol} (96%) rename foundry/test/executors/{UniswapV2SwapExecutor.t.sol => UniswapV2Executor.t.sol} (89%) diff --git a/foundry/src/executors/Uniswapv2SwapExecutor.sol b/foundry/src/executors/UniswapV2Executor.sol similarity index 95% rename from foundry/src/executors/Uniswapv2SwapExecutor.sol rename to foundry/src/executors/UniswapV2Executor.sol index c54e726..ebcde96 100644 --- a/foundry/src/executors/Uniswapv2SwapExecutor.sol +++ b/foundry/src/executors/UniswapV2Executor.sol @@ -3,11 +3,11 @@ pragma solidity ^0.8.28; import "@uniswap-v2/contracts/interfaces/IUniswapV2Pair.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import {ISwapExecutor} from "../interfaces/ISwapExecutor.sol"; +import {IExecutor} from "../interfaces/IExecutor.sol"; error UniswapV2Executor__InvalidDataLength(); -contract UniswapV2SwapExecutor is ISwapExecutor { +contract UniswapV2Executor is IExecutor { using SafeERC20 for IERC20; function swap(uint256 givenAmount, bytes calldata data) diff --git a/foundry/src/interfaces/ISwapExecutor.sol b/foundry/src/interfaces/IExecutor.sol similarity index 96% rename from foundry/src/interfaces/ISwapExecutor.sol rename to foundry/src/interfaces/IExecutor.sol index d8bf82c..560fd54 100644 --- a/foundry/src/interfaces/ISwapExecutor.sol +++ b/foundry/src/interfaces/IExecutor.sol @@ -5,7 +5,7 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; pragma abicoder v2; -interface ISwapExecutor { +interface IExecutor { /** * @notice Performs a swap on a liquidity pool. * @dev This method can either take the amount of the input token or the amount @@ -31,7 +31,7 @@ interface ISwapExecutor { returns (uint256 calculatedAmount); } -interface ISwapExecutorErrors { +interface IExecutorErrors { error InvalidParameterLength(uint256); error UnknownCurveType(uint8); } diff --git a/foundry/test/executors/UniswapV2SwapExecutor.t.sol b/foundry/test/executors/UniswapV2Executor.t.sol similarity index 89% rename from foundry/test/executors/UniswapV2SwapExecutor.t.sol rename to foundry/test/executors/UniswapV2Executor.t.sol index c2011f1..1598a6a 100644 --- a/foundry/test/executors/UniswapV2SwapExecutor.t.sol +++ b/foundry/test/executors/UniswapV2Executor.t.sol @@ -1,11 +1,11 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.28; -import "@src/executors/Uniswapv2SwapExecutor.sol"; +import "@src/executors/UniswapV2Executor.sol"; import {Test} from "../../lib/forge-std/src/Test.sol"; import {Constants} from "../Constants.sol"; -contract UniswapV2SwapExecutorExposed is UniswapV2SwapExecutor { +contract UniswapV2ExecutorExposed is UniswapV2Executor { function decodeParams(bytes calldata data) external pure @@ -28,14 +28,10 @@ contract UniswapV2SwapExecutorExposed is UniswapV2SwapExecutor { } } -contract UniswapV2SwapExecutorTest is - UniswapV2SwapExecutorExposed, - Test, - Constants -{ +contract UniswapV2ExecutorTest is UniswapV2ExecutorExposed, Test, Constants { using SafeERC20 for IERC20; - UniswapV2SwapExecutorExposed uniswapV2Exposed; + UniswapV2ExecutorExposed uniswapV2Exposed; IERC20 WETH = IERC20(WETH_ADDR); IERC20 DAI = IERC20(DAI_ADDR); address WETH_DAI_POOL = 0xA478c2975Ab1Ea89e8196811F51A7B7Ade33eB11; @@ -43,7 +39,7 @@ contract UniswapV2SwapExecutorTest is function setUp() public { uint256 forkBlock = 17323404; vm.createSelectFork(vm.rpcUrl("mainnet"), forkBlock); - uniswapV2Exposed = new UniswapV2SwapExecutorExposed(); + uniswapV2Exposed = new UniswapV2ExecutorExposed(); } function testDecodeParams() public view { From 1ad04bc9e907df040501077967c889b4076e6a62 Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Fri, 24 Jan 2025 16:53:40 +0000 Subject: [PATCH 5/6] chore: After rebase fixes --- don't change below this line --- ENG-4033 Took 5 minutes --- foundry/src/interfaces/IExecutor.sol | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/foundry/src/interfaces/IExecutor.sol b/foundry/src/interfaces/IExecutor.sol index 560fd54..764623a 100644 --- a/foundry/src/interfaces/IExecutor.sol +++ b/foundry/src/interfaces/IExecutor.sol @@ -8,23 +8,17 @@ pragma abicoder v2; interface IExecutor { /** * @notice Performs a swap on a liquidity pool. - * @dev This method can either take the amount of the input token or the amount - * of the output token that we would like to swap. If called with the amount of - * the input token, the amount of the output token will be returned, and vice - * versa. Whether it is the input or output that is given, is encoded in the data - * parameter. + * @dev This method takes the amount of the input token and returns the amount of + * the output token which has been swapped. * * Note Part of the informal interface is that the executor supports sending the received * tokens to a receiver address. If the underlying smart contract does not provide this * functionality consider adding an additional transfer in the implementation. * - * This function is marked as `payable` to accommodate delegatecalls, which can forward - * a potential `msg.value` to it. - * - * @param givenAmount The amount of either the input token or output token to swap. + * @param givenAmount The amount of the input token to swap. * @param data Data that holds information necessary to perform the swap. - * @return calculatedAmount The amount of either the input token or output token - * swapped, depending on the givenAmount inputted. + * @return calculatedAmount The amount of the output token swapped, depending on + * the givenAmount inputted. */ function swap(uint256 givenAmount, bytes calldata data) external @@ -33,5 +27,5 @@ interface IExecutor { interface IExecutorErrors { error InvalidParameterLength(uint256); - error UnknownCurveType(uint8); + error UnknownPoolType(uint8); } From 9c2b205c3093b36510970f44f7b555a034abf38e Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Fri, 24 Jan 2025 17:02:33 +0000 Subject: [PATCH 6/6] chore: Rename all SwapExecutor to Executor only for simplicity --- don't change below this line --- ENG-4033 Took 9 minutes --- foundry/{src => }/interfaces/IExecutor.sol | 0 foundry/interfaces/ISwapExecutor.sol | 31 --------- ...Dispatcher.sol => ExecutionDispatcher.sol} | 42 ++++++------ foundry/src/TychoRouter.sol | 28 ++++---- foundry/src/executors/UniswapV2Executor.sol | 4 +- ...atcher.t.sol => ExecutionDispatcher.t.sol} | 64 +++++++++---------- foundry/test/TychoRouter.t.sol | 8 +-- 7 files changed, 73 insertions(+), 104 deletions(-) rename foundry/{src => }/interfaces/IExecutor.sol (100%) delete mode 100644 foundry/interfaces/ISwapExecutor.sol rename foundry/src/{SwapExecutionDispatcher.sol => ExecutionDispatcher.sol} (64%) rename foundry/test/{SwapExecutionDispatcher.t.sol => ExecutionDispatcher.t.sol} (77%) diff --git a/foundry/src/interfaces/IExecutor.sol b/foundry/interfaces/IExecutor.sol similarity index 100% rename from foundry/src/interfaces/IExecutor.sol rename to foundry/interfaces/IExecutor.sol diff --git a/foundry/interfaces/ISwapExecutor.sol b/foundry/interfaces/ISwapExecutor.sol deleted file mode 100644 index fef5750..0000000 --- a/foundry/interfaces/ISwapExecutor.sol +++ /dev/null @@ -1,31 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.28; - -import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; - -pragma abicoder v2; - -interface ISwapExecutor { - /** - * @notice Performs a swap on a liquidity pool. - * @dev This method takes the amount of the input token and returns the amount of - * the output token which has been swapped. - * - * Note Part of the informal interface is that the executor supports sending the received - * tokens to a receiver address. If the underlying smart contract does not provide this - * functionality consider adding an additional transfer in the implementation. - * - * @param givenAmount The amount of the input token to swap. - * @param data Data that holds information necessary to perform the swap. - * @return calculatedAmount The amount of the output token swapped, depending on - * the givenAmount inputted. - */ - function swap(uint256 givenAmount, bytes calldata data) - external - returns (uint256 calculatedAmount); -} - -interface ISwapExecutorErrors { - error InvalidParameterLength(uint256); - error UnknownPoolType(uint8); -} diff --git a/foundry/src/SwapExecutionDispatcher.sol b/foundry/src/ExecutionDispatcher.sol similarity index 64% rename from foundry/src/SwapExecutionDispatcher.sol rename to foundry/src/ExecutionDispatcher.sol index 99f7743..090368e 100644 --- a/foundry/src/SwapExecutionDispatcher.sol +++ b/foundry/src/ExecutionDispatcher.sol @@ -1,13 +1,13 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.28; -import "@interfaces/ISwapExecutor.sol"; +import "@interfaces/IExecutor.sol"; -error SwapExecutionDispatcher__UnapprovedExecutor(); -error SwapExecutionDispatcher__NonContractExecutor(); +error ExecutionDispatcher__UnapprovedExecutor(); +error ExecutionDispatcher__NonContractExecutor(); /** - * @title SwapExecutionDispatcher - Dispatch swap execution to external contracts + * @title ExecutionDispatcher - Dispatch execution to external contracts * @author PropellerHeads Devs * @dev Provides the ability to delegate execution of swaps to external * contracts. This allows dynamically adding new supported protocols @@ -15,34 +15,34 @@ error SwapExecutionDispatcher__NonContractExecutor(); * be called using delegatecall so they can share state with the main * contract if needed. * - * Note Executor contracts need to implement the ISwapExecutor interface unless + * Note Executor contracts need to implement the IExecutor interface unless * an alternate selector is specified. */ -contract SwapExecutionDispatcher { - mapping(address => bool) public swapExecutors; +contract ExecutionDispatcher { + mapping(address => bool) public executors; event ExecutorSet(address indexed executor); event ExecutorRemoved(address indexed executor); /** - * @dev Adds or replaces an approved swap executor contract address if it is a + * @dev Adds or replaces an approved executor contract address if it is a * contract. - * @param target address of the swap executor contract + * @param target address of the executor contract */ - function _setSwapExecutor(address target) internal { + function _setExecutor(address target) internal { if (target.code.length == 0) { - revert SwapExecutionDispatcher__NonContractExecutor(); + revert ExecutionDispatcher__NonContractExecutor(); } - swapExecutors[target] = true; + executors[target] = true; emit ExecutorSet(target); } /** - * @dev Removes an approved swap executor contract address - * @param target address of the swap executor contract + * @dev Removes an approved executor contract address + * @param target address of the executor contract */ - function _removeSwapExecutor(address target) internal { - delete swapExecutors[target]; + function _removeExecutor(address target) internal { + delete executors[target]; emit ExecutorRemoved(target); } @@ -51,7 +51,7 @@ contract SwapExecutionDispatcher { * protocol-specific data required by the executor. */ // slither-disable-next-line dead-code - function _callSwapExecutor(uint256 amount, bytes calldata data) + function _callExecutor(uint256 amount, bytes calldata data) internal returns (uint256 calculatedAmount) { @@ -62,12 +62,12 @@ contract SwapExecutionDispatcher { (executor, decodedSelector, protocolData) = _decodeExecutorAndSelector(data); - if (!swapExecutors[executor]) { - revert SwapExecutionDispatcher__UnapprovedExecutor(); + if (!executors[executor]) { + revert ExecutionDispatcher__UnapprovedExecutor(); } bytes4 selector = decodedSelector == bytes4(0) - ? ISwapExecutor.swap.selector + ? IExecutor.swap.selector : decodedSelector; // slither-disable-next-line low-level-calls @@ -80,7 +80,7 @@ contract SwapExecutionDispatcher { string( result.length > 0 ? result - : abi.encodePacked("Swap execution failed") + : abi.encodePacked("Execution failed") ) ); } diff --git a/foundry/src/TychoRouter.sol b/foundry/src/TychoRouter.sol index 635c2ea..44d7752 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -5,7 +5,7 @@ import "@openzeppelin/contracts/access/AccessControl.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import "@permit2/src/interfaces/IAllowanceTransfer.sol"; -import "./SwapExecutionDispatcher.sol"; +import "./ExecutionDispatcher.sol"; import "./CallbackVerificationDispatcher.sol"; error TychoRouter__WithdrawalFailed(); @@ -14,7 +14,7 @@ error TychoRouter__NonContractVerifier(); contract TychoRouter is AccessControl, - SwapExecutionDispatcher, + ExecutionDispatcher, CallbackVerificationDispatcher { IAllowanceTransfer public immutable permit2; @@ -92,30 +92,30 @@ contract TychoRouter is } /** - * @dev Entrypoint to add or replace an approved swap executor contract address - * @param target address of the swap executor contract + * @dev Entrypoint to add or replace an approved executor contract address + * @param target address of the executor contract */ - function setSwapExecutor(address target) + function setExecutor(address target) external onlyRole(EXECUTOR_SETTER_ROLE) { - _setSwapExecutor(target); + _setExecutor(target); } /** - * @dev Entrypoint to remove an approved swap executor contract address - * @param target address of the swap executor contract + * @dev Entrypoint to remove an approved executor contract address + * @param target address of the executor contract */ - function removeSwapExecutor(address target) + function removeExecutor(address target) external onlyRole(EXECUTOR_SETTER_ROLE) { - _removeSwapExecutor(target); + _removeExecutor(target); } /** - * @dev Entrypoint to add or replace an approved swap executor contract address - * @param target address of the swap method contract + * @dev Entrypoint to add or replace an approved callback verifier contract address + * @param target address of the callback verifier contract */ function setCallbackVerifier(address target) external @@ -127,8 +127,8 @@ contract TychoRouter is } /** - * @dev Entrypoint to remove an approved swap executor contract address - * @param target address of the swap method contract + * @dev Entrypoint to remove an approved callback verifier contract address + * @param target address of the callback verifier contract */ function removeCallbackVerifier(address target) external diff --git a/foundry/src/executors/UniswapV2Executor.sol b/foundry/src/executors/UniswapV2Executor.sol index ebcde96..01a507e 100644 --- a/foundry/src/executors/UniswapV2Executor.sol +++ b/foundry/src/executors/UniswapV2Executor.sol @@ -1,9 +1,9 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.28; -import "@uniswap-v2/contracts/interfaces/IUniswapV2Pair.sol"; +import "@interfaces/IExecutor.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import {IExecutor} from "../interfaces/IExecutor.sol"; +import "@uniswap-v2/contracts/interfaces/IUniswapV2Pair.sol"; error UniswapV2Executor__InvalidDataLength(); diff --git a/foundry/test/SwapExecutionDispatcher.t.sol b/foundry/test/ExecutionDispatcher.t.sol similarity index 77% rename from foundry/test/SwapExecutionDispatcher.t.sol rename to foundry/test/ExecutionDispatcher.t.sol index c313694..4895d03 100644 --- a/foundry/test/SwapExecutionDispatcher.t.sol +++ b/foundry/test/ExecutionDispatcher.t.sol @@ -1,15 +1,15 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.28; -import "@src/SwapExecutionDispatcher.sol"; +import "@src/ExecutionDispatcher.sol"; import "./TychoRouterTestSetup.sol"; -contract SwapExecutionDispatcherExposed is SwapExecutionDispatcher { - function exposedCallSwapExecutor(uint256 amount, bytes calldata data) +contract ExecutionDispatcherExposed is ExecutionDispatcher { + function exposedCallExecutor(uint256 amount, bytes calldata data) external returns (uint256 calculatedAmount) { - return _callSwapExecutor(amount, data); + return _callExecutor(amount, data); } function exposedDecodeExecutorAndSelector(bytes calldata data) @@ -20,17 +20,17 @@ contract SwapExecutionDispatcherExposed is SwapExecutionDispatcher { return _decodeExecutorAndSelector(data); } - function exposedSetSwapExecutor(address target) external { - _setSwapExecutor(target); + function exposedSetExecutor(address target) external { + _setExecutor(target); } - function exposedRemoveSwapExecutor(address target) external { - _removeSwapExecutor(target); + function exposedRemoveExecutor(address target) external { + _removeExecutor(target); } } -contract SwapExecutionDispatcherTest is Constants { - SwapExecutionDispatcherExposed dispatcherExposed; +contract ExecutionDispatcherTest is Constants { + ExecutionDispatcherExposed dispatcherExposed; event ExecutorSet(address indexed executor); event ExecutorRemoved(address indexed executor); @@ -38,7 +38,7 @@ contract SwapExecutionDispatcherTest is Constants { function setUp() public { uint256 forkBlock = 20673900; vm.createSelectFork(vm.rpcUrl("mainnet"), forkBlock); - dispatcherExposed = new SwapExecutionDispatcherExposed(); + dispatcherExposed = new ExecutionDispatcherExposed(); deal(WETH_ADDR, address(dispatcherExposed), 15 ether); deployDummyContract(); } @@ -47,34 +47,34 @@ contract SwapExecutionDispatcherTest is Constants { vm.expectEmit(); // Define the event we expect to be emitted at the next step emit ExecutorSet(DUMMY); - dispatcherExposed.exposedSetSwapExecutor(DUMMY); - assert(dispatcherExposed.swapExecutors(DUMMY) == true); + dispatcherExposed.exposedSetExecutor(DUMMY); + assert(dispatcherExposed.executors(DUMMY) == true); } function testRemoveExecutor() public { - dispatcherExposed.exposedSetSwapExecutor(DUMMY); + dispatcherExposed.exposedSetExecutor(DUMMY); vm.expectEmit(); // Define the event we expect to be emitted at the next step emit ExecutorRemoved(DUMMY); - dispatcherExposed.exposedRemoveSwapExecutor(DUMMY); - assert(dispatcherExposed.swapExecutors(DUMMY) == false); + dispatcherExposed.exposedRemoveExecutor(DUMMY); + assert(dispatcherExposed.executors(DUMMY) == false); } function testRemoveUnSetExecutor() public { - dispatcherExposed.exposedRemoveSwapExecutor(BOB); - assert(dispatcherExposed.swapExecutors(BOB) == false); + dispatcherExposed.exposedRemoveExecutor(BOB); + assert(dispatcherExposed.executors(BOB) == false); } function testSetExecutorNonContract() public { vm.expectRevert( abi.encodeWithSelector( - SwapExecutionDispatcher__NonContractExecutor.selector + ExecutionDispatcher__NonContractExecutor.selector ) ); - dispatcherExposed.exposedSetSwapExecutor(BOB); + dispatcherExposed.exposedSetExecutor(BOB); } - function testCallSwapExecutor() public { + function testCallExecutor() public { // Test case taken from existing transaction // 0x755d603962b30f416cf3eefae8d55204d6ffdf746465b2a94aca216faab63804 // For this test, we can use any executor and any calldata that we know works @@ -84,18 +84,18 @@ contract SwapExecutionDispatcherTest is Constants { // Thus, we chose a previously-deployed Hashflow executor for simplicity. To // change this test, we can find any of our transactions that succeeded, and // obtain the calldata passed to the executor via Tenderly. - dispatcherExposed.exposedSetSwapExecutor( + dispatcherExposed.exposedSetExecutor( address(0xe592557AB9F4A75D992283fD6066312FF013ba3d) ); bytes memory data = hex"e592557AB9F4A75D992283fD6066312FF013ba3dbd0625ab5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593c81c"; uint256 givenAmount = 15 ether; uint256 amount = - dispatcherExposed.exposedCallSwapExecutor(givenAmount, data); + dispatcherExposed.exposedCallExecutor(givenAmount, data); assert(amount == 35144641819); } - function testCallSwapExecutorNoSelector() public { + function testCallExecutorNoSelector() public { // Test case taken from existing transaction // 0x755d603962b30f416cf3eefae8d55204d6ffdf746465b2a94aca216faab63804 // No selector is passed, so the standard swap selector should be used @@ -107,33 +107,33 @@ contract SwapExecutionDispatcherTest is Constants { // Thus, we chose a previously-deployed Hashflow executor for simplicity. To // change this test, we can find any of our transactions that succeeded, and // obtain the calldata passed to the executor via Tenderly. - dispatcherExposed.exposedSetSwapExecutor( + dispatcherExposed.exposedSetExecutor( address(0xe592557AB9F4A75D992283fD6066312FF013ba3d) ); bytes memory data = hex"e592557AB9F4A75D992283fD6066312FF013ba3d000000005615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593c81c"; uint256 givenAmount = 15 ether; uint256 amount = - dispatcherExposed.exposedCallSwapExecutor(givenAmount, data); + dispatcherExposed.exposedCallExecutor(givenAmount, data); assert(amount == 35144641819); } - function testCallSwapExecutorCallFailed() public { - // Bad data is provided to an approved swap executor - causing the call to fail - dispatcherExposed.exposedSetSwapExecutor( + function testCallExecutorCallFailed() public { + // Bad data is provided to an approved executor - causing the call to fail + dispatcherExposed.exposedSetExecutor( address(0xe592557AB9F4A75D992283fD6066312FF013ba3d) ); bytes memory data = hex"e592557AB9F4A75D992283fD6066312FF013ba3dbd0625ab5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593"; vm.expectRevert(); - dispatcherExposed.exposedCallSwapExecutor(0, data); + dispatcherExposed.exposedCallExecutor(0, data); } - function testCallSwapExecutorUnapprovedExecutor() public { + function testCallExecutorUnapprovedExecutor() public { bytes memory data = hex"5d622C9053b8FFB1B3465495C8a42E603632bA70aabbccdd1111111111111111"; vm.expectRevert(); - dispatcherExposed.exposedCallSwapExecutor(0, data); + dispatcherExposed.exposedCallExecutor(0, data); } function testDecodeExecutorAndSelector() public { diff --git a/foundry/test/TychoRouter.t.sol b/foundry/test/TychoRouter.t.sol index 809586f..d5019bc 100644 --- a/foundry/test/TychoRouter.t.sol +++ b/foundry/test/TychoRouter.t.sol @@ -21,19 +21,19 @@ contract TychoRouterTest is TychoRouterTestSetup { function testSetExecutorValidRole() public { vm.startPrank(executorSetter); - tychoRouter.setSwapExecutor(DUMMY); + tychoRouter.setExecutor(DUMMY); vm.stopPrank(); - assert(tychoRouter.swapExecutors(DUMMY) == true); + assert(tychoRouter.executors(DUMMY) == true); } function testRemoveExecutorMissingSetterRole() public { vm.expectRevert(); - tychoRouter.removeSwapExecutor(BOB); + tychoRouter.removeExecutor(BOB); } function testSetExecutorMissingSetterRole() public { vm.expectRevert(); - tychoRouter.setSwapExecutor(DUMMY); + tychoRouter.setExecutor(DUMMY); } function testSetValidVerifier() public {