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.

Sign up for GitHub

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

fix(contracts-rfq): add forwardTo to ZapData for Zap Actions that don't forward assets to the user #3451

Merged
merged 12 commits into from
Dec 10, 2024
Merged

fix(contracts-rfq): add forwardTo to ZapData for Zap Actions that don't forward assets to the user #3451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5eaccd1
feat: scaffold finalToken, forwardTo in ZapDataV1
ChiTimesChi Dec 8, 2024
535b4e7
feat: add finalToken, forwardTo
ChiTimesChi Dec 8, 2024
1a37090
feat: scaffold TokenZap.encodeZapData
ChiTimesChi Dec 7, 2024
52ba760
test: update existing tests
ChiTimesChi Dec 7, 2024
08a5883
test: forwardTo scenarios
ChiTimesChi Dec 7, 2024
6d7cc51
feat: token zeor address checks
ChiTimesChi Dec 7, 2024
26ed1e7
feat:scaffold token forwarding
ChiTimesChi Dec 7, 2024
0c7ca8b
test: more revert cases
ChiTimesChi Dec 7, 2024
812a106
feat: final token forwarding
ChiTimesChi Dec 7, 2024
0821a51
test: forwardTo behaviour in SIP
ChiTimesChi Dec 8, 2024
2bda040
feat: support optional `forwardTo` in SIP
ChiTimesChi Dec 8, 2024
4b925f9
deploy: redeploy SIP, TokenZap
ChiTimesChi Dec 8, 2024
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
3 changes: 3 additions & 0 deletions packages/contracts-rfq/contracts/interfaces/ISynapseIntentPreviewer.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ interface ISynapseIntentPreviewer {
/// @dev Will not revert if the intent cannot be completed, returns empty values instead.
/// @dev Returns (amountIn, []) if the intent is a no-op (tokenIn == tokenOut).
/// @param swapQuoter Peripheral contract to use for swap quoting
/// @param forwardTo The address to which the proceeds of the intent should be forwarded to.
/// Note: if no forwarding is required (or done within the intent), use address(0).
/// @param tokenIn Initial token for the intent
/// @param tokenOut Final token for the intent
/// @param amountIn Initial amount of tokens to use for the intent
Expand All @@ -16,6 +18,7 @@ interface ISynapseIntentPreviewer {
/// Empty if the intent cannot be completed, or if intent is a no-op (tokenIn == tokenOut).
function previewIntent(
address swapQuoter,
address forwardTo,
address tokenIn,
address tokenOut,
uint256 amountIn
Expand Down
40 changes: 35 additions & 5 deletions packages/contracts-rfq/contracts/libs/ZapDataV1.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@
// Offsets of the fields in the packed ZapData struct
// uint16 version [000 .. 002)
// uint16 amountPosition [002 .. 004)
// address target [004 .. 024)
// bytes payload [024 .. ***)
// address finalToken [004 .. 024)
// address forwardTo [024 .. 044)
// address target [044 .. 064)
// bytes payload [064 .. ***)

// forgefmt: disable-start
uint256 private constant OFFSET_AMOUNT_POSITION = 2;
uint256 private constant OFFSET_TARGET = 4;
uint256 private constant OFFSET_PAYLOAD = 24;
uint256 private constant OFFSET_FINAL_TOKEN = 4;
uint256 private constant OFFSET_FORWARD_TO = 24;
uint256 private constant OFFSET_TARGET = 44;
uint256 private constant OFFSET_PAYLOAD = 64;
// forgefmt: disable-end

error ZapDataV1__InvalidEncoding();
Expand All @@ -44,13 +48,23 @@
/// 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 token amount is not encoded within `payload_`.
/// @param finalToken_ The token produced as a result of the Zap action (ERC20 or native gas token).
/// A zero address value signals that the Zap action doesn't result in any asset per se,
/// like bridging or depositing into a vault without an LP token.
/// Note: this parameter must be set to a non-zero value if the `forwardTo_` parameter is
/// set to a non-zero value.
/// @param forwardTo_ The address to which `finalToken` should be forwarded. This parameter is required only
/// if the Zap action does not automatically transfer the token to the intended recipient.
/// Otherwise, it must be set to address(0).
Comment on lines +51 to +58
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

Ensure finalToken_ and forwardTo_ parameters are validated appropriately

The addition of finalToken_ and forwardTo_ parameters enhances the flexibility of the encodeV1 function. However, there should be explicit validations to ensure that if forwardTo_ is not the zero address, then finalToken_ must also be a non-zero address, as mentioned in the documentation. Consider adding a require statement to enforce this constraint.

Apply this diff to include the validation:

function encodeV1(
    uint16 amountPosition_,
    address finalToken_,
    address forwardTo_,
    address target_,
    bytes memory payload_
)
    internal
    pure
    returns (bytes memory encodedZapData)
{
    if (target_ == address(0)) revert ZapDataV1__TargetZeroAddress();
+   if (forwardTo_ != address(0) && finalToken_ == address(0)) revert ZapDataV1__FinalTokenZeroAddress();
    // 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_, finalToken_, forwardTo_, target_, payload_);
}

This ensures that the function reverts if forwardTo_ is set without a corresponding finalToken_, preventing potential runtime errors.

Also applies to: 66-67

/// @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.
function encodeV1(
uint16 amountPosition_,
address finalToken_,
address forwardTo_,
address target_,
bytes memory payload_
)
Expand All @@ -63,7 +77,7 @@
if (amountPosition_ != AMOUNT_NOT_PRESENT && (uint256(amountPosition_) + 32 > payload_.length)) {
revert ZapDataV1__InvalidEncoding();
}
return abi.encodePacked(VERSION, amountPosition_, target_, payload_);
return abi.encodePacked(VERSION, amountPosition_, finalToken_, forwardTo_, target_, payload_);
}

/// @notice Extracts the version from the encoded Zap Data.
Expand All @@ -74,6 +88,22 @@
}
}

