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

refactor(contracts-rfq): Zaps [SLT-351] #3300

Merged
merged 11 commits into from
Oct 28, 2024
107 changes: 58 additions & 49 deletions packages/contracts-rfq/contracts/FastBridgeV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,20 @@ import {SafeERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeE
import {Address} from "@openzeppelin/contracts/utils/Address.sol";

import {BridgeTransactionV2Lib} from "./libs/BridgeTransactionV2.sol";
import {UniversalTokenLib} from "./libs/UniversalToken.sol";

import {Admin} from "./Admin.sol";
import {IFastBridge} from "./interfaces/IFastBridge.sol";
import {IFastBridgeV2} from "./interfaces/IFastBridgeV2.sol";
import {IFastBridgeV2Errors} from "./interfaces/IFastBridgeV2Errors.sol";
import {IFastBridgeRecipient} from "./interfaces/IFastBridgeRecipient.sol";
import {IZapRecipient} from "./interfaces/IZapRecipient.sol";

/// @notice FastBridgeV2 is a contract for bridging tokens across chains.
contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
using BridgeTransactionV2Lib for bytes;
using SafeERC20 for IERC20;
using UniversalTokenLib for address;

/// @notice Address reserved for native gas token (ETH on Ethereum and most L2s, AVAX on Avalanche, etc)
address public constant NATIVE_GAS_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
Comment on lines +20 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Remaining references to UniversalTokenLib.ETH_ADDRESS detected. Please ensure all instances are updated to use NATIVE_GAS_TOKEN.

🔗 Analysis chain

Define NATIVE_GAS_TOKEN constant appropriately

The constant NATIVE_GAS_TOKEN is defined to represent the native gas token (e.g., ETH, AVAX) across different networks. This enhances clarity and replaces previous references to UniversalTokenLib.ETH_ADDRESS.

To ensure all references to UniversalTokenLib.ETH_ADDRESS have been updated, you can run:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining references to `UniversalTokenLib.ETH_ADDRESS`
rg 'UniversalTokenLib\.ETH_ADDRESS' --type solidity

Length of output: 86


Script:

#!/bin/bash
# Description: Check for any remaining references to `UniversalTokenLib.ETH_ADDRESS` in Solidity files
rg 'UniversalTokenLib\.ETH_ADDRESS' --glob '*.sol'

Length of output: 5843


/// @notice Dispute period for relayed transactions
uint256 public constant DISPUTE_PERIOD = 30 minutes;
Expand All @@ -28,8 +29,8 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
/// @notice Minimum deadline period to relay a requested bridge transaction
uint256 public constant MIN_DEADLINE_PERIOD = 30 minutes;

/// @notice Maximum length of accepted callParams
uint256 public constant MAX_CALL_PARAMS_LENGTH = 2 ** 16 - 1;
/// @notice Maximum length of accepted zapData
uint256 public constant MAX_ZAP_DATA_LENGTH = 2 ** 16 - 1;

/// @notice Status of the bridge tx on origin chain
mapping(bytes32 => BridgeTxDetails) public bridgeTxDetails;
Expand All @@ -56,8 +57,8 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
quoteRelayer: address(0),
quoteExclusivitySeconds: 0,
quoteId: bytes(""),
callValue: 0,
callParams: bytes("")
zapNative: 0,
zapData: bytes("")
})
});
}
Expand Down Expand Up @@ -124,7 +125,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
// Emit the event before any external calls
emit BridgeDepositRefunded(transactionId, to, token, amount);
// Complete the user refund as the last transaction action
if (token == UniversalTokenLib.ETH_ADDRESS) {
if (token == NATIVE_GAS_TOKEN) {
Address.sendValue(payable(to), amount);
} else {
IERC20(token).safeTransfer(to, amount);
Expand All @@ -142,13 +143,13 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {

/// @inheritdoc IFastBridge
/// @dev This method is added to achieve backwards compatibility with decoding requests into V1 structs:
/// - `callValue` is partially reported as a zero/non-zero flag
/// - `callParams` is ignored
/// - `zapNative` is partially reported as a zero/non-zero flag
/// - `zapData` is ignored
/// In order to process all kinds of requests use getBridgeTransactionV2 instead.
function getBridgeTransaction(bytes calldata request) external view returns (BridgeTransaction memory) {
// Try decoding into V2 struct first. This will revert if V1 struct is passed
try this.getBridgeTransactionV2(request) returns (BridgeTransactionV2 memory txV2) {
// Note: we entirely ignore the callParams field, as it was not present in V1
// Note: we entirely ignore the zapData field, as it was not present in V1
return BridgeTransaction({
originChainId: txV2.originChainId,
destChainId: txV2.destChainId,
Expand All @@ -159,7 +160,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
originAmount: txV2.originAmount,
destAmount: txV2.destAmount,
originFeeAmount: txV2.originFeeAmount,
sendChainGas: txV2.callValue != 0,
sendChainGas: txV2.zapNative != 0,
deadline: txV2.deadline,
nonce: txV2.nonce
});
Expand Down Expand Up @@ -208,8 +209,8 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
exclusivityRelayer: paramsV2.quoteRelayer,
// We checked exclusivityEndTime to be in range (0 .. params.deadline] above, so can safely cast
exclusivityEndTime: uint256(exclusivityEndTime),
callValue: paramsV2.callValue,
callParams: paramsV2.callParams
zapNative: paramsV2.zapNative,
zapData: paramsV2.zapData
ChiTimesChi marked this conversation as resolved.
Show resolved Hide resolved
})
);
bytes32 transactionId = keccak256(request);
Expand All @@ -224,7 +225,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
destToken: params.destToken,
originAmount: originAmount,
destAmount: params.destAmount,
sendChainGas: paramsV2.callValue != 0
sendChainGas: paramsV2.zapNative != 0
});
emit BridgeQuoteDetails(transactionId, paramsV2.quoteId);
}
Expand All @@ -238,11 +239,11 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
bridgeRelayDetails[transactionId] =
BridgeRelay({blockNumber: uint48(block.number), blockTimestamp: uint48(block.timestamp), relayer: relayer});

