AddChain inbound CCIP integration test #14377

connorwstein · 2024-09-09T20:35:30Z

CCIP-3062

Integrates the MCMS library for adding inbound lanes to a new chain. The test generates a proposal to enable a 4th chain while there are 3 pre-existing chains, signs and executes it, then sends traffic via the test router for the new lanes. Serves an example of how to use the MCMS library for future workflows.
Renaming PR -> FeeQuoter
Rename to OnRamp/OffRamp
Add test router deployment and dummy MCMS key
Extracting some test utilities

Follow ups:

Several TODOs around parameterizing MCMS inputs https://smartcontract-it.atlassian.net/browse/CCIP-3411
Enabling outbound from the new chain (2nd half of adding a new chain) https://smartcontract-it.atlassian.net/browse/CCIP-3412

…kflows-part2

AnieeG

There are FeeQuoter and PriceRegistry references used alternatively, can we make it uniform?

integration-tests/deployment/ccip/add_chain.go

integration-tests/deployment/ccip/deploy.go

connorwstein · 2024-09-12T18:27:10Z

integration-tests/deployment/ccip/changeset/2_initial_deploy.go

 	}
 	js, err := ccipdeployment.NewCCIPJobSpecs(env.NodeIDs, env.Offchain)
 	if err != nil {
 		return deployment.ChangesetOutput{}, err
 	}
-	proposal, err := ccipdeployment.GenerateAcceptOwnershipProposal(env, env.AllChainSelectors(), ab)


This can be its own changeset, probably too much to deploy all the contracts and accept ownership in the same one

connorwstein · 2024-09-12T18:30:01Z

integration-tests/deployment/ccip/deploy_home_chain.go

 	if _, err := deployment.ConfirmIfNoError(h, tx, err); err != nil {
 		return ccip_config.CCIPConfigTypesChainConfigInfo{}, err
 	}
 	lggr.Infow("Applied chain config updates", "chainConfig", chainConfig)
 	return chainConfig, nil
 }

