From 0df8f08b3617e84f1ae5d71b5e45f172e08ce2db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CF=87=C2=B2?= <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 14 Oct 2024 10:29:51 +0100 Subject: [PATCH] feat(contracts-rfq): arbitrary call with value [SLT-233] [SLT-318] (#3246) * feat: add `callValue` to bridge params * feat: add `callValue` to bridge tx V2 * test: add cases for SRC calls with `callValue` * refactor: simplify tests further * test: add cases for DST relays with `callValue` * feat: support `callValue` in `bridge()` * feat: support `callValue` in `relay()` * test: update revert message for failed ETH transfers * refactor: always forward full msg.value for the hook call * refactor: use `_pullToken` only in `bridge()` * refactor: isolate validation of relay params * refactor: isolate validation of the bridge params * docs: could -> can * test: enable the backwards compatibility encoding test * fix: getBridgeTransaction partial support for V2 structs * test: add clarifications about expected reverts * test: more incorrect relay scenarios * refactor: better naming for `_pullToken`, docs * refactor: remove unnecessary cast * refactor: read from stack, not memory where possible * fix: emit the event prior to the external calls --- .../contracts-rfq/contracts/FastBridgeV2.sol | 259 ++++++++++-------- .../contracts/interfaces/IFastBridge.sol | 2 +- .../contracts/interfaces/IFastBridgeV2.sol | 7 +- .../interfaces/IFastBridgeV2Errors.sol | 1 + .../test/FastBridgeV2.Dst.ArbitraryCall.t.sol | 111 +++++++- .../test/FastBridgeV2.Dst.Exclusivity.t.sol | 21 +- .../contracts-rfq/test/FastBridgeV2.Dst.t.sol | 247 +++++++++++++++-- .../test/FastBridgeV2.Encoding.t.sol | 16 +- .../test/FastBridgeV2.Src.ArbitraryCall.t.sol | 53 ++++ .../contracts-rfq/test/FastBridgeV2.Src.t.sol | 2 +- .../contracts-rfq/test/FastBridgeV2.t.sol | 14 +- 11 files changed, 570 insertions(+), 163 deletions(-) diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index dd1d109126..b77e68a0c9 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -54,6 +54,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { quoteRelayer: address(0), quoteExclusivitySeconds: 0, quoteId: bytes(""), + callValue: 0, callParams: bytes("") }) }); @@ -126,34 +127,42 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { } /// @inheritdoc IFastBridge - function getBridgeTransaction(bytes memory request) external pure returns (BridgeTransaction memory) { - // TODO: the note below isn't true anymore with the BridgeTransactionV2 struct - // since the variable length `callParams` was added. This needs to be fixed/acknowledged. - - // Note: when passing V2 request, this will decode the V1 fields correctly since the new fields were - // added as the last fields of the struct and hence the ABI decoder will simply ignore the extra data. - return abi.decode(request, (BridgeTransaction)); + /// @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 + /// In order to process all kinds of requests use getBridgeTransactionV2 instead. + function getBridgeTransaction(bytes memory 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 + return BridgeTransaction({ + originChainId: txV2.originChainId, + destChainId: txV2.destChainId, + originSender: txV2.originSender, + destRecipient: txV2.destRecipient, + originToken: txV2.originToken, + destToken: txV2.destToken, + originAmount: txV2.originAmount, + destAmount: txV2.destAmount, + originFeeAmount: txV2.originFeeAmount, + sendChainGas: txV2.callValue != 0, + deadline: txV2.deadline, + nonce: txV2.nonce + }); + } catch { + // Fallback to V1 struct + return abi.decode(request, (BridgeTransaction)); + } } /// @inheritdoc IFastBridgeV2 - // TODO: reduce cyclomatic complexity alongside arbitrary call - // solhint-disable-next-line code-complexity function bridge(BridgeParams memory params, BridgeParamsV2 memory paramsV2) public payable { - // check bridge params - if (params.dstChainId == block.chainid) revert ChainIncorrect(); - if (params.originAmount == 0 || params.destAmount == 0) revert AmountIncorrect(); - if (params.sender == address(0) || params.to == address(0)) revert ZeroAddress(); - if (params.originToken == address(0) || params.destToken == address(0)) revert ZeroAddress(); - if (params.deadline < block.timestamp + MIN_DEADLINE_PERIOD) revert DeadlineTooShort(); - if (paramsV2.callParams.length > MAX_CALL_PARAMS_LENGTH) revert CallParamsLengthAboveMax(); int256 exclusivityEndTime = int256(block.timestamp) + paramsV2.quoteExclusivitySeconds; - // exclusivityEndTime must be in range (0 .. params.deadline] - if (exclusivityEndTime <= 0 || exclusivityEndTime > int256(params.deadline)) { - revert ExclusivityParamsIncorrect(); - } + _validateBridgeParams(params, paramsV2, exclusivityEndTime); + // transfer tokens to bridge contract - // @dev use returned originAmount in request in case of transfer fees - uint256 originAmount = _pullToken(address(this), params.originToken, params.originAmount); + /// @dev use returned originAmount in request in case of transfer fees + uint256 originAmount = _takeBridgedUserAsset(params.originToken, params.originAmount); // track amount of origin token owed to protocol uint256 originFeeAmount; @@ -172,7 +181,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { originAmount: originAmount, destAmount: params.destAmount, originFeeAmount: originFeeAmount, - sendChainGas: params.sendChainGas, + callValue: paramsV2.callValue, deadline: params.deadline, nonce: senderNonces[params.sender]++, // increment nonce on every bridge exclusivityRelayer: paramsV2.quoteRelayer, @@ -184,91 +193,73 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { bytes32 transactionId = keccak256(request); bridgeTxDetails[transactionId].status = BridgeStatus.REQUESTED; - emit BridgeRequested( - transactionId, - params.sender, - request, - params.dstChainId, - params.originToken, - params.destToken, - originAmount, - params.destAmount, - params.sendChainGas - ); + emit BridgeRequested({ + transactionId: transactionId, + sender: params.sender, + request: request, + destChainId: params.dstChainId, + originToken: params.originToken, + destToken: params.destToken, + originAmount: originAmount, + destAmount: params.destAmount, + sendChainGas: paramsV2.callValue != 0 + }); emit BridgeQuoteDetails(transactionId, paramsV2.quoteId); } /// @inheritdoc IFastBridgeV2 - // TODO: reduce cyclomatic complexity alongside arbitrary call - // solhint-disable-next-line code-complexity function relay(bytes memory request, address relayer) public payable { - if (relayer == address(0)) revert ZeroAddress(); - // Check if the transaction has already been relayed bytes32 transactionId = keccak256(request); - if (bridgeRelays(transactionId)) revert TransactionRelayed(); - // Decode the transaction and check that it could be relayed on this chain BridgeTransactionV2 memory transaction = getBridgeTransactionV2(request); - if (transaction.destChainId != uint32(block.chainid)) revert ChainIncorrect(); - // Check the deadline for relay to happen - if (block.timestamp > transaction.deadline) revert DeadlineExceeded(); - // Check the exclusivity period, if it is still ongoing - // forgefmt: disable-next-item - if ( - transaction.exclusivityRelayer != address(0) && - transaction.exclusivityRelayer != relayer && - block.timestamp <= transaction.exclusivityEndTime - ) { - revert ExclusivityPeriodNotPassed(); - } + _validateRelayParams(transaction, transactionId, relayer); // mark bridge transaction as relayed bridgeRelayDetails[transactionId] = BridgeRelay({blockNumber: uint48(block.number), blockTimestamp: uint48(block.timestamp), relayer: relayer}); - // transfer tokens to recipient on destination chain and gas rebate if requested + // transfer tokens to recipient on destination chain and do an arbitrary call if requested address to = transaction.destRecipient; address token = transaction.destToken; uint256 amount = transaction.destAmount; + uint256 callValue = transaction.callValue; + + // Emit the event before any external calls + emit BridgeRelayed({ + transactionId: transactionId, + relayer: relayer, + to: to, + originChainId: transaction.originChainId, + originToken: transaction.originToken, + destToken: token, + originAmount: transaction.originAmount, + destAmount: amount, + chainGasAmount: callValue + }); // 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 (transaction.callParams.length == 0) { - // No arbitrary call requested, so we just pull the tokens from the Relayer to the recipient, - // or transfer ETH to the recipient (if token is ETH_ADDRESS) - _pullToken(to, token, amount); - } else if (token != UniversalTokenLib.ETH_ADDRESS) { - // Arbitrary call requested with ERC20: pull the tokens from the Relayer to the recipient first - _pullToken(to, token, amount); - // Follow up with the hook function call - _checkedCallRecipient({ - recipient: to, - msgValue: 0, - token: token, - amount: amount, - callParams: transaction.callParams - }); + if (token == UniversalTokenLib.ETH_ADDRESS) { + // For ETH non-zero callValue is not allowed + if (callValue != 0) revert NativeTokenCallValueNotSupported(); + // Check that the correct msg.value was sent + if (msg.value != amount) revert MsgValueIncorrect(); } else { - // Arbitrary call requested with ETH: combine the ETH transfer with the call - _checkedCallRecipient({ - recipient: to, - msgValue: amount, - token: token, - amount: amount, - callParams: transaction.callParams - }); + // For ERC20s, we check that the correct msg.value was sent + if (msg.value != callValue) revert MsgValueIncorrect(); + // We need to transfer the tokens from the Relayer to the recipient first before performing an + // optional post-transfer arbitrary call. + IERC20(token).safeTransferFrom(msg.sender, to, amount); } - emit BridgeRelayed( - transactionId, - relayer, - to, - transaction.originChainId, - transaction.originToken, - transaction.destToken, - transaction.originAmount, - transaction.destAmount, - // chainGasAmount is 0 since the gas rebate function is deprecated - 0 - ); + if (transaction.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: transaction.callParams}); + } 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. + Address.sendValue(payable(to), msg.value); + } } /// @inheritdoc IFastBridgeV2 @@ -335,26 +326,24 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { return abi.decode(request, (BridgeTransactionV2)); } - /// @notice Pulls a requested token from the user to the requested recipient. - /// @dev Be careful of re-entrancy issues when msg.value > 0 and recipient != address(this) - function _pullToken(address recipient, address token, uint256 amount) internal returns (uint256 amountPulled) { - if (token != UniversalTokenLib.ETH_ADDRESS) { + /// @notice Takes the bridged asset from the user into FastBridgeV2 custody. It will be later + /// 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. + // Supplied `msg.value` is already in FastBridgeV2 custody. + if (amount != msg.value) revert MsgValueIncorrect(); + amountTaken = msg.value; + } else { + // 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(); - // Token needs to be pulled only if msg.value is zero - // This way user can specify WETH as the origin asset if (msg.value != 0) revert MsgValueIncorrect(); - // Record token balance before transfer - amountPulled = IERC20(token).balanceOf(recipient); - IERC20(token).safeTransferFrom(msg.sender, recipient, amount); - // Use the difference between the recorded balance and the current balance as the amountPulled - amountPulled = IERC20(token).balanceOf(recipient) - amountPulled; - } else { - // Otherwise, we need to check that ETH amount matches msg.value - if (amount != msg.value) revert MsgValueIncorrect(); - // Transfer value to recipient if not this address - if (recipient != address(this)) token.universalTransfer(recipient, amount); - // We will forward msg.value in the external call later, if recipient is not this contract - amountPulled = msg.value; + 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; } } @@ -362,7 +351,6 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { /// all the necessary checks for the returned value. function _checkedCallRecipient( address recipient, - uint256 msgValue, address token, uint256 amount, bytes memory callParams @@ -372,7 +360,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { bytes memory hookData = abi.encodeCall(IFastBridgeRecipient.fastBridgeTransferReceived, (token, amount, callParams)); // This will bubble any revert messages from the hook function - bytes memory returnData = Address.functionCallWithValue({target: recipient, data: hookData, value: msgValue}); + bytes memory returnData = Address.functionCallWithValue({target: recipient, data: hookData, 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 @@ -394,4 +382,59 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { delta = uint40(block.timestamp) - proofBlockTimestamp; } } + + /// @notice Performs all the necessary checks for a bridge to happen. + /// @dev There's no good way to refactor this function to reduce cyclomatic complexity due to + /// the number of checks that need to be performed, so we skip the code-complexity rule here. + // solhint-disable-next-line code-complexity + function _validateBridgeParams( + BridgeParams memory params, + BridgeParamsV2 memory paramsV2, + int256 exclusivityEndTime + ) + internal + view + { + // Check V1 (legacy) params + if (params.dstChainId == block.chainid) revert ChainIncorrect(); + if (params.originAmount == 0 || params.destAmount == 0) revert AmountIncorrect(); + if (params.sender == address(0) || params.to == address(0)) revert ZeroAddress(); + 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(); + } + // exclusivityEndTime must be in range (0 .. params.deadline] + if (exclusivityEndTime <= 0 || exclusivityEndTime > int256(params.deadline)) { + revert ExclusivityParamsIncorrect(); + } + } + + /// @notice Performs all the necessary checks for a relay to happen. + function _validateRelayParams( + BridgeTransactionV2 memory transaction, + bytes32 transactionId, + address relayer + ) + internal + view + { + if (relayer == address(0)) revert ZeroAddress(); + // Check if the transaction has already been relayed + if (bridgeRelays(transactionId)) revert TransactionRelayed(); + if (transaction.destChainId != block.chainid) revert ChainIncorrect(); + // Check the deadline for relay to happen + if (block.timestamp > transaction.deadline) revert DeadlineExceeded(); + // Check the exclusivity period, if it is still ongoing + // forgefmt: disable-next-item + if ( + transaction.exclusivityRelayer != address(0) && + transaction.exclusivityRelayer != relayer && + block.timestamp <= transaction.exclusivityEndTime + ) { + revert ExclusivityPeriodNotPassed(); + } + } } diff --git a/packages/contracts-rfq/contracts/interfaces/IFastBridge.sol b/packages/contracts-rfq/contracts/interfaces/IFastBridge.sol index b691dfb5b4..033d602f76 100644 --- a/packages/contracts-rfq/contracts/interfaces/IFastBridge.sol +++ b/packages/contracts-rfq/contracts/interfaces/IFastBridge.sol @@ -97,7 +97,7 @@ interface IFastBridge { /// @notice Decodes bridge request into a bridge transaction /// @param request The bridge request to decode - function getBridgeTransaction(bytes memory request) external pure returns (BridgeTransaction memory); + function getBridgeTransaction(bytes memory request) external view returns (BridgeTransaction memory); /// @notice Checks if the dispute period has passed so bridge deposit can be claimed /// @param transactionId The transaction id associated with the encoded bridge transaction to check diff --git a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol index c4c3751a5b..54dbb5f3f6 100644 --- a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol @@ -30,14 +30,17 @@ 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) /// @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) struct BridgeParamsV2 { address quoteRelayer; int256 quoteExclusivitySeconds; bytes quoteId; + uint256 callValue; bytes callParams; } @@ -54,7 +57,7 @@ interface IFastBridgeV2 is IFastBridge { uint256 originAmount; // amount in on origin bridge less originFeeAmount uint256 destAmount; uint256 originFeeAmount; - bool sendChainGas; + uint256 callValue; // ETH value to send to the recipient (if any) - replaces V1's sendChainGas flag uint256 deadline; // user specified deadline for destination relay uint256 nonce; address exclusivityRelayer; @@ -67,7 +70,7 @@ interface IFastBridgeV2 is IFastBridge { /// @notice Initiates bridge on origin chain to be relayed by off-chain relayer, with the ability /// to provide temporary exclusivity fill rights for the quote relayer. /// @param params The parameters required to bridge - /// @param paramsV2 The parameters for exclusivity fill rights (optional, could be left empty) + /// @param paramsV2 The parameters for exclusivity fill rights (optional, can be left empty) function bridge(BridgeParams memory params, BridgeParamsV2 memory paramsV2) external payable; /// @notice Relays destination side of bridge transaction by off-chain relayer diff --git a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2Errors.sol b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2Errors.sol index 7cc1423a84..f40b760c30 100644 --- a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2Errors.sol +++ b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2Errors.sol @@ -7,6 +7,7 @@ interface IFastBridgeV2Errors { error ChainIncorrect(); error ExclusivityParamsIncorrect(); error MsgValueIncorrect(); + error NativeTokenCallValueNotSupported(); error SenderIncorrect(); error StatusIncorrect(); error ZeroAddress(); diff --git a/packages/contracts-rfq/test/FastBridgeV2.Dst.ArbitraryCall.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Dst.ArbitraryCall.t.sol index af4f6235c6..1fb3ed8148 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Dst.ArbitraryCall.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Dst.ArbitraryCall.t.sol @@ -1,7 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {FastBridgeV2DstExclusivityTest, IFastBridgeV2} from "./FastBridgeV2.Dst.Exclusivity.t.sol"; +import {IFastBridgeV2} from "../contracts/interfaces/IFastBridgeV2.sol"; +import {FastBridgeV2DstExclusivityTest} from "./FastBridgeV2.Dst.Exclusivity.t.sol"; import {RecipientMock} from "./mocks/RecipientMock.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; @@ -98,6 +99,28 @@ contract FastBridgeV2DstArbitraryCallTest is FastBridgeV2DstExclusivityTest { relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: 0, bridgeTx: tokenTx}); } + function test_relay_token_withCallValue_excessiveReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + override + { + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(excessiveReturnValueRecipient); + vm.expectRevert(RecipientIncorrectReturnValue.selector); + relay({caller: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddressCallValue_excessiveReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + override + { + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(excessiveReturnValueRecipient); + vm.expectRevert(RecipientIncorrectReturnValue.selector); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + function test_relay_eth_excessiveReturnValueRecipient_revertWhenCallParamsPresent() public virtual override { setEthTestRecipient(excessiveReturnValueRecipient); vm.expectRevert(RecipientIncorrectReturnValue.selector); @@ -132,6 +155,28 @@ contract FastBridgeV2DstArbitraryCallTest is FastBridgeV2DstExclusivityTest { relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: 0, bridgeTx: tokenTx}); } + function test_relay_token_withCallValue_incorrectReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + override + { + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(incorrectReturnValueRecipient); + vm.expectRevert(RecipientIncorrectReturnValue.selector); + relay({caller: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddressCallValue_incorrectReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + override + { + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(incorrectReturnValueRecipient); + vm.expectRevert(RecipientIncorrectReturnValue.selector); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + function test_relay_eth_incorrectReturnValueRecipient_revertWhenCallParamsPresent() public virtual override { setEthTestRecipient(incorrectReturnValueRecipient); vm.expectRevert(RecipientIncorrectReturnValue.selector); @@ -150,6 +195,8 @@ contract FastBridgeV2DstArbitraryCallTest is FastBridgeV2DstExclusivityTest { // ══════════════════════════════════════════════ NO-OP RECIPIENT ══════════════════════════════════════════════════ + // Note: in these tests NoOpRecipient doesn't implement hook function, so we expect a generic OZ library revert. + function test_relay_token_noOpRecipient_revertWhenCallParamsPresent() public virtual override { setTokenTestRecipient(noOpRecipient); vm.expectRevert(Address.FailedInnerCall.selector); @@ -162,6 +209,24 @@ contract FastBridgeV2DstArbitraryCallTest is FastBridgeV2DstExclusivityTest { relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: 0, bridgeTx: tokenTx}); } + function test_relay_token_withCallValue_noOpRecipient_revertWhenCallParamsPresent() public virtual override { + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(noOpRecipient); + vm.expectRevert(Address.FailedInnerCall.selector); + relay({caller: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddressCallValue_noOpRecipient_revertWhenCallParamsPresent() + public + virtual + override + { + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(noOpRecipient); + vm.expectRevert(Address.FailedInnerCall.selector); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + function test_relay_eth_noOpRecipient_revertWhenCallParamsPresent() public virtual override { setEthTestRecipient(noOpRecipient); vm.expectRevert(Address.FailedInnerCall.selector); @@ -192,6 +257,28 @@ contract FastBridgeV2DstArbitraryCallTest is FastBridgeV2DstExclusivityTest { relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: 0, bridgeTx: tokenTx}); } + function test_relay_token_withCallValue_noReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + override + { + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(noReturnValueRecipient); + vm.expectRevert(RecipientNoReturnValue.selector); + relay({caller: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddressCallValue_noReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + override + { + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(noReturnValueRecipient); + vm.expectRevert(RecipientNoReturnValue.selector); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + function test_relay_eth_noReturnValueRecipient_revertWhenCallParamsPresent() public virtual override { setEthTestRecipient(noReturnValueRecipient); vm.expectRevert(RecipientNoReturnValue.selector); @@ -222,6 +309,20 @@ contract FastBridgeV2DstArbitraryCallTest is FastBridgeV2DstExclusivityTest { relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: 0, bridgeTx: tokenTx}); } + function test_relay_token_withCallValue_revert_recipientReverts() public { + setTokenTestCallValue(CALL_VALUE); + mockRecipientRevert(tokenTx); + vm.expectRevert(REVERT_MSG); + relay({caller: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddressCallValue_revert_recipientReverts() public { + setTokenTestCallValue(CALL_VALUE); + mockRecipientRevert(tokenTx); + vm.expectRevert(REVERT_MSG); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + function test_relay_eth_revert_recipientReverts() public { mockRecipientRevert(ethTx); vm.expectRevert(REVERT_MSG); @@ -237,14 +338,18 @@ contract FastBridgeV2DstArbitraryCallTest is FastBridgeV2DstExclusivityTest { function test_relay_eth_noCallParams_revert_recipientReverts() public { setEthTestCallParams(""); vm.mockCallRevert({callee: userB, data: "", revertData: bytes(REVERT_MSG)}); - vm.expectRevert("ETH transfer failed"); + // Note: OZ library doesn't bubble the revert message for just sending ETH + // (as opposed to doing an external hook call). Therefore we expect a generic library revert. + vm.expectRevert(Address.FailedInnerCall.selector); relay({caller: relayerB, msgValue: ethParams.destAmount, bridgeTx: ethTx}); } function test_relay_eth_withRelayerAddress_noCallParams_revert_recipientReverts() public { setEthTestCallParams(""); vm.mockCallRevert({callee: userB, data: "", revertData: bytes(REVERT_MSG)}); - vm.expectRevert("ETH transfer failed"); + // Note: OZ library doesn't bubble the revert message for just sending ETH + // (as opposed to doing an external hook call). Therefore we expect a generic library revert. + vm.expectRevert(Address.FailedInnerCall.selector); relayWithAddress({caller: relayerA, relayer: relayerB, msgValue: ethParams.destAmount, bridgeTx: ethTx}); } } diff --git a/packages/contracts-rfq/test/FastBridgeV2.Dst.Exclusivity.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Dst.Exclusivity.t.sol index a616872f93..ba66ae53c4 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Dst.Exclusivity.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Dst.Exclusivity.t.sol @@ -1,30 +1,15 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {FastBridgeV2DstTest, IFastBridgeV2} from "./FastBridgeV2.Dst.t.sol"; +import {FastBridgeV2DstTest} from "./FastBridgeV2.Dst.t.sol"; // solhint-disable func-name-mixedcase, ordering contract FastBridgeV2DstExclusivityTest is FastBridgeV2DstTest { uint256 public constant EXCLUSIVITY_PERIOD = 60 seconds; function createFixturesV2() public virtual override { - tokenParamsV2 = IFastBridgeV2.BridgeParamsV2({ - quoteRelayer: relayerA, - quoteExclusivitySeconds: int256(EXCLUSIVITY_PERIOD), - quoteId: "", - callParams: "" - }); - ethParamsV2 = IFastBridgeV2.BridgeParamsV2({ - quoteRelayer: relayerB, - quoteExclusivitySeconds: int256(EXCLUSIVITY_PERIOD), - quoteId: "", - callParams: "" - }); - - tokenTx.exclusivityRelayer = relayerA; - tokenTx.exclusivityEndTime = block.timestamp + EXCLUSIVITY_PERIOD; - ethTx.exclusivityRelayer = relayerB; - ethTx.exclusivityEndTime = block.timestamp + EXCLUSIVITY_PERIOD; + setTokenTestExclusivityParams(relayerA, EXCLUSIVITY_PERIOD); + setEthTestExclusivityParams(relayerB, EXCLUSIVITY_PERIOD); } // ═══════════════════════════════════════════════ RELAY: TOKEN ════════════════════════════════════════════════════ diff --git a/packages/contracts-rfq/test/FastBridgeV2.Dst.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Dst.t.sol index 026fed12f9..047ad6bec2 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Dst.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Dst.t.sol @@ -23,6 +23,8 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { uint256 chainGasAmount ); + uint256 public constant CALL_VALUE = 1_337_420; + address public excessiveReturnValueRecipient; address public incorrectReturnValueRecipient; address public noOpRecipient; @@ -67,6 +69,7 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { public virtual { + uint256 chainGasAmount = bridgeTx.destToken == ETH_ADDRESS ? 0 : bridgeTx.callValue; vm.expectEmit(address(fastBridge)); emit BridgeRelayed({ transactionId: txId, @@ -77,7 +80,7 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { destToken: bridgeTx.destToken, originAmount: bridgeTx.originAmount, destAmount: bridgeTx.destAmount, - chainGasAmount: 0 + chainGasAmount: chainGasAmount }); } @@ -89,15 +92,25 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { assertEq(relayer, expectedRelayer); } + function checkTokenBalances(address recipient, address relayCaller) public view { + assertEq(dstToken.balanceOf(recipient), tokenParams.destAmount); + assertEq(dstToken.balanceOf(relayCaller), LEFTOVER_BALANCE); + assertEq(dstToken.balanceOf(address(fastBridge)), 0); + } + + function checkEthBalances(address recipient, address relayCaller) public view { + assertEq(recipient.balance, ethParams.destAmount); + assertEq(relayCaller.balance, LEFTOVER_BALANCE); + assertEq(address(fastBridge).balance, 0); + } + /// @notice RelayerA completes the ERC20 bridge request function test_relay_token() public { bytes32 txId = getTxId(tokenTx); expectBridgeRelayed(tokenTx, txId, relayerA); relay({caller: relayerA, msgValue: 0, bridgeTx: tokenTx}); checkRelayedViews({txId: txId, expectedRelayer: relayerA}); - assertEq(dstToken.balanceOf(userB), tokenParams.destAmount); - assertEq(dstToken.balanceOf(relayerA), LEFTOVER_BALANCE); - assertEq(dstToken.balanceOf(address(fastBridge)), 0); + checkTokenBalances({recipient: userB, relayCaller: relayerA}); } /// @notice RelayerB completes the ERC20 bridge request, using relayerA's address @@ -106,9 +119,7 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { expectBridgeRelayed(tokenTx, txId, relayerA); relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: 0, bridgeTx: tokenTx}); checkRelayedViews({txId: txId, expectedRelayer: relayerA}); - assertEq(dstToken.balanceOf(userB), tokenParams.destAmount); - assertEq(dstToken.balanceOf(relayerB), LEFTOVER_BALANCE); - assertEq(dstToken.balanceOf(address(fastBridge)), 0); + checkTokenBalances({recipient: userB, relayCaller: relayerB}); } /// @notice RelayerB completes the ETH bridge request @@ -117,9 +128,7 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { expectBridgeRelayed(ethTx, txId, relayerB); relay({caller: relayerB, msgValue: ethParams.destAmount, bridgeTx: ethTx}); checkRelayedViews({txId: txId, expectedRelayer: relayerB}); - assertEq(userB.balance, ethParams.destAmount); - assertEq(relayerB.balance, LEFTOVER_BALANCE); - assertEq(address(fastBridge).balance, 0); + checkEthBalances({recipient: userB, relayCaller: relayerB}); } /// @notice RelayerA completes the ETH bridge request, using relayerB's address @@ -128,9 +137,7 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { expectBridgeRelayed(ethTx, txId, relayerB); relayWithAddress({caller: relayerA, relayer: relayerB, msgValue: ethParams.destAmount, bridgeTx: ethTx}); checkRelayedViews({txId: txId, expectedRelayer: relayerB}); - assertEq(userB.balance, ethParams.destAmount); - assertEq(relayerA.balance, LEFTOVER_BALANCE); - assertEq(address(fastBridge).balance, 0); + checkEthBalances({recipient: userB, relayCaller: relayerA}); } /// @notice RelayerA completes the ETH bridge request, using relayerB's address @@ -146,9 +153,29 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { assertEq(recordedBlockNumber, 987_654_321); assertEq(recordedBlockTimestamp, 123_456_789); assertEq(recordedRelayer, relayerB); - assertEq(userB.balance, ethParams.destAmount); - assertEq(relayerA.balance, LEFTOVER_BALANCE); - assertEq(address(fastBridge).balance, 0); + checkEthBalances({recipient: userB, relayCaller: relayerA}); + } + + // ══════════════════════════════════════════ RELAYS WITH CALL VALUE ═══════════════════════════════════════════════ + + function test_relay_token_withCallValue() public { + setTokenTestCallValue(CALL_VALUE); + bytes32 txId = getTxId(tokenTx); + expectBridgeRelayed(tokenTx, txId, relayerA); + relay({caller: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + checkRelayedViews({txId: txId, expectedRelayer: relayerA}); + checkTokenBalances({recipient: userB, relayCaller: relayerA}); + assertEq(userB.balance, CALL_VALUE); + } + + function test_relay_token_withRelayerAddressCallValue() public { + setTokenTestCallValue(CALL_VALUE); + bytes32 txId = getTxId(tokenTx); + expectBridgeRelayed(tokenTx, txId, relayerA); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + checkRelayedViews({txId: txId, expectedRelayer: relayerA}); + checkTokenBalances({recipient: userB, relayCaller: relayerB}); + assertEq(userB.balance, CALL_VALUE); } // ═════════════════════════════════════ EXCESSIVE RETURN VALUE RECIPIENT ══════════════════════════════════════════ @@ -171,6 +198,26 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { test_relay_token_withRelayerAddress(); } + function test_relay_token_withCallValue_excessiveReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + { + assertEmptyCallParams(tokenTx.callParams); + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(excessiveReturnValueRecipient); + test_relay_token_withCallValue(); + } + + function test_relay_token_withRelayerAddressCallValue_excessiveReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + { + assertEmptyCallParams(tokenTx.callParams); + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(excessiveReturnValueRecipient); + test_relay_token_withRelayerAddressCallValue(); + } + function test_relay_eth_excessiveReturnValueRecipient_revertWhenCallParamsPresent() public virtual { assertEmptyCallParams(ethTx.callParams); setEthTestRecipient(excessiveReturnValueRecipient); @@ -206,6 +253,26 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { test_relay_token_withRelayerAddress(); } + function test_relay_token_withCallValue_incorrectReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + { + assertEmptyCallParams(tokenTx.callParams); + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(incorrectReturnValueRecipient); + test_relay_token_withCallValue(); + } + + function test_relay_token_withRelayerAddressCallValue_incorrectReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + { + assertEmptyCallParams(tokenTx.callParams); + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(incorrectReturnValueRecipient); + test_relay_token_withRelayerAddressCallValue(); + } + function test_relay_eth_incorrectReturnValueRecipient_revertWhenCallParamsPresent() public virtual { assertEmptyCallParams(ethTx.callParams); setEthTestRecipient(incorrectReturnValueRecipient); @@ -234,6 +301,20 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { test_relay_token_withRelayerAddress(); } + function test_relay_token_withCallValue_nonPayableRecipient_revert() public { + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(nonPayableRecipient); + vm.expectRevert(); + relay({caller: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddressCallValue_nonPayableRecipient_revert() public { + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(nonPayableRecipient); + vm.expectRevert(); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: CALL_VALUE, bridgeTx: tokenTx}); + } + function test_relay_eth_revert_nonPayableRecipient() public { setEthTestRecipient(nonPayableRecipient); vm.expectRevert(); @@ -263,6 +344,20 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { test_relay_token_withRelayerAddress(); } + function test_relay_token_withCallValue_noOpRecipient_revertWhenCallParamsPresent() public virtual { + assertEmptyCallParams(tokenTx.callParams); + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(noOpRecipient); + test_relay_token_withCallValue(); + } + + function test_relay_token_withRelayerAddressCallValue_noOpRecipient_revertWhenCallParamsPresent() public virtual { + assertEmptyCallParams(tokenTx.callParams); + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(noOpRecipient); + test_relay_token_withRelayerAddressCallValue(); + } + function test_relay_eth_noOpRecipient_revertWhenCallParamsPresent() public virtual { assertEmptyCallParams(ethTx.callParams); setEthTestRecipient(noOpRecipient); @@ -292,6 +387,23 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { test_relay_token_withRelayerAddress(); } + function test_relay_token_withCallValue_noReturnValueRecipient_revertWhenCallParamsPresent() public virtual { + assertEmptyCallParams(tokenTx.callParams); + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(noReturnValueRecipient); + test_relay_token_withCallValue(); + } + + function test_relay_token_withRelayerAddressCallValue_noReturnValueRecipient_revertWhenCallParamsPresent() + public + virtual + { + assertEmptyCallParams(tokenTx.callParams); + setTokenTestCallValue(CALL_VALUE); + setTokenTestRecipient(noReturnValueRecipient); + test_relay_token_withRelayerAddressCallValue(); + } + function test_relay_eth_noReturnValueRecipient_revertWhenCallParamsPresent() public virtual { assertEmptyCallParams(ethTx.callParams); setEthTestRecipient(noReturnValueRecipient); @@ -361,6 +473,69 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { relayWithAddress({caller: relayerA, relayer: address(0), msgValue: 0, bridgeTx: tokenTx}); } + function test_relay_token_withCallValue_revert_zeroCallValue() public { + setTokenTestCallValue(CALL_VALUE); + vm.expectRevert(MsgValueIncorrect.selector); + relay({caller: relayerA, msgValue: 0, bridgeTx: tokenTx}); + } + + function test_relay_token_withCallValue_revert_lowerCallValue() public { + setTokenTestCallValue(CALL_VALUE); + vm.expectRevert(MsgValueIncorrect.selector); + relay({caller: relayerA, msgValue: CALL_VALUE - 1, bridgeTx: tokenTx}); + } + + function test_relay_token_withCallValue_revert_higherCallValue() public { + setTokenTestCallValue(CALL_VALUE); + vm.expectRevert(MsgValueIncorrect.selector); + relay({caller: relayerA, msgValue: CALL_VALUE + 1, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddressCallValue_revert_zeroCallValue() public { + setTokenTestCallValue(CALL_VALUE); + vm.expectRevert(MsgValueIncorrect.selector); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: 0, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddressCallValue_revert_lowerCallValue() public { + setTokenTestCallValue(CALL_VALUE); + vm.expectRevert(MsgValueIncorrect.selector); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: CALL_VALUE - 1, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddressCallValue_revert_higherCallValue() public { + setTokenTestCallValue(CALL_VALUE); + vm.expectRevert(MsgValueIncorrect.selector); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: CALL_VALUE + 1, bridgeTx: tokenTx}); + } + + function test_relay_eth_withCallValue_revert_notSupported() public { + setEthTestCallValue(CALL_VALUE); + // Neither destAmount, CALL_VALUE, nor destAmount + CALL_VALUE should work here + vm.expectRevert(NativeTokenCallValueNotSupported.selector); + relay({caller: relayerB, msgValue: CALL_VALUE, bridgeTx: ethTx}); + vm.expectRevert(NativeTokenCallValueNotSupported.selector); + relay({caller: relayerB, msgValue: ethParams.destAmount, bridgeTx: ethTx}); + vm.expectRevert(NativeTokenCallValueNotSupported.selector); + relay({caller: relayerB, msgValue: ethParams.destAmount + CALL_VALUE, bridgeTx: ethTx}); + } + + function test_relay_eth_withRelayerAddressCallValue_revert_notSupported() public { + setEthTestCallValue(CALL_VALUE); + // Neither destAmount, CALL_VALUE, nor destAmount + CALL_VALUE should work here + vm.expectRevert(NativeTokenCallValueNotSupported.selector); + relayWithAddress({caller: relayerA, relayer: relayerB, msgValue: CALL_VALUE, bridgeTx: ethTx}); + vm.expectRevert(NativeTokenCallValueNotSupported.selector); + relayWithAddress({caller: relayerA, relayer: relayerB, msgValue: ethParams.destAmount, bridgeTx: ethTx}); + vm.expectRevert(NativeTokenCallValueNotSupported.selector); + relayWithAddress({ + caller: relayerA, + relayer: relayerB, + msgValue: ethParams.destAmount + CALL_VALUE, + bridgeTx: ethTx + }); + } + function test_relay_token_revert_approvedZero() public { vm.prank(relayerA); dstToken.approve(address(fastBridge), 0); @@ -380,18 +555,52 @@ contract FastBridgeV2DstTest is FastBridgeV2DstBaseTest { relay({caller: relayerA, msgValue: tokenParams.destAmount, bridgeTx: tokenTx}); } + function test_relay_token_withRelayerAddress_revert_approvedZero() public { + vm.prank(relayerB); + dstToken.approve(address(fastBridge), 0); + vm.expectRevert(); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: 0, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddress_revert_approvedNotEnough() public { + vm.prank(relayerB); + dstToken.approve(address(fastBridge), tokenParams.destAmount - 1); + vm.expectRevert(); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: 0, bridgeTx: tokenTx}); + } + + function test_relay_token_withRelayerAddress_revert_nonZeroMsgValue() public { + vm.expectRevert(); + relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: tokenParams.destAmount, bridgeTx: tokenTx}); + } + function test_relay_eth_revert_lowerMsgValue() public { vm.expectRevert(); - relay({caller: relayerA, msgValue: ethParams.destAmount - 1, bridgeTx: ethTx}); + relay({caller: relayerB, msgValue: ethParams.destAmount - 1, bridgeTx: ethTx}); } function test_relay_eth_revert_higherMsgValue() public { vm.expectRevert(); - relay({caller: relayerA, msgValue: ethParams.destAmount + 1, bridgeTx: ethTx}); + relay({caller: relayerB, msgValue: ethParams.destAmount + 1, bridgeTx: ethTx}); } function test_relay_eth_revert_zeroMsgValue() public { vm.expectRevert(); - relay({caller: relayerA, msgValue: 0, bridgeTx: ethTx}); + relay({caller: relayerB, msgValue: 0, bridgeTx: ethTx}); + } + + function test_relay_eth_withRelayerAddress_revert_lowerMsgValue() public { + vm.expectRevert(); + relayWithAddress({caller: relayerA, relayer: relayerB, msgValue: ethParams.destAmount - 1, bridgeTx: ethTx}); + } + + function test_relay_eth_withRelayerAddress_revert_higherMsgValue() public { + vm.expectRevert(); + relayWithAddress({caller: relayerA, relayer: relayerB, msgValue: ethParams.destAmount + 1, bridgeTx: ethTx}); + } + + function test_relay_eth_withRelayerAddress_revert_zeroMsgValue() public { + vm.expectRevert(); + relayWithAddress({caller: relayerA, relayer: relayerB, msgValue: 0, bridgeTx: ethTx}); } } diff --git a/packages/contracts-rfq/test/FastBridgeV2.Encoding.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Encoding.t.sol index 0918c66bf5..21d32876fc 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Encoding.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Encoding.t.sol @@ -47,17 +47,15 @@ contract FastBridgeV2EncodingTest is FastBridgeV2Test { assertEq(decodedTx, bridgeTx); } - // The addition of variable length field (callParams) in BridgeTransactionV2 breaks the compatibility - // with the original BridgeTransaction struct. - // Solidity's abi.encode(bridgeTxV2) will use the first 32 bytes to encode the data offset for the whole struct, - // which is ALWAYS equal to 32 (data starts right after the offset). This is weird, but it is what it is. - // https://ethereum.stackexchange.com/questions/152971/abi-encode-decode-mystery-additional-32-byte-field-uniswap-v2 - function test_getBridgeTransaction_supportsV2(IFastBridgeV2.BridgeTransactionV2 memory bridgeTxV2) public { - // TODO: reevaluate the necessity of this test if/when the encoding scheme is changed - vm.skip(true); + /// @notice We expect all the V1 fields except for `sendChainGas` to match. + /// `sendChainGas` is replaced with `callValue` in V2, therefore we expect non-zero `callValue` + /// to match `sendChainGas = true` in V1 + function test_getBridgeTransaction_supportsV2(IFastBridgeV2.BridgeTransactionV2 memory bridgeTxV2) public view { bytes memory request = abi.encode(bridgeTxV2); IFastBridge.BridgeTransaction memory decodedTx = fastBridge.getBridgeTransaction(request); - assertEq(decodedTx, extractV1(bridgeTxV2)); + IFastBridge.BridgeTransaction memory expectedTx = extractV1(bridgeTxV2); + expectedTx.sendChainGas = bridgeTxV2.callValue > 0; + assertEq(decodedTx, expectedTx); } function test_getBridgeTransactionV2(IFastBridgeV2.BridgeTransactionV2 memory bridgeTxV2) public view { diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.ArbitraryCall.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.ArbitraryCall.t.sol index 2b1e83a30c..5b6e84b732 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.ArbitraryCall.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.ArbitraryCall.t.sol @@ -6,6 +6,7 @@ import {FastBridgeV2SrcExclusivityTest} from "./FastBridgeV2.Src.Exclusivity.t.s // solhint-disable func-name-mixedcase, ordering contract FastBridgeV2SrcArbitraryCallTest is FastBridgeV2SrcExclusivityTest { bytes public constant CALL_PARAMS = abi.encode("Hello, World!"); + uint256 public constant CALL_VALUE = 1_337_420; function createFixturesV2() public virtual override { super.createFixturesV2(); @@ -41,4 +42,56 @@ contract FastBridgeV2SrcArbitraryCallTest is FastBridgeV2SrcExclusivityTest { vm.expectRevert(CallParamsLengthAboveMax.selector); bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams, paramsV2: ethParamsV2}); } + + // ══════════════════════════════════════ WITH CALL VALUE, NO CALL PARAMS ══════════════════════════════════════════ + + function test_bridge_token_withCallValue_noCallParams() public { + setTokenTestCallParams(""); + setTokenTestCallValue(CALL_VALUE); + test_bridge_token(); + } + + function test_bridge_token_diffSender_withCallValue_noCallParams() public { + setTokenTestCallParams(""); + setTokenTestCallValue(CALL_VALUE); + test_bridge_token_diffSender(); + } + + function test_bridge_eth_withCallValue_noCallParams_revert() public { + setEthTestCallParams(""); + setEthTestCallValue(CALL_VALUE); + vm.expectRevert(NativeTokenCallValueNotSupported.selector); + bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams, paramsV2: ethParamsV2}); + } + + function test_bridge_eth_diffSender_withCallValue_noCallParams_revert() public { + setEthTestCallParams(""); + setEthTestCallValue(CALL_VALUE); + vm.expectRevert(NativeTokenCallValueNotSupported.selector); + bridge({caller: userB, msgValue: ethParams.originAmount, params: ethParams, paramsV2: ethParamsV2}); + } + + // ═══════════════════════════════════════ WITH CALL VALUE & CALL PARAMS ═══════════════════════════════════════════ + + function test_bridge_token_withCallValue_withCallParams() public { + setTokenTestCallValue(CALL_VALUE); + test_bridge_token(); + } + + function test_bridge_token_diffSender_withCallValue_withCallParams() public { + setTokenTestCallValue(CALL_VALUE); + test_bridge_token_diffSender(); + } + + function test_bridge_eth_withCallValue_withCallParams_revert() public { + setEthTestCallValue(CALL_VALUE); + vm.expectRevert(NativeTokenCallValueNotSupported.selector); + bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams, paramsV2: ethParamsV2}); + } + + function test_bridge_eth_diffSender_withCallValue_withCallParams_revert() public { + setEthTestCallValue(CALL_VALUE); + vm.expectRevert(NativeTokenCallValueNotSupported.selector); + bridge({caller: userB, msgValue: ethParams.originAmount, params: ethParams, paramsV2: ethParamsV2}); + } } diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol index 3a9edd328c..5baa42bda0 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol @@ -42,7 +42,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { destToken: bridgeTx.destToken, originAmount: bridgeTx.originAmount, destAmount: bridgeTx.destAmount, - sendChainGas: bridgeTx.sendChainGas + sendChainGas: bridgeTx.callValue > 0 }); } diff --git a/packages/contracts-rfq/test/FastBridgeV2.t.sol b/packages/contracts-rfq/test/FastBridgeV2.t.sol index e195d8fabf..fca2bad7c3 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.t.sol @@ -128,12 +128,14 @@ abstract contract FastBridgeV2Test is Test, IFastBridgeV2Errors { quoteRelayer: address(0), quoteExclusivitySeconds: 0, quoteId: bytes(""), + callValue: 0, callParams: bytes("") }); ethParamsV2 = IFastBridgeV2.BridgeParamsV2({ quoteRelayer: address(0), quoteExclusivitySeconds: 0, quoteId: bytes(""), + callValue: 0, callParams: bytes("") }); @@ -158,7 +160,6 @@ abstract contract FastBridgeV2Test is Test, IFastBridgeV2Errors { txV2.originAmount = txV1.originAmount; txV2.destAmount = txV1.destAmount; txV2.originFeeAmount = txV1.originFeeAmount; - txV2.sendChainGas = txV1.sendChainGas; txV2.deadline = txV1.deadline; txV2.nonce = txV1.nonce; } @@ -168,6 +169,11 @@ abstract contract FastBridgeV2Test is Test, IFastBridgeV2Errors { tokenTx.callParams = callParams; } + function setTokenTestCallValue(uint256 callValue) public { + tokenParamsV2.callValue = callValue; + tokenTx.callValue = callValue; + } + function setTokenTestExclusivityParams(address relayer, uint256 exclusivitySeconds) public { tokenParamsV2.quoteRelayer = relayer; tokenParamsV2.quoteExclusivitySeconds = int256(exclusivitySeconds); @@ -182,6 +188,11 @@ abstract contract FastBridgeV2Test is Test, IFastBridgeV2Errors { ethTx.callParams = callParams; } + function setEthTestCallValue(uint256 callValue) public { + ethParamsV2.callValue = callValue; + ethTx.callValue = callValue; + } + function setEthTestExclusivityParams(address relayer, uint256 exclusivitySeconds) public { ethParamsV2.quoteRelayer = relayer; ethParamsV2.quoteExclusivitySeconds = int256(exclusivitySeconds); @@ -205,7 +216,6 @@ abstract contract FastBridgeV2Test is Test, IFastBridgeV2Errors { txV1.originAmount = txV2.originAmount; txV1.destAmount = txV2.destAmount; txV1.originFeeAmount = txV2.originFeeAmount; - txV1.sendChainGas = txV2.sendChainGas; txV1.deadline = txV2.deadline; txV1.nonce = txV2.nonce; }