From 7ad44444497f6a201e1fde533aaf13e149c0be35 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 28 Oct 2024 19:04:05 +0000 Subject: [PATCH 01/17] feat: scaffold `ZapData` library --- .../contracts/libs/ZapDataV1.sol | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 packages/contracts-rfq/contracts/libs/ZapDataV1.sol diff --git a/packages/contracts-rfq/contracts/libs/ZapDataV1.sol b/packages/contracts-rfq/contracts/libs/ZapDataV1.sol new file mode 100644 index 0000000000..b7cc6ce469 --- /dev/null +++ b/packages/contracts-rfq/contracts/libs/ZapDataV1.sol @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +library ZapDataV1 { + /// @notice Version of the Zap Data struct. + uint16 internal constant VERSION = 1; + + /// @notice Value that indicates the amount is not present in the target function's payload. + uint16 internal constant AMOUNT_NOT_PRESENT = 0xFFFF; + + // Offsets of the fields in the packed ZapData struct + // uint16 version [000 .. 002) + // uint16 amountPosition [002 .. 004) + // address target [004 .. 024) + // bytes payload [024 .. ***) + + // forgefmt: disable-start + uint256 private constant OFFSET_AMOUNT_POSITION = 2; + uint256 private constant OFFSET_TARGET = 4; + uint256 private constant OFFSET_PAYLOAD = 24; + // forgefmt: disable-end + + error ZapDataV1__InvalidEncoding(); + error ZapDataV1__UnsupportedVersion(uint16 version); + + /// @notice Validates the encodedZapData to be a tightly packed encoded payload for ZapData struct. + /// @dev Checks that all the required fields are present, version is correct and amount position is valid. + function validateV1(bytes calldata encodedZapData) internal pure { + // TODO: implement + } + + /// @notice Encodes the ZapData struct by tightly packing the fields. + /// Note: we don't know the exact amount that will be used for the Zap at the time of encoding, + /// so we provide the reference index where the amount is encoded within `payload_`. This allows up to + /// hot-swap the amount in the payload, when the Zap is performed. + /// @dev `abi.decode` will not work as a result of the tightly packed fields. Use `decodeZapData` instead. + /// @param amountPosition_ Position (start index) where the amount is encoded within `payload_`. + /// This will usually be `4 + 32 * n`, where `n` is the position of the amount in + /// the list of parameters of the target function (starting from 0). + /// Or `AMOUNT_NOT_PRESENT` if the amount is not encoded within `payload_`. + /// @param target_ Address of the target contract. + /// @param payload_ Payload to be used as a calldata for the `target_` contract call. + function encodeV1( + uint16 amountPosition_, + address target_, + bytes memory payload_ + ) + internal + pure + returns (bytes memory encodedZapData) + { + // TODO: implement + } + + /// @notice Extracts the version from the encoded Zap Data. + function version(bytes calldata encodedZapData) internal pure returns (uint16) { + // TODO: implement + } + + /// @notice Extracts the target address from the encoded Zap Data. + function target(bytes calldata encodedZapData) internal pure returns (address) { + // TODO: implement + } + + /// @notice Extracts the payload from the encoded Zap Data. Replaces the amount with the provided value, + /// if it was present in the original data (amountPosition is not AMOUNT_NOT_PRESENT). + /// @dev This payload will be used as a calldata for the target contract. + function payload(bytes calldata encodedZapData, uint256 amount) internal pure returns (bytes memory) { + // TODO: implement + } +} From b7858bf8fc849e23a08553f5326d362932cbc025 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 28 Oct 2024 19:30:53 +0000 Subject: [PATCH 02/17] test: define expected behavior for ZapDataV1 --- .../test/harnesses/ZapDataV1Harness.sol | 34 +++++ .../contracts-rfq/test/libs/ZapDataV1.t.sol | 143 ++++++++++++++++++ 2 files changed, 177 insertions(+) create mode 100644 packages/contracts-rfq/test/harnesses/ZapDataV1Harness.sol create mode 100644 packages/contracts-rfq/test/libs/ZapDataV1.t.sol diff --git a/packages/contracts-rfq/test/harnesses/ZapDataV1Harness.sol b/packages/contracts-rfq/test/harnesses/ZapDataV1Harness.sol new file mode 100644 index 0000000000..b1b5cef18e --- /dev/null +++ b/packages/contracts-rfq/test/harnesses/ZapDataV1Harness.sol @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {ZapDataV1} from "../../contracts/libs/ZapDataV1.sol"; + +contract ZapDataV1Harness { + function validateV1(bytes calldata encodedZapData) public pure { + ZapDataV1.validateV1(encodedZapData); + } + + function encodeV1( + uint16 amountPosition_, + address target_, + bytes memory payload_ + ) + public + pure + returns (bytes memory encodedZapData) + { + return ZapDataV1.encodeV1(amountPosition_, target_, payload_); + } + + function version(bytes calldata encodedZapData) public pure returns (uint16) { + return ZapDataV1.version(encodedZapData); + } + + function target(bytes calldata encodedZapData) public pure returns (address) { + return ZapDataV1.target(encodedZapData); + } + + function payload(bytes calldata encodedZapData, uint256 amount) public pure returns (bytes memory) { + return ZapDataV1.payload(encodedZapData, amount); + } +} diff --git a/packages/contracts-rfq/test/libs/ZapDataV1.t.sol b/packages/contracts-rfq/test/libs/ZapDataV1.t.sol new file mode 100644 index 0000000000..19e5b378fd --- /dev/null +++ b/packages/contracts-rfq/test/libs/ZapDataV1.t.sol @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {ZapDataV1Harness, ZapDataV1} from "../harnesses/ZapDataV1Harness.sol"; + +import {Test} from "forge-std/Test.sol"; + +// solhint-disable func-name-mixedcase, ordering +contract ZapDataV1Test is Test { + uint16 internal constant EXPECTED_VERSION = 1; + + ZapDataV1Harness internal harness; + + function setUp() public { + harness = new ZapDataV1Harness(); + } + + function encodeZapData( + uint16 version, + uint16 amountPosition, + address target, + bytes memory payload + ) + public + pure + returns (bytes memory) + { + return abi.encodePacked(version, amountPosition, target, payload); + } + + function test_roundtrip_withAmount( + address target, + uint256 amount, + bytes memory prefix, + bytes memory postfix + ) + public + view + { + vm.assume(prefix.length + 32 + postfix.length < type(uint16).max); + + // We don't know the amount at the time of encoding, so we provide a placeholder. + uint16 amountPosition = uint16(prefix.length); + bytes memory encodedPayload = abi.encodePacked(prefix, uint256(0), postfix); + // We expect the correct amount to be substituted in the payload at the time of Zap. + bytes memory finalPayload = abi.encodePacked(prefix, amount, postfix); + + bytes memory zapData = harness.encodeV1(amountPosition, target, encodedPayload); + + harness.validateV1(zapData); + assertEq(harness.version(zapData), 1); + assertEq(harness.target(zapData), target); + assertEq(harness.payload(zapData, amount), finalPayload); + // Check against manually encoded ZapData. + assertEq(zapData, encodeZapData(EXPECTED_VERSION, amountPosition, target, encodedPayload)); + } + + function test_roundtrip_noAmount(address target, uint256 amount, bytes memory payload) public view { + vm.assume(payload.length < type(uint16).max); + + uint16 amountPosition = type(uint16).max; + bytes memory zapData = harness.encodeV1(amountPosition, target, payload); + + harness.validateV1(zapData); + assertEq(harness.version(zapData), 1); + assertEq(harness.target(zapData), target); + assertEq(harness.payload(zapData, amount), payload); + // Check against manually encoded ZapData. + assertEq(zapData, encodeZapData(EXPECTED_VERSION, amountPosition, target, payload)); + } + + function test_encodeDecodeV1_revert_invalidAmountPosition( + address target, + uint16 amountPosition, + uint256 amount, + bytes memory payload + ) + public + { + vm.assume(payload.length < type(uint16).max); + // Make sure that (amountPosition + 32) is outside the bounds of the payload. + uint16 incorrectMin = payload.length > 31 ? uint16(payload.length) - 31 : 0; + uint16 incorrectMax = type(uint16).max - 1; + amountPosition = uint16(bound(uint256(amountPosition), incorrectMin, incorrectMax)); + bytes memory invalidEncodedZapData = abi.encodePacked(uint16(1), amountPosition, target, payload); + + vm.expectRevert(ZapDataV1.ZapDataV1__InvalidEncoding.selector); + harness.encodeV1(amountPosition, target, payload); + + // Validation should pass + harness.validateV1(invalidEncodedZapData); + harness.target(invalidEncodedZapData); + // But payload extraction should revert + vm.expectRevert(ZapDataV1.ZapDataV1__InvalidEncoding.selector); + harness.payload(invalidEncodedZapData, amount); + } + + function test_validateV1_revert_unsupportedVersion_withAmount( + uint16 version, + address target, + bytes memory prefix, + bytes memory postfix + ) + public + { + vm.assume(version != 1); + vm.assume(prefix.length + 32 + postfix.length < type(uint16).max); + // We don't know the amount at the time of encoding, so we provide a placeholder. + uint16 amountPosition = uint16(prefix.length); + bytes memory encodedPayload = abi.encodePacked(prefix, uint256(0), postfix); + + bytes memory invalidEncodedZapData = encodeZapData(version, amountPosition, target, encodedPayload); + + vm.expectRevert(abi.encodeWithSelector(ZapDataV1.ZapDataV1__UnsupportedVersion.selector, version)); + harness.validateV1(invalidEncodedZapData); + } + + function test_validateV1_revert_unsupportedVersion_noAmount( + uint16 version, + address target, + bytes memory payload + ) + public + { + vm.assume(version != 1); + vm.assume(payload.length < type(uint16).max); + + uint16 amountPosition = type(uint16).max; + bytes memory invalidEncodedZapData = encodeZapData(version, amountPosition, target, payload); + + vm.expectRevert(abi.encodeWithSelector(ZapDataV1.ZapDataV1__UnsupportedVersion.selector, version)); + harness.validateV1(invalidEncodedZapData); + } + + function test_validateV1_revert_invalidLength(bytes calldata fuzzData) public { + bytes memory minimumValidZapData = encodeZapData(EXPECTED_VERSION, type(uint16).max, address(0), ""); + uint256 invalidLength = fuzzData.length % minimumValidZapData.length; + bytes calldata invalidEncodedZapData = fuzzData[:invalidLength]; + + vm.expectRevert(ZapDataV1.ZapDataV1__InvalidEncoding.selector); + harness.validateV1(invalidEncodedZapData); + } +} From 926525784a846d049b35c859305629619793bd2e Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 28 Oct 2024 19:38:24 +0000 Subject: [PATCH 03/17] feat: encoding, validation --- .../contracts/libs/ZapDataV1.sol | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/contracts-rfq/contracts/libs/ZapDataV1.sol b/packages/contracts-rfq/contracts/libs/ZapDataV1.sol index b7cc6ce469..22fc5f6fb9 100644 --- a/packages/contracts-rfq/contracts/libs/ZapDataV1.sol +++ b/packages/contracts-rfq/contracts/libs/ZapDataV1.sol @@ -24,22 +24,29 @@ library ZapDataV1 { error ZapDataV1__UnsupportedVersion(uint16 version); /// @notice Validates the encodedZapData to be a tightly packed encoded payload for ZapData struct. - /// @dev Checks that all the required fields are present, version is correct and amount position is valid. + /// @dev Checks that all the required fields are present and the version is correct. function validateV1(bytes calldata encodedZapData) internal pure { - // TODO: implement + // Check the minimum length: must at least include all static fields. + if (encodedZapData.length < OFFSET_PAYLOAD) revert ZapDataV1__InvalidEncoding(); + // Once we validated the length, we can be sure that the version field is present. + uint16 version_ = version(encodedZapData); + if (version_ != VERSION) revert ZapDataV1__UnsupportedVersion(version_); } /// @notice Encodes the ZapData struct by tightly packing the fields. - /// Note: we don't know the exact amount that will be used for the Zap at the time of encoding, - /// so we provide the reference index where the amount is encoded within `payload_`. This allows up to - /// hot-swap the amount in the payload, when the Zap is performed. + /// Note: we don't know the exact amount of tokens that will be used for the Zap at the time of encoding, + /// so we provide the reference index where the token amount is encoded within `payload_`. This allows up to + /// hot-swap the token amount in the payload, when the Zap is performed. /// @dev `abi.decode` will not work as a result of the tightly packed fields. Use `decodeZapData` instead. - /// @param amountPosition_ Position (start index) where the amount is encoded within `payload_`. - /// This will usually be `4 + 32 * n`, where `n` is the position of the amount in + /// @param amountPosition_ Position (start index) where the token amount is encoded within `payload_`. + /// This will usually be `4 + 32 * n`, where `n` is the position of the token amount in /// the list of parameters of the target function (starting from 0). - /// Or `AMOUNT_NOT_PRESENT` if the amount is not encoded within `payload_`. + /// Or `AMOUNT_NOT_PRESENT` if the token amount is not encoded within `payload_`. /// @param target_ Address of the target contract. - /// @param payload_ Payload to be used as a calldata for the `target_` contract call. + /// @param payload_ ABI-encoded calldata to be used for the `target_` contract call. + /// If the target function has the token amount as an argument, any placeholder amount value + /// can be used for the original ABI encoding of `payload_`. The placeholder amount will + /// be replaced with the actual amount, when the Zap Data is decoded. function encodeV1( uint16 amountPosition_, address target_, @@ -49,7 +56,11 @@ library ZapDataV1 { pure returns (bytes memory encodedZapData) { - // TODO: implement + // Amount is encoded in [amountPosition_ .. amountPosition_ + 32), which should be within the payload. + if (amountPosition_ != AMOUNT_NOT_PRESENT && (uint256(amountPosition_) + 32 > payload_.length)) { + revert ZapDataV1__InvalidEncoding(); + } + return abi.encodePacked(VERSION, amountPosition_, target_, payload_); } /// @notice Extracts the version from the encoded Zap Data. @@ -62,8 +73,8 @@ library ZapDataV1 { // TODO: implement } - /// @notice Extracts the payload from the encoded Zap Data. Replaces the amount with the provided value, - /// if it was present in the original data (amountPosition is not AMOUNT_NOT_PRESENT). + /// @notice Extracts the payload from the encoded Zap Data. Replaces the token amount with the provided value, + /// if it was present in the original data (if amountPosition is not AMOUNT_NOT_PRESENT). /// @dev This payload will be used as a calldata for the target contract. function payload(bytes calldata encodedZapData, uint256 amount) internal pure returns (bytes memory) { // TODO: implement From 38d8673afc0341c462c806a271f1ecd7eb310ca2 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 28 Oct 2024 19:48:37 +0000 Subject: [PATCH 04/17] feat: decoding --- .../contracts/libs/ZapDataV1.sol | 45 ++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/packages/contracts-rfq/contracts/libs/ZapDataV1.sol b/packages/contracts-rfq/contracts/libs/ZapDataV1.sol index 22fc5f6fb9..c82f01dd8e 100644 --- a/packages/contracts-rfq/contracts/libs/ZapDataV1.sol +++ b/packages/contracts-rfq/contracts/libs/ZapDataV1.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; +// solhint-disable no-inline-assembly library ZapDataV1 { /// @notice Version of the Zap Data struct. uint16 internal constant VERSION = 1; @@ -64,19 +65,53 @@ library ZapDataV1 { } /// @notice Extracts the version from the encoded Zap Data. - function version(bytes calldata encodedZapData) internal pure returns (uint16) { - // TODO: implement + function version(bytes calldata encodedZapData) internal pure returns (uint16 version_) { + // Load 32 bytes from the start and shift it 240 bits to the right to get the highest 16 bits. + assembly { + version_ := shr(240, calldataload(encodedZapData.offset)) + } } /// @notice Extracts the target address from the encoded Zap Data. - function target(bytes calldata encodedZapData) internal pure returns (address) { - // TODO: implement + function target(bytes calldata encodedZapData) internal pure returns (address target_) { + // Load 32 bytes from the offset and shift it 96 bits to the right to get the highest 160 bits. + assembly { + target_ := shr(96, calldataload(add(encodedZapData.offset, OFFSET_TARGET))) + } } /// @notice Extracts the payload from the encoded Zap Data. Replaces the token amount with the provided value, /// if it was present in the original data (if amountPosition is not AMOUNT_NOT_PRESENT). /// @dev This payload will be used as a calldata for the target contract. function payload(bytes calldata encodedZapData, uint256 amount) internal pure returns (bytes memory) { - // TODO: implement + // The original payload is located at encodedZapData[OFFSET_PAYLOAD:]. + uint16 amountPosition = _amountPosition(encodedZapData); + // If the amount was not present in the original payload, return the payload as is. + if (amountPosition == AMOUNT_NOT_PRESENT) { + return encodedZapData[OFFSET_PAYLOAD:]; + } + // Calculate the start and end indexes of the amount in ZapData from its position within the payload. + // Note: we use inclusive start and exclusive end indexes for easier slicing of the ZapData. + uint256 amountStartIndexIncl = OFFSET_PAYLOAD + amountPosition; + uint256 amountEndIndexExcl = amountStartIndexIncl + 32; + // Check that the amount is within the ZapData. + if (amountEndIndexExcl > encodedZapData.length) revert ZapDataV1__InvalidEncoding(); + // Otherwise we need to replace the amount in the payload with the provided value. + return abi.encodePacked( + // Copy the original payload up to the amount + encodedZapData[OFFSET_PAYLOAD:amountStartIndexIncl], + // Replace the originally encoded amount with the provided value + amount, + // Copy the rest of the payload after the amount + encodedZapData[amountEndIndexExcl:] + ); + } + + /// @notice Extracts the amount position from the encoded Zap Data. + function _amountPosition(bytes calldata encodedZapData) private pure returns (uint16 amountPosition) { + // Load 32 bytes from the offset and shift it 240 bits to the right to get the highest 16 bits. + assembly { + amountPosition := shr(240, calldataload(add(encodedZapData.offset, OFFSET_AMOUNT_POSITION))) + } } } From 61eb61d3270f4b663ad71d0ee932c2af6f45bf1d Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Tue, 29 Oct 2024 12:40:06 +0000 Subject: [PATCH 05/17] feat: scaffold `TokenZap` --- .../contracts-rfq/contracts/zaps/TokenZap.sol | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 packages/contracts-rfq/contracts/zaps/TokenZap.sol diff --git a/packages/contracts-rfq/contracts/zaps/TokenZap.sol b/packages/contracts-rfq/contracts/zaps/TokenZap.sol new file mode 100644 index 0000000000..84a6dcad4f --- /dev/null +++ b/packages/contracts-rfq/contracts/zaps/TokenZap.sol @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {IZapRecipient} from "../interfaces/IZapRecipient.sol"; + +contract TokenZap is IZapRecipient { + error TokenZap__AmountIncorrect(); + error TokenZap__PayloadLengthAboveMax(); + + /// @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). + /// @dev The provided ZapData contains the target address and calldata for the Zap action, and must be + /// encoded using the encodeZapData function. + /// @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 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) { + // TODO: implement + } + + /// @notice Encodes the ZapData for a Zap action. + /// Note: at the time of encoding we don't know the exact amount of tokens that will be used for the Zap, + /// as we don't have a quote for performing a Zap. Therefore a placeholder value for amount must be used + /// when abi-encoding the payload. A reference index where the actual amount is encoded within the payload + /// must be provided in order to replace the placeholder with the actual amount when the Zap is performed. + /// @param target Address of the target contract. + /// @param payload ABI-encoded calldata to be used for the `target` contract call. + /// If the target function has the token amount as an argument, any placeholder amount value + /// can be used for the original ABI encoding of `payload`. The placeholder amount will + /// be replaced with the actual amount, when the Zap Data is decoded. + /// @param amountPosition Position (start index) where the token amount is encoded within `payload`. + /// This will usually be `4 + 32 * n`, where `n` is the position of the token amount in + /// the list of parameters of the target function (starting from 0). + /// Any value greater or equal to `payload.length` can be used if the token amount is + /// not an argument of the target function. + function encodeZapData( + address target, + bytes memory payload, + uint256 amountPosition + ) + external + view + returns (bytes memory) + { + // TODO: implement + } + + /// @notice Decodes the ZapData for a Zap action. Replaces the placeholder amount with the actual amount, + /// if it was present in the original `payload`. Otherwise returns the original `payload` as is. + /// @param zapData Encoded Zap Data containing the target address and calldata for the Zap action. + /// @param amount Actual amount of the token to be used for the Zap action. + function decodeZapData( + bytes calldata zapData, + uint256 amount + ) + external + view + returns (address target, bytes memory payload) + { + // TODO: implement + } +} From 217859cc0294c4b5240330b08fd06fa8842d36ab Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Tue, 29 Oct 2024 13:44:12 +0000 Subject: [PATCH 06/17] test: add coverage for TokenZap --- .../test/mocks/VaultManyArguments.sol | 33 +++++ .../contracts-rfq/test/mocks/VaultMock.sol | 24 +++ .../contracts-rfq/test/zaps/TokenZap.t.sol | 137 ++++++++++++++++++ 3 files changed, 194 insertions(+) create mode 100644 packages/contracts-rfq/test/mocks/VaultManyArguments.sol create mode 100644 packages/contracts-rfq/test/mocks/VaultMock.sol create mode 100644 packages/contracts-rfq/test/zaps/TokenZap.t.sol diff --git a/packages/contracts-rfq/test/mocks/VaultManyArguments.sol b/packages/contracts-rfq/test/mocks/VaultManyArguments.sol new file mode 100644 index 0000000000..6b8149cddd --- /dev/null +++ b/packages/contracts-rfq/test/mocks/VaultManyArguments.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {VaultMock} from "./VaultMock.sol"; + +/// @notice Vault mock for testing purposes. DO NOT USE IN PRODUCTION. +contract VaultManyArguments is VaultMock { + error VaultManyArguments__InvalidBytes(); + + function deposit( + address token, + bytes memory encodedToken, + uint256 amount, + address user, + bytes memory encodedUser + ) + external + payable + { + // Make sure the data is not malformed + _validateBytes(token, encodedToken); + _validateBytes(user, encodedUser); + _deposit(user, token, amount); + } + + function depositNative(address user) external payable { + _deposit(user, NATIVE_GAS_TOKEN, msg.value); + } + + function _validateBytes(address addr, bytes memory encoded) internal pure { + if (keccak256(abi.encode(addr)) != keccak256(encoded)) revert VaultManyArguments__InvalidBytes(); + } +} diff --git a/packages/contracts-rfq/test/mocks/VaultMock.sol b/packages/contracts-rfq/test/mocks/VaultMock.sol new file mode 100644 index 0000000000..b23fd55a41 --- /dev/null +++ b/packages/contracts-rfq/test/mocks/VaultMock.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {SafeERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; + +/// @notice Vault mock for testing purposes. DO NOT USE IN PRODUCTION. +abstract contract VaultMock { + using SafeERC20 for IERC20; + + address internal constant NATIVE_GAS_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; + + mapping(address user => mapping(address token => uint256 amount)) public balanceOf; + + error VaultMock__AmountIncorrect(); + + function _deposit(address user, address token, uint256 amount) internal { + if (token == NATIVE_GAS_TOKEN) { + if (msg.value != amount) revert VaultMock__AmountIncorrect(); + } else { + IERC20(token).safeTransferFrom(msg.sender, address(this), amount); + } + balanceOf[user][token] += amount; + } +} diff --git a/packages/contracts-rfq/test/zaps/TokenZap.t.sol b/packages/contracts-rfq/test/zaps/TokenZap.t.sol new file mode 100644 index 0000000000..e82c708f1e --- /dev/null +++ b/packages/contracts-rfq/test/zaps/TokenZap.t.sol @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {TokenZap} from "../../contracts/zaps/TokenZap.sol"; +import {VaultManyArguments} from "../mocks/VaultManyArguments.sol"; +import {MockERC20} from "../MockERC20.sol"; + +import {Test} from "forge-std/Test.sol"; + +// solhint-disable func-name-mixedcase, ordering +contract TokenZapTest is Test { + uint256 internal constant AMOUNT = 0.987 ether; + + TokenZap internal tokenZap; + VaultManyArguments internal vault; + MockERC20 internal erc20; + + address internal user; + address internal nativeGasToken = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; + + function setUp() public { + tokenZap = new TokenZap(); + vault = new VaultManyArguments(); + erc20 = new MockERC20("TKN", 18); + + user = makeAddr("user"); + + erc20.mint(address(this), 100 * AMOUNT); + deal(address(this), 100 * AMOUNT); + } + + function getVaultPayload(address token, uint256 amount) public view returns (bytes memory) { + return abi.encodeCall(vault.deposit, (token, abi.encode(token), amount, user, abi.encode(user))); + } + + function getVaultPayloadNoAmount() public view returns (bytes memory) { + return abi.encodeCall(vault.depositNative, (user)); + } + + function getZapData(bytes memory originalPayload) public view returns (bytes memory) { + // Amount is the third argument of the deposit function + return tokenZap.encodeZapData(address(vault), originalPayload, 4 + 32 * 2); + } + + function checkERC20HappyPath(bytes memory originalPayload) public { + bytes memory zapData = getZapData(originalPayload); + + // Transfer tokens to the zap contract first + erc20.transfer(address(tokenZap), AMOUNT); + bytes4 returnValue = tokenZap.zap(address(erc20), AMOUNT, zapData); + assertEq(returnValue, tokenZap.zap.selector); + // Check that the vault registered the deposit + assertEq(vault.balanceOf(user, address(erc20)), AMOUNT); + } + + function test_zap_erc20_placeholderZero() public { + bytes memory originalPayload = getVaultPayload(address(erc20), 0); + checkERC20HappyPath(originalPayload); + } + + function test_zap_erc20_placeholderNonZero() public { + // Use the approximate amount of tokens as placeholder + bytes memory originalPayload = getVaultPayload(address(erc20), 1 ether); + checkERC20HappyPath(originalPayload); + } + + function checkNativeHappyPath(bytes memory originalPayload) public { + bytes memory zapData = getZapData(originalPayload); + + bytes4 returnValue = tokenZap.zap{value: AMOUNT}(nativeGasToken, AMOUNT, zapData); + assertEq(returnValue, tokenZap.zap.selector); + // Check that the vault registered the deposit + assertEq(vault.balanceOf(user, nativeGasToken), AMOUNT); + } + + function test_zap_native_placeholderZero() public { + bytes memory originalPayload = getVaultPayload(nativeGasToken, 0); + checkNativeHappyPath(originalPayload); + } + + function test_zap_native_placeholderNonZero() public { + // Use the approximate amount of tokens as placeholder + bytes memory originalPayload = getVaultPayload(nativeGasToken, 1 ether); + checkNativeHappyPath(originalPayload); + } + + function test_zap_native_noAmount() public { + bytes memory originalPayload = getVaultPayloadNoAmount(); + checkNativeHappyPath(originalPayload); + } + + function test_encodeZapData_roundtrip(address token, uint256 placeholderAmount, uint256 amount) public view { + bytes memory originalPayload = getVaultPayload(token, placeholderAmount); + bytes memory expectedPayload = getVaultPayload(token, amount); + + bytes memory zapData = getZapData(originalPayload); + (address target, bytes memory payload) = tokenZap.decodeZapData(zapData, amount); + + assertEq(target, address(vault)); + assertEq(payload, expectedPayload); + } + + function test_encodeZapData_roundtripNoAmount(uint256 amountPosition) public view { + bytes memory payload = getVaultPayloadNoAmount(); + // Any value >= payload.length could be used to signal that the amount is not an argument of the target function + amountPosition = bound(amountPosition, payload.length, type(uint256).max); + + bytes memory zapData = tokenZap.encodeZapData(address(vault), payload, amountPosition); + (address target, bytes memory decodedPayload) = tokenZap.decodeZapData(zapData, 0); + assertEq(target, address(vault)); + assertEq(decodedPayload, payload); + } + + // ══════════════════════════════════════════════════ REVERTS ══════════════════════════════════════════════════════ + + function test_zap_native_revert_msgValueLowerThanExpected() public { + bytes memory originalPayload = getVaultPayload(nativeGasToken, 0); + bytes memory zapData = getZapData(originalPayload); + + vm.expectRevert(TokenZap.TokenZap__AmountIncorrect.selector); + 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); + + vm.expectRevert(TokenZap.TokenZap__AmountIncorrect.selector); + tokenZap.zap{value: 1 ether + 1 wei}(nativeGasToken, 1 ether, zapData); + } + + function test_encodeZapData_revert_payloadLengthAboveMax() public { + bytes memory tooLongPayload = new bytes(2 ** 16); + vm.expectRevert(TokenZap.TokenZap__PayloadLengthAboveMax.selector); + tokenZap.encodeZapData(address(vault), tooLongPayload, 0); + } +} From 14cef361744783dd2b346f9e1b7cd52f6727573a Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Tue, 29 Oct 2024 13:44:27 +0000 Subject: [PATCH 07/17] feat: expose encoding/decoding --- .../contracts-rfq/contracts/zaps/TokenZap.sol | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/contracts-rfq/contracts/zaps/TokenZap.sol b/packages/contracts-rfq/contracts/zaps/TokenZap.sol index 84a6dcad4f..ccdcd6d704 100644 --- a/packages/contracts-rfq/contracts/zaps/TokenZap.sol +++ b/packages/contracts-rfq/contracts/zaps/TokenZap.sol @@ -2,8 +2,11 @@ pragma solidity 0.8.24; import {IZapRecipient} from "../interfaces/IZapRecipient.sol"; +import {ZapDataV1} from "../libs/ZapDataV1.sol"; contract TokenZap is IZapRecipient { + using ZapDataV1 for bytes; + error TokenZap__AmountIncorrect(); error TokenZap__PayloadLengthAboveMax(); @@ -41,10 +44,17 @@ contract TokenZap is IZapRecipient { uint256 amountPosition ) external - view + pure returns (bytes memory) { - // TODO: implement + if (payload.length > ZapDataV1.AMOUNT_NOT_PRESENT) { + revert TokenZap__PayloadLengthAboveMax(); + } + if (amountPosition >= payload.length) { + amountPosition = ZapDataV1.AMOUNT_NOT_PRESENT; + } + // At this point we checked that both amountPosition and payload.length fit in uint16 + return ZapDataV1.encodeV1(uint16(amountPosition), target, payload); } /// @notice Decodes the ZapData for a Zap action. Replaces the placeholder amount with the actual amount, @@ -56,9 +66,11 @@ contract TokenZap is IZapRecipient { uint256 amount ) external - view + pure returns (address target, bytes memory payload) { - // TODO: implement + zapData.validateV1(); + target = zapData.target(); + payload = zapData.payload(amount); } } From 5ca648c82ff5086366a527215b82b06763264e2f Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Tue, 29 Oct 2024 14:47:23 +0000 Subject: [PATCH 08/17] feat: implement `zap` --- .../contracts-rfq/contracts/zaps/TokenZap.sol | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/contracts-rfq/contracts/zaps/TokenZap.sol b/packages/contracts-rfq/contracts/zaps/TokenZap.sol index ccdcd6d704..fe4ecd4e7f 100644 --- a/packages/contracts-rfq/contracts/zaps/TokenZap.sol +++ b/packages/contracts-rfq/contracts/zaps/TokenZap.sol @@ -4,9 +4,15 @@ pragma solidity 0.8.24; import {IZapRecipient} from "../interfaces/IZapRecipient.sol"; import {ZapDataV1} from "../libs/ZapDataV1.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; +import {SafeERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; + contract TokenZap is IZapRecipient { + using SafeERC20 for IERC20; using ZapDataV1 for bytes; + address public constant NATIVE_GAS_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; + error TokenZap__AmountIncorrect(); error TokenZap__PayloadLengthAboveMax(); @@ -20,7 +26,25 @@ contract TokenZap is IZapRecipient { /// @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) { - // TODO: implement + // Check that the ZapData is valid before decoding it + zapData.validateV1(); + address target = zapData.target(); + // Approve the target contract to spend the token. TokenZap does not custody any tokens outside of the + // zap action, so we can approve the arbitrary target contract. + if (token == NATIVE_GAS_TOKEN) { + // No approvals are needed for the native gas token, just check that the amount is correct + if (msg.value != amount) revert TokenZap__AmountIncorrect(); + } else { + // Issue the approval only if the current allowance is less than the required amount + if (IERC20(token).allowance(address(this), target) < amount) { + IERC20(token).forceApprove(target, type(uint256).max); + } + } + // Perform the Zap action, forwarding full msg.value to the target contract + // Note: this will bubble up any revert from the target contract + bytes memory payload = zapData.payload(amount); + Address.functionCallWithValue({target: target, data: payload, value: msg.value}); + return this.zap.selector; } /// @notice Encodes the ZapData for a Zap action. @@ -65,7 +89,7 @@ contract TokenZap is IZapRecipient { bytes calldata zapData, uint256 amount ) - external + public pure returns (address target, bytes memory payload) { From 7f4c1c485da63dd1cb15bcec6004d14d4e7a03ea Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Tue, 29 Oct 2024 19:13:29 +0000 Subject: [PATCH 09/17] fix: noAmount test, slight refactor --- .../contracts-rfq/test/zaps/TokenZap.t.sol | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/contracts-rfq/test/zaps/TokenZap.t.sol b/packages/contracts-rfq/test/zaps/TokenZap.t.sol index e82c708f1e..b993436efd 100644 --- a/packages/contracts-rfq/test/zaps/TokenZap.t.sol +++ b/packages/contracts-rfq/test/zaps/TokenZap.t.sol @@ -42,9 +42,11 @@ contract TokenZapTest is Test { return tokenZap.encodeZapData(address(vault), originalPayload, 4 + 32 * 2); } - function checkERC20HappyPath(bytes memory originalPayload) public { - bytes memory zapData = getZapData(originalPayload); + function getZapDataNoAmount(bytes memory originalPayload) public view returns (bytes memory) { + return tokenZap.encodeZapData(address(vault), originalPayload, originalPayload.length); + } + function checkERC20HappyPath(bytes memory zapData) public { // Transfer tokens to the zap contract first erc20.transfer(address(tokenZap), AMOUNT); bytes4 returnValue = tokenZap.zap(address(erc20), AMOUNT, zapData); @@ -54,19 +56,17 @@ contract TokenZapTest is Test { } function test_zap_erc20_placeholderZero() public { - bytes memory originalPayload = getVaultPayload(address(erc20), 0); - checkERC20HappyPath(originalPayload); + bytes memory zapData = getZapData(getVaultPayload(address(erc20), 0)); + checkERC20HappyPath(zapData); } function test_zap_erc20_placeholderNonZero() public { // Use the approximate amount of tokens as placeholder - bytes memory originalPayload = getVaultPayload(address(erc20), 1 ether); - checkERC20HappyPath(originalPayload); + bytes memory zapData = getZapData(getVaultPayload(address(erc20), 1 ether)); + checkERC20HappyPath(zapData); } - function checkNativeHappyPath(bytes memory originalPayload) public { - bytes memory zapData = getZapData(originalPayload); - + function checkNativeHappyPath(bytes memory zapData) public { bytes4 returnValue = tokenZap.zap{value: AMOUNT}(nativeGasToken, AMOUNT, zapData); assertEq(returnValue, tokenZap.zap.selector); // Check that the vault registered the deposit @@ -74,19 +74,19 @@ contract TokenZapTest is Test { } function test_zap_native_placeholderZero() public { - bytes memory originalPayload = getVaultPayload(nativeGasToken, 0); - checkNativeHappyPath(originalPayload); + bytes memory zapData = getZapData(getVaultPayload(nativeGasToken, 0)); + checkNativeHappyPath(zapData); } function test_zap_native_placeholderNonZero() public { // Use the approximate amount of tokens as placeholder - bytes memory originalPayload = getVaultPayload(nativeGasToken, 1 ether); - checkNativeHappyPath(originalPayload); + bytes memory zapData = getZapData(getVaultPayload(nativeGasToken, 1 ether)); + checkNativeHappyPath(zapData); } function test_zap_native_noAmount() public { - bytes memory originalPayload = getVaultPayloadNoAmount(); - checkNativeHappyPath(originalPayload); + bytes memory zapData = getZapDataNoAmount(getVaultPayloadNoAmount()); + checkNativeHappyPath(zapData); } function test_encodeZapData_roundtrip(address token, uint256 placeholderAmount, uint256 amount) public view { From 596c60dd13bb1201b4c27397a3f1c1ad9d4bca36 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Tue, 29 Oct 2024 19:15:27 +0000 Subject: [PATCH 10/17] test: scenarios where target contract reverts --- .../test/mocks/VaultManyArguments.sol | 8 ++++++-- .../contracts-rfq/test/zaps/TokenZap.t.sol | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/contracts-rfq/test/mocks/VaultManyArguments.sol b/packages/contracts-rfq/test/mocks/VaultManyArguments.sol index 6b8149cddd..1d3c415aef 100644 --- a/packages/contracts-rfq/test/mocks/VaultManyArguments.sol +++ b/packages/contracts-rfq/test/mocks/VaultManyArguments.sol @@ -5,7 +5,7 @@ import {VaultMock} from "./VaultMock.sol"; /// @notice Vault mock for testing purposes. DO NOT USE IN PRODUCTION. contract VaultManyArguments is VaultMock { - error VaultManyArguments__InvalidBytes(); + error VaultManyArguments__SomeError(); function deposit( address token, @@ -27,7 +27,11 @@ contract VaultManyArguments is VaultMock { _deposit(user, NATIVE_GAS_TOKEN, msg.value); } + function depositWithRevert() external payable { + revert VaultManyArguments__SomeError(); + } + function _validateBytes(address addr, bytes memory encoded) internal pure { - if (keccak256(abi.encode(addr)) != keccak256(encoded)) revert VaultManyArguments__InvalidBytes(); + if (keccak256(abi.encode(addr)) != keccak256(encoded)) revert VaultManyArguments__SomeError(); } } diff --git a/packages/contracts-rfq/test/zaps/TokenZap.t.sol b/packages/contracts-rfq/test/zaps/TokenZap.t.sol index b993436efd..f99c165f81 100644 --- a/packages/contracts-rfq/test/zaps/TokenZap.t.sol +++ b/packages/contracts-rfq/test/zaps/TokenZap.t.sol @@ -37,6 +37,10 @@ contract TokenZapTest is Test { return abi.encodeCall(vault.depositNative, (user)); } + function getVaultPayloadWithRevert() public view returns (bytes memory) { + return abi.encodeCall(vault.depositWithRevert, ()); + } + function getZapData(bytes memory originalPayload) public view returns (bytes memory) { // Amount is the third argument of the deposit function return tokenZap.encodeZapData(address(vault), originalPayload, 4 + 32 * 2); @@ -113,6 +117,20 @@ contract TokenZapTest is Test { // ══════════════════════════════════════════════════ REVERTS ══════════════════════════════════════════════════════ + function test_zap_erc20_revert_targetReverted() public { + bytes memory zapData = getZapData(getVaultPayloadWithRevert()); + // Transfer tokens to the zap contract first + erc20.transfer(address(tokenZap), AMOUNT); + vm.expectRevert(VaultManyArguments.VaultManyArguments__SomeError.selector); + tokenZap.zap(address(erc20), AMOUNT, zapData); + } + + function test_zap_native_revert_targetReverted() public { + bytes memory zapData = getZapData(getVaultPayloadWithRevert()); + vm.expectRevert(VaultManyArguments.VaultManyArguments__SomeError.selector); + tokenZap.zap{value: AMOUNT}(nativeGasToken, AMOUNT, zapData); + } + function test_zap_native_revert_msgValueLowerThanExpected() public { bytes memory originalPayload = getVaultPayload(nativeGasToken, 0); bytes memory zapData = getZapData(originalPayload); From 3202b7059beea76d84953ed178678184db04b0d6 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Tue, 29 Oct 2024 19:39:57 +0000 Subject: [PATCH 11/17] test: extra/missing funds scenarios --- .../contracts-rfq/test/zaps/TokenZap.t.sol | 65 +++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/packages/contracts-rfq/test/zaps/TokenZap.t.sol b/packages/contracts-rfq/test/zaps/TokenZap.t.sol index f99c165f81..35b1fb3e43 100644 --- a/packages/contracts-rfq/test/zaps/TokenZap.t.sol +++ b/packages/contracts-rfq/test/zaps/TokenZap.t.sol @@ -50,10 +50,10 @@ contract TokenZapTest is Test { return tokenZap.encodeZapData(address(vault), originalPayload, originalPayload.length); } - function checkERC20HappyPath(bytes memory zapData) public { + function checkERC20HappyPath(bytes memory zapData, uint256 msgValue) public { // Transfer tokens to the zap contract first erc20.transfer(address(tokenZap), AMOUNT); - bytes4 returnValue = tokenZap.zap(address(erc20), AMOUNT, zapData); + bytes4 returnValue = tokenZap.zap{value: msgValue}(address(erc20), AMOUNT, zapData); assertEq(returnValue, tokenZap.zap.selector); // Check that the vault registered the deposit assertEq(vault.balanceOf(user, address(erc20)), AMOUNT); @@ -61,13 +61,41 @@ contract TokenZapTest is Test { function test_zap_erc20_placeholderZero() public { bytes memory zapData = getZapData(getVaultPayload(address(erc20), 0)); - checkERC20HappyPath(zapData); + checkERC20HappyPath(zapData, 0); } function test_zap_erc20_placeholderNonZero() public { // Use the approximate amount of tokens as placeholder bytes memory zapData = getZapData(getVaultPayload(address(erc20), 1 ether)); - checkERC20HappyPath(zapData); + checkERC20HappyPath(zapData, 0); + } + + function test_zap_erc20_placeholderZero_withMsgValue() public { + bytes memory zapData = getZapData(getVaultPayload(address(erc20), 0)); + checkERC20HappyPath(zapData, 123_456); + // Should forward the msg.value to the vault + assertEq(address(vault).balance, 123_456); + } + + function test_zap_erc20_placeholderNonZero_withMsgValue() public { + bytes memory zapData = getZapData(getVaultPayload(address(erc20), 1 ether)); + checkERC20HappyPath(zapData, 123_456); + // Should forward the msg.value to the vault + assertEq(address(vault).balance, 123_456); + } + + function test_zap_erc20_placeholderZero_extraTokens() public { + // Mint some extra tokens to the zap contract + erc20.mint(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_erc20_placeholderZero(); + } + + function test_zap_erc20_placeholderNonZero_extraTokens() public { + // Mint some extra tokens to the zap contract + erc20.mint(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_erc20_placeholderNonZero(); } function checkNativeHappyPath(bytes memory zapData) public { @@ -93,6 +121,27 @@ contract TokenZapTest is Test { checkNativeHappyPath(zapData); } + function test_zap_native_placeholderZero_extraNative() public { + // Mint some extra native tokens to the zap contract + deal(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_native_placeholderZero(); + } + + function test_zap_native_placeholderNonZero_extraNative() public { + // Mint some extra native tokens to the zap contract + deal(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_native_placeholderNonZero(); + } + + function test_zap_native_noAmount_extraNative() public { + // Mint some extra native tokens to the zap contract + deal(address(tokenZap), AMOUNT); + // Should not affect the zap + test_zap_native_noAmount(); + } + function test_encodeZapData_roundtrip(address token, uint256 placeholderAmount, uint256 amount) public view { bytes memory originalPayload = getVaultPayload(token, placeholderAmount); bytes memory expectedPayload = getVaultPayload(token, amount); @@ -117,6 +166,14 @@ contract TokenZapTest is Test { // ══════════════════════════════════════════════════ REVERTS ══════════════════════════════════════════════════════ + 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 + erc20.transfer(address(tokenZap), AMOUNT - 1); + vm.expectRevert(); + tokenZap.zap(address(erc20), AMOUNT, zapData); + } + function test_zap_erc20_revert_targetReverted() public { bytes memory zapData = getZapData(getVaultPayloadWithRevert()); // Transfer tokens to the zap contract first From e76174f585df67d20702c7859a0f9916210797f6 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:23:21 +0000 Subject: [PATCH 12/17] refactor: TokenZap -> TokenZapV1 --- .../zaps/{TokenZap.sol => TokenZapV1.sol} | 12 ++++++------ .../test/zaps/{TokenZap.t.sol => TokenZapV1.t.sol} | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) rename packages/contracts-rfq/contracts/zaps/{TokenZap.sol => TokenZapV1.sol} (94%) rename packages/contracts-rfq/test/zaps/{TokenZap.t.sol => TokenZapV1.t.sol} (95%) diff --git a/packages/contracts-rfq/contracts/zaps/TokenZap.sol b/packages/contracts-rfq/contracts/zaps/TokenZapV1.sol similarity index 94% rename from packages/contracts-rfq/contracts/zaps/TokenZap.sol rename to packages/contracts-rfq/contracts/zaps/TokenZapV1.sol index fe4ecd4e7f..0e843d1dfd 100644 --- a/packages/contracts-rfq/contracts/zaps/TokenZap.sol +++ b/packages/contracts-rfq/contracts/zaps/TokenZapV1.sol @@ -7,14 +7,14 @@ import {ZapDataV1} from "../libs/ZapDataV1.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {SafeERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -contract TokenZap is IZapRecipient { +contract TokenZapV1 is IZapRecipient { using SafeERC20 for IERC20; using ZapDataV1 for bytes; address public constant NATIVE_GAS_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; - error TokenZap__AmountIncorrect(); - error TokenZap__PayloadLengthAboveMax(); + error TokenZapV1__AmountIncorrect(); + error TokenZapV1__PayloadLengthAboveMax(); /// @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). @@ -29,11 +29,11 @@ contract TokenZap is IZapRecipient { // Check that the ZapData is valid before decoding it zapData.validateV1(); address target = zapData.target(); - // Approve the target contract to spend the token. TokenZap does not custody any tokens outside of the + // Approve the target contract to spend the token. TokenZapV1 does not custody any tokens outside of the // zap action, so we can approve the arbitrary target contract. if (token == NATIVE_GAS_TOKEN) { // No approvals are needed for the native gas token, just check that the amount is correct - if (msg.value != amount) revert TokenZap__AmountIncorrect(); + if (msg.value != amount) revert TokenZapV1__AmountIncorrect(); } else { // Issue the approval only if the current allowance is less than the required amount if (IERC20(token).allowance(address(this), target) < amount) { @@ -72,7 +72,7 @@ contract TokenZap is IZapRecipient { returns (bytes memory) { if (payload.length > ZapDataV1.AMOUNT_NOT_PRESENT) { - revert TokenZap__PayloadLengthAboveMax(); + revert TokenZapV1__PayloadLengthAboveMax(); } if (amountPosition >= payload.length) { amountPosition = ZapDataV1.AMOUNT_NOT_PRESENT; diff --git a/packages/contracts-rfq/test/zaps/TokenZap.t.sol b/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol similarity index 95% rename from packages/contracts-rfq/test/zaps/TokenZap.t.sol rename to packages/contracts-rfq/test/zaps/TokenZapV1.t.sol index 35b1fb3e43..87351f0f82 100644 --- a/packages/contracts-rfq/test/zaps/TokenZap.t.sol +++ b/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol @@ -1,17 +1,17 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.24; -import {TokenZap} from "../../contracts/zaps/TokenZap.sol"; +import {TokenZapV1} from "../../contracts/zaps/TokenZapV1.sol"; import {VaultManyArguments} from "../mocks/VaultManyArguments.sol"; import {MockERC20} from "../MockERC20.sol"; import {Test} from "forge-std/Test.sol"; // solhint-disable func-name-mixedcase, ordering -contract TokenZapTest is Test { +contract TokenZapV1Test is Test { uint256 internal constant AMOUNT = 0.987 ether; - TokenZap internal tokenZap; + TokenZapV1 internal tokenZap; VaultManyArguments internal vault; MockERC20 internal erc20; @@ -19,7 +19,7 @@ contract TokenZapTest is Test { address internal nativeGasToken = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; function setUp() public { - tokenZap = new TokenZap(); + tokenZap = new TokenZapV1(); vault = new VaultManyArguments(); erc20 = new MockERC20("TKN", 18); @@ -192,7 +192,7 @@ contract TokenZapTest is Test { bytes memory originalPayload = getVaultPayload(nativeGasToken, 0); bytes memory zapData = getZapData(originalPayload); - vm.expectRevert(TokenZap.TokenZap__AmountIncorrect.selector); + vm.expectRevert(TokenZapV1.TokenZapV1__AmountIncorrect.selector); tokenZap.zap{value: 1 ether - 1 wei}(nativeGasToken, 1 ether, zapData); } @@ -200,13 +200,13 @@ contract TokenZapTest is Test { bytes memory originalPayload = getVaultPayload(nativeGasToken, 0); bytes memory zapData = getZapData(originalPayload); - vm.expectRevert(TokenZap.TokenZap__AmountIncorrect.selector); + vm.expectRevert(TokenZapV1.TokenZapV1__AmountIncorrect.selector); tokenZap.zap{value: 1 ether + 1 wei}(nativeGasToken, 1 ether, zapData); } function test_encodeZapData_revert_payloadLengthAboveMax() public { bytes memory tooLongPayload = new bytes(2 ** 16); - vm.expectRevert(TokenZap.TokenZap__PayloadLengthAboveMax.selector); + vm.expectRevert(TokenZapV1.TokenZapV1__PayloadLengthAboveMax.selector); tokenZap.encodeZapData(address(vault), tooLongPayload, 0); } } From ae4fe604e4fedb5a46f40485289a842074810428 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:34:46 +0000 Subject: [PATCH 13/17] test: FastBridgeV2 + TokenZapV1 integration --- .../FastBridgeV2.TokenZapV1.Dst.t.sol | 119 +++++++++++++ .../FastBridgeV2.TokenZapV1.Src.t.sol | 111 ++++++++++++ .../test/integration/TokenZapV1.t.sol | 160 ++++++++++++++++++ .../test/mocks/VaultManyArguments.sol | 2 +- .../contracts-rfq/test/zaps/TokenZapV1.t.sol | 2 +- 5 files changed, 392 insertions(+), 2 deletions(-) create mode 100644 packages/contracts-rfq/test/integration/FastBridgeV2.TokenZapV1.Dst.t.sol create mode 100644 packages/contracts-rfq/test/integration/FastBridgeV2.TokenZapV1.Src.t.sol create mode 100644 packages/contracts-rfq/test/integration/TokenZapV1.t.sol diff --git a/packages/contracts-rfq/test/integration/FastBridgeV2.TokenZapV1.Dst.t.sol b/packages/contracts-rfq/test/integration/FastBridgeV2.TokenZapV1.Dst.t.sol new file mode 100644 index 0000000000..9a32c1a259 --- /dev/null +++ b/packages/contracts-rfq/test/integration/FastBridgeV2.TokenZapV1.Dst.t.sol @@ -0,0 +1,119 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {TokenZapV1IntegrationTest, VaultManyArguments, IFastBridge, IFastBridgeV2} from "./TokenZapV1.t.sol"; + +// solhint-disable func-name-mixedcase, ordering +contract FastBridgeV2TokenZapV1DstTest is TokenZapV1IntegrationTest { + event BridgeRelayed( + bytes32 indexed transactionId, + address indexed relayer, + address indexed to, + uint32 originChainId, + address originToken, + address destToken, + uint256 originAmount, + uint256 destAmount, + uint256 chainGasAmount + ); + + function setUp() public virtual override { + vm.chainId(DST_CHAIN_ID); + super.setUp(); + } + + function mintTokens() public virtual override { + deal(relayer, DST_AMOUNT); + dstToken.mint(relayer, DST_AMOUNT); + vm.prank(relayer); + dstToken.approve(address(fastBridge), type(uint256).max); + } + + function relay( + IFastBridge.BridgeParams memory params, + IFastBridgeV2.BridgeParamsV2 memory paramsV2, + bool isToken + ) + public + { + bytes memory encodedBridgeTx = encodeBridgeTx(params, paramsV2); + vm.prank({msgSender: relayer, txOrigin: relayer}); + fastBridge.relay{value: isToken ? paramsV2.zapNative : DST_AMOUNT}(encodedBridgeTx); + } + + function expectEventBridgeRelayed( + IFastBridge.BridgeParams memory params, + IFastBridgeV2.BridgeParamsV2 memory paramsV2, + bool isToken + ) + public + { + bytes32 txId = keccak256(encodeBridgeTx(params, paramsV2)); + vm.expectEmit(address(fastBridge)); + emit BridgeRelayed({ + transactionId: txId, + relayer: relayer, + to: address(dstZap), + originChainId: SRC_CHAIN_ID, + originToken: isToken ? address(srcToken) : NATIVE_GAS_TOKEN, + destToken: isToken ? address(dstToken) : NATIVE_GAS_TOKEN, + originAmount: SRC_AMOUNT, + destAmount: DST_AMOUNT, + chainGasAmount: paramsV2.zapNative + }); + } + + function checkBalances(bool isToken) public view { + if (isToken) { + assertEq(dstToken.balanceOf(user), 0); + assertEq(dstToken.balanceOf(relayer), 0); + assertEq(dstToken.balanceOf(address(fastBridge)), 0); + assertEq(dstToken.balanceOf(address(dstZap)), 0); + assertEq(dstToken.balanceOf(address(dstVault)), DST_AMOUNT); + assertEq(dstVault.balanceOf(user, address(dstToken)), DST_AMOUNT); + } else { + assertEq(address(user).balance, 0); + assertEq(address(relayer).balance, 0); + assertEq(address(fastBridge).balance, 0); + assertEq(address(dstZap).balance, 0); + assertEq(address(dstVault).balance, DST_AMOUNT); + assertEq(dstVault.balanceOf(user, NATIVE_GAS_TOKEN), DST_AMOUNT); + } + } + + function test_relay_depositTokenParams() public { + expectEventBridgeRelayed({params: tokenParams, paramsV2: depositTokenParams, isToken: true}); + relay({params: tokenParams, paramsV2: depositTokenParams, isToken: true}); + checkBalances({isToken: true}); + } + + function test_relay_depositTokenWithZapNativeParams() public { + expectEventBridgeRelayed({params: tokenParams, paramsV2: depositTokenWithZapNativeParams, isToken: true}); + relay({params: tokenParams, paramsV2: depositTokenWithZapNativeParams, isToken: true}); + checkBalances({isToken: true}); + // Extra ETH will be also custodied by the Vault + assertEq(address(dstVault).balance, ZAP_NATIVE); + } + + function test_relay_depositTokenRevertParams_revert() public { + vm.expectRevert(VaultManyArguments.VaultManyArguments__SomeError.selector); + relay({params: tokenParams, paramsV2: depositTokenRevertParams, isToken: true}); + } + + function test_relay_depositNativeParams() public { + expectEventBridgeRelayed({params: nativeParams, paramsV2: depositNativeParams, isToken: false}); + relay({params: nativeParams, paramsV2: depositNativeParams, isToken: false}); + checkBalances({isToken: false}); + } + + function test_relay_depositNativeNoAmountParams() public { + expectEventBridgeRelayed({params: nativeParams, paramsV2: depositNativeNoAmountParams, isToken: false}); + relay({params: nativeParams, paramsV2: depositNativeNoAmountParams, isToken: false}); + checkBalances({isToken: false}); + } + + function test_relay_depositNativeRevertParams_revert() public { + vm.expectRevert(VaultManyArguments.VaultManyArguments__SomeError.selector); + relay({params: nativeParams, paramsV2: depositNativeRevertParams, isToken: false}); + } +} diff --git a/packages/contracts-rfq/test/integration/FastBridgeV2.TokenZapV1.Src.t.sol b/packages/contracts-rfq/test/integration/FastBridgeV2.TokenZapV1.Src.t.sol new file mode 100644 index 0000000000..40541c6056 --- /dev/null +++ b/packages/contracts-rfq/test/integration/FastBridgeV2.TokenZapV1.Src.t.sol @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {TokenZapV1IntegrationTest, IFastBridge, IFastBridgeV2} from "./TokenZapV1.t.sol"; + +// solhint-disable func-name-mixedcase, ordering +contract FastBridgeV2TokenZapV1SrcTest is TokenZapV1IntegrationTest { + event BridgeRequested( + bytes32 indexed transactionId, + address indexed sender, + bytes request, + uint32 destChainId, + address originToken, + address destToken, + uint256 originAmount, + uint256 destAmount, + bool sendChainGas + ); + + function setUp() public virtual override { + vm.chainId(SRC_CHAIN_ID); + super.setUp(); + } + + function mintTokens() public virtual override { + deal(user, SRC_AMOUNT); + srcToken.mint(user, SRC_AMOUNT); + vm.prank(user); + srcToken.approve(address(fastBridge), type(uint256).max); + } + + function bridge( + IFastBridge.BridgeParams memory params, + IFastBridgeV2.BridgeParamsV2 memory paramsV2, + bool isToken + ) + public + { + vm.prank({msgSender: user, txOrigin: user}); + fastBridge.bridge{value: isToken ? 0 : SRC_AMOUNT}(params, paramsV2); + } + + function expectEventBridgeRequested( + IFastBridge.BridgeParams memory params, + IFastBridgeV2.BridgeParamsV2 memory paramsV2, + bool isToken + ) + public + { + bytes memory encodedBridgeTx = encodeBridgeTx(params, paramsV2); + bytes32 txId = keccak256(encodedBridgeTx); + vm.expectEmit(address(fastBridge)); + emit BridgeRequested({ + transactionId: txId, + sender: user, + request: encodedBridgeTx, + destChainId: DST_CHAIN_ID, + originToken: isToken ? address(srcToken) : NATIVE_GAS_TOKEN, + destToken: isToken ? address(dstToken) : NATIVE_GAS_TOKEN, + originAmount: SRC_AMOUNT, + destAmount: DST_AMOUNT, + sendChainGas: paramsV2.zapNative > 0 + }); + } + + function checkBalances(bool isToken) public view { + if (isToken) { + assertEq(srcToken.balanceOf(user), 0); + assertEq(srcToken.balanceOf(address(fastBridge)), SRC_AMOUNT); + } else { + assertEq(address(user).balance, 0); + assertEq(address(fastBridge).balance, SRC_AMOUNT); + } + } + + function test_bridge_depositTokenParams() public { + expectEventBridgeRequested({params: tokenParams, paramsV2: depositTokenParams, isToken: true}); + bridge({params: tokenParams, paramsV2: depositTokenParams, isToken: true}); + checkBalances({isToken: true}); + } + + function test_bridge_depositTokenWithZapNativeParams() public { + expectEventBridgeRequested({params: tokenParams, paramsV2: depositTokenWithZapNativeParams, isToken: true}); + bridge({params: tokenParams, paramsV2: depositTokenWithZapNativeParams, isToken: true}); + checkBalances({isToken: true}); + } + + function test_bridge_depositTokenRevertParams() public { + expectEventBridgeRequested({params: tokenParams, paramsV2: depositTokenRevertParams, isToken: true}); + bridge({params: tokenParams, paramsV2: depositTokenRevertParams, isToken: true}); + checkBalances({isToken: true}); + } + + function test_bridge_depositNativeParams() public { + expectEventBridgeRequested({params: nativeParams, paramsV2: depositNativeParams, isToken: false}); + bridge({params: nativeParams, paramsV2: depositNativeParams, isToken: false}); + checkBalances({isToken: false}); + } + + function test_bridge_depositNativeNoAmountParams() public { + expectEventBridgeRequested({params: nativeParams, paramsV2: depositNativeNoAmountParams, isToken: false}); + bridge({params: nativeParams, paramsV2: depositNativeNoAmountParams, isToken: false}); + checkBalances({isToken: false}); + } + + function test_bridge_depositNativeRevertParams() public { + expectEventBridgeRequested({params: nativeParams, paramsV2: depositNativeRevertParams, isToken: false}); + bridge({params: nativeParams, paramsV2: depositNativeRevertParams, isToken: false}); + checkBalances({isToken: false}); + } +} diff --git a/packages/contracts-rfq/test/integration/TokenZapV1.t.sol b/packages/contracts-rfq/test/integration/TokenZapV1.t.sol new file mode 100644 index 0000000000..d3abcd41df --- /dev/null +++ b/packages/contracts-rfq/test/integration/TokenZapV1.t.sol @@ -0,0 +1,160 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {FastBridgeV2, IFastBridge, IFastBridgeV2} from "../../contracts/FastBridgeV2.sol"; +import {BridgeTransactionV2Lib} from "../../contracts/libs/BridgeTransactionV2.sol"; +import {ZapDataV1} from "../../contracts/libs/ZapDataV1.sol"; +import {TokenZapV1} from "../../contracts/zaps/TokenZapV1.sol"; + +import {VaultManyArguments} from "../mocks/VaultManyArguments.sol"; +import {MockERC20} from "../MockERC20.sol"; + +import {Test} from "forge-std/Test.sol"; + +// solhint-disable ordering +abstract contract TokenZapV1IntegrationTest is Test { + address internal constant NATIVE_GAS_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; + + uint32 internal constant SRC_CHAIN_ID = 1337; + uint32 internal constant DST_CHAIN_ID = 7331; + + uint256 internal constant SRC_AMOUNT = 1 ether; + uint256 internal constant DST_AMOUNT = 0.9999 ether; + uint256 internal constant ZAP_NATIVE = 123_456; + + FastBridgeV2 internal fastBridge; + TokenZapV1 internal dstZap; + + address internal user = makeAddr("User"); + address internal relayer = makeAddr("Relayer"); + + MockERC20 internal srcToken; + MockERC20 internal dstToken; + + VaultManyArguments internal dstVault; + + IFastBridge.BridgeParams internal tokenParams; + IFastBridge.BridgeParams internal nativeParams; + + IFastBridgeV2.BridgeParamsV2 internal depositTokenParams; + IFastBridgeV2.BridgeParamsV2 internal depositTokenWithZapNativeParams; + IFastBridgeV2.BridgeParamsV2 internal depositTokenRevertParams; + IFastBridgeV2.BridgeParamsV2 internal depositNativeParams; + IFastBridgeV2.BridgeParamsV2 internal depositNativeNoAmountParams; + IFastBridgeV2.BridgeParamsV2 internal depositNativeRevertParams; + + function setUp() public virtual { + fastBridge = new FastBridgeV2(address(this)); + fastBridge.grantRole(fastBridge.RELAYER_ROLE(), relayer); + + srcToken = new MockERC20("SRC", 18); + dstToken = new MockERC20("DST", 18); + + dstZap = new TokenZapV1(); + dstVault = new VaultManyArguments(); + + createFixtures(); + mintTokens(); + } + + function createFixtures() public virtual { + tokenParams = IFastBridge.BridgeParams({ + dstChainId: DST_CHAIN_ID, + sender: user, + to: address(dstZap), + originToken: address(srcToken), + destToken: address(dstToken), + originAmount: SRC_AMOUNT, + destAmount: DST_AMOUNT, + sendChainGas: false, + deadline: block.timestamp + 1 days + }); + nativeParams = IFastBridge.BridgeParams({ + dstChainId: DST_CHAIN_ID, + sender: user, + to: address(dstZap), + originToken: NATIVE_GAS_TOKEN, + destToken: NATIVE_GAS_TOKEN, + originAmount: SRC_AMOUNT, + destAmount: DST_AMOUNT, + sendChainGas: false, + deadline: block.timestamp + 1 days + }); + // Deposit token + bytes memory zapData = dstZap.encodeZapData({ + target: address(dstVault), + payload: getDepositPayload(address(dstToken)), + amountPosition: 4 + 32 * 2 + }); + depositTokenParams.zapData = zapData; + depositTokenWithZapNativeParams.zapData = zapData; + depositTokenWithZapNativeParams.zapNative = ZAP_NATIVE; + // Deposit native + depositNativeParams.zapData = dstZap.encodeZapData({ + target: address(dstVault), + payload: getDepositPayload(NATIVE_GAS_TOKEN), + amountPosition: 4 + 32 * 2 + }); + // Deposit no amount + depositNativeNoAmountParams.zapData = dstZap.encodeZapData({ + target: address(dstVault), + payload: getDepositNoAmountPayload(), + amountPosition: ZapDataV1.AMOUNT_NOT_PRESENT + }); + // Deposit revert + depositTokenRevertParams.zapData = dstZap.encodeZapData({ + target: address(dstVault), + payload: getDepositRevertPayload(), + amountPosition: ZapDataV1.AMOUNT_NOT_PRESENT + }); + depositNativeRevertParams.zapData = dstZap.encodeZapData({ + target: address(dstVault), + payload: getDepositRevertPayload(), + amountPosition: ZapDataV1.AMOUNT_NOT_PRESENT + }); + } + + function mintTokens() public virtual; + + function encodeBridgeTx( + IFastBridge.BridgeParams memory params, + IFastBridgeV2.BridgeParamsV2 memory paramsV2 + ) + public + pure + returns (bytes memory) + { + IFastBridgeV2.BridgeTransactionV2 memory bridgeTx = IFastBridgeV2.BridgeTransactionV2({ + originChainId: SRC_CHAIN_ID, + destChainId: params.dstChainId, + originSender: params.sender, + destRecipient: params.to, + originToken: params.originToken, + destToken: params.destToken, + originAmount: params.originAmount, + destAmount: params.destAmount, + // No protocol fees for the test + originFeeAmount: 0, + deadline: params.deadline, + // Single tx is sent, so nonce is 0 + nonce: 0, + exclusivityRelayer: address(0), + exclusivityEndTime: 0, + zapNative: paramsV2.zapNative, + zapData: paramsV2.zapData + }); + return BridgeTransactionV2Lib.encodeV2(bridgeTx); + } + + function getDepositPayload(address token) public view returns (bytes memory) { + return abi.encodeCall(dstVault.deposit, (token, abi.encode(token), DST_AMOUNT, user, abi.encode(user))); + } + + function getDepositNoAmountPayload() public view returns (bytes memory) { + return abi.encodeCall(dstVault.depositNoAmount, (user)); + } + + function getDepositRevertPayload() public view returns (bytes memory) { + return abi.encodeCall(dstVault.depositWithRevert, ()); + } +} diff --git a/packages/contracts-rfq/test/mocks/VaultManyArguments.sol b/packages/contracts-rfq/test/mocks/VaultManyArguments.sol index 1d3c415aef..7e43817bd1 100644 --- a/packages/contracts-rfq/test/mocks/VaultManyArguments.sol +++ b/packages/contracts-rfq/test/mocks/VaultManyArguments.sol @@ -23,7 +23,7 @@ contract VaultManyArguments is VaultMock { _deposit(user, token, amount); } - function depositNative(address user) external payable { + function depositNoAmount(address user) external payable { _deposit(user, NATIVE_GAS_TOKEN, msg.value); } diff --git a/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol b/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol index 87351f0f82..c2f1cc142f 100644 --- a/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol +++ b/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol @@ -34,7 +34,7 @@ contract TokenZapV1Test is Test { } function getVaultPayloadNoAmount() public view returns (bytes memory) { - return abi.encodeCall(vault.depositNative, (user)); + return abi.encodeCall(vault.depositNoAmount, (user)); } function getVaultPayloadWithRevert() public view returns (bytes memory) { From 1ec3df8a049b34077096013ce4bf9cc7dfbe3618 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 30 Oct 2024 14:04:25 +0000 Subject: [PATCH 14/17] fix: should revert when zero target in encoding --- packages/contracts-rfq/contracts/libs/ZapDataV1.sol | 2 ++ packages/contracts-rfq/test/libs/ZapDataV1.t.sol | 5 +++++ packages/contracts-rfq/test/zaps/TokenZapV1.t.sol | 9 +++++++++ 3 files changed, 16 insertions(+) diff --git a/packages/contracts-rfq/contracts/libs/ZapDataV1.sol b/packages/contracts-rfq/contracts/libs/ZapDataV1.sol index c82f01dd8e..c13113ea1b 100644 --- a/packages/contracts-rfq/contracts/libs/ZapDataV1.sol +++ b/packages/contracts-rfq/contracts/libs/ZapDataV1.sol @@ -22,6 +22,7 @@ library ZapDataV1 { // forgefmt: disable-end error ZapDataV1__InvalidEncoding(); + error ZapDataV1__TargetZeroAddress(); error ZapDataV1__UnsupportedVersion(uint16 version); /// @notice Validates the encodedZapData to be a tightly packed encoded payload for ZapData struct. @@ -57,6 +58,7 @@ library ZapDataV1 { pure returns (bytes memory encodedZapData) { + if (target_ == address(0)) revert ZapDataV1__TargetZeroAddress(); // Amount is encoded in [amountPosition_ .. amountPosition_ + 32), which should be within the payload. if (amountPosition_ != AMOUNT_NOT_PRESENT && (uint256(amountPosition_) + 32 > payload_.length)) { revert ZapDataV1__InvalidEncoding(); diff --git a/packages/contracts-rfq/test/libs/ZapDataV1.t.sol b/packages/contracts-rfq/test/libs/ZapDataV1.t.sol index 19e5b378fd..209b8bf58b 100644 --- a/packages/contracts-rfq/test/libs/ZapDataV1.t.sol +++ b/packages/contracts-rfq/test/libs/ZapDataV1.t.sol @@ -69,6 +69,11 @@ contract ZapDataV1Test is Test { assertEq(zapData, encodeZapData(EXPECTED_VERSION, amountPosition, target, payload)); } + function test_encodeV1_revert_targetZeroAddress() public { + vm.expectRevert(ZapDataV1.ZapDataV1__TargetZeroAddress.selector); + harness.encodeV1(type(uint16).max, address(0), ""); + } + function test_encodeDecodeV1_revert_invalidAmountPosition( address target, uint16 amountPosition, diff --git a/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol b/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol index c2f1cc142f..7587131d3c 100644 --- a/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol +++ b/packages/contracts-rfq/test/zaps/TokenZapV1.t.sol @@ -1,7 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.24; +import {ZapDataV1} from "../../contracts/libs/ZapDataV1.sol"; import {TokenZapV1} from "../../contracts/zaps/TokenZapV1.sol"; + import {VaultManyArguments} from "../mocks/VaultManyArguments.sol"; import {MockERC20} from "../MockERC20.sol"; @@ -209,4 +211,11 @@ contract TokenZapV1Test is Test { vm.expectRevert(TokenZapV1.TokenZapV1__PayloadLengthAboveMax.selector); tokenZap.encodeZapData(address(vault), tooLongPayload, 0); } + + function test_encodeZapData_revert_targetZeroAddress() public { + bytes memory payload = getVaultPayloadNoAmount(); + + vm.expectRevert(ZapDataV1.ZapDataV1__TargetZeroAddress.selector); + tokenZap.encodeZapData(address(0), payload, payload.length); + } } From cbc4fbda5bb3e45c93f6bb54cf37eb576ddc7b4e Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 30 Oct 2024 17:09:32 +0000 Subject: [PATCH 15/17] chore: docs --- .../contracts/zaps/TokenZapV1.sol | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/contracts-rfq/contracts/zaps/TokenZapV1.sol b/packages/contracts-rfq/contracts/zaps/TokenZapV1.sol index 0e843d1dfd..e5795c3549 100644 --- a/packages/contracts-rfq/contracts/zaps/TokenZapV1.sol +++ b/packages/contracts-rfq/contracts/zaps/TokenZapV1.sol @@ -7,6 +7,14 @@ import {ZapDataV1} from "../libs/ZapDataV1.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {SafeERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +/// @notice TokenZapV1 enables atomic token operations called "Zaps". A Zap executes a predefined action with tokens +/// on behalf of a user, such as depositing into a vault or performing a token swap. +/// The contract supports both ERC20 tokens and the chain's native gas token (e.g. ETH). +/// @dev For security and efficiency, TokenZapV1 requires tokens to be pre-transferred to the contract before executing +/// a Zap, rather than using transferFrom. For native tokens, the amount must be sent as msg.value. +/// IMPORTANT: This contract is stateless and does not custody assets between Zaps. Any tokens left in the contract +/// can be claimed by anyone. To prevent loss of funds, ensure that Zaps either consume the entire token amount +/// or revert the transaction. contract TokenZapV1 is IZapRecipient { using SafeERC20 for IERC20; using ZapDataV1 for bytes; @@ -26,24 +34,28 @@ contract TokenZapV1 is IZapRecipient { /// @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) { - // Check that the ZapData is valid before decoding it + // Validate the ZapData format and extract target address zapData.validateV1(); address target = zapData.target(); - // Approve the target contract to spend the token. TokenZapV1 does not custody any tokens outside of the - // zap action, so we can approve the arbitrary target contract. if (token == NATIVE_GAS_TOKEN) { - // No approvals are needed for the native gas token, just check that the amount is correct + // For native gas token (e.g. ETH), verify msg.value matches expected amount. + // No approval needed since native token doesn't use allowances. if (msg.value != amount) revert TokenZapV1__AmountIncorrect(); } else { - // Issue the approval only if the current allowance is less than the required amount + // For ERC20 tokens, grant unlimited approval to target if current allowance insufficient. + // This is safe since contract doesn't custody tokens between zaps. if (IERC20(token).allowance(address(this), target) < amount) { IERC20(token).forceApprove(target, type(uint256).max); } + // Note: Balance check omitted as target contract will revert if insufficient funds } - // Perform the Zap action, forwarding full msg.value to the target contract - // Note: this will bubble up any revert from the target contract + // 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}); + // Return function selector to indicate successful execution return this.zap.selector; } @@ -74,10 +86,13 @@ contract TokenZapV1 is IZapRecipient { if (payload.length > ZapDataV1.AMOUNT_NOT_PRESENT) { revert TokenZapV1__PayloadLengthAboveMax(); } + // External integrations do not need to understand the specific `AMOUNT_NOT_PRESENT` semantics. + // Therefore, they can specify any value greater than or equal to `payload.length` to indicate + // that the amount is not present in the payload. if (amountPosition >= payload.length) { amountPosition = ZapDataV1.AMOUNT_NOT_PRESENT; } - // At this point we checked that both amountPosition and payload.length fit in uint16 + // At this point we checked that both `amountPosition` and `payload.length` fit in uint16. return ZapDataV1.encodeV1(uint16(amountPosition), target, payload); } From ad8d47995860d9826630326e8c357d5b4f8c9c01 Mon Sep 17 00:00:00 2001 From: parodime Date: Mon, 4 Nov 2024 09:45:06 -0500 Subject: [PATCH 16/17] added target != addr 0 assumptions --- packages/contracts-rfq/test/libs/ZapDataV1.t.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/contracts-rfq/test/libs/ZapDataV1.t.sol b/packages/contracts-rfq/test/libs/ZapDataV1.t.sol index 209b8bf58b..31bcee2149 100644 --- a/packages/contracts-rfq/test/libs/ZapDataV1.t.sol +++ b/packages/contracts-rfq/test/libs/ZapDataV1.t.sol @@ -38,6 +38,7 @@ contract ZapDataV1Test is Test { view { vm.assume(prefix.length + 32 + postfix.length < type(uint16).max); + vm.assume(target != address(0)); // We don't know the amount at the time of encoding, so we provide a placeholder. uint16 amountPosition = uint16(prefix.length); @@ -57,6 +58,7 @@ contract ZapDataV1Test is Test { function test_roundtrip_noAmount(address target, uint256 amount, bytes memory payload) public view { vm.assume(payload.length < type(uint16).max); + vm.assume(target != address(0)); uint16 amountPosition = type(uint16).max; bytes memory zapData = harness.encodeV1(amountPosition, target, payload); From 2e5f7d1686543222e1d57c21ca70eadc28fc87fd Mon Sep 17 00:00:00 2001 From: parodime Date: Mon, 4 Nov 2024 09:46:25 -0500 Subject: [PATCH 17/17] added one more target != addr 0 assumption --- packages/contracts-rfq/test/libs/ZapDataV1.t.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/contracts-rfq/test/libs/ZapDataV1.t.sol b/packages/contracts-rfq/test/libs/ZapDataV1.t.sol index 31bcee2149..d93d9775bd 100644 --- a/packages/contracts-rfq/test/libs/ZapDataV1.t.sol +++ b/packages/contracts-rfq/test/libs/ZapDataV1.t.sol @@ -85,6 +85,7 @@ contract ZapDataV1Test is Test { public { vm.assume(payload.length < type(uint16).max); + vm.assume(target != address(0)); // Make sure that (amountPosition + 32) is outside the bounds of the payload. uint16 incorrectMin = payload.length > 31 ? uint16(payload.length) - 31 : 0; uint16 incorrectMax = type(uint16).max - 1;