From d1caaa33e496f540a411207be0809120a894c600 Mon Sep 17 00:00:00 2001 From: krehermann <16602512+krehermann@users.noreply.github.com> Date: Wed, 11 Dec 2024 12:37:22 -0700 Subject: [PATCH] refactor update don capability (#15623) * refactor update & append capabilities * update don changeset mcms * update don with test * cleanup dupes * configurable MCMS; infered MCMS usage from it --- .../changeset/append_node_capabilities.go | 6 +- .../append_node_capabilities_test.go | 2 +- .../keystone/changeset/deploy_forwarder.go | 53 +++++- .../changeset/deploy_forwarder_test.go | 2 +- deployment/keystone/changeset/deploy_ocr3.go | 51 +++++- .../keystone/changeset/deploy_ocr3_test.go | 3 +- .../keystone/changeset/internal/update_don.go | 41 +++-- .../changeset/internal/update_don_test.go | 29 ++- deployment/keystone/changeset/update_don.go | 94 +++++++++- .../keystone/changeset/update_don_test.go | 165 ++++++++++++++++++ .../changeset/update_node_capabilities.go | 17 +- .../update_node_capabilities_test.go | 2 +- deployment/keystone/changeset/update_nodes.go | 26 ++- .../keystone/changeset/update_nodes_test.go | 4 +- deployment/keystone/deploy.go | 34 +--- deployment/keystone/forwarder_deployer.go | 12 +- deployment/keystone/ocr3config.go | 27 +-- 17 files changed, 456 insertions(+), 112 deletions(-) create mode 100644 deployment/keystone/changeset/update_don_test.go diff --git a/deployment/keystone/changeset/append_node_capabilities.go b/deployment/keystone/changeset/append_node_capabilities.go index f0bad959551..688d4fd8d2f 100644 --- a/deployment/keystone/changeset/append_node_capabilities.go +++ b/deployment/keystone/changeset/append_node_capabilities.go @@ -29,7 +29,7 @@ func AppendNodeCapabilities(env deployment.Environment, req *AppendNodeCapabilit return deployment.ChangesetOutput{}, err } out := deployment.ChangesetOutput{} - if req.UseMCMS { + if req.UseMCMS() { if r.Ops == nil { return out, fmt.Errorf("expected MCMS operation to be non-nil") } @@ -45,7 +45,7 @@ func AppendNodeCapabilities(env deployment.Environment, req *AppendNodeCapabilit proposerMCMSes, []timelock.BatchChainOperation{*r.Ops}, "proposal to set update node capabilities", - 0, + req.MCMSConfig.MinDuration, ) if err != nil { return out, fmt.Errorf("failed to build proposal: %w", err) @@ -76,6 +76,6 @@ func (req *AppendNodeCapabilitiesRequest) convert(e deployment.Environment) (*in Chain: registryChain, ContractSet: &contracts, P2pToCapabilities: req.P2pToCapabilities, - UseMCMS: req.UseMCMS, + UseMCMS: req.UseMCMS(), }, nil } diff --git a/deployment/keystone/changeset/append_node_capabilities_test.go b/deployment/keystone/changeset/append_node_capabilities_test.go index 7fbbbfc8a83..159500ab5a7 100644 --- a/deployment/keystone/changeset/append_node_capabilities_test.go +++ b/deployment/keystone/changeset/append_node_capabilities_test.go @@ -75,7 +75,7 @@ func TestAppendNodeCapabilities(t *testing.T) { cfg := changeset.AppendNodeCapabilitiesRequest{ RegistryChainSel: te.RegistrySelector, P2pToCapabilities: newCapabilities, - UseMCMS: true, + MCMSConfig: &changeset.MCMSConfig{MinDuration: 0}, } csOut, err := changeset.AppendNodeCapabilities(te.Env, &cfg) diff --git a/deployment/keystone/changeset/deploy_forwarder.go b/deployment/keystone/changeset/deploy_forwarder.go index cf116decd54..1e4066770bd 100644 --- a/deployment/keystone/changeset/deploy_forwarder.go +++ b/deployment/keystone/changeset/deploy_forwarder.go @@ -3,7 +3,11 @@ package changeset import ( "fmt" + "github.com/ethereum/go-ethereum/common" + "github.com/smartcontractkit/ccip-owner-contracts/pkg/gethwrappers" + "github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/timelock" "github.com/smartcontractkit/chainlink/deployment" + "github.com/smartcontractkit/chainlink/deployment/common/proposalutils" kslib "github.com/smartcontractkit/chainlink/deployment/keystone" ) @@ -35,7 +39,8 @@ type ConfigureForwardContractsRequest struct { WFNodeIDs []string RegistryChainSel uint64 - UseMCMS bool + // MCMSConfig is optional. If non-nil, the changes will be proposed using MCMS. + MCMSConfig *MCMSConfig } func (r ConfigureForwardContractsRequest) Validate() error { @@ -45,6 +50,10 @@ func (r ConfigureForwardContractsRequest) Validate() error { return nil } +func (r ConfigureForwardContractsRequest) UseMCMS() bool { + return r.MCMSConfig != nil +} + func ConfigureForwardContracts(env deployment.Environment, req ConfigureForwardContractsRequest) (deployment.ChangesetOutput, error) { wfDon, err := kslib.NewRegisteredDon(env, kslib.RegisteredDonConfig{ NodeIDs: req.WFNodeIDs, @@ -56,12 +65,46 @@ func ConfigureForwardContracts(env deployment.Environment, req ConfigureForwardC } r, err := kslib.ConfigureForwardContracts(&env, kslib.ConfigureForwarderContractsRequest{ Dons: []kslib.RegisteredDon{*wfDon}, - UseMCMS: req.UseMCMS, + UseMCMS: req.UseMCMS(), }) if err != nil { return deployment.ChangesetOutput{}, fmt.Errorf("failed to configure forward contracts: %w", err) } - return deployment.ChangesetOutput{ - Proposals: r.Proposals, - }, nil + + cresp, err := kslib.GetContractSets(env.Logger, &kslib.GetContractSetsRequest{ + Chains: env.Chains, + AddressBook: env.ExistingAddresses, + }) + if err != nil { + return deployment.ChangesetOutput{}, fmt.Errorf("failed to get contract sets: %w", err) + } + + var out deployment.ChangesetOutput + if req.UseMCMS() { + if len(r.OpsPerChain) == 0 { + return out, fmt.Errorf("expected MCMS operation to be non-nil") + } + for chainSelector, op := range r.OpsPerChain { + contracts := cresp.ContractSets[chainSelector] + timelocksPerChain := map[uint64]common.Address{ + chainSelector: contracts.Timelock.Address(), + } + proposerMCMSes := map[uint64]*gethwrappers.ManyChainMultiSig{ + chainSelector: contracts.ProposerMcm, + } + + proposal, err := proposalutils.BuildProposalFromBatches( + timelocksPerChain, + proposerMCMSes, + []timelock.BatchChainOperation{op}, + "proposal to set update nodes", + req.MCMSConfig.MinDuration, + ) + if err != nil { + return out, fmt.Errorf("failed to build proposal: %w", err) + } + out.Proposals = append(out.Proposals, *proposal) + } + } + return out, nil } diff --git a/deployment/keystone/changeset/deploy_forwarder_test.go b/deployment/keystone/changeset/deploy_forwarder_test.go index 82454599226..dd894fde9d9 100644 --- a/deployment/keystone/changeset/deploy_forwarder_test.go +++ b/deployment/keystone/changeset/deploy_forwarder_test.go @@ -109,7 +109,7 @@ func TestConfigureForwarders(t *testing.T) { WFDonName: "test-wf-don", WFNodeIDs: wfNodes, RegistryChainSel: te.RegistrySelector, - UseMCMS: true, + MCMSConfig: &changeset.MCMSConfig{MinDuration: 0}, } csOut, err := changeset.ConfigureForwardContracts(te.Env, cfg) require.NoError(t, err) diff --git a/deployment/keystone/changeset/deploy_ocr3.go b/deployment/keystone/changeset/deploy_ocr3.go index 0ce3d02844b..4dfed1e292c 100644 --- a/deployment/keystone/changeset/deploy_ocr3.go +++ b/deployment/keystone/changeset/deploy_ocr3.go @@ -5,9 +5,12 @@ import ( "fmt" "io" + "github.com/ethereum/go-ethereum/common" + "github.com/smartcontractkit/ccip-owner-contracts/pkg/gethwrappers" "github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/timelock" "github.com/smartcontractkit/chainlink/deployment" + "github.com/smartcontractkit/chainlink/deployment/common/proposalutils" kslib "github.com/smartcontractkit/chainlink/deployment/keystone" ) @@ -38,7 +41,12 @@ type ConfigureOCR3Config struct { DryRun bool WriteGeneratedConfig io.Writer // if not nil, write the generated config to this writer as JSON [OCR2OracleConfig] - UseMCMS bool + // MCMSConfig is optional. If non-nil, the changes will be proposed using MCMS. + MCMSConfig *MCMSConfig +} + +func (cfg ConfigureOCR3Config) UseMCMS() bool { + return cfg.MCMSConfig != nil } func ConfigureOCR3Contract(env deployment.Environment, cfg ConfigureOCR3Config) (deployment.ChangesetOutput, error) { @@ -47,7 +55,7 @@ func ConfigureOCR3Contract(env deployment.Environment, cfg ConfigureOCR3Config) NodeIDs: cfg.NodeIDs, OCR3Config: cfg.OCR3Config, DryRun: cfg.DryRun, - UseMCMS: cfg.UseMCMS, + UseMCMS: cfg.UseMCMS(), }) if err != nil { return deployment.ChangesetOutput{}, fmt.Errorf("failed to configure OCR3Capability: %w", err) @@ -67,11 +75,38 @@ func ConfigureOCR3Contract(env deployment.Environment, cfg ConfigureOCR3Config) } } // does not create any new addresses - var proposals []timelock.MCMSWithTimelockProposal - if cfg.UseMCMS { - proposals = append(proposals, *resp.Proposal) + var out deployment.ChangesetOutput + if cfg.UseMCMS() { + if resp.Ops == nil { + return out, fmt.Errorf("expected MCMS operation to be non-nil") + } + r, err := kslib.GetContractSets(env.Logger, &kslib.GetContractSetsRequest{ + Chains: env.Chains, + AddressBook: env.ExistingAddresses, + }) + if err != nil { + return out, fmt.Errorf("failed to get contract sets: %w", err) + } + contracts := r.ContractSets[cfg.ChainSel] + timelocksPerChain := map[uint64]common.Address{ + cfg.ChainSel: contracts.Timelock.Address(), + } + proposerMCMSes := map[uint64]*gethwrappers.ManyChainMultiSig{ + cfg.ChainSel: contracts.ProposerMcm, + } + + proposal, err := proposalutils.BuildProposalFromBatches( + timelocksPerChain, + proposerMCMSes, + []timelock.BatchChainOperation{*resp.Ops}, + "proposal to set update nodes", + cfg.MCMSConfig.MinDuration, + ) + if err != nil { + return out, fmt.Errorf("failed to build proposal: %w", err) + } + out.Proposals = []timelock.MCMSWithTimelockProposal{*proposal} + } - return deployment.ChangesetOutput{ - Proposals: proposals, - }, nil + return out, nil } diff --git a/deployment/keystone/changeset/deploy_ocr3_test.go b/deployment/keystone/changeset/deploy_ocr3_test.go index 60abd702929..5d02f83500d 100644 --- a/deployment/keystone/changeset/deploy_ocr3_test.go +++ b/deployment/keystone/changeset/deploy_ocr3_test.go @@ -71,7 +71,6 @@ func TestConfigureOCR3(t *testing.T) { NodeIDs: wfNodes, OCR3Config: &c, WriteGeneratedConfig: w, - UseMCMS: false, } csOut, err := changeset.ConfigureOCR3Contract(te.Env, cfg) @@ -104,7 +103,7 @@ func TestConfigureOCR3(t *testing.T) { NodeIDs: wfNodes, OCR3Config: &c, WriteGeneratedConfig: w, - UseMCMS: true, + MCMSConfig: &changeset.MCMSConfig{MinDuration: 0}, } csOut, err := changeset.ConfigureOCR3Contract(te.Env, cfg) diff --git a/deployment/keystone/changeset/internal/update_don.go b/deployment/keystone/changeset/internal/update_don.go index dae0e46eca7..fc7e410e540 100644 --- a/deployment/keystone/changeset/internal/update_don.go +++ b/deployment/keystone/changeset/internal/update_don.go @@ -6,9 +6,11 @@ import ( "encoding/hex" "encoding/json" "fmt" + "math/big" "sort" "github.com/ethereum/go-ethereum/accounts/abi/bind" + "github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/mcms" "github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/timelock" "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink/deployment" @@ -36,7 +38,7 @@ type UpdateDonRequest struct { UseMCMS bool } -func (r *UpdateDonRequest) appendNodeCapabilitiesRequest() *AppendNodeCapabilitiesRequest { +func (r *UpdateDonRequest) AppendNodeCapabilitiesRequest() *AppendNodeCapabilitiesRequest { out := &AppendNodeCapabilitiesRequest{ Chain: r.Chain, ContractSet: r.ContractSet, @@ -65,8 +67,8 @@ func (r *UpdateDonRequest) Validate() error { } type UpdateDonResponse struct { - DonInfo kcr.CapabilitiesRegistryDONInfo - Proposals []timelock.MCMSWithTimelockProposal + DonInfo kcr.CapabilitiesRegistryDONInfo + Ops *timelock.BatchChainOperation } func UpdateDon(lggr logger.Logger, req *UpdateDonRequest) (*UpdateDonResponse, error) { @@ -89,24 +91,37 @@ func UpdateDon(lggr logger.Logger, req *UpdateDonRequest) (*UpdateDonResponse, e return nil, fmt.Errorf("failed to compute configs: %w", err) } - _, err = AppendNodeCapabilitiesImpl(lggr, req.appendNodeCapabilitiesRequest()) - if err != nil { - return nil, fmt.Errorf("failed to append node capabilities: %w", err) + txOpts := req.Chain.DeployerKey + if req.UseMCMS { + txOpts = deployment.SimTransactOpts() } - - tx, err := registry.UpdateDON(req.Chain.DeployerKey, don.Id, don.NodeP2PIds, cfgs, don.IsPublic, don.F) + tx, err := registry.UpdateDON(txOpts, 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) } - - _, err = req.Chain.Confirm(tx) - if err != nil { - return nil, fmt.Errorf("failed to confirm UpdateDON transaction %s: %w", tx.Hash().String(), err) + var ops *timelock.BatchChainOperation + if !req.UseMCMS { + _, err = req.Chain.Confirm(tx) + if err != nil { + return nil, fmt.Errorf("failed to confirm UpdateDON transaction %s: %w", tx.Hash().String(), err) + } + } else { + ops = &timelock.BatchChainOperation{ + ChainIdentifier: mcms.ChainIdentifier(req.Chain.Selector), + Batch: []mcms.Operation{ + { + To: registry.Address(), + Data: tx.Data(), + Value: big.NewInt(0), + }, + }, + } } + out := don out.CapabilityConfigurations = cfgs - return &UpdateDonResponse{DonInfo: out}, nil + return &UpdateDonResponse{DonInfo: out, Ops: ops}, nil } func PeerIDsToBytes(p2pIDs []p2pkey.PeerID) [][32]byte { diff --git a/deployment/keystone/changeset/internal/update_don_test.go b/deployment/keystone/changeset/internal/update_don_test.go index 49ddee538bf..93857b26f78 100644 --- a/deployment/keystone/changeset/internal/update_don_test.go +++ b/deployment/keystone/changeset/internal/update_don_test.go @@ -83,13 +83,13 @@ func TestUpdateDon(t *testing.T) { admin: admin_4, }) // capabilities - cap_A = kcr.CapabilitiesRegistryCapability{ + initialCap = kcr.CapabilitiesRegistryCapability{ LabelledName: "test", Version: "1.0.0", CapabilityType: 0, } - cap_B = kcr.CapabilitiesRegistryCapability{ + capToAdd = kcr.CapabilitiesRegistryCapability{ LabelledName: "cap b", Version: "1.0.0", CapabilityType: 1, @@ -104,7 +104,7 @@ func TestUpdateDon(t *testing.T) { { Name: "don 1", Nodes: []deployment.Node{node_1, node_2, node_3, node_4}, - Capabilities: []kcr.CapabilitiesRegistryCapability{cap_A}, + Capabilities: []kcr.CapabilitiesRegistryCapability{initialCap}, }, }, nops: []keystone.NOP{ @@ -115,14 +115,26 @@ func TestUpdateDon(t *testing.T) { }, } - testCfg := setupUpdateDonTest(t, lggr, cfg) + testCfg := registerTestDon(t, lggr, cfg) + // add the new capabilities to registry + m := make(map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability) + for _, node := range cfg.dons[0].Nodes { + m[node.PeerID] = append(m[node.PeerID], capToAdd) + } + + _, err := internal.AppendNodeCapabilitiesImpl(lggr, &internal.AppendNodeCapabilitiesRequest{ + Chain: testCfg.Chain, + ContractSet: testCfg.ContractSet, + P2pToCapabilities: m, + }) + require.NoError(t, err) req := &internal.UpdateDonRequest{ 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}, + {Capability: initialCap}, {Capability: capToAdd}, }, } want := &internal.UpdateDonResponse{ @@ -131,8 +143,8 @@ func TestUpdateDon(t *testing.T) { ConfigCount: 1, NodeP2PIds: internal.PeerIDsToBytes([]p2pkey.PeerID{p2p_1.PeerID(), p2p_2.PeerID(), p2p_3.PeerID(), p2p_4.PeerID()}), CapabilityConfigurations: []kcr.CapabilitiesRegistryCapabilityConfiguration{ - {CapabilityId: kstest.MustCapabilityId(t, testCfg.Registry, cap_A)}, - {CapabilityId: kstest.MustCapabilityId(t, testCfg.Registry, cap_B)}, + {CapabilityId: kstest.MustCapabilityId(t, testCfg.Registry, initialCap)}, + {CapabilityId: kstest.MustCapabilityId(t, testCfg.Registry, capToAdd)}, }, }, } @@ -220,10 +232,11 @@ type setupUpdateDonTestResult struct { chain deployment.Chain } -func setupUpdateDonTest(t *testing.T, lggr logger.Logger, cfg setupUpdateDonTestConfig) *kstest.SetupTestRegistryResponse { +func registerTestDon(t *testing.T, lggr logger.Logger, cfg setupUpdateDonTestConfig) *kstest.SetupTestRegistryResponse { t.Helper() req := newSetupTestRegistryRequest(t, cfg.dons, cfg.nops) return kstest.SetupTestRegistry(t, lggr, req) + } func newSetupTestRegistryRequest(t *testing.T, dons []kslib.DonInfo, nops []keystone.NOP) *kstest.SetupTestRegistryRequest { diff --git a/deployment/keystone/changeset/update_don.go b/deployment/keystone/changeset/update_don.go index 1ab40d5a935..3f43ea513be 100644 --- a/deployment/keystone/changeset/update_don.go +++ b/deployment/keystone/changeset/update_don.go @@ -4,8 +4,11 @@ import ( "fmt" "github.com/smartcontractkit/chainlink/deployment" + kslib "github.com/smartcontractkit/chainlink/deployment/keystone" + "github.com/smartcontractkit/chainlink/deployment/keystone/changeset/internal" kcr "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry" + "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/p2pkey" ) var _ deployment.ChangeSet[*UpdateDonRequest] = UpdateDon @@ -13,7 +16,28 @@ var _ deployment.ChangeSet[*UpdateDonRequest] = UpdateDon // CapabilityConfig is a struct that holds a capability and its configuration type CapabilityConfig = internal.CapabilityConfig -type UpdateDonRequest = internal.UpdateDonRequest +type UpdateDonRequest struct { + RegistryChainSel uint64 + P2PIDs []p2pkey.PeerID // this is the unique identifier for the don + CapabilityConfigs []CapabilityConfig // if Config subfield is nil, a default config is used + + // MCMSConfig is optional. If non-nil, the changes will be proposed using MCMS. + MCMSConfig *MCMSConfig +} + +func (r *UpdateDonRequest) Validate() error { + if len(r.P2PIDs) == 0 { + return fmt.Errorf("p2pIDs is required") + } + if len(r.CapabilityConfigs) == 0 { + return fmt.Errorf("capabilityConfigs is required") + } + return nil +} + +func (r UpdateDonRequest) UseMCMS() bool { + return r.MCMSConfig != nil +} type UpdateDonResponse struct { DonInfo kcr.CapabilitiesRegistryDONInfo @@ -23,9 +47,73 @@ type UpdateDonResponse struct { // This a complex action in practice that involves registering missing capabilities, adding the nodes, and updating // the capabilities of the DON func UpdateDon(env deployment.Environment, req *UpdateDonRequest) (deployment.ChangesetOutput, error) { - _, err := internal.UpdateDon(env.Logger, req) + appendResult, err := AppendNodeCapabilities(env, appendRequest(req)) + if err != nil { + return deployment.ChangesetOutput{}, fmt.Errorf("failed to append node capabilities: %w", err) + } + + ur, err := updateDonRequest(env, req) + if err != nil { + return deployment.ChangesetOutput{}, fmt.Errorf("failed to create update don request: %w", err) + } + updateResult, err := internal.UpdateDon(env.Logger, ur) if err != nil { return deployment.ChangesetOutput{}, fmt.Errorf("failed to update don: %w", err) } - return deployment.ChangesetOutput{}, nil + + out := deployment.ChangesetOutput{} + if req.UseMCMS() { + if updateResult.Ops == nil { + return out, fmt.Errorf("expected MCMS operation to be non-nil") + } + if len(appendResult.Proposals) == 0 { + return out, fmt.Errorf("expected append node capabilities to return proposals") + } + + out.Proposals = appendResult.Proposals + + // add the update don to the existing batch + // this makes the proposal all-or-nothing because all the operations are in the same batch, there is only one tr + // transaction and only one proposal + out.Proposals[0].Transactions[0].Batch = append(out.Proposals[0].Transactions[0].Batch, updateResult.Ops.Batch...) + + } + return out, nil + +} + +func appendRequest(r *UpdateDonRequest) *AppendNodeCapabilitiesRequest { + out := &AppendNodeCapabilitiesRequest{ + RegistryChainSel: r.RegistryChainSel, + P2pToCapabilities: make(map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability), + MCMSConfig: r.MCMSConfig, + } + for _, p2pid := range r.P2PIDs { + if _, exists := out.P2pToCapabilities[p2pid]; !exists { + out.P2pToCapabilities[p2pid] = make([]kcr.CapabilitiesRegistryCapability, 0) + } + for _, cc := range r.CapabilityConfigs { + out.P2pToCapabilities[p2pid] = append(out.P2pToCapabilities[p2pid], cc.Capability) + } + } + return out +} + +func updateDonRequest(env deployment.Environment, r *UpdateDonRequest) (*internal.UpdateDonRequest, error) { + resp, err := kslib.GetContractSets(env.Logger, &kslib.GetContractSetsRequest{ + Chains: env.Chains, + AddressBook: env.ExistingAddresses, + }) + if err != nil { + return nil, fmt.Errorf("failed to get contract sets: %w", err) + } + contractSet := resp.ContractSets[r.RegistryChainSel] + + return &internal.UpdateDonRequest{ + Chain: env.Chains[r.RegistryChainSel], + ContractSet: &contractSet, + P2PIDs: r.P2PIDs, + CapabilityConfigs: r.CapabilityConfigs, + UseMCMS: r.UseMCMS(), + }, nil } diff --git a/deployment/keystone/changeset/update_don_test.go b/deployment/keystone/changeset/update_don_test.go new file mode 100644 index 00000000000..18287da6887 --- /dev/null +++ b/deployment/keystone/changeset/update_don_test.go @@ -0,0 +1,165 @@ +package changeset_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + commonchangeset "github.com/smartcontractkit/chainlink/deployment/common/changeset" + "github.com/smartcontractkit/chainlink/deployment/keystone/changeset" + "github.com/smartcontractkit/chainlink/deployment/keystone/changeset/internal" + kcr "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry" + "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/p2pkey" +) + +func TestUpdateDon(t *testing.T) { + t.Parallel() + + var ( + capA = kcr.CapabilitiesRegistryCapability{ + LabelledName: "capA", + Version: "0.4.2", + } + capB = kcr.CapabilitiesRegistryCapability{ + LabelledName: "capB", + Version: "3.16.0", + } + caps = []kcr.CapabilitiesRegistryCapability{capA, capB} + ) + t.Run("no mcms", func(t *testing.T) { + te := SetupTestEnv(t, TestConfig{ + WFDonConfig: DonConfig{N: 4}, + AssetDonConfig: DonConfig{N: 4}, + WriterDonConfig: DonConfig{N: 4}, + NumChains: 1, + }) + + // contract set is already deployed with capabilities + // we have to keep track of the existing capabilities to add to the new ones + var p2pIDs []p2pkey.PeerID + newCapabilities := make(map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability) + for id, _ := range te.WFNodes { + k, err := p2pkey.MakePeerID(id) + require.NoError(t, err) + p2pIDs = append(p2pIDs, k) + newCapabilities[k] = caps + } + + t.Run("succeeds if update sets new and existing capabilities", func(t *testing.T) { + cfg := changeset.UpdateDonRequest{ + RegistryChainSel: te.RegistrySelector, + P2PIDs: p2pIDs, + CapabilityConfigs: []changeset.CapabilityConfig{ + { + Capability: capA, + }, + { + Capability: capB, + }, + }, + } + + csOut, err := changeset.UpdateDon(te.Env, &cfg) + require.NoError(t, err) + require.Len(t, csOut.Proposals, 0) + require.Nil(t, csOut.AddressBook) + + assertDonContainsCapabilities(t, te.ContractSets()[te.RegistrySelector].CapabilitiesRegistry, caps, p2pIDs) + }) + }) + t.Run("with mcms", func(t *testing.T) { + te := SetupTestEnv(t, TestConfig{ + WFDonConfig: DonConfig{N: 4}, + AssetDonConfig: DonConfig{N: 4}, + WriterDonConfig: DonConfig{N: 4}, + NumChains: 1, + UseMCMS: true, + }) + + // contract set is already deployed with capabilities + // we have to keep track of the existing capabilities to add to the new ones + var p2pIDs []p2pkey.PeerID + for id, _ := range te.WFNodes { + k, err := p2pkey.MakePeerID(id) + require.NoError(t, err) + p2pIDs = append(p2pIDs, k) + } + + cfg := changeset.UpdateDonRequest{ + RegistryChainSel: te.RegistrySelector, + P2PIDs: p2pIDs, + CapabilityConfigs: []changeset.CapabilityConfig{ + { + Capability: capA, + }, + { + Capability: capB, + }, + }, + MCMSConfig: &changeset.MCMSConfig{MinDuration: 0}, + } + + csOut, err := changeset.UpdateDon(te.Env, &cfg) + require.NoError(t, err) + + if true { + require.Len(t, csOut.Proposals, 1) + require.Len(t, csOut.Proposals[0].Transactions, 1) // append node capabilties cs, update don + require.Len(t, csOut.Proposals[0].Transactions[0].Batch, 3) // add capabilities, update nodes, update don + require.Nil(t, csOut.AddressBook) + } else { + require.Len(t, csOut.Proposals, 1) + require.Len(t, csOut.Proposals[0].Transactions, 2) // append node capabilties cs, update don + require.Len(t, csOut.Proposals[0].Transactions[0].Batch, 2) // add capabilities, update nodes + require.Len(t, csOut.Proposals[0].Transactions[1].Batch, 1) // update don + require.Nil(t, csOut.AddressBook) + } + + // now apply the changeset such that the proposal is signed and execed + contracts := te.ContractSets()[te.RegistrySelector] + timelockContracts := map[uint64]*commonchangeset.TimelockExecutionContracts{ + te.RegistrySelector: { + Timelock: contracts.Timelock, + CallProxy: contracts.CallProxy, + }, + } + _, err = commonchangeset.ApplyChangesets(t, te.Env, timelockContracts, []commonchangeset.ChangesetApplication{ + { + Changeset: commonchangeset.WrapChangeSet(changeset.UpdateDon), + Config: &cfg, + }, + }) + require.NoError(t, err) + assertDonContainsCapabilities(t, te.ContractSets()[te.RegistrySelector].CapabilitiesRegistry, caps, p2pIDs) + }) +} + +func assertDonContainsCapabilities(t *testing.T, registry *kcr.CapabilitiesRegistry, want []kcr.CapabilitiesRegistryCapability, p2pIDs []p2pkey.PeerID) { + dons, err := registry.GetDONs(nil) + require.NoError(t, err) + var got *kcr.CapabilitiesRegistryDONInfo + for i, don := range dons { + if internal.SortedHash(internal.PeerIDsToBytes(p2pIDs)) == internal.SortedHash(don.NodeP2PIds) { + got = &dons[i] + break + } + } + require.NotNil(t, got, "missing don with p2pIDs %v", p2pIDs) + wantHashes := make([][32]byte, len(want)) + for i, c := range want { + h, err := registry.GetHashedCapabilityId(nil, c.LabelledName, c.Version) + require.NoError(t, err) + wantHashes[i] = h + assert.Contains(t, capIDsFromCapCfgs(got.CapabilityConfigurations), h, "missing capability %v", c) + } + assert.LessOrEqual(t, len(want), len(got.CapabilityConfigurations), "too many capabilities") +} + +func capIDsFromCapCfgs(cfgs []kcr.CapabilitiesRegistryCapabilityConfiguration) [][32]byte { + out := make([][32]byte, len(cfgs)) + for i, c := range cfgs { + out[i] = c.CapabilityId + } + return out +} diff --git a/deployment/keystone/changeset/update_node_capabilities.go b/deployment/keystone/changeset/update_node_capabilities.go index d50c07c9f06..9c9d5585fc2 100644 --- a/deployment/keystone/changeset/update_node_capabilities.go +++ b/deployment/keystone/changeset/update_node_capabilities.go @@ -53,10 +53,11 @@ type UpdateNodeCapabilitiesRequest = MutateNodeCapabilitiesRequest // MutateNodeCapabilitiesRequest is a request to change the capabilities of nodes in the registry type MutateNodeCapabilitiesRequest struct { - RegistryChainSel uint64 - + RegistryChainSel uint64 P2pToCapabilities map[p2pkey.PeerID][]kcr.CapabilitiesRegistryCapability - UseMCMS bool + + // MCMSConfig is optional. If non-nil, the changes will be proposed using MCMS. + MCMSConfig *MCMSConfig } func (req *MutateNodeCapabilitiesRequest) Validate() error { @@ -71,6 +72,10 @@ func (req *MutateNodeCapabilitiesRequest) Validate() error { return nil } +func (req *MutateNodeCapabilitiesRequest) UseMCMS() bool { + return req.MCMSConfig != nil +} + func (req *MutateNodeCapabilitiesRequest) updateNodeCapabilitiesImplRequest(e deployment.Environment) (*internal.UpdateNodeCapabilitiesImplRequest, error) { if err := req.Validate(); err != nil { return nil, fmt.Errorf("failed to validate UpdateNodeCapabilitiesRequest: %w", err) @@ -95,7 +100,7 @@ func (req *MutateNodeCapabilitiesRequest) updateNodeCapabilitiesImplRequest(e de Chain: registryChain, ContractSet: &contractSet, P2pToCapabilities: req.P2pToCapabilities, - UseMCMS: req.UseMCMS, + UseMCMS: req.UseMCMS(), }, nil } @@ -112,7 +117,7 @@ func UpdateNodeCapabilities(env deployment.Environment, req *UpdateNodeCapabilit } out := deployment.ChangesetOutput{} - if req.UseMCMS { + if req.UseMCMS() { if r.Ops == nil { return out, fmt.Errorf("expected MCMS operation to be non-nil") } @@ -128,7 +133,7 @@ func UpdateNodeCapabilities(env deployment.Environment, req *UpdateNodeCapabilit proposerMCMSes, []timelock.BatchChainOperation{*r.Ops}, "proposal to set update node capabilities", - 0, + req.MCMSConfig.MinDuration, ) if err != nil { return out, fmt.Errorf("failed to build proposal: %w", err) diff --git a/deployment/keystone/changeset/update_node_capabilities_test.go b/deployment/keystone/changeset/update_node_capabilities_test.go index 8c6378d809f..cb5588ff3d1 100644 --- a/deployment/keystone/changeset/update_node_capabilities_test.go +++ b/deployment/keystone/changeset/update_node_capabilities_test.go @@ -106,7 +106,7 @@ func TestUpdateNodeCapabilities(t *testing.T) { cfg := changeset.UpdateNodeCapabilitiesRequest{ RegistryChainSel: te.RegistrySelector, P2pToCapabilities: capabiltiesToSet, - UseMCMS: true, + MCMSConfig: &changeset.MCMSConfig{MinDuration: 0}, } csOut, err := changeset.UpdateNodeCapabilities(te.Env, &cfg) diff --git a/deployment/keystone/changeset/update_nodes.go b/deployment/keystone/changeset/update_nodes.go index 4e2a4f7f4c6..bb12f32cb94 100644 --- a/deployment/keystone/changeset/update_nodes.go +++ b/deployment/keystone/changeset/update_nodes.go @@ -2,6 +2,7 @@ package changeset import ( "fmt" + "time" "github.com/ethereum/go-ethereum/common" "github.com/smartcontractkit/ccip-owner-contracts/pkg/gethwrappers" @@ -14,14 +15,31 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/p2pkey" ) +type MCMSConfig struct { + MinDuration time.Duration +} + var _ deployment.ChangeSet[*UpdateNodesRequest] = UpdateNodes type UpdateNodesRequest struct { RegistryChainSel uint64 P2pToUpdates map[p2pkey.PeerID]NodeUpdate - UseMCMS bool + // MCMSConfig is optional. If non-nil, the changes will be proposed using MCMS. + MCMSConfig *MCMSConfig +} + +func (r *UpdateNodesRequest) Validate() error { + if r.P2pToUpdates == nil { + return fmt.Errorf("P2pToUpdates must be non-nil") + } + return nil +} + +func (r UpdateNodesRequest) UseMCMS() bool { + return r.MCMSConfig != nil } + type NodeUpdate = internal.NodeUpdate // UpdateNodes updates the a set of nodes. @@ -48,14 +66,14 @@ func UpdateNodes(env deployment.Environment, req *UpdateNodesRequest) (deploymen Chain: registryChain, ContractSet: &contracts, P2pToUpdates: req.P2pToUpdates, - UseMCMS: req.UseMCMS, + UseMCMS: req.UseMCMS(), }) if err != nil { return deployment.ChangesetOutput{}, fmt.Errorf("failed to update don: %w", err) } out := deployment.ChangesetOutput{} - if req.UseMCMS { + if req.UseMCMS() { if resp.Ops == nil { return out, fmt.Errorf("expected MCMS operation to be non-nil") } @@ -71,7 +89,7 @@ func UpdateNodes(env deployment.Environment, req *UpdateNodesRequest) (deploymen proposerMCMSes, []timelock.BatchChainOperation{*resp.Ops}, "proposal to set update nodes", - 0, + req.MCMSConfig.MinDuration, ) if err != nil { return out, fmt.Errorf("failed to build proposal: %w", err) diff --git a/deployment/keystone/changeset/update_nodes_test.go b/deployment/keystone/changeset/update_nodes_test.go index aebe10aa3d5..be3bfb12ee6 100644 --- a/deployment/keystone/changeset/update_nodes_test.go +++ b/deployment/keystone/changeset/update_nodes_test.go @@ -79,7 +79,7 @@ func TestUpdateNodes(t *testing.T) { cfg := changeset.UpdateNodesRequest{ RegistryChainSel: te.RegistrySelector, P2pToUpdates: updates, - UseMCMS: true, + MCMSConfig: &changeset.MCMSConfig{MinDuration: 0}, } csOut, err := changeset.UpdateNodes(te.Env, &cfg) @@ -101,7 +101,7 @@ func TestUpdateNodes(t *testing.T) { Config: &changeset.UpdateNodesRequest{ RegistryChainSel: te.RegistrySelector, P2pToUpdates: updates, - UseMCMS: true, + MCMSConfig: &changeset.MCMSConfig{MinDuration: 0}, }, }, }) diff --git a/deployment/keystone/deploy.go b/deployment/keystone/deploy.go index b83b5114391..7d3e5391219 100644 --- a/deployment/keystone/deploy.go +++ b/deployment/keystone/deploy.go @@ -14,15 +14,12 @@ import ( "time" "github.com/ethereum/go-ethereum/accounts/abi/bind" - "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/rpc" "golang.org/x/exp/maps" - "github.com/smartcontractkit/ccip-owner-contracts/pkg/gethwrappers" "github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/mcms" "github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/timelock" "github.com/smartcontractkit/chainlink/deployment" - "github.com/smartcontractkit/chainlink/deployment/common/proposalutils" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/durationpb" @@ -348,7 +345,7 @@ func ConfigureOCR3Contract(env *deployment.Environment, chainSel uint64, dons [] type ConfigureOCR3Resp struct { OCR2OracleConfig - Proposal *timelock.MCMSWithTimelockProposal + Ops *timelock.BatchChainOperation } type ConfigureOCR3Config struct { @@ -405,7 +402,7 @@ func ConfigureOCR3ContractFromJD(env *deployment.Environment, cfg ConfigureOCR3C } return &ConfigureOCR3Resp{ OCR2OracleConfig: r.ocrConfig, - Proposal: r.proposal, + Ops: r.ops, }, nil } @@ -941,13 +938,13 @@ func containsAllDONs(donInfos []kcr.CapabilitiesRegistryDONInfo, p2pIdsToDon map // 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, contractSet ContractSet, dons []RegisteredDon, useMCMS bool) ([]timelock.MCMSWithTimelockProposal, error) { +func configureForwarder(lggr logger.Logger, chain deployment.Chain, contractSet ContractSet, dons []RegisteredDon, useMCMS bool) (map[uint64]timelock.BatchChainOperation, error) { if contractSet.Forwarder == nil { return nil, errors.New("nil forwarder contract") } var ( - fwdr = contractSet.Forwarder - proposals []timelock.MCMSWithTimelockProposal + fwdr = contractSet.Forwarder + opMap = make(map[uint64]timelock.BatchChainOperation) ) for _, dn := range dons { if !dn.Info.AcceptsWorkflows { @@ -982,26 +979,9 @@ func configureForwarder(lggr logger.Logger, chain deployment.Chain, contractSet }, }, } - timelocksPerChain := map[uint64]common.Address{ - chain.Selector: contractSet.Timelock.Address(), - } - proposerMCMSes := map[uint64]*gethwrappers.ManyChainMultiSig{ - chain.Selector: contractSet.ProposerMcm, - } - - proposal, err := proposalutils.BuildProposalFromBatches( - timelocksPerChain, - proposerMCMSes, - []timelock.BatchChainOperation{ops}, - "proposal to set forward config", - 0, - ) - if err != nil { - return nil, fmt.Errorf("failed to build proposal: %w", err) - } - proposals = append(proposals, *proposal) + opMap[chain.Selector] = ops } lggr.Debugw("configured forwarder", "forwarder", fwdr.Address().String(), "donId", dn.Info.Id, "version", ver, "f", dn.Info.F, "signers", signers) } - return proposals, nil + return opMap, nil } diff --git a/deployment/keystone/forwarder_deployer.go b/deployment/keystone/forwarder_deployer.go index d7cfa7991f4..7c7b3a1ed93 100644 --- a/deployment/keystone/forwarder_deployer.go +++ b/deployment/keystone/forwarder_deployer.go @@ -64,7 +64,7 @@ type ConfigureForwarderContractsRequest struct { UseMCMS bool } type ConfigureForwarderContractsResponse struct { - Proposals []timelock.MCMSWithTimelockProposal + OpsPerChain map[uint64]timelock.BatchChainOperation } // Depreciated: use [changeset.ConfigureForwarders] instead @@ -79,7 +79,7 @@ func ConfigureForwardContracts(env *deployment.Environment, req ConfigureForward return nil, fmt.Errorf("failed to get contract sets: %w", err) } - var allProposals []timelock.MCMSWithTimelockProposal + opPerChain := make(map[uint64]timelock.BatchChainOperation) // configure forwarders on all chains for _, chain := range env.Chains { // get the forwarder contract for the chain @@ -87,13 +87,15 @@ func ConfigureForwardContracts(env *deployment.Environment, req ConfigureForward if !ok { return nil, fmt.Errorf("failed to get contract set for chain %d", chain.Selector) } - proposals, err := configureForwarder(env.Logger, chain, contracts, req.Dons, req.UseMCMS) + ops, err := configureForwarder(env.Logger, chain, contracts, req.Dons, req.UseMCMS) if err != nil { return nil, fmt.Errorf("failed to configure forwarder for chain selector %d: %w", chain.Selector, err) } - allProposals = append(allProposals, proposals...) + for k, op := range ops { + opPerChain[k] = op + } } return &ConfigureForwarderContractsResponse{ - Proposals: allProposals, + OpsPerChain: opPerChain, }, nil } diff --git a/deployment/keystone/ocr3config.go b/deployment/keystone/ocr3config.go index ccd738636ed..aed142ea116 100644 --- a/deployment/keystone/ocr3config.go +++ b/deployment/keystone/ocr3config.go @@ -18,12 +18,10 @@ import ( "github.com/smartcontractkit/libocr/offchainreporting2plus/ocr3confighelper" "github.com/smartcontractkit/libocr/offchainreporting2plus/types" - "github.com/smartcontractkit/ccip-owner-contracts/pkg/gethwrappers" "github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/mcms" "github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/timelock" "github.com/smartcontractkit/chainlink/deployment" - "github.com/smartcontractkit/chainlink/deployment/common/proposalutils" kocr3 "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/ocr3_capability" "github.com/smartcontractkit/chainlink/v2/core/services/keystore/chaintype" "github.com/smartcontractkit/chainlink/v2/core/services/ocrcommon" @@ -300,7 +298,7 @@ func (r configureOCR3Request) generateOCR3Config() (OCR2OracleConfig, error) { type configureOCR3Response struct { ocrConfig OCR2OracleConfig - proposal *timelock.MCMSWithTimelockProposal + ops *timelock.BatchChainOperation } func configureOCR3contract(req configureOCR3Request) (*configureOCR3Response, error) { @@ -333,7 +331,7 @@ func configureOCR3contract(req configureOCR3Request) (*configureOCR3Response, er return nil, fmt.Errorf("failed to call SetConfig for OCR3 contract %s using mcms: %T: %w", req.contract.Address().String(), req.useMCMS, err) } - var proposal *timelock.MCMSWithTimelockProposal + var ops *timelock.BatchChainOperation if !req.useMCMS { _, err = req.chain.Confirm(tx) if err != nil { @@ -341,7 +339,7 @@ func configureOCR3contract(req configureOCR3Request) (*configureOCR3Response, er return nil, fmt.Errorf("failed to confirm SetConfig for OCR3 contract %s: %w", req.contract.Address().String(), err) } } else { - ops := timelock.BatchChainOperation{ + ops = &timelock.BatchChainOperation{ ChainIdentifier: mcms.ChainIdentifier(req.chain.Selector), Batch: []mcms.Operation{ { @@ -351,24 +349,7 @@ func configureOCR3contract(req configureOCR3Request) (*configureOCR3Response, er }, }, } - timelocksPerChain := map[uint64]common.Address{ - req.chain.Selector: req.contractSet.Timelock.Address(), - } - proposerMCMSes := map[uint64]*gethwrappers.ManyChainMultiSig{ - req.chain.Selector: req.contractSet.ProposerMcm, - } - - proposal, err = proposalutils.BuildProposalFromBatches( - timelocksPerChain, - proposerMCMSes, - []timelock.BatchChainOperation{ops}, - "proposal to set ocr3 config", - 0, - ) - if err != nil { - return nil, fmt.Errorf("failed to build proposal: %w", err) - } } - return &configureOCR3Response{ocrConfig, proposal}, nil + return &configureOCR3Response{ocrConfig, ops}, nil }