Ks/update nodes cs cleanup (#15080)

* update capability view defintion

* test wip

* wip, better types for view

* finish type refactoring

* cleanup

* more denormalized form

* code cleanup

* simplify helpers

* clean update node changeset to add helpers

* take new code

* unify capability mutation request

deployment/keystone/changeset/append_node_capbilities.go

@@ -3,56 +3,17 @@ package changeset
 import (
 	"fmt"
 
-	chainsel "github.com/smartcontractkit/chain-selectors"
-	kcr "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry"
-	"github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/p2pkey"
-
 	"github.com/smartcontractkit/chainlink/deployment"
 	kslib "github.com/smartcontractkit/chainlink/deployment/keystone"
 	"github.com/smartcontractkit/chainlink/deployment/keystone/changeset/internal"
 )
 
 var _ deployment.ChangeSet = AppendNodeCapabilities
 
-type AppendNodeCapabilitiesRequest struct {
-	AddressBook      deployment.AddressBook
-	RegistryChainSel uint64
-
-	P2pToCapabilities map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability
-	NopToNodes        map[kcr.CapabilitiesRegistryNodeOperator][]*P2PSignerEnc
-}
-
-func (req *AppendNodeCapabilitiesRequest) Validate() error {
-	if len(req.P2pToCapabilities) == 0 {
-		return fmt.Errorf("p2pToCapabilities is empty")
-	}
-	if len(req.NopToNodes) == 0 {
-		return fmt.Errorf("nopToNodes is empty")
-	}
-	if req.AddressBook == nil {
-		return fmt.Errorf("registry is nil")
-	}
-	_, exists := chainsel.ChainBySelector(req.RegistryChainSel)
-	if !exists {
-		return fmt.Errorf("registry chain selector %d does not exist", req.RegistryChainSel)
-	}
-
-	return nil
-}
-
-/*
-// AppendNodeCapabilibity adds any new capabilities to the registry, merges the new capabilities with the existing capabilities
-// of the node, and updates the nodes in the registry host the union of the new and existing capabilities.
-func AppendNodeCapabilities(lggr logger.Logger, req *AppendNodeCapabilitiesRequest) (deployment.ChangesetOutput, error) {
-	_, err := appendNodeCapabilitiesImpl(lggr, req)
-	if err != nil {
-		return deployment.ChangesetOutput{}, err
-	}
-	return deployment.ChangesetOutput{}, nil
-}
-*/
+// AppendNodeCapabilitiesRequest is a request to add capabilities to the existing capabilities of nodes in the registry
+type AppendNodeCapabilitiesRequest = MutateNodeCapabilitiesRequest
 
-// AppendNodeCapabilibity adds any new capabilities to the registry, merges the new capabilities with the existing capabilities
+// AppendNodeCapabilities adds any new capabilities to the registry, merges the new capabilities with the existing capabilities
 // of the node, and updates the nodes in the registry host the union of the new and existing capabilities.
 func AppendNodeCapabilities(env deployment.Environment, config any) (deployment.ChangesetOutput, error) {
 	req, ok := config.(*AppendNodeCapabilitiesRequest)
@@ -79,7 +40,7 @@ func (req *AppendNodeCapabilitiesRequest) convert(e deployment.Environment) (*in
 	if !ok {
 		return nil, fmt.Errorf("registry chain selector %d does not exist in environment", req.RegistryChainSel)
 	}
-	contracts, err := kslib.GetContractSets(&kslib.GetContractSetsRequest{
+	contracts, err := kslib.GetContractSets(e.Logger, &kslib.GetContractSetsRequest{
 		Chains:      map[uint64]deployment.Chain{req.RegistryChainSel: registryChain},
 		AddressBook: req.AddressBook,
 	})

deployment/keystone/changeset/update_node_capabilities.go

@@ -6,6 +6,7 @@ import (
 
 	chainsel "github.com/smartcontractkit/chain-selectors"
 	"github.com/smartcontractkit/chainlink/deployment"
+	"github.com/smartcontractkit/chainlink/deployment/environment/clo/models"
 	kslib "github.com/smartcontractkit/chainlink/deployment/keystone"
 	"github.com/smartcontractkit/chainlink/deployment/keystone/changeset/internal"
 
@@ -17,15 +18,31 @@ var _ deployment.ChangeSet = UpdateNodeCapabilities
 
 type P2PSignerEnc = internal.P2PSignerEnc
 
-type UpdateNodeCapabilitiesRequest struct {
+func NewP2PSignerEnc(n *models.Node, registryChainSel uint64) (*P2PSignerEnc, error) {
+	p2p, signer, enc, err := kslib.ExtractKeys(n, registryChainSel)
+	if err != nil {
+		return nil, fmt.Errorf("failed to extract keys: %w", err)
+	}
+	return &P2PSignerEnc{
+		Signer:              signer,
+		P2PKey:              p2p,
+		EncryptionPublicKey: enc,
+	}, nil
+}
+
+// UpdateNodeCapabilitiesRequest is a request to set the capabilities of nodes in the registry
+type UpdateNodeCapabilitiesRequest = MutateNodeCapabilitiesRequest
+
+// MutateNodeCapabilitiesRequest is a request to change the capabilities of nodes in the registry
+type MutateNodeCapabilitiesRequest struct {
 	AddressBook      deployment.AddressBook
 	RegistryChainSel uint64
 
 	P2pToCapabilities map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability
 	NopToNodes        map[kcr.CapabilitiesRegistryNodeOperator][]*P2PSignerEnc
 }
 
-func (req *UpdateNodeCapabilitiesRequest) Validate() error {
+func (req *MutateNodeCapabilitiesRequest) Validate() error {
 	if req.AddressBook == nil {
 		return fmt.Errorf("address book is nil")
 	}
@@ -39,40 +56,19 @@ func (req *UpdateNodeCapabilitiesRequest) Validate() error {
 	if !exists {
 		return fmt.Errorf("registry chain selector %d does not exist", req.RegistryChainSel)
 	}
-	return nil
-}
-
-type UpdateNodeCapabilitiesImplRequest struct {
-	Chain    deployment.Chain
-	Registry *kcr.CapabilitiesRegistry
-
-	P2pToCapabilities map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability
-	NopToNodes        map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc
-}
-
-func (req *UpdateNodeCapabilitiesImplRequest) Validate() error {
-	if len(req.P2pToCapabilities) == 0 {
-		return fmt.Errorf("p2pToCapabilities is empty")
-	}
-	if len(req.NopToNodes) == 0 {
-		return fmt.Errorf("nopToNodes is empty")
-	}
-	if req.Registry == nil {
-		return fmt.Errorf("registry is nil")
-	}
 
 	return nil
 }
 
-func (req *UpdateNodeCapabilitiesRequest) updateNodeCapabilitiesImplRequest(e deployment.Environment) (*internal.UpdateNodeCapabilitiesImplRequest, error) {
+func (req *MutateNodeCapabilitiesRequest) updateNodeCapabilitiesImplRequest(e deployment.Environment) (*internal.UpdateNodeCapabilitiesImplRequest, error) {
 	if err := req.Validate(); err != nil {
 		return nil, fmt.Errorf("failed to validate UpdateNodeCapabilitiesRequest: %w", err)
 	}
 	registryChain, ok := e.Chains[req.RegistryChainSel]
 	if !ok {
 		return nil, fmt.Errorf("registry chain selector %d does not exist in environment", req.RegistryChainSel)
 	}
-	contracts, err := kslib.GetContractSets(&kslib.GetContractSetsRequest{
+	contracts, err := kslib.GetContractSets(e.Logger, &kslib.GetContractSetsRequest{
 		Chains:      map[uint64]deployment.Chain{req.RegistryChainSel: registryChain},
 		AddressBook: req.AddressBook,
 	})
@@ -83,6 +79,7 @@ func (req *UpdateNodeCapabilitiesRequest) updateNodeCapabilitiesImplRequest(e de
 	if registry == nil {
 		return nil, fmt.Errorf("capabilities registry not found for chain %d", req.RegistryChainSel)
 	}
+
 	return &internal.UpdateNodeCapabilitiesImplRequest{
 		Chain:             registryChain,
 		Registry:          registry,
@@ -93,9 +90,9 @@ func (req *UpdateNodeCapabilitiesRequest) updateNodeCapabilitiesImplRequest(e de
 
 // UpdateNodeCapabilities updates the capabilities of nodes in the registry
 func UpdateNodeCapabilities(env deployment.Environment, config any) (deployment.ChangesetOutput, error) {
-	req, ok := config.(*UpdateNodeCapabilitiesRequest)
+	req, ok := config.(*MutateNodeCapabilitiesRequest)
 	if !ok {
-		return deployment.ChangesetOutput{}, fmt.Errorf("invalid config type. want %T, got %T", &UpdateNodeCapabilitiesRequest{}, config)
+		return deployment.ChangesetOutput{}, fmt.Errorf("invalid config type. want %T, got %T", &MutateNodeCapabilitiesRequest{}, config)
 	}
 	c, err := req.updateNodeCapabilitiesImplRequest(env)
 	if err != nil {

deployment/keystone/changeset/view.go

@@ -14,7 +14,7 @@ import (
 var _ deployment.ViewState = ViewKeystone
 
 func ViewKeystone(e deployment.Environment) (json.Marshaler, error) {
-	state, err := keystone.GetContractSets(&keystone.GetContractSetsRequest{
+	state, err := keystone.GetContractSets(e.Logger, &keystone.GetContractSetsRequest{
 		Chains:      e.Chains,
 		AddressBook: e.ExistingAddresses,
 	})

deployment/keystone/deploy.go

@@ -145,7 +145,7 @@ func ConfigureRegistry(ctx context.Context, lggr logger.Logger, req ConfigureCon
 		return nil, fmt.Errorf("chain %d not found in environment", req.RegistryChainSel)
 	}
 
-	contractSetsResp, err := GetContractSets(&GetContractSetsRequest{
+	contractSetsResp, err := GetContractSets(req.Env.Logger, &GetContractSetsRequest{
 		Chains:      req.Env.Chains,
 		AddressBook: addrBook,
 	})
@@ -244,7 +244,7 @@ func ConfigureRegistry(ctx context.Context, lggr logger.Logger, req ConfigureCon
 // ConfigureForwardContracts configures the forwarder contracts on all chains for the given DONS
 // the address book is required to contain the an address of the deployed forwarder contract for every chain in the environment
 func ConfigureForwardContracts(env *deployment.Environment, dons []RegisteredDon, addrBook deployment.AddressBook) error {
-	contractSetsResp, err := GetContractSets(&GetContractSetsRequest{
+	contractSetsResp, err := GetContractSets(env.Logger, &GetContractSetsRequest{
 		Chains:      env.Chains,
 		AddressBook: addrBook,
 	})
@@ -279,7 +279,7 @@ func ConfigureOCR3Contract(env *deployment.Environment, chainSel uint64, dons []
 		return fmt.Errorf("chain %d not found in environment", chainSel)
 	}
 
-	contractSetsResp, err := GetContractSets(&GetContractSetsRequest{
+	contractSetsResp, err := GetContractSets(env.Logger, &GetContractSetsRequest{
 		Chains:      env.Chains,
 		AddressBook: addrBook,
 	})
@@ -319,7 +319,7 @@ func ConfigureOCR3ContractFromCLO(env *deployment.Environment, chainSel uint64,
 	if !ok {
 		return fmt.Errorf("chain %d not found in environment", chainSel)
 	}
-	contractSetsResp, err := GetContractSets(&GetContractSetsRequest{
+	contractSetsResp, err := GetContractSets(env.Logger, &GetContractSetsRequest{
 		Chains:      env.Chains,
 		AddressBook: addrBook,
 	})

deployment/keystone/deploy_test.go

@@ -112,7 +112,7 @@ func TestDeploy(t *testing.T) {
 		AddressBook: ad,
 	}
 
-	contractSetsResp, err := keystone.GetContractSets(req)
+	contractSetsResp, err := keystone.GetContractSets(lggr, req)
 	require.NoError(t, err)
 	require.Len(t, contractSetsResp.ContractSets, len(env.Chains))
 	// check the registry

deployment/keystone/state.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 
+	"github.com/smartcontractkit/chainlink-common/pkg/logger"
 	"github.com/smartcontractkit/chainlink/deployment"
 	common_v1_0 "github.com/smartcontractkit/chainlink/deployment/common/view/v1_0"
 	"github.com/smartcontractkit/chainlink/deployment/keystone/view"
@@ -40,7 +41,7 @@ func (cs ContractSet) View() (view.KeystoneChainView, error) {
 	return out, nil
 }
 
-func GetContractSets(req *GetContractSetsRequest) (*GetContractSetsResponse, error) {
+func GetContractSets(lggr logger.Logger, req *GetContractSetsRequest) (*GetContractSetsResponse, error) {
 	resp := &GetContractSetsResponse{
 		ContractSets: make(map[uint64]ContractSet),
 	}
@@ -49,7 +50,7 @@ func GetContractSets(req *GetContractSetsRequest) (*GetContractSetsResponse, err
 		if err != nil {
 			return nil, fmt.Errorf("failed to get addresses for chain %d: %w", id, err)
 		}
-		cs, err := loadContractSet(chain, addrs)
+		cs, err := loadContractSet(lggr, chain, addrs)
 		if err != nil {
 			return nil, fmt.Errorf("failed to load contract set for chain %d: %w", id, err)
 		}
@@ -58,7 +59,7 @@ func GetContractSets(req *GetContractSetsRequest) (*GetContractSetsResponse, err
 	return resp, nil
 }
 
-func loadContractSet(chain deployment.Chain, addresses map[string]deployment.TypeAndVersion) (*ContractSet, error) {
+func loadContractSet(lggr logger.Logger, chain deployment.Chain, addresses map[string]deployment.TypeAndVersion) (*ContractSet, error) {
 	var out ContractSet
 
 	for addr, tv := range addresses {
@@ -83,7 +84,8 @@ func loadContractSet(chain deployment.Chain, addresses map[string]deployment.Typ
 			}
 			out.OCR3 = c
 		default:
-			return nil, fmt.Errorf("unknown contract type %s", tv.Type)
+			lggr.Warnw("unknown contract type", "type", tv.Type)
+			// ignore unknown contract types
 		}
 	}
 	return &out, nil

deployment/keystone/types.go

@@ -100,6 +100,7 @@ func (o *ocr2Node) toNodeKeys() NodeKeys {
 		AptosOnchainPublicKey: aptosOnchainPublicKey,
 	}
 }
+
 func newOcr2NodeFromClo(n *models.Node, registryChainSel uint64) (*ocr2Node, error) {
 	if n.PublicKey == nil {
 		return nil, errors.New("no public key")
@@ -123,6 +124,14 @@ func newOcr2NodeFromClo(n *models.Node, registryChainSel uint64) (*ocr2Node, err
 	return newOcr2Node(n.ID, cfgs, *n.PublicKey)
 }
 
+func ExtractKeys(n *models.Node, registerChainSel uint64) (p2p p2pkey.PeerID, signer [32]byte, encPubKey [32]byte, err error) {
+	orc2n, err := newOcr2NodeFromClo(n, registerChainSel)
+	if err != nil {
+		return p2p, signer, encPubKey, fmt.Errorf("failed to create ocr2 node for node %s: %w", n.ID, err)
+	}
+	return orc2n.P2PKey, orc2n.Signer, orc2n.EncryptionPublicKey, nil
+}
+
 func newOcr2Node(id string, ccfgs map[chaintype.ChainType]*v1.ChainConfig, csaPubKey string) (*ocr2Node, error) {
 	if ccfgs == nil {
 		return nil, errors.New("nil ocr2config")
@@ -202,32 +211,41 @@ type DonCapabilities struct {
 
 // map the node id to the NOP
 func (dc DonCapabilities) nodeIdToNop(cs uint64) (map[string]capabilities_registry.CapabilitiesRegistryNodeOperator, error) {
-	cid, err := chainsel.ChainIdFromSelector(cs)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get chain id from selector %d: %w", cs, err)
-	}
-	cidStr := strconv.FormatUint(cid, 10)
 	out := make(map[string]capabilities_registry.CapabilitiesRegistryNodeOperator)
 	for _, nop := range dc.Nops {
 		for _, node := range nop.Nodes {
-			found := false
-			for _, chain := range node.ChainConfigs {
-				if chain.Network.ChainID == cidStr {
-					found = true
-					out[node.ID] = capabilities_registry.CapabilitiesRegistryNodeOperator{
-						Name:  nop.Name,
-						Admin: adminAddr(chain.AdminAddress),
-					}
-				}
-			}
-			if !found {
-				return nil, fmt.Errorf("node '%s' %s does not support chain %d", node.Name, node.ID, cid)
+			a, err := AdminAddress(node, cs)
+			if err != nil {
+				return nil, fmt.Errorf("failed to get admin address for node %s: %w", node.ID, err)
 			}
+			out[node.ID] = NodeOperator(dc.Name, a)
+
 		}
 	}
 	return out, nil
 }
 
+func NodeOperator(name string, adminAddress string) capabilities_registry.CapabilitiesRegistryNodeOperator {
+	return capabilities_registry.CapabilitiesRegistryNodeOperator{
+		Name:  name,
+		Admin: adminAddr(adminAddress),
+	}
+}
+
+func AdminAddress(n *models.Node, chainSel uint64) (string, error) {
+	cid, err := chainsel.ChainIdFromSelector(chainSel)
+	if err != nil {
+		return "", fmt.Errorf("failed to get chain id from selector %d: %w", chainSel, err)
+	}
+	cidStr := strconv.FormatUint(cid, 10)
+	for _, chain := range n.ChainConfigs {
+		if chain.Network.ChainID == cidStr {
+			return chain.AdminAddress, nil
+		}
+	}
+	return "", fmt.Errorf("no chain config for chain %d", cid)
+}
+
 // helpers to maintain compatibility with the existing registration functions
 // nodesToNops converts a list of DonCapabilities to a map of node id to NOP
 func nodesToNops(dons []DonCapabilities, chainSel uint64) (map[string]capabilities_registry.CapabilitiesRegistryNodeOperator, error) {