refactor(keyston/changeset): minor refactor

deployment/keystone/changeset/internal/deploy.go

@@ -733,27 +733,10 @@ func RegisterNodes(lggr logger.Logger, req *RegisterNodesRequest) (*RegisterNode
 	}
 
 	lggr.Debugw("checking for existing nodes", "count", len(nodeIDToParams))
-	nodes2Add := make([]capabilities_registry.CapabilitiesRegistryNodeParams, 0)
-	for nodeID, nodeParams := range nodeIDToParams {
-		var ni capabilities_registry.INodeInfoProviderNodeInfo
-		if ni, err = registry.GetNode(&bind.CallOpts{}, nodeParams.P2pId); err != nil {
-			if err = deployment.DecodeErr(capabilities_registry.CapabilitiesRegistryABI, err); strings.Contains(err.Error(), "NodeDoesNotExist") {
-				nodes2Add = append(nodes2Add, nodeParams)
-				continue
-			}
-			return nil, fmt.Errorf("failed to call GetNode for node %s: %w", nodeID, err)
-		}
 
-		// if no error, but node info is empty, then the node does not exist and should be added.
-		if hex.EncodeToString(ni.P2pId[:]) != hex.EncodeToString(nodeParams.P2pId[:]) && hex.EncodeToString(ni.P2pId[:]) == "0000000000000000000000000000000000000000000000000000000000000000" {
-			lggr.Debugw("node came back empty, adding it",
-				"p2p bytes", nodeParams.P2pId,
-				"p2pid", hex.EncodeToString(nodeParams.P2pId[:]),
-				"nodeID", nodeID, "don", nodeIDToDon[nodeID], "ni", ni.P2pId,
-				"ni p2pid", hex.EncodeToString(ni.P2pId[:]))
-			nodes2Add = append(nodes2Add, nodeParams)
-			continue
-		}
+	nodes2Add, err := getNodesToRegister(registry, nodeIDToParams)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get nodes to register: %w", err)
 	}
 
 	lggr.Debugf("found %d missing nodes", len(nodes2Add))
@@ -818,6 +801,34 @@ func RegisterNodes(lggr logger.Logger, req *RegisterNodesRequest) (*RegisterNode
 	}, nil
 }
 
+// getNodesToRegister returns the nodes that are not already registered in the registry
+func getNodesToRegister(
+	registry *capabilities_registry.CapabilitiesRegistry,
+	nodeIDToParams map[string]capabilities_registry.CapabilitiesRegistryNodeParams,
+) ([]capabilities_registry.CapabilitiesRegistryNodeParams, error) {
+	nodes2Add := make([]capabilities_registry.CapabilitiesRegistryNodeParams, 0)
+	for nodeID, nodeParams := range nodeIDToParams {
+		var (
+			ni  capabilities_registry.INodeInfoProviderNodeInfo
+			err error
+		)
+		if ni, err = registry.GetNode(&bind.CallOpts{}, nodeParams.P2pId); err != nil {
+			if err = deployment.DecodeErr(capabilities_registry.CapabilitiesRegistryABI, err); strings.Contains(err.Error(), "NodeDoesNotExist") {
+				nodes2Add = append(nodes2Add, nodeParams)
+				continue
+			}
+			return nil, fmt.Errorf("failed to call GetNode for node %s: %w", nodeID, err)
+		}
+
+		// if no error, but node info is empty, then the node does not exist and should be added.
+		if hex.EncodeToString(ni.P2pId[:]) != hex.EncodeToString(nodeParams.P2pId[:]) && hex.EncodeToString(ni.P2pId[:]) == "0000000000000000000000000000000000000000000000000000000000000000" {
+			nodes2Add = append(nodes2Add, nodeParams)
+			continue
+		}
+	}
+	return nodes2Add, nil
+}
+
 // addNodesMCMSProposal generates a single call to AddNodes for all the node params at once.
 func addNodesMCMSProposal(registry *capabilities_registry.CapabilitiesRegistry, params []capabilities_registry.CapabilitiesRegistryNodeParams, regChain deployment.Chain) (*timelock.BatchChainOperation, error) {
 	tx, err := registry.AddNodes(deployment.SimTransactOpts(), params)

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

@@ -30,9 +30,14 @@ type Don struct {
 }
 
 type SetupTestRegistryRequest struct {
+	// P2pToCapabilities maps a node's p2pID to the capabilities it has
 	P2pToCapabilities map[p2pkey.PeerID][]capabilities_registry.CapabilitiesRegistryCapability
-	NopToNodes        map[capabilities_registry.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc
-	Dons              []Don
+
+	// NopToNodes maps a node operator to the nodes they operate
+	NopToNodes map[capabilities_registry.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc
+
+	// Dons groups the p2pIDs of the nodes that comprise it and the capabilities they have
+	Dons []Don
 	// TODO maybe add support for MCMS at this level
 }
 
@@ -80,11 +85,15 @@ func SetupTestRegistry(t *testing.T, lggr logger.Logger, req *SetupTestRegistryR
 	}
 }
 
+// ToNodeParams transforms a map of node operators to nops and a map of node p2pID to capabilities
+// into a slice of node params required to register the nodes.  The number of capabilities
+// must match the number of nodes.
 func ToNodeParams(t *testing.T,
 	nop2Nodes map[capabilities_registry.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc,
 	p2pToCapabilities map[p2pkey.PeerID][][32]byte,
 ) []capabilities_registry.CapabilitiesRegistryNodeParams {
 	t.Helper()
+
 	var nodeParams []capabilities_registry.CapabilitiesRegistryNodeParams
 	var i uint32
 	for _, p2pSignerEncs := range nop2Nodes {
@@ -102,6 +111,7 @@ func ToNodeParams(t *testing.T,
 		}
 		i++
 	}
+
 	return nodeParams
 }
 
@@ -221,7 +231,13 @@ func addNops(t *testing.T, lggr logger.Logger, chain deployment.Chain, registry
 	return resp
 }
 
-func AddNodes(t *testing.T, lggr logger.Logger, chain deployment.Chain, registry *capabilities_registry.CapabilitiesRegistry, nodes []capabilities_registry.CapabilitiesRegistryNodeParams) {
+func AddNodes(
+	t *testing.T,
+	lggr logger.Logger,
+	chain deployment.Chain,
+	registry *capabilities_registry.CapabilitiesRegistry,
+	nodes []capabilities_registry.CapabilitiesRegistryNodeParams,
+) {
 	tx, err := registry.AddNodes(chain.DeployerKey, nodes)
 	if err != nil {
 		err2 := deployment.DecodeErr(capabilities_registry.CapabilitiesRegistryABI, err)
@@ -231,7 +247,14 @@ func AddNodes(t *testing.T, lggr logger.Logger, chain deployment.Chain, registry
 	require.NoError(t, err)
 }
 
-func addDons(t *testing.T, lggr logger.Logger, chain deployment.Chain, registry *capabilities_registry.CapabilitiesRegistry, capCache *CapabilityCache, dons []Don) {
+func addDons(
+	t *testing.T,
+	_ logger.Logger,
+	chain deployment.Chain,
+	registry *capabilities_registry.CapabilitiesRegistry,
+	capCache *CapabilityCache,
+	dons []Don,
+) {
 	for _, don := range dons {
 		acceptsWorkflows := false
 		// lookup the capabilities