From 694a81234c53d272c26b0d46093ec3ba713afbef Mon Sep 17 00:00:00 2001 From: Domino Valdano <2644901+reductionista@users.noreply.github.com> Date: Fri, 31 May 2024 20:49:26 -0700 Subject: [PATCH 1/3] Unregister filters for both Keepers 2.1 & 2.0 jobs Also: fix log msg typo --- core/services/job/spawner_test.go | 1 + core/services/ocr2/delegate.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/core/services/job/spawner_test.go b/core/services/job/spawner_test.go index 8ed08a1cb8a..31d54713b57 100644 --- a/core/services/job/spawner_test.go +++ b/core/services/job/spawner_test.go @@ -18,6 +18,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/types" "github.com/smartcontractkit/chainlink-common/pkg/utils" "github.com/smartcontractkit/chainlink-common/pkg/utils/mailbox/mailboxtest" + "github.com/smartcontractkit/chainlink/v2/core/capabilities" "github.com/smartcontractkit/chainlink/v2/core/bridges" diff --git a/core/services/ocr2/delegate.go b/core/services/ocr2/delegate.go index 1c70195dd43..48d19d6bd5b 100644 --- a/core/services/ocr2/delegate.go +++ b/core/services/ocr2/delegate.go @@ -303,7 +303,7 @@ func (d *Delegate) cleanupEVM(ctx context.Context, jb job.Job, relayID types.Rel spec := jb.OCR2OracleSpec chain, err := d.legacyChains.Get(relayID.ChainID) if err != nil { - d.lggr.Error("cleanupEVM: failed to chain get chain %s", "err", relayID.ChainID, err) + d.lggr.Errorw("cleanupEVM: failed to get chain id", "chainId", relayID.ChainID, "err", err) return nil } lp := chain.LogPoller() @@ -316,10 +316,16 @@ func (d *Delegate) cleanupEVM(ctx context.Context, jb job.Job, relayID types.Rel d.lggr.Errorw("failed to derive ocr2vrf filter names from spec", "err", err, "spec", spec) } case types.OCR2Keeper: + // Not worth the effort to validate and parse the job spec config to figure out whether this is v2.0 or v2.1, + // simpler and faster to just Unregister them both filters, err = ocr2keeper.FilterNamesFromSpec20(spec) if err != nil { d.lggr.Errorw("failed to derive ocr2keeper filter names from spec", "err", err, "spec", spec) } + filters, err = ocr2keeper.FilterNamesFromSpec21(spec) + if err != nil { + d.lggr.Errorw("failed to derive ocr2keeper filter names from spec", "err", err, "spec", spec) + } default: return nil } From e9021dfcbad4526926f3567fd91e9e886c40bb54 Mon Sep 17 00:00:00 2001 From: Domino Valdano <2644901+reductionista@users.noreply.github.com> Date: Tue, 28 May 2024 09:19:50 -0700 Subject: [PATCH 2/3] Replace ocr2vrf job with ocr2keeper job in TestSpawner_CreateJobDeleteJob test --- core/services/job/helpers_test.go | 78 ++++++++++++------------------- core/services/job/spawner_test.go | 19 ++++---- 2 files changed, 39 insertions(+), 58 deletions(-) diff --git a/core/services/job/helpers_test.go b/core/services/job/helpers_test.go index 7120fe4200c..d69056ae819 100644 --- a/core/services/job/helpers_test.go +++ b/core/services/job/helpers_test.go @@ -1,8 +1,6 @@ package job_test import ( - "crypto/rand" - "encoding/hex" "fmt" "math/big" "testing" @@ -10,6 +8,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/google/uuid" + "github.com/hashicorp/consul/sdk/freeport" "github.com/lib/pq" "github.com/pelletier/go-toml" "github.com/stretchr/testify/require" @@ -49,31 +48,30 @@ observationSource = """ %s """ ` - ocr2vrfJobSpecTemplate = ` -type = "offchainreporting2" -schemaVersion = 1 -name = "ocr2 vrf spec" -maxTaskDuration = "10s" -contractID = "%s" -ocrKeyBundleID = "%s" -relay = "evm" -pluginType = "ocr2vrf" -transmitterID = "%s" -forwardingAllowed = %t + + ocr2Keeper21JobSpecTemplate = ` +type = "offchainreporting2" +pluginType = "ocr2automation" +relay = "evm" +name = "ocr2keeper" +schemaVersion = 1 +contractID = "%s" +contractConfigTrackerPollInterval = "15s" +ocrKeyBundleID = "%s" +transmitterID = "%s" +p2pv2Bootstrappers = [ +"%s" +] [relayConfig] -chainID = %d -fromBlock = %d -sendingKeys = [%s] +chainID = %d [pluginConfig] -dkgEncryptionPublicKey = "%s" -dkgSigningPublicKey = "%s" -dkgKeyID = "%s" -dkgContractAddress = "%s" - -vrfCoordinatorAddress = "%s" -linkEthFeedAddress = "%s" +maxServiceWorkers = 100 +cacheEvictionInterval = "1s" +mercuryCredentialName = "%s" +contractVersion = "v2.1" +useBufferV1 = %v ` voterTurnoutDataSourceTemplate = ` // data source 1 @@ -268,38 +266,20 @@ func makeOCRJobSpecFromToml(t *testing.T, jobSpecToml string) *job.Job { return &jb } -func makeOCR2VRFJobSpec(t testing.TB, ks keystore.Master, transmitter common.Address, chainID *big.Int, fromBlock uint64) *job.Job { +func makeOCR2Keeper21JobSpec(t testing.TB, ks keystore.Master, transmitter common.Address, chainID *big.Int) *job.Job { t.Helper() ctx := testutils.Context(t) - useForwarders := false - _, beacon := cltest.MustInsertRandomKey(t, ks.Eth()) - _, coordinator := cltest.MustInsertRandomKey(t, ks.Eth()) - _, feed := cltest.MustInsertRandomKey(t, ks.Eth()) - _, dkg := cltest.MustInsertRandomKey(t, ks.Eth()) - sendingKeys := fmt.Sprintf(`"%s"`, transmitter) + bootstrapNodePort := freeport.GetOne(t) + bootstrapPeerID := "peerId" + kb, _ := ks.OCR2().Create(ctx, chaintype.EVM) + _, registry := cltest.MustInsertRandomKey(t, ks.Eth()) - vrfKey := make([]byte, 32) - _, err := rand.Read(vrfKey) - require.NoError(t, err) + ocr2Keeper21Job := fmt.Sprintf(ocr2Keeper21JobSpecTemplate, registry.String(), kb.ID(), transmitter, + fmt.Sprintf("%s127.0.0.1:%d", bootstrapPeerID, bootstrapNodePort), chainID, "mercury cred", false) - ocr2vrfJob := fmt.Sprintf(ocr2vrfJobSpecTemplate, - beacon.String(), - kb.ID(), - "", - useForwarders, - chainID, - fromBlock, - sendingKeys, - ks.DKGEncrypt(), - ks.DKGSign(), - hex.EncodeToString(vrfKey[:]), - dkg.String(), - coordinator.String(), - feed.String(), - ) - jobSpec := makeOCR2JobSpecFromToml(t, ocr2vrfJob) + jobSpec := makeOCR2JobSpecFromToml(t, ocr2Keeper21Job) return jobSpec } diff --git a/core/services/job/spawner_test.go b/core/services/job/spawner_test.go index 31d54713b57..bf5e2a2d21e 100644 --- a/core/services/job/spawner_test.go +++ b/core/services/job/spawner_test.go @@ -292,7 +292,8 @@ func TestSpawner_CreateJobDeleteJob(t *testing.T) { relayExtenders := evmtest.NewChainRelayExtenders(t, testopts) assert.Equal(t, relayExtenders.Len(), 1) legacyChains := evmrelay.NewLegacyChainsFromRelayerExtenders(relayExtenders) - chain := evmtest.MustGetDefaultChain(t, legacyChains) + chain, err := legacyChains.Get("0") + require.NoError(t, err) evmRelayer, err := evmrelayer.NewRelayer(lggr, chain, evmrelayer.RelayerOpts{ DS: db, @@ -306,7 +307,7 @@ func TestSpawner_CreateJobDeleteJob(t *testing.T) { r: evmRelayer, } - jobOCR2VRF := makeOCR2VRFJobSpec(t, keyStore, address, chain.ID(), 2) + jobOCR2Keeper := makeOCR2Keeper21JobSpec(t, keyStore, address, chain.ID()) orm := NewTestORM(t, db, pipeline.NewORM(db, lggr, config.JobPipeline().MaxSuccessfulRuns()), bridges.NewORM(db), keyStore) mailMon := servicetest.Run(t, mailboxtest.NewMonitor(t)) @@ -316,26 +317,26 @@ func TestSpawner_CreateJobDeleteJob(t *testing.T) { d := ocr2.NewDelegate(nil, orm, nil, nil, nil, nil, nil, monitoringEndpoint, legacyChains, lggr, ocr2DelegateConfig, keyStore.OCR2(), keyStore.DKGSign(), keyStore.DKGEncrypt(), ethKeyStore, testRelayGetter, mailMon, capabilities.NewRegistry(lggr)) - delegateOCR2 := &delegate{jobOCR2VRF.Type, []job.ServiceCtx{}, 0, nil, d} + delegateOCR2 := &delegate{jobOCR2Keeper.Type, []job.ServiceCtx{}, 0, nil, d} spawner := job.NewSpawner(orm, config.Database(), noopChecker{}, map[job.Type]job.Delegate{ - jobOCR2VRF.Type: delegateOCR2, + jobOCR2Keeper.Type: delegateOCR2, }, lggr, nil) ctx := testutils.Context(t) - err = spawner.CreateJob(ctx, nil, jobOCR2VRF) + err = spawner.CreateJob(ctx, nil, jobOCR2Keeper) require.NoError(t, err) - jobSpecID := jobOCR2VRF.ID - delegateOCR2.jobID = jobOCR2VRF.ID + jobSpecID := jobOCR2Keeper.ID + delegateOCR2.jobID = jobOCR2Keeper.ID lp.On("UnregisterFilter", mock.Anything, mock.Anything).Return(nil).Run(func(args mock.Arguments) { - lggr.Debugf("Got here, with args %v", args) + lggr.Debugf("UnregisterFilter called with args %v", args) }) err = spawner.DeleteJob(ctx, nil, jobSpecID) require.NoError(t, err) - lp.AssertNumberOfCalls(t, "UnregisterFilter", 3) + lp.AssertNumberOfCalls(t, "UnregisterFilter", 4) lp.On("Close").Return(nil).Once() spawner.Close() From 022f8722df1ae124ab0d7d504e93d0244ca2710b Mon Sep 17 00:00:00 2001 From: Domino Valdano <2644901+reductionista@users.noreply.github.com> Date: Fri, 31 May 2024 21:35:14 -0700 Subject: [PATCH 3/3] Append filter names instead of overwriting --- core/services/job/spawner_test.go | 2 +- core/services/ocr2/delegate.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/core/services/job/spawner_test.go b/core/services/job/spawner_test.go index bf5e2a2d21e..4abb81eda3a 100644 --- a/core/services/job/spawner_test.go +++ b/core/services/job/spawner_test.go @@ -336,7 +336,7 @@ func TestSpawner_CreateJobDeleteJob(t *testing.T) { err = spawner.DeleteJob(ctx, nil, jobSpecID) require.NoError(t, err) - lp.AssertNumberOfCalls(t, "UnregisterFilter", 4) + lp.AssertNumberOfCalls(t, "UnregisterFilter", 6) lp.On("Close").Return(nil).Once() spawner.Close() diff --git a/core/services/ocr2/delegate.go b/core/services/ocr2/delegate.go index 48d19d6bd5b..2c253d592c2 100644 --- a/core/services/ocr2/delegate.go +++ b/core/services/ocr2/delegate.go @@ -322,10 +322,11 @@ func (d *Delegate) cleanupEVM(ctx context.Context, jb job.Job, relayID types.Rel if err != nil { d.lggr.Errorw("failed to derive ocr2keeper filter names from spec", "err", err, "spec", spec) } - filters, err = ocr2keeper.FilterNamesFromSpec21(spec) - if err != nil { + filters21, err2 := ocr2keeper.FilterNamesFromSpec21(spec) + if err2 != nil { d.lggr.Errorw("failed to derive ocr2keeper filter names from spec", "err", err, "spec", spec) } + filters = append(filters, filters21...) default: return nil }