Refactor DefaultCapConfig into multiple funcs

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,21 +15,19 @@ import (
 
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
+	capabilitiespb "github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb"
+	"github.com/smartcontractkit/chainlink-common/pkg/values"
 	"golang.org/x/exp/maps"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/durationpb"
 
 	"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"
-
 	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"
 
@@ -142,7 +140,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 +413,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
 }
 
 type RegisterCapabilitiesResponse struct {
@@ -424,21 +422,30 @@ type RegisterCapabilitiesResponse struct {
 
 type RegisteredCapability struct {
 	capabilities_registry.CapabilitiesRegistryCapability
-	ID [32]byte
+	ID     [32]byte
+	Config []byte
 }
 
-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)
+	}
+	cfgB, err := proto.Marshal(cfg)
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal config for capability %v: %w", capReg, err)
 	}
 	return &RegisteredCapability{
-		CapabilitiesRegistryCapability: *cap,
+		CapabilitiesRegistryCapability: *capReg,
 		ID:                             id,
+		Config:                         cfgB,
 	}, nil
 }
 
@@ -465,18 +472,19 @@ func RegisterCapabilities(lggr logger.Logger, req RegisterCapabilitiesRequest) (
 	for don, caps := range req.DonToCapabilities {
 		var registerCaps []RegisteredCapability
 		for _, cap := range caps {
-			id, ok := uniqueCaps[cap]
+			id, ok := uniqueCaps[cap.Capability]
 			if !ok {
 				var err error
-				id, err = registry.GetHashedCapabilityId(&bind.CallOpts{}, cap.LabelledName, cap.Version)
+				id, err = registry.GetHashedCapabilityId(&bind.CallOpts{}, cap.Capability.LabelledName, cap.Capability.Version)
 				if err != nil {
 					return nil, fmt.Errorf("failed to call GetHashedCapabilityId for capability %v: %w", cap, err)
 				}
-				uniqueCaps[cap] = id
+				uniqueCaps[cap.Capability] = id
 			}
 			registerCap := RegisteredCapability{
-				CapabilitiesRegistryCapability: cap,
 				ID:                             id,
+				Config:                         cap.Config,
+				CapabilitiesRegistryCapability: cap.Capability,
 			}
 			lggr.Debugw("hashed capability id", "capability", cap, "id", id)
 			registerCaps = append(registerCaps, registerCap)
@@ -575,41 +583,6 @@ func RegisterNOPS(ctx context.Context, lggr logger.Logger, req RegisterNOPSReque
 	return resp, 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
@@ -836,25 +809,22 @@ func RegisterDons(lggr logger.Logger, req RegisterDonsRequest) (*RegisterDonsRes
 			continue
 		}
 
-		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)
 			}
 			cfgs = append(cfgs, capabilities_registry.CapabilitiesRegistryCapabilityConfiguration{
-				CapabilityId: cap.ID,
-				Config:       cfgb,
+				CapabilityId: regCap.ID,
+				Config:       regCap.Config,
 			})
 		}
 
@@ -970,3 +940,28 @@ func configureForwarder(lggr logger.Logger, chain deployment.Chain, contractSet
 	}
 	return opMap, nil
 }
+
+func GetDefaultCapConfig(capability capabilities_registry.CapabilitiesRegistryCapability) ([]byte, error) {
+	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 proto.Marshal(defaultCfg)
+}

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

