From c2347ac79ec670615de5f6b90982670d9bb739ed Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Tue, 28 Jan 2025 12:19:07 +0000 Subject: [PATCH] feat: Add executor and selector to Swap Add tests to Swap Modify ExecutionDispatcher and TychoRouter to account for these changes --- don't change below this line --- ENG-4041 Took 57 minutes Took 10 seconds --- foundry/{src/Swap.sol => lib/LibSwap.sol} | 22 ++++++- foundry/src/ExecutionDispatcher.sol | 35 +++-------- foundry/src/TychoRouter.sol | 12 ++-- foundry/test/ExecutionDispatcher.t.sol | 67 ++++++++++----------- foundry/test/LibSwap.t.sol | 41 +++++++++++++ foundry/test/TychoRouter.t.sol | 72 +++++++++++++++++++---- foundry/test/TychoRouterTestSetup.sol | 13 ++-- 7 files changed, 176 insertions(+), 86 deletions(-) rename foundry/{src/Swap.sol => lib/LibSwap.sol} (65%) create mode 100644 foundry/test/LibSwap.t.sol diff --git a/foundry/src/Swap.sol b/foundry/lib/LibSwap.sol similarity index 65% rename from foundry/src/Swap.sol rename to foundry/lib/LibSwap.sol index 7c78277..f581d8c 100644 --- a/foundry/src/Swap.sol +++ b/foundry/lib/LibSwap.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.28; -library Swap { +library LibSwap { /// Returns the InToken index into an array of tokens function tokenInIndex(bytes calldata swap) internal @@ -29,12 +29,30 @@ library Swap { res = uint24(bytes3(swap[2:5])); } + /// The address of the executor contract + function executor(bytes calldata swap) + internal + pure + returns (address res) + { + res = address(uint160(bytes20(swap[5:25]))); + } + + /// The selector to be used of the executor contract + function executorSelector(bytes calldata swap) + internal + pure + returns (bytes4 res) + { + res = bytes4(swap[25:29]); + } + /// Remaining bytes are interpreted as protocol data function protocolData(bytes calldata swap) internal pure returns (bytes calldata res) { - res = swap[5:]; + res = swap[29:]; } } diff --git a/foundry/src/ExecutionDispatcher.sol b/foundry/src/ExecutionDispatcher.sol index 090368e..8a80d74 100644 --- a/foundry/src/ExecutionDispatcher.sol +++ b/foundry/src/ExecutionDispatcher.sol @@ -51,28 +51,21 @@ contract ExecutionDispatcher { * protocol-specific data required by the executor. */ // slither-disable-next-line dead-code - function _callExecutor(uint256 amount, bytes calldata data) - internal - returns (uint256 calculatedAmount) - { - address executor; - bytes4 decodedSelector; - bytes memory protocolData; - - (executor, decodedSelector, protocolData) = - _decodeExecutorAndSelector(data); - + function _callExecutor( + address executor, + bytes4 selector, + uint256 amount, + bytes calldata data + ) internal returns (uint256 calculatedAmount) { if (!executors[executor]) { revert ExecutionDispatcher__UnapprovedExecutor(); } - bytes4 selector = decodedSelector == bytes4(0) - ? IExecutor.swap.selector - : decodedSelector; + selector = selector == bytes4(0) ? IExecutor.swap.selector : selector; // slither-disable-next-line low-level-calls (bool success, bytes memory result) = executor.delegatecall( - abi.encodeWithSelector(selector, amount, protocolData) + abi.encodeWithSelector(selector, amount, data) ); if (!success) { @@ -87,16 +80,4 @@ contract ExecutionDispatcher { calculatedAmount = abi.decode(result, (uint256)); } - - // slither-disable-next-line dead-code - function _decodeExecutorAndSelector(bytes calldata data) - internal - pure - returns (address executor, bytes4 selector, bytes memory protocolData) - { - require(data.length >= 24, "Invalid data length"); - executor = address(uint160(bytes20(data[:20]))); - selector = bytes4(data[20:24]); - protocolData = data[24:]; - } } diff --git a/foundry/src/TychoRouter.sol b/foundry/src/TychoRouter.sol index 1308868..880aa77 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -11,7 +11,7 @@ import "@permit2/src/interfaces/IAllowanceTransfer.sol"; import "./ExecutionDispatcher.sol"; import "./CallbackVerificationDispatcher.sol"; import "@openzeppelin/contracts/utils/Pausable.sol"; -import {Swap} from "./Swap.sol"; +import {LibSwap} from "../lib/LibSwap.sol"; error TychoRouter__WithdrawalFailed(); error TychoRouter__AddressZero(); @@ -29,7 +29,7 @@ contract TychoRouter is using SafeERC20 for IERC20; using LibPrefixLengthEncodedByteArray for bytes; - using Swap for bytes; + using LibSwap for bytes; //keccak256("NAME_OF_ROLE") : save gas on deployment bytes32 public constant EXECUTOR_SETTER_ROLE = @@ -185,8 +185,12 @@ contract TychoRouter is currentAmountIn = split > 0 ? (amounts[tokenInIndex] * split) / 0xffffff : remainingAmounts[tokenInIndex]; - currentAmountOut = - _callExecutor(currentAmountIn, swapData.protocolData()); + currentAmountOut = _callExecutor( + swapData.executor(), + swapData.executorSelector(), + currentAmountIn, + swapData.protocolData() + ); amounts[tokenOutIndex] += currentAmountOut; remainingAmounts[tokenOutIndex] += currentAmountOut; remainingAmounts[tokenInIndex] -= currentAmountIn; diff --git a/foundry/test/ExecutionDispatcher.t.sol b/foundry/test/ExecutionDispatcher.t.sol index 2e49482..a88dc44 100644 --- a/foundry/test/ExecutionDispatcher.t.sol +++ b/foundry/test/ExecutionDispatcher.t.sol @@ -5,19 +5,13 @@ import "@src/ExecutionDispatcher.sol"; import "./TychoRouterTestSetup.sol"; contract ExecutionDispatcherExposed is ExecutionDispatcher { - function exposedCallExecutor(uint256 amount, bytes calldata data) - external - returns (uint256 calculatedAmount) - { - return _callExecutor(amount, data); - } - - function exposedDecodeExecutorAndSelector(bytes calldata data) - external - pure - returns (address executor, bytes4 selector, bytes memory protocolData) - { - return _decodeExecutorAndSelector(data); + function exposedCallExecutor( + address executor, + bytes4 selector, + uint256 amount, + bytes calldata data + ) external returns (uint256 calculatedAmount) { + return _callExecutor(executor, selector, amount, data); } function exposedSetExecutor(address target) external { @@ -88,10 +82,14 @@ contract ExecutionDispatcherTest is Constants { address(0xe592557AB9F4A75D992283fD6066312FF013ba3d) ); bytes memory data = - hex"e592557AB9F4A75D992283fD6066312FF013ba3dbd0625ab5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593c81c"; + hex"5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593c81c"; uint256 givenAmount = 15 ether; - uint256 amount = - dispatcherExposed.exposedCallExecutor(givenAmount, data); + uint256 amount = dispatcherExposed.exposedCallExecutor( + 0xe592557AB9F4A75D992283fD6066312FF013ba3d, + IExecutor.swap.selector, + givenAmount, + data + ); assert(amount == 35144641819); } @@ -111,10 +109,14 @@ contract ExecutionDispatcherTest is Constants { address(0xe592557AB9F4A75D992283fD6066312FF013ba3d) ); bytes memory data = - hex"e592557AB9F4A75D992283fD6066312FF013ba3d000000005615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593c81c"; + hex"5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593c81c"; uint256 givenAmount = 15 ether; - uint256 amount = - dispatcherExposed.exposedCallExecutor(givenAmount, data); + uint256 amount = dispatcherExposed.exposedCallExecutor( + 0xe592557AB9F4A75D992283fD6066312FF013ba3d, + bytes4(0), + givenAmount, + data + ); assert(amount == 35144641819); } @@ -124,26 +126,21 @@ contract ExecutionDispatcherTest is Constants { address(0xe592557AB9F4A75D992283fD6066312FF013ba3d) ); bytes memory data = - hex"e592557AB9F4A75D992283fD6066312FF013ba3dbd0625ab5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593"; + hex"5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593"; vm.expectRevert(); - dispatcherExposed.exposedCallExecutor(0, data); + dispatcherExposed.exposedCallExecutor( + 0xe592557AB9F4A75D992283fD6066312FF013ba3d, + IExecutor.swap.selector, + 0, + data + ); } function testCallExecutorUnapprovedExecutor() public { - bytes memory data = - hex"5d622C9053b8FFB1B3465495C8a42E603632bA70aabbccdd1111111111111111"; + bytes memory data = hex"aabbccdd1111111111111111"; vm.expectRevert(); - dispatcherExposed.exposedCallExecutor(0, data); - } - - function testDecodeExecutorAndSelector() public view { - bytes memory data = - hex"6611e616d2db3244244a54c754a16dd3ac7ca7a2aabbccdd1111111111111111"; - (address executor, bytes4 selector, bytes memory protocolData) = - dispatcherExposed.exposedDecodeExecutorAndSelector(data); - assert(executor == address(0x6611e616d2db3244244A54c754A16dd3ac7cA7a2)); - assert(selector == bytes4(0xaabbccdd)); - // Direct bytes comparison not supported - must use keccak - assert(keccak256(protocolData) == keccak256(hex"1111111111111111")); + dispatcherExposed.exposedCallExecutor( + 0x5d622C9053b8FFB1B3465495C8a42E603632bA70, bytes4(0), 0, data + ); } } diff --git a/foundry/test/LibSwap.t.sol b/foundry/test/LibSwap.t.sol new file mode 100644 index 0000000..eedc931 --- /dev/null +++ b/foundry/test/LibSwap.t.sol @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.28; + +import "forge-std/Test.sol"; +import "../lib/LibSwap.sol"; + +contract LibSwapTest is Test { + using LibSwap for bytes; + + function testSwap() public view { + uint8 tokenInIndex = 1; + uint8 tokenOutIndex = 2; + uint24 split = 3; + address executor = 0x1234567890123456789012345678901234567890; + bytes4 selector = 0x12345678; + bytes memory protocolData = abi.encodePacked(uint256(456)); + + bytes memory swap = abi.encodePacked( + tokenInIndex, tokenOutIndex, split, executor, selector, protocolData + ); + this.assertSwap( + swap, tokenInIndex, tokenOutIndex, split, executor, selector + ); + } + + // This is necessary so that the compiler accepts bytes as a LibSwap.sol + function assertSwap( + bytes calldata swap, + uint8 tokenInIndex, + uint8 tokenOutIndex, + uint24 split, + address executor, + bytes4 selector + ) public pure { + assert(swap.tokenInIndex() == tokenInIndex); + assert(swap.tokenOutIndex() == tokenOutIndex); + assert(swap.splitPercentage() == split); + assert(swap.executor() == executor); + assert(swap.executorSelector() == selector); + } +} diff --git a/foundry/test/TychoRouter.t.sol b/foundry/test/TychoRouter.t.sol index 9f3b1b9..caad604 100644 --- a/foundry/test/TychoRouter.t.sol +++ b/foundry/test/TychoRouter.t.sol @@ -222,8 +222,14 @@ contract TychoRouterTest is TychoRouterTestSetup { bytes memory protocolData = encodeUniswapV2Swap(WETH_ADDR, WETH_DAI_POOL, ALICE, false); - bytes memory swap = - encodeSwap(uint8(0), uint8(1), uint24(0), protocolData); + bytes memory swap = encodeSwap( + uint8(0), + uint8(1), + uint24(0), + address(usv2Executor), + bytes4(0), + protocolData + ); bytes[] memory swaps = new bytes[](1); swaps[0] = swap; @@ -247,6 +253,8 @@ contract TychoRouterTest is TychoRouterTestSetup { uint8(0), uint8(1), uint24(0), + address(usv2Executor), + bytes4(0), encodeUniswapV2Swap( WETH_ADDR, WETH_DAI_POOL, address(tychoRouter), false ) @@ -257,6 +265,8 @@ contract TychoRouterTest is TychoRouterTestSetup { uint8(1), uint8(2), uint24(0), + address(usv2Executor), + bytes4(0), encodeUniswapV2Swap(DAI_ADDR, DAI_USDC_POOL, ALICE, true) ); @@ -282,6 +292,8 @@ contract TychoRouterTest is TychoRouterTestSetup { uint8(0), uint8(1), (0xffffff * 60) / 100, // 60% + address(usv2Executor), + bytes4(0), encodeUniswapV2Swap( WETH_ADDR, WETH_WBTC_POOL, address(tychoRouter), false ) @@ -291,6 +303,8 @@ contract TychoRouterTest is TychoRouterTestSetup { uint8(1), uint8(2), uint24(0), + address(usv2Executor), + bytes4(0), encodeUniswapV2Swap(WBTC_ADDR, USDC_WBTC_POOL, ALICE, true) ); // WETH -> DAI @@ -298,6 +312,8 @@ contract TychoRouterTest is TychoRouterTestSetup { uint8(0), uint8(3), uint24(0), + address(usv2Executor), + bytes4(0), encodeUniswapV2Swap( WETH_ADDR, WETH_DAI_POOL, address(tychoRouter), false ) @@ -308,6 +324,8 @@ contract TychoRouterTest is TychoRouterTestSetup { uint8(3), uint8(2), uint24(0), + address(usv2Executor), + bytes4(0), encodeUniswapV2Swap(DAI_ADDR, DAI_USDC_POOL, ALICE, true) ); @@ -335,8 +353,14 @@ contract TychoRouterTest is TychoRouterTestSetup { bytes memory protocolData = encodeUniswapV2Swap(WETH_ADDR, WETH_DAI_POOL, ALICE, false); - bytes memory swap = - encodeSwap(uint8(0), uint8(1), uint24(0), protocolData); + bytes memory swap = encodeSwap( + uint8(0), + uint8(1), + uint24(0), + address(usv2Executor), + bytes4(0), + protocolData + ); bytes[] memory swaps = new bytes[](1); swaps[0] = swap; @@ -381,8 +405,14 @@ contract TychoRouterTest is TychoRouterTestSetup { bytes memory protocolData = encodeUniswapV2Swap(WETH_ADDR, WETH_DAI_POOL, ALICE, false); - bytes memory swap = - encodeSwap(uint8(0), uint8(1), uint24(0), protocolData); + bytes memory swap = encodeSwap( + uint8(0), + uint8(1), + uint24(0), + address(usv2Executor), + bytes4(0), + protocolData + ); bytes[] memory swaps = new bytes[](1); swaps[0] = swap; @@ -434,8 +464,14 @@ contract TychoRouterTest is TychoRouterTestSetup { WETH_ADDR, WETH_DAI_POOL, address(tychoRouter), false ); - bytes memory swap = - encodeSwap(uint8(0), uint8(1), uint24(0), protocolData); + bytes memory swap = encodeSwap( + uint8(0), + uint8(1), + uint24(0), + address(usv2Executor), + bytes4(0), + protocolData + ); bytes[] memory swaps = new bytes[](1); swaps[0] = swap; @@ -484,8 +520,14 @@ contract TychoRouterTest is TychoRouterTestSetup { bytes memory protocolData = encodeUniswapV2Swap(WETH_ADDR, WETH_DAI_POOL, ALICE, false); - bytes memory swap = - encodeSwap(uint8(0), uint8(1), uint24(0), protocolData); + bytes memory swap = encodeSwap( + uint8(0), + uint8(1), + uint24(0), + address(usv2Executor), + bytes4(0), + protocolData + ); bytes[] memory swaps = new bytes[](1); swaps[0] = swap; @@ -528,8 +570,14 @@ contract TychoRouterTest is TychoRouterTestSetup { DAI_ADDR, WETH_DAI_POOL, address(tychoRouter), true ); - bytes memory swap = - encodeSwap(uint8(0), uint8(1), uint24(0), protocolData); + bytes memory swap = encodeSwap( + uint8(0), + uint8(1), + uint24(0), + address(usv2Executor), + bytes4(0), + protocolData + ); bytes[] memory swaps = new bytes[](1); swaps[0] = swap; diff --git a/foundry/test/TychoRouterTestSetup.sol b/foundry/test/TychoRouterTestSetup.sol index 45212f8..f80ea11 100644 --- a/foundry/test/TychoRouterTestSetup.sol +++ b/foundry/test/TychoRouterTestSetup.sol @@ -167,10 +167,13 @@ contract TychoRouterTestSetup is Test, Constants { uint8 tokenInIndex, uint8 tokenOutIndex, uint24 split, + address executor, + bytes4 selector, bytes memory protocolData ) internal pure returns (bytes memory) { - return - abi.encodePacked(tokenInIndex, tokenOutIndex, split, protocolData); + return abi.encodePacked( + tokenInIndex, tokenOutIndex, split, executor, selector, protocolData + ); } function encodeUniswapV2Swap( @@ -178,9 +181,7 @@ contract TychoRouterTestSetup is Test, Constants { address target, address receiver, bool zero2one - ) internal view returns (bytes memory) { - return abi.encodePacked( - usv2Executor, bytes4(0), tokenIn, target, receiver, zero2one - ); + ) internal pure returns (bytes memory) { + return abi.encodePacked(tokenIn, target, receiver, zero2one); } }