-func AddDON(
+// AddDONToCR adds a DON with the given nodes to the capabilities registry.
+func BuildAddDONArgs(


Need helpers for constructing the args in the case where we want to use SimTransactOpts to build proposal data

connorwstein · 2024-09-12T18:31:17Z

integration-tests/deployment/ccip/deploy_home_chain.go

+	return encodedConfigs, nil
+}
+
+func LatestCCIPDON(registry *capabilities_registry.CapabilitiesRegistry) (*capabilities_registry.CapabilitiesRegistryDONInfo, error) {


Parsing events is more annoying when the don is added as part of proposal, so using this to get the donID in the case where you know precisely one don was added.

connorwstein · 2024-09-12T18:39:46Z

integration-tests/deployment/ccip/deploy.go

+		return ab, err
+	}
+	groupQuorums, groupParents, signerAddresses, signerGroups := c.ExtractSetConfigInputs()
+	mcmsTx, err := mcm.Contract.SetConfig(chain.DeployerKey,


Just a 1of1 with the test key for now. Need to parameterize in a follow up.

connorwstein · 2024-09-12T18:40:40Z

integration-tests/deployment/ccip/deploy_home_chain.go

+var (
+	// TODO: sort out why the mismatch here
+	//CCIPCapabilityId [32]byte = utils.MustHash(hexutil.Encode(MustABIEncode(`[{"type": "string"}, {"type": "string"}]`, CapabilityLabelledName, CapabilityVersion)))
+	CCIPCapabilityId = MustHashFromBytes(hexutil.MustDecode("0xe0da3c2b9005178f4731c9f40164f1933ad00bac9d6c13ad4ca1a8a763416380"))


This just avoids RPC calls all the time to get the capability ID. We expect it to be fixed per version

connorwstein · 2024-09-12T18:40:58Z

integration-tests/deployment/ccip/deploy_home_chain.go

 ) (ccip_config.CCIPConfigTypesChainConfigInfo, error) {
 	// Need to sort, otherwise _checkIsValidUniqueSubset onChain will fail
-	sortP2PIDS(p2pIDs)


sorting these once in the helper

Sorting no longer needed, can remove it entirely

integration-tests/deployment/ccip/add_chain.go

integration-tests/deployment/environment.go

AnieeG · 2024-09-12T18:56:11Z

integration-tests/deployment/ccip/propose.go

+		[]mcms.Signature{},
+		false,
+		metaDataPerChain,
+		"blah", // TODO


nit : ticket?

part of CCIP-3411 too

integration-tests/deployment/ccip/deploy.go

AnieeG · 2024-09-12T21:59:22Z

integration-tests/deployment/environment.go

+			// NOTE: Assume same peerID/multiAddr for all chains.
+			// Might make sense to change proto as peerID/multiAddr is 1-1 with nodeID?
+			peerID = MustPeerIDFromString(chainConfig.Ocr2Config.P2PKeyBundle.PeerId)
+			multiAddr = chainConfig.Ocr2Config.Multiaddr


The multiaddr is only populated for bootstrap

Out of curiosity, will it cause issues if it was set for non bootstrap nodes?

I think leaving it blank for non-bootstraps is ok for now. @asoliman92 no don't think it will

makramkd · 2024-09-13T10:29:22Z

integration-tests/deployment/ccip/add_chain.go

+	metaDataPerChain := make(map[mcms.ChainIdentifier]timelock.MCMSWithTimelockChainMetadata)
+	for _, source := range sources {
+		chain, _ := chainsel.ChainBySelector(source)
+		enableOnRampDest, err := state.Chains[source].OnRamp.ApplyDestChainConfigUpdates(SimTransactOpts(), []onramp.OnRampDestChainConfigArgs{


Does this actually "send" the tx to the simchain? Proposals just need calldata right? I'm wondering what'll happen here for the non-sim chains which have "automine" or similar on.

it doesn't - uses the NoSend flag in transact opts, just a scalable way to get the calldata for proposals

Oh that's clever, alright did not notice that

makramkd · 2024-09-13T10:31:03Z

integration-tests/deployment/ccip/add_chain.go

+		},
+	})
+	return timelock.NewMCMSWithTimelockProposal(
+		"1",


Comments on string/bool/int literal fields would be useful

yeah lets punt to CCIP-3411

makramkd · 2024-09-13T10:33:36Z

integration-tests/deployment/ccip/add_chain_test.go

+
+	// Transfer onramp/fq ownership to timelock.
+	// Enable the new dest on the test router.
+	for _, source := range initialDeploy {


Semantically distinct code blocks below (already designated by comments) can probably be moved to functions to make this test read/understand better

agree - was thinking we wait for another proposal to see if there are nice re-usable utilities we can extract

makramkd · 2024-09-13T10:34:59Z

integration-tests/deployment/ccip/add_lane_test.go

+	// TODO: The offchain code doesn't yet support partial lane
+	// enablement, need to address then re-enable this test.


What is needed still for offchain?

nothing now if the IsEnabled flag work is done, then we can complete this test

integration-tests/deployment/ccip/deploy_home_chain.go

makramkd · 2024-09-13T10:42:59Z

integration-tests/deployment/ccip/deploy_home_chain.go

 ) (ccip_config.CCIPConfigTypesChainConfigInfo, error) {
 	// Need to sort, otherwise _checkIsValidUniqueSubset onChain will fail
-	sortP2PIDS(p2pIDs)


Sorting no longer needed, can remove it entirely

…kflows-part2

cl-sonarqube-production · 2024-09-13T15:43:52Z

Quality Gate passed

Issues
23 New issues
14 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

* Port * Use mcms lib * Proposal signing working * Almost working. Just need ramps to make timelock owner first * Accept ownership proposal working * Fee quoter accept ownership * Fix existing tests * Enable to new chain traffic working * Mod tidy * Fix build * lint * More lint * Use Address() for MCM/Timelock * Rename * Self review cleanup * Clean up job management and bootstrap handling * Comments

Port

ab6b4e4

connorwstein mentioned this pull request Sep 9, 2024

CCIP capability AddChain/AddLane workflow tests smartcontractkit/ccip#1342

Closed

connorwstein added 9 commits September 9, 2024 17:02

Use mcms lib

bdef10f

Proposal signing working

a5c48c3

Almost working. Just need ramps to make timelock owner first

902155c

Accept ownership proposal working

dda8ca8

Fee quoter accept ownership

d75fbec

Fix existing tests

f79b3b0

Enable to new chain traffic working

41b314e

Mod tidy

4ab3826

Merge branch 'develop' into CCIP-3062-integration-tests-for-major-wor…

adbf236

…kflows-part2

connorwstein marked this pull request as ready for review September 12, 2024 16:37

connorwstein requested review from a team as code owners September 12, 2024 16:37

connorwstein changed the title ~~AddChain/AddLane CCIP integration tests~~ AddChain inbound CCIP integration test Sep 12, 2024

connorwstein added 4 commits September 12, 2024 12:49

Fix build

186c7fc

lint

554769f

More lint

92052a3

Use Address() for MCM/Timelock

fe703f9

AnieeG reviewed Sep 12, 2024

View reviewed changes

connorwstein added 2 commits September 12, 2024 14:22

Rename

4be4c6e

Self review cleanup

300676c

connorwstein commented Sep 12, 2024

View reviewed changes

AnieeG reviewed Sep 12, 2024

View reviewed changes

Clean up job management and bootstrap handling

0a63818

AnieeG previously approved these changes Sep 12, 2024

View reviewed changes

AnieeG reviewed Sep 12, 2024

View reviewed changes

makramkd reviewed Sep 13, 2024

View reviewed changes

Comments

4ea8911

connorwstein dismissed AnieeG’s stale review via 4ea8911 September 13, 2024 13:30

asoliman92 approved these changes Sep 13, 2024

View reviewed changes

Merge branch 'develop' into CCIP-3062-integration-tests-for-major-wor…

55fb99b

…kflows-part2

jmank88 approved these changes Sep 13, 2024

View reviewed changes

connorwstein added this pull request to the merge queue Sep 13, 2024

Merged via the queue into develop with commit 5ec7cbb Sep 13, 2024
152 of 153 checks passed

connorwstein deleted the CCIP-3062-integration-tests-for-major-workflows-part2 branch September 13, 2024 16:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AddChain inbound CCIP integration test #14377

AddChain inbound CCIP integration test #14377

connorwstein commented Sep 9, 2024 •

edited

Loading

AnieeG left a comment

connorwstein Sep 12, 2024

connorwstein Sep 12, 2024

connorwstein Sep 12, 2024

connorwstein Sep 12, 2024

connorwstein Sep 12, 2024

connorwstein Sep 12, 2024

asoliman92 Sep 13, 2024

makramkd Sep 13, 2024

AnieeG Sep 12, 2024

connorwstein Sep 12, 2024

AnieeG Sep 12, 2024

asoliman92 Sep 13, 2024 •

edited

Loading

connorwstein Sep 13, 2024

makramkd Sep 13, 2024

connorwstein Sep 13, 2024

makramkd Sep 13, 2024

makramkd Sep 13, 2024

connorwstein Sep 13, 2024

makramkd Sep 13, 2024

connorwstein Sep 13, 2024

makramkd Sep 13, 2024

connorwstein Sep 13, 2024

makramkd Sep 13, 2024

cl-sonarqube-production bot commented Sep 13, 2024

		// TODO: The offchain code doesn't yet support partial lane
		// enablement, need to address then re-enable this test.

AddChain inbound CCIP integration test #14377

AddChain inbound CCIP integration test #14377

Conversation

connorwstein commented Sep 9, 2024 • edited Loading

AnieeG left a 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

asoliman92 Sep 13, 2024 • 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

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 Sep 13, 2024

Quality Gate passed

connorwstein commented Sep 9, 2024 •

edited

Loading

asoliman92 Sep 13, 2024 •

edited

Loading