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

store relayer on relay [SLT-182] #3170

Merged
merged 4 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions packages/contracts-rfq/contracts/FastBridgeV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
mapping(bytes32 => BridgeStatus) public bridgeStatuses;
/// @notice Proof of relayed bridge tx on origin chain
mapping(bytes32 => BridgeProof) public bridgeProofs;
/// @notice Whether bridge has been relayed on destination chain
mapping(bytes32 => bool) public bridgeRelays;
/// @notice Relay details on destination chain
mapping(bytes32 => BridgeRelay) public bridgeRelayDetails;

/// @dev to prevent replays
uint256 public nonce;
Expand Down Expand Up @@ -131,16 +131,19 @@

/// @inheritdoc IFastBridgeV2
function relay(bytes memory request, address relayer) public payable {
if (relayer == address(0)) revert ZeroAddress();
bytes32 transactionId = keccak256(request);
BridgeTransaction memory transaction = getBridgeTransaction(request);
if (transaction.destChainId != uint32(block.chainid)) revert ChainIncorrect();

// check haven't exceeded deadline for relay to happen
if (block.timestamp > transaction.deadline) revert DeadlineExceeded();

if (bridgeRelayDetails[transactionId].relayer != address(0)) revert TransactionRelayed();
Copy link
Collaborator

Choose a reason for hiding this comment

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

The idea was to utilise the public view here, so that in the event we want/need to update it, we won't end up with outdated check here

Suggested change
if (bridgeRelayDetails[transactionId].relayer != address(0)) revert TransactionRelayed();
if (bridgeRelays(transactionId)) revert TransactionRelayed();


// mark bridge transaction as relayed
if (bridgeRelays[transactionId]) revert TransactionRelayed();
bridgeRelays[transactionId] = true;
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
address to = transaction.destRecipient;
Expand Down Expand Up @@ -174,6 +177,12 @@
);
}

/// @inheritdoc IFastBridgeV2
function bridgeRelays(bytes32 transactionId) public view returns (bool) {
// has this transactionId been relayed?
return bridgeRelayDetails[transactionId].relayer != address(0);
}
Dismissed Show dismissed Hide dismissed

/// @inheritdoc IFastBridge
function prove(bytes memory request, bytes32 destTxHash) external {
prove(request, destTxHash, msg.sender);
Expand Down
11 changes: 10 additions & 1 deletion packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ pragma solidity ^0.8.20;
import {IFastBridge} from "./IFastBridge.sol";

interface IFastBridgeV2 is IFastBridge {

/// @notice Relays destination side of bridge transaction by off-chain relayer
/// @param request The encoded bridge transaction to relay on destination chain
/// @param relayer The address of the relaying entity which should have control of the origin funds when claimed
Expand All @@ -20,4 +19,14 @@ interface IFastBridgeV2 is IFastBridge {
/// @param request The encoded bridge transaction to claim on origin chain
function claim(bytes memory request) external;

/// @notice Checks if a transaction has been relayed
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs forge fmt + in general we try to define the structs at the top of the contract, prior to the functions. In hindsight, both of these should've been flagged by CI workflows - I will make a ticket to update these with a reasonable settings (e.g. older FastBridge has a ton of linter warnings that might as well be ignored at this point)

/// @param transactionId The ID of the transaction to check
/// @return True if the transaction has been relayed, false otherwise
function bridgeRelays(bytes32 transactionId) external view returns (bool);

struct BridgeRelay {
uint48 blockNumber;
uint48 blockTimestamp;
address relayer;
}
}
22 changes: 21 additions & 1 deletion packages/contracts-rfq/test/FastBridgeV2.Dst.t.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import {ChainIncorrect, DeadlineExceeded, TransactionRelayed} from "../contracts/libs/Errors.sol";
import {ChainIncorrect, DeadlineExceeded, TransactionRelayed, ZeroAddress} from "../contracts/libs/Errors.sol";

import {FastBridgeV2, FastBridgeV2Test, IFastBridge} from "./FastBridgeV2.t.sol";

Expand Down Expand Up @@ -115,6 +115,21 @@ contract FastBridgeV2DstTest is FastBridgeV2Test {
assertEq(address(fastBridge).balance, 0);
}

/// @notice RelayerB completes the ETH bridge request, using relayerA's address
function test_relay_eth_withRelayerAddress_checkBlockData() public {
vm.roll(987_654_321);
vm.warp(123_456_789);
bytes32 txId = getTxId(ethTx);
expectBridgeRelayed(ethTx, txId, address(relayerA));
relayWithAddress({caller: relayerB, relayer: relayerA, msgValue: ethParams.destAmount, bridgeTx: ethTx});
assertTrue(fastBridge.bridgeRelays(txId));
(uint48 recordedBlockNumber, uint48 recordedblockTimestamp,) = fastBridge.bridgeRelayDetails(txId);
assertEq(recordedBlockNumber, 987_654_321);
assertEq(recordedblockTimestamp, 123_456_789);
assertEq(address(userB).balance, ethParams.destAmount);
assertEq(address(relayerB).balance, LEFTOVER_BALANCE);
assertEq(address(fastBridge).balance, 0);
}
// ══════════════════════════════════════════════════ REVERTS ══════════════════════════════════════════════════════

function test_relay_revert_chainIncorrect() public {
Expand Down Expand Up @@ -152,4 +167,9 @@ contract FastBridgeV2DstTest is FastBridgeV2Test {
vm.expectRevert(DeadlineExceeded.selector);
relayWithAddress({caller: relayerA, relayer: relayerB, msgValue: 0, bridgeTx: tokenTx});
}

function test_relay_withRelayerAddress_revert_zeroAddr() public {
vm.expectRevert(ZeroAddress.selector);
relayWithAddress({caller: relayerA, relayer: address(0), msgValue: 0, bridgeTx: tokenTx});
}
}
Loading