From 03e20aa0b2a7d368b3d52bb25c10f60835e0d511 Mon Sep 17 00:00:00 2001 From: george-dorin Date: Tue, 12 Nov 2024 16:36:53 +0200 Subject: [PATCH 01/11] -Remove adaptive send -Add dual transmitter relay config -Add dual transmitter as contract transmitter --- core/services/job/job_orm_test.go | 60 ++++++++++++++---- core/services/job/models.go | 24 -------- core/services/job/models_test.go | 61 ------------------- core/services/job/orm.go | 28 +++++++-- core/services/ocr2/validate/validate.go | 10 --- core/services/ocrcommon/transmitter.go | 50 ++++++++++++++- .../relay/evm/contract_transmitter.go | 27 ++++++++ core/services/relay/evm/evm.go | 1 + core/services/relay/evm/types/types.go | 10 +++ core/testdata/testspecs/v2_specs.go | 30 +++------ 10 files changed, 165 insertions(+), 136 deletions(-) diff --git a/core/services/job/job_orm_test.go b/core/services/job/job_orm_test.go index bc6ec0d6ea2..6009dd2452b 100644 --- a/core/services/job/job_orm_test.go +++ b/core/services/job/job_orm_test.go @@ -2014,7 +2014,7 @@ func mustInsertPipelineRun(t *testing.T, orm pipeline.ORM, j job.Job) pipeline.R return run } -func TestORM_CreateJob_OCR2_With_AdaptiveSend(t *testing.T) { +func TestORM_CreateJob_OCR2_With_DualTransmission(t *testing.T) { ctx := testutils.Context(t) customChainID := big.New(testutils.NewRandomEVMChainID()) @@ -2030,27 +2030,63 @@ func TestORM_CreateJob_OCR2_With_AdaptiveSend(t *testing.T) { db := pgtest.NewSqlxDB(t) keyStore := cltest.NewKeyStore(t, db) require.NoError(t, keyStore.OCR2().Add(ctx, cltest.DefaultOCR2Key)) - _, transmitterID := cltest.MustInsertRandomKey(t, keyStore.Eth()) + baseJobSpec := fmt.Sprintf(testspecs.OCR2EVMDualTransmissionSpecMinimalTemplate, transmitterID.String()) + lggr := logger.TestLogger(t) pipelineORM := pipeline.NewORM(db, lggr, config.JobPipeline().MaxSuccessfulRuns()) bridgesORM := bridges.NewORM(db) jobORM := NewTestORM(t, db, pipelineORM, bridgesORM, keyStore) - adaptiveSendKey := cltest.MustGenerateRandomKey(t) + //Enabled but no config set + enabledDualTransmissionSpec := ` + enableDualTransmission=true` - jb, err := ocr2validate.ValidatedOracleSpecToml(testutils.Context(t), config.OCR2(), config.Insecure(), testspecs.GetOCR2EVMWithAdaptiveSendSpecMinimal(cltest.DefaultOCR2Key.ID(), transmitterID.String(), adaptiveSendKey.EIP55Address.String()), nil) + jb, err := ocr2validate.ValidatedOracleSpecToml(testutils.Context(t), config.OCR2(), config.Insecure(), baseJobSpec+enabledDualTransmissionSpec, nil) + require.NoError(t, err) + require.ErrorContains(t, jobORM.CreateJob(ctx, &jb), "dual transmission is enabled but no dual transmission config present") + + //contractAddress not set + emptyContractAddress := ` + enableDualTransmission=true + [relayConfig.dualTransmission] + contractAddress="" + ` + jb, err = ocr2validate.ValidatedOracleSpecToml(testutils.Context(t), config.OCR2(), config.Insecure(), baseJobSpec+emptyContractAddress, nil) + require.NoError(t, err) + require.ErrorContains(t, jobORM.CreateJob(ctx, &jb), "invalid contract address in dual transmission config") + + //Transmitter address not set + emptyTransmitterAddress := ` + enableDualTransmission=true + [relayConfig.dualTransmission] + contractAddress = '0x613a38AC1659769640aaE063C651F48E0250454C' + transmitterAddress = '' + ` + jb, err = ocr2validate.ValidatedOracleSpecToml(testutils.Context(t), config.OCR2(), config.Insecure(), baseJobSpec+emptyTransmitterAddress, nil) + require.NoError(t, err) + require.ErrorContains(t, jobORM.CreateJob(ctx, &jb), "invalid transmitter address in dual transmission config") + + dtTransmitterAddress := cltest.MustGenerateRandomKey(t) + completeDualTransmissionSpec := fmt.Sprintf(` + enableDualTransmission=true + [relayConfig.dualTransmission] + contractAddress = '0x613a38AC1659769640aaE063C651F48E0250454C' + transmitterAddress = '%s' + `, + dtTransmitterAddress.Address.String()) + + jb, err = ocr2validate.ValidatedOracleSpecToml(testutils.Context(t), config.OCR2(), config.Insecure(), baseJobSpec+completeDualTransmissionSpec, nil) require.NoError(t, err) - require.Equal(t, "arbitrary-value", jb.AdaptiveSendSpec.Metadata["arbitraryParam"]) - t.Run("unknown transmitter address", func(t *testing.T) { - require.ErrorContains(t, jobORM.CreateJob(ctx, &jb), "failed to validate AdaptiveSendSpec.TransmitterAddress: no EVM key matching") - }) + jb.OCR2OracleSpec.TransmitterID = null.StringFrom(transmitterID.String()) - t.Run("multiple jobs", func(t *testing.T) { - keyStore.Eth().XXXTestingOnlyAdd(ctx, adaptiveSendKey) - require.NoError(t, jobORM.CreateJob(ctx, &jb), "failed to validate AdaptiveSendSpec.TransmitterAddress: no EVM key matching") - }) + //unknown transmitter address + require.ErrorContains(t, jobORM.CreateJob(ctx, &jb), "unknown dual transmission transmitterAddress: no EVM key matching:") + + //Should not error + keyStore.Eth().XXXTestingOnlyAdd(ctx, dtTransmitterAddress) + require.NoError(t, jobORM.CreateJob(ctx, &jb)) } diff --git a/core/services/job/models.go b/core/services/job/models.go index 5054abd08b5..231bf10fda0 100644 --- a/core/services/job/models.go +++ b/core/services/job/models.go @@ -185,7 +185,6 @@ type Job struct { CCIPSpecID *int32 CCIPSpec *CCIPSpec CCIPBootstrapSpecID *int32 - AdaptiveSendSpec *AdaptiveSendSpec `toml:"adaptiveSend"` JobSpecErrors []SpecError Type Type `toml:"type"` SchemaVersion uint32 `toml:"schemaVersion"` @@ -1061,26 +1060,3 @@ type CCIPSpec struct { // and RMN network info for offchain blessing. PluginConfig JSONConfig `toml:"pluginConfig"` } - -type AdaptiveSendSpec struct { - TransmitterAddress *evmtypes.EIP55Address `toml:"transmitterAddress"` - ContractAddress *evmtypes.EIP55Address `toml:"contractAddress"` - Delay time.Duration `toml:"delay"` - Metadata JSONConfig `toml:"metadata"` -} - -func (o *AdaptiveSendSpec) Validate() error { - if o.TransmitterAddress == nil { - return errors.New("no AdaptiveSendSpec.TransmitterAddress found") - } - - if o.ContractAddress == nil { - return errors.New("no AdaptiveSendSpec.ContractAddress found") - } - - if o.Delay.Seconds() <= 1 { - return errors.New("AdaptiveSendSpec.Delay not set or smaller than 1s") - } - - return nil -} diff --git a/core/services/job/models_test.go b/core/services/job/models_test.go index a21a4553219..5ef36ea9f48 100644 --- a/core/services/job/models_test.go +++ b/core/services/job/models_test.go @@ -11,8 +11,6 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/codec" "github.com/smartcontractkit/chainlink-common/pkg/types" pkgworkflows "github.com/smartcontractkit/chainlink-common/pkg/workflows" - "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" - "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" "github.com/smartcontractkit/chainlink/v2/core/services/job" "github.com/smartcontractkit/chainlink/v2/core/services/relay" @@ -352,62 +350,3 @@ func TestWorkflowSpec_Validate(t *testing.T) { require.NotEmpty(t, w.WorkflowID) }) } - -func TestAdaptiveSendConfig(t *testing.T) { - tests := []struct { - name string - shouldError bool - expectedErrorMessage string - config job.AdaptiveSendSpec - }{ - { - name: "AdaptiveSendSpec.TransmitterAddress not set", - shouldError: true, - expectedErrorMessage: "no AdaptiveSendSpec.TransmitterAddress found", - config: job.AdaptiveSendSpec{ - TransmitterAddress: nil, - ContractAddress: ptr(cltest.NewEIP55Address()), - Delay: time.Second * 30, - }, - }, - { - name: "AdaptiveSendSpec.ContractAddress not set", - shouldError: true, - expectedErrorMessage: "no AdaptiveSendSpec.ContractAddress found", - config: job.AdaptiveSendSpec{ - TransmitterAddress: ptr(cltest.NewEIP55Address()), - ContractAddress: nil, - Delay: time.Second * 30, - }, - }, - { - name: "AdaptiveSendSpec.Delay not set", - shouldError: true, - expectedErrorMessage: "AdaptiveSendSpec.Delay not set or smaller than 1s", - config: job.AdaptiveSendSpec{ - TransmitterAddress: ptr(cltest.NewEIP55Address()), - ContractAddress: ptr(cltest.NewEIP55Address()), - }, - }, - { - name: "AdaptiveSendSpec.Delay set to 50ms", - shouldError: true, - expectedErrorMessage: "AdaptiveSendSpec.Delay not set or smaller than 1s", - config: job.AdaptiveSendSpec{ - TransmitterAddress: ptr(cltest.NewEIP55Address()), - ContractAddress: ptr(cltest.NewEIP55Address()), - Delay: time.Millisecond * 50, - }, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - if test.shouldError { - require.ErrorContains(t, test.config.Validate(), test.expectedErrorMessage) - } else { - require.NoError(t, test.config.Validate()) - } - }) - } -} diff --git a/core/services/job/orm.go b/core/services/job/orm.go index a86da5f7111..7d3178de6b8 100644 --- a/core/services/job/orm.go +++ b/core/services/job/orm.go @@ -20,7 +20,6 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/sqlutil" "github.com/smartcontractkit/chainlink-common/pkg/types" - "github.com/smartcontractkit/chainlink/v2/core/bridges" evmconfig "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" @@ -304,10 +303,29 @@ func (o *orm) CreateJob(ctx context.Context, jb *Job) error { } } - if jb.AdaptiveSendSpec != nil { - err = validateKeyStoreMatchForRelay(ctx, jb.OCR2OracleSpec.Relay, tx.keyStore, jb.AdaptiveSendSpec.TransmitterAddress.String()) - if err != nil { - return fmt.Errorf("failed to validate AdaptiveSendSpec.TransmitterAddress: %w", err) + if enableDualTransmission, ok := jb.OCR2OracleSpec.RelayConfig["enableDualTransmission"]; ok && enableDualTransmission != nil { + rawDualTransmissionConfig, ok := jb.OCR2OracleSpec.RelayConfig["dualTransmission"] + if !ok { + return errors.New("dual transmission is enabled but no dual transmission config present") + } + + dualTransmissionConfig, ok := rawDualTransmissionConfig.(map[string]interface{}) + if !ok { + return errors.New("invalid dual transmission config") + } + + dtContractAddress, ok := dualTransmissionConfig["contractAddress"].(string) + if !ok || !common.IsHexAddress(dtContractAddress) { + return errors.New("invalid contract address in dual transmission config") + } + + dtTransmitterAddress, ok := dualTransmissionConfig["transmitterAddress"].(string) + if !ok || !common.IsHexAddress(dtTransmitterAddress) { + return errors.New("invalid transmitter address in dual transmission config") + } + + if err := validateKeyStoreMatchForRelay(ctx, jb.OCR2OracleSpec.Relay, tx.keyStore, dtTransmitterAddress); err != nil { + return errors.Wrap(err, "unknown dual transmission transmitterAddress") } } diff --git a/core/services/ocr2/validate/validate.go b/core/services/ocr2/validate/validate.go index 40e2c11c3e7..c2f3e455232 100644 --- a/core/services/ocr2/validate/validate.go +++ b/core/services/ocr2/validate/validate.go @@ -73,9 +73,6 @@ func ValidatedOracleSpecToml(ctx context.Context, config OCR2Config, insConf Ins if err = validateTimingParameters(config, insConf, spec); err != nil { return jb, err } - if err = validateAdaptiveSendSpec(ctx, jb); err != nil { - return jb, err - } return jb, nil } @@ -380,10 +377,3 @@ func validateOCR2LLOSpec(jsonConfig job.JSONConfig) error { } return pkgerrors.Wrap(pluginConfig.Validate(), "LLO PluginConfig is invalid") } - -func validateAdaptiveSendSpec(ctx context.Context, spec job.Job) error { - if spec.AdaptiveSendSpec != nil { - return spec.AdaptiveSendSpec.Validate() - } - return nil -} diff --git a/core/services/ocrcommon/transmitter.go b/core/services/ocrcommon/transmitter.go index 5d2de45295f..dfe7977f480 100644 --- a/core/services/ocrcommon/transmitter.go +++ b/core/services/ocrcommon/transmitter.go @@ -11,6 +11,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/forwarders" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" + types2 "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/types" ) type roundRobinKeystore interface { @@ -88,13 +89,14 @@ func NewOCR2FeedsTransmitter( checker txmgr.TransmitCheckerSpec, chainID *big.Int, keystore roundRobinKeystore, + dualTransmissionConfig *types2.DualTransmissionConfig, ) (Transmitter, error) { // Ensure that a keystore is provided. if keystore == nil { return nil, errors.New("nil keystore provided to transmitter") } - return &ocr2FeedsTransmitter{ + baseTransmitter := &ocr2FeedsTransmitter{ ocr2Aggregator: ocr2Aggregator, txManagerOCR2: txm, transmitter: transmitter{ @@ -107,7 +109,17 @@ func NewOCR2FeedsTransmitter( chainID: chainID, keystore: keystore, }, - }, nil + } + + if dualTransmissionConfig != nil { + return &ocr2FeedsDualTransmission{ + primaryTransmitter: *baseTransmitter, + secondaryContractAddress: dualTransmissionConfig.ContractAddress, + secondaryFromAddress: dualTransmissionConfig.TransmitterAddress, + secondaryMeta: dualTransmissionConfig.Meta, + }, nil + } + return baseTransmitter, nil } func (t *transmitter) CreateEthTransaction(ctx context.Context, toAddress common.Address, payload []byte, txMeta *txmgr.TxMeta) error { @@ -203,3 +215,37 @@ func (t *ocr2FeedsTransmitter) forwarderAddress(ctx context.Context, eoa, ocr2Ag return forwarderAddress, nil } + +type ocr2FeedsDualTransmission struct { + primaryTransmitter ocr2FeedsTransmitter + + secondaryContractAddress common.Address + secondaryFromAddress common.Address + secondaryMeta map[string]interface{} +} + +func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, toAddress common.Address, payload []byte, txMeta *txmgr.TxMeta) error { + //Primary transmission + err := t.primaryTransmitter.CreateEthTransaction(ctx, toAddress, payload, txMeta) + if err != nil { + return err + } + + //Secondary transmission + _, err = t.primaryTransmitter.txm.CreateTransaction(ctx, txmgr.TxRequest{ + FromAddress: t.secondaryFromAddress, + ToAddress: t.secondaryContractAddress, + EncodedPayload: payload, + FeeLimit: t.primaryTransmitter.gasLimit, + //ForwarderAddress: forwarderAddress, TODO @george-dorin: should be sent? + Strategy: t.primaryTransmitter.strategy, + Checker: t.primaryTransmitter.checker, + //Meta: txMeta, TODO @george-dorin: add dual transmission params + }) + + return errors.Wrap(err, "skipped secondary transmission") +} + +func (t *ocr2FeedsDualTransmission) FromAddress(ctx context.Context) common.Address { + return t.primaryTransmitter.FromAddress(ctx) +} diff --git a/core/services/relay/evm/contract_transmitter.go b/core/services/relay/evm/contract_transmitter.go index aead9f6ca8a..28a223d32b4 100644 --- a/core/services/relay/evm/contract_transmitter.go +++ b/core/services/relay/evm/contract_transmitter.go @@ -12,6 +12,7 @@ import ( "github.com/ethereum/go-ethereum/common" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/pkg/errors" + types2 "github.com/smartcontractkit/libocr/offchainreporting2/types" "github.com/smartcontractkit/libocr/offchainreporting2plus/chains/evmutil" ocrtypes "github.com/smartcontractkit/libocr/offchainreporting2plus/types" @@ -246,3 +247,29 @@ func (oc *contractTransmitter) HealthReport() map[string]error { return map[string]error{oc.Name(): nil} } func (oc *contractTransmitter) Name() string { return oc.lggr.Name() } + +var _ types2.ContractTransmitter = (*dualContractTransmitter)(nil) + +type dualContractTransmitter struct { + baseContractTransmitter contractTransmitter +} + +func (d *dualContractTransmitter) secondaryTransmit(ctx context.Context, reportContext types2.ReportContext, report types2.Report, signatures []types2.AttributedOnchainSignature) error { + return nil +} + +func (d *dualContractTransmitter) Transmit(ctx context.Context, reportContext types2.ReportContext, report types2.Report, signatures []types2.AttributedOnchainSignature) error { + err := d.secondaryTransmit(ctx, reportContext, report, signatures) + if err != nil { + d.baseContractTransmitter.lggr.Warnw("secondary transmission failed", "err", err) + } + return d.baseContractTransmitter.Transmit(ctx, reportContext, report, signatures) +} + +func (d *dualContractTransmitter) LatestConfigDigestAndEpoch(ctx context.Context) (configDigest types2.ConfigDigest, epoch uint32, err error) { + return d.baseContractTransmitter.LatestConfigDigestAndEpoch(ctx) +} + +func (d *dualContractTransmitter) FromAccount(ctx context.Context) (types2.Account, error) { + return d.baseContractTransmitter.FromAccount(ctx) +} diff --git a/core/services/relay/evm/evm.go b/core/services/relay/evm/evm.go index 8e63a26a9d7..d657be71733 100644 --- a/core/services/relay/evm/evm.go +++ b/core/services/relay/evm/evm.go @@ -802,6 +802,7 @@ func generateTransmitterFrom(ctx context.Context, rargs commontypes.RelayArgs, e checker, configWatcher.chain.ID(), ethKeystore, + &relayConfig.DualTransmissionConfig, ) case commontypes.CCIPExecution: transmitter, err = cciptransmitter.NewTransmitterWithStatusChecker( diff --git a/core/services/relay/evm/types/types.go b/core/services/relay/evm/types/types.go index a96dd9f9508..73c8fc7dcce 100644 --- a/core/services/relay/evm/types/types.go +++ b/core/services/relay/evm/types/types.go @@ -192,6 +192,12 @@ func (c LLOConfigMode) String() string { return string(c) } +type DualTransmissionConfig struct { + ContractAddress common.Address `json:"contractAddress" toml:"contractAddress"` + TransmitterAddress common.Address `json:"transmitterAddress" toml:"transmitterAddress"` + Meta map[string]interface{} `json:"meta" toml:"meta"` +} + type RelayConfig struct { ChainID *big.Big `json:"chainID"` FromBlock uint64 `json:"fromBlock"` @@ -213,6 +219,10 @@ type RelayConfig struct { // LLO-specific LLODONID uint32 `json:"lloDonID" toml:"lloDonID"` LLOConfigMode LLOConfigMode `json:"lloConfigMode" toml:"lloConfigMode"` + + //DualTransmission specific + EnableDualTransmission bool `json:"enableDualTransmission" toml:"enableDualTransmission"` + DualTransmissionConfig DualTransmissionConfig `json:"dualTransmission" toml:"dualTransmission"` } var ErrBadRelayConfig = errors.New("bad relay config") diff --git a/core/testdata/testspecs/v2_specs.go b/core/testdata/testspecs/v2_specs.go index d3d72467a83..d519ace6479 100644 --- a/core/testdata/testspecs/v2_specs.go +++ b/core/testdata/testspecs/v2_specs.go @@ -951,42 +951,28 @@ targets: inputs: consensus_output: $(a-consensus.outputs) ` -var OCR2EVMSpecMinimalWithAdaptiveSendTemplate = ` +var OCR2EVMDualTransmissionSpecMinimalTemplate = ` type = "offchainreporting2" schemaVersion = 1 -name = "%s" +name = "test-job" +relay = "evm" contractID = "0x613a38AC1659769640aaE063C651F48E0250454C" p2pv2Bootstrappers = [] -ocrKeyBundleID = "%s" -relay = "evm" -pluginType = "median" transmitterID = "%s" - +pluginType = "median" observationSource = """ ds [type=http method=GET url="https://chain.link/ETH-USD"]; ds_parse [type=jsonparse path="data.price" separator="."]; ds_multiply [type=multiply times=100]; ds -> ds_parse -> ds_multiply; """ - [pluginConfig] juelsPerFeeCoinSource = """ - ds1 [type=http method=GET url="https://chain.link/jules" allowunrestrictednetworkaccess="true"]; - ds1_parse [type=jsonparse path="answer"]; - ds1_multiply [type=multiply times=1]; - ds1 -> ds1_parse -> ds1_multiply; + ds [type=http method=GET url="https://chain.link/ETH-USD"]; + ds_parse [type=jsonparse path="data.price" separator="."]; + ds_multiply [type=multiply times=100]; + ds -> ds_parse -> ds_multiply; """ [relayConfig] chainID = 0 - -[adaptiveSend] -transmitterAddress = '%s' -contractAddress = '0xF67D0290337bca0847005C7ffD1BC75BA9AAE6e4' -delay = '30s' -[adaptiveSend.metadata] -arbitraryParam = 'arbitrary-value' ` - -func GetOCR2EVMWithAdaptiveSendSpecMinimal(keyBundle, transmitterID, secondaryTransmitterAddress string) string { - return fmt.Sprintf(OCR2EVMSpecMinimalWithAdaptiveSendTemplate, uuid.New(), keyBundle, transmitterID, secondaryTransmitterAddress) -} From 801ff0cff295fc04fef55c05084ca2b5541252d1 Mon Sep 17 00:00:00 2001 From: george-dorin Date: Tue, 12 Nov 2024 18:01:44 +0200 Subject: [PATCH 02/11] -Fix lint --- common/txmgr/types/tx.go | 4 ++++ core/services/job/orm.go | 2 +- core/services/ocrcommon/transmitter.go | 9 ++++---- core/services/relay/evm/types/types.go | 2 +- .../actions/ocr2_helpers_local.go | 23 +++++++++++++++---- 5 files changed, 29 insertions(+), 11 deletions(-) diff --git a/common/txmgr/types/tx.go b/common/txmgr/types/tx.go index b65f7edf6e5..ea3fc55330c 100644 --- a/common/txmgr/types/tx.go +++ b/common/txmgr/types/tx.go @@ -159,6 +159,10 @@ type TxMeta[ADDR types.Hashable, TX_HASH types.Hashable] struct { MessageIDs []string `json:"MessageIDs,omitempty"` // SeqNumbers is used by CCIP for tx to committed sequence numbers correlation in logs SeqNumbers []uint64 `json:"SeqNumbers,omitempty"` + + // Dual Broadcast + DualBroadcast bool `json:"DualBroadcast,omitempty"` + DualBroadcastParams string `json:"DualBroadcastParams,omitempty"` } type TxAttempt[ diff --git a/core/services/job/orm.go b/core/services/job/orm.go index 7d3178de6b8..63fa2cb3b0d 100644 --- a/core/services/job/orm.go +++ b/core/services/job/orm.go @@ -324,7 +324,7 @@ func (o *orm) CreateJob(ctx context.Context, jb *Job) error { return errors.New("invalid transmitter address in dual transmission config") } - if err := validateKeyStoreMatchForRelay(ctx, jb.OCR2OracleSpec.Relay, tx.keyStore, dtTransmitterAddress); err != nil { + if err = validateKeyStoreMatchForRelay(ctx, jb.OCR2OracleSpec.Relay, tx.keyStore, dtTransmitterAddress); err != nil { return errors.Wrap(err, "unknown dual transmission transmitterAddress") } } diff --git a/core/services/ocrcommon/transmitter.go b/core/services/ocrcommon/transmitter.go index dfe7977f480..ed91cc3f097 100644 --- a/core/services/ocrcommon/transmitter.go +++ b/core/services/ocrcommon/transmitter.go @@ -230,6 +230,8 @@ func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, to if err != nil { return err } + txMeta.DualBroadcast = true + txMeta.DualBroadcastParams = "" //Secondary transmission _, err = t.primaryTransmitter.txm.CreateTransaction(ctx, txmgr.TxRequest{ @@ -237,10 +239,9 @@ func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, to ToAddress: t.secondaryContractAddress, EncodedPayload: payload, FeeLimit: t.primaryTransmitter.gasLimit, - //ForwarderAddress: forwarderAddress, TODO @george-dorin: should be sent? - Strategy: t.primaryTransmitter.strategy, - Checker: t.primaryTransmitter.checker, - //Meta: txMeta, TODO @george-dorin: add dual transmission params + Strategy: t.primaryTransmitter.strategy, + Checker: t.primaryTransmitter.checker, + Meta: txMeta, }) return errors.Wrap(err, "skipped secondary transmission") diff --git a/core/services/relay/evm/types/types.go b/core/services/relay/evm/types/types.go index 73c8fc7dcce..421bf34291e 100644 --- a/core/services/relay/evm/types/types.go +++ b/core/services/relay/evm/types/types.go @@ -220,7 +220,7 @@ type RelayConfig struct { LLODONID uint32 `json:"lloDonID" toml:"lloDonID"` LLOConfigMode LLOConfigMode `json:"lloConfigMode" toml:"lloConfigMode"` - //DualTransmission specific + // DualTransmission specific EnableDualTransmission bool `json:"enableDualTransmission" toml:"enableDualTransmission"` DualTransmissionConfig DualTransmissionConfig `json:"dualTransmission" toml:"dualTransmission"` } diff --git a/integration-tests/actions/ocr2_helpers_local.go b/integration-tests/actions/ocr2_helpers_local.go index 2330003717c..2054fda0ad4 100644 --- a/integration-tests/actions/ocr2_helpers_local.go +++ b/integration-tests/actions/ocr2_helpers_local.go @@ -41,6 +41,7 @@ func CreateOCRv2JobsLocal( chainId uint64, // EVM chain ID forwardingAllowed bool, enableChainReaderAndCodec bool, + enableDualTransmission bool, ) error { // Collect P2P ID bootstrapP2PIds, err := bootstrapNode.MustReadP2PKeys() @@ -106,6 +107,20 @@ func CreateOCRv2JobsLocal( return fmt.Errorf("creating bridge on CL node failed: %w", err) } + relayConfig := map[string]interface{}{ + "chainID": chainId, + } + + chainlinkNode.PrimaryEthAddress() + if enableDualTransmission { + relayConfig["enableDualTransmission"] = true + relayConfig["dualTransmission"] = evmtypes.DualTransmissionConfig{ + ContractAddress: common.Address{}, + TransmitterAddress: null.StringFrom(nodeTransmitterAddress), + Meta: nil, + } + } + ocrSpec := &nodeclient.OCR2TaskJobSpec{ Name: fmt.Sprintf("ocr2-%s", uuid.NewString()), JobType: "offchainreporting2", @@ -113,11 +128,9 @@ func CreateOCRv2JobsLocal( ObservationSource: nodeclient.ObservationSourceSpecBridge(bta), ForwardingAllowed: forwardingAllowed, OCR2OracleSpec: job.OCR2OracleSpec{ - PluginType: "median", - Relay: "evm", - RelayConfig: map[string]interface{}{ - "chainID": chainId, - }, + PluginType: "median", + Relay: "evm", + RelayConfig: relayConfig, PluginConfig: map[string]any{ "juelsPerFeeCoinSource": fmt.Sprintf("\"\"\"%s\"\"\"", nodeclient.ObservationSourceSpecBridge(juelsBridge)), }, From ddd95439dc95edf81adc1b2ee1262aec0d7540f9 Mon Sep 17 00:00:00 2001 From: george-dorin Date: Wed, 13 Nov 2024 15:26:26 +0200 Subject: [PATCH 03/11] Add tx.Meta --- core/services/ocrcommon/transmitter.go | 36 +++++++--- core/services/ocrcommon/transmitter_test.go | 76 +++++++++++++++++++++ core/services/relay/evm/types/types.go | 6 +- 3 files changed, 105 insertions(+), 13 deletions(-) diff --git a/core/services/ocrcommon/transmitter.go b/core/services/ocrcommon/transmitter.go index ed91cc3f097..cdbc6444776 100644 --- a/core/services/ocrcommon/transmitter.go +++ b/core/services/ocrcommon/transmitter.go @@ -3,6 +3,7 @@ package ocrcommon import ( "context" "math/big" + "net/url" "slices" "github.com/ethereum/go-ethereum/common" @@ -113,7 +114,7 @@ func NewOCR2FeedsTransmitter( if dualTransmissionConfig != nil { return &ocr2FeedsDualTransmission{ - primaryTransmitter: *baseTransmitter, + transmitter: *baseTransmitter, secondaryContractAddress: dualTransmissionConfig.ContractAddress, secondaryFromAddress: dualTransmissionConfig.TransmitterAddress, secondaryMeta: dualTransmissionConfig.Meta, @@ -217,30 +218,35 @@ func (t *ocr2FeedsTransmitter) forwarderAddress(ctx context.Context, eoa, ocr2Ag } type ocr2FeedsDualTransmission struct { - primaryTransmitter ocr2FeedsTransmitter + transmitter ocr2FeedsTransmitter secondaryContractAddress common.Address secondaryFromAddress common.Address - secondaryMeta map[string]interface{} + secondaryMeta map[string][]string } func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, toAddress common.Address, payload []byte, txMeta *txmgr.TxMeta) error { //Primary transmission - err := t.primaryTransmitter.CreateEthTransaction(ctx, toAddress, payload, txMeta) + err := t.transmitter.CreateEthTransaction(ctx, toAddress, payload, txMeta) if err != nil { return err } + + if txMeta == nil { + txMeta = &txmgr.TxMeta{} + } + txMeta.DualBroadcast = true - txMeta.DualBroadcastParams = "" + txMeta.DualBroadcastParams = t.urlParams() //Secondary transmission - _, err = t.primaryTransmitter.txm.CreateTransaction(ctx, txmgr.TxRequest{ + _, err = t.transmitter.txm.CreateTransaction(ctx, txmgr.TxRequest{ FromAddress: t.secondaryFromAddress, ToAddress: t.secondaryContractAddress, EncodedPayload: payload, - FeeLimit: t.primaryTransmitter.gasLimit, - Strategy: t.primaryTransmitter.strategy, - Checker: t.primaryTransmitter.checker, + FeeLimit: t.transmitter.gasLimit, + Strategy: t.transmitter.strategy, + Checker: t.transmitter.checker, Meta: txMeta, }) @@ -248,5 +254,15 @@ func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, to } func (t *ocr2FeedsDualTransmission) FromAddress(ctx context.Context) common.Address { - return t.primaryTransmitter.FromAddress(ctx) + return t.transmitter.FromAddress(ctx) +} + +func (t *ocr2FeedsDualTransmission) urlParams() string { + values := url.Values{} + for k, v := range t.secondaryMeta { + for _, p := range v { + values.Add(k, p) + } + } + return values.Encode() } diff --git a/core/services/ocrcommon/transmitter_test.go b/core/services/ocrcommon/transmitter_test.go index d6a07190800..80f053b26d1 100644 --- a/core/services/ocrcommon/transmitter_test.go +++ b/core/services/ocrcommon/transmitter_test.go @@ -5,16 +5,19 @@ import ( "testing" "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" commontxmmocks "github.com/smartcontractkit/chainlink/v2/common/txmgr/types/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" txmmocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr/mocks" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/utils" "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" "github.com/smartcontractkit/chainlink/v2/core/services/ocrcommon" + "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/types" ) func newMockTxStrategy(t *testing.T) *commontxmmocks.TxStrategy { @@ -169,3 +172,76 @@ func Test_DefaultTransmitter_Forwarding_Enabled_CreateEthTransaction_No_Keystore ) require.Error(t, err) } + +func Test_DualTransmitter(t *testing.T) { + t.Parallel() + + db := pgtest.NewSqlxDB(t) + ethKeyStore := cltest.NewKeyStore(t, db).Eth() + + _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) + _, secondaryFromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) + + contractAddress := utils.RandomAddress() + secondaryContractAddress := utils.RandomAddress() + + gasLimit := uint64(1000) + chainID := big.NewInt(0) + effectiveTransmitterAddress := fromAddress + toAddress := testutils.NewAddress() + payload := []byte{1, 2, 3} + txm := txmmocks.NewMockEvmTxManager(t) + strategy := newMockTxStrategy(t) + dualTransmissionConfig := &types.DualTransmissionConfig{ + ContractAddress: secondaryContractAddress, + TransmitterAddress: secondaryFromAddress, + Meta: map[string][]string{ + "key1": {"value1"}, + "key2": {"value2", "value3"}, + "key3": {"value4", "value5", "value6"}, + }, + } + + transmitter, err := ocrcommon.NewOCR2FeedsTransmitter( + txm, + []common.Address{fromAddress}, + contractAddress, + gasLimit, + effectiveTransmitterAddress, + strategy, + txmgr.TransmitCheckerSpec{}, + chainID, + ethKeyStore, + dualTransmissionConfig, + ) + require.NoError(t, err) + + primaryTxConfirmed := false + seconryTxConfirmed := false + + txm.On("CreateTransaction", mock.Anything, mock.MatchedBy(func(tx txmgr.TxRequest) bool { + if tx.FromAddress == fromAddress { + //Primary transmission + assert.Equal(t, tx.ToAddress, toAddress, "unexpected primary toAddress") + assert.Nil(t, tx.Meta, "Meta should be empty") + primaryTxConfirmed = true + } else if tx.FromAddress == secondaryFromAddress { + //Secondary transmission + assert.Equal(t, tx.ToAddress, secondaryContractAddress, "unexpected secondary toAddress") + assert.True(t, tx.Meta.DualBroadcast, "DualBroadcast should be true") + assert.Equal(t, tx.Meta.DualBroadcastParams, "key1=value1&key2=value2&key2=value3&key3=value4&key3=value5&key3=value6", "DualBroadcastParams not equal") + seconryTxConfirmed = true + + } else { + //Should never be reached + return false + } + + return true + })).Twice().Return(txmgr.Tx{}, nil) + + require.NoError(t, transmitter.CreateEthTransaction(testutils.Context(t), toAddress, payload, nil)) + + require.True(t, primaryTxConfirmed) + require.True(t, seconryTxConfirmed) +} diff --git a/core/services/relay/evm/types/types.go b/core/services/relay/evm/types/types.go index 421bf34291e..b63dcdc3adb 100644 --- a/core/services/relay/evm/types/types.go +++ b/core/services/relay/evm/types/types.go @@ -193,9 +193,9 @@ func (c LLOConfigMode) String() string { } type DualTransmissionConfig struct { - ContractAddress common.Address `json:"contractAddress" toml:"contractAddress"` - TransmitterAddress common.Address `json:"transmitterAddress" toml:"transmitterAddress"` - Meta map[string]interface{} `json:"meta" toml:"meta"` + ContractAddress common.Address `json:"contractAddress" toml:"contractAddress"` + TransmitterAddress common.Address `json:"transmitterAddress" toml:"transmitterAddress"` + Meta map[string][]string `json:"meta" toml:"meta"` } type RelayConfig struct { From c6693a84cdfbe550267b2f64382cbd4f2ec86fad Mon Sep 17 00:00:00 2001 From: george-dorin Date: Wed, 13 Nov 2024 15:29:04 +0200 Subject: [PATCH 04/11] Revert ocr2 helpers --- .../actions/ocr2_helpers_local.go | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/integration-tests/actions/ocr2_helpers_local.go b/integration-tests/actions/ocr2_helpers_local.go index 2054fda0ad4..2330003717c 100644 --- a/integration-tests/actions/ocr2_helpers_local.go +++ b/integration-tests/actions/ocr2_helpers_local.go @@ -41,7 +41,6 @@ func CreateOCRv2JobsLocal( chainId uint64, // EVM chain ID forwardingAllowed bool, enableChainReaderAndCodec bool, - enableDualTransmission bool, ) error { // Collect P2P ID bootstrapP2PIds, err := bootstrapNode.MustReadP2PKeys() @@ -107,20 +106,6 @@ func CreateOCRv2JobsLocal( return fmt.Errorf("creating bridge on CL node failed: %w", err) } - relayConfig := map[string]interface{}{ - "chainID": chainId, - } - - chainlinkNode.PrimaryEthAddress() - if enableDualTransmission { - relayConfig["enableDualTransmission"] = true - relayConfig["dualTransmission"] = evmtypes.DualTransmissionConfig{ - ContractAddress: common.Address{}, - TransmitterAddress: null.StringFrom(nodeTransmitterAddress), - Meta: nil, - } - } - ocrSpec := &nodeclient.OCR2TaskJobSpec{ Name: fmt.Sprintf("ocr2-%s", uuid.NewString()), JobType: "offchainreporting2", @@ -128,9 +113,11 @@ func CreateOCRv2JobsLocal( ObservationSource: nodeclient.ObservationSourceSpecBridge(bta), ForwardingAllowed: forwardingAllowed, OCR2OracleSpec: job.OCR2OracleSpec{ - PluginType: "median", - Relay: "evm", - RelayConfig: relayConfig, + PluginType: "median", + Relay: "evm", + RelayConfig: map[string]interface{}{ + "chainID": chainId, + }, PluginConfig: map[string]any{ "juelsPerFeeCoinSource": fmt.Sprintf("\"\"\"%s\"\"\"", nodeclient.ObservationSourceSpecBridge(juelsBridge)), }, From 6d5008a9bcbefc0d5c4e85f66cff3819a1531fae Mon Sep 17 00:00:00 2001 From: george-dorin Date: Wed, 13 Nov 2024 15:45:12 +0200 Subject: [PATCH 05/11] Fix lint --- core/services/ocrcommon/transmitter.go | 4 ++-- core/services/ocrcommon/transmitter_test.go | 22 ++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/core/services/ocrcommon/transmitter.go b/core/services/ocrcommon/transmitter.go index cdbc6444776..02a52938d4d 100644 --- a/core/services/ocrcommon/transmitter.go +++ b/core/services/ocrcommon/transmitter.go @@ -226,7 +226,7 @@ type ocr2FeedsDualTransmission struct { } func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, toAddress common.Address, payload []byte, txMeta *txmgr.TxMeta) error { - //Primary transmission + // Primary transmission err := t.transmitter.CreateEthTransaction(ctx, toAddress, payload, txMeta) if err != nil { return err @@ -239,7 +239,7 @@ func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, to txMeta.DualBroadcast = true txMeta.DualBroadcastParams = t.urlParams() - //Secondary transmission + // Secondary transmission _, err = t.transmitter.txm.CreateTransaction(ctx, txmgr.TxRequest{ FromAddress: t.secondaryFromAddress, ToAddress: t.secondaryContractAddress, diff --git a/core/services/ocrcommon/transmitter_test.go b/core/services/ocrcommon/transmitter_test.go index 80f053b26d1..b8755d50c5f 100644 --- a/core/services/ocrcommon/transmitter_test.go +++ b/core/services/ocrcommon/transmitter_test.go @@ -217,23 +217,23 @@ func Test_DualTransmitter(t *testing.T) { require.NoError(t, err) primaryTxConfirmed := false - seconryTxConfirmed := false + secondaryTxConfirmed := false txm.On("CreateTransaction", mock.Anything, mock.MatchedBy(func(tx txmgr.TxRequest) bool { - if tx.FromAddress == fromAddress { - //Primary transmission + switch tx.FromAddress { + case fromAddress: + // Primary transmission assert.Equal(t, tx.ToAddress, toAddress, "unexpected primary toAddress") assert.Nil(t, tx.Meta, "Meta should be empty") primaryTxConfirmed = true - } else if tx.FromAddress == secondaryFromAddress { - //Secondary transmission + case secondaryFromAddress: + // Secondary transmission assert.Equal(t, tx.ToAddress, secondaryContractAddress, "unexpected secondary toAddress") assert.True(t, tx.Meta.DualBroadcast, "DualBroadcast should be true") - assert.Equal(t, tx.Meta.DualBroadcastParams, "key1=value1&key2=value2&key2=value3&key3=value4&key3=value5&key3=value6", "DualBroadcastParams not equal") - seconryTxConfirmed = true - - } else { - //Should never be reached + assert.Equal(t, "key1=value1&key2=value2&key2=value3&key3=value4&key3=value5&key3=value6", tx.Meta.DualBroadcastParams, "DualBroadcastParams not equal") + secondaryTxConfirmed = true + default: + // Should never be reached return false } @@ -243,5 +243,5 @@ func Test_DualTransmitter(t *testing.T) { require.NoError(t, transmitter.CreateEthTransaction(testutils.Context(t), toAddress, payload, nil)) require.True(t, primaryTxConfirmed) - require.True(t, seconryTxConfirmed) + require.True(t, secondaryTxConfirmed) } From eaace85731deedff4a491f009272292b9441243a Mon Sep 17 00:00:00 2001 From: george-dorin Date: Wed, 13 Nov 2024 15:54:40 +0200 Subject: [PATCH 06/11] Fix lint --- core/services/job/job_orm_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/services/job/job_orm_test.go b/core/services/job/job_orm_test.go index 6009dd2452b..97dc6d4a78a 100644 --- a/core/services/job/job_orm_test.go +++ b/core/services/job/job_orm_test.go @@ -2040,7 +2040,7 @@ func TestORM_CreateJob_OCR2_With_DualTransmission(t *testing.T) { jobORM := NewTestORM(t, db, pipelineORM, bridgesORM, keyStore) - //Enabled but no config set + // Enabled but no config set enabledDualTransmissionSpec := ` enableDualTransmission=true` @@ -2048,7 +2048,7 @@ func TestORM_CreateJob_OCR2_With_DualTransmission(t *testing.T) { require.NoError(t, err) require.ErrorContains(t, jobORM.CreateJob(ctx, &jb), "dual transmission is enabled but no dual transmission config present") - //contractAddress not set + // ContractAddress not set emptyContractAddress := ` enableDualTransmission=true [relayConfig.dualTransmission] @@ -2058,7 +2058,7 @@ func TestORM_CreateJob_OCR2_With_DualTransmission(t *testing.T) { require.NoError(t, err) require.ErrorContains(t, jobORM.CreateJob(ctx, &jb), "invalid contract address in dual transmission config") - //Transmitter address not set + // Transmitter address not set emptyTransmitterAddress := ` enableDualTransmission=true [relayConfig.dualTransmission] @@ -2083,10 +2083,10 @@ func TestORM_CreateJob_OCR2_With_DualTransmission(t *testing.T) { jb.OCR2OracleSpec.TransmitterID = null.StringFrom(transmitterID.String()) - //unknown transmitter address + // Unknown transmitter address require.ErrorContains(t, jobORM.CreateJob(ctx, &jb), "unknown dual transmission transmitterAddress: no EVM key matching:") - //Should not error + // Should not error keyStore.Eth().XXXTestingOnlyAdd(ctx, dtTransmitterAddress) require.NoError(t, jobORM.CreateJob(ctx, &jb)) } From 0efa3be3fdb65f6701817d6720e61b14f158fa28 Mon Sep 17 00:00:00 2001 From: george-dorin Date: Wed, 13 Nov 2024 16:25:16 +0200 Subject: [PATCH 07/11] Change tx.meta to pointers Send secondary tx even if primary fails --- common/txmgr/types/tx.go | 4 ++-- core/services/ocrcommon/transmitter.go | 19 +++++++++++-------- core/services/ocrcommon/transmitter_test.go | 4 ++-- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/common/txmgr/types/tx.go b/common/txmgr/types/tx.go index ea3fc55330c..f04047a36c1 100644 --- a/common/txmgr/types/tx.go +++ b/common/txmgr/types/tx.go @@ -161,8 +161,8 @@ type TxMeta[ADDR types.Hashable, TX_HASH types.Hashable] struct { SeqNumbers []uint64 `json:"SeqNumbers,omitempty"` // Dual Broadcast - DualBroadcast bool `json:"DualBroadcast,omitempty"` - DualBroadcastParams string `json:"DualBroadcastParams,omitempty"` + DualBroadcast *bool `json:"DualBroadcast,omitempty"` + DualBroadcastParams *string `json:"DualBroadcastParams,omitempty"` } type TxAttempt[ diff --git a/core/services/ocrcommon/transmitter.go b/core/services/ocrcommon/transmitter.go index 02a52938d4d..3602a74eed9 100644 --- a/core/services/ocrcommon/transmitter.go +++ b/core/services/ocrcommon/transmitter.go @@ -2,6 +2,7 @@ package ocrcommon import ( "context" + errors2 "errors" "math/big" "net/url" "slices" @@ -227,20 +228,21 @@ type ocr2FeedsDualTransmission struct { func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, toAddress common.Address, payload []byte, txMeta *txmgr.TxMeta) error { // Primary transmission - err := t.transmitter.CreateEthTransaction(ctx, toAddress, payload, txMeta) - if err != nil { - return err - } + errPrimary := t.transmitter.CreateEthTransaction(ctx, toAddress, payload, txMeta) + errPrimary = errors.Wrap(errPrimary, "skipped primary transmission") if txMeta == nil { txMeta = &txmgr.TxMeta{} } - txMeta.DualBroadcast = true - txMeta.DualBroadcastParams = t.urlParams() + dualBroadcast := true + dualBroadcastParams := t.urlParams() + + txMeta.DualBroadcast = &dualBroadcast + txMeta.DualBroadcastParams = &dualBroadcastParams // Secondary transmission - _, err = t.transmitter.txm.CreateTransaction(ctx, txmgr.TxRequest{ + _, errSecondary := t.transmitter.txm.CreateTransaction(ctx, txmgr.TxRequest{ FromAddress: t.secondaryFromAddress, ToAddress: t.secondaryContractAddress, EncodedPayload: payload, @@ -250,7 +252,8 @@ func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, to Meta: txMeta, }) - return errors.Wrap(err, "skipped secondary transmission") + errSecondary = errors.Wrap(errSecondary, "skipped secondary transmission") + return errors2.Join(errPrimary, errSecondary) } func (t *ocr2FeedsDualTransmission) FromAddress(ctx context.Context) common.Address { diff --git a/core/services/ocrcommon/transmitter_test.go b/core/services/ocrcommon/transmitter_test.go index b8755d50c5f..5f434e59c62 100644 --- a/core/services/ocrcommon/transmitter_test.go +++ b/core/services/ocrcommon/transmitter_test.go @@ -229,8 +229,8 @@ func Test_DualTransmitter(t *testing.T) { case secondaryFromAddress: // Secondary transmission assert.Equal(t, tx.ToAddress, secondaryContractAddress, "unexpected secondary toAddress") - assert.True(t, tx.Meta.DualBroadcast, "DualBroadcast should be true") - assert.Equal(t, "key1=value1&key2=value2&key2=value3&key3=value4&key3=value5&key3=value6", tx.Meta.DualBroadcastParams, "DualBroadcastParams not equal") + assert.True(t, *tx.Meta.DualBroadcast, "DualBroadcast should be true") + assert.Equal(t, "key1=value1&key2=value2&key2=value3&key3=value4&key3=value5&key3=value6", *tx.Meta.DualBroadcastParams, "DualBroadcastParams not equal") secondaryTxConfirmed = true default: // Should never be reached From b039e3a16b4e5c08f332b94cb88cd62494d851c9 Mon Sep 17 00:00:00 2001 From: george-dorin Date: Wed, 13 Nov 2024 16:59:17 +0200 Subject: [PATCH 08/11] Add meta fields to test --- core/services/job/job_orm_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/services/job/job_orm_test.go b/core/services/job/job_orm_test.go index 97dc6d4a78a..1ddb18a3d89 100644 --- a/core/services/job/job_orm_test.go +++ b/core/services/job/job_orm_test.go @@ -2075,6 +2075,9 @@ func TestORM_CreateJob_OCR2_With_DualTransmission(t *testing.T) { [relayConfig.dualTransmission] contractAddress = '0x613a38AC1659769640aaE063C651F48E0250454C' transmitterAddress = '%s' + [relayConfig.dualTransmission.meta] + key1 = 'val1' + key2 = ['val2','val3'] `, dtTransmitterAddress.Address.String()) From e8c9963344e781fbb2d4fe909c6f196bb188bfe6 Mon Sep 17 00:00:00 2001 From: george-dorin Date: Thu, 14 Nov 2024 13:54:11 +0200 Subject: [PATCH 09/11] Remove unused code Change DualTransmissionConfig to pointer --- .../relay/evm/contract_transmitter.go | 28 ------------------- core/services/relay/evm/evm.go | 2 +- core/services/relay/evm/types/types.go | 4 +-- 3 files changed, 3 insertions(+), 31 deletions(-) diff --git a/core/services/relay/evm/contract_transmitter.go b/core/services/relay/evm/contract_transmitter.go index 28a223d32b4..6b9ad3df24b 100644 --- a/core/services/relay/evm/contract_transmitter.go +++ b/core/services/relay/evm/contract_transmitter.go @@ -12,8 +12,6 @@ import ( "github.com/ethereum/go-ethereum/common" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/pkg/errors" - types2 "github.com/smartcontractkit/libocr/offchainreporting2/types" - "github.com/smartcontractkit/libocr/offchainreporting2plus/chains/evmutil" ocrtypes "github.com/smartcontractkit/libocr/offchainreporting2plus/types" @@ -247,29 +245,3 @@ func (oc *contractTransmitter) HealthReport() map[string]error { return map[string]error{oc.Name(): nil} } func (oc *contractTransmitter) Name() string { return oc.lggr.Name() } - -var _ types2.ContractTransmitter = (*dualContractTransmitter)(nil) - -type dualContractTransmitter struct { - baseContractTransmitter contractTransmitter -} - -func (d *dualContractTransmitter) secondaryTransmit(ctx context.Context, reportContext types2.ReportContext, report types2.Report, signatures []types2.AttributedOnchainSignature) error { - return nil -} - -func (d *dualContractTransmitter) Transmit(ctx context.Context, reportContext types2.ReportContext, report types2.Report, signatures []types2.AttributedOnchainSignature) error { - err := d.secondaryTransmit(ctx, reportContext, report, signatures) - if err != nil { - d.baseContractTransmitter.lggr.Warnw("secondary transmission failed", "err", err) - } - return d.baseContractTransmitter.Transmit(ctx, reportContext, report, signatures) -} - -func (d *dualContractTransmitter) LatestConfigDigestAndEpoch(ctx context.Context) (configDigest types2.ConfigDigest, epoch uint32, err error) { - return d.baseContractTransmitter.LatestConfigDigestAndEpoch(ctx) -} - -func (d *dualContractTransmitter) FromAccount(ctx context.Context) (types2.Account, error) { - return d.baseContractTransmitter.FromAccount(ctx) -} diff --git a/core/services/relay/evm/evm.go b/core/services/relay/evm/evm.go index c05dad499cf..db0fe90796b 100644 --- a/core/services/relay/evm/evm.go +++ b/core/services/relay/evm/evm.go @@ -806,7 +806,7 @@ func generateTransmitterFrom(ctx context.Context, rargs commontypes.RelayArgs, e checker, configWatcher.chain.ID(), ethKeystore, - &relayConfig.DualTransmissionConfig, + relayConfig.DualTransmissionConfig, ) case commontypes.CCIPExecution: transmitter, err = cciptransmitter.NewTransmitterWithStatusChecker( diff --git a/core/services/relay/evm/types/types.go b/core/services/relay/evm/types/types.go index f491e064237..2b56aee6379 100644 --- a/core/services/relay/evm/types/types.go +++ b/core/services/relay/evm/types/types.go @@ -223,8 +223,8 @@ type RelayConfig struct { LLOConfigMode LLOConfigMode `json:"lloConfigMode" toml:"lloConfigMode"` // DualTransmission specific - EnableDualTransmission bool `json:"enableDualTransmission" toml:"enableDualTransmission"` - DualTransmissionConfig DualTransmissionConfig `json:"dualTransmission" toml:"dualTransmission"` + EnableDualTransmission bool `json:"enableDualTransmission" toml:"enableDualTransmission"` + DualTransmissionConfig *DualTransmissionConfig `json:"dualTransmission" toml:"dualTransmission"` } var ErrBadRelayConfig = errors.New("bad relay config") From 4985cf950f33f93c43cbe1c01ee1dde60bc87a72 Mon Sep 17 00:00:00 2001 From: george-dorin Date: Thu, 14 Nov 2024 16:29:43 +0200 Subject: [PATCH 10/11] Implement feedback --- core/services/ocrcommon/transmitter.go | 4 ++-- core/services/relay/evm/contract_transmitter.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core/services/ocrcommon/transmitter.go b/core/services/ocrcommon/transmitter.go index 3602a74eed9..d162dd68067 100644 --- a/core/services/ocrcommon/transmitter.go +++ b/core/services/ocrcommon/transmitter.go @@ -3,6 +3,7 @@ package ocrcommon import ( "context" errors2 "errors" + "fmt" "math/big" "net/url" "slices" @@ -229,8 +230,7 @@ type ocr2FeedsDualTransmission struct { func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, toAddress common.Address, payload []byte, txMeta *txmgr.TxMeta) error { // Primary transmission errPrimary := t.transmitter.CreateEthTransaction(ctx, toAddress, payload, txMeta) - errPrimary = errors.Wrap(errPrimary, "skipped primary transmission") - + errPrimary = fmt.Errorf("skipped primary transmission: %w", errPrimary) if txMeta == nil { txMeta = &txmgr.TxMeta{} } diff --git a/core/services/relay/evm/contract_transmitter.go b/core/services/relay/evm/contract_transmitter.go index 6b9ad3df24b..aead9f6ca8a 100644 --- a/core/services/relay/evm/contract_transmitter.go +++ b/core/services/relay/evm/contract_transmitter.go @@ -12,6 +12,7 @@ import ( "github.com/ethereum/go-ethereum/common" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/pkg/errors" + "github.com/smartcontractkit/libocr/offchainreporting2plus/chains/evmutil" ocrtypes "github.com/smartcontractkit/libocr/offchainreporting2plus/types" From edf74cfebc61a2ad46755a146f3a32824e1fb6c7 Mon Sep 17 00:00:00 2001 From: george-dorin Date: Thu, 14 Nov 2024 16:44:57 +0200 Subject: [PATCH 11/11] Fix failing test --- core/services/ocrcommon/transmitter.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/services/ocrcommon/transmitter.go b/core/services/ocrcommon/transmitter.go index d162dd68067..8121f3778d2 100644 --- a/core/services/ocrcommon/transmitter.go +++ b/core/services/ocrcommon/transmitter.go @@ -230,7 +230,10 @@ type ocr2FeedsDualTransmission struct { func (t *ocr2FeedsDualTransmission) CreateEthTransaction(ctx context.Context, toAddress common.Address, payload []byte, txMeta *txmgr.TxMeta) error { // Primary transmission errPrimary := t.transmitter.CreateEthTransaction(ctx, toAddress, payload, txMeta) - errPrimary = fmt.Errorf("skipped primary transmission: %w", errPrimary) + if errPrimary != nil { + errPrimary = fmt.Errorf("skipped primary transmission: %w", errPrimary) + } + if txMeta == nil { txMeta = &txmgr.TxMeta{} }