[Keystone][Deployments] Make adding NOPs and DONs idempotent

Similarly to adding capabilities or nodes, don't add NOPs or DONs that already exist.

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

@@ -109,7 +109,7 @@ func deployCapReg(t *testing.T, lggr logger.Logger, chain deployment.Chain) *kcr
 }
 
 func addNops(t *testing.T, lggr logger.Logger, chain deployment.Chain, registry *kcr.CapabilitiesRegistry, nops []kcr.CapabilitiesRegistryNodeOperator) *kslib.RegisterNOPSResponse {
-	resp, err := kslib.RegisterNOPS(context.TODO(), kslib.RegisterNOPSRequest{
+	resp, err := kslib.RegisterNOPS(context.TODO(), lggr, kslib.RegisterNOPSRequest{
 		Chain:    chain,
 		Registry: registry,
 		Nops:     nops,

deployment/keystone/deploy.go

@@ -24,7 +24,6 @@ import (
 
 	capabilitiespb "github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb"
 	"github.com/smartcontractkit/chainlink-common/pkg/values"
-	"github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry"
 	kcr "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry"
 	kf "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/forwarder"
 	kocr3 "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/ocr3_capability"
@@ -196,7 +195,7 @@ func ConfigureRegistry(ctx context.Context, lggr logger.Logger, req ConfigureCon
 	for _, nop := range nodeIdToNop {
 		nops = append(nops, nop)
 	}
-	nopsResp, err := RegisterNOPS(ctx, RegisterNOPSRequest{
+	nopsResp, err := RegisterNOPS(ctx, lggr, RegisterNOPSRequest{
 		Chain:    registryChain,
 		Registry: registry,
 		Nops:     nops,
@@ -421,8 +420,36 @@ type RegisterNOPSResponse struct {
 	Nops []*kcr.CapabilitiesRegistryNodeOperatorAdded
 }
 
-func RegisterNOPS(ctx context.Context, req RegisterNOPSRequest) (*RegisterNOPSResponse, error) {
-	nops := req.Nops
+func RegisterNOPS(ctx context.Context, lggr logger.Logger, req RegisterNOPSRequest) (*RegisterNOPSResponse, error) {
+	existingNops, err := req.Registry.GetNodeOperators(&bind.CallOpts{})
+	if err != nil {
+		return nil, err
+	}
+	existingNopsAddrToID := make(map[string]uint32)
+	for id, nop := range existingNops {
+		existingNopsAddrToID[nop.Admin.String()] = uint32(id)
+	}
+	resp := &RegisterNOPSResponse{
+		Nops: []*kcr.CapabilitiesRegistryNodeOperatorAdded{},
+	}
+	nops := []kcr.CapabilitiesRegistryNodeOperator{}
+	for _, nop := range req.Nops {
+		nop := nop
+		if id, ok := existingNopsAddrToID[nop.Admin.String()]; !ok {
+			nops = append(nops, nop)
+		} else {
+			lggr.Debugw("node operator already exists", "admin", nop.Admin.String())
+			resp.Nops = append(resp.Nops, &kcr.CapabilitiesRegistryNodeOperatorAdded{
+				NodeOperatorId: id,
+				Name:           nop.Name,
+				Admin:          nop.Admin,
+			})
+		}
+	}
+	if len(nops) == 0 {
+		lggr.Debug("no new node operators to register")
+		return resp, nil
+	}
 	tx, err := req.Registry.AddNodeOperators(req.Chain.DeployerKey, nops)
 	if err != nil {
 		err = DecodeErr(kcr.CapabilitiesRegistryABI, err)
@@ -442,15 +469,12 @@ func RegisterNOPS(ctx context.Context, req RegisterNOPSRequest) (*RegisterNOPSRe
 	if len(receipt.Logs) != len(nops) {
 		return nil, fmt.Errorf("expected %d log entries for AddNodeOperators, got %d", len(nops), len(receipt.Logs))
 	}
-	resp := &RegisterNOPSResponse{
-		Nops: make([]*kcr.CapabilitiesRegistryNodeOperatorAdded, len(receipt.Logs)),
-	}
 	for i, log := range receipt.Logs {
 		o, err := req.Registry.ParseNodeOperatorAdded(*log)
 		if err != nil {
 			return nil, fmt.Errorf("failed to parse log %d for operator added: %w", i, err)
 		}
-		resp.Nops[i] = o
+		resp.Nops = append(resp.Nops, o)
 	}
 
 	return resp, nil
@@ -680,6 +704,16 @@ func registerDons(lggr logger.Logger, req registerDonsRequest) (*registerDonsRes
 	p2pIdsToDon := make(map[string]string)
 	var registeredDons = 0
 
+	donInfos, err := req.registry.GetDONs(&bind.CallOpts{})
+	if err != nil {
+		err = DecodeErr(kcr.CapabilitiesRegistryABI, err)
+		return nil, fmt.Errorf("failed to call GetDONs: %w", err)
+	}
+	existingDONs := make(map[string]struct{})
+	for _, donInfo := range donInfos {
+		existingDONs[sortedHash(donInfo.NodeP2PIds)] = struct{}{}
+	}
+
 	for don, ocr2nodes := range req.donToOcr2Nodes {
 		var p2pIds [][32]byte
 		for _, n := range ocr2nodes {
@@ -695,6 +729,12 @@ func registerDons(lggr logger.Logger, req registerDonsRequest) (*registerDonsRes
 
 		p2pSortedHash := sortedHash(p2pIds)
 		p2pIdsToDon[p2pSortedHash] = don
+
+		if _, ok := existingDONs[p2pSortedHash]; ok {
+			lggr.Debugw("don already exists, ignoring", "don", don, "p2p sorted hash", p2pSortedHash)
+			continue
+		}
+
 		caps, ok := req.donToCapabilities[don]
 		if !ok {
 			return nil, fmt.Errorf("capabilities not found for node operator %s", don)
@@ -734,38 +774,50 @@ func registerDons(lggr logger.Logger, req registerDonsRequest) (*registerDonsRes
 
 	// occasionally the registry does not return the expected number of DONS immediately after the txns above
 	// so we retry a few times. while crude, it is effective
-	var donInfos []capabilities_registry.CapabilitiesRegistryDONInfo
-	var err error
+	foundAll := false
 	for i := 0; i < 10; i++ {
 		lggr.Debug("attempting to get DONS from registry", i)
 		donInfos, err = req.registry.GetDONs(&bind.CallOpts{})
-		if len(donInfos) != registeredDons {
+		if !containsAllDONs(donInfos, p2pIdsToDon) {
 			lggr.Debugw("expected dons not registered", "expected", registeredDons, "got", len(donInfos))
 			time.Sleep(2 * time.Second)
 		} else {
+			foundAll = true
 			break
 		}
 	}
 	if err != nil {
 		err = DecodeErr(kcr.CapabilitiesRegistryABI, err)
 		return nil, fmt.Errorf("failed to call GetDONs: %w", err)
 	}
+	if !foundAll {
+		return nil, fmt.Errorf("did not find all desired DONS")
+	}
 
 	for i, donInfo := range donInfos {
 		donName, ok := p2pIdsToDon[sortedHash(donInfo.NodeP2PIds)]
 		if !ok {
-			return nil, fmt.Errorf("don not found for p2pids %s in %v", sortedHash(donInfo.NodeP2PIds), p2pIdsToDon)
+			lggr.Debugw("extra DON found in the registry, ignoring", "p2p sorted hash", sortedHash(donInfo.NodeP2PIds))
+			continue
 		}
 		lggr.Debugw("adding don info", "don", donName, "cnt", i)
 		resp.donInfos[donName] = donInfos[i]
 	}
-	lggr.Debugw("found registered DONs", "count", len(resp.donInfos))
-	if len(resp.donInfos) != registeredDons {
-		return nil, fmt.Errorf("expected %d dons, got %d", registeredDons, len(resp.donInfos))
-	}
 	return &resp, nil
 }
 
+// are all DONs from p2pIdsToDon in donInfos
+func containsAllDONs(donInfos []kcr.CapabilitiesRegistryDONInfo, p2pIdsToDon map[string]string) bool {
+	found := make(map[string]struct{})
+	for _, donInfo := range donInfos {
+		hash := sortedHash(donInfo.NodeP2PIds)
+		if _, ok := p2pIdsToDon[hash]; ok {
+			found[hash] = struct{}{}
+		}
+	}
+	return len(found) == len(p2pIdsToDon)
+}
+
 // configureForwarder sets the config for the forwarder contract on the chain for all Dons that accept workflows
 // dons that don't accept workflows are not registered with the forwarder
 func configureForwarder(lggr logger.Logger, chain deployment.Chain, fwdr *kf.KeystoneForwarder, dons []RegisteredDon) error {