Skip to content

Commit

Permalink
feat(contracts-rfq): arbitrary call with value [SLT-233] [SLT-318] (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
ChiTimesChi authored Oct 14, 2024
1 parent 70f17d3 commit 0df8f08
Show file tree
Hide file tree
Showing 11 changed files with 570 additions and 163 deletions.
259 changes: 151 additions & 108 deletions packages/contracts-rfq/contracts/FastBridgeV2.sol

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ interface IFastBridgeV2Errors {
error ChainIncorrect();
error ExclusivityParamsIncorrect();
error MsgValueIncorrect();
error NativeTokenCallValueNotSupported();
error SenderIncorrect();
error StatusIncorrect();
error ZeroAddress();
Expand Down
111 changes: 108 additions & 3 deletions packages/contracts-rfq/test/FastBridgeV2.Dst.ArbitraryCall.t.sol
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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});
}
}
21 changes: 3 additions & 18 deletions packages/contracts-rfq/test/FastBridgeV2.Dst.Exclusivity.t.sol
Original file line number Diff line number Diff line change
@@ -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 ════════════════════════════════════════════════════
Expand Down
Loading

0 comments on commit 0df8f08

Please sign in to comment.