From 02aba697f4ffe7aff127d205f07f0e747ff49c26 Mon Sep 17 00:00:00 2001 From: krehermann <16602512+krehermann@users.noreply.github.com> Date: Mon, 9 Dec 2024 18:06:24 -0700 Subject: [PATCH] fix tests; cleanup registry vs contractset --- .../changeset/append_node_capbilities.go | 1 - .../internal/append_node_capabilities.go | 10 ++-- .../internal/append_node_capabilities_test.go | 2 +- .../keystone/changeset/internal/test/utils.go | 4 ++ .../keystone/changeset/internal/update_don.go | 15 +++--- .../changeset/internal/update_don_test.go | 6 +-- .../internal/update_node_capabilities.go | 4 +- .../internal/update_node_capabilities_test.go | 2 +- .../changeset/internal/update_nodes.go | 23 +++++---- .../changeset/internal/update_nodes_test.go | 47 +++++++++---------- .../changeset/update_node_capabilities.go | 3 +- deployment/keystone/changeset/update_nodes.go | 1 - 12 files changed, 57 insertions(+), 61 deletions(-) diff --git a/deployment/keystone/changeset/append_node_capbilities.go b/deployment/keystone/changeset/append_node_capbilities.go index 7601b1bb206..f0bad959551 100644 --- a/deployment/keystone/changeset/append_node_capbilities.go +++ b/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, diff --git a/deployment/keystone/changeset/internal/append_node_capabilities.go b/deployment/keystone/changeset/internal/append_node_capabilities.go index ced741d416a..892aa4c1e16 100644 --- a/deployment/keystone/changeset/internal/append_node_capabilities.go +++ b/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, diff --git a/deployment/keystone/changeset/internal/append_node_capabilities_test.go b/deployment/keystone/changeset/internal/append_node_capabilities_test.go index d28dcd73230..f2ec2bf5c8f 100644 --- a/deployment/keystone/changeset/internal/append_node_capabilities_test.go +++ b/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 { diff --git a/deployment/keystone/changeset/internal/test/utils.go b/deployment/keystone/changeset/internal/test/utils.go index a7aed2c9cb1..0a23f7e60a7 100644 --- a/deployment/keystone/changeset/internal/test/utils.go +++ b/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, + }, } } diff --git a/deployment/keystone/changeset/internal/update_don.go b/deployment/keystone/changeset/internal/update_don.go index d56f77c1c78..dae0e46eca7 100644 --- a/deployment/keystone/changeset/internal/update_don.go +++ b/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) diff --git a/deployment/keystone/changeset/internal/update_don_test.go b/deployment/keystone/changeset/internal/update_don_test.go index e500ade60d7..49ddee538bf 100644 --- a/deployment/keystone/changeset/internal/update_don_test.go +++ b/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}, }, diff --git a/deployment/keystone/changeset/internal/update_node_capabilities.go b/deployment/keystone/changeset/internal/update_node_capabilities.go index d060f47d810..fe101c90296 100644 --- a/deployment/keystone/changeset/internal/update_node_capabilities.go +++ 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, diff --git a/deployment/keystone/changeset/internal/update_node_capabilities_test.go b/deployment/keystone/changeset/internal/update_node_capabilities_test.go index 0346ff20dd6..ac39e57b32d 100644 --- a/deployment/keystone/changeset/internal/update_node_capabilities_test.go +++ b/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 { diff --git a/deployment/keystone/changeset/internal/update_nodes.go b/deployment/keystone/changeset/internal/update_nodes.go index 51382c3745d..3480f39b084 100644 --- a/deployment/keystone/changeset/internal/update_nodes.go +++ b/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), } diff --git a/deployment/keystone/changeset/internal/update_nodes_test.go b/deployment/keystone/changeset/internal/update_nodes_test.go index b167f210811..e730668f806 100644 --- a/deployment/keystone/changeset/internal/update_nodes_test.go +++ b/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) diff --git a/deployment/keystone/changeset/update_node_capabilities.go b/deployment/keystone/changeset/update_node_capabilities.go index 67bd8ac39cd..d50c07c9f06 100644 --- a/deployment/keystone/changeset/update_node_capabilities.go +++ b/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 } diff --git a/deployment/keystone/changeset/update_nodes.go b/deployment/keystone/changeset/update_nodes.go index d07ffe5b025..4e2a4f7f4c6 100644 --- a/deployment/keystone/changeset/update_nodes.go +++ b/deployment/keystone/changeset/update_nodes.go @@ -46,7 +46,6 @@ func UpdateNodes(env deployment.Environment, req *UpdateNodesRequest) (deploymen resp, err := internal.UpdateNodes(env.Logger, &internal.UpdateNodesRequest{ Chain: registryChain, - Registry: contracts.CapabilitiesRegistry, ContractSet: &contracts, P2pToUpdates: req.P2pToUpdates, UseMCMS: req.UseMCMS,