From 288257a4f0a0ab714630f266c32953a2b0619c36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Fri, 9 Aug 2024 22:07:17 +0900 Subject: [PATCH] multi-chain fixes (#14077) * ocr2: Shouldn't require "publicKey" under the multi-chain codepath * ocr2: Use parent ocrKeyBundleID for publicKey when using multi-chain --- .../__snapshots__/88_gen_jobspecs_test.snap | 4 -- core/scripts/keystone/templates/oracle.toml | 1 - core/services/ocr2/delegate.go | 2 +- core/services/ocr2/validate/validate.go | 19 ------ core/services/ocr2/validate/validate_test.go | 60 ------------------- core/services/ocrcommon/adapters.go | 5 +- core/services/ocrcommon/adapters_test.go | 4 +- 7 files changed, 5 insertions(+), 90 deletions(-) diff --git a/core/scripts/keystone/src/__snapshots__/88_gen_jobspecs_test.snap b/core/scripts/keystone/src/__snapshots__/88_gen_jobspecs_test.snap index 21e28f3801c..a4b4e6e3021 100755 --- a/core/scripts/keystone/src/__snapshots__/88_gen_jobspecs_test.snap +++ b/core/scripts/keystone/src/__snapshots__/88_gen_jobspecs_test.snap @@ -39,7 +39,6 @@ telemetryType = "plugin" [onchainSigningStrategy] strategyName = 'single-chain' [onchainSigningStrategy.config] -publicKey = '8fa807463ad73f9ee855cfd60ba406dcf98a2855b3dd8af613107b0f6890a707' -------------------------------- Oracle 1: @@ -69,7 +68,6 @@ telemetryType = "plugin" [onchainSigningStrategy] strategyName = 'single-chain' [onchainSigningStrategy.config] -publicKey = '8fa807463ad73f9ee855cfd60ba406dcf98a2855b3dd8af613107b0f6890a707' -------------------------------- Oracle 2: @@ -99,7 +97,6 @@ telemetryType = "plugin" [onchainSigningStrategy] strategyName = 'single-chain' [onchainSigningStrategy.config] -publicKey = '8fa807463ad73f9ee855cfd60ba406dcf98a2855b3dd8af613107b0f6890a707' -------------------------------- Oracle 3: @@ -129,7 +126,6 @@ telemetryType = "plugin" [onchainSigningStrategy] strategyName = 'single-chain' [onchainSigningStrategy.config] -publicKey = '8fa807463ad73f9ee855cfd60ba406dcf98a2855b3dd8af613107b0f6890a707' --- diff --git a/core/scripts/keystone/templates/oracle.toml b/core/scripts/keystone/templates/oracle.toml index f5d539873fe..6049ad925d4 100644 --- a/core/scripts/keystone/templates/oracle.toml +++ b/core/scripts/keystone/templates/oracle.toml @@ -23,4 +23,3 @@ telemetryType = "plugin" [onchainSigningStrategy] strategyName = 'single-chain' [onchainSigningStrategy.config] -publicKey = '8fa807463ad73f9ee855cfd60ba406dcf98a2855b3dd8af613107b0f6890a707' diff --git a/core/services/ocr2/delegate.go b/core/services/ocr2/delegate.go index 5c44825ca2a..f53ceaefa14 100644 --- a/core/services/ocr2/delegate.go +++ b/core/services/ocr2/delegate.go @@ -772,7 +772,7 @@ func (d *Delegate) newServicesGenericPlugin( } keyBundles[name] = os } - onchainKeyringAdapter, err = ocrcommon.NewOCR3OnchainKeyringMultiChainAdapter(keyBundles, lggr) + onchainKeyringAdapter, err = ocrcommon.NewOCR3OnchainKeyringMultiChainAdapter(keyBundles, kb.PublicKey(), lggr) if err != nil { return nil, err } diff --git a/core/services/ocr2/validate/validate.go b/core/services/ocr2/validate/validate.go index 8d98a282674..a224249e1e8 100644 --- a/core/services/ocr2/validate/validate.go +++ b/core/services/ocr2/validate/validate.go @@ -197,18 +197,6 @@ func (o *OCR2OnchainSigningStrategy) IsMultiChain() bool { return o.StrategyName == "multi-chain" } -func (o *OCR2OnchainSigningStrategy) PublicKey() (string, error) { - pk, ok := o.Config["publicKey"] - if !ok { - return "", nil - } - pkString, ok := pk.(string) - if !ok { - return "", fmt.Errorf("expected string publicKey value, but got: %T", pk) - } - return pkString, nil -} - func (o *OCR2OnchainSigningStrategy) ConfigCopy() job.JSONConfig { copiedConfig := make(job.JSONConfig) for k, v := range o.Config { @@ -251,13 +239,6 @@ func validateGenericPluginSpec(ctx context.Context, spec *job.OCR2OracleSpec, rc if err != nil { return err } - pk, ossErr := onchainSigningStrategy.PublicKey() - if ossErr != nil { - return ossErr - } - if pk == "" { - return errors.New("generic config invalid: must provide public key for the onchain signing strategy") - } } plugEnv := env.NewPlugin(p.PluginName) diff --git a/core/services/ocr2/validate/validate_test.go b/core/services/ocr2/validate/validate_test.go index b92752c647d..1356e0db628 100644 --- a/core/services/ocr2/validate/validate_test.go +++ b/core/services/ocr2/validate/validate_test.go @@ -49,7 +49,6 @@ chainID = 1337 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] juelsPerFeeCoinSource = """ ds1 [type=bridge name=voter_turnout]; @@ -105,7 +104,6 @@ chainID = 1337 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] juelsPerFeeCoinSource = """ ds1 [type=bridge name=voter_turnout]; @@ -150,7 +148,6 @@ chainID = 1337 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] `, assertion: func(t *testing.T, os job.Job, err error) { @@ -174,7 +171,6 @@ chainID = 1337 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] `, assertion: func(t *testing.T, os job.Job, err error) { @@ -200,7 +196,6 @@ chainID = 1337 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] `, assertion: func(t *testing.T, os job.Job, err error) { @@ -226,7 +221,6 @@ chainID = 1337 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] `, assertion: func(t *testing.T, os job.Job, err error) { @@ -253,7 +247,6 @@ chainID = 1337 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] `, assertion: func(t *testing.T, os job.Job, err error) { @@ -279,7 +272,6 @@ chainID = 1337 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] `, assertion: func(t *testing.T, os job.Job, err error) { @@ -303,7 +295,6 @@ chainID = 1337 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] `, assertion: func(t *testing.T, os job.Job, err error) { @@ -344,7 +335,6 @@ answer1 [type=median index=0]; strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] juelsPerFeeCoinSource = """ ds1 [type=bridge name=voter_turnout]; @@ -383,7 +373,6 @@ answer1 [type=median index=0]; strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] juelsPerFeeCoinSource = """ -> @@ -415,7 +404,6 @@ answer1 [type=median index=0]; strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] juelsPerFeeCoinSource = """ ds1 [type=bridge name=voter_turnout]; @@ -429,46 +417,6 @@ chainID = 1337 require.Contains(t, err.Error(), "no such relay blerg supported") }, }, - { - name: "Generic public onchain signing strategy with no public key", - toml: ` -type = "offchainreporting2" -pluginType = "plugin" -schemaVersion = 1 -relay = "evm" -contractID = "0x613a38AC1659769640aaE063C651F48E0250454C" -p2pPeerID = "12D3KooWHfYFQ8hGttAYbMCevQVESEQhzJAqFZokMVtom8bNxwGq" -p2pv2Bootstrappers = [ -"12D3KooWHfYFQ8hGttAYbMCevQVESEQhzJAqFZokMVtom8bNxwGq@127.0.0.1:5001", -] -ocrKeyBundleID = "73e8966a78ca09bb912e9565cfb79fbe8a6048fab1f0cf49b18047c3895e0447" -monitoringEndpoint = "chain.link:4321" -transmitterID = "0xF67D0290337bca0847005C7ffD1BC75BA9AAE6e4" -observationTimeout = "10s" -observationSource = """ -ds1 [type=bridge name=voter_turnout]; -ds1_parse [type=jsonparse path="one,two"]; -ds1_multiply [type=multiply times=1.23]; -ds1 -> ds1_parse -> ds1_multiply -> answer1; -answer1 [type=median index=0]; -""" -[relayConfig] -chainID = 1337 -[onchainSigningStrategy] -strategyName = "single-chain" -[onchainSigningStrategy.config] -evm = "" -publicKey = "" -[pluginConfig] -pluginName = "median" -telemetryType = "median" -OCRVersion=2 -`, - assertion: func(t *testing.T, os job.Job, err error) { - require.Error(t, err) - require.Contains(t, err.Error(), "must provide public key for the onchain signing strategy") - }, - }, { name: "Generic plugin config validation - nothing provided", toml: ` @@ -493,7 +441,6 @@ chainID = 4 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] `, @@ -525,7 +472,6 @@ chainID = 4 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] PluginName="some random name" @@ -559,7 +505,6 @@ chainID = 4 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] PluginName="some random name" @@ -594,7 +539,6 @@ chainID = 4 strategyName = "single-chain" [onchainSigningStrategy.config] evm = "" -publicKey = "0x1234567890123456789012345678901234567890" [pluginConfig] PluginName="some random name" @@ -712,7 +656,6 @@ func TestOCR2OnchainSigningStrategy_Unmarshal(t *testing.T) { strategyName = "single-chain" [onchainSigningStrategy.config] evm = "08d14c6eed757414d72055d28de6caf06535806c6a14e450f3a2f1c854420e17" -publicKey = "0x1234567890123456789012345678901234567890" ` oss := &envelope2{} tree, err := toml.Load(payload) @@ -725,12 +668,9 @@ publicKey = "0x1234567890123456789012345678901234567890" err = json.Unmarshal(b, oss) require.NoError(t, err) - pk, err := oss.OnchainSigningStrategy.PublicKey() - require.NoError(t, err) kbID, err := oss.OnchainSigningStrategy.KeyBundleID("evm") require.NoError(t, err) assert.False(t, oss.OnchainSigningStrategy.IsMultiChain()) - assert.Equal(t, "0x1234567890123456789012345678901234567890", pk) assert.Equal(t, "08d14c6eed757414d72055d28de6caf06535806c6a14e450f3a2f1c854420e17", kbID) } diff --git a/core/services/ocrcommon/adapters.go b/core/services/ocrcommon/adapters.go index 372d9e37f15..53e62be9a07 100644 --- a/core/services/ocrcommon/adapters.go +++ b/core/services/ocrcommon/adapters.go @@ -87,12 +87,11 @@ type OCR3OnchainKeyringMultiChainAdapter struct { lggr logger.Logger } -func NewOCR3OnchainKeyringMultiChainAdapter(ost map[string]ocr2key.KeyBundle, lggr logger.Logger) (*OCR3OnchainKeyringMultiChainAdapter, error) { +func NewOCR3OnchainKeyringMultiChainAdapter(ost map[string]ocr2key.KeyBundle, publicKey ocrtypes.OnchainPublicKey, lggr logger.Logger) (*OCR3OnchainKeyringMultiChainAdapter, error) { if len(ost) == 0 { return nil, errors.New("no key bundles provided") } - // We don't need to check for the existence of `publicKey` in the keyBundles map because it is required on validation on `validate/validate.go` - return &OCR3OnchainKeyringMultiChainAdapter{ost, ost["publicKey"].PublicKey(), lggr}, nil + return &OCR3OnchainKeyringMultiChainAdapter{ost, publicKey, lggr}, nil } func (a *OCR3OnchainKeyringMultiChainAdapter) PublicKey() ocrtypes.OnchainPublicKey { diff --git a/core/services/ocrcommon/adapters_test.go b/core/services/ocrcommon/adapters_test.go index fed854b0b32..e7d45627299 100644 --- a/core/services/ocrcommon/adapters_test.go +++ b/core/services/ocrcommon/adapters_test.go @@ -162,9 +162,9 @@ publicKey = "pub-key" keyBundles[name] = os } - adapter, err := ocrcommon.NewOCR3OnchainKeyringMultiChainAdapter(keyBundles, logger.TestLogger(t)) + adapter, err := ocrcommon.NewOCR3OnchainKeyringMultiChainAdapter(keyBundles, pk, logger.TestLogger(t)) require.NoError(t, err) - _, err = ocrcommon.NewOCR3OnchainKeyringMultiChainAdapter(map[string]ocr2key.KeyBundle{}, logger.TestLogger(t)) + _, err = ocrcommon.NewOCR3OnchainKeyringMultiChainAdapter(map[string]ocr2key.KeyBundle{}, pk, logger.TestLogger(t)) require.Error(t, err, "no key bundles provided") sig, err := adapter.Sign(configDigest, seqNr, reportInfo)