/// @notice Extracts the finalToken address from the encoded Zap Data.
function finalToken(bytes calldata encodedZapData) internal pure returns (address finalToken_) {
// Load 32 bytes from the offset and shift it 96 bits to the right to get the highest 160 bits.
assembly {
finalToken_ := shr(96, calldataload(add(encodedZapData.offset, OFFSET_FINAL_TOKEN)))
}
}
Comment on lines +92 to +97

Check warning

Code scanning / Slither

Assembly usage Warning

ZapDataV1.finalToken(bytes) uses assembly
- INLINE ASM

/// @notice Extracts the forwardTo address from the encoded Zap Data.
function forwardTo(bytes calldata encodedZapData) internal pure returns (address forwardTo_) {
// Load 32 bytes from the offset and shift it 96 bits to the right to get the highest 160 bits.
assembly {
forwardTo_ := shr(96, calldataload(add(encodedZapData.offset, OFFSET_FORWARD_TO)))
}
}
Comment on lines +100 to +105

Check warning

Code scanning / Slither

Assembly usage Warning

ZapDataV1.forwardTo(bytes) uses assembly
- INLINE ASM

/// @notice Extracts the target address from the encoded Zap Data.
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.
Expand Down
70 changes: 49 additions & 21 deletions packages/contracts-rfq/contracts/router/SynapseIntentPreviewer.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
/// @dev Amount value that signals that the Zap step should be performed using the full ZapRecipient balance.
uint256 internal constant FULL_BALANCE = type(uint256).max;

error SIP__NoOpForwardNotSupported();
error SIP__PoolTokenMismatch();
error SIP__PoolZeroAddress();
error SIP__RawParamsEmpty();
error SIP__TokenNotNative();

