CCIP Bridge #443
Merged
CCIP Bridge #443
+2,995
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 tasks
| vm.selectFork(mainnetFork); | ||
| uint128 limit = mainnetBridge.getRateLimit(ARBITRUM_CHAIN_SELECTOR); | ||
|
|
||
| vm.assume(amount > limit && amount < 1e32); // made top limit to prevent arithmetic overflow |
There was a problem hiding this comment.
Might be better to use bound here in case of running with very high fuzz runs, to avoid vm.assume rejecting too many inputs
|
|
||
| bridge = new AaveGhoCcipBridge(address(mockRouter), address(mockGho), collector, admin); | ||
|
|
||
| vm.startPrank(alice); |
There was a problem hiding this comment.
small nit: No need to start and stop prank, can just use vm.prank(alice)
Contributor
Author
|
@CheyenneAtapour updated! thanks |
src/bridges/ccip/CCIPReceiver.sol
Outdated
There was a problem hiding this comment.
is this contract provided by chainlink? wondering if makes sense to move this file and Client lib into a dedicated chainlink folder, under the ccip one
| * | ||
| * -- Permissions | ||
| * The contract implements AccessControl for permissioned functions. | ||
| * The DEFAULT_ADMIN will always be the respective network's Short Executor (Governance). |
| * @title AaveGhoCcipBridge | ||
| * @author TokenLogic | ||
| * @notice It provides bridging capabilities for the GHO token across networks. | ||
| * |
There was a problem hiding this comment.
I d move the spec to a README file
Comment on lines
80
to
85
| /** | ||
| * @param router The address of the Chainlink CCIP router | ||
| * @param gho The address of the GHO token | ||
| * @param collector The address of collector on same chain | ||
| * @param executor The address of the contract executor | ||
| */ |
There was a problem hiding this comment.
Suggested change
| /** | |
| * @param router The address of the Chainlink CCIP router | |
| * @param gho The address of the GHO token | |
| * @param collector The address of collector on same chain | |
| * @param executor The address of the contract executor | |
| */ | |
| /** | |
| * @dev Constructor | |
| * @param router The address of the Chainlink CCIP router | |
| * @param gho The address of the GHO token | |
| * @param collector The address of the Aave Collector | |
| * @param initialAdmin The address of the initial admin | |
| */ |
There was a problem hiding this comment.
(and the last param in the constructor should change to match ofc)
| address public immutable COLLECTOR; | ||
|
|
||
| /// @inheritdoc IAaveGhoCcipBridge | ||
| address public immutable EXECUTOR; |
There was a problem hiding this comment.
I d remove this and use a role for whoCanRescue or just the default admin role.
It's confusing having 2 access control systems in the same contract: access control and dedicated executor access control for rescue.
Contributor
Author
There was a problem hiding this comment.
what would you return in whoCanRescue() for the address? didn't understand that part.
something like:
whoCanRescue() {
if (hasRole(DEFAULT_ADMIN, msg.sender) return msg.sender
?
There was a problem hiding this comment.
it seems you gotta use RescuableACL instead and add proper ACL checks to _checkRescueGuardian. We can use DEFAULT_ADMIN or an ad-hoc role, no strong opinion
| uint128 limit = _getRateLimit(chainSelector); | ||
| if (amount > limit) { | ||
| revert RateLimitExceeded(limit); | ||
| } |
There was a problem hiding this comment.
not checking for bridge limit on Ethereum. Both things (rate limit and bridge limit) will be checked later when executing the transfer on CCIP side, so we can remove both checks if we wanna rely on them
Contributor
Author
There was a problem hiding this comment.
didn't understand the first part of not checking for bridge limit on Ethereum.
There was a problem hiding this comment.
a bit of literature about GHO CCIP and bridge limit: https://www.llamarisk.com/research/explainer-series-ccip
I d suggest to remove rate limit check and keep this simple
Contributor
There was a problem hiding this comment.
agree on this, we validate the rateLimit on TokenPool contract already so cleaner to remove from here
| if (feeToken == address(0)) { | ||
| if (msg.value < fee) revert InsufficientFee(); | ||
| } else { | ||
| IERC20(feeToken).transferFrom(msg.sender, address(this), fee); |
There was a problem hiding this comment.
why we do 2 transfers of GHO instead of 1. Separately, pls use safeTransfer
There was a problem hiding this comment.
ah, was assuming we only work with GHO as fee token if not ETH.
| * @param feeToken The address of the fee token (use address(0) for fee in native token) | ||
| * @return message EVM2AnyMessage to transfer GHO cross-chain | ||
| */ | ||
| function _buildCCIPMessage(uint64 chainSelector, uint256 amount, uint256 gasLimit, address feeToken) |
There was a problem hiding this comment.
do we wanna put any restriction on feeToken?
also, what happens if the token is not supported by CCIP. Cannot recall where it d fail
Contributor
Author
There was a problem hiding this comment.
the CCIP contract is validating this already, have a test for it here: https://github.com/bgd-labs/aave-helpers/pull/443/files#diff-7dc455c50fa609f9da307dc1b4b6fa831fb5e8d6f57f2d8e1a21effc7920df91R323
| } | ||
|
|
||
| /// @inheritdoc IAaveGhoCcipBridge | ||
| function setDestinationChain(uint64 chainSelector, address bridge) external onlyRole(DEFAULT_ADMIN_ROLE) { |
There was a problem hiding this comment.
what is the permissions hierarchy / model? Everything is under default admin role...
is just that we wanna have multiple admins?
Contributor
Author
There was a problem hiding this comment.
to me, these set functions should only be done via governance, which would be the admin.
for bridging, i'd allow a guardian (bridger role) to call that so it can be assigned to an allowed party like the aave finance committee
| function removeDestinationChain(uint64 chainSelector) external onlyRole(DEFAULT_ADMIN_ROLE) { | ||
| delete destinations[chainSelector]; | ||
|
|
||
| emit DestinationChainRemoved(chainSelector); |
There was a problem hiding this comment.
feels that this can be replaced with emit DestinationChainSet(chainSelector, address(0));
| revert UnknownSourceDestination(); | ||
| } | ||
|
|
||
| _ccipReceive(message); |
There was a problem hiding this comment.
i don't think an internal method is needed here, only used once
Contributor
Author
There was a problem hiding this comment.
it's overriding the CCIPReceiver.sol abstract contract
| } | ||
|
|
||
| /// @inheritdoc IAaveGhoCcipBridge | ||
| function processMessage(Client.Any2EVMMessage calldata message) external onlySelf { |
Contributor
Author
There was a problem hiding this comment.
try/catch on solidity only works for external methods
| try this.processMessage(message) {} | ||
| catch (bytes memory err) { | ||
| failedMessages[message.messageId] = true; | ||
| failedTokenTransfers[message.messageId] = message; |
There was a problem hiding this comment.
we got feedback from Chainlink saying that it's not needed to store the entire msg, can you revisit that please?
Contributor
🌈 Test Results zksyncCompiling 112 files with Solc 0.8.24
Solc 0.8.24 finished in 5.77s
Compiler run successful!
Compiling 112 files with zksolc and solc 0.8.24
zksolc and solc 0.8.24 finished in 75.65s
Compiler run successful with warnings:
Warning
Warning: You are using 'create'/'create2' in an assembly block, probably by providing bytecode and expecting an EVM-like behavior.
EraVM does not use bytecode for contract deployment. Instead, it refers to contracts using their bytecode hashes.
In order to deploy a contract, please use the `new` operator in Solidity instead of raw 'create'/'create2' in assembly.
In Solidity v0.6 and older, it can be a false-positive warning if there is 'create(' or 'create2(' in comments within assembly.
Learn more about CREATE/CREATE2 EraVM limitations at https: //docs.zksync.io/zksync-protocol/differences/evm-instructions#create-create2
You may disable this warning with:
1. `suppressedWarnings = ["assemblycreate"]` in standard JSON.
2. `--suppress-warnings assemblycreate` in the CLI.
--> lib/forge-std/src/StdCheats.sol: 506:19
|
506 | addr := create(0, add(bytecode, 0x20), mload(bytecode))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning
Warning: You are using 'create'/'create2' in an assembly block, probably by providing bytecode and expecting an EVM-like behavior.
EraVM does not use bytecode for contract deployment. Instead, it refers to contracts using their bytecode hashes.
In order to deploy a contract, please use the `new` operator in Solidity instead of raw 'create'/'create2' in assembly.
In Solidity v0.6 and older, it can be a false-positive warning if there is 'create(' or 'create2(' in comments within assembly.
Learn more about CREATE/CREATE2 EraVM limitations at https: //docs.zksync.io/zksync-protocol/differences/evm-instructions#create-create2
You may disable this warning with:
1. `suppressedWarnings = ["assemblycreate"]` in standard JSON.
2. `--suppress-warnings assemblycreate` in the CLI.
--> lib/forge-std/src/StdCheats.sol: 516:19
|
516 | addr := create(0, add(bytecode, 0x20), mload(bytecode))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning
Warning: You are using 'create'/'create2' in an assembly block, probably by providing bytecode and expecting an EVM-like behavior.
EraVM does not use bytecode for contract deployment. Instead, it refers to contracts using their bytecode hashes.
In order to deploy a contract, please use the `new` operator in Solidity instead of raw 'create'/'create2' in assembly.
In Solidity v0.6 and older, it can be a false-positive warning if there is 'create(' or 'create2(' in comments within assembly.
Learn more about CREATE/CREATE2 EraVM limitations at https: //docs.zksync.io/zksync-protocol/differences/evm-instructions#create-create2
You may disable this warning with:
1. `suppressedWarnings = ["assemblycreate"]` in standard JSON.
2. `--suppress-warnings assemblycreate` in the CLI.
--> lib/forge-std/src/StdCheats.sol: 527:19
|
527 | addr := create(val, add(bytecode, 0x20), mload(bytecode))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning
Warning: You are using 'create'/'create2' in an assembly block, probably by providing bytecode and expecting an EVM-like behavior.
EraVM does not use bytecode for contract deployment. Instead, it refers to contracts using their bytecode hashes.
In order to deploy a contract, please use the `new` operator in Solidity instead of raw 'create'/'create2' in assembly.
In Solidity v0.6 and older, it can be a false-positive warning if there is 'create(' or 'create2(' in comments within assembly.
Learn more about CREATE/CREATE2 EraVM limitations at https: //docs.zksync.io/zksync-protocol/differences/evm-instructions#create-create2
You may disable this warning with:
1. `suppressedWarnings = ["assemblycreate"]` in standard JSON.
2. `--suppress-warnings assemblycreate` in the CLI.
--> lib/forge-std/src/StdCheats.sol: 537:19
|
537 | addr := create(val, add(bytecode, 0x20), mload(bytecode))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2025-07-15T11:16:15.223187Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:15.545376Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:25.596472Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:38.343733Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:38.569581Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:38.794992Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:39.533608Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:39.999165Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:40.876242Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:41.298624Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:42.092081Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:42.883777Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:43.998173Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:44.591459Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:45.016863Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:46.099285Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:46.329908Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:46.548705Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:47.244779Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:47.652491Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:48.696039Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:49.156614Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:49.965556Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:50.792467Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:51.869054Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:52.889680Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:53.319453Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:54.387245Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:54.613006Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:54.830968Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:55.516719Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:55.934435Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:56.776886Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:57.218561Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:58.004324Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:58.772804Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:16:59.793902Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:00.612968Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:01.021085Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:02.088797Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:02.317293Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:02.534171Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:03.219525Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:03.628833Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:04.508910Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:04.940945Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:05.758674Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:06.533441Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:07.615410Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:08.448997Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:08.846594Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:09.866524Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:10.088807Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:10.298448Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:10.974269Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:11.375495Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:12.229737Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:12.761253Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:13.543134Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:14.357244Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:15.394381Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:16.272582Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
2025-07-15T11:17:16.703328Z ERROR foundry_zksync_core::vm::inspect: reverting initiator tx nonce for CALL address=0x1804c8ab1f12e6bbf3894d4083f33e07309d1f38 from=3 to=2 deploy_nonce=2
Ran 1 test for zksync/tests/ProtocolV3TestBase.t.sol:ProtocolV3TestBaseTest
[PASS] test_helpers() (gas: 86657142)
Logs:
0x1d17CBcF0D6D143135aE902365D2E5e2A16538D4
0x493257fD37EDB34451f62EDf8D2a0C418852bA4C
0x5AEa5775959fBC2557Cc8789bC1bf90A239D9a91
0x703b52F2b28fEbcB60E1372858AF5b18849FE867
0x5A7d6b2F92C77FAD6CCaBd7EE0624E64907Eaf3E
E2E: Collateral USDC, TestAsset USDC
SUPPLY: USDC, Amount: 100021569651
SUPPLY: USDC, Amount: 1000215696
WITHDRAW: USDC, Amount: 500107848
WITHDRAW: USDC, Amount: 500107848
BORROW: USDC, Amount 1000215696
REPAY: USDC, Amount: 1000215696
E2E: Collateral USDC, TestAsset USDT
SUPPLY: USDC, Amount: 100021569651
SUPPLY: USDT, Amount: 999072570
WITHDRAW: USDT, Amount: 499536285
WITHDRAW: USDT, Amount: 499536284
BORROW: USDT, Amount 999072570
REPAY: USDT, Amount: 999072570
E2E: Collateral USDC, TestAsset WETH
SUPPLY: USDC, Amount: 100021569651
SUPPLY: WETH, Amount: 247815794951065425
WITHDRAW: WETH, Amount: 123907897475532712
WITHDRAW: WETH, Amount: 123907897475532714
BORROW: WETH, Amount 247815794951065425
REPAY: WETH, Amount: 247815794951065425
E2E: Collateral USDC, TestAsset wstETH
SUPPLY: USDC, Amount: 100021569651
SUPPLY: wstETH, Amount: 208779402016001902
WITHDRAW: wstETH, Amount: 104389701008000951
WITHDRAW: wstETH, Amount: 104389701008000952
BORROW: wstETH, Amount 208779402016001902
REPAY: wstETH, Amount: 208779402016001902
E2E: Collateral USDC, TestAsset ZK
SUPPLY: USDC, Amount: 100021569651
SUPPLY: ZK, Amount: 3854761082871773878172
WITHDRAW: ZK, Amount: 1927380541435886939086
WITHDRAW: ZK, Amount: 1927380541435886939085
BORROW: ZK, Amount 3854761082871773878172
REPAY: ZK, Amount: 3854761082871773878172
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 63.11s (61.23s CPU time)
Ran 1 test suite in 63.12s (63.11s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests) |
Contributor
♻️ Forge Gas SnapshotsSeems like you are not measuring gas of any operations yet. 🤔 |
Contributor
Forge Build Sizes🔕 Unchanged
|
| /** | ||
| * @title AaveGhoCcipBridge | ||
| * @author TokenLogic | ||
| * @notice It provides bridging capabilities for the GHO token across networks. |
There was a problem hiding this comment.
Remove the word 'It' from the beginning
Suggested change
| * @notice It provides bridging capabilities for the GHO token across networks. | |
| * @notice Provides bridging capabilities for the GHO token across networks. |
| ) external view returns (Client.EVMTokenAmount[] memory); | ||
|
|
||
| /** | ||
| * @dev Returns the bridge rate limit for a given chain. |
There was a problem hiding this comment.
Suggested change
| * @dev Returns the bridge rate limit for a given chain. | |
| * @notice Returns the bridge rate limit for a given chain. |
| uint256 fee = IRouterClient(ROUTER).getFee(chainSelector, message); | ||
|
|
||
| if (feeToken == address(0)) { | ||
| if (msg.value < fee) revert InsufficientFee(); |
There was a problem hiding this comment.
should we refund the user ETH if they supply excess fee?
Contributor
Author
There was a problem hiding this comment.
@CheyenneAtapour I replaced the payback in order to simplify the logic. The extra ETH can be withdrawn by the Rescuable functions and i don't really envision ETH being used much to be honest.
The caller is always going to be governance, I'm also thinking of moving this back to Ownable.
What do you think? @miguelmtzinf
There was a problem hiding this comment.
sounds good. However, if the purpose of the contract is to be pre-funded always, we can remove this logic of pulling funds from sender and the payable (we should add a receive instead)
miguelmtzinf
reviewed
Sep 4, 2025
| revert UnsupportedChain(); | ||
| } | ||
|
|
||
| uint128 limit = _getRateLimit(chainSelector); |
There was a problem hiding this comment.
we are not checking bridgeLimit on ETH token pool. We can either check both things or none of them (this is for better devx, as it d revert anyway)
Contributor
Author
There was a problem hiding this comment.
@miguelmtzinf i have updated to check the current remaining limit (bridge - bridgedAmount), as it's most likely going to be an unreachable statement as the rateLimit is never going to be greater than the bridgeLimit itself.
efecarranza
commented
Sep 8, 2025
| } | ||
|
|
||
| /// @inheritdoc CCIPReceiver | ||
| function ccipReceive(Client.Any2EVMMessage calldata message) external override onlyRouter { |
Contributor
Author
There was a problem hiding this comment.
On processMessage() we are checking if message.sender is the bridge configured.
Contributor
Author
|
@miguelmtzinf @CheyenneAtapour @brotherlymite i have updated the PR. thank you |
Comment on lines
24
to
26
| * @dev The fee token must be pre-funded for the bridge message to be sent. When doing a governance | ||
| * proposal ensure funds are present on this contract beforehand (GHO/LINK) or transfer funds from the | ||
| * treasury as part of the proposal. |
There was a problem hiding this comment.
Suggested change
| * @dev The fee token must be pre-funded for the bridge message to be sent. When doing a governance | |
| * proposal ensure funds are present on this contract beforehand (GHO/LINK) or transfer funds from the | |
| * treasury as part of the proposal. | |
| * @dev The fees required for bridge messages must be available in advance. For governance proposals, ensure | |
| * this contract is pre-funded with the necessary tokens (GHO/LINK), or include a transfer from the treasury | |
| * as part of the proposal. |
| address public immutable COLLECTOR; | ||
|
|
||
| /// @inheritdoc IAaveGhoCcipBridge | ||
| mapping(uint64 chainSelector => RemoteChainConfig remoteConfig) public destinations; |
There was a problem hiding this comment.
Suggested change
| mapping(uint64 chainSelector => RemoteChainConfig remoteConfig) public destinations; | |
| mapping(uint64 chainSelector => RemoteChainConfig) public destinations; |
just because this was added in 0.8.18 and may bring a bit of friction for integrations
|
|
||
| /// https://etherscan.io/address/0x06179f7C1be40863405f374E7f5F8806c728660A | ||
| /// @dev Upgradeable contract so address unlikely to change | ||
| address public constant MAINNET_TOKEN_POOL = 0x06179f7C1be40863405f374E7f5F8806c728660A; |
There was a problem hiding this comment.
we must fetch this from the router
Contributor
Author
There was a problem hiding this comment.
this isn't available in the router, didn't know where to fetch it from
There was a problem hiding this comment.
router.getOnRamp(destionationChainSelector).getPoolBySourceToken(destionationChainSelector, GHO_TOKEN)
| * @param messageId The ID of the cross-chain message | ||
| * @param destinationChainSelector The selector of the destination chain | ||
| * @param from The address of sender on source chain | ||
| * @param amount The total amount of GHO transfered |
There was a problem hiding this comment.
Suggested change
| * @param amount The total amount of GHO transfered | |
| * @param amount The total amount of GHO transferred |
Comment on lines
9
to
11
| import {RescuableBase, IRescuableBase} from 'solidity-utils/contracts/utils/RescuableBase.sol'; | ||
|
|
||
| import {Client} from 'src/dependencies/chainlink/libraries/Client.sol'; |
There was a problem hiding this comment.
Suggested change
| import {RescuableBase, IRescuableBase} from 'solidity-utils/contracts/utils/RescuableBase.sol'; | |
| import {Client} from 'src/dependencies/chainlink/libraries/Client.sol'; | |
| import {RescuableBase, IRescuableBase} from 'solidity-utils/contracts/utils/RescuableBase.sol'; | |
| import {Client} from 'src/dependencies/chainlink/libraries/Client.sol'; |
|
|
||
| /// @dev Map containing failed token transfer amounts for a message | ||
| mapping(bytes32 messageId => Client.EVMTokenAmount[] destTokenAmounts) | ||
| private _failedTokenTransfers; |
There was a problem hiding this comment.
Suggested change
| private _failedTokenTransfers; | |
| internal _failedTokenTransfers; |
internal is fine, same with _failedMessages
| uint256 fee = IRouterClient(ROUTER).getFee(chainSelector, message); | ||
|
|
||
| if (feeToken == address(0)) { | ||
| if (msg.value < fee) revert InsufficientFee(); |
There was a problem hiding this comment.
sounds good. However, if the purpose of the contract is to be pre-funded always, we can remove this logic of pulling funds from sender and the payable (we should add a receive instead)
| // Only applicable to Mainnet | ||
| // Should be unreachable as it's unlikely to be lower than the rate limit | ||
| if (block.chainid == 1) { | ||
| uint256 bridgeLimit = ITokenPool(MAINNET_TOKEN_POOL).getBridgeLimit() - |
There was a problem hiding this comment.
Suggested change
| uint256 bridgeLimit = ITokenPool(MAINNET_TOKEN_POOL).getBridgeLimit() - | |
| uint256 availableToBridge = ITokenPool(MAINNET_TOKEN_POOL).getBridgeLimit() - |
| } | ||
|
|
||
| // Only applicable to Mainnet | ||
| // Should be unreachable as it's unlikely to be lower than the rate limit |
There was a problem hiding this comment.
this is wrong, because the rate limit gets replenished based on the velocity (token per second), and bridge limit does not.
| */ | ||
| function destinations( | ||
| uint64 chainSelector | ||
| ) external view returns (bytes memory, bytes memory, uint32); |
There was a problem hiding this comment.
weird that we are exposing 3 bytes when we have our own struct for this (RemoteChainConfig) here, and exposing a CCIP struct (Client.EVMTokenAmount[]) on getInvalidMessage. From the integration pov i think it d be better to use our own struct for getInvalidMessage so it's easy to consume.
| transferOwnership(initialOwner); | ||
| } | ||
|
|
||
| /// @dev Default receive function so the contract can be sent Ether |
There was a problem hiding this comment.
Suggested change
| /// @dev Default receive function so the contract can be sent Ether | |
| /// @dev Default receive function enabling the contract to accept native tokens |
| address public immutable COLLECTOR; | ||
|
|
||
| /// @dev Map containing destination chain's configuration | ||
| mapping(uint64 => RemoteChainConfig) internal destinations; |
There was a problem hiding this comment.
Suggested change
| mapping(uint64 => RemoteChainConfig) internal destinations; | |
| mapping(uint64 => RemoteChainConfig) internal _destinations; |
| gasLimit: gasLimit | ||
| }); | ||
|
|
||
| emit DestinationChainSet(chainSelector, destination, gasLimit); |
| extraArgs: remoteConfig.extraArgsOverride.length > 0 | ||
| ? remoteConfig.extraArgsOverride | ||
| : Client._argsToBytes( | ||
| Client.GenericExtraArgs({gasLimit: remoteConfig.gasLimit, allowOutOfOrderExecution: true}) |
There was a problem hiding this comment.
so how gasLimit would work here? it will always use the configured value and never the default?
Contributor
Author
There was a problem hiding this comment.
Yes, it will use the configured, I ran tests with chainlink and not using the default for EVM chains would save > 100,000 gas.
Contributor
Author
There was a problem hiding this comment.
I added to the README a description on gas to help set it
src/bridges/ccip/README.md
Outdated
|
|
||
| ### Destination Addresses | ||
|
|
||
| The destination chain address must be provided as bytes. For EVM chains, convert the address to bytes with `abi.encode(address)`, where address if an EVM address. For Solana/Aptos, `abi.encode(bytes32)` where bytes32 is the Aptos/Solana address. |
There was a problem hiding this comment.
We can remove this paragraph, favoring the one below
| /** | ||
| * Struct representing a destination chain | ||
| * @return extraArgsOverride Any extra arguments to pass with message to the destination chain | ||
| * @return bridge The bytes representation of an address (EVM or non-EVM) |
There was a problem hiding this comment.
this should be destination and should be the first return
| /// @dev if the receiver is a contracts that signals support for CCIP execution through EIP-165. | ||
| /// the contract is called. If not, only tokens are transferred. | ||
| /// @return success A boolean value indicating whether the ccip message was received without errors. | ||
| /// @return retBytes A bytes array containing return data form CCIP receiver. |
There was a problem hiding this comment.
Suggested change
| /// @return retBytes A bytes array containing return data form CCIP receiver. | |
| /// @return retBytes A bytes array containing return data from CCIP receiver. |
|
|
||
| /// @param destinationChainSelector The destination chainSelector. | ||
| /// @param message The cross-chain CCIP message including data and/or tokens. | ||
| /// @return fee returns execution fee for the message. |
There was a problem hiding this comment.
Suggested change
| /// @return fee returns execution fee for the message. | |
| /// @return fee returns execution fee for the message |
Looks like the sentence is meant to be continued below
There was a problem hiding this comment.
Ah, sry, this is a chainlink dep. So all good!
| /// @param destinationChainSelector The destination chain ID. | ||
| /// @param message The cross-chain CCIP message including data and/or tokens. | ||
| /// @return messageId The message ID. | ||
| /// @dev Note if msg.value is larger than the required fee (from getFee) we accept. |
There was a problem hiding this comment.
Suggested change
| /// @dev Note if msg.value is larger than the required fee (from getFee) we accept. | |
| /// @dev Note if msg.value is larger than the required fee (from getFee) we accept |
Same, sentence continued in next line
brotherlymite
approved these changes
Oct 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelog
Add CCIP Bridge for Aave to transfer GHO from mainnet to pre-defined networks.
Add fork tests.
Add unit tests.