[CCIP Merge] Capabilities fix [CCIP-2943] #14048

asoliman92 · 2024-08-06T13:31:52Z

Fixing ccip capabilities to be merged into chainlink

core/capabilities/ccip/launcher/diff.go

makramkd · 2024-08-06T14:05:59Z

core/capabilities/ccip/launcher/diff.go

-	capabilityLabelledName string,
-	state registrysyncer.State,
-) (kcr.CapabilitiesRegistryCapabilityInfo, error) {
+	capabilityID string,


Feels less error prone to make this structured ID an actual structure rather than just a string.

@cedric-cordenier wdyt? Maybe something like:

type CapabilityID struct { Name string Version string } func (cid CapabilityID) String() string { // perhaps some semver regex validation using the semver package return fmt.Sprintf("%s@%s", cid.Name, cid.Version) }

I said a similar comment on call with @cedric-cordenier last week as well :)

This should not be a blocker for this PR though.

Yes, I'm not against this 👍

@asoliman92 can you create a ticket in the backlog for this if you're not going to do it in this PR

core/services/chainlink/application.go

Make application.go ready for adding more fixes

We want to define and use the appropriate OCR offchain config for each plugin. Requires smartcontractkit/chainlink-ccip#36

makramkd · 2024-08-07T15:48:07Z

core/capabilities/ccip/delegate.go

@@ -99,6 +106,25 @@ func (d *Delegate) ServicesForSpec(ctx context.Context, spec job.Job) (services
 		return nil, fmt.Errorf("failed to get all p2p keys: %w", err)
 	}

+	cfg := d.capabilityConfig
+	rid := cfg.ExternalRegistry().RelayID()
+	registryAddress := cfg.ExternalRegistry().Address()


Nit: you can inline this

makramkd · 2024-08-07T15:48:26Z

core/capabilities/ccip/launcher/diff_test.go

+var defaultCapability = getCapability(ccipCapName, ccipCapVersion)
+var newCapability = getCapability(ccipCapName, ccipCapNewVersion)


Merge these var decls into one var decl

makramkd · 2024-08-07T15:48:54Z

core/capabilities/ccip/launcher/diff_test.go

+var p2pID1 = getP2PID(1)
+var p2pID2 = getP2PID(2)
+
+var defaultCapCfgs = map[string]registrysyncer.CapabilityConfiguration{
+	defaultCapability.ID: registrysyncer.CapabilityConfiguration{},
+}
+var defaultRegistryDon = registrysyncer.DON{
+	DON:                      getDON(1, []ragep2ptypes.PeerID{p2pID1}, 0),
+	CapabilityConfigurations: defaultCapCfgs,
+}


Merge into one var decl at the top of the file for better organization

makramkd · 2024-08-07T15:49:16Z

core/capabilities/ccip/launcher/launcher.go

+	ccipreader "github.com/smartcontractkit/chainlink-ccip/pkg/reader"
 	ccipreaderpkg "github.com/smartcontractkit/chainlink-ccip/pkg/reader"


Duplicate import

makramkd · 2024-08-07T15:51:11Z

core/capabilities/launcher.go

@@ -46,6 +50,42 @@ type launcher struct {
 	subServices []services.Service
 }

+func unmarshalCapabilityConfig(data []byte) (capabilities.CapabilityConfiguration, error) {


Which capability config is this unmarshalling? Not CCIP right?

Not CCIP AFAIU. @cedric-cordenier right?

Yeah this is only unmarshaling the Keystone capability configs

makramkd · 2024-08-07T15:52:13Z

core/services/chainlink/application.go

@@ -17,6 +17,8 @@ import (
 	"go.uber.org/multierr"
 	"go.uber.org/zap/zapcore"

+	"github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip"


Nit: group this with other chainlink/v2/ imports?

cl-sonarqube-production · 2024-08-07T16:49:57Z

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.2% Coverage on New Code
1.2% Duplication on New Code

See analysis details on SonarQube

* [CCIP Merge] Add ccip capabilities directory [CCIP-2943] (#14044) * Add ccip capabilities directory * [CCIP Merge] Capabilities fix [CCIP-2943] (#14048) * Fix compilation for launcher, diff Make application.go ready for adding more fixes * Fix launcher tests * ks-409 fix the mock trigger to ensure events are sent (#14047) * Add ccip to job orm * Add capabilities directory under BUSL license * Prep to instantiate separate registrysyncer for CCIP * Move registrySyncer creation into ccip delegate * [chore] Change registrysyncer config to bytes * Fix launcher diff tests after changing structs in syncer * Fix linting * MAke simulated backend client work with chains other than 1337 * core/capabilities/ccip: use OCR offchain config (#1264) We want to define and use the appropriate OCR offchain config for each plugin. Requires smartcontractkit/chainlink-ccip#36 * Cleaning up * Add capabilities types to mockery --------- Co-authored-by: Cedric Cordenier <[email protected]> Co-authored-by: Matthew Pendrey <[email protected]> Co-authored-by: Makram <[email protected]> * make modgraph * Add changeset * Fix test with new TxMgr constructor --------- Co-authored-by: Cedric Cordenier <[email protected]> Co-authored-by: Matthew Pendrey <[email protected]> Co-authored-by: Makram <[email protected]>

asoliman92 changed the title ~~Ccip/capabilities merge fix~~ [CCIP Merge] Capabilities fix [CCIP-2943] Aug 6, 2024

asoliman92 force-pushed the ccip/capabilities-merge-fix branch from afb93d7 to b089224 Compare August 6, 2024 13:43

asoliman92 force-pushed the ccip/capabilities-directory-only branch from 7f010e1 to 7d44b17 Compare August 6, 2024 13:47

asoliman92 force-pushed the ccip/capabilities-merge-fix branch from b089224 to 23dea4c Compare August 6, 2024 13:49

asoliman92 temporarily deployed to sdlc August 6, 2024 13:49 — with GitHub Actions Inactive

makramkd requested changes Aug 6, 2024

View reviewed changes

Base automatically changed from ccip/capabilities-directory-only to ccip/capabilities-merge-clean August 6, 2024 14:34

asoliman92 and others added 4 commits August 6, 2024 16:51

[WIP] Fix compilation for launcher, diff

f5a5894

Make application.go ready for adding more fixes

[WIP] Fix compilation for chainlink.go

19a7977

Fix launcher tests

5054d14

WIP fix ocr3 tests

51e8ad9

asoliman92 force-pushed the ccip/capabilities-merge-fix branch from 9cbff85 to 51e8ad9 Compare August 6, 2024 14:52

asoliman92 temporarily deployed to sdlc August 6, 2024 14:52 — with GitHub Actions Inactive

ettec and others added 2 commits August 6, 2024 16:53

ks-409 fix the mock trigger to ensure events are sent (#14047)

36e4767

Add ccip to job orm

abab516

asoliman92 temporarily deployed to sdlc August 6, 2024 15:33 — with GitHub Actions Inactive

cedric-cordenier had a problem deploying to sdlc August 7, 2024 10:31 — with GitHub Actions Failure

asoliman92 and others added 3 commits August 7, 2024 17:00

Add capabilities directory under BUSL license

f774ecf

Prep to instantiate separate registrysyncer for CCIP

1267e61

Move registrySyncer creation into ccip delegate

e35df28

asoliman92 force-pushed the ccip/capabilities-merge-fix branch from 213a58f to e35df28 Compare August 7, 2024 14:20

asoliman92 temporarily deployed to sdlc August 7, 2024 14:20 — with GitHub Actions Inactive

cedric-cordenier and others added 2 commits August 7, 2024 18:32

[chore] Change registrysyncer config to bytes

d73234f

Fix launcher diff tests after changing structs in syncer

b092a14

asoliman92 temporarily deployed to sdlc August 7, 2024 14:47 — with GitHub Actions Inactive

Fix linting

8226595

asoliman92 temporarily deployed to sdlc August 7, 2024 15:05 — with GitHub Actions Inactive

asoliman92 and others added 2 commits August 7, 2024 19:35

MAke simulated backend client work with chains other than 1337

371e5f1

core/capabilities/ccip: use OCR offchain config (#1264)

3fcaec9

We want to define and use the appropriate OCR offchain config for each plugin. Requires smartcontractkit/chainlink-ccip#36

asoliman92 temporarily deployed to sdlc August 7, 2024 15:38 — with GitHub Actions Inactive

asoliman92 marked this pull request as ready for review August 7, 2024 15:42

asoliman92 requested review from a team as code owners August 7, 2024 15:42

asoliman92 requested a review from makramkd August 7, 2024 15:42

makramkd reviewed Aug 7, 2024

View reviewed changes

asoliman92 temporarily deployed to sdlc August 7, 2024 16:04 — with GitHub Actions Inactive

asoliman92 requested a review from makramkd August 7, 2024 16:07

Cleaning up

c495985

asoliman92 force-pushed the ccip/capabilities-merge-fix branch from a58854b to c495985 Compare August 7, 2024 16:11

asoliman92 temporarily deployed to sdlc August 7, 2024 16:11 — with GitHub Actions Inactive

Add capabilities types to mockery

3aedc2c

asoliman92 temporarily deployed to sdlc August 7, 2024 16:33 — with GitHub Actions Inactive

makramkd approved these changes Aug 7, 2024

View reviewed changes

asoliman92 merged commit ba0e8c5 into ccip/capabilities-merge-clean Aug 7, 2024
128 of 129 checks passed

asoliman92 deleted the ccip/capabilities-merge-fix branch August 7, 2024 17:11

winder mentioned this pull request Aug 7, 2024

[CCIP Merge] Capabilities [CCIP-2943] #14068

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CCIP Merge] Capabilities fix [CCIP-2943] #14048

[CCIP Merge] Capabilities fix [CCIP-2943] #14048

asoliman92 commented Aug 6, 2024 •

edited

Loading

makramkd Aug 6, 2024

asoliman92 Aug 6, 2024

asoliman92 Aug 6, 2024

cedric-cordenier Aug 7, 2024

makramkd Aug 7, 2024

asoliman92 Aug 7, 2024

makramkd Aug 7, 2024

makramkd Aug 7, 2024

makramkd Aug 7, 2024

makramkd Aug 7, 2024

makramkd Aug 7, 2024

asoliman92 Aug 7, 2024 •

edited

Loading

cedric-cordenier Aug 7, 2024

makramkd Aug 7, 2024

cl-sonarqube-production bot commented Aug 7, 2024

		var defaultCapability = getCapability(ccipCapName, ccipCapVersion)
		var newCapability = getCapability(ccipCapName, ccipCapNewVersion)

		ccipreader "github.com/smartcontractkit/chainlink-ccip/pkg/reader"
		ccipreaderpkg "github.com/smartcontractkit/chainlink-ccip/pkg/reader"

[CCIP Merge] Capabilities fix [CCIP-2943] #14048

[CCIP Merge] Capabilities fix [CCIP-2943] #14048

Conversation

asoliman92 commented Aug 6, 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

asoliman92 Aug 7, 2024 • edited Loading

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 Aug 7, 2024

Quality Gate passed

asoliman92 commented Aug 6, 2024 •

edited

Loading

asoliman92 Aug 7, 2024 •

edited

Loading