From 9b2a044e0f6b85506385d1023ac10d020483731c Mon Sep 17 00:00:00 2001 From: AnieeG Date: Fri, 31 Jan 2025 17:23:29 -0800 Subject: [PATCH] remove jobspec from changeset output --- deployment/ccip/changeset/cs_deploy_chain.go | 2 - deployment/ccip/changeset/cs_home_chain.go | 3 -- deployment/ccip/changeset/cs_jobspec_test.go | 52 ++++++++++++------- deployment/ccip/changeset/cs_prerequisites.go | 2 - .../changeset/testhelpers/test_environment.go | 17 ++---- deployment/ccip/changeset/v1_5/cs_jobspec.go | 28 +++++++++- deployment/changeset.go | 2 - deployment/common/changeset/save_existing.go | 1 - deployment/common/changeset/test_helpers.go | 20 ------- 9 files changed, 65 insertions(+), 62 deletions(-) diff --git a/deployment/ccip/changeset/cs_deploy_chain.go b/deployment/ccip/changeset/cs_deploy_chain.go index b8305916947..17e056a5e3a 100644 --- a/deployment/ccip/changeset/cs_deploy_chain.go +++ b/deployment/ccip/changeset/cs_deploy_chain.go @@ -59,8 +59,6 @@ func DeployChainContractsChangeset(env deployment.Environment, c DeployChainCont return deployment.ChangesetOutput{ Proposals: []timelock.MCMSWithTimelockProposal{}, AddressBook: newAddresses, - Jobs: nil, - JobSpecs: nil, }, nil } diff --git a/deployment/ccip/changeset/cs_home_chain.go b/deployment/ccip/changeset/cs_home_chain.go index 90f4bcd7933..8c6159f945c 100644 --- a/deployment/ccip/changeset/cs_home_chain.go +++ b/deployment/ccip/changeset/cs_home_chain.go @@ -46,10 +46,7 @@ func DeployHomeChainChangeset(env deployment.Environment, cfg DeployHomeChainCon } return deployment.ChangesetOutput{ - Proposals: []timelock.MCMSWithTimelockProposal{}, AddressBook: ab, - JobSpecs: nil, - Jobs: nil, }, nil } diff --git a/deployment/ccip/changeset/cs_jobspec_test.go b/deployment/ccip/changeset/cs_jobspec_test.go index 8ffa810711d..d7ec83b017d 100644 --- a/deployment/ccip/changeset/cs_jobspec_test.go +++ b/deployment/ccip/changeset/cs_jobspec_test.go @@ -4,37 +4,51 @@ import ( "testing" "github.com/stretchr/testify/require" - "go.uber.org/zap/zapcore" "github.com/smartcontractkit/chainlink/deployment" "github.com/smartcontractkit/chainlink/deployment/ccip/changeset" "github.com/smartcontractkit/chainlink/deployment/ccip/changeset/testhelpers" - "github.com/smartcontractkit/chainlink/deployment/environment/memory" + commonchangeset "github.com/smartcontractkit/chainlink/deployment/common/changeset" ccip "github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/validate" - "github.com/smartcontractkit/chainlink/v2/core/logger" ) func TestJobSpecChangeset(t *testing.T) { t.Parallel() - lggr := logger.TestLogger(t) - e := memory.NewMemoryEnvironment(t, lggr, zapcore.InfoLevel, memory.MemoryEnvironmentConfig{ - Chains: 1, - Nodes: 4, + var err error + tenv, _ := testhelpers.NewMemoryEnvironment(t, testhelpers.WithNoJobsAndContracts()) + e := tenv.Env + nodes, err := deployment.NodeInfo(e.NodeIDs, e.Offchain) + require.NoError(t, err) + e, err = commonchangeset.ApplyChangesets(t, e, nil, []commonchangeset.ChangesetApplication{ + { + Changeset: commonchangeset.WrapChangeSet(changeset.DeployHomeChainChangeset), + Config: changeset.DeployHomeChainConfig{ + HomeChainSel: tenv.HomeChainSel, + RMNDynamicConfig: testhelpers.NewTestRMNDynamicConfig(), + RMNStaticConfig: testhelpers.NewTestRMNStaticConfig(), + NodeOperators: testhelpers.NewTestNodeOperator(e.Chains[tenv.HomeChainSel].DeployerKey.From), + NodeP2PIDsPerNodeOpAdmin: map[string][][32]byte{ + testhelpers.TestNodeOperator: nodes.NonBootstraps().PeerIDs(), + }, + }, + }, }) - // TODO: Replace this with a changeset which proposes the jobs, and returns job ids. - output, err := changeset.CCIPCapabilityJobspecChangeset(e, nil) require.NoError(t, err) - require.NotNil(t, output.JobSpecs) - nodes, err := deployment.NodeInfo(e.NodeIDs, e.Offchain) + output, err := changeset.CCIPCapabilityJobspecChangeset(e, nil) require.NoError(t, err) + require.NotEmpty(t, output.Jobs) + nodeIDs := make(map[string]struct{}) + for _, job := range output.Jobs { + require.NotEmpty(t, job.JobID) + require.NotEmpty(t, job.Spec) + require.NotEmpty(t, job.Node) + nodeIDs[job.Node] = struct{}{} + _, err = ccip.ValidatedCCIPSpec(job.Spec) + require.NoError(t, err) + } for _, node := range nodes { - jobs, exists := output.JobSpecs[node.NodeID] - require.True(t, exists) - require.NotNil(t, jobs) - for _, job := range jobs { - _, err = ccip.ValidatedCCIPSpec(job) - require.NoError(t, err) - } + _, ok := nodeIDs[node.NodeID] + require.True(t, ok) } } @@ -44,5 +58,5 @@ func TestJobSpecChangesetIdempotent(t *testing.T) { // as the job specs are already created in the first call output, err := changeset.CCIPCapabilityJobspecChangeset(e.Env, nil) require.NoError(t, err) - require.Empty(t, output.JobSpecs) + require.Empty(t, output.Jobs) } diff --git a/deployment/ccip/changeset/cs_prerequisites.go b/deployment/ccip/changeset/cs_prerequisites.go index c5b1f12a6fa..4871a51f250 100644 --- a/deployment/ccip/changeset/cs_prerequisites.go +++ b/deployment/ccip/changeset/cs_prerequisites.go @@ -53,8 +53,6 @@ func DeployPrerequisitesChangeset(env deployment.Environment, cfg DeployPrerequi return deployment.ChangesetOutput{ Proposals: []timelock.MCMSWithTimelockProposal{}, AddressBook: ab, - JobSpecs: nil, - Jobs: nil, }, nil } diff --git a/deployment/ccip/changeset/testhelpers/test_environment.go b/deployment/ccip/changeset/testhelpers/test_environment.go index 991d67a2e26..5a89c10f954 100644 --- a/deployment/ccip/changeset/testhelpers/test_environment.go +++ b/deployment/ccip/changeset/testhelpers/test_environment.go @@ -14,7 +14,6 @@ import ( "go.uber.org/zap/zapcore" "github.com/smartcontractkit/chainlink-common/pkg/logger" - jobv1 "github.com/smartcontractkit/chainlink-protos/job-distributor/v1/job" "github.com/smartcontractkit/chainlink-testing-framework/lib/utils/testcontext" "github.com/smartcontractkit/chainlink/deployment/ccip/changeset" @@ -243,19 +242,13 @@ func (d *DeployedEnv) TimelockContracts(t *testing.T) map[uint64]*proposalutils. } func (d *DeployedEnv) SetupJobs(t *testing.T) { - ctx := testcontext.Get(t) out, err := changeset.CCIPCapabilityJobspecChangeset(d.Env, struct{}{}) require.NoError(t, err) - for nodeID, jobs := range out.JobSpecs { - for _, job := range jobs { - // Note these auto-accept - _, err := d.Env.Offchain.ProposeJob(ctx, - &jobv1.ProposeJobRequest{ - NodeId: nodeID, - Spec: job, - }) - require.NoError(t, err) - } + require.NotEmpty(t, out.Jobs) + for _, job := range out.Jobs { + require.NotEmpty(t, job.JobID) + require.NotEmpty(t, job.Spec) + require.NotEmpty(t, job.Node) } // Wait for plugins to register filters? // TODO: Investigate how to avoid. diff --git a/deployment/ccip/changeset/v1_5/cs_jobspec.go b/deployment/ccip/changeset/v1_5/cs_jobspec.go index fd80a392136..882d78c6ff2 100644 --- a/deployment/ccip/changeset/v1_5/cs_jobspec.go +++ b/deployment/ccip/changeset/v1_5/cs_jobspec.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" + jobv1 "github.com/smartcontractkit/chainlink-protos/job-distributor/v1/job" + "github.com/smartcontractkit/chainlink/deployment" "github.com/smartcontractkit/chainlink/deployment/ccip/changeset" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/config" @@ -69,8 +71,32 @@ func JobSpecsForLanesChangeset(env deployment.Environment, c JobSpecsForLanesCon if err != nil { return deployment.ChangesetOutput{}, err } + // Now we propose the job specs to the offchain system. + var Jobs []deployment.ProposedJob + for nodeID, jobs := range nodesToJobSpecs { + for _, job := range jobs { + Jobs = append(Jobs, deployment.ProposedJob{ + Node: nodeID, + Spec: job, + }) + res, err := env.Offchain.ProposeJob(env.GetContext(), + &jobv1.ProposeJobRequest{ + NodeId: nodeID, + Spec: job, + }) + if err != nil { + // If we fail to propose a job, we should return an error and the jobs we've already proposed. + // This is so that we can retry the proposal with manual intervention. + // JOBID will be empty if the proposal failed. + return deployment.ChangesetOutput{ + Jobs: Jobs, + }, fmt.Errorf("failed to propose job: %w", err) + } + Jobs[len(Jobs)-1].JobID = res.Proposal.JobId + } + } return deployment.ChangesetOutput{ - JobSpecs: nodesToJobSpecs, + Jobs: Jobs, }, nil } diff --git a/deployment/changeset.go b/deployment/changeset.go index fbde1dbb3d1..9a72ae58070 100644 --- a/deployment/changeset.go +++ b/deployment/changeset.go @@ -33,8 +33,6 @@ type ProposedJob struct { // The address book here should contain only new addresses created in // this changeset. type ChangesetOutput struct { - // Deprecated: Prefer Jobs instead. - JobSpecs map[string][]string `deprecated:"true"` Jobs []ProposedJob Proposals []timelock.MCMSWithTimelockProposal AddressBook AddressBook diff --git a/deployment/common/changeset/save_existing.go b/deployment/common/changeset/save_existing.go index 602d41dc84e..b9b959ba10f 100644 --- a/deployment/common/changeset/save_existing.go +++ b/deployment/common/changeset/save_existing.go @@ -84,7 +84,6 @@ func SaveExistingContractsChangeset(env deployment.Environment, cfg ExistingCont return deployment.ChangesetOutput{ Proposals: []timelock.MCMSWithTimelockProposal{}, AddressBook: ab, - JobSpecs: nil, Jobs: nil, }, nil } diff --git a/deployment/common/changeset/test_helpers.go b/deployment/common/changeset/test_helpers.go index 3c2409566eb..92619cc0aeb 100644 --- a/deployment/common/changeset/test_helpers.go +++ b/deployment/common/changeset/test_helpers.go @@ -6,9 +6,6 @@ import ( mapset "github.com/deckarep/golang-set/v2" - jobv1 "github.com/smartcontractkit/chainlink-protos/job-distributor/v1/job" - "github.com/smartcontractkit/chainlink-testing-framework/lib/utils/testcontext" - "github.com/smartcontractkit/chainlink/deployment" "github.com/smartcontractkit/chainlink/deployment/common/proposalutils" ) @@ -51,23 +48,6 @@ func ApplyChangesets(t *testing.T, e deployment.Environment, timelockContractsPe } else { addresses = currentEnv.ExistingAddresses } - if out.JobSpecs != nil { - // TODO: Delete this when out.JobSpecs are no longer in use. - ctx := testcontext.Get(t) - for nodeID, jobs := range out.JobSpecs { - for _, job := range jobs { - // Note these auto-accept - _, err := currentEnv.Offchain.ProposeJob(ctx, - &jobv1.ProposeJobRequest{ - NodeId: nodeID, - Spec: job, - }) - if err != nil { - return e, fmt.Errorf("failed to propose job: %w", err) - } - } - } - } if out.Jobs != nil { // do nothing, as these jobs auto-accept. }