// transfer tokens to recipient on destination chain and do an arbitrary call if requested
// transfer tokens to recipient on destination chain and trigger Zap if requested
address to = request.destRecipient();
address token = request.destToken();
uint256 amount = request.destAmount();
uint256 callValue = request.callValue();
uint256 zapNative = request.zapNative();

// Emit the event before any external calls
emit BridgeRelayed({
Expand All @@ -254,33 +255,44 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
destToken: token,
originAmount: request.originAmount(),
destAmount: amount,
chainGasAmount: callValue
chainGasAmount: zapNative
});

// All state changes have been done at this point, can proceed to the external calls.
// This follows the checks-effects-interactions pattern to mitigate potential reentrancy attacks.
if (token == UniversalTokenLib.ETH_ADDRESS) {
// For ETH non-zero callValue is not allowed
if (callValue != 0) revert NativeTokenCallValueNotSupported();
if (token == NATIVE_GAS_TOKEN) {
// For the native gas token, additional zapNative is not allowed
if (zapNative != 0) revert ZapNativeNotSupported();
// Check that the correct msg.value was sent
if (msg.value != amount) revert MsgValueIncorrect();
// Don't do a native transfer yet: we will handle it alongside the Zap below
} else {
// For ERC20s, we check that the correct msg.value was sent
if (msg.value != callValue) revert MsgValueIncorrect();
if (msg.value != zapNative) revert MsgValueIncorrect();
// We need to transfer the tokens from the Relayer to the recipient first before performing an
// optional post-transfer arbitrary call.
// optional post-transfer Zap.
IERC20(token).safeTransferFrom(msg.sender, to, amount);
}
// At this point we have done:
// - Transferred the requested amount of ERC20 tokens to the recipient.
// At this point we have confirmed:
// - For ERC20s: msg.value matches the requested zapNative amount.
// - For the native gas token: msg.value matches the requested destAmount.
// Remaining optional things to do:
// - Forward the full msg.value to the recipient (if non-zero).
// - Trigger a Zap (if zapData is present).
bytes calldata zapData = request.zapData();
if (zapData.length != 0) {
// Zap Data is present: Zap has been requested by the recipient. Trigger it forwarding the full msg.value.

bytes calldata callParams = request.callParams();
if (callParams.length != 0) {
// Arbitrary call requested, perform it while supplying full msg.value to the recipient
// Note: if token has a fee on transfers, the recipient will have received less than `amount`.
// This is a very niche edge case and should be handled by the recipient contract.
_checkedCallRecipient({recipient: to, token: token, amount: amount, callParams: callParams});
_triggerZapWithChecks({recipient: to, token: token, amount: amount, zapData: zapData});
} else if (msg.value != 0) {
// No arbitrary call requested, but msg.value was sent. This is either a relay with ETH,
// or a non-zero callValue request with an ERC20. In both cases, transfer the ETH to the recipient.
// Zap Data is missing, but msg.value was sent. This could happen in two different cases:
// - Relay with the native gas token is happening.
// - Relay with ERC20 is happening, with a `zapNative > 0` request.
// In both cases, we need to transfer the full msg.value to the recipient.
Address.sendValue(payable(to), msg.value);
}
}
Expand Down Expand Up @@ -337,7 +349,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
// Emit the event before any external calls
emit BridgeDepositClaimed(transactionId, proofRelayer, to, token, amount);
// Complete the relayer claim as the last transaction action
if (token == UniversalTokenLib.ETH_ADDRESS) {
if (token == NATIVE_GAS_TOKEN) {
Address.sendValue(payable(to), amount);
} else {
IERC20(token).safeTransfer(to, amount);
Expand Down Expand Up @@ -367,43 +379,40 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
/// claimed by the relayer who completed the relay on destination chain, or refunded back to the user,
/// should no one complete the relay.
function _takeBridgedUserAsset(address token, uint256 amount) internal returns (uint256 amountTaken) {
if (token == UniversalTokenLib.ETH_ADDRESS) {
// For ETH we just need to check that the supplied msg.value is correct.
if (token == NATIVE_GAS_TOKEN) {
// For the native gas token, we just need to check that the supplied msg.value is correct.
// Supplied `msg.value` is already in FastBridgeV2 custody.
if (amount != msg.value) revert MsgValueIncorrect();
amountTaken = msg.value;
} else {
ChiTimesChi marked this conversation as resolved.
Show resolved Hide resolved
// For ERC20s, token is explicitly transferred from the user to FastBridgeV2.
// We don't allow non-zero `msg.value` to avoid extra funds from being stuck in FastBridgeV2.
token.assertIsContract();
if (msg.value != 0) revert MsgValueIncorrect();
// Throw an explicit error if the provided token address is not a contract
if (token.code.length == 0) revert TokenNotContract();
amountTaken = IERC20(token).balanceOf(address(this));
IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
// Use the balance difference as the amount taken in case of fee on transfer tokens.
amountTaken = IERC20(token).balanceOf(address(this)) - amountTaken;
}
}

/// @notice Calls the Recipient's hook function with the specified callParams and performs
/// @notice Calls the Recipient's hook function with the specified zapData and performs
/// all the necessary checks for the returned value.
function _checkedCallRecipient(
address recipient,
address token,
uint256 amount,
bytes calldata callParams
)
internal
{
bytes memory hookData =
abi.encodeCall(IFastBridgeRecipient.fastBridgeTransferReceived, (token, amount, callParams));
function _triggerZapWithChecks(address recipient, address token, uint256 amount, bytes calldata zapData) internal {
// This will bubble any revert messages from the hook function
bytes memory returnData = Address.functionCallWithValue({target: recipient, data: hookData, value: msg.value});
bytes memory returnData = Address.functionCallWithValue({
target: recipient,
data: abi.encodeCall(IZapRecipient.zap, (token, amount, zapData)),
// Note: see `relay()` for reasoning behind passing msg.value
value: msg.value
});
// Explicit revert if no return data at all
if (returnData.length == 0) revert RecipientNoReturnValue();
// Check that exactly a single return value was returned
if (returnData.length != 32) revert RecipientIncorrectReturnValue();
// Return value should be abi-encoded hook function selector
if (bytes32(returnData) != bytes32(IFastBridgeRecipient.fastBridgeTransferReceived.selector)) {
if (bytes32(returnData) != bytes32(IZapRecipient.zap.selector)) {
ChiTimesChi marked this conversation as resolved.
Show resolved Hide resolved
revert RecipientIncorrectReturnValue();
}
}
Expand Down Expand Up @@ -439,9 +448,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
if (params.originToken == address(0) || params.destToken == address(0)) revert ZeroAddress();
if (params.deadline < block.timestamp + MIN_DEADLINE_PERIOD) revert DeadlineTooShort();
// Check V2 params
if (paramsV2.callParams.length > MAX_CALL_PARAMS_LENGTH) revert CallParamsLengthAboveMax();
if (paramsV2.callValue != 0 && params.destToken == UniversalTokenLib.ETH_ADDRESS) {
revert NativeTokenCallValueNotSupported();
if (paramsV2.zapData.length > MAX_ZAP_DATA_LENGTH) revert ZapDataLengthAboveMax();
if (paramsV2.zapNative != 0 && params.destToken == NATIVE_GAS_TOKEN) {
revert ZapNativeNotSupported();
ChiTimesChi marked this conversation as resolved.
Show resolved Hide resolved
}
// exclusivityEndTime must be in range (0 .. params.deadline]
if (exclusivityEndTime <= 0 || exclusivityEndTime > int256(params.deadline)) {
Expand Down

This file was deleted.

16 changes: 8 additions & 8 deletions packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,22 @@ interface IFastBridgeV2 is IFastBridge {
/// for backwards compatibility.
/// Note: quoteRelayer and quoteExclusivitySeconds are either both zero (indicating no exclusivity)
/// or both non-zero (indicating exclusivity for the given period).
/// Note: callValue > 0 can NOT be used with destToken = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE (ETH_ADDRESS)
/// Note: zapNative > 0 can NOT be used with destToken = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE (native token)
/// @param quoteRelayer Relayer that provided the quote for the transaction
/// @param quoteExclusivitySeconds Period of time the quote relayer is guaranteed exclusivity after user's deposit
/// @param quoteId Unique quote identifier used for tracking the quote
/// @param callValue ETH value to send to the recipient (if any)
/// @param callParams Parameters for the arbitrary call to the destination recipient (if any)
/// @param zapNative ETH value to send to the recipient (if any)
/// @param zapData Parameters for the Zap to the destination recipient (if any)
struct BridgeParamsV2 {
address quoteRelayer;
int256 quoteExclusivitySeconds;
bytes quoteId;
uint256 callValue;
bytes callParams;
uint256 zapNative;
bytes zapData;
}

/// @notice Updated bridge transaction struct to include parameters introduced in FastBridgeV2.
/// Note: only `exclusivityRelayer` can fill such a transaction until `exclusivityEndTime`.
/// TODO: consider changing the encoding scheme to prevent spending extra gas on decoding.
struct BridgeTransactionV2 {
uint32 originChainId;
uint32 destChainId;
Expand All @@ -57,12 +56,13 @@ interface IFastBridgeV2 is IFastBridge {
uint256 originAmount; // amount in on origin bridge less originFeeAmount
uint256 destAmount;
uint256 originFeeAmount;
uint256 callValue; // ETH value to send to the recipient (if any) - replaces V1's sendChainGas flag
// Note: sendChainGas flag from V1 is deprecated
uint256 deadline; // user specified deadline for destination relay
uint256 nonce;
address exclusivityRelayer;
uint256 exclusivityEndTime;
bytes callParams;
uint256 zapNative; // ETH value to send to the recipient (if any)
bytes zapData; // data to pass for the Zap action (if any)
}

event BridgeQuoteDetails(bytes32 indexed transactionId, bytes quoteId);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
pragma solidity ^0.8.20;

interface IFastBridgeV2Errors {
error AmountIncorrect();
error CallParamsLengthAboveMax();
error ChainIncorrect();
error ExclusivityParamsIncorrect();
error MsgValueIncorrect();
error NativeTokenCallValueNotSupported();
error SenderIncorrect();
error StatusIncorrect();
error TokenNotContract();
error ZapDataLengthAboveMax();
error ZapNativeNotSupported();
error ZeroAddress();

error RecipientIncorrectReturnValue();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
Fixed Show fixed Hide fixed

interface IZapRecipient {
function zap(address token, uint256 amount, bytes memory zapData) external payable returns (bytes4);
}
Loading
Loading