diff --git a/foundry/interfaces/ISwapExecutor.sol b/foundry/interfaces/ISwapExecutor.sol index d8bf82c..fef5750 100644 --- a/foundry/interfaces/ISwapExecutor.sol +++ b/foundry/interfaces/ISwapExecutor.sol @@ -8,23 +8,17 @@ 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. + * @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 ISwapExecutor { interface ISwapExecutorErrors { error InvalidParameterLength(uint256); - error UnknownCurveType(uint8); + error UnknownPoolType(uint8); } diff --git a/foundry/src/SwapExecutionDispatcher.sol b/foundry/src/SwapExecutionDispatcher.sol index c8edd7a..30f335a 100644 --- a/foundry/src/SwapExecutionDispatcher.sol +++ b/foundry/src/SwapExecutionDispatcher.sol @@ -24,7 +24,7 @@ contract SwapExecutionDispatcher { event ExecutorSet(address indexed executor); /** - * @dev Adds or replace an approved swap executor contract address if it is a + * @dev Adds or replaces an approved swap executor contract address if it is a * contract. * @param target address of the swap executor contract */ @@ -37,7 +37,7 @@ contract SwapExecutionDispatcher { } /** - * @dev Remove an approved swap executor contract address + * @dev Removes an approved swap executor contract address * @param target address of the swap executor contract */ function _removeSwapExecutor(address target) internal { @@ -46,7 +46,7 @@ contract SwapExecutionDispatcher { /** * @dev Calls an executor, assumes swap.protocolData contains - * token addresses if required by the executor. + * protocol-specific data required by the executor. */ // slither-disable-next-line dead-code function _callSwapExecutor(uint256 amount, bytes calldata data) @@ -60,14 +60,14 @@ contract SwapExecutionDispatcher { (executor, decodedSelector, protocolData) = _decodeExecutorAndSelector(data); - bytes4 selector = decodedSelector == bytes4(0) - ? ISwapExecutor.swap.selector - : decodedSelector; - 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) diff --git a/foundry/test/SwapExecutionDispatcher.t.sol b/foundry/test/SwapExecutionDispatcher.t.sol index 6608c04..635b3a1 100644 --- a/foundry/test/SwapExecutionDispatcher.t.sol +++ b/foundry/test/SwapExecutionDispatcher.t.sol @@ -38,7 +38,7 @@ contract SwapExecutionDispatcherTest is Constants { uint256 forkBlock = 20673900; vm.createSelectFork(vm.rpcUrl("mainnet"), forkBlock); dispatcherExposed = new SwapExecutionDispatcherExposed(); - deal(WETH_ADDR, address(dispatcherExposed), 15000000000000000000); + deal(WETH_ADDR, address(dispatcherExposed), 15 ether); deployDummyContract(); } @@ -73,12 +73,19 @@ contract SwapExecutionDispatcherTest is Constants { 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 = 15000000000000000000; + uint256 givenAmount = 15 ether; uint256 amount = dispatcherExposed.exposedCallSwapExecutor(givenAmount, data); assert(amount == 35144641819); @@ -88,12 +95,20 @@ contract SwapExecutionDispatcherTest is Constants { // 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 = 15000000000000000000; + uint256 givenAmount = 15 ether; uint256 amount = dispatcherExposed.exposedCallSwapExecutor(givenAmount, data); assert(amount == 35144641819);