/// @inheritdoc ISynapseIntentPreviewer
// solhint-disable-next-line code-complexity
function previewIntent(
address swapQuoter,
address forwardTo,
address tokenIn,
address tokenOut,
uint256 amountIn
Expand All @@ -39,6 +42,7 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
{
// First, check if the intent is a no-op.
if (tokenIn == tokenOut) {
if (forwardTo != address(0)) revert SIP__NoOpForwardNotSupported();
return (amountIn, new ISynapseIntentRouter.StepParams[](0));
}

Expand All @@ -59,13 +63,13 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {

// Create the steps for the intent based on the action type.
if (params.action == Action.Swap) {
steps = _createSwapSteps(tokenIn, tokenOut, amountIn, params);
steps = _createSwapSteps(tokenIn, tokenOut, amountIn, params, forwardTo);
} else if (params.action == Action.AddLiquidity) {
steps = _createAddLiquiditySteps(tokenIn, tokenOut, params);
steps = _createAddLiquiditySteps(tokenIn, tokenOut, params, forwardTo);
} else if (params.action == Action.RemoveLiquidity) {
steps = _createRemoveLiquiditySteps(tokenIn, tokenOut, params);
steps = _createRemoveLiquiditySteps(tokenIn, tokenOut, params, forwardTo);
} else {
steps = _createHandleHativeSteps(tokenIn, tokenOut, amountIn);
steps = _createHandleHativeSteps(tokenIn, tokenOut, amountIn, forwardTo);
}
}

Expand All @@ -74,7 +78,8 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
address tokenIn,
address tokenOut,
uint256 amountIn,
DefaultParams memory params
DefaultParams memory params,
address forwardTo
)
internal
view
Expand All @@ -89,10 +94,10 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
address wrappedNative = IDefaultPool(pool).getToken(params.tokenIndexFrom);
// Sanity check tokenOut vs tokenIndexTo.
if (IDefaultPool(pool).getToken(params.tokenIndexTo) != tokenOut) revert SIP__PoolTokenMismatch();
// Native => WrappedNative + WrappedNative => TokenOut.
// Native => WrappedNative + WrappedNative => TokenOut. Forwarding is done in the second step.
return _toStepsArray(
_createWrapNativeStep({wrappedNative: wrappedNative, amountIn: amountIn}),
_createSwapStep({tokenIn: wrappedNative, params: params})
_createWrapNativeStep({wrappedNative: wrappedNative, msgValue: amountIn, forwardTo: address(0)}),
_createSwapStep({tokenIn: wrappedNative, tokenOut: tokenOut, params: params, forwardTo: forwardTo})
);
}

Expand All @@ -103,25 +108,28 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
if (tokenOut == NATIVE_GAS_TOKEN) {
// Get the address of the wrapped native token.
address wrappedNative = IDefaultPool(pool).getToken(params.tokenIndexTo);
// TokenIn => WrappedNative + WrappedNative => Native.
// TokenIn => WrappedNative + WrappedNative => Native. Forwarding is done in the second step.
return _toStepsArray(
_createSwapStep({tokenIn: tokenIn, params: params}),
_createUnwrapNativeStep({wrappedNative: wrappedNative})
_createSwapStep({tokenIn: tokenIn, tokenOut: wrappedNative, params: params, forwardTo: address(0)}),
_createUnwrapNativeStep({wrappedNative: wrappedNative, forwardTo: forwardTo})
);
}

// Sanity check tokenOut vs tokenIndexTo.
if (IDefaultPool(pool).getToken(params.tokenIndexTo) != tokenOut) revert SIP__PoolTokenMismatch();

// TokenIn => TokenOut.
return _toStepsArray(_createSwapStep({tokenIn: tokenIn, params: params}));
ISynapseIntentRouter.StepParams memory step =
_createSwapStep({tokenIn: tokenIn, tokenOut: tokenOut, params: params, forwardTo: forwardTo});
return _toStepsArray(step);
}

/// @notice Helper function to create steps for adding liquidity.
function _createAddLiquiditySteps(
address tokenIn,
address tokenOut,
DefaultParams memory params
DefaultParams memory params,
address forwardTo
)
internal
view
Expand Down Expand Up @@ -152,6 +160,8 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
msgValue: 0,
zapData: ZapDataV1.encodeV1({
target_: pool,
finalToken_: tokenOut,
forwardTo_: forwardTo,
// addLiquidity(amounts, minToMint, deadline)
payload_: abi.encodeCall(IDefaultExtendedPool.addLiquidity, (amounts, 0, type(uint256).max)),
// amountIn is encoded within `amounts` at `TOKEN_IN_INDEX`, `amounts` is encoded after
Expand All @@ -166,7 +176,8 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
function _createRemoveLiquiditySteps(
address tokenIn,
address tokenOut,
DefaultParams memory params
DefaultParams memory params,
address forwardTo
)
internal
view
Expand All @@ -185,6 +196,8 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
msgValue: 0,
zapData: ZapDataV1.encodeV1({
target_: pool,
finalToken_: tokenOut,
forwardTo_: forwardTo,
// removeLiquidityOneToken(tokenAmount, tokenIndex, minAmount, deadline)
payload_: abi.encodeCall(
IDefaultExtendedPool.removeLiquidityOneToken, (0, params.tokenIndexTo, 0, type(uint256).max)
Expand All @@ -205,26 +218,31 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
function _createHandleHativeSteps(
address tokenIn,
address tokenOut,
uint256 amountIn
uint256 amountIn,
address forwardTo
)
internal
pure
returns (ISynapseIntentRouter.StepParams[] memory steps)
{
if (tokenIn == NATIVE_GAS_TOKEN) {
// tokenOut is Wrapped Native
return _toStepsArray(_createWrapNativeStep({wrappedNative: tokenOut, amountIn: amountIn}));
return _toStepsArray(
_createWrapNativeStep({wrappedNative: tokenOut, msgValue: amountIn, forwardTo: forwardTo})
);
}
// Sanity check tokenOut
if (tokenOut != NATIVE_GAS_TOKEN) revert SIP__TokenNotNative();
// tokenIn is Wrapped Native
return _toStepsArray(_createUnwrapNativeStep({wrappedNative: tokenIn}));
return _toStepsArray(_createUnwrapNativeStep({wrappedNative: tokenIn, forwardTo: forwardTo}));
}

/// @notice Helper function to create a single step for a swap.
function _createSwapStep(
address tokenIn,
DefaultParams memory params
address tokenOut,
DefaultParams memory params,
address forwardTo
)
internal
pure
Expand All @@ -236,6 +254,8 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
msgValue: 0,
zapData: ZapDataV1.encodeV1({
target_: params.pool,
finalToken_: tokenOut,
forwardTo_: forwardTo,
// swap(tokenIndexFrom, tokenIndexTo, dx, minDy, deadline)
payload_: abi.encodeCall(
IDefaultPool.swap, (params.tokenIndexFrom, params.tokenIndexTo, 0, 0, type(uint256).max)
Expand All @@ -249,7 +269,8 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
/// @notice Helper function to create a single step for wrapping native gas tokens.
function _createWrapNativeStep(
address wrappedNative,
uint256 amountIn
uint256 msgValue,
address forwardTo
)
internal
pure
Expand All @@ -258,9 +279,11 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
return ISynapseIntentRouter.StepParams({
token: NATIVE_GAS_TOKEN,
amount: FULL_BALANCE,
msgValue: amountIn,
msgValue: msgValue,
zapData: ZapDataV1.encodeV1({
target_: wrappedNative,
finalToken_: wrappedNative,
forwardTo_: forwardTo,
// deposit()
payload_: abi.encodeCall(IWETH9.deposit, ()),
// amountIn is not encoded
Expand All @@ -270,7 +293,10 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
}

/// @notice Helper function to create a single step for unwrapping native gas tokens.
function _createUnwrapNativeStep(address wrappedNative)
function _createUnwrapNativeStep(
address wrappedNative,
address forwardTo
)
internal
pure
returns (ISynapseIntentRouter.StepParams memory)
Expand All @@ -281,6 +307,8 @@ contract SynapseIntentPreviewer is ISynapseIntentPreviewer {
msgValue: 0,
zapData: ZapDataV1.encodeV1({
target_: wrappedNative,
finalToken_: NATIVE_GAS_TOKEN,
forwardTo_: forwardTo,
// withdraw(amount)
payload_: abi.encodeCall(IWETH9.withdraw, (0)),
// amountIn encoded as the first parameter
Expand Down
Loading
Loading