Gho ccip bridge (#7) #347

LucasWongC · 2024-09-30T13:18:53Z

AaveCcipGhoBridge Smart Contract

Overview

The AaveCcipGhoBridge smart contract facilitates the secure bridging of GHO tokens across different blockchain networks using Chainlink's Cross-Chain Interoperability Protocol (CCIP). By leveraging Chainlink's decentralized infrastructure, it ensures reliable and transparent cross-chain token transfers while using GHO tokens exclusively for fees.

Key Components

Router and GHO Addresses:
- ROUTER: The Chainlink CCIP router address for cross-chain communication.
- GHO: Address of the GHO token, which the contract bridges across chains.
Bridges Mapping:
- A mapping of destination chain selectors (chain identifiers) to corresponding bridge addresses on the destination chains.

Main Functionalities

Token Transfers:
- Facilitates secure GHO token transfers between chains.
- Collects GHO tokens from the sender, validates the destination chain and bridge configuration, and transfers the tokens via Chainlink CCIP.
- The checkDestination modifier ensures that the destination chain and bridge are properly configured before any transfer.
Fee Payment:
- Fees are paid exclusively in GHO tokens.
- Calculates the required fee for a transfer and deducts it directly from the user's GHO balance.
Cross-Chain Message Handling:
- Processes incoming cross-chain messages using the _ccipReceive function.
- Decodes the message, verifies its authenticity, and releases the corresponding GHO tokens to the recipient on the destination chain.
Quote Transfer:
- The quoteTransfer function allows users to estimate the GHO fee required for a transfer without executing the transfer.
Setting Destination Bridges:
- The contract owner can configure or update the bridge addresses for different destination chains via the setDestinationBridge function.

Security Features

Role-Based Access Control:
Uses AccessControl to manage permissions:
- DEFAULT_ADMIN_ROLE: Grants administrative rights.
- BRIDGER_ROLE: Restricts who can initiate bridging operations.
  This ensures that only authorized users can perform critical functions.
Destination Validation:
The checkDestination modifier ensures that the destination chain and bridge address are properly configured before executing transfers.
Token Approval Management:
Securely handles GHO token approvals for the router, mitigating over-approval risks.
Rescue Functionality:
Implements a Rescuable mechanism, allowing the EXECUTOR to rescue funds in emergencies while maintaining security boundaries.
CCIP Sender and Receiver Validation:
- Verifies messages originate from the correct source bridge using Chainlink CCIP.
- Incoming messages must match the expected sender address for the specified source chain.

Events

TransferIssued: Emitted when a transfer is successfully initiated.
TransferFinished: Emitted when a transfer completes successfully on the destination chain.
DestinationUpdated: Emitted when a new bridge address is set for a specific chain.

Error Handling

UnsupportedChain: Thrown when attempting to transfer to an unsupported destination chain.
InvalidTransferAmount: Thrown when the total amount of tokens to transfer is zero.
InsufficientFee: Thrown if the provided fee is insufficient to cover the transaction.
InvalidMessage: Thrown when a message received from a source chain does not come from the expected bridge address.

Deployed Addresses

Mainnet: 0x5648f519b2064ff30385828e76fefda749749ac2, Arbitrum: 0x03f589a825ee129c5c6d2f6ef5c259870019567b

Confirmed Bridges:

Direct	CCIP Message ID	Source Tx	Destination Tx
Ethereum -> Arbitrum	0x87790f04c4cef490f46143efd7086147326ac65a3421c4aca82dd251f55bef82	0xa0c72c9e705ce20bb53ba0a57d249d082930d791c3f733a95ea07398b946e4b3	0xcea9e503c001119535abf0016f3e2ccc8e71fe0e202522dec92dd52811334393
Arbitrum -> Ethereum	0xe7eae1ddf0138b4a410a46b801aecbd1ca2ef29750980625fafa12adcf37946f	0xa9751980b3a66903031a9751d92ab56c331ad76916fea9b21ac3a92f8970d743	0x5310d08839775003e63a763c73721491d51439aea43de6b298388790922b8ba3

Conclusion

The AaveCcipGhoBridge smart contract provides a robust mechanism for bridging GHO tokens across multiple chains via Chainlink's CCIP. It offers flexibility in fee payments, supports secure cross-chain transfers, and allows for easy configuration of destination bridges.

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

tests/bridges/chainlink-ccip/AaveCcipGhoBridgeTest.t.sol

foundry.toml

scripts/DeployBridges.s.sol

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

src/bridges/chainlink-ccip/IAaveCcipGhoBridge.sol

src/bridges/chainlink-ccip/README.md

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

scripts/DeployBridges.s.sol

sendra · 2024-10-25T12:45:00Z

src/bridges/chainlink-ccip/README.md

+
+### Deployed Addresses
+
+Mainnet: [0x5648f519b2064ff30385828e76fefda749749ac2](https://etherscan.io/address/0x5648f519b2064ff30385828e76fefda749749ac2), Arbitrum: [0x03f589a825ee129c5c6d2f6ef5c259870019567b](https://arbiscan.io/address/0x03f589a825ee129c5c6d2f6ef5c259870019567b)


Update with final deployment addresses

It's not deployed yet for production and these addresses are deployed address to test main functionality

will update this address after fix all feedbacks

pavelvm5 · 2024-10-28T13:04:08Z

Can you clarify why all issues are considered resolved, although the code is not updated and does not compile with current versions?

We just don't understand a little, if the changes are in another PR, can you send a link?

LucasWongC · 2024-10-28T13:11:04Z

@pavelvm5 maybe you should run yarn install again

pavelvm5 · 2024-10-28T13:33:18Z

@pavelvm5 maybe you should run yarn install again

the problem is not yarn, it's that the new version has a maxRescuable function that needs to be implemented

You either need to set the commit in the solidity-utils library to the old one in foundry.toml config, or write an implementation for the new version.

I recommend doing the second version, solidity-utils will not be updated anytime soon.

The same goes for the chainlink dependency, which now requires 9 arguments instead of 6. And the same about guardians, their addresses exist in address-book for now.

LucasWongC · 2024-10-28T13:36:47Z

Can you try after run yarn install and forge install.

chainlink packages updated and this updates only applied for testnet. so we should use prev persion and I configured this on package.json correctly now.

foundry.toml

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

tests/bridges/chainlink-ccip/AaveCcipGhoBridgeTest.t.sol

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

src/bridges/chainlink-ccip/IAaveCcipGhoBridge.sol

src/bridges/chainlink-ccip/README.md

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

scripts/DeployBridges.s.sol

CheyenneAtapour · 2024-12-10T09:12:46Z

Tests fail

Failing tests:
Encountered 1 failing test in tests/bridges/arbitrum/AaveArbEthERC20BridgeTest.t.sol:EmergencyTokenTransfer
[FAIL: Error != expected error: OnlyRescueGuardian() != ONLY_RESCUE_GUARDIAN] test_revertsIf_invalidCaller() (gas: 12705)

Encountered 1 failing test in tests/bridges/optimism/AaveOpEthERC20BridgeTest.t.sol:EmergencyTokenTransfer
[FAIL: Error != expected error: OnlyRescueGuardian() != ONLY_RESCUE_GUARDIAN] test_revertsIf_invalidCaller() (gas: 12907)

Encountered 1 failing test in tests/bridges/polygon/AavePolEthERC20BridgeTest.t.sol:EmergencyTokenTransfer
[FAIL: Error != expected error: OnlyRescueGuardian() != ONLY_RESCUE_GUARDIAN] test_revertsIf_invalidCaller() (gas: 12719)

Encountered 1 failing test in tests/bridges/polygon/AavePolEthPlasmaBridge.t.sol:EmergencyTokenTransfer
[FAIL: Error != expected error: OnlyRescueGuardian() != ONLY_RESCUE_GUARDIAN] test_revertsIf_invalidCaller() (gas: 12727)

Encountered 1 failing test in tests/swaps/AaveSwapperTest.t.sol:EmergencyTokenTransfer
[FAIL: Error != expected error: OnlyRescueGuardian() != ONLY_RESCUE_GUARDIAN] test_revertsIf_invalidCaller() (gas: 10878)

Encountered 1 failing test in tests/v2-config-engine/AaveV2ConfigEngineTest.t.sol:AaveV2ConfigEngineTest
[FAIL: EvmError: Revert] testV2RateStrategiesUpdates() (gas: 2777880)

Encountered a total of 6 failing tests, 217 tests succeeded

There is a custom error on IRescuable error OnlyRescueGuardian(); which is invoked upon calling emergencyTokenTransfer if it is called by the wrong party. It is not the string as currently checked for in the tests: vm.expectRevert('ONLY_RESCUE_GUARDIAN');

There is another error as well, please check the above

miguelmtzinf · 2024-12-10T11:01:41Z

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

+        data: '',
+        tokenAmounts: tokenAmounts,
+        extraArgs: '',
+        feeToken: GHO


Imposing the fee in GHO can be problematic if GHO is not supported in that specific lane.
In other words, if GHO is not supported as fee token, bridge won't work.

This bridge won't be deployed on such network that doesn't support GHO. actually it's built for ARB -> ETH GHO bridge

miguelmtzinf · 2024-12-10T11:03:29Z

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

+  /// @inheritdoc IAaveCcipGhoBridge
+  function getInvalidMessage(
+    bytes32 messageId
+  ) external view checkInvalidMessage(messageId) returns (Client.Any2EVMMessage memory message) {


why this getter only works for invalid messages, rather than a general getter for messages, which can be helpful

we don't save another messages on contract. it will burn gas

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

miguelmtzinf · 2024-12-10T11:05:38Z

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

+  /// @inheritdoc IAaveCcipGhoBridge
+  function handleInvalidMessage(
+    bytes32 messageId
+  ) external onlyRole(DEFAULT_ADMIN_ROLE) checkInvalidMessage(messageId) {


unclear to me what this function does. Interface docs are not very descriptive neither.

This is probably based on this receiver example. It is meant to rescue tokens for messages that might not ever be executable. This means that even if the message is somehow malformed, the funds attached to it can always be recovered.

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

miguelmtzinf · 2024-12-10T11:07:50Z

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

+        data: '',
+        tokenAmounts: tokenAmounts,
+        extraArgs: Client._argsToBytes(
+          Client.EVMExtraArgsV2({gasLimit: gasLimit, allowOutOfOrderExecution: true})


the allowOutOfOrderExecution flag is not set in the other branch

I'm not sure how can I set this value if gasLimit is not set.

@RensR could you check this?

miguelmtzinf · 2024-12-10T11:09:22Z

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

+  }
+
+  /// @inheritdoc CCIPReceiver
+  function ccipReceive(Client.Any2EVMMessage calldata message) external override onlyRouter {


why are we overriding this instead of overriding _ccipReceive internal function, which is the recommendation as far as I see.

calldata can be used for storage, but memory can't be used for storage.
In definition of _ccipReceive, message type is memory, so can't use this to save on storage

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

RensR · 2024-12-10T11:38:45Z

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

+  /// @inheritdoc CCIPReceiver
+  function ccipReceive(Client.Any2EVMMessage calldata message) external override onlyRouter {
+    try this.processMessage(message) {} catch {
+      emit FailedToDecodeMessage();


Should this catch not be used to set the message in the failed state? See this example receiver code

I think we don't need to save failed status. we just need save failed or no.

RensR · 2024-12-10T11:46:41Z

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol

+  mapping(uint64 selector => address bridge) public bridges;
+
+  /// @dev Saves invalid message
+  mapping(bytes32 messageId => Client.Any2EVMMessage message) private messageContents;


Depending on what you aim to do with this message, you could consider storing only the relevant part of the message. The entire message is pretty large and would take hundreds of thousands of gas to store, which might not be ideal on chains like Ethereum. Also note that this gas would come out of the message ccip specified gasLimit. Say you always send a msg with a gasLimit of 100k, the storing of failed messages would always run out of gas and you end up needing to manually execute the tx with a higher gas limit to properly store it as a failed message. Only then you can run handleInvalidMessage to release the funds.

If you only need the tokens and amounts it's probably best to store those and potentially some context if required. With the messageID as key you could always get that context from CCIP as well but that would be an "external" dependency.

from where, can I get context of message with message Id?

CheyenneAtapour reviewed Oct 25, 2024

View reviewed changes

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol Outdated Show resolved Hide resolved

CheyenneAtapour reviewed Oct 25, 2024

View reviewed changes

tests/bridges/chainlink-ccip/AaveCcipGhoBridgeTest.t.sol Outdated Show resolved Hide resolved

CheyenneAtapour reviewed Oct 25, 2024

View reviewed changes

foundry.toml Outdated Show resolved Hide resolved

sendra reviewed Oct 25, 2024

View reviewed changes

src/bridges/chainlink-ccip/README.md Show resolved Hide resolved

sendra reviewed Oct 25, 2024

View reviewed changes

src/bridges/chainlink-ccip/README.md Outdated Show resolved Hide resolved

sendra reviewed Oct 25, 2024

View reviewed changes

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol Outdated Show resolved Hide resolved

sendra reviewed Oct 25, 2024

View reviewed changes

scripts/DeployBridges.s.sol Show resolved Hide resolved

sendra reviewed Oct 25, 2024

View reviewed changes

LucasWongC requested review from sendra and CheyenneAtapour October 25, 2024 13:42

LucasWongC closed this Oct 28, 2024

LucasWongC reopened this Oct 28, 2024

sendra reviewed Oct 28, 2024

View reviewed changes

foundry.toml Outdated Show resolved Hide resolved

pavelvm5 reviewed Oct 28, 2024

View reviewed changes

LucasWongC requested review from pavelvm5 and sendra October 28, 2024 16:19

miguelmtzinf reviewed Oct 29, 2024

View reviewed changes

src/bridges/chainlink-ccip/IAaveCcipGhoBridge.sol Outdated Show resolved Hide resolved

sendra reviewed Oct 29, 2024

View reviewed changes

src/bridges/chainlink-ccip/README.md Outdated Show resolved Hide resolved

LucasWongC requested review from miguelmtzinf and sendra October 29, 2024 14:09

miguelmtzinf reviewed Oct 30, 2024

View reviewed changes

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol Show resolved Hide resolved

LucasWongC requested a review from miguelmtzinf October 30, 2024 13:54

CheyenneAtapour reviewed Nov 1, 2024

View reviewed changes

scripts/DeployBridges.s.sol Outdated Show resolved Hide resolved

LucasWongC force-pushed the gho-ccip-bridge branch from eebbcd2 to 83f6ab5 Compare November 1, 2024 02:56

LucasWongC requested a review from CheyenneAtapour November 1, 2024 02:59

LucasWongC added 14 commits November 28, 2024 12:02

fix from feedbacks

02f0ef3

added quote function

4a2994e

used AccessControl instead of Ownable

7cc4d08

remove comments

796b1f8

removed comment

1687c87

removed sender from Finished event and added Issued event

64a266d

added arb -> eth test

362b349

added gasLimit customization

b888f8b

added invalid message handling part

cecb3a8

transfer to collectoer

97438ac

used to try-catch

66d5c41

added test for processMessage function

ea85f3c

added test for ccip internal functions

ec3cbe5

resolved yarn.lock manullay

64631dc

LucasWongC force-pushed the gho-ccip-bridge branch from d288da4 to 64631dc Compare November 28, 2024 17:07

LucasWongC added 2 commits December 2, 2024 15:12

updated chainlink version to latest

dcdf804

added more tests

4b5a4e7

miguelmtzinf reviewed Dec 10, 2024

View reviewed changes

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol Outdated Show resolved Hide resolved

miguelmtzinf reviewed Dec 10, 2024

View reviewed changes

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol Outdated Show resolved Hide resolved

miguelmtzinf reviewed Dec 10, 2024

View reviewed changes

src/bridges/chainlink-ccip/AaveCcipGhoBridge.sol Show resolved Hide resolved

RensR reviewed Dec 10, 2024

View reviewed changes

bugfix from feedback

f64ed23

LucasWongC requested a review from RensR December 10, 2024 14:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gho ccip bridge (#7) #347

Gho ccip bridge (#7) #347

LucasWongC commented Sep 30, 2024 •

edited

Loading

sendra Oct 25, 2024

LucasWongC Oct 25, 2024 •

edited

Loading

LucasWongC Oct 25, 2024 •

edited

Loading

pavelvm5 commented Oct 28, 2024

LucasWongC commented Oct 28, 2024

pavelvm5 commented Oct 28, 2024 •

edited

Loading

LucasWongC commented Oct 28, 2024

CheyenneAtapour commented Dec 10, 2024 •

edited

Loading

miguelmtzinf Dec 10, 2024

LucasWongC Dec 10, 2024

miguelmtzinf Dec 10, 2024

LucasWongC Dec 10, 2024

miguelmtzinf Dec 10, 2024

RensR Dec 10, 2024

miguelmtzinf Dec 10, 2024

LucasWongC Dec 10, 2024

miguelmtzinf Dec 10, 2024

LucasWongC Dec 12, 2024

RensR Dec 10, 2024

LucasWongC Dec 10, 2024

RensR Dec 10, 2024

LucasWongC Dec 10, 2024


		### Deployed Addresses

		Mainnet: [0x5648f519b2064ff30385828e76fefda749749ac2](https://etherscan.io/address/0x5648f519b2064ff30385828e76fefda749749ac2), Arbitrum: [0x03f589a825ee129c5c6d2f6ef5c259870019567b](https://arbiscan.io/address/0x03f589a825ee129c5c6d2f6ef5c259870019567b)

Gho ccip bridge (#7) #347

Are you sure you want to change the base?

Gho ccip bridge (#7) #347

Conversation

LucasWongC commented Sep 30, 2024 • edited Loading

AaveCcipGhoBridge Smart Contract

Overview

Key Components

Main Functionalities

Security Features

Events

Error Handling

Deployed Addresses

Conclusion

Choose a reason for hiding this comment

LucasWongC Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

LucasWongC Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

pavelvm5 commented Oct 28, 2024

LucasWongC commented Oct 28, 2024

pavelvm5 commented Oct 28, 2024 • edited Loading

LucasWongC commented Oct 28, 2024

CheyenneAtapour commented Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LucasWongC commented Sep 30, 2024 •

edited

Loading

LucasWongC Oct 25, 2024 •

edited

Loading

LucasWongC Oct 25, 2024 •

edited

Loading

pavelvm5 commented Oct 28, 2024 •

edited

Loading

CheyenneAtapour commented Dec 10, 2024 •

edited

Loading