add IMessageTransformer interface to CCIP onramp and offramp #15849

jinhoonbang · 2025-01-07T04:27:26Z

add hooks to CCIP onramp and offramps for pre/post-processing messages using custom transformer
- introduces MessageTransformerOnRamp/OffRamps which inherit canonical on/offRamps

github-actions · 2025-01-07T04:28:54Z

Static analysis results are available

Hey @Rishabh570, you can view Slither reports in the job summary here or download them as artifact here.
Please check them before merging and make sure you have addressed all issues.

github-actions · 2025-01-07T04:46:23Z

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

….6 on/off ramps

…it/chainlink into ccip-message-transformers

github-actions · 2025-01-09T21:28:59Z

I see you updated files related to contracts. Please run pnpm changeset in the contracts directory to add a changeset.

RensR · 2025-01-13T10:38:03Z

contracts/scripts/native_solc_compile_all_ccip

 compileContract() {
  local contract
  contract=$(basename "$1")
  echo "Compiling" "$contract"

+  local viaIR=""


Every contract already compiled with via-ir enabled. See the ccip-compile foundry profile

got it. removing this logic

RensR · 2025-01-13T10:39:54Z

contracts/src/v0.8/ccip/interfaces/IMessageTransformer.sol

+import {Internal} from "../libraries/Internal.sol";
+
+/// @notice Interface for plug-in message hook contracts that transform OffRamp & OnRamp messages.
+///         The transformer functions are expected to revert on transform failures.


nit: start the comment at the start of the line.
nit: what does plug-in mean here? I think the comment would be better without it

makes sense. removed plug-in

RensR · 2025-01-13T10:41:34Z

contracts/src/v0.8/ccip/interfaces/IMessageTransformer.sol

+  /// @notice Transforms the given OffRamp message. Reverts on transform failure
+  /// @param message to transform
+  /// @return transformed message
+  function transformInboundMessage(


nit switch the two function to follow the msg lifecycle.

Can you add something to the comments why this is useful or what type of transformations could exist?

RensR · 2025-01-13T10:43:42Z

contracts/src/v0.8/ccip/offRamp/MessageTransformerOffRamp.sol

+import {OffRamp} from "./OffRamp.sol";
+
+/// @notice OffRamp that uses a message transformer to transform messages before execution
+contract MessageTransformerOffRamp is OffRamp {


Can we name them OffRampWithMsgTransformer instead? They're primarily a ramp so the name should reflect that imo

RensR · 2025-01-13T10:44:14Z

contracts/src/v0.8/ccip/offRamp/MessageTransformerOffRamp.sol

+  }
+
+  function getMessageTransformerAddress() external view returns (address) {
+    return s_messageTransformer;


missing setMessageTransformer

Is it needed?

When a new ramp needs to be introduced, do we usually deploy a new ramp and associate it with existing and other dependencies? If so, I believe we can follow the same convention when a new message transformer is needed and just redeploy a new ramp with new message transformer.

RensR · 2025-01-13T10:52:36Z

...acts/src/v0.8/ccip/test/offRamp/OffRamp/MessageTransformerOffRamp.executeSingleMessage.t.sol

+  }
+
+  function test_executeSingleMessage_WithMessageTransformer() public {
+    vm.stopPrank();


No need to stop the prank, startPrank overrides it anyway. Same in other places. Generally if all tests start with the same startPrank you can put it in the setUp

RensR · 2025-01-13T10:53:47Z

contracts/src/v0.8/ccip/test/offRamp/OffRamp/OffRampSetup.t.sol

@@ -53,6 +55,7 @@ contract OffRampSetup is FeeQuoterSetup, MultiOCR3BaseSetup {
    MultiOCR3BaseSetup.setUp();

    s_inboundMessageInterceptor = new MessageInterceptorHelper();
+    s_inboundMessageTransformer = new MessageTransformerHelper();


Let's keep these clean from setup they don't need. The testsuite is already pretty slow

RensR · 2025-01-13T10:54:08Z

contracts/src/v0.8/ccip/test/onRamp/OnRamp/MessageTransformerOnRamp.forwardFromRouter.t.sol

+pragma solidity ^0.8.24;
+
+import {AuthorizedCallers} from "../../../../shared/access/AuthorizedCallers.sol";
+import {IERC20} from "../../../../vendor/openzeppelin-solidity/v4.8.3/contracts/interfaces/IERC20.sol";


nit: vendor goes at the end below a newline, see the style guide

RensR · 2025-01-13T10:55:18Z

contracts/src/v0.8/ccip/test/onRamp/OnRamp/MessageTransformerOnRamp.forwardFromRouter.t.sol

+    vm.startPrank(address(s_sourceRouter));
+  }
+
+  function test_forwardFromRouter_WithMessageTransformer_Success() public {


please follow the new test naming convention for all new tests. See the Foundry guide for the specification. Not all tests in ccip follow it right now but we're slowly replacing them all.

yep change function names to follow the convention here

RensR · 2025-01-13T10:55:50Z

contracts/src/v0.8/ccip/test/onRamp/OnRamp/MessageTransformerOnRamp.forwardFromRouter.t.sol

+    uint256 feeAmount = 1234567890;
+    message.tokenAmounts = new Client.EVMTokenAmount[](1);
+    message.tokenAmounts[0].amount = 1e18;
+    message.tokenAmounts[0].token = s_sourceTokens[0];


Does this setup actually meaningfully impact the test being done? If not, please remove

contracts/src/v0.8/ccip/onRamp/OnRampWithMessageTransformer.sol

RensR · 2025-01-15T12:14:11Z

.../src/v0.8/ccip/test/offRamp/OffRamp/OffRampWithMessageTransformer.executeSingleMessage.t.sol

@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity ^0.8.24;
+


missing tests for the new setMessageTransformer functions

added in separate files

cl-sonarqube-production · 2025-01-17T11:20:00Z

Quality Gate failed

Failed conditions
16.57% Technical Debt Ratio on New Code (required ≤ 4%)
C Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

RensR · 2025-01-20T09:42:54Z

...s/src/v0.8/ccip/test/offRamp/OffRamp/OffRampWithMessageTransformer.executeSingleReport.t.sol

+import {Internal} from "../../../libraries/Internal.sol";
+
+import {OffRamp} from "../../../offRamp/OffRamp.sol";
+


newlines have meaning in imports, please remove follow the styleguide

<interfaces> <other> <third party>

Please check other files as well

…gMsgDataReceiver

RensR · 2025-01-20T12:01:33Z

core/gethwrappers/ccip/generation/generated-wrapper-dependency-versions-do-not-edit.txt

@@ -2,21 +2,27 @@ GETH_VERSION: 1.14.11
 burn_from_mint_token_pool: ../../../contracts/solc/ccip/BurnFromMintTokenPool/BurnFromMintTokenPool.sol/BurnFromMintTokenPool.abi.json ../../../contracts/solc/ccip/BurnFromMintTokenPool/BurnFromMintTokenPool.sol/BurnFromMintTokenPool.bin ae4e15dc926517092d46e108cd5e24863d58e689444ce310bb00c1390f711ba9
 burn_mint_token_pool: ../../../contracts/solc/ccip/BurnMintTokenPool/BurnMintTokenPool.sol/BurnMintTokenPool.abi.json ../../../contracts/solc/ccip/BurnMintTokenPool/BurnMintTokenPool.sol/BurnMintTokenPool.bin 7360dc05306d51b247abdf9a3aa8704847b1f4fb91fdb822a2dfc54e1d86cda1
 burn_with_from_mint_token_pool: ../../../contracts/solc/ccip/BurnWithFromMintTokenPool/BurnWithFromMintTokenPool.sol/BurnWithFromMintTokenPool.abi.json ../../../contracts/solc/ccip/BurnWithFromMintTokenPool/BurnWithFromMintTokenPool.sol/BurnWithFromMintTokenPool.bin 66715c303bb2da2b49bba100a788f6471b0f94d255d40f92306e279b909ae33b
+ccip_dummy_receiver: ../../../contracts/solc/ccip/CCIPDummyReceiver/CCIPDummyReceiver.sol/CCIPDummyReceiver.abi.json ../../../contracts/solc/ccip/CCIPDummyReceiver/CCIPDummyReceiver.sol/CCIPDummyReceiver.bin e6c3b994b7d3fe7521e6750939447b4da5f96d9ce9fb47c7e7f4d7e7c605277c


Please remove

add IMessageTransformer interface to CCIP onramp and offramp

f84bc28

jinhoonbang requested review from masha256 and Rishabh570 January 7, 2025 04:27

jinhoonbang added the do not merge label Jan 7, 2025

jinhoonbang and others added 5 commits January 9, 2025 12:39

create message transformer on/off ramps that inherit from canonical 1…

5d5e85d

….6 on/off ramps

Merge 5d5e85d into 8811103

f3be5f5

Update gethwrappers

b950b59

fix return values for transformer on off ramps

16febd5

Merge branch 'ccip-message-transformers' of github.com:smartcontractk…

9653f03

…it/chainlink into ccip-message-transformers

jinhoonbang added 2 commits January 9, 2025 13:48

generate gethwrappers for message transformer on off ramps

1077d42

fix linting issue

05f3201

jinhoonbang force-pushed the ccip-message-transformers branch from 123c8c8 to 05f3201 Compare January 9, 2025 21:55

jinhoonbang added 3 commits January 9, 2025 14:20

forge fmt and snapshot

74e2f05

update snapshot

e4b82f7

update comments

064f508

jinhoonbang removed the do not merge label Jan 9, 2025

jinhoonbang marked this pull request as ready for review January 9, 2025 23:47

jinhoonbang requested review from a team as code owners January 9, 2025 23:47

jinhoonbang requested a review from RensR January 9, 2025 23:48

regenerate message transformer offramp with correct configurations

d6172be

RensR reviewed Jan 13, 2025

View reviewed changes

jinhoonbang added 4 commits January 13, 2025 18:31

address comments

6130f3e

Merge branch 'develop' into ccip-message-transformers

6e51471

regen wrappers with correct name

8d0304e

forge fmt

4331c9a

jinhoonbang requested a review from RensR January 14, 2025 05:12

RensR reviewed Jan 15, 2025

View reviewed changes

contracts/src/v0.8/ccip/onRamp/OnRampWithMessageTransformer.sol Show resolved Hide resolved

RensR reviewed Jan 15, 2025

View reviewed changes

jinhoonbang added 6 commits January 15, 2025 21:03

Merge branch 'develop' into ccip-message-transformers

dd911da

address comments

d44604e

add natspec comments everywhere

b007ae0

forge fmt

0a14201

casing

d511cd1

fix casing

b28a251

jinhoonbang force-pushed the ccip-message-transformers branch from 8360166 to 6d2b8fe Compare January 16, 2025 07:23

fix casing

3dac425

jinhoonbang force-pushed the ccip-message-transformers branch from 6d2b8fe to 3dac425 Compare January 16, 2025 07:27

rename test file

f5c71d4

jinhoonbang requested a review from RensR January 16, 2025 07:32

Rishabh570 and others added 9 commits January 16, 2025 13:55

adds dummy ccip receiver to emit received data

5b724cf

use interface for CCIP receiver in CCIPChainState

6b56e2c

updates CCIPDummyReceiver

d92b242

move offramp hook prior to nonce logic

673cbb0

Merge branch 'develop' into ccip-message-transformers

414a999

fix unit tests

181f673

forge fmt

3bfe9ce

adds new receiver state in CCIPChainState

3a3b4ee

use new type for ReceiverWithMsgData

976eaab

RensR reviewed Jan 20, 2025

View reviewed changes

Rishabh570 added 2 commits January 20, 2025 16:51

remove CCIPDummyReceiver contract and implement ITypeAndVersion in Lo…

65356b7

…gMsgDataReceiver

import styleguide updates

dbd9934

Rishabh570 requested a review from RensR January 20, 2025 11:53

RensR reviewed Jan 20, 2025

View reviewed changes

updates LogMessageDataReceiver contract

f282823

RensR approved these changes Jan 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add IMessageTransformer interface to CCIP onramp and offramp #15849

add IMessageTransformer interface to CCIP onramp and offramp #15849

jinhoonbang commented Jan 7, 2025 •

edited

Loading

github-actions bot commented Jan 7, 2025 •

edited

Loading

github-actions bot commented Jan 7, 2025 •

edited

Loading

github-actions bot commented Jan 9, 2025

RensR Jan 13, 2025

jinhoonbang Jan 14, 2025

RensR Jan 13, 2025

jinhoonbang Jan 13, 2025

RensR Jan 13, 2025

jinhoonbang Jan 14, 2025

RensR Jan 13, 2025

RensR Jan 13, 2025

jinhoonbang Jan 13, 2025

RensR Jan 13, 2025

RensR Jan 13, 2025

jinhoonbang Jan 14, 2025

RensR Jan 13, 2025

RensR Jan 13, 2025

jinhoonbang Jan 14, 2025

RensR Jan 13, 2025

RensR Jan 15, 2025

jinhoonbang Jan 16, 2025

cl-sonarqube-production bot commented Jan 17, 2025

RensR Jan 20, 2025 •

edited

Loading

RensR Jan 20, 2025

		@@ -0,0 +1,84 @@
		// SPDX-License-Identifier: BUSL-1.1
		pragma solidity ^0.8.24;

		import {Internal} from "../../../libraries/Internal.sol";

		import {OffRamp} from "../../../offRamp/OffRamp.sol";

add IMessageTransformer interface to CCIP onramp and offramp #15849

Are you sure you want to change the base?

add IMessageTransformer interface to CCIP onramp and offramp #15849

Conversation

jinhoonbang commented Jan 7, 2025 • edited Loading

github-actions bot commented Jan 7, 2025 • edited Loading

Static analysis results are available

github-actions bot commented Jan 7, 2025 • edited Loading

AER Report: CI Core ran successfully ✅

AER Report: Operator UI CI ran successfully ✅

github-actions bot commented Jan 9, 2025

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

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

cl-sonarqube-production bot commented Jan 17, 2025

Quality Gate failed

RensR Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jinhoonbang commented Jan 7, 2025 •

edited

Loading

github-actions bot commented Jan 7, 2025 •

edited

Loading

github-actions bot commented Jan 7, 2025 •

edited

Loading

RensR Jan 20, 2025 •

edited

Loading