code cleanup

deployment/keystone/changeset/internal/append_node_capabilities.go

@@ -15,7 +15,6 @@ type AppendNodeCapabilitiesRequest struct {
 	Registry *kcr.CapabilitiesRegistry
 
 	P2pToCapabilities map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability
-	// NopToNodes        map[kcr.CapabilitiesRegistryNodeOperator][]*P2PSignerEnc
 }
 
 func (req *AppendNodeCapabilitiesRequest) Validate() error {
@@ -56,7 +55,6 @@ func AppendNodeCapabilitiesImpl(lggr logger.Logger, req *AppendNodeCapabilitiesR
 		Chain:             req.Chain,
 		Registry:          req.Registry,
 		P2pToCapabilities: capsByPeer,
-		//		NopToNodes:        req.NopToNodes,
 	}
 	resp, err := UpdateNodes(lggr, updateNodesReq)
 	if err != nil {

deployment/keystone/changeset/internal/append_node_capabilities_test.go

@@ -84,7 +84,6 @@ func TestAppendNodeCapabilities(t *testing.T) {
 							},
 						},
 					},
-					//					NopToNodes: nopToNodes,
 				},
 			},
 			want:    deployment.ChangesetOutput{},
@@ -93,7 +92,6 @@ func TestAppendNodeCapabilities(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// chagen the name and args to be mor egeneral
 			setupResp := kstest.SetupTestRegistry(t, lggr, tt.args.initialState)
 
 			tt.args.req.Registry = setupResp.Registry

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

@@ -68,48 +68,30 @@ func SetupTestRegistry(t *testing.T, lggr logger.Logger, req *SetupTestRegistryR
 	}
 	require.Len(t, registeredCapabilities, len(expectedDeduped))
 
-	// add the nodes with the phony capabilities. cannot register a node without a capability and capability must exist
-	// to do this make an initial phony request and extract the node params
+	// make the nodes and register node
+	var nodeParams []kcr.CapabilitiesRegistryNodeParams
 	initialp2pToCapabilities := make(map[p2pkey.PeerID][][32]byte)
 	for p2pID := range req.P2pToCapabilities {
 		initialp2pToCapabilities[p2pID] = mustCapabilityIds(t, registry, registeredCapabilities)
 	}
-	// register the nodes
-	var np []kcr.CapabilitiesRegistryNodeParams
+	// create node with initial capabilities assigned to nop
 	for i, nop := range nops {
 		if _, exists := req.NopToNodes[nop]; !exists {
 			require.Fail(t, "missing nopToNodes for %s", nop.Name)
 		}
 		for _, p2pSignerEnc := range req.NopToNodes[nop] {
-			np = append(np, kcr.CapabilitiesRegistryNodeParams{
+			nodeParams = append(nodeParams, kcr.CapabilitiesRegistryNodeParams{
 				Signer:              p2pSignerEnc.Signer,
 				P2pId:               p2pSignerEnc.P2PKey,
 				EncryptionPublicKey: p2pSignerEnc.EncryptionPublicKey,
 				HashedCapabilityIds: initialp2pToCapabilities[p2pSignerEnc.P2PKey],
-				NodeOperatorId:      uint32(i + 1), // 1-indexed
+				NodeOperatorId:      uint32(i + 1), // nopid in contract is 1-indexed
 			})
 		}
 	}
-	/*
-		tx, err := registry.AddNodes(chain.DeployerKey, np)
-		if err != nil {
-			err2 := kslib.DecodeErr(kcr.CapabilitiesRegistryABI, err)
-			require.Fail(t, fmt.Sprintf("failed to call AddNodes: %s:  %s", err, err2))
-		}
-		_, err = chain.Confirm(tx)
-		require.NoError(t, err)
-	*/
-	/*	phonyRequest := &internal.UpdateNodesRequest{
-			Chain:             chain,
-			Registry:          registry,
-			P2pToCapabilities: req.P2pToCapabilities,
-			//		NopToNodes:        req.NopToNodes,
-		}
-		nodeParams, err := phonyRequest.NodeParams()
-		require.NoError(t, err)
-	*/
-	nodeParams := np
 	addNodes(t, lggr, chain, registry, nodeParams)
+
+	// add the Dons
 	addDons(t, lggr, chain, registry, capCache, req.Dons)
 
 	return &SetupTestRegistryResponse{

deployment/keystone/changeset/internal/update_don.go

@@ -29,10 +29,8 @@ type UpdateDonRequest struct {
 	Registry *kcr.CapabilitiesRegistry
 	Chain    deployment.Chain
 
-	P2PIDs []p2pkey.PeerID // this is the unique identifier for the done
-	// CapabilityId field is ignored. it is determined dynamically by the registry
-	// If the Config is nil, a default config is used
-	CapabilityConfigs []CapabilityConfig
+	P2PIDs            []p2pkey.PeerID    // this is the unique identifier for the don
+	CapabilityConfigs []CapabilityConfig // if Config subfield is nil, a default config is used
 }
 
 func (r *UpdateDonRequest) appendNodeCapabilitiesRequest() *AppendNodeCapabilitiesRequest {
@@ -75,38 +73,10 @@ func UpdateDon(lggr logger.Logger, req *UpdateDonRequest) (*UpdateDonResponse, e
 	if err != nil {
 		return nil, fmt.Errorf("failed to get Dons: %w", err)
 	}
-	wantedDonID := SortedHash(PeerIDsToBytes(req.P2PIDs))
-	var don kcr.CapabilitiesRegistryDONInfo
-	found := false
-	for i, di := range getDonsResp {
-		gotID := SortedHash(di.NodeP2PIds)
-		if gotID == wantedDonID {
-			don = getDonsResp[i]
-			found = true
-			break
-		}
-	}
-	if !found {
-		type debugDonInfo struct {
-			OnchainID  uint32
-			P2PIDsHash string
-			Want       []p2pkey.PeerID
-			Got        []p2pkey.PeerID
-		}
-		debugIds := make([]debugDonInfo, len(getDonsResp))
-		for i, di := range getDonsResp {
-			debugIds[i] = debugDonInfo{
-				OnchainID:  di.Id,
-				P2PIDsHash: SortedHash(di.NodeP2PIds),
-				Want:       req.P2PIDs,
-				Got:        BytesToPeerIDs(di.NodeP2PIds),
-			}
-		}
-		b, err2 := json.Marshal(debugIds)
-		if err2 == nil {
-			return nil, fmt.Errorf("don not found by p2pIDs %s in %s", wantedDonID, b)
-		}
-		return nil, fmt.Errorf("don not found by p2pIDs %s in %v", wantedDonID, debugIds)
+
+	don, err := lookupDonByPeerIDs(getDonsResp, req.P2PIDs)
+	if err != nil {
+		return nil, fmt.Errorf("failed to lookup don by p2pIDs: %w", err)
 	}
 	cfgs, err := computeConfigs(req.Registry, req.CapabilityConfigs, don)
 	if err != nil {
@@ -180,3 +150,45 @@ func SortedHash(p2pids [][32]byte) string {
 	}
 	return hex.EncodeToString(sha256Hash.Sum(nil))
 }
+
+func lookupDonByPeerIDs(donResp []kcr.CapabilitiesRegistryDONInfo, wanted []p2pkey.PeerID) (kcr.CapabilitiesRegistryDONInfo, error) {
+	var don kcr.CapabilitiesRegistryDONInfo
+	wantedDonID := SortedHash(PeerIDsToBytes(wanted))
+	found := false
+	for i, di := range donResp {
+		gotID := SortedHash(di.NodeP2PIds)
+		if gotID == wantedDonID {
+			don = donResp[i]
+			found = true
+			break
+		}
+	}
+	if !found {
+		return don, verboseDonNotFound(donResp, wanted)
+	}
+	return don, nil
+}
+
+func verboseDonNotFound(donResp []kcr.CapabilitiesRegistryDONInfo, wanted []p2pkey.PeerID) error {
+	type debugDonInfo struct {
+		OnchainID  uint32
+		P2PIDsHash string
+		Want       []p2pkey.PeerID
+		Got        []p2pkey.PeerID
+	}
+	debugIds := make([]debugDonInfo, len(donResp))
+	for i, di := range donResp {
+		debugIds[i] = debugDonInfo{
+			OnchainID:  di.Id,
+			P2PIDsHash: SortedHash(di.NodeP2PIds),
+			Want:       wanted,
+			Got:        BytesToPeerIDs(di.NodeP2PIds),
+		}
+	}
+	wantedID := SortedHash(PeerIDsToBytes(wanted))
+	b, err2 := json.Marshal(debugIds)
+	if err2 == nil {
+		return fmt.Errorf("don not found by p2pIDs %s in %s", wantedID, b)
+	}
+	return fmt.Errorf("don not found by p2pIDs %s in %v", wantedID, debugIds)
+}

deployment/keystone/changeset/internal/update_don_test.go

@@ -25,8 +25,6 @@ import (
 func TestUpdateDon(t *testing.T) {
 	var (
 		registryChain = chainsel.TEST_90000001
-		//nop_1         = "test nop 1"
-		//nop_2         = "test nop 2"
 		// nodes
 		p2p_1     = p2pkey.MustNewV2XXXTestingOnly(big.NewInt(100))
 		pubKey_1  = "11114981a6119ca3f932cdb8c402d71a72d672adae7849f581ecff8b8e1098e7" // valid csa key
@@ -92,7 +90,6 @@ func TestUpdateDon(t *testing.T) {
 			CapabilityType: 1,
 		}
 	)
-	//		p2p_3 = p2pkey.MustNewV2XXXTestingOnly(big.NewInt(300))
 
 	lggr := logger.Test(t)
 

deployment/keystone/changeset/internal/update_node_capabilities.go

@@ -15,16 +15,12 @@ type UpdateNodeCapabilitiesImplRequest struct {
 	Registry *kcr.CapabilitiesRegistry
 
 	P2pToCapabilities map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability
-	NopToNodes        map[kcr.CapabilitiesRegistryNodeOperator][]*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")
 	}
@@ -57,59 +53,3 @@ func UpdateNodeCapabilitiesImpl(lggr logger.Logger, req *UpdateNodeCapabilitiesI
 	}
 	return resp, nil
 }
-
-/*
-// AddCapabilities adds the capabilities to the registry
-// it tries to add all capabilities in one go, if that fails, it falls back to adding them one by one
-func AddCapabilities(lggr logger.Logger, registry *kcr.CapabilitiesRegistry, chain deployment.Chain, capabilities []kcr.CapabilitiesRegistryCapability) error {
-	if len(capabilities) == 0 {
-		return nil
-	}
-	// dedup capabilities
-	var deduped []kcr.CapabilitiesRegistryCapability
-	seen := make(map[string]struct{})
-	for _, cap := range capabilities {
-		if _, ok := seen[CapabilityID(cap)]; !ok {
-			seen[CapabilityID(cap)] = struct{}{}
-			deduped = append(deduped, cap)
-		}
-	}
-
-	tx, err := registry.AddCapabilities(chain.DeployerKey, deduped)
-	if err != nil {
-		err = DecodeErr(kcr.CapabilitiesRegistryABI, err)
-		// no typed errors in the abi, so we have to do string matching
-		// try to add all capabilities in one go, if that fails, fall back to 1-by-1
-		if !strings.Contains(err.Error(), "CapabilityAlreadyExists") {
-			return fmt.Errorf("failed to call AddCapabilities: %w", err)
-		}
-		lggr.Warnw("capabilities already exist, falling back to 1-by-1", "capabilities", deduped)
-		for _, cap := range deduped {
-			tx, err = registry.AddCapabilities(chain.DeployerKey, []kcr.CapabilitiesRegistryCapability{cap})
-			if err != nil {
-				err = DecodeErr(kcr.CapabilitiesRegistryABI, err)
-				if strings.Contains(err.Error(), "CapabilityAlreadyExists") {
-					lggr.Warnw("capability already exists, skipping", "capability", cap)
-					continue
-				}
-				return fmt.Errorf("failed to call AddCapabilities for capability %v: %w", cap, err)
-			}
-			// 1-by-1 tx is pending and we need to wait for it to be mined
-			_, err = chain.Confirm(tx)
-			if err != nil {
-				return fmt.Errorf("failed to confirm AddCapabilities confirm transaction %s: %w", tx.Hash().String(), err)
-			}
-			lggr.Debugw("registered capability", "capability", cap)
-
-		}
-	} else {
-		// the bulk add tx is pending and we need to wait for it to be mined
-		_, err = chain.Confirm(tx)
-		if err != nil {
-			return fmt.Errorf("failed to confirm AddCapabilities confirm transaction %s: %w", tx.Hash().String(), err)
-		}
-		lggr.Info("registered capabilities", "capabilities", deduped)
-	}
-	return nil
-}
-*/

deployment/keystone/changeset/internal/update_node_capabilities_test.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/smartcontractkit/chainlink-common/pkg/logger"
 	"github.com/smartcontractkit/chainlink/deployment"
-	//"github.com/smartcontractkit/chainlink/deployment/keystone/changeset"
 	kslib "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"
@@ -84,7 +83,6 @@ func TestUpdateNodeCapabilities(t *testing.T) {
 							},
 						},
 					},
-					NopToNodes: nopToNodes,
 				},
 			},
 			want:    deployment.ChangesetOutput{},

deployment/keystone/changeset/internal/update_nodes.go

@@ -1,8 +1,10 @@
 package internal
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
+	"sort"
 
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 
@@ -19,7 +21,6 @@ type UpdateNodesRequest struct {
 	Registry *kcr.CapabilitiesRegistry
 
 	P2pToCapabilities map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability
-	//NopToNodes        map[kcr.CapabilitiesRegistryNodeOperator][]*P2PSignerEnc
 }
 
 func (req *UpdateNodesRequest) NodeParams() ([]kcr.CapabilitiesRegistryNodeParams, error) {
@@ -165,6 +166,13 @@ func makeNodeParams(registry *kcr.CapabilitiesRegistry,
 			Signer:              node.Signer,
 		})
 	}
+	sort.Slice(out, func(i, j int) bool {
+		if out[i].NodeOperatorId == out[j].NodeOperatorId {
+			return bytes.Compare(out[i].P2pId[:], out[j].P2pId[:]) < 0
+		}
+		return out[i].NodeOperatorId < out[j].NodeOperatorId
+	})
+
 	return out, nil
 
 }

deployment/keystone/changeset/internal/update_nodes_test.go

@@ -25,10 +25,9 @@ import (
 func Test_UpdateNodesRequest_validate(t *testing.T) {
 	type fields struct {
 		p2pToCapabilities map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability
-		//nopToNodes        map[uint32][]*internal.P2PSigner
-		nopToNodes map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc
-		chain      deployment.Chain
-		registry   *kcr.CapabilitiesRegistry
+		nopToNodes        map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc
+		chain             deployment.Chain
+		registry          *kcr.CapabilitiesRegistry
 	}
 	tests := []struct {
 		name    string
@@ -50,9 +49,8 @@ func Test_UpdateNodesRequest_validate(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			req := &internal.UpdateNodesRequest{
 				P2pToCapabilities: tt.fields.p2pToCapabilities,
-				//	NopToNodes:        tt.fields.nopToNodes,
-				Chain:    tt.fields.chain,
-				Registry: tt.fields.registry,
+				Chain:             tt.fields.chain,
+				Registry:          tt.fields.registry,
 			}
 			if err := req.Validate(); (err != nil) != tt.wantErr {
 				t.Errorf("internal.UpdateNodesRequest.validate() error = %v, wantErr %v", err, tt.wantErr)
@@ -313,8 +311,6 @@ func TestUpdateNodes(t *testing.T) {
 			tt.args.req.Registry = setupResp.Registry
 			tt.args.req.Chain = setupResp.Chain
 
-			//registry := kstest.SetupUpdateNodes(t, tt.args.lggr, tt.args.req)
-			//tt.args.req.Registry = registry
 			// register the capabilities that the Update will use
 			expectedUpdatedCaps := make(map[p2pkey.PeerID][]kslib.RegisteredCapability)
 			capCache := kstest.NewCapabiltyCache(t)
@@ -416,9 +412,8 @@ func TestUpdateNodes(t *testing.T) {
 
 		var req = &internal.UpdateNodesRequest{
 			P2pToCapabilities: p2pToCapabilitiesUpdated,
-			//			NopToNodes:        nopToNodes,
-			Chain:    chain,
-			Registry: registry,
+			Chain:             chain,
+			Registry:          registry,
 		}
 		_, err = internal.UpdateNodes(lggr, req)
 		require.NoError(t, err)

deployment/keystone/changeset/types.go

@@ -45,6 +45,6 @@ func newP2PSignerEncFromJD(ccfg *v1.ChainConfig, pubkey [32]byte) (*P2PSignerEnc
 	return &P2PSignerEnc{
 		Signer:              sigb,
 		P2PKey:              p2p,
-		EncryptionPublicKey: pubkey, //[32]byte{3: 84, 2: 79, 1: 68, 0: 79}, // TODO. no current way to get this from the node itself (and therefore not in clo or jd)
+		EncryptionPublicKey: pubkey, // TODO. no current way to get this from the node itself (and therefore not in clo or jd)
 	}, nil
 }