PLEX-2149 fix add capabilities CLD to prevent removal of existing abilities in the DON

deployment/cre/capabilities_registry/v2/changeset/add_capabilities_test.go

@@ -22,6 +22,7 @@ import (
 
 var (
 	newCapID       = "[email protected]"
+	anotherCapID   = "[email protected]"
 	newCapMetadata = map[string]any{"capabilityType": float64(0), "responseType": float64(0)}
 	newCapConfig   = map[string]any{
 		"restrictedConfig": map[string]any{
@@ -103,17 +104,14 @@ func TestAddCapabilities_VerifyPreconditions(t *testing.T) {
 	require.NoError(t, err)
 }
 
-func TestAddCapabilities_Apply(t *testing.T) {
-	// SetupEnvV2 deploys a cap reg v2 and configures it. So no need to do that here, just leverage the existing one.
-	fixture := test.SetupEnvV2(t, false)
-
+func addNewCapability(t *testing.T, fixture *test.EnvWrapperV2, capID string) {
 	input := changeset.AddCapabilitiesInput{
 		RegistryChainSel:  fixture.RegistrySelector,
 		RegistryQualifier: test.RegistryQualifier,
 		DonName:           test.DONName,
 		CapabilityConfigs: []contracts.CapabilityConfig{{
 			Capability: contracts.Capability{
-				CapabilityID:          newCapID,
+				CapabilityID:          capID,
 				ConfigurationContract: common.Address{},
 				Metadata:              newCapMetadata,
 			},
@@ -129,29 +127,21 @@ func TestAddCapabilities_Apply(t *testing.T) {
 	// Apply
 	_, err = changeset.AddCapabilities{}.Apply(*fixture.Env, input)
 	require.NoError(t, err)
+}
 
+func requireCapability(t *testing.T, fixture *test.EnvWrapperV2, capID string) {
 	// Validate on-chain state
 	capReg, err := capabilities_registry_v2.NewCapabilitiesRegistry(
 		fixture.RegistryAddress,
 		fixture.Env.BlockChains.EVMChains()[fixture.RegistrySelector].Client,
 	)
 	require.NoError(t, err)
 
-	// Here we check that the uptyped input of the changeset was correctly applied on-chain as proto and can be decoded back to the same config
-	// encoding to proto bytes is same as in the changeset and decoding to cap cfg is same as in the v2 registry syncer
-	capCfg := pkg.CapabilityConfig(newCapConfig)
-	configProtoBytes, err := capCfg.MarshalProto() // on chain it is stored as proto bytes
-	require.NoError(t, err, "should be able to marshal new capability config to proto bytes")
-
-	expectedConfig := new(pkg.CapabilityConfig) // expected decoded config, to be compared with decoded on-chain config
-	err = expectedConfig.UnmarshalProto(configProtoBytes)
-	require.NoError(t, err, "should be able to unmarshal new capability config from proto bytes")
-
 	caps, err := pkg.GetCapabilities(nil, capReg)
 	require.NoError(t, err)
 	var found bool
 	for _, c := range caps {
-		if c.CapabilityId == newCapID {
+		if c.CapabilityId == capID {
 			// metadata check
 			var gotMeta map[string]any
 			require.NoError(t, json.Unmarshal(c.Metadata, &gotMeta))
@@ -160,21 +150,31 @@ func TestAddCapabilities_Apply(t *testing.T) {
 			break
 		}
 	}
-	require.True(t, found, "new capability should be registered")
+	require.True(t, found, "new capability %s should be registered", capID)
 
 	// Nodes should now include new capability id
 	nodes, err := pkg.GetNodes(nil, capReg)
 	require.NoError(t, err)
 	for _, n := range nodes {
-		assert.Contains(t, n.CapabilityIds, newCapID, "node should have new capability id appended")
+		assert.Contains(t, n.CapabilityIds, capID, "node should have new capability id appended")
 	}
 
+	// Here we check that the uptyped input of the changeset was correctly applied on-chain as proto and can be decoded back to the same config
+	// encoding to proto bytes is same as in the changeset and decoding to cap cfg is same as in the v2 registry syncer
+	capCfg := pkg.CapabilityConfig(newCapConfig)
+	configProtoBytes, err := capCfg.MarshalProto() // on chain it is stored as proto bytes
+	require.NoError(t, err, "should be able to marshal new capability config to proto bytes")
+
+	expectedConfig := new(pkg.CapabilityConfig) // expected decoded config, to be compared with decoded on-chain config
+	err = expectedConfig.UnmarshalProto(configProtoBytes)
+	require.NoError(t, err, "should be able to unmarshal new capability config from proto bytes")
+
 	// DON capability configurations should include new capability config
 	don, err := capReg.GetDONByName(nil, test.DONName)
 	require.NoError(t, err)
 	var cfgFound bool
 	for _, cfg := range don.CapabilityConfigurations {
-		if cfg.CapabilityId == newCapID {
+		if cfg.CapabilityId == capID {
 			got := new(pkg.CapabilityConfig)
 			require.NoError(t, got.UnmarshalProto(cfg.Config), "unmarshal capability config proto bytes should not error")
 			if diff := cmp.Diff(expectedConfig, got, protocmp.Transform()); diff != "" {
@@ -184,7 +184,20 @@ func TestAddCapabilities_Apply(t *testing.T) {
 			cfgFound = true
 		}
 	}
-	require.True(t, cfgFound, "don should have new capability configuration")
+	require.True(t, cfgFound, "expected don to have %s capability configuration", capID)
+}
+
+func TestAddCapabilities_Apply(t *testing.T) {
+	// SetupEnvV2 deploys a cap reg v2 and configures it. So no need to do that here, just leverage the existing one.
+	fixture := test.SetupEnvV2(t, false)
+
+	addNewCapability(t, fixture, newCapID)
+	requireCapability(t, fixture, newCapID)
+
+	// add another capability and ensure that both are present
+	addNewCapability(t, fixture, anotherCapID)
+	requireCapability(t, fixture, newCapID)
+	requireCapability(t, fixture, anotherCapID)
 }
 
 func TestAddCapabilities_Apply_MCMS(t *testing.T) {

deployment/cre/capabilities_registry/v2/changeset/operations/contracts/update_don.go

@@ -3,6 +3,7 @@ package contracts
 import (
 	"errors"
 	"fmt"
+	"sort"
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
@@ -30,8 +31,9 @@ type UpdateDONInput struct {
 	ChainSelector uint64
 
 	// P2PIDs are the peer ids that compose the don. Optional, only provided if the DON composition is changing.
-	P2PIDs            []p2pkey.PeerID
-	CapabilityConfigs []CapabilityConfig
+	P2PIDs                            []p2pkey.PeerID
+	CapabilityConfigs                 []CapabilityConfig
+	MergeCapabilityConfigsWithOnChain bool
 
 	// DonName to update, this is required
 	DonName string
@@ -109,7 +111,7 @@ var UpdateDON = operations.NewOperation[UpdateDONInput, UpdateDONOutput, UpdateD
 			return UpdateDONOutput{}, fmt.Errorf("refusing to update workflow don %d at config version %d because we cannot validate that all forwarder contracts are ready to accept the new configure version", don.Id, don.ConfigCount)
 		}
 
-		cfgs, err := computeConfigs(input.CapabilityConfigs, don.CapabilityConfigurations)
+		cfgs, err := computeConfigs(input.CapabilityConfigs, don.CapabilityConfigurations, input.MergeCapabilityConfigsWithOnChain)
 		if err != nil {
 			return UpdateDONOutput{}, fmt.Errorf("failed to compute configs: %w", err)
 		}
@@ -162,21 +164,56 @@ var UpdateDON = operations.NewOperation[UpdateDONInput, UpdateDONOutput, UpdateD
 	},
 )
 
-func computeConfigs(capCfgs []CapabilityConfig, existingCapConfigs []capabilities_registry_v2.CapabilitiesRegistryCapabilityConfiguration) ([]capabilities_registry_v2.CapabilitiesRegistryCapabilityConfiguration, error) {
-	var out []capabilities_registry_v2.CapabilitiesRegistryCapabilityConfiguration
+func computeConfigs(
+	capCfgs []CapabilityConfig,
+	existingCapConfigs []capabilities_registry_v2.CapabilitiesRegistryCapabilityConfiguration,
+	mergeCapabilities bool) ([]capabilities_registry_v2.CapabilitiesRegistryCapabilityConfiguration, error) {
+	capSet := make(map[string]capabilities_registry_v2.CapabilitiesRegistryCapabilityConfiguration)
 	for _, capCfg := range capCfgs {
-		cfg := capabilities_registry_v2.CapabilitiesRegistryCapabilityConfiguration{}
-		cfg.CapabilityId = capCfg.Capability.CapabilityID
-		var err error
-		x := pkg.CapabilityConfig(capCfg.Config)
-		cfg.Config, err = x.MarshalProto()
+		onChainCap, err := capabilityConfigToOnChain(capCfg)
 		if err != nil {
-			return nil, fmt.Errorf("failed to marshal capability configuration config: %w", err)
+			return nil, fmt.Errorf("failed to convert capability config to on-chain format: %w", err)
 		}
-		if cfg.Config == nil {
-			return nil, fmt.Errorf("config is required for capability %s", capCfg.Capability.CapabilityID)
+
+		_, ok := capSet[onChainCap.CapabilityId]
+		if ok {
+			return nil, fmt.Errorf("duplicate capability configuration for id: %s", onChainCap.CapabilityId)
 		}
-		out = append(out, cfg)
+
+		capSet[onChainCap.CapabilityId] = *onChainCap
+	}
+
+	if mergeCapabilities {
+		for _, existingCapConfig := range existingCapConfigs {
+			_, ok := capSet[existingCapConfig.CapabilityId]
+			if !ok {
+				capSet[existingCapConfig.CapabilityId] = existingCapConfig
+			}
+		}
+	}
+	var out []capabilities_registry_v2.CapabilitiesRegistryCapabilityConfiguration
+	for _, capCfg := range capSet {
+		out = append(out, capCfg)
 	}
+
+	sort.Slice(out, func(i, j int) bool {
+		return out[i].CapabilityId < out[j].CapabilityId
+	})
 	return out, nil
 }
+
+func capabilityConfigToOnChain(capCfg CapabilityConfig) (*capabilities_registry_v2.CapabilitiesRegistryCapabilityConfiguration, error) {
+	cfg := capabilities_registry_v2.CapabilitiesRegistryCapabilityConfiguration{}
+	cfg.CapabilityId = capCfg.Capability.CapabilityID
+	var err error
+	x := pkg.CapabilityConfig(capCfg.Config)
+	cfg.Config, err = x.MarshalProto()
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal capability configuration config: %w", err)
+	}
+	if cfg.Config == nil {
+		return nil, fmt.Errorf("config is required for capability %s", capCfg.Capability.CapabilityID)
+	}
+
+	return &cfg, nil
+}

deployment/cre/capabilities_registry/v2/changeset/sequences/add_capabilities.go

@@ -184,14 +184,15 @@ var AddCapabilities = operations.NewSequence[AddCapabilitiesInput, AddCapabiliti
 				Strategy:             strategy,
 			},
 			contracts.UpdateDONInput{
-				ChainSelector:     chainSel,
-				P2PIDs:            p2pIDs,
-				CapabilityConfigs: input.CapabilityConfigs,
-				DonName:           input.DonName,
-				F:                 don.F,
-				IsPrivate:         !don.IsPublic,
-				Force:             input.Force,
-				MCMSConfig:        input.MCMSConfig,
+				ChainSelector:                     chainSel,
+				P2PIDs:                            p2pIDs,
+				CapabilityConfigs:                 input.CapabilityConfigs,
+				MergeCapabilityConfigsWithOnChain: true,
+				DonName:                           input.DonName,
+				F:                                 don.F,
+				IsPrivate:                         !don.IsPublic,
+				Force:                             input.Force,
+				MCMSConfig:                        input.MCMSConfig,
 			},
 		)
 		if err != nil {