Solana todos #16391

yashnevatia · 2025-02-13T16:58:57Z

Using a common ValidateRamp func instead of ValidateOffRamp
Adding check for isOCR3ConfigSet for solana
Adding changeset to set fee aggregator for solana
Adding support for Solana -> EVM
- Changeset modification
- test_helpers modification
Removing some TODOs that are already complete
Adding AddTokenPool test for WSOL
Adding validation for token before minting

This reverts commit aaab52e.

…develop

deployment/ccip/changeset/solana/cs_billing.go

deployment/ccip/changeset/cs_chain_contracts.go

AnieeG · 2025-02-14T18:33:16Z

deployment/ccip/changeset/state.go

@@ -812,7 +812,7 @@ func LoadChainState(ctx context.Context, chain deployment.Chain, addresses map[s
 	return state, nil
 }

-func (s CCIPOnChainState) ValidateOffRamp(chainSelector uint64) error {
+func (s CCIPOnChainState) ValidateRamp(chainSelector uint64, rampType deployment.ContractType) error {


nit : not for this PR but it will probably help if we extend it to all contracts like you did for Ramps. It can take a slice of deployment.ContractType which will ensure all the types passed in param are non-empty in state

AnieeG · 2025-02-14T18:34:52Z

deployment/ccip/changeset/cs_chain_contracts.go

+			sourceChainFamily, _ := chain_selectors.GetSelectorFamily(source)
+
+			onRampBytes := []byte{}
+			if sourceChainFamily == chain_selectors.FamilyEVM {
+				onRampBytes = state.Chains[source].OnRamp.Address().Bytes()
+			} else if sourceChainFamily == chain_selectors.FamilySolana {
+				// for solana the router contract has the onramp logic
+				onRampBytes = state.SolChains[source].Router.Bytes()
+			}
+


let's have a method on state to return this something like - https://github.com/smartcontractkit/chainlink/blob/develop/deployment/ccip/changeset/state.go#L166C25-L166C41

AnieeG · 2025-02-14T18:35:37Z

deployment/ccip/changeset/solana/cs_add_remote_chain.go

+		}
+		var destChainStateAccount solRouter.DestChain
+		err = chain.GetAccountDataBorshInto(context.Background(), routerDestChainPDA, &destChainStateAccount)
+		if err == nil {


is this right condition?

yes, if the err is nil, that means the remote chain has already been configured and the routerDestChainPDA has already been initialised.
If the err is not nil (err="account not found"), that means the remote chain has not already been initialised and we can proceed with doing the same.

AnieeG · 2025-02-14T18:38:02Z

deployment/ccip/changeset/solana/cs_add_remote_chain.go

-			return fmt.Errorf("support for solana chain as remote chain is not implemented yet %d", remoteChainSel)
+			onRampAddress := s.SolChains[remoteChainSel].Router.String()
+			addressBytes := []byte(onRampAddress)
+			copy(onRampBytes[:], addressBytes)


unrelated to this PR - considering this is specifically for solana do you still need the switch case?

yes because the remoteChainSelector can be solana or evm.
we are considering solana as a family where other solana chains might be added in the future.

I actually had to modify this for it to work with the node: e7de104

I think this field should use the first byte to denote the length of the data, that way the padding can be trimmed in a chain agnostic way

did the padding here

AnieeG · 2025-02-14T18:52:08Z

deployment/ccip/changeset/solana/cs_chain_contracts_test.go

+		deployment.SPLTokens:     state.SolChains[solChain].WSOL,
+	}
+
+	for tokenProgramName, tokenAddress := range tokenMap {


nit : how about you update the CS input to receive multiple token programs and token addresses. IRL you might also need to do it for multiple tokens

i am refraining from doing that for token pool changesets as we should not be deploying more than a couple.
will leave as is for now if thats ok, and will add a couple of loops if we end up deploying for third parties.

deployment/ccip/changeset/solana/cs_deploy_chain.go

AnieeG · 2025-02-14T18:53:33Z

deployment/ccip/changeset/solana/cs_deploy_chain.go

+}
+
+func (cfg SetFeeAggregatorConfig) Validate(e deployment.Environment) error {
+	state, _ := cs.LoadOnchainState(e)


why skipping the errcheck

AnieeG · 2025-02-14T18:54:05Z

deployment/ccip/changeset/solana/cs_deploy_chain.go

+	chainState := state.SolChains[cfg.ChainSelector]
+	chain := e.SolChains[cfg.ChainSelector]


need to check the existence of these chains in state

AnieeG · 2025-02-14T18:56:50Z

deployment/ccip/changeset/testhelpers/test_helpers.go

-							IsEnabled:        true,
-							TestRouter:       isTestRouter,
-							AllowListEnabled: false,
+	if fromFamily == chainsel.FamilyEVM {


will it be clean to have sep methods for diff EVM<> Solana combination?

have made it cleaner by wrapping src/dest changesets in separate functions.

tt-cll · 2025-02-15T08:16:06Z

We should also address https://github.com/smartcontractkit/chainlink/pull/16324/files#r1952898111

yashnevatia · 2025-02-17T13:50:14Z

We should also address https://github.com/smartcontractkit/chainlink/pull/16324/files#r1952898111

setTokenPoolCounterPart this is a test function which will not be hit if the chains are not supported as we try to deploy token pools to these chains before calling this function.

github-actions · 2025-02-17T14:28:36Z

Flakeguard Summary

Ran new or updated tests between develop and 94a7d69 (solana-todos).

View Flaky Detector Details | Compare Changes

Found Flaky Tests ❌

2 Results

Name	Pass Ratio	Panicked?	Timed Out?	Race?	Runs	Successes	Failures	Skips	Package	Package Panicked?	Avg Duration	Code Owners
TestDeployerGroup	0%	true	true	false	2	0	2	0	github.com/smartcontractkit/chainlink/deployment/ccip/changeset	true	0s	@smartcontractkit/ccip-tooling, @smartcontractkit/ccip-offchain, @smartcontractkit/core, @smartcontractkit/deployment-automation
TestJobSpecChangesetIdempotent	66.6667%	true	true	false	3	2	1	0	github.com/smartcontractkit/chainlink/deployment/ccip/changeset	true	8.93s	@smartcontractkit/ccip-tooling, @smartcontractkit/ccip-offchain, @smartcontractkit/core, @smartcontractkit/deployment-automation

Artifacts

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

github-actions · 2025-02-17T15:53:02Z

Flakeguard Summary

Ran new or updated tests between develop and a352b09 (solana-todos).

View Flaky Detector Details | Compare Changes

Found Flaky Tests ❌

2 Results

Name	Pass Ratio	Panicked?	Timed Out?	Race?	Runs	Successes	Failures	Skips	Package	Package Panicked?	Avg Duration	Code Owners
TestSetPoolChangeset_Execution	66.6667%	true	true	false	3	2	1	0	github.com/smartcontractkit/chainlink/deployment/ccip/changeset	true	5.915s	@smartcontractkit/ccip-tooling, @smartcontractkit/ccip-offchain, @smartcontractkit/core, @smartcontractkit/deployment-automation
TestTransferAdminRoleChangeset_Execution	0%	true	true	false	2	0	2	0	github.com/smartcontractkit/chainlink/deployment/ccip/changeset	true	0s	@smartcontractkit/ccip-tooling, @smartcontractkit/ccip-offchain, @smartcontractkit/core, @smartcontractkit/deployment-automation

Artifacts

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

github-actions · 2025-02-17T16:36:25Z

Flakeguard Summary

Ran new or updated tests between develop and a9e966d (solana-todos).

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
TestDeployHomeChainIdempotent	66.6667%	true	true	false	3	2	1	0	github.com/smartcontractkit/chainlink/deployment/ccip/changeset	true	9.57s	@smartcontractkit/ccip-tooling, @smartcontractkit/ccip-offchain, @smartcontractkit/core, @smartcontractkit/deployment-automation

Artifacts

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

tt-cll · 2025-02-17T18:46:04Z

deployment/ccip/changeset/cs_add_lane_test.go

+	solSelectors := e.Env.AllChainSelectorsSolana()
+	solChain := solSelectors[0]
+	testhelpers.AddLaneWithDefaultPricesAndFeeQuoterConfig(t, &e, state, chain1, solChain, true)
+	// AddLaneWithDefaultPricesAndFeeQuoterConfig involves calling AddRemoteChainToSolana


Maybe we should make AddRemoteChainToSolana idempotent. Doesn't have to be in this PR

cl-sonarqube-production · 2025-02-18T14:03:34Z

Quality Gate passed

Issues
6 New issues
10 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

github-actions · 2025-02-18T14:32:06Z

Flakeguard Summary

Ran new or updated tests between develop and 9eee32a (solana-todos).

View Flaky Detector Details | Compare Changes

Found Flaky Tests ❌

2 Results

Name	Pass Ratio	Panicked?	Timed Out?	Race?	Runs	Successes	Failures	Skips	Package	Package Panicked?	Avg Duration	Code Owners
TestRemoveDons	66.6667%	true	true	false	3	2	1	0	github.com/smartcontractkit/chainlink/deployment/ccip/changeset	true	10.155s	@smartcontractkit/ccip-tooling, @smartcontractkit/ccip-offchain, @smartcontractkit/core, @smartcontractkit/deployment-automation
TestSetPoolChangeset_Execution	0%	true	true	false	2	0	2	0	github.com/smartcontractkit/chainlink/deployment/ccip/changeset	true	0s	@smartcontractkit/ccip-tooling, @smartcontractkit/ccip-offchain, @smartcontractkit/core, @smartcontractkit/deployment-automation

Artifacts

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

yashnevatia added 15 commits January 3, 2025 15:18

Adding solchains in NewEnv

aaab52e

Revert "Adding solchains in NewEnv"

f19598e

This reverts commit aaab52e.

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

87f6e75

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

4bf2980

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

6712ecd

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

a1e15fa

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

827ccd6

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

29f6dc4

…develop

remove todo

dfe582d

remove todo

4699cc9

add cs to set fee agg

9b53d65

add cs

3e6ce6a

address comments

484898f

validation on mint

d44f0fd

bump chains/solana

89569b4

yashnevatia requested review from a team as code owners February 13, 2025 16:58

yashnevatia requested review from EasterTheBunny, winder, asoliman92, makramkd, dimkouv, mateusz-sekara, 0xAustinWang and 0xnogo February 13, 2025 16:58

yashnevatia commented Feb 13, 2025

View reviewed changes

deployment/ccip/changeset/solana/cs_billing.go Show resolved Hide resolved

yashnevatia added 2 commits February 14, 2025 14:11

removing commented func

243089f

lint

e69844e

tt-cll reviewed Feb 14, 2025

View reviewed changes

deployment/ccip/changeset/cs_chain_contracts.go Show resolved Hide resolved

bug

1f6dd17

tt-cll previously approved these changes Feb 14, 2025

View reviewed changes

AnieeG reviewed Feb 14, 2025

View reviewed changes

tidying

a2e5f3e

yashnevatia dismissed tt-cll’s stale review via a2e5f3e February 17, 2025 13:04

yashnevatia added 2 commits February 17, 2025 13:20

add method to get onrampbytes

565af6e

making addlane modular

94a7d69

Reverting something

a352b09

fix

a9e966d

tt-cll reviewed Feb 17, 2025

View reviewed changes

tt-cll previously approved these changes Feb 17, 2025

View reviewed changes

onramp bytes

9eee32a

yashnevatia dismissed tt-cll’s stale review via 9eee32a February 18, 2025 13:50

archseer approved these changes Feb 18, 2025

View reviewed changes

ecPablo approved these changes Feb 18, 2025

View reviewed changes

tt-cll approved these changes Feb 18, 2025

View reviewed changes

tt-cll added this pull request to the merge queue Feb 18, 2025

Merged via the queue into develop with commit 537ef14 Feb 18, 2025
181 of 183 checks passed

tt-cll deleted the solana-todos branch February 18, 2025 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Solana todos #16391

Solana todos #16391

yashnevatia commented Feb 13, 2025 •

edited

Loading

AnieeG Feb 14, 2025

AnieeG Feb 14, 2025

yashnevatia Feb 17, 2025

AnieeG Feb 14, 2025

yashnevatia Feb 17, 2025

AnieeG Feb 14, 2025

yashnevatia Feb 17, 2025

archseer Feb 18, 2025

yashnevatia Feb 18, 2025 •

edited

Loading

AnieeG Feb 14, 2025

yashnevatia Feb 17, 2025

AnieeG Feb 14, 2025

yashnevatia Feb 17, 2025 •

edited

Loading

AnieeG Feb 14, 2025

yashnevatia Feb 17, 2025 •

edited

Loading

AnieeG Feb 14, 2025

yashnevatia Feb 17, 2025

yashnevatia Feb 17, 2025

tt-cll commented Feb 15, 2025 •

edited

Loading

yashnevatia commented Feb 17, 2025

github-actions bot commented Feb 17, 2025

github-actions bot commented Feb 17, 2025

github-actions bot commented Feb 17, 2025

tt-cll Feb 17, 2025 •

edited

Loading

cl-sonarqube-production bot commented Feb 18, 2025

github-actions bot commented Feb 18, 2025

		chainState := state.SolChains[cfg.ChainSelector]
		chain := e.SolChains[cfg.ChainSelector]

Solana todos #16391

Solana todos #16391

Conversation

yashnevatia commented Feb 13, 2025 • 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

yashnevatia Feb 18, 2025 • 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

yashnevatia Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yashnevatia Feb 17, 2025 • 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

tt-cll commented Feb 15, 2025 • edited Loading

yashnevatia commented Feb 17, 2025

github-actions bot commented Feb 17, 2025

Flakeguard Summary

Found Flaky Tests ❌

Artifacts

github-actions bot commented Feb 17, 2025

Flakeguard Summary

Found Flaky Tests ❌

Artifacts

github-actions bot commented Feb 17, 2025

Flakeguard Summary

Found Flaky Tests ❌

Artifacts

tt-cll Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

cl-sonarqube-production bot commented Feb 18, 2025

Quality Gate passed

github-actions bot commented Feb 18, 2025

Flakeguard Summary

Found Flaky Tests ❌

Artifacts

yashnevatia commented Feb 13, 2025 •

edited

Loading

yashnevatia Feb 18, 2025 •

edited

Loading

yashnevatia Feb 17, 2025 •

edited

Loading

yashnevatia Feb 17, 2025 •

edited

Loading

tt-cll commented Feb 15, 2025 •

edited

Loading

tt-cll Feb 17, 2025 •

edited

Loading