diff --git a/packages/contracts-rfq/contracts/zaps/TokenZapV1.sol b/packages/contracts-rfq/contracts/zaps/TokenZapV1.sol index a754c55f57..bd58d8f391 100644 --- a/packages/contracts-rfq/contracts/zaps/TokenZapV1.sol +++ b/packages/contracts-rfq/contracts/zaps/TokenZapV1.sol @@ -15,37 +15,51 @@ import {IERC20, SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeE import {Address} from "@openzeppelin/contracts/utils/Address.sol"; /// @title TokenZapV1 -/// @notice Facilitates atomic token operations known as "Zaps," allowing to execute predefined actions -/// on behalf of users like deposits or swaps. Supports ERC20 tokens and native gas tokens (e.g., ETH). -/// @dev Tokens must be pre-transferred to the contract for execution, with native tokens sent as msg.value. +/// @notice Facilitates atomic token operations known as "Zaps", allowing the execution of predefined actions +/// on behalf of users, such as deposits or swaps. Supports ERC20 tokens and native gas tokens (e.g., ETH). +/// @dev Tokens must be transferred to the contract before execution, native tokens could be provided as `msg.value`. /// This contract is stateless and does not hold assets between Zaps; leftover tokens can be claimed by anyone. -/// Ensure Zaps fully utilize tokens or revert to prevent fund loss. +/// Ensure that Zaps fully utilize tokens or revert to prevent the loss of funds. contract TokenZapV1 is IZapRecipient { using SafeERC20 for IERC20; using ZapDataV1 for bytes; address public constant NATIVE_GAS_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; - error TokenZapV1__AmountIncorrect(); error TokenZapV1__PayloadLengthAboveMax(); + error TokenZapV1__TargetZeroAddress(); - /// @notice Performs a Zap action using the specified token and amount. This amount must be previously - /// transferred to this contract (or supplied as msg.value if the token is native gas token). + /// @notice Allows the contract to receive ETH. + /// @dev Leftover ETH can be claimed by anyone. Ensure the full balance is spent during Zaps. + receive() external payable {} + + /// @notice Performs a Zap action using the specified token and amount. This amount must have previously been + /// transferred to this contract (could also be supplied as msg.value if the token is a native gas token). + /// Zap action will be performed forwarding full `msg.value` for ERC20s or `amount` for native gas tokens. + /// Note: all funds remaining after the Zap action is performed can be claimed by anyone. + /// Make sure to spend the full balance during the Zaps and avoid sending extra funds if a single Zap is performed. /// @dev The provided ZapData contains the target address and calldata for the Zap action, and must be - /// encoded using the encodeZapData function. + /// encoded using the encodeZapData function. Native gas token transfers could be done by using empty `payload`, + /// this is the only case where target could be an EOA. /// @param token Address of the token to be used for the Zap action. /// @param amount Amount of the token to be used for the Zap action. - /// Must match msg.value if the token is a native gas token. /// @param zapData Encoded Zap Data containing the target address and calldata for the Zap action. /// @return selector Selector of this function to signal the caller about the success of the Zap action. function zap(address token, uint256 amount, bytes calldata zapData) external payable returns (bytes4) { // Validate the ZapData format and extract the target address. zapData.validateV1(); address target = zapData.target(); + if (target == address(0)) revert TokenZapV1__TargetZeroAddress(); + // Note: we don't check the amount that was transferred to TokenZapV1 (or msg.value for native gas tokens). + // Transferring more than `amount` will lead to remaining funds in TokenZapV1, which can be claimed by anyone. + // Ensure that you send the exact amount for a single Zap or spend the full balance for multiple `zap()` calls. + uint256 msgValue = msg.value; if (token == NATIVE_GAS_TOKEN) { - // For native gas token (e.g., ETH), verify msg.value matches the expected amount. - // No approval needed since native token doesn't use allowances. - if (msg.value != amount) revert TokenZapV1__AmountIncorrect(); + // For native gas tokens, we forward the requested amount to the target contract during the Zap action. + // Similar to ERC20s, we allow using pre-transferred native tokens for the Zap. + msgValue = amount; + // No approval is needed since native tokens don't use allowances. + // Note: balance check is performed within `Address.sendValue` or `Address.functionCallWithValue` below. } else { // For ERC20 tokens, grant unlimited approval to the target if the current allowance is insufficient. // This is safe since the contract doesn't custody tokens between zaps. @@ -57,9 +71,16 @@ contract TokenZapV1 is IZapRecipient { // Construct the payload for the target contract call with the Zap action. // The payload is modified to replace the placeholder amount with the actual amount. bytes memory payload = zapData.payload(amount); - // Perform the Zap action, forwarding full msg.value to the target contract. - // Note: this will bubble up any revert from the target contract. - Address.functionCallWithValue({target: target, data: payload, value: msg.value}); + if (payload.length == 0 && token == NATIVE_GAS_TOKEN) { + // Zap Action in a form of native gas token transfer to the target is requested. + // Note: we avoid using `functionCallWithValue` because the target might be an EOA. This will + // revert with a generic custom error should the target contract revert on incoming transfer. + Address.sendValue({recipient: payable(target), amount: msgValue}); + } else { + // Perform the Zap action, forwarding the requested native value to the target contract. + // Note: this will bubble up any revert from the target contract, and revert if target is EOA. + Address.functionCallWithValue({target: target, data: payload, value: msgValue}); + } // Return function selector to indicate successful execution return this.zap.selector; } diff --git a/packages/contracts-rfq/test/mocks/WETHMock.sol b/packages/contracts-rfq/test/mocks/WETHMock.sol new file mode 100644 index 0000000000..75b5c4d5dd --- /dev/null +++ b/packages/contracts-rfq/test/mocks/WETHMock.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; + +// solhint-disable no-empty-blocks +/// @notice WETH mock for testing purposes. DO NOT USE IN PRODUCTION. +contract WETHMock is ERC20 { + constructor() ERC20("Mock Wrapped Ether", "Mock WETH") {} + + receive() external payable { + deposit(); + } + + /// @notice We include an empty "test" function so that this contract does not appear in the coverage report. + function testWETHMock() external {} + + function withdraw(uint256 amount) external { + _burn(msg.sender, amount); + Address.sendValue(payable(msg.sender), amount); + } + + function deposit() public payable { + _mint(msg.sender, msg.value); + } +} diff --git a/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol b/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol index 37a07838e2..68afd42331 100644 --- a/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol +++ b/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol @@ -5,8 +5,12 @@ import {ZapDataV1} from "../../contracts/libs/ZapDataV1.sol"; import {TokenZapV1} from "../../contracts/zaps/TokenZapV1.sol"; import {MockERC20} from "../MockERC20.sol"; +import {NonPayableRecipient} from "../mocks/NonPayableRecipient.sol"; +import {RecipientMock} from "../mocks/RecipientMock.sol"; import {VaultManyArguments} from "../mocks/VaultManyArguments.sol"; +import {WETHMock} from "../mocks/WETHMock.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {Test} from "forge-std/Test.sol"; // solhint-disable func-name-mixedcase, ordering @@ -16,6 +20,9 @@ contract TokenZapV1Test is Test { TokenZapV1 internal tokenZap; VaultManyArguments internal vault; MockERC20 internal erc20; + WETHMock internal weth; + address internal payableMock; + address internal nonPayableMock; address internal user; address internal nativeGasToken = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; @@ -24,11 +31,15 @@ contract TokenZapV1Test is Test { tokenZap = new TokenZapV1(); vault = new VaultManyArguments(); erc20 = new MockERC20("TKN", 18); + weth = new WETHMock(); + payableMock = address(new RecipientMock()); + nonPayableMock = address(new NonPayableRecipient()); user = makeAddr("user"); erc20.mint(address(this), 100 * AMOUNT); - deal(address(this), 100 * AMOUNT); + deal(address(this), 200 * AMOUNT); + weth.deposit{value: 100 * AMOUNT}(); } function getVaultPayload(address token, uint256 amount) public view returns (bytes memory) { @@ -144,6 +155,200 @@ contract TokenZapV1Test is Test { test_zap_native_noAmount(); } + /// @notice Should be able to use amount lower than msg.value. + function test_zap_native_msgValueHigherThanAmount() public { + bytes memory zapData = getZapData(getVaultPayload(nativeGasToken, 1 ether)); + bytes4 returnValue = tokenZap.zap{value: AMOUNT + 1 wei}(nativeGasToken, AMOUNT, zapData); + assertEq(returnValue, tokenZap.zap.selector); + // Check that the vault registered the deposit + assertEq(vault.balanceOf(user, nativeGasToken), AMOUNT); + // Note: the extra funds are left in the contract + assertEq(address(tokenZap).balance, 1 wei); + } + + /// @notice Should be able to utilize both msg.value and existing native balance. + function test_zap_native_msgValueLowerThanAmount_extraNative() public { + deal(address(tokenZap), 1337); + bytes memory zapData = getZapData(getVaultPayload(nativeGasToken, 1 ether)); + bytes4 returnValue = tokenZap.zap{value: AMOUNT - 1337}(nativeGasToken, AMOUNT, zapData); + assertEq(returnValue, tokenZap.zap.selector); + // Check that the vault registered the deposit + assertEq(vault.balanceOf(user, nativeGasToken), AMOUNT); + } + + // ═════════════════════════════════════════════════ MULTIHOPS ═════════════════════════════════════════════════════ + + function getZapDataWithdraw(uint256 amount) public view returns (bytes memory) { + return tokenZap.encodeZapData(address(weth), abi.encodeCall(WETHMock.withdraw, (amount)), 4); + } + + function test_zap_withdraw_depositNative_placeholderZero() public { + bytes memory zapDataWithdraw = getZapDataWithdraw(0); + bytes memory zapDataDeposit = getZapDataNoAmount(getVaultPayloadNoAmount()); + weth.transfer(address(tokenZap), AMOUNT); + // Do two Zaps in a row + bytes4 returnValue = tokenZap.zap(address(weth), AMOUNT, zapDataWithdraw); + assertEq(returnValue, tokenZap.zap.selector); + returnValue = tokenZap.zap(nativeGasToken, AMOUNT, zapDataDeposit); + assertEq(returnValue, tokenZap.zap.selector); + // Check that the vault registered the deposit + assertEq(vault.balanceOf(user, nativeGasToken), AMOUNT); + } + + function test_zap_withdraw_depositNative_placeholderNonZero() public { + // Use the approximate amount of tokens as placeholder + bytes memory zapDataWithdraw = getZapDataWithdraw(1 ether); + bytes memory zapDataDeposit = getZapDataNoAmount(getVaultPayloadNoAmount()); + weth.transfer(address(tokenZap), AMOUNT); + // Do two Zaps in a row + bytes4 returnValue = tokenZap.zap(address(weth), AMOUNT, zapDataWithdraw); + assertEq(returnValue, tokenZap.zap.selector); + returnValue = tokenZap.zap(nativeGasToken, AMOUNT, zapDataDeposit); + assertEq(returnValue, tokenZap.zap.selector); + // Check that the vault registered the deposit + assertEq(vault.balanceOf(user, nativeGasToken), AMOUNT); + } + + function test_zap_withdraw_depositNative_placeholderZero_extraTokens() public { + // Transfer some extra tokens to the zap contract + weth.transfer(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_depositNative_placeholderZero(); + } + + function test_zap_withdraw_depositNative_placeholderZero_extraNative() public { + // Transfer some extra native tokens to the zap contract + deal(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_depositNative_placeholderZero(); + } + + function test_zap_withdraw_depositNative_placeholderNonZero_extraTokens() public { + // Transfer some extra tokens to the zap contract + weth.transfer(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_depositNative_placeholderNonZero(); + } + + function test_zap_withdraw_depositNative_placeholderNonZero_extraNative() public { + // Transfer some extra native tokens to the zap contract + deal(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_depositNative_placeholderNonZero(); + } + + function test_zap_withdraw_transferNativeEOA_placeholderZero() public { + bytes memory zapDataWithdraw = getZapDataWithdraw(0); + bytes memory zapDataTransfer = tokenZap.encodeZapData({target: user, payload: "", amountPosition: 0}); + weth.transfer(address(tokenZap), AMOUNT); + // Do two Zaps in a row + bytes4 returnValue = tokenZap.zap(address(weth), AMOUNT, zapDataWithdraw); + assertEq(returnValue, tokenZap.zap.selector); + returnValue = tokenZap.zap(nativeGasToken, AMOUNT, zapDataTransfer); + assertEq(returnValue, tokenZap.zap.selector); + // Check that the user received the native tokens + assertEq(user.balance, AMOUNT); + } + + function test_zap_withdraw_transferNativeEOA_placeholderNonZero() public { + // Use the approximate amount of tokens as placeholder + bytes memory zapDataWithdraw = getZapDataWithdraw(1 ether); + bytes memory zapDataTransfer = tokenZap.encodeZapData({target: user, payload: "", amountPosition: 0}); + weth.transfer(address(tokenZap), AMOUNT); + // Do two Zaps in a row + bytes4 returnValue = tokenZap.zap(address(weth), AMOUNT, zapDataWithdraw); + assertEq(returnValue, tokenZap.zap.selector); + returnValue = tokenZap.zap(nativeGasToken, AMOUNT, zapDataTransfer); + assertEq(returnValue, tokenZap.zap.selector); + // Check that the user received the native tokens + assertEq(user.balance, AMOUNT); + } + + function test_zap_withdraw_transferNativeEOA_placeholderZero_extraTokens() public { + // Transfer some extra tokens to the zap contract + weth.transfer(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_transferNativeEOA_placeholderZero(); + } + + function test_zap_withdraw_transferNativeEOA_placeholderZero_extraNative() public { + // Transfer some extra native tokens to the zap contract + deal(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_transferNativeEOA_placeholderZero(); + } + + function test_zap_withdraw_transferNativeEOA_placeholderNonZero_extraTokens() public { + // Transfer some extra tokens to the zap contract + weth.transfer(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_transferNativeEOA_placeholderNonZero(); + } + + function test_zap_withdraw_transferNativeEOA_placeholderNonZero_extraNative() public { + // Transfer some extra native tokens to the zap contract + deal(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_transferNativeEOA_placeholderNonZero(); + } + + function test_zap_withdraw_transferNativeContract_placeholderZero() public { + bytes memory zapDataWithdraw = getZapDataWithdraw(0); + bytes memory zapDataTransfer = tokenZap.encodeZapData({target: payableMock, payload: "", amountPosition: 0}); + weth.transfer(address(tokenZap), AMOUNT); + // Do two Zaps in a row + bytes4 returnValue = tokenZap.zap(address(weth), AMOUNT, zapDataWithdraw); + assertEq(returnValue, tokenZap.zap.selector); + returnValue = tokenZap.zap(nativeGasToken, AMOUNT, zapDataTransfer); + assertEq(returnValue, tokenZap.zap.selector); + // Check that the contract received the native tokens + assertEq(payableMock.balance, AMOUNT); + } + + function test_zap_withdraw_transferNativeContract_placeholderNonZero() public { + // Use the approximate amount of tokens as placeholder + bytes memory zapDataWithdraw = getZapDataWithdraw(1 ether); + bytes memory zapDataTransfer = tokenZap.encodeZapData({target: payableMock, payload: "", amountPosition: 0}); + weth.transfer(address(tokenZap), AMOUNT); + // Do two Zaps in a row + bytes4 returnValue = tokenZap.zap(address(weth), AMOUNT, zapDataWithdraw); + assertEq(returnValue, tokenZap.zap.selector); + returnValue = tokenZap.zap(nativeGasToken, AMOUNT, zapDataTransfer); + assertEq(returnValue, tokenZap.zap.selector); + // Check that the contract received the native tokens + assertEq(payableMock.balance, AMOUNT); + } + + function test_zap_withdraw_transferNativeContract_placeholderZero_extraTokens() public { + // Transfer some extra tokens to the zap contract + weth.transfer(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_transferNativeContract_placeholderZero(); + } + + function test_zap_withdraw_transferNativeContract_placeholderZero_extraNative() public { + // Transfer some extra native tokens to the zap contract + deal(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_transferNativeContract_placeholderZero(); + } + + function test_zap_withdraw_transferNativeContract_placeholderNonZero_extraTokens() public { + // Transfer some extra tokens to the zap contract + weth.transfer(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_transferNativeContract_placeholderNonZero(); + } + + function test_zap_withdraw_transferNativeContract_placeholderNonZero_extraNative() public { + // Transfer some extra native tokens to the zap contract + deal(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_withdraw_transferNativeContract_placeholderNonZero(); + } + + // ═════════════════════════════════════════════════ ENCODING ══════════════════════════════════════════════════════ + function test_encodeZapData_roundtrip(address token, uint256 placeholderAmount, uint256 amount) public view { bytes memory originalPayload = getVaultPayload(token, placeholderAmount); bytes memory expectedPayload = getVaultPayload(token, amount); @@ -168,6 +373,11 @@ contract TokenZapV1Test is Test { // ══════════════════════════════════════════════════ REVERTS ══════════════════════════════════════════════════════ + function getZeroTargetZapData(bytes memory payload, uint16 amountPosition) public pure returns (bytes memory) { + // Encode manually as the library checks for zero address + return abi.encodePacked(ZapDataV1.VERSION, amountPosition, address(0), payload); + } + function test_zap_erc20_revert_notEnoughTokens() public { bytes memory zapData = getZapData(getVaultPayload(address(erc20), 0)); // Transfer tokens to the zap contract first, but not enough @@ -184,6 +394,43 @@ contract TokenZapV1Test is Test { tokenZap.zap(address(erc20), AMOUNT, zapData); } + function test_zap_erc20_revert_targetZeroAddress_emptyPayload() public { + bytes memory zapData = getZeroTargetZapData({payload: "", amountPosition: 0}); + // Transfer tokens to the zap contract first + erc20.transfer(address(tokenZap), AMOUNT); + vm.expectRevert(TokenZapV1.TokenZapV1__TargetZeroAddress.selector); + tokenZap.zap(address(erc20), AMOUNT, zapData); + } + + function test_zap_erc20_revert_targetZeroAddress_nonEmptyPayload() public { + bytes memory zapData = + getZeroTargetZapData({payload: getVaultPayload(address(erc20), 0), amountPosition: 4 + 32 * 2}); + // Transfer tokens to the zap contract first + erc20.transfer(address(tokenZap), AMOUNT); + vm.expectRevert(TokenZapV1.TokenZapV1__TargetZeroAddress.selector); + tokenZap.zap(address(erc20), AMOUNT, zapData); + } + + function test_zap_erc20_revert_targetEOA_nonEmptyPayload() public { + bytes memory zapData = tokenZap.encodeZapData({ + target: user, + payload: getVaultPayload(address(erc20), 0), + amountPosition: 4 + 32 * 2 + }); + // Transfer tokens to the zap contract first + erc20.transfer(address(tokenZap), AMOUNT); + vm.expectRevert(abi.encodeWithSelector(Address.AddressEmptyCode.selector, user)); + tokenZap.zap(address(erc20), AMOUNT, zapData); + } + + function test_zap_erc20_revert_targetEOA_emptyPayload() public { + bytes memory zapData = tokenZap.encodeZapData({target: user, payload: "", amountPosition: 0}); + // Transfer tokens to the zap contract first + erc20.transfer(address(tokenZap), AMOUNT); + vm.expectRevert(abi.encodeWithSelector(Address.AddressEmptyCode.selector, user)); + tokenZap.zap(address(erc20), AMOUNT, zapData); + } + function test_zap_native_revert_targetReverted() public { bytes memory zapData = getZapData(getVaultPayloadWithRevert()); vm.expectRevert(VaultManyArguments.VaultManyArguments__SomeError.selector); @@ -194,16 +441,46 @@ contract TokenZapV1Test is Test { bytes memory originalPayload = getVaultPayload(nativeGasToken, 0); bytes memory zapData = getZapData(originalPayload); - vm.expectRevert(TokenZapV1.TokenZapV1__AmountIncorrect.selector); + vm.expectRevert(abi.encodeWithSelector(Address.AddressInsufficientBalance.selector, tokenZap)); tokenZap.zap{value: 1 ether - 1 wei}(nativeGasToken, 1 ether, zapData); } - function test_zap_native_revert_msgValueHigherThanExpected() public { - bytes memory originalPayload = getVaultPayload(nativeGasToken, 0); - bytes memory zapData = getZapData(originalPayload); + function test_zap_withdraw_transferNative_revert_targetReverted() public { + bytes memory zapDataWithdraw = getZapDataWithdraw(0); + bytes memory zapDataTransfer = tokenZap.encodeZapData({target: nonPayableMock, payload: "", amountPosition: 0}); + weth.transfer(address(tokenZap), AMOUNT); + tokenZap.zap(address(weth), AMOUNT, zapDataWithdraw); + vm.expectRevert(Address.FailedInnerCall.selector); + tokenZap.zap(address(weth), AMOUNT, zapDataTransfer); + } + + function test_zap_withdraw_transferNative_revert_targetZeroAddress_emptyPayload() public { + bytes memory zapDataWithdraw = getZapDataWithdraw(0); + bytes memory zapDataTransfer = getZeroTargetZapData({payload: "", amountPosition: 0}); + weth.transfer(address(tokenZap), AMOUNT); + tokenZap.zap(address(weth), AMOUNT, zapDataWithdraw); + vm.expectRevert(TokenZapV1.TokenZapV1__TargetZeroAddress.selector); + tokenZap.zap(address(weth), AMOUNT, zapDataTransfer); + } + + function test_zap_withdraw_transferNative_revert_targetZeroAddress_nonEmptyPayload() public { + bytes memory zapDataWithdraw = getZapDataWithdraw(0); + bytes memory payload = getVaultPayloadNoAmount(); + bytes memory zapDataTransfer = getZeroTargetZapData({payload: payload, amountPosition: uint16(payload.length)}); + weth.transfer(address(tokenZap), AMOUNT); + tokenZap.zap(address(weth), AMOUNT, zapDataWithdraw); + vm.expectRevert(TokenZapV1.TokenZapV1__TargetZeroAddress.selector); + tokenZap.zap(address(weth), AMOUNT, zapDataTransfer); + } - vm.expectRevert(TokenZapV1.TokenZapV1__AmountIncorrect.selector); - tokenZap.zap{value: 1 ether + 1 wei}(nativeGasToken, 1 ether, zapData); + function test_zap_withdraw_transferNative_revert_targetEOA_nonEmptyPayload() public { + bytes memory zapDataWithdraw = getZapDataWithdraw(0); + bytes memory zapDataTransfer = + tokenZap.encodeZapData({target: user, payload: getVaultPayloadNoAmount(), amountPosition: 0}); + weth.transfer(address(tokenZap), AMOUNT); + tokenZap.zap(address(weth), AMOUNT, zapDataWithdraw); + vm.expectRevert(abi.encodeWithSelector(Address.AddressEmptyCode.selector, user)); + tokenZap.zap(address(weth), AMOUNT, zapDataTransfer); } function test_encodeZapData_revert_payloadLengthAboveMax() public {