Skip to content

support token receiver in solana extra codec#17024

Merged
prashantkumar1982 merged 18 commits intodevelopfrom
NONEVM-1497/support-tokenReceiver-in-solana-extra-data-codec
Apr 8, 2025
Merged

support token receiver in solana extra codec#17024
prashantkumar1982 merged 18 commits intodevelopfrom
NONEVM-1497/support-tokenReceiver-in-solana-extra-data-codec

Conversation

@huangzhen1997
Copy link
Contributor

@huangzhen1997 huangzhen1997 commented Mar 29, 2025

Jira

Update Solana execute codec and msg hasher to use tokenReceiver address from extraArgs, instead of Msg.Receiver.

Slack reference

@huangzhen1997 huangzhen1997 marked this pull request as ready for review March 29, 2025 02:05
@huangzhen1997 huangzhen1997 requested a review from a team as a code owner March 29, 2025 02:05
…NONEVM-1497/support-tokenReceiver-in-solana-extra-data-codec
@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2025

Flakeguard Summary

Ran new or updated tests between develop and dfeaab4 (NONEVM-1497/support-tokenReceiver-in-solana-extra-data-codec).

View Flaky Detector Details | Compare Changes

Found Flaky Tests ❌

1 Results
Name Pass Ratio Panicked? Timed Out? Race? Runs Successes Failures Skips Package Package Panicked? Avg Duration Code Owners
TestMessageHasher_EVM2SVM 0% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/ccipsolana false 0s @smartcontractkit/ccip-offchain

Artifacts

For detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2025

Flakeguard Summary

Ran new or updated tests between develop and 2a7833b (NONEVM-1497/support-tokenReceiver-in-solana-extra-data-codec).

View Flaky Detector Details | Compare Changes

Found Flaky Tests ❌

1 Results
Name Pass Ratio Panicked? Timed Out? Race? Runs Successes Failures Skips Package Package Panicked? Avg Duration Code Owners
TestMessageHasher_EVM2SVM 0% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/ccipsolana false 0s @smartcontractkit/ccip-offchain

Artifacts

For detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2025

Flakeguard Summary

Ran new or updated tests between develop and b40e25d (NONEVM-1497/support-tokenReceiver-in-solana-extra-data-codec).

View Flaky Detector Details | Compare Changes

Found Flaky Tests ❌

1 Results
Name Pass Ratio Panicked? Timed Out? Race? Runs Successes Failures Skips Package Package Panicked? Avg Duration Code Owners
TestMessageHasher_EVM2SVM 0% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/ccipsolana false 0s @smartcontractkit/ccip-offchain

Artifacts

For detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json.

Sender: msg.Sender,
Data: msg.Data,
TokenReceiver: solana.PublicKeyFromBytes(msg.Receiver),
TokenReceiver: ed.tokenReceiver,
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is though, msg.Receiver needs to be part of the hash too

accounts := ed.accounts
// if logical receiver is empty, don't prepend it to the accounts list
if !msg.Receiver.IsZeroOrEmpty() {
accounts = append([]solana.PublicKey{solana.PublicKeyFromBytes(msg.Receiver)}, accounts...)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see, you're doing it here

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure where this validation should live, but if the receiver is empty then the accounts list needs to be empty too. Maybe this already happens in CW (receiver execution is skipped?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, we don't have that validation in CW. If logic receiver is empty, we just skip adding any accounts for messaging. We don't confirm that extraArgs.Accounts is empty

Copy link
Contributor

@archseer archseer Apr 8, 2025

Choose a reason for hiding this comment

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

I think that's the correct implementation though or we'll keep failing on these messages that were already committed. Validation should happen in the ccipSend logic somewhere here:

if (svmExtraArgsV1.accounts.length > Client.SVM_EXTRA_ARGS_MAX_ACCOUNTS) {
revert TooManySVMExtraArgsAccounts(svmExtraArgsV1.accounts.length, Client.SVM_EXTRA_ARGS_MAX_ACCOUNTS);
}

amit-momin
amit-momin previously approved these changes Apr 8, 2025
amit-momin
amit-momin previously approved these changes Apr 8, 2025
@cl-sonarqube-production
Copy link

@prashantkumar1982 prashantkumar1982 added this pull request to the merge queue Apr 8, 2025
Merged via the queue into develop with commit 4f6a888 Apr 8, 2025
197 of 199 checks passed
@prashantkumar1982 prashantkumar1982 deleted the NONEVM-1497/support-tokenReceiver-in-solana-extra-data-codec branch April 8, 2025 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments