diff --git a/foundry/interfaces/ISwapExecutor.sol b/foundry/interfaces/ISwapExecutor.sol new file mode 100644 index 0000000..fef5750 --- /dev/null +++ b/foundry/interfaces/ISwapExecutor.sol @@ -0,0 +1,31 @@ +// 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/remappings.txt b/foundry/remappings.txt index f1fbdf6..0ab4175 100644 --- a/foundry/remappings.txt +++ b/foundry/remappings.txt @@ -1,3 +1,4 @@ @openzeppelin/=lib/openzeppelin-contracts/ +@interfaces/=interfaces/ @permit2/=lib/permit2/ @src/=src/ \ No newline at end of file diff --git a/foundry/src/SwapExecutionDispatcher.sol b/foundry/src/SwapExecutionDispatcher.sol index 3173104..99f7743 100644 --- a/foundry/src/SwapExecutionDispatcher.sol +++ b/foundry/src/SwapExecutionDispatcher.sol @@ -1,6 +1,11 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.28; +import "@interfaces/ISwapExecutor.sol"; + +error SwapExecutionDispatcher__UnapprovedExecutor(); +error SwapExecutionDispatcher__NonContractExecutor(); + /** * @title SwapExecutionDispatcher - Dispatch swap execution to external contracts * @author PropellerHeads Devs @@ -10,8 +15,88 @@ pragma solidity ^0.8.28; * be called using delegatecall so they can share state with the main * contract if needed. * - * Note Executor contracts need to implement the ISwapExecutor interface + * Note Executor contracts need to implement the ISwapExecutor interface unless + * an alternate selector is specified. */ contract SwapExecutionDispatcher { mapping(address => bool) public swapExecutors; + + event ExecutorSet(address indexed executor); + event ExecutorRemoved(address indexed executor); + + /** + * @dev Adds or replaces an approved swap executor contract address if it is a + * contract. + * @param target address of the swap executor contract + */ + function _setSwapExecutor(address target) internal { + if (target.code.length == 0) { + revert SwapExecutionDispatcher__NonContractExecutor(); + } + swapExecutors[target] = true; + emit ExecutorSet(target); + } + + /** + * @dev Removes an approved swap executor contract address + * @param target address of the swap executor contract + */ + function _removeSwapExecutor(address target) internal { + delete swapExecutors[target]; + emit ExecutorRemoved(target); + } + + /** + * @dev Calls an executor, assumes swap.protocolData contains + * protocol-specific data required by the executor. + */ + // slither-disable-next-line dead-code + function _callSwapExecutor(uint256 amount, bytes calldata data) + internal + returns (uint256 calculatedAmount) + { + address executor; + bytes4 decodedSelector; + bytes memory protocolData; + + (executor, decodedSelector, protocolData) = + _decodeExecutorAndSelector(data); + + if (!swapExecutors[executor]) { + revert SwapExecutionDispatcher__UnapprovedExecutor(); + } + + bytes4 selector = decodedSelector == bytes4(0) + ? ISwapExecutor.swap.selector + : decodedSelector; + + // slither-disable-next-line low-level-calls + (bool success, bytes memory result) = executor.delegatecall( + abi.encodeWithSelector(selector, amount, protocolData) + ); + + if (!success) { + revert( + string( + result.length > 0 + ? result + : abi.encodePacked("Swap execution failed") + ) + ); + } + + 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 4383618..635c2ea 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -10,7 +10,6 @@ import "./CallbackVerificationDispatcher.sol"; error TychoRouter__WithdrawalFailed(); error TychoRouter__AddressZero(); -error TychoRouter__NonContractExecutor(); error TychoRouter__NonContractVerifier(); contract TychoRouter is @@ -45,7 +44,6 @@ contract TychoRouter is address indexed oldFeeReceiver, address indexed newFeeReceiver ); event FeeSet(uint256 indexed oldFee, uint256 indexed newFee); - event ExecutorSet(address indexed executor); event CallbackVerifierSet(address indexed callbackVerifier); constructor(address _permit2) { @@ -95,26 +93,24 @@ contract TychoRouter is /** * @dev Entrypoint to add or replace an approved swap executor contract address - * @param target address of the swap method contract + * @param target address of the swap executor contract */ function setSwapExecutor(address target) external onlyRole(EXECUTOR_SETTER_ROLE) { - if (target.code.length == 0) revert TychoRouter__NonContractExecutor(); - swapExecutors[target] = true; - emit ExecutorSet(target); + _setSwapExecutor(target); } /** * @dev Entrypoint to remove an approved swap executor contract address - * @param target address of the swap method contract + * @param target address of the swap executor contract */ function removeSwapExecutor(address target) external onlyRole(EXECUTOR_SETTER_ROLE) { - delete swapExecutors[target]; + _removeSwapExecutor(target); } /** diff --git a/foundry/test/Constants.sol b/foundry/test/Constants.sol index 521dff7..dfd2c1b 100644 --- a/foundry/test/Constants.sol +++ b/foundry/test/Constants.sol @@ -8,7 +8,18 @@ contract Constants is Test { address BOB = makeAddr("bob"); //bob=someone!=us address FUND_RESCUER = makeAddr("fundRescuer"); address FEE_SETTER = makeAddr("feeSetter"); + address FEE_RECEIVER = makeAddr("feeReceiver"); + // dummy contracts address DUMMY = makeAddr("dummy"); - address FEE_RECEIVER = makeAddr("feeReceiver"); + + address WETH_ADDR = address(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2); + + /** + * @dev Deploys a dummy contract with non-empty bytecode + */ + function deployDummyContract() internal { + bytes memory minimalBytecode = hex"01"; // Single-byte bytecode + vm.etch(DUMMY, minimalBytecode); // Deploy minimal bytecode + } } diff --git a/foundry/test/SwapExecutionDispatcher.t.sol b/foundry/test/SwapExecutionDispatcher.t.sol new file mode 100644 index 0000000..c313694 --- /dev/null +++ b/foundry/test/SwapExecutionDispatcher.t.sol @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.28; + +import "@src/SwapExecutionDispatcher.sol"; +import "./TychoRouterTestSetup.sol"; + +contract SwapExecutionDispatcherExposed is SwapExecutionDispatcher { + function exposedCallSwapExecutor(uint256 amount, bytes calldata data) + external + returns (uint256 calculatedAmount) + { + return _callSwapExecutor(amount, data); + } + + function exposedDecodeExecutorAndSelector(bytes calldata data) + external + pure + returns (address executor, bytes4 selector, bytes memory protocolData) + { + return _decodeExecutorAndSelector(data); + } + + function exposedSetSwapExecutor(address target) external { + _setSwapExecutor(target); + } + + function exposedRemoveSwapExecutor(address target) external { + _removeSwapExecutor(target); + } +} + +contract SwapExecutionDispatcherTest is Constants { + SwapExecutionDispatcherExposed dispatcherExposed; + + event ExecutorSet(address indexed executor); + event ExecutorRemoved(address indexed executor); + + function setUp() public { + uint256 forkBlock = 20673900; + vm.createSelectFork(vm.rpcUrl("mainnet"), forkBlock); + dispatcherExposed = new SwapExecutionDispatcherExposed(); + deal(WETH_ADDR, address(dispatcherExposed), 15 ether); + deployDummyContract(); + } + + function testSetValidExecutor() public { + 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); + } + + function testRemoveExecutor() public { + dispatcherExposed.exposedSetSwapExecutor(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); + } + + function testRemoveUnSetExecutor() public { + dispatcherExposed.exposedRemoveSwapExecutor(BOB); + assert(dispatcherExposed.swapExecutors(BOB) == false); + } + + function testSetExecutorNonContract() public { + vm.expectRevert( + abi.encodeWithSelector( + SwapExecutionDispatcher__NonContractExecutor.selector + ) + ); + dispatcherExposed.exposedSetSwapExecutor(BOB); + } + + function testCallSwapExecutor() public { + // Test case taken from existing transaction + // 0x755d603962b30f416cf3eefae8d55204d6ffdf746465b2a94aca216faab63804 + // For this test, we can use any executor and any calldata that we know works + // for this executor. We don't care about which calldata/executor, since we are + // only testing the functionality of the delegatecall and not the inner + // workings of the executor. + // 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( + address(0xe592557AB9F4A75D992283fD6066312FF013ba3d) + ); + bytes memory data = + hex"e592557AB9F4A75D992283fD6066312FF013ba3dbd0625ab5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593c81c"; + uint256 givenAmount = 15 ether; + uint256 amount = + dispatcherExposed.exposedCallSwapExecutor(givenAmount, data); + assert(amount == 35144641819); + } + + function testCallSwapExecutorNoSelector() public { + // Test case taken from existing transaction + // 0x755d603962b30f416cf3eefae8d55204d6ffdf746465b2a94aca216faab63804 + // No selector is passed, so the standard swap selector should be used + + // For this test, we can use any executor and any calldata that we know works + // for this executor. We don't care about which calldata/executor, since we are + // only testing the functionality of the delegatecall and not the inner + // workings of the executor. + // 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( + address(0xe592557AB9F4A75D992283fD6066312FF013ba3d) + ); + bytes memory data = + hex"e592557AB9F4A75D992283fD6066312FF013ba3d000000005615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593c81c"; + uint256 givenAmount = 15 ether; + uint256 amount = + dispatcherExposed.exposedCallSwapExecutor(givenAmount, data); + assert(amount == 35144641819); + } + + function testCallSwapExecutorCallFailed() public { + // Bad data is provided to an approved swap executor - causing the call to fail + dispatcherExposed.exposedSetSwapExecutor( + address(0xe592557AB9F4A75D992283fD6066312FF013ba3d) + ); + bytes memory data = + hex"e592557AB9F4A75D992283fD6066312FF013ba3dbd0625ab5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72fc8c39af7983bf329086de522229a7be5fc4e41cc51c72848c68a965f66fa7a88855f9f7784502a7f2606beffe61000613d6a25b5bfef4cd7652aa94777d4a46b39f2e206411280a12c9344b769ff1066c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000000000082ec8ad1b0000000000000000000000000000000000000000000000000000000066d7b65800000000000000000000000000000000000000000000000000000191ba9f843c125000064000640000d52de09955f0ffffffffffffff00225c389e595fe9000001fcc910754b349f821e4bb5d8444822a63920be943aba6f1b31ee14ef0fc6840b6d28d604e04a78834b668dba24a6c082ffb901e4fffa9600649e8d991af593"; + vm.expectRevert(); + dispatcherExposed.exposedCallSwapExecutor(0, data); + } + + function testCallSwapExecutorUnapprovedExecutor() public { + bytes memory data = + hex"5d622C9053b8FFB1B3465495C8a42E603632bA70aabbccdd1111111111111111"; + vm.expectRevert(); + dispatcherExposed.exposedCallSwapExecutor(0, data); + } + + function testDecodeExecutorAndSelector() public { + 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")); + } +} diff --git a/foundry/test/TychoRouter.t.sol b/foundry/test/TychoRouter.t.sol index fb9ad4b..809586f 100644 --- a/foundry/test/TychoRouter.t.sol +++ b/foundry/test/TychoRouter.t.sol @@ -14,39 +14,18 @@ contract TychoRouterTest is TychoRouterTestSetup { bytes32 public constant FUND_RESCUER_ROLE = 0x912e45d663a6f4cc1d0491d8f046e06c616f40352565ea1cdb86a0e1aaefa41b; - event ExecutorSet(address indexed executor); event CallbackVerifierSet(address indexed callbackVerifier); event Withdrawal( address indexed token, uint256 amount, address indexed receiver ); - function testSetValidExecutor() public { + function testSetExecutorValidRole() public { vm.startPrank(executorSetter); - vm.expectEmit(); - // Define the event we expect to be emitted at the next step - emit ExecutorSet(DUMMY); - tychoRouter.setSwapExecutor(DUMMY); vm.stopPrank(); - assert(tychoRouter.swapExecutors(DUMMY) == true); } - function testRemoveExecutor() public { - vm.startPrank(executorSetter); - tychoRouter.setSwapExecutor(DUMMY); - tychoRouter.removeSwapExecutor(DUMMY); - vm.stopPrank(); - assert(tychoRouter.swapExecutors(DUMMY) == false); - } - - function testRemoveUnSetExecutor() public { - vm.startPrank(executorSetter); - tychoRouter.removeSwapExecutor(BOB); - vm.stopPrank(); - assert(tychoRouter.swapExecutors(BOB) == false); - } - function testRemoveExecutorMissingSetterRole() public { vm.expectRevert(); tychoRouter.removeSwapExecutor(BOB); @@ -57,15 +36,6 @@ contract TychoRouterTest is TychoRouterTestSetup { tychoRouter.setSwapExecutor(DUMMY); } - function testSetExecutorNonContract() public { - vm.startPrank(executorSetter); - vm.expectRevert( - abi.encodeWithSelector(TychoRouter__NonContractExecutor.selector) - ); - tychoRouter.setSwapExecutor(BOB); - vm.stopPrank(); - } - function testSetValidVerifier() public { vm.startPrank(executorSetter); vm.expectEmit(); diff --git a/foundry/test/TychoRouterTestSetup.sol b/foundry/test/TychoRouterTestSetup.sol index 9765d76..a97e3a9 100644 --- a/foundry/test/TychoRouterTestSetup.sol +++ b/foundry/test/TychoRouterTestSetup.sol @@ -28,14 +28,6 @@ contract TychoRouterTestSetup is Test, Constants { vm.stopPrank(); } - /** - * @dev Deploys a dummy contract with non-empty bytecode - */ - function deployDummyContract() internal { - bytes memory minimalBytecode = hex"01"; // Single-byte bytecode - vm.etch(DUMMY, minimalBytecode); // Deploy minimal bytecode - } - /** * @dev Mints tokens to the given address * @param amount The amount of tokens to mint