[BCF-2365] Consolidate *Provider Constructors into One #10475

cedric-cordenier · 2023-09-04T15:38:59Z

Adapt the MercuryProvider so that it strictly extends the PluginProvider type. The additional functions in the MercuryTransmitter have moved to a separate MercuryServerFetcher component.
Introduce a wrapper relayerServerAdapter type which contains similar logic to the GRPC relayerServer, whose NewPluginProvider function returns a different provider type depending on the passed in ProviderType.
Replace uses of New{Mercury|Median|Functions}Provider with calls to NewPluginProvider with the correct ProviderType passed in.

github-actions · 2023-09-04T15:39:19Z

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

jmank88 · 2023-09-05T12:33:37Z

go.mod

@@ -66,7 +66,7 @@ require (
 	github.com/shopspring/decimal v1.3.1
 	github.com/smartcontractkit/caigo v0.0.0-20230621050857-b29a4ca8c704
 	github.com/smartcontractkit/chainlink-cosmos v0.4.1-0.20230831132059-42af68994512
-	github.com/smartcontractkit/chainlink-relay v0.1.7-0.20230901143551-bf6bd84d6938
+	github.com/smartcontractkit/chainlink-relay v0.1.7-0.20230904152856-39e4d6f4a24b


[BCF-2365] Combine providers chainlink-common#164

core/services/relay/relay.go

jmank88 · 2023-09-05T12:45:14Z

core/services/ocrbootstrap/delegate.go

 			ctx,
 			types.RelayArgs{
 				ExternalJobID: jobSpec.ExternalJobID,
 				JobID:         spec.ID,
 				ContractID:    spec.ContractID,
 				RelayConfig:   spec.RelayConfig.Bytes(),
 				New:           d.isNewlyCreatedJob,
+				ProviderType:  types.ProviderTypeFunctions,


I'm still not 100% sure that I understand this new types.ProviderType*. Where will the mapping from plugin type to provider type exist long-term? Core cannot continue to make this associate explicit in the delegate like this. To be product agnostic, it would have to just pass along information from the job spec. It seems like we either need to add this new field to the jobspec, or infer/derive from the plugin type inside the relayer instead of here in core.

I understand that we don't want to lock in plugin type and provider type as 1-1, but we also have to balance that against node op UX simplicity, so I would favor hiding this difference for now.

Assuming you're talking about the mapping from the ProviderType value to the ProviderType type, then this mapping should live either in the RelayerServerAdapter (which shouldn't really live in the core node tbh) or in the RelayerServer.

This field should live in the pluginConfig of the jobspec long-term (such as for generic plugin job types). However, based on our discussion the other day where we said that some parts of the codebase will still need to be provider aware, I just added them statically in those places for now (my reasoning being that there is no difference between a call to say, NewMedianProvider or a call to NewPluginProvider with providerType = "median"). That's the case for this block of code here, which has to do with router proxies, which is a functions only feature.

I had a similar reaction.

It seems like the job spec is the string that ties all the services together and so the provider type should be coming from there.

how does core know which services to start today for say, mercury vs functions? is something in the jobspec or is it inferred from something else?

my real question is: if we add the provider field to the spec, is it a breaking change or can we infer the correct behavior and backfill on-the-fly as needed to update the spec?

how does core know which services to start today for say, mercury vs functions? is something in the jobspec or is it inferred from something else?

That comes from the plugin type on the job spec at the moment.

my real question is: if we add the provider field to the spec, is it a breaking change or can we infer the correct behavior and backfill on-the-fly as needed to update the spec?

For old job types of the OCR2 job type we could backfill on the fly. I don't know that we could do that in this particular case (ocrbootstrap) since I'm not sure if the pluginType is used. For functions I assume it isn't since they're checking the router DonID rather than the pluginType.

core/services/relay/evm/mercury_provider.go

cedric-cordenier · 2023-09-05T14:03:01Z

core/services/ocrbootstrap/delegate.go

 			ctx,
 			types.RelayArgs{
 				ExternalJobID: jobSpec.ExternalJobID,
 				JobID:         spec.ID,
 				ContractID:    spec.ContractID,
 				RelayConfig:   spec.RelayConfig.Bytes(),
 				New:           d.isNewlyCreatedJob,
+				ProviderType:  string(job.OCR2Functions),


@jmank88 @krehermann Was this the kind of change you had in mind? (Here taken explicitly, but taken from the job spec in the case of Median and Mercury)

I've coerced the value to a string since that way it's acts as a passthrough value when we source it from the providerType.

Oh I didn't realize this one was such an oddball 🤔
Yeah, I think this works for now, and we should be able to evolve in a compatible way 🤔

github-actions · 2023-09-07T09:58:44Z

Running downstream job at https://github.com/smartcontractkit/operator-ui/actions/runs/6108242478

github-actions · 2023-09-07T10:23:03Z

Running downstream job at https://github.com/smartcontractkit/operator-ui/actions/runs/6108501128

github-actions · 2023-09-07T12:05:58Z

Running downstream job at https://github.com/smartcontractkit/operator-ui/actions/runs/6109453760

github-actions · 2023-09-07T13:49:38Z

Running downstream job at https://github.com/smartcontractkit/operator-ui/actions/runs/6110623679

core/services/ocr2/plugins/median/services.go

core/services/ocrbootstrap/delegate.go

core/services/relay/evm/loop_impl.go

core/services/relay/relay.go

github-actions · 2023-09-07T16:06:14Z

Running downstream job at https://github.com/smartcontractkit/operator-ui/actions/runs/6112141783

- Adapt the MercuryProvider so that it strictly extends the PluginProvider type. The additional functions in the MercuryTransmitter have moved to a separate MercuryServerFetcher component. - Introduce a wrapper relayerServerAdapter type which contains similar logic to the GRPC relayerServer, whose NewPluginProvider function returns a different provider type depending on the passed in ProviderType. - Replace uses of New{Mercury|Median|Functions}Provider with calls to NewPluginProvider with the correct ProviderType passed in.

github-actions · 2023-09-07T16:59:00Z

Running downstream job at https://github.com/smartcontractkit/operator-ui/actions/runs/6112640045

github-actions · 2023-09-08T10:37:12Z

Running downstream job at https://github.com/smartcontractkit/operator-ui/actions/runs/6120721460

github-actions · 2023-09-08T13:04:09Z

Running downstream job at https://github.com/smartcontractkit/operator-ui/actions/runs/6122089885

github-actions · 2023-09-08T13:13:31Z

Running downstream job at https://github.com/smartcontractkit/operator-ui/actions/runs/6122192882

github-actions · 2023-09-08T13:31:30Z

Running downstream job at https://github.com/smartcontractkit/operator-ui/actions/runs/6122378021

cl-sonarqube-production · 2023-09-08T13:48:46Z

SonarQube Quality Gate

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

91.9% Coverage
0.0% Duplication

cedric-cordenier force-pushed the BCF-2365-combine-providers branch 2 times, most recently from fb677fa to b2813d7 Compare September 4, 2023 16:49

cedric-cordenier marked this pull request as ready for review September 5, 2023 09:01

cedric-cordenier requested review from a team, jmank88, krehermann, connorwstein, samsondav, archseer and cfal as code owners September 5, 2023 09:01

cedric-cordenier changed the title ~~Bcf 2365 combine providers~~ [BCF-2365] Consolidate *Provider Constructors into One Sep 5, 2023

cedric-cordenier force-pushed the BCF-2365-combine-providers branch from b2813d7 to 368154b Compare September 5, 2023 09:38

jmank88 reviewed Sep 5, 2023

View reviewed changes

core/services/relay/relay.go Outdated Show resolved Hide resolved

jmank88 reviewed Sep 5, 2023

View reviewed changes

core/services/relay/relay.go Show resolved Hide resolved

jmank88 reviewed Sep 5, 2023

View reviewed changes

krehermann reviewed Sep 5, 2023

View reviewed changes

core/services/relay/evm/mercury_provider.go Show resolved Hide resolved

cedric-cordenier commented Sep 5, 2023

View reviewed changes

cedric-cordenier force-pushed the BCF-2365-combine-providers branch from 75cc7d0 to 608f982 Compare September 7, 2023 09:58

cedric-cordenier force-pushed the BCF-2365-combine-providers branch from 608f982 to dc72147 Compare September 7, 2023 10:22

cedric-cordenier force-pushed the BCF-2365-combine-providers branch from dc72147 to 6be2fc3 Compare September 7, 2023 12:05

cedric-cordenier force-pushed the BCF-2365-combine-providers branch from 6be2fc3 to 316ccfe Compare September 7, 2023 13:49

cedric-cordenier requested a review from a team as a code owner September 7, 2023 13:49

cedric-cordenier force-pushed the BCF-2365-combine-providers branch from 316ccfe to bb747fa Compare September 7, 2023 14:03

cedric-cordenier requested a review from jkongie as a code owner September 7, 2023 14:03

cedric-cordenier requested review from jmank88 and krehermann September 7, 2023 14:30

krehermann reviewed Sep 7, 2023

View reviewed changes

core/services/ocr2/plugins/median/services.go Show resolved Hide resolved

core/services/ocrbootstrap/delegate.go Show resolved Hide resolved

core/services/relay/evm/loop_impl.go Outdated Show resolved Hide resolved

core/services/relay/relay.go Show resolved Hide resolved

cedric-cordenier added 2 commits September 7, 2023 17:58

Address review comment

c5f6e4f

cedric-cordenier force-pushed the BCF-2365-combine-providers branch from 8db4cb1 to c5f6e4f Compare September 7, 2023 16:58

Merge branch 'develop' into BCF-2365-combine-providers

2d3c3bb

cedric-cordenier mentioned this pull request Sep 8, 2023

[BCF-2365] Combine providers smartcontractkit/chainlink-common#164

Merged

Update go.mod/sum now that the commit has been merged

bfd60d2

cedric-cordenier force-pushed the BCF-2365-combine-providers branch from 887424a to bfd60d2 Compare September 8, 2023 13:30

krehermann self-requested a review September 8, 2023 13:33

krehermann approved these changes Sep 8, 2023

View reviewed changes

jmank88 approved these changes Sep 8, 2023

View reviewed changes

cedric-cordenier added this pull request to the merge queue Sep 8, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 8, 2023

cedric-cordenier added this pull request to the merge queue Sep 8, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 8, 2023

cedric-cordenier added this pull request to the merge queue Sep 8, 2023

Merged via the queue into develop with commit 6daa8ff Sep 8, 2023
117 checks passed

cedric-cordenier deleted the BCF-2365-combine-providers branch September 8, 2023 16:09

snehaagni mentioned this pull request Oct 18, 2023

release/2.6.0 -> master #10997

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BCF-2365] Consolidate *Provider Constructors into One #10475

[BCF-2365] Consolidate *Provider Constructors into One #10475

cedric-cordenier commented Sep 4, 2023 •

edited

Loading

github-actions bot commented Sep 4, 2023

jmank88 Sep 5, 2023

jmank88 Sep 5, 2023

jmank88 Sep 5, 2023

cedric-cordenier Sep 5, 2023 •

edited

Loading

krehermann Sep 5, 2023

cedric-cordenier Sep 5, 2023

cedric-cordenier Sep 5, 2023

jmank88 Sep 5, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 8, 2023

github-actions bot commented Sep 8, 2023

github-actions bot commented Sep 8, 2023

github-actions bot commented Sep 8, 2023

cl-sonarqube-production bot commented Sep 8, 2023

[BCF-2365] Consolidate *Provider Constructors into One #10475

[BCF-2365] Consolidate *Provider Constructors into One #10475

Conversation

cedric-cordenier commented Sep 4, 2023 • edited Loading

github-actions bot commented Sep 4, 2023

jmank88 Sep 5, 2023

Choose a reason for hiding this comment

jmank88 Sep 5, 2023

Choose a reason for hiding this comment

jmank88 Sep 5, 2023

Choose a reason for hiding this comment

cedric-cordenier Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

krehermann Sep 5, 2023

Choose a reason for hiding this comment

cedric-cordenier Sep 5, 2023

Choose a reason for hiding this comment

cedric-cordenier Sep 5, 2023

Choose a reason for hiding this comment

jmank88 Sep 5, 2023

Choose a reason for hiding this comment

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

github-actions bot commented Sep 8, 2023

github-actions bot commented Sep 8, 2023

github-actions bot commented Sep 8, 2023

github-actions bot commented Sep 8, 2023

cl-sonarqube-production bot commented Sep 8, 2023

cedric-cordenier commented Sep 4, 2023 •

edited

Loading

cedric-cordenier Sep 5, 2023 •

edited

Loading