fix tests; cleanup registry vs contractset

deployment/keystone/changeset/append_node_capbilities.go

@@ -74,7 +74,6 @@ func (req *AppendNodeCapabilitiesRequest) convert(e deployment.Environment) (*in
 
 	return &internal.AppendNodeCapabilitiesRequest{
 		Chain:             registryChain,
-		Registry:          contracts.CapabilitiesRegistry,
 		ContractSet:       &contracts,
 		P2pToCapabilities: req.P2pToCapabilities,
 		UseMCMS:           req.UseMCMS,

deployment/keystone/changeset/internal/append_node_capabilities.go

@@ -11,10 +11,9 @@ import (
 )
 
 type AppendNodeCapabilitiesRequest struct {
-	Chain    deployment.Chain
-	Registry *kcr.CapabilitiesRegistry
+	Chain       deployment.Chain
+	ContractSet *kslib.ContractSet
 
-	ContractSet       *kslib.ContractSet
 	P2pToCapabilities map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability
 	UseMCMS           bool
 }
@@ -23,7 +22,7 @@ func (req *AppendNodeCapabilitiesRequest) Validate() error {
 	if len(req.P2pToCapabilities) == 0 {
 		return fmt.Errorf("p2pToCapabilities is empty")
 	}
-	if req.Registry == nil {
+	if req.ContractSet.CapabilitiesRegistry == nil {
 		return fmt.Errorf("registry is nil")
 	}
 	return nil
@@ -37,7 +36,7 @@ func AppendNodeCapabilitiesImpl(lggr logger.Logger, req *AppendNodeCapabilitiesR
 	// for each node, merge the new capabilities with the existing ones and update the node
 	updatesByPeer := make(map[p2pkey.PeerID]NodeUpdate)
 	for p2pID, caps := range req.P2pToCapabilities {
-		caps, err := AppendCapabilities(lggr, req.Registry, req.Chain, []p2pkey.PeerID{p2pID}, caps)
+		caps, err := AppendCapabilities(lggr, req.ContractSet.CapabilitiesRegistry, req.Chain, []p2pkey.PeerID{p2pID}, caps)
 		if err != nil {
 			return nil, fmt.Errorf("failed to append capabilities for p2p %s: %w", p2pID, err)
 		}
@@ -56,7 +55,6 @@ func AppendNodeCapabilitiesImpl(lggr logger.Logger, req *AppendNodeCapabilitiesR
 
 	updateNodesReq := &UpdateNodesRequest{
 		Chain:        req.Chain,
-		Registry:     req.Registry,
 		ContractSet:  req.ContractSet,
 		P2pToUpdates: updatesByPeer,
 		UseMCMS:      req.UseMCMS,

deployment/keystone/changeset/internal/append_node_capabilities_test.go

@@ -94,8 +94,8 @@ func TestAppendNodeCapabilities(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			setupResp := kstest.SetupTestRegistry(t, lggr, tt.args.initialState)
 
-			tt.args.req.Registry = setupResp.Registry
 			tt.args.req.Chain = setupResp.Chain
+			tt.args.req.ContractSet = setupResp.ContractSet
 
 			got, err := internal.AppendNodeCapabilitiesImpl(tt.args.lggr, tt.args.req)
 			if (err != nil) != tt.wantErr {

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

@@ -40,6 +40,7 @@ type SetupTestRegistryResponse struct {
 	Registry         *kcr.CapabilitiesRegistry
 	Chain            deployment.Chain
 	RegistrySelector uint64
+	ContractSet      *kslib.ContractSet
 }
 
 func SetupTestRegistry(t *testing.T, lggr logger.Logger, req *SetupTestRegistryRequest) *SetupTestRegistryResponse {
@@ -100,6 +101,9 @@ func SetupTestRegistry(t *testing.T, lggr logger.Logger, req *SetupTestRegistryR
 		Registry:         registry,
 		Chain:            chain,
 		RegistrySelector: chain.Selector,
+		ContractSet: &kslib.ContractSet{
+			CapabilitiesRegistry: registry,
+		},
 	}
 }
 

deployment/keystone/changeset/internal/update_don.go

@@ -27,8 +27,8 @@ type CapabilityConfig struct {
 }
 
 type UpdateDonRequest struct {
-	Registry *kcr.CapabilitiesRegistry
-	Chain    deployment.Chain
+	Chain       deployment.Chain
+	ContractSet *kslib.ContractSet // contract set for the given chain
 
 	P2PIDs            []p2pkey.PeerID    // this is the unique identifier for the don
 	CapabilityConfigs []CapabilityConfig // if Config subfield is nil, a default config is used
@@ -39,7 +39,7 @@ type UpdateDonRequest struct {
 func (r *UpdateDonRequest) appendNodeCapabilitiesRequest() *AppendNodeCapabilitiesRequest {
 	out := &AppendNodeCapabilitiesRequest{
 		Chain:             r.Chain,
-		Registry:          r.Registry,
+		ContractSet:       r.ContractSet,
 		P2pToCapabilities: make(map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability),
 		UseMCMS:           r.UseMCMS,
 	}
@@ -55,7 +55,7 @@ func (r *UpdateDonRequest) appendNodeCapabilitiesRequest() *AppendNodeCapabiliti
 }
 
 func (r *UpdateDonRequest) Validate() error {
-	if r.Registry == nil {
+	if r.ContractSet.CapabilitiesRegistry == nil {
 		return fmt.Errorf("registry is required")
 	}
 	if len(r.P2PIDs) == 0 {
@@ -74,7 +74,8 @@ func UpdateDon(lggr logger.Logger, req *UpdateDonRequest) (*UpdateDonResponse, e
 		return nil, fmt.Errorf("failed to validate request: %w", err)
 	}
 
-	getDonsResp, err := req.Registry.GetDONs(&bind.CallOpts{})
+	registry := req.ContractSet.CapabilitiesRegistry
+	getDonsResp, err := registry.GetDONs(&bind.CallOpts{})
 	if err != nil {
 		return nil, fmt.Errorf("failed to get Dons: %w", err)
 	}
@@ -83,7 +84,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(req.Registry, req.CapabilityConfigs, don)
+	cfgs, err := computeConfigs(registry, req.CapabilityConfigs, don)
 	if err != nil {
 		return nil, fmt.Errorf("failed to compute configs: %w", err)
 	}
@@ -93,7 +94,7 @@ func UpdateDon(lggr logger.Logger, req *UpdateDonRequest) (*UpdateDonResponse, e
 		return nil, fmt.Errorf("failed to append node capabilities: %w", err)
 	}
 
-	tx, err := req.Registry.UpdateDON(req.Chain.DeployerKey, don.Id, don.NodeP2PIds, cfgs, don.IsPublic, don.F)
+	tx, err := registry.UpdateDON(req.Chain.DeployerKey, don.Id, don.NodeP2PIds, cfgs, don.IsPublic, don.F)
 	if err != nil {
 		err = kslib.DecodeErr(kcr.CapabilitiesRegistryABI, err)
 		return nil, fmt.Errorf("failed to call UpdateDON: %w", err)

deployment/keystone/changeset/internal/update_don_test.go

@@ -118,9 +118,9 @@ func TestUpdateDon(t *testing.T) {
 		testCfg := setupUpdateDonTest(t, lggr, cfg)
 
 		req := &internal.UpdateDonRequest{
-			Registry: testCfg.Registry,
-			Chain:    testCfg.Chain,
-			P2PIDs:   []p2pkey.PeerID{p2p_1.PeerID(), p2p_2.PeerID(), p2p_3.PeerID(), p2p_4.PeerID()},
+			ContractSet: testCfg.ContractSet,
+			Chain:       testCfg.Chain,
+			P2PIDs:      []p2pkey.PeerID{p2p_1.PeerID(), p2p_2.PeerID(), p2p_3.PeerID(), p2p_4.PeerID()},
 			CapabilityConfigs: []internal.CapabilityConfig{
 				{Capability: cap_A}, {Capability: cap_B},
 			},

deployment/keystone/changeset/internal/update_node_capabilities.go

@@ -12,7 +12,6 @@ import (
 
 type UpdateNodeCapabilitiesImplRequest struct {
 	Chain             deployment.Chain
-	Registry          *kcr.CapabilitiesRegistry
 	ContractSet       *kslib.ContractSet
 	P2pToCapabilities map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability
 
@@ -23,7 +22,7 @@ func (req *UpdateNodeCapabilitiesImplRequest) Validate() error {
 	if len(req.P2pToCapabilities) == 0 {
 		return fmt.Errorf("p2pToCapabilities is empty")
 	}
-	if req.Registry == nil {
+	if req.ContractSet == nil {
 		return fmt.Errorf("registry is nil")
 	}
 
@@ -51,7 +50,6 @@ func UpdateNodeCapabilitiesImpl(lggr logger.Logger, req *UpdateNodeCapabilitiesI
 
 	updateNodesReq := &UpdateNodesRequest{
 		Chain:        req.Chain,
-		Registry:     req.Registry,
 		P2pToUpdates: p2pToUpdates,
 		ContractSet:  req.ContractSet,
 		Ops:          op,

deployment/keystone/changeset/internal/update_node_capabilities_test.go

@@ -92,8 +92,8 @@ func TestUpdateNodeCapabilities(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			setupResp := kstest.SetupTestRegistry(t, lggr, tt.args.initialState)
-			tt.args.req.Registry = setupResp.Registry
 			tt.args.req.Chain = setupResp.Chain
+			tt.args.req.ContractSet = setupResp.ContractSet
 
 			got, err := kslib.UpdateNodeCapabilitiesImpl(tt.args.lggr, tt.args.req)
 			if (err != nil) != tt.wantErr {

deployment/keystone/changeset/internal/update_nodes.go

@@ -29,18 +29,19 @@ type NodeUpdate struct {
 }
 
 type UpdateNodesRequest struct {
-	Chain    deployment.Chain
-	Registry *kcr.CapabilitiesRegistry
+	Chain       deployment.Chain
+	ContractSet *kslib.ContractSet // contract set for the given chain
 
 	P2pToUpdates map[p2pkey.PeerID]NodeUpdate
 
-	ContractSet *kslib.ContractSet // contract set for the given chain
-	Ops         *timelock.BatchChainOperation
-	UseMCMS     bool
+	UseMCMS bool
+	// If UseMCMS is true, and Ops is not nil then the UpdateNodes contract operation
+	// will be added to the Ops.Batch
+	Ops *timelock.BatchChainOperation
 }
 
 func (req *UpdateNodesRequest) NodeParams() ([]kcr.CapabilitiesRegistryNodeParams, error) {
-	return makeNodeParams(req.Registry, req.P2pToUpdates)
+	return makeNodeParams(req.ContractSet.CapabilitiesRegistry, req.P2pToUpdates)
 }
 
 // P2PSignerEnc represent the key fields in kcr.CapabilitiesRegistryNodeParams
@@ -78,7 +79,7 @@ func (req *UpdateNodesRequest) Validate() error {
 		}
 	}
 
-	if req.Registry == nil {
+	if req.ContractSet.CapabilitiesRegistry == nil {
 		return errors.New("registry is nil")
 	}
 
@@ -87,7 +88,8 @@ func (req *UpdateNodesRequest) Validate() error {
 
 type UpdateNodesResponse struct {
 	NodeParams []kcr.CapabilitiesRegistryNodeParams
-	//Proposals  []timelock.MCMSWithTimelockProposal
+	// MCMS operation to update the nodes
+	// The operation is added to the Batch of the given Ops if not nil
 	Ops *timelock.BatchChainOperation
 }
 
@@ -108,7 +110,8 @@ func UpdateNodes(lggr logger.Logger, req *UpdateNodesRequest) (*UpdateNodesRespo
 	if req.UseMCMS {
 		txOpts = deployment.SimTransactOpts()
 	}
-	tx, err := req.Registry.UpdateNodes(txOpts, params)
+	registry := req.ContractSet.CapabilitiesRegistry
+	tx, err := registry.UpdateNodes(txOpts, params)
 	if err != nil {
 		err = kslib.DecodeErr(kcr.CapabilitiesRegistryABI, err)
 		return nil, fmt.Errorf("failed to call UpdateNodes: %w", err)
@@ -122,7 +125,7 @@ func UpdateNodes(lggr logger.Logger, req *UpdateNodesRequest) (*UpdateNodesRespo
 		}
 	} else {
 		op := mcms.Operation{
-			To:    req.Registry.Address(),
+			To:    registry.Address(),
 			Data:  tx.Data(),
 			Value: big.NewInt(0),
 		}

deployment/keystone/changeset/internal/update_nodes_test.go

@@ -30,7 +30,7 @@ func Test_UpdateNodesRequest_validate(t *testing.T) {
 		p2pToUpdates map[p2pkey.PeerID]internal.NodeUpdate
 		nopToNodes   map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc
 		chain        deployment.Chain
-		registry     *kcr.CapabilitiesRegistry
+		contractSet  *kslib.ContractSet
 	}
 	tests := []struct {
 		name    string
@@ -43,7 +43,7 @@ func Test_UpdateNodesRequest_validate(t *testing.T) {
 				p2pToUpdates: map[p2pkey.PeerID]internal.NodeUpdate{},
 				nopToNodes:   nil,
 				chain:        deployment.Chain{},
-				registry:     nil,
+				contractSet:  nil,
 			},
 			wantErr: true,
 		},
@@ -55,9 +55,9 @@ func Test_UpdateNodesRequest_validate(t *testing.T) {
 						EncryptionPublicKey: "jk",
 					},
 				},
-				nopToNodes: nil,
-				chain:      deployment.Chain{},
-				registry:   nil,
+				nopToNodes:  nil,
+				chain:       deployment.Chain{},
+				contractSet: nil,
 			},
 			wantErr: true,
 		},
@@ -69,9 +69,9 @@ func Test_UpdateNodesRequest_validate(t *testing.T) {
 						EncryptionPublicKey: "aabb",
 					},
 				},
-				nopToNodes: nil,
-				chain:      deployment.Chain{},
-				registry:   nil,
+				nopToNodes:  nil,
+				chain:       deployment.Chain{},
+				contractSet: nil,
 			},
 			wantErr: true,
 		},
@@ -81,7 +81,7 @@ func Test_UpdateNodesRequest_validate(t *testing.T) {
 			req := &internal.UpdateNodesRequest{
 				P2pToUpdates: tt.fields.p2pToUpdates,
 				Chain:        tt.fields.chain,
-				Registry:     tt.fields.registry,
+				ContractSet:  tt.fields.contractSet,
 			}
 			if err := req.Validate(); (err != nil) != tt.wantErr {
 				t.Errorf("internal.UpdateNodesRequest.validate() error = %v, wantErr %v", err, tt.wantErr)
@@ -130,8 +130,7 @@ func TestUpdateNodes(t *testing.T) {
 							},
 						},
 					},
-					Chain:    chain,
-					Registry: nil, // set in test to ensure no conflicts
+					Chain: chain,
 				},
 				nopsToNodes: map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc{
 					testNop(t, "nop1"): []*internal.P2PSignerEnc{
@@ -177,8 +176,7 @@ func TestUpdateNodes(t *testing.T) {
 							},
 						},
 					},
-					Chain:    chain,
-					Registry: nil, // set in test to ensure no conflicts
+					Chain: chain,
 				},
 				nopsToNodes: map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc{
 					testNop(t, "nop1"): []*internal.P2PSignerEnc{
@@ -235,8 +233,7 @@ func TestUpdateNodes(t *testing.T) {
 							},
 						},
 					},
-					Chain:    chain,
-					Registry: nil, // set in test to ensure no conflicts
+					Chain: chain,
 				},
 				nopsToNodes: map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc{
 					testNop(t, "nopA"): []*internal.P2PSignerEnc{
@@ -300,8 +297,7 @@ func TestUpdateNodes(t *testing.T) {
 							},
 						},
 					},
-					Chain:    chain,
-					Registry: nil, // set in test to ensure no conflicts
+					Chain: chain,
 				},
 				nopsToNodes: map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc{
 					testNop(t, "nopA"): []*internal.P2PSignerEnc{
@@ -350,8 +346,8 @@ func TestUpdateNodes(t *testing.T) {
 							EncryptionPublicKey: newKeyStr,
 						},
 					},
-					Chain:    chain,
-					Registry: nil, // set in test to ensure no conflicts
+					Chain:       chain,
+					ContractSet: nil, // set in test to ensure no conflicts
 				},
 				nopsToNodes: map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc{
 					testNop(t, "nop1"): []*internal.P2PSignerEnc{
@@ -385,8 +381,8 @@ func TestUpdateNodes(t *testing.T) {
 							Signer: [32]byte{0: 2, 1: 3},
 						},
 					},
-					Chain:    chain,
-					Registry: nil, // set in test to ensure no conflicts
+					Chain:       chain,
+					ContractSet: nil, // set in test to ensure no conflicts
 				},
 				nopsToNodes: map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc{
 					testNop(t, "nop1"): []*internal.P2PSignerEnc{
@@ -420,8 +416,7 @@ func TestUpdateNodes(t *testing.T) {
 							NodeOperatorID: 2,
 						},
 					},
-					Chain:    chain,
-					Registry: nil, // set in test to ensure no conflicts
+					Chain: chain,
 				},
 				nopsToNodes: map[kcr.CapabilitiesRegistryNodeOperator][]*internal.P2PSignerEnc{
 					testNop(t, "nop1"): []*internal.P2PSignerEnc{
@@ -463,7 +458,7 @@ func TestUpdateNodes(t *testing.T) {
 				NopToNodes:        tt.args.nopsToNodes,
 			})
 			registry := setupResp.Registry
-			tt.args.req.Registry = setupResp.Registry
+			tt.args.req.ContractSet = setupResp.ContractSet
 			tt.args.req.Chain = setupResp.Chain
 
 			id, err := registry.GetHashedCapabilityId(&bind.CallOpts{}, phonyCap.LabelledName, phonyCap.Version)
@@ -581,8 +576,8 @@ func TestUpdateNodes(t *testing.T) {
 					Capabilities: toRegister,
 				},
 			},
-			Chain:    chain,
-			Registry: registry,
+			Chain:       chain,
+			ContractSet: setupResp.ContractSet,
 		}
 		_, err = internal.UpdateNodes(lggr, req)
 		require.NoError(t, err)

deployment/keystone/changeset/update_node_capabilities.go

@@ -93,9 +93,8 @@ func (req *MutateNodeCapabilitiesRequest) updateNodeCapabilitiesImplRequest(e de
 
 	return &internal.UpdateNodeCapabilitiesImplRequest{
 		Chain:             registryChain,
-		Registry:          contractSet.CapabilitiesRegistry,
-		P2pToCapabilities: req.P2pToCapabilities,
 		ContractSet:       &contractSet,
+		P2pToCapabilities: req.P2pToCapabilities,
 		UseMCMS:           req.UseMCMS,
 	}, nil
 }