Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(contracts-rfq): zap router #3431

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions packages/contracts-rfq/contracts/interfaces/IZapRouterV1.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

interface IZapRouterV1 {
struct ZapParams {
address token;
uint256 amount;
uint256 msgValue;
bytes zapData;
}

/// @notice Perform a series of Zap actions in a single transaction. Verifies that the ZapRecipient balance
/// for every token in `zapParams` has not increased after the last Zap.
/// - Each step is verified to be a correct Zap as per `IZapRecipient` specification.
/// - The amounts used for each Zap can be predetermined or based on the proceeds from the previous Zaps.
/// - ZapRouter does not perform any checks on the Zap Data, nor the ZapRecipient balance after the Zaps are
/// performed.
Comment on lines +16 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Clarify documentation regarding balance checks

In the performZapsWithBalanceChecks function, the documentation states that "ZapRouter does not perform any checks on the Zap Data, nor the ZapRecipient balance after the Zaps are performed." However, this function is intended to verify that the ZapRecipient balance has not increased after the last Zap. Please update the documentation to accurately reflect the functionality.

/// - The user is responsible for selecting a correct ZapRecipient and for the correct encoding of the Zap Data.
/// - ZapRecipient must be able to modify the Zap Data to adjust to possible changes in the passed amount value.
/// @dev Typical workflow involves a series of "prep" Zaps followed by a final Zap, such as
/// bridging, depositing, or a transfer to the final recipient. ZapRecipient must be set as the funds recipient
/// for the prep Zaps, while a different recipient must be set for the last Zap.
/// @dev This function will revert in any of the following cases:
/// - The deadline has passed.
/// - The array of ZapParams is empty.
/// - The amount of tokens to use for the last Zap is below the specified minimum.
/// - Any Zap fails.
/// @param zapRecipient Address of the IZapRecipient contract to use for the series of Zaps
/// @param amountIn Initial amount of tokens (zapParams[0].token) to transfer into ZapRecipient
/// @param minLastZapAmountIn Minimum amount of tokens (zapParams[N-1].token) to use for the last Zap
/// @param deadline Deadline for the series of Zaps
/// @param zapParams Parameters for each Zap. Use amount = type(uint256).max for Zaps that
/// should use the full ZapRecipient balance.
function performZapsWithBalanceChecks(
address zapRecipient,
uint256 amountIn,
uint256 minLastZapAmountIn,
uint256 deadline,
ZapParams[] memory zapParams
)
external
payable;

/// @notice Perform a series of Zap actions in a single transaction.
/// @dev This function is identical to `performZapsWithBalanceChecks` except that it does not verify that
/// the ZapRecipient balance for every token in `zapParams` has not increased after the last Zap.
/// Anyone using this function must validate that the funds are fully spent by ZapRecipient
/// using other means like separate on-chain checks or off-chain simulation.
function performZaps(
address zapRecipient,
uint256 amountIn,
uint256 minLastZapAmountIn,
uint256 deadline,
ZapParams[] memory zapParams
)
external
payable;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

interface IZapRouterV1Errors {
error ZapRouterV1__AmountInsufficient();
error ZapRouterV1__DeadlineExceeded();
error ZapRouterV1__MsgValueIncorrect();
error ZapRouterV1__NoZapsProvided();
error ZapRouterV1__TokenNotContract();
error ZapRouterV1__ZapIncorrectReturnValue();
error ZapRouterV1__ZapNoReturnValue();
error ZapRouterV1__ZapUnspentFunds();
}
139 changes: 139 additions & 0 deletions packages/contracts-rfq/contracts/router/ZapRouterV1.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;

// ════════════════════════════════════════════════ INTERFACES ═════════════════════════════════════════════════════

import {IZapRecipient} from "../interfaces/IZapRecipient.sol";
import {IZapRouterV1} from "../interfaces/IZapRouterV1.sol";
import {IZapRouterV1Errors} from "../interfaces/IZapRouterV1Errors.sol";

// ═════════════════════════════════════════════ EXTERNAL IMPORTS ══════════════════════════════════════════════════

import {IERC20, SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import {Address} from "@openzeppelin/contracts/utils/Address.sol";

contract ZapRouterV1 is IZapRouterV1, IZapRouterV1Errors {
using SafeERC20 for IERC20;

/// @notice The address reserved for the native gas token (ETH on Ethereum and most L2s, AVAX on Avalanche, etc.).
address public constant NATIVE_GAS_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;

/// @dev Zap Amount value that signals that the Zap should be performed using the full ZapRecipient balance.
uint256 internal constant FULL_BALANCE = type(uint256).max;

/// @inheritdoc IZapRouterV1
function performZapsWithBalanceChecks(
address zapRecipient,
uint256 amountIn,
uint256 minLastZapAmountIn,
uint256 deadline,
ZapParams[] calldata zapParams
)
public
payable
{
// Record the initial balances of ZapRecipient for each token.
uint256 length = zapParams.length;
uint256[] memory initialBalances = new uint256[](length);
for (uint256 i = 0; i < length; i++) {
address token = zapParams[i].token;
initialBalances[i] =
token == NATIVE_GAS_TOKEN ? zapRecipient.balance : IERC20(token).balanceOf(zapRecipient);
}

// Perform the Zaps as usual.
performZaps(zapRecipient, amountIn, minLastZapAmountIn, deadline, zapParams);

// Verify that the ZapRecipient balance for each token has not increased.
for (uint256 i = 0; i < length; i++) {
address token = zapParams[i].token;
uint256 newBalance =
token == NATIVE_GAS_TOKEN ? zapRecipient.balance : IERC20(token).balanceOf(zapRecipient);
if (newBalance > initialBalances[i]) revert ZapRouterV1__ZapUnspentFunds();
}
}
Comment on lines +25 to +54
Comment on lines +25 to +54

/// @inheritdoc IZapRouterV1
function performZaps(
address zapRecipient,
uint256 amountIn,
uint256 minLastZapAmountIn,
uint256 deadline,
ZapParams[] calldata zapParams
)
public
payable
{
// Validate the input parameters before proceeding.
uint256 length = zapParams.length;
if (block.timestamp > deadline) revert ZapRouterV1__DeadlineExceeded();
if (length == 0) revert ZapRouterV1__NoZapsProvided();

// Transfer the zapped asset from the user to ZapRecipient. `zapParams[0]` exists as per check above.
_transferZappedAsset(zapRecipient, zapParams[0].token, amountIn);

// Perform the Zaps, using predetermined amounts or the full balance of ZapRecipient, if instructed.
uint256 totalUsedMsgValue = 0;
for (uint256 i = 0; i < length; i++) {
address token = zapParams[i].token;
uint256 msgValue = zapParams[i].msgValue;

// Adjust amount to be the full balance, if needed.
amountIn = zapParams[i].amount;
if (amountIn == FULL_BALANCE) {
amountIn = token == NATIVE_GAS_TOKEN
// Existing native balance + msg.value that will be forwarded
? zapRecipient.balance + msgValue
: IERC20(token).balanceOf(zapRecipient);
}

_performZap({
zapRecipient: zapRecipient,
msgValue: msgValue,
zapRecipientCallData: abi.encodeCall(IZapRecipient.zap, (token, amountIn, zapParams[i].zapData))
});
unchecked {
// Can do unchecked addition here since we're guaranteed that the sum of all msg.value
// used for the Zaps won't overflow.
totalUsedMsgValue += msgValue;
}
}

// Verify amountIn used for the last Zap, and that we fully spent `msg.value`.
if (amountIn < minLastZapAmountIn) revert ZapRouterV1__AmountInsufficient();
if (totalUsedMsgValue < msg.value) revert ZapRouterV1__MsgValueIncorrect();
}
Comment on lines +24 to +105
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding reentrancy guards to public functions

The performZapsWithBalanceChecks and performZaps functions make external calls to the zapRecipient. To prevent potential reentrancy attacks, consider adding a nonReentrant modifier (e.g., from OpenZeppelin's ReentrancyGuard) to these functions.

Comment on lines +57 to +105

Check notice

Code scanning / Slither

Calls inside a loop Low

Comment on lines +57 to +105

Check notice

Code scanning / Slither

Block timestamp Low


// ═════════════════════════════════════════════ INTERNAL METHODS ══════════════════════════════════════════════════

/// @notice Transfers the zapped asset from the user into ZapRecipient custody. This asset will later be
/// used to perform the zap actions.
function _transferZappedAsset(address zapRecipient, address token, uint256 amount) internal {
if (token == NATIVE_GAS_TOKEN) {
// For the native gas token, we just need to check that the supplied msg.value is correct.
if (amount != msg.value) revert ZapRouterV1__MsgValueIncorrect();
} else {
// For ERC20s, token is explicitly transferred from the user to ZapRecipient.
// Throw an explicit error if the provided token address is not a contract.
if (token.code.length == 0) revert ZapRouterV1__TokenNotContract();
IERC20(token).safeTransferFrom(msg.sender, zapRecipient, amount);
}
}

/// @notice Performs a Zap action, using the provided msg.value and calldata.
/// Validates the return data from ZapRecipient as per `IZapRecipient` specification.
function _performZap(address zapRecipient, uint256 msgValue, bytes memory zapRecipientCallData) internal {
// Perform the low-level call to ZapRecipient, bubbling up any revert reason.
bytes memory returnData =
Address.functionCallWithValue({target: zapRecipient, data: zapRecipientCallData, value: msgValue});

// Explicit revert if no return data at all.
if (returnData.length == 0) revert ZapRouterV1__ZapNoReturnValue();
// Check that exactly a single return value was returned.
if (returnData.length != 32) revert ZapRouterV1__ZapIncorrectReturnValue();
// Return value should be abi-encoded hook function selector.
if (bytes32(returnData) != bytes32(IZapRecipient.zap.selector)) {
revert ZapRouterV1__ZapIncorrectReturnValue();

Check warning on line 136 in packages/contracts-rfq/contracts/router/ZapRouterV1.sol

View check run for this annotation

Codecov / codecov/patch

packages/contracts-rfq/contracts/router/ZapRouterV1.sol#L136

Added line #L136 was not covered by tests
}
}
Comment on lines +125 to +138

Check warning

Code scanning / Slither

Dangerous strict equalities Medium

}
44 changes: 44 additions & 0 deletions packages/contracts-rfq/test/mocks/PoolMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import {IERC20, SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

// solhint-disable no-empty-blocks
/// @notice Pool mock for testing purposes. DO NOT USE IN PRODUCTION.
contract PoolMock {
using SafeERC20 for IERC20;

address public immutable token0;
address public immutable token1;

uint256 public ratioWei = 1e18;

error PoolMock__TokenNotSupported();

constructor(address token0_, address token1_) {
token0 = token0_;
token1 = token1_;
}

/// @notice We include an empty "test" function so that this contract does not appear in the coverage report.
function testPoolMock() external {}

function setRatioWei(uint256 ratioWei_) external {
ratioWei = ratioWei_;
}

function swap(uint256 amountIn, address tokenIn) external returns (uint256 amountOut) {
address tokenOut;
if (tokenIn == token0) {
tokenOut = token1;
amountOut = amountIn * ratioWei / 1e18;
} else if (tokenIn == token1) {
tokenOut = token0;
amountOut = amountIn * 1e18 / ratioWei;
} else {
revert PoolMock__TokenNotSupported();
}
IERC20(tokenIn).safeTransferFrom(msg.sender, address(this), amountIn);
IERC20(tokenOut).safeTransfer(msg.sender, amountOut);
}
}
10 changes: 9 additions & 1 deletion packages/contracts-rfq/test/mocks/WETHMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ pragma solidity ^0.8.20;
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {Address} from "@openzeppelin/contracts/utils/Address.sol";

import {CommonBase} from "forge-std/Base.sol";

// solhint-disable no-empty-blocks
/// @notice WETH mock for testing purposes. DO NOT USE IN PRODUCTION.
contract WETHMock is ERC20 {
contract WETHMock is ERC20, CommonBase {
constructor() ERC20("Mock Wrapped Ether", "Mock WETH") {}

receive() external payable {
Expand All @@ -16,6 +18,12 @@ contract WETHMock is ERC20 {
/// @notice We include an empty "test" function so that this contract does not appear in the coverage report.
function testWETHMock() external {}

function mint(address to, uint256 amount) external {
uint256 newBalance = address(this).balance + amount;
vm.deal(address(this), newBalance);
_mint(to, amount);
}

function withdraw(uint256 amount) external {
_burn(msg.sender, amount);
Address.sendValue(payable(msg.sender), amount);
Expand Down
Loading
Loading