Refactor DefaultCapConfig into multiple funcs (#15882)

* Refactors `DefaultCapConfig` into multiple funcs

* Fixes lint

* Expects a `config` on `FromCapabilitiesRegistryCapability` calls

* Exposes `RegisteredCapabilityConfig` to consumers

* Fixes lint

* Exposes more needed types

* Expects bytes config as part of the `RegisteredCapability` struct

* Uses bytes cap config on `RegisterDons()` calls

* Fixes test

* Fixes CI

* Fixes CI

* Takes proto object instead of bytes on `FromCapabilitiesRegistryCapability`

* Exposes `DONCapabilityWithConfig`

* Fixes lint

* Fixes lint

* gomods tidy

* Fixes CI

* Fixes CI

* gomods tidy

* Updates typing

* Fixes lint

* Fixes lint

* Refactors test util

deployment/go.mod

@@ -9,7 +9,7 @@ replace github.com/smartcontractkit/chainlink/v2 => ../
 
 // Using a separate inline `require` here to avoid surrounding line changes
 // creating potential merge conflicts.
-require github.com/smartcontractkit/chainlink/v2 v2.0.0-20250121175436-6a8b51d8b5f5
+require github.com/smartcontractkit/chainlink/v2 v2.0.0-20250113180450-c33ab50ac8c0
 
 require (
 	github.com/Khan/genqlient v0.7.0

deployment/keystone/changeset/compatiblity.go

@@ -89,6 +89,8 @@ type ConfigureContractsResponse = internal.ConfigureContractsResponse
 // in is in a convenient form to handle the CLO representation of the nop data
 type DonCapabilities = internal.DonCapabilities
 
+type DONCapabilityWithConfig = internal.DONCapabilityWithConfig
+
 type DeployRequest = internal.DeployRequest
 type DeployResponse = internal.DeployResponse
 

deployment/keystone/changeset/internal/deploy.go

@@ -15,25 +15,17 @@ import (
 
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
-	"golang.org/x/exp/maps"
-
 	"github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/mcms"
 	"github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/timelock"
-
-	"github.com/smartcontractkit/chainlink/deployment"
-
-	"google.golang.org/protobuf/proto"
-	"google.golang.org/protobuf/types/known/durationpb"
-
 	chainsel "github.com/smartcontractkit/chain-selectors"
-
 	capabilitiespb "github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb"
-	"github.com/smartcontractkit/chainlink-common/pkg/values"
+	"github.com/smartcontractkit/chainlink-common/pkg/logger"
+	"golang.org/x/exp/maps"
+	"google.golang.org/protobuf/proto"
 
+	"github.com/smartcontractkit/chainlink/deployment"
 	capabilities_registry "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry_1_1_0"
 	kf "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/forwarder_1_0_0"
-
-	"github.com/smartcontractkit/chainlink-common/pkg/logger"
 )
 
 type ConfigureContractsRequest struct {
@@ -142,7 +134,7 @@ type DonInfo struct {
 	Name         string
 	F            uint8
 	Nodes        []deployment.Node
-	Capabilities []capabilities_registry.CapabilitiesRegistryCapability // every capability is hosted on each node
+	Capabilities []DONCapabilityWithConfig // every capability is hosted on each node
 }
 
 func DonInfos(dons []DonCapabilities, jd deployment.OffchainClient) ([]DonInfo, error) {
@@ -415,7 +407,7 @@ func ConfigureOCR3ContractFromJD(env *deployment.Environment, cfg ConfigureOCR3C
 type RegisterCapabilitiesRequest struct {
 	Env                   *deployment.Environment
 	RegistryChainSelector uint64
-	DonToCapabilities     map[string][]capabilities_registry.CapabilitiesRegistryCapability
+	DonToCapabilities     map[string][]DONCapabilityWithConfig
 
 	// if UseMCMS is true, a batch proposal is returned and no transaction is confirmed on chain.
 	UseMCMS bool
@@ -428,21 +420,26 @@ type RegisterCapabilitiesResponse struct {
 
 type RegisteredCapability struct {
 	capabilities_registry.CapabilitiesRegistryCapability
-	ID [32]byte
+	ID     [32]byte
+	Config *capabilitiespb.CapabilityConfig
 }
 
-func FromCapabilitiesRegistryCapability(cap *capabilities_registry.CapabilitiesRegistryCapability, e deployment.Environment, registryChainSelector uint64) (*RegisteredCapability, error) {
+func FromCapabilitiesRegistryCapability(capReg *capabilities_registry.CapabilitiesRegistryCapability, cfg *capabilitiespb.CapabilityConfig, e deployment.Environment, registryChainSelector uint64) (*RegisteredCapability, error) {
 	registry, _, err := GetRegistryContract(&e, registryChainSelector)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get registry: %w", err)
 	}
-	id, err := registry.GetHashedCapabilityId(&bind.CallOpts{}, cap.LabelledName, cap.Version)
+	id, err := registry.GetHashedCapabilityId(&bind.CallOpts{}, capReg.LabelledName, capReg.Version)
 	if err != nil {
-		return nil, fmt.Errorf("failed to call GetHashedCapabilityId for capability %v: %w", cap, err)
+		return nil, fmt.Errorf("failed to call GetHashedCapabilityId for capability %v: %w", capReg, err)
+	}
+	if cfg == nil {
+		return nil, fmt.Errorf("config is required for capability %v", capReg)
 	}
 	return &RegisteredCapability{
-		CapabilitiesRegistryCapability: *cap,
+		CapabilitiesRegistryCapability: *capReg,
 		ID:                             id,
+		Config:                         cfg,
 	}, nil
 }
 
@@ -468,29 +465,31 @@ func RegisterCapabilities(lggr logger.Logger, req RegisterCapabilitiesRequest) (
 	uniqueCaps := make(map[capabilities_registry.CapabilitiesRegistryCapability][32]byte)
 	for don, caps := range req.DonToCapabilities {
 		var registerCaps []RegisteredCapability
-		for _, cap := range caps {
-			id, ok := uniqueCaps[cap]
+		for i := range caps {
+			regCap := &caps[i]
+			id, ok := uniqueCaps[regCap.Capability]
 			if !ok {
 				var err error
-				id, err = registry.GetHashedCapabilityId(&bind.CallOpts{}, cap.LabelledName, cap.Version)
+				id, err = registry.GetHashedCapabilityId(&bind.CallOpts{}, regCap.Capability.LabelledName, regCap.Capability.Version)
 				if err != nil {
-					return nil, fmt.Errorf("failed to call GetHashedCapabilityId for capability %v: %w", cap, err)
+					return nil, fmt.Errorf("failed to call GetHashedCapabilityId for capability %v: %w", regCap, err)
 				}
-				uniqueCaps[cap] = id
+				uniqueCaps[regCap.Capability] = id
 			}
 			registerCap := RegisteredCapability{
-				CapabilitiesRegistryCapability: cap,
 				ID:                             id,
+				Config:                         regCap.Config,
+				CapabilitiesRegistryCapability: regCap.Capability,
 			}
-			lggr.Debugw("hashed capability id", "capability", cap, "id", id)
+			lggr.Debugw("hashed capability id", "capability", regCap, "id", id)
 			registerCaps = append(registerCaps, registerCap)
 		}
 		resp.DonToCapabilities[don] = registerCaps
 	}
 
 	var capabilities []capabilities_registry.CapabilitiesRegistryCapability
-	for cap := range uniqueCaps {
-		capabilities = append(capabilities, cap)
+	for uniqueCap := range uniqueCaps {
+		capabilities = append(capabilities, uniqueCap)
 	}
 	if len(capabilities) == 0 {
 		lggr.Warn("no new capabilities to register")
@@ -615,41 +614,6 @@ func addNOPsMCMSProposal(registry *capabilities_registry.CapabilitiesRegistry, n
 	}, nil
 }
 
-func DefaultCapConfig(capType uint8, nNodes int) *capabilitiespb.CapabilityConfig {
-	switch capType {
-	// TODO: use the enum defined in ??
-	case uint8(0): // trigger
-		return &capabilitiespb.CapabilityConfig{
-			DefaultConfig: values.Proto(values.EmptyMap()).GetMapValue(),
-			RemoteConfig: &capabilitiespb.CapabilityConfig_RemoteTriggerConfig{
-				RemoteTriggerConfig: &capabilitiespb.RemoteTriggerConfig{
-					RegistrationRefresh: durationpb.New(20 * time.Second),
-					RegistrationExpiry:  durationpb.New(60 * time.Second),
-					// F + 1; assuming n = 3f+1
-					MinResponsesToAggregate: uint32(nNodes/3) + 1,
-				},
-			},
-		}
-	case uint8(2): // consensus
-		return &capabilitiespb.CapabilityConfig{
-			DefaultConfig: values.Proto(values.EmptyMap()).GetMapValue(),
-		}
-	case uint8(3): // target
-		return &capabilitiespb.CapabilityConfig{
-			DefaultConfig: values.Proto(values.EmptyMap()).GetMapValue(),
-			RemoteConfig: &capabilitiespb.CapabilityConfig_RemoteTargetConfig{
-				RemoteTargetConfig: &capabilitiespb.RemoteTargetConfig{
-					RequestHashExcludedAttributes: []string{"signed_report.Signatures"}, // TODO: const defn in a common place
-				},
-			},
-		}
-	default:
-		return &capabilitiespb.CapabilityConfig{
-			DefaultConfig: values.Proto(values.EmptyMap()).GetMapValue(),
-		}
-	}
-}
-
 // register nodes
 type RegisterNodesRequest struct {
 	Env                   *deployment.Environment
@@ -916,26 +880,26 @@ func RegisterDons(lggr logger.Logger, req RegisterDonsRequest) (*RegisterDonsRes
 		}
 
 		lggr.Debugw("registering DON", "don", don.Name, "p2p sorted hash", p2pSortedHash)
-
-		caps, ok := req.DonToCapabilities[don.Name]
+		regCaps, ok := req.DonToCapabilities[don.Name]
 		if !ok {
 			return nil, fmt.Errorf("capabilities not found for DON %s", don.Name)
 		}
 		wfSupported := false
 		var cfgs []capabilities_registry.CapabilitiesRegistryCapabilityConfiguration
-		for _, cap := range caps {
-			if cap.CapabilityType == 2 { // OCR3 capability => WF supported
+		for _, regCap := range regCaps {
+			if regCap.CapabilityType == 2 { // OCR3 capability => WF supported
 				wfSupported = true
 			}
-			// TODO: accept configuration from external source for each (don,capability)
-			capCfg := DefaultCapConfig(cap.CapabilityType, len(p2pIds))
-			cfgb, err := proto.Marshal(capCfg)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal capability config for %v: %w", cap, err)
+			if regCap.Config == nil {
+				return nil, fmt.Errorf("config not found for capability %v", regCap)
+			}
+			cfgB, capErr := proto.Marshal(regCap.Config)
+			if capErr != nil {
+				return nil, fmt.Errorf("failed to marshal config for capability %v: %w", regCap, capErr)
 			}
 			cfgs = append(cfgs, capabilities_registry.CapabilitiesRegistryCapabilityConfiguration{
-				CapabilityId: cap.ID,
-				Config:       cfgb,
+				CapabilityId: regCap.ID,
+				Config:       cfgB,
 			})
 		}
 

deployment/keystone/changeset/internal/deploy_test.go

@@ -4,13 +4,14 @@ import (
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/smartcontractkit/chainlink-common/pkg/logger"
+
 	"github.com/smartcontractkit/chainlink/deployment"
 	"github.com/smartcontractkit/chainlink/deployment/keystone/changeset/internal"
 	kstest "github.com/smartcontractkit/chainlink/deployment/keystone/changeset/internal/test"
 	kcr "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry_1_1_0"
-
-	"github.com/stretchr/testify/require"
 )
 
 func Test_RegisterNOPS(t *testing.T) {

deployment/keystone/changeset/internal/test/utils.go

@@ -5,18 +5,19 @@ import (
 	"fmt"
 	"sort"
 	"testing"
+	"time"
 
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/durationpb"
 
 	capabilitiespb "github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb"
 	"github.com/smartcontractkit/chainlink-common/pkg/logger"
 	"github.com/smartcontractkit/chainlink-common/pkg/values"
 
 	"github.com/smartcontractkit/chainlink/deployment"
 	"github.com/smartcontractkit/chainlink/deployment/environment/memory"
-
 	"github.com/smartcontractkit/chainlink/deployment/keystone/changeset/internal"
 	capabilities_registry "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry_1_1_0"
 	"github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/p2pkey"
@@ -211,25 +212,26 @@ func (cc *CapabilityCache) Get(cap capabilities_registry.CapabilitiesRegistryCap
 // AddCapabilities adds the capabilities to the registry and returns the registered capabilities
 // if the capability is already registered, it will not be re-registered
 // if duplicate capabilities are passed, they will be deduped
-func (cc *CapabilityCache) AddCapabilities(lggr logger.Logger, chain deployment.Chain, registry *capabilities_registry.CapabilitiesRegistry, capabilities []capabilities_registry.CapabilitiesRegistryCapability) []internal.RegisteredCapability {
+func (cc *CapabilityCache) AddCapabilities(_ logger.Logger, chain deployment.Chain, registry *capabilities_registry.CapabilitiesRegistry, capabilities []capabilities_registry.CapabilitiesRegistryCapability) []internal.RegisteredCapability {
 	t := cc.t
 	var out []internal.RegisteredCapability
 	// get the registered capabilities & dedup
 	seen := make(map[capabilities_registry.CapabilitiesRegistryCapability]struct{})
 	var toRegister []capabilities_registry.CapabilitiesRegistryCapability
-	for _, cap := range capabilities {
-		id, cached := cc.nameToId[internal.CapabilityID(cap)]
+	for _, c := range capabilities {
+		id, cached := cc.nameToId[internal.CapabilityID(c)]
 		if cached {
 			out = append(out, internal.RegisteredCapability{
-				CapabilitiesRegistryCapability: cap,
+				CapabilitiesRegistryCapability: c,
 				ID:                             id,
+				Config:                         GetDefaultCapConfig(t, c),
 			})
 			continue
 		}
 		// dedup
-		if _, exists := seen[cap]; !exists {
-			seen[cap] = struct{}{}
-			toRegister = append(toRegister, cap)
+		if _, exists := seen[c]; !exists {
+			seen[c] = struct{}{}
+			toRegister = append(toRegister, c)
 		}
 	}
 	if len(toRegister) == 0 {
@@ -251,6 +253,7 @@ func (cc *CapabilityCache) AddCapabilities(lggr logger.Logger, chain deployment.
 		out = append(out, internal.RegisteredCapability{
 			CapabilitiesRegistryCapability: capb,
 			ID:                             id,
+			Config:                         GetDefaultCapConfig(t, capb),
 		})
 		// cache the id
 		cc.nameToId[internal.CapabilityID(capb)] = id
@@ -294,3 +297,29 @@ func MustCapabilityId(t *testing.T, registry *capabilities_registry.Capabilities
 	require.NoError(t, err)
 	return id
 }
+
+func GetDefaultCapConfig(t *testing.T, capability capabilities_registry.CapabilitiesRegistryCapability) *capabilitiespb.CapabilityConfig {
+	t.Helper()
+	defaultCfg := &capabilitiespb.CapabilityConfig{
+		DefaultConfig: values.Proto(values.EmptyMap()).GetMapValue(),
+	}
+	switch capability.CapabilityType {
+	case uint8(0): // trigger
+		defaultCfg.RemoteConfig = &capabilitiespb.CapabilityConfig_RemoteTriggerConfig{
+			RemoteTriggerConfig: &capabilitiespb.RemoteTriggerConfig{
+				RegistrationRefresh:     durationpb.New(20 * time.Second),
+				RegistrationExpiry:      durationpb.New(60 * time.Second),
+				MinResponsesToAggregate: uint32(10),
+			},
+		}
+	case uint8(3): // target
+		defaultCfg.RemoteConfig = &capabilitiespb.CapabilityConfig_RemoteTargetConfig{
+			RemoteTargetConfig: &capabilitiespb.RemoteTargetConfig{
+				RequestHashExcludedAttributes: []string{"signed_report.Signatures"},
+			},
+		}
+	case uint8(2): // consensus
+	default:
+	}
+	return defaultCfg
+}

deployment/keystone/changeset/internal/types.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"github.com/ethereum/go-ethereum/common"
+	capabilitiespb "github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb"
 
 	chainsel "github.com/smartcontractkit/chain-selectors"
 
@@ -131,7 +132,12 @@ type DonCapabilities struct {
 	Name         string
 	F            uint8
 	Nops         []NOP
-	Capabilities []kcr.CapabilitiesRegistryCapability // every capability is hosted on each nop
+	Capabilities []DONCapabilityWithConfig // every capability is hosted on each nop
+}
+
+type DONCapabilityWithConfig struct {
+	Capability kcr.CapabilitiesRegistryCapability
+	Config     *capabilitiespb.CapabilityConfig
 }
 
 func (v DonCapabilities) Validate() error {
@@ -195,8 +201,8 @@ func nopsToNodes(donInfos []DonInfo, dons []DonCapabilities, chainSelector uint6
 }
 
 // mapDonsToCaps converts a list of DonCapabilities to a map of don name to capabilities
-func mapDonsToCaps(dons []DonInfo) map[string][]kcr.CapabilitiesRegistryCapability {
-	out := make(map[string][]kcr.CapabilitiesRegistryCapability)
+func mapDonsToCaps(dons []DonInfo) map[string][]DONCapabilityWithConfig {
+	out := make(map[string][]DONCapabilityWithConfig)
 	for _, don := range dons {
 		out[don.Name] = don.Capabilities
 	}