diff --git a/.github/workflows/tests-and-lints-template.yaml b/.github/workflows/tests-and-lints-template.yaml index 2c9ffcc..6f85b08 100644 --- a/.github/workflows/tests-and-lints-template.yaml +++ b/.github/workflows/tests-and-lints-template.yaml @@ -59,9 +59,12 @@ jobs: git config --global url."https://x-access-token:${{ steps.generate-token.outputs.token }}@github.com".insteadOf ssh://github.com - name: Setup toolchain - uses: dtolnay/rust-toolchain@888c2e1ea69ab0d4330cbf0af1ecc7b68f368cc1 + id: toolchain + uses: actions-rs/toolchain@v1 with: - toolchain: ${{ matrix.toolchain }} + toolchain: nightly + components: rustfmt, clippy + override: true - name: Setup Rust Cache uses: Swatinem/rust-cache@9d47c6ad4b02e050fd481d890b2ea34778fd09d6 @@ -100,28 +103,20 @@ jobs: echo "https://${{ steps.generate-token.outputs.token }}@github.com" > ~/.git-credentials git config --global url."https://x-access-token:${{ steps.generate-token.outputs.token }}@github.com".insteadOf ssh://github.com - - name: Setup clippy toolchain - stable - uses: dtolnay/rust-toolchain@888c2e1ea69ab0d4330cbf0af1ecc7b68f368cc1 + - name: Setup toolchain + id: toolchain + uses: actions-rs/toolchain@v1 with: - toolchain: stable - components: clippy - + toolchain: nightly + components: rustfmt, clippy + override: true - name: Setup Rust Cache - uses: Swatinem/rust-cache@9d47c6ad4b02e050fd481d890b2ea34778fd09d6 + uses: Swatinem/rust-cache@v2 with: cache-on-failure: true - - run: cargo clippy --workspace --lib --all-targets --all-features -- -D clippy::dbg-macro - env: - RUSTFLAGS: -Dwarnings + - name: Clippy + run: cargo clippy --workspace --all-targets --all-features - - run: cargo check --no-default-features - env: - RUSTFLAGS: -Dwarnings - - - name: Setup rustfmt toolchain - nightly - uses: dtolnay/rust-toolchain@a02741459ec5e501b9843ed30b535ca0a0376ae4 - with: - components: rustfmt - - - run: cargo +nightly fmt --all --check + - name: Rustfmt + run: cargo fmt --all --check diff --git a/CHANGELOG.md b/CHANGELOG.md index 4509807..38cf75e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,23 @@ +## [0.90.0](https://github.com/propeller-heads/tycho-execution/compare/0.89.0...0.90.0) (2025-05-15) + + +### Features + +* Explicitly handle the TransferType.NONE case ([65bd0d0](https://github.com/propeller-heads/tycho-execution/commit/65bd0d07499b79c261efdb00debb19487d6af543)) +* Verify the amount out was received correctly for arbitrage swaps ([70230bf](https://github.com/propeller-heads/tycho-execution/commit/70230bf05f4bdfdd54f62799b72bd998d351983e)) + + +### Bug Fixes + +* Revert if the TransferType is not valid ([b0c254a](https://github.com/propeller-heads/tycho-execution/commit/b0c254add44e56d10f203eae30301c861fd9d8ff)) + +## [0.89.0](https://github.com/propeller-heads/tycho-execution/compare/0.88.0...0.89.0) (2025-05-14) + + +### Features + +* Remove special handling of the Univ4 callback ([f14c8ee](https://github.com/propeller-heads/tycho-execution/commit/f14c8ee29ba67ca4cd8cfe8b00e8b907a0352f9a)) + ## [0.88.0](https://github.com/propeller-heads/tycho-execution/compare/0.87.0...0.88.0) (2025-05-06) diff --git a/Cargo.lock b/Cargo.lock index dac751c..34133d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4469,7 +4469,7 @@ dependencies = [ [[package]] name = "tycho-execution" -version = "0.88.0" +version = "0.90.0" dependencies = [ "alloy", "alloy-primitives", diff --git a/Cargo.toml b/Cargo.toml index 30ec15a..9cc28e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "tycho-execution" -version = "0.88.0" +version = "0.90.0" edition = "2021" description = "Provides tools for encoding and executing swaps against Tycho router and protocol executors." repository = "https://github.com/propeller-heads/tycho-execution" diff --git a/foundry/src/Dispatcher.sol b/foundry/src/Dispatcher.sol index 1bb6209..dcbd1b4 100644 --- a/foundry/src/Dispatcher.sol +++ b/foundry/src/Dispatcher.sol @@ -127,7 +127,9 @@ contract Dispatcher { tstore(_CURRENTLY_SWAPPING_EXECUTOR_SLOT, 0) } - // this is necessary because the delegatecall will prepend extra bytes we don't want like the length and prefix + // The final callback result should not be ABI encoded. That is why we are decoding here. + // ABI encoding is very gas expensive and we want to avoid it if possible. + // The result from `handleCallback` is always ABI encoded. bytes memory decodedResult = abi.decode(result, (bytes)); return decodedResult; } diff --git a/foundry/src/TychoRouter.sol b/foundry/src/TychoRouter.sol index 4d9d7b7..5c41e9f 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -436,15 +436,14 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { Address.sendValue(payable(receiver), amountOut); } - if (tokenIn != tokenOut) { - uint256 currentBalanceTokenOut = _balanceOf(tokenOut, receiver); - uint256 userAmount = currentBalanceTokenOut - initialBalanceTokenOut; - if (userAmount != amountOut) { - revert TychoRouter__AmountOutNotFullyReceived( - userAmount, amountOut - ); - } - } + _verifyAmountOutWasReceived( + tokenIn, + tokenOut, + initialBalanceTokenOut, + amountOut, + receiver, + amountIn + ); } /** @@ -493,15 +492,14 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { Address.sendValue(payable(receiver), amountOut); } - if (tokenIn != tokenOut) { - uint256 currentBalanceTokenOut = _balanceOf(tokenOut, receiver); - uint256 userAmount = currentBalanceTokenOut - initialBalanceTokenOut; - if (userAmount != amountOut) { - revert TychoRouter__AmountOutNotFullyReceived( - userAmount, amountOut - ); - } - } + _verifyAmountOutWasReceived( + tokenIn, + tokenOut, + initialBalanceTokenOut, + amountOut, + receiver, + amountIn + ); } /** @@ -546,16 +544,14 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { _unwrapETH(amountOut); Address.sendValue(payable(receiver), amountOut); } - - if (tokenIn != tokenOut) { - uint256 currentBalanceTokenOut = _balanceOf(tokenOut, receiver); - uint256 userAmount = currentBalanceTokenOut - initialBalanceTokenOut; - if (userAmount != amountOut) { - revert TychoRouter__AmountOutNotFullyReceived( - userAmount, amountOut - ); - } - } + _verifyAmountOutWasReceived( + tokenIn, + tokenOut, + initialBalanceTokenOut, + amountOut, + receiver, + amountIn + ); } /** @@ -657,13 +653,8 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { /** * @dev We use the fallback function to allow flexibility on callback. */ - fallback() external { - bytes memory result = _callHandleCallbackOnExecutor(msg.data); - // slither-disable-next-line assembly - assembly ("memory-safe") { - // Propagate the result - return(add(result, 32), mload(result)) - } + fallback(bytes calldata data) external returns (bytes memory) { + return _callHandleCallbackOnExecutor(data); } /** @@ -778,18 +769,6 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { require(msg.sender.code.length != 0); } - /** - * @dev Called by UniswapV4 pool manager after achieving unlock state. - */ - function unlockCallback(bytes calldata data) - external - returns (bytes memory) - { - if (data.length < 24) revert TychoRouter__InvalidDataLength(); - bytes memory result = _callHandleCallbackOnExecutor(data); - return result; - } - /** * @dev Gets balance of a token for a given address. Supports both native ETH and ERC20 tokens. */ @@ -801,4 +780,27 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard { return token == address(0) ? owner.balance : IERC20(token).balanceOf(owner); } + + /** + * @dev Verifies that the expected amount of output tokens was received by the receiver. + * It also handles the case of arbitrage swaps where the input and output tokens are the same. + */ + function _verifyAmountOutWasReceived( + address tokenIn, + address tokenOut, + uint256 initialBalanceTokenOut, + uint256 amountOut, + address receiver, + uint256 amountIn + ) internal view { + uint256 currentBalanceTokenOut = _balanceOf(tokenOut, receiver); + if (tokenIn == tokenOut) { + // If it is an arbitrage, we need to remove the amountIn from the initial balance to get a correct userAmount + initialBalanceTokenOut -= amountIn; + } + uint256 userAmount = currentBalanceTokenOut - initialBalanceTokenOut; + if (userAmount != amountOut) { + revert TychoRouter__AmountOutNotFullyReceived(userAmount, amountOut); + } + } } diff --git a/foundry/src/executors/CurveExecutor.sol b/foundry/src/executors/CurveExecutor.sol index ef2af90..e10a213 100644 --- a/foundry/src/executors/CurveExecutor.sol +++ b/foundry/src/executors/CurveExecutor.sol @@ -149,6 +149,11 @@ contract CurveExecutor is IExecutor, TokenTransfer { receiver = address(bytes20(data[65:85])); } + /** + * @dev Even though this contract is mostly called through delegatecall, we still want to be able to receive ETH. + * This is needed when using the executor directly and it makes testing easier. + * There are some curve pools that take ETH directly. + */ receive() external payable { require(msg.sender.code.length != 0); } diff --git a/foundry/src/executors/MaverickV2Executor.sol b/foundry/src/executors/MaverickV2Executor.sol index 0775177..caf8ff0 100644 --- a/foundry/src/executors/MaverickV2Executor.sol +++ b/foundry/src/executors/MaverickV2Executor.sol @@ -13,14 +13,12 @@ contract MaverickV2Executor is IExecutor, TokenTransfer { using SafeERC20 for IERC20; address public immutable factory; - address private immutable self; constructor(address _factory, address _permit2) TokenTransfer(_permit2) { if (_factory == address(0)) { revert MaverickV2Executor__InvalidFactory(); } factory = _factory; - self = address(this); } // slither-disable-next-line locked-ether diff --git a/foundry/src/executors/TokenTransfer.sol b/foundry/src/executors/TokenTransfer.sol index b5f8629..6eac1c0 100644 --- a/foundry/src/executors/TokenTransfer.sol +++ b/foundry/src/executors/TokenTransfer.sol @@ -6,6 +6,7 @@ import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import "@permit2/src/interfaces/IAllowanceTransfer.sol"; error TokenTransfer__AddressZero(); +error TokenTransfer__InvalidTransferType(); contract TokenTransfer { using SafeERC20 for IERC20; @@ -45,7 +46,9 @@ contract TokenTransfer { uint256 amount, TransferType transferType ) internal { - if (transferType == TransferType.TRANSFER_TO_PROTOCOL) { + if (transferType == TransferType.NONE) { + return; + } else if (transferType == TransferType.TRANSFER_TO_PROTOCOL) { if (tokenIn == address(0)) { payable(receiver).transfer(amount); } else { @@ -65,6 +68,8 @@ contract TokenTransfer { permit2.transferFrom( sender, address(this), uint160(amount), tokenIn ); + } else { + revert TokenTransfer__InvalidTransferType(); } } } diff --git a/foundry/src/executors/UniswapV4Executor.sol b/foundry/src/executors/UniswapV4Executor.sol index f594adc..9b878ad 100644 --- a/foundry/src/executors/UniswapV4Executor.sol +++ b/foundry/src/executors/UniswapV4Executor.sol @@ -184,8 +184,11 @@ contract UniswapV4Executor is external returns (bytes memory) { - verifyCallback(data); - return _unlockCallback(data); + bytes calldata stripped = data[68:]; + verifyCallback(stripped); + // Our general callback logic returns a not ABI encoded result. + // However, the pool manager expects the result to be ABI encoded. That is why we need to encode it here again. + return abi.encode(_unlockCallback(stripped)); } function verifyCallback(bytes calldata) public view poolManagerOnly {}