@@ -7,16 +7,14 @@ import (
 	"testing"
 
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
-	"github.com/stretchr/testify/require"
-	"google.golang.org/protobuf/proto"
-
 	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/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
 
 	"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 +209,28 @@ 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 {
+			cfgB, err := internal.GetDefaultCapConfig(c)
+			require.NoError(t, err)
 			out = append(out, internal.RegisteredCapability{
-				CapabilitiesRegistryCapability: cap,
+				CapabilitiesRegistryCapability: c,
 				ID:                             id,
+				Config:                         cfgB,
 			})
 			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 {
@@ -248,9 +249,12 @@ func (cc *CapabilityCache) AddCapabilities(lggr logger.Logger, chain deployment.
 		capb := capb
 		id, err := registry.GetHashedCapabilityId(&bind.CallOpts{}, capb.LabelledName, capb.Version)
 		require.NoError(t, err)
+		cfgB, err := internal.GetDefaultCapConfig(capb)
+		require.NoError(t, err)
 		out = append(out, internal.RegisteredCapability{
 			CapabilitiesRegistryCapability: capb,
 			ID:                             id,
+			Config:                         cfgB,
 		})
 		// cache the id
 		cc.nameToId[internal.CapabilityID(capb)] = id

deployment/keystone/changeset/internal/types.go

@@ -131,7 +131,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     []byte
 }
 
 func (v DonCapabilities) Validate() error {
@@ -195,8 +200,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
 	}

deployment/keystone/changeset/internal/update_don.go

@@ -11,15 +11,14 @@ import (
 	"sort"
 
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
+
 	"github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/mcms"
 	"github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/timelock"
-	"google.golang.org/protobuf/proto"
-
 	"github.com/smartcontractkit/chainlink-common/pkg/logger"
-	"github.com/smartcontractkit/chainlink/deployment"
-	"github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/p2pkey"
 
+	"github.com/smartcontractkit/chainlink/deployment"
 	kcr "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry_1_1_0"
+	"github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/p2pkey"
 )
 
 // CapabilityConfig is a struct that holds a capability and its configuration
@@ -71,7 +70,7 @@ type UpdateDonResponse struct {
 	Ops     *timelock.BatchChainOperation
 }
 
-func UpdateDon(lggr logger.Logger, req *UpdateDonRequest) (*UpdateDonResponse, error) {
+func UpdateDon(_ logger.Logger, req *UpdateDonRequest) (*UpdateDonResponse, error) {
 	if err := req.Validate(); err != nil {
 		return nil, fmt.Errorf("failed to validate request: %w", err)
 	}
@@ -86,7 +85,7 @@ func UpdateDon(lggr logger.Logger, req *UpdateDonRequest) (*UpdateDonResponse, e
 	if err != nil {
 		return nil, fmt.Errorf("failed to lookup don by p2pIDs: %w", err)
 	}
-	cfgs, err := computeConfigs(registry, req.CapabilityConfigs, don)
+	cfgs, err := computeConfigs(registry, req.CapabilityConfigs)
 	if err != nil {
 		return nil, fmt.Errorf("failed to compute configs: %w", err)
 	}
@@ -140,22 +139,18 @@ func BytesToPeerIDs(p2pIDs [][32]byte) []p2pkey.PeerID {
 	return out
 }
 
-func computeConfigs(registry *kcr.CapabilitiesRegistry, caps []CapabilityConfig, donInfo kcr.CapabilitiesRegistryDONInfo) ([]kcr.CapabilitiesRegistryCapabilityConfiguration, error) {
-	out := make([]kcr.CapabilitiesRegistryCapabilityConfiguration, len(caps))
-	for i, cap := range caps {
+func computeConfigs(registry *kcr.CapabilitiesRegistry, capCfgs []CapabilityConfig) ([]kcr.CapabilitiesRegistryCapabilityConfiguration, error) {
+	out := make([]kcr.CapabilitiesRegistryCapabilityConfiguration, len(capCfgs))
+	for i, capCfg := range capCfgs {
 		out[i] = kcr.CapabilitiesRegistryCapabilityConfiguration{}
-		id, err := registry.GetHashedCapabilityId(&bind.CallOpts{}, cap.Capability.LabelledName, cap.Capability.Version)
+		id, err := registry.GetHashedCapabilityId(&bind.CallOpts{}, capCfg.Capability.LabelledName, capCfg.Capability.Version)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get capability id: %w", err)
 		}
 		out[i].CapabilityId = id
+		out[i].Config = capCfg.Config
 		if out[i].Config == nil {
-			c := DefaultCapConfig(cap.Capability.CapabilityType, int(donInfo.F))
-			cb, err := proto.Marshal(c)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal capability config for %v: %w", c, err)
-			}
-			out[i].Config = cb
+			return nil, fmt.Errorf("config is required for capability %s", capCfg.Capability.LabelledName)
 		}
 	}
 	return out, nil