Forwarder #1179

augustbleeds · 2025-04-10T18:28:09Z

Description

Design of forwarder based off of #614

EDIT: please see the committed README.md docs for more info

instructions:

initialize: creates a new forwarder state, and precomputes the bump for the forwarder authority pda which signs the CPI to the receiver
transfer/accept_ownership: self-explanatory
init/update/close_oracles_config: oracle configs (which are called oracle sets in EVM) are stored as separate data accounts since a new config version / config_id with the same DON would create a new oracle config so the size is unbounded (as opposed to finding a soft-limit on the new dons that are created)
report: processes the report which is of format data = bump | len_signatures (N) | signatures (N*65) | raw_report (M) | report_context (96). A report will create a new execution_state pda data account if doesn't exist via create_account instruction. It executes a CPI to a receiver program where the forwarder authority pda is the signer.

Requires Dependencies

Resolves Dependencies

dhaidashenko · 2025-04-14T11:49:35Z

contracts/programs/keystone-forwarder/src/lib.rs

+
+    use super::*;
+
+    pub fn initialize(ctx: Context<Initialize>) -> Result<()> {


Curious about what steps we need to perform for deployment. Is it

Deploy the program.

Initialize (can we do it as part of the deployment transaction?)

Init oracles config

Transfer ownership to MCMS

Accept ownership on MCMS

contracts/programs/keystone-forwarder/src/lib.rs

dhaidashenko · 2025-04-14T12:06:11Z

contracts/programs/keystone-forwarder/src/lib.rs

+#[account]
+#[derive(Default)]
+#[derive(InitSpace)]
+pub struct ExecutionState {


I wonder how we can verify if the report was forwarded on the WriteTarget level

can you elaborate on the scenario? we can add an event to the report function which you can listen to or you can query the execution status account associated with the transmission

contracts/programs/keystone-forwarder/src/lib.rs

archseer · 2025-04-15T01:19:26Z

contracts/programs/keystone-forwarder/src/lib.rs

+pub struct OraclesConfig {
+    config_id: u64,
+    f: u8,
+    signer_addresses: Vec<EthAddress>, // maybe think about doing ECDSA over ed25519?


Personally I'd use a constant size array plus a length int and make the type zero_copy

@chray-zhang

archseer · 2025-04-15T04:02:51Z

contracts/programs/keystone-forwarder/src/lib.rs

+    let data = &data[2..];
+    let _signatures = &data[..65*len];
+    let data = &data[65*len..];
+    let _report_context = &data[data.len()-96..];


I think it would make more sense to have the report context before the report

I kept it this way because the existing solana keyring assumes the report context comes after https://github.com/smartcontractkit/chainlink/blob/develop/core/services/keystore/keys/ocr2key/solana_keyring.go#L38 . But open to changing it as well.

We should be using ocr2 signatures, not ocr3 right?

contracts/programs/keystone-forwarder/src/lib.rs

archseer · 2025-04-15T04:06:37Z

contracts/programs/keystone-forwarder/src/lib.rs

+        prev_signer = curr_signer;
+    }
+
+    oracles_config.config_id = ((don_id as u64) << 32) | (config_version as u64);


This should use get_config_id

archseer · 2025-04-15T04:07:13Z

contracts/programs/keystone-forwarder/src/lib.rs

+}
+
+fn get_config_id(don_id: u32, config_version: u32) -> u64 {
+    ((don_id as u64) << 32) | (config_version as u64)


Config id could technically just be a struct with two u32 fields, it would have the same underlying size

silaslenihan · 2025-05-19T12:17:27Z

contracts/programs/keystone-forwarder/src/lib.rs

+) -> Result<()> {
+    // ensure MAX_SIGNERS fit in the bits of uniques
+    let mut uniques: u32 = 0;
+    assert!(uniques.count_ones() + uniques.count_zeros() >= MAX_ORACLES as u32);


better to use require! and a custom error here?

silaslenihan · 2025-05-19T12:18:40Z

contracts/programs/keystone-forwarder/src/utils.rs

+
+pub fn extract_config_id(raw_report: &[u8]) -> [u8; 8] {
+    // don_id | don_config_version
+    raw_report[37..45].try_into().expect("Expected 8 bytes")


Another case where this will panic, would it be better to guard this with require!

good check, this something we have a ticket for @chray-zhang will pick up some of the small improvements

silaslenihan · 2025-05-19T12:28:01Z

docs/forwarder/README.md

+
+`forwarder_authority` is the PDA which is responsible for signing the CPI to a receiver program via invoke_signed()
+
+`execution_state` is the PDA which stores a report's execution status. For now, it only stores successes.


how will offchain components (writetarget) know when a transmission is failed / fatal?

Good question. This is something that the platform expansion team will need to add in later iterations. I think the body of work is too large for our timeline at the moment. But I have some thoughts here. I think the first option is the most robust, but we have to have a new capabilities for that to happen.

As it is right now, the write targeter will just keep on trying until it reaches the max tries and then stop.

cl-sonarqube-production · 2025-05-22T19:40:39Z

Quality Gate passed

Issues
2 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

augustbleeds requested a review from a team as a code owner April 10, 2025 18:28

augustbleeds temporarily deployed to integration April 10, 2025 18:28 — with GitHub Actions Inactive

augustbleeds had a problem deploying to integration April 10, 2025 18:28 — with GitHub Actions Failure

augustbleeds temporarily deployed to integration April 10, 2025 18:28 — with GitHub Actions Inactive

dhaidashenko reviewed Apr 14, 2025

View reviewed changes

archseer reviewed Apr 15, 2025

View reviewed changes

contracts/programs/keystone-forwarder/src/lib.rs Outdated Show resolved Hide resolved

archseer reviewed Apr 15, 2025

View reviewed changes

archseer requested review from toblich, PabloMansanet and agusaldasoro April 15, 2025 04:07

augustbleeds temporarily deployed to integration April 18, 2025 19:31 — with GitHub Actions Inactive

augustbleeds had a problem deploying to integration April 18, 2025 19:31 — with GitHub Actions Failure

augustbleeds temporarily deployed to integration April 18, 2025 19:31 — with GitHub Actions Inactive

augustbleeds temporarily deployed to integration April 21, 2025 17:21 — with GitHub Actions Inactive

augustbleeds had a problem deploying to integration April 21, 2025 17:21 — with GitHub Actions Failure

augustbleeds temporarily deployed to integration April 22, 2025 19:46 — with GitHub Actions Inactive

augustbleeds had a problem deploying to integration April 22, 2025 19:46 — with GitHub Actions Failure

augustbleeds temporarily deployed to integration April 22, 2025 19:46 — with GitHub Actions Inactive

augustbleeds had a problem deploying to integration April 22, 2025 19:46 — with GitHub Actions Failure

augustbleeds temporarily deployed to integration April 22, 2025 19:46 — with GitHub Actions Inactive

augustbleeds temporarily deployed to integration April 22, 2025 19:54 — with GitHub Actions Inactive

augustbleeds temporarily deployed to integration May 14, 2025 18:24 — with GitHub Actions Inactive

silaslenihan reviewed May 19, 2025

View reviewed changes

chray-zhang self-requested a review May 20, 2025 15:27

chray-zhang approved these changes May 20, 2025

View reviewed changes

Merge branch 'develop' into augustus.NONEVM-1516.forwarder

92be3de

augustbleeds temporarily deployed to integration May 20, 2025 15:28 — with GitHub Actions Inactive

augustbleeds had a problem deploying to integration May 20, 2025 15:28 — with GitHub Actions Failure

augustbleeds temporarily deployed to integration May 20, 2025 15:28 — with GitHub Actions Inactive

augustbleeds had a problem deploying to integration May 20, 2025 15:56 — with GitHub Actions Failure

chray-zhang mentioned this pull request May 21, 2025

Adding events to forwarder contract #1251

Open

Merge branch 'develop' into augustus.NONEVM-1516.forwarder

57dae86

augustbleeds temporarily deployed to integration May 22, 2025 16:00 — with GitHub Actions Inactive

augustbleeds temporarily deployed to integration May 22, 2025 16:09 — with GitHub Actions Inactive

Merge branch 'develop' into augustus.NONEVM-1516.forwarder

252c1f0

augustbleeds temporarily deployed to integration May 22, 2025 19:27 — with GitHub Actions Inactive

augustbleeds temporarily deployed to integration May 22, 2025 19:32 — with GitHub Actions Inactive

augustbleeds temporarily deployed to integration May 22, 2025 19:33 — with GitHub Actions Inactive


		use super::*;

		pub fn initialize(ctx: Context<Initialize>) -> Result<()> {


		`forwarder_authority` is the PDA which is responsible for signing the CPI to a receiver program via invoke_signed()

		`execution_state` is the PDA which stores a report's execution status. For now, it only stores successes.

Forwarder #1179

Are you sure you want to change the base?

Forwarder #1179

Uh oh!

Conversation

augustbleeds commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Requires Dependencies

Resolves Dependencies

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cl-sonarqube-production bot commented May 22, 2025

Quality Gate passed

Uh oh!

Uh oh!

augustbleeds commented Apr 10, 2025 •

edited

Loading