[chore] Move core service types to types/core to mirror package structure; Add RelayerSet #475

cedric-cordenier · 2024-04-18T12:55:33Z

Move the core API services (keystore, capabilities_registry, kv store, telemetry, etc) into core.
Also move the API "integration path" methods relating to integration reporting plugins into core.
Add a RelayerSet API. This duplicates some structs (PluginArgs, RelayArgs), but omits job-related fields.

NOTE: Working on preparing the corresponding PRs to core/chainlink-solana, and will hold off on merging this until those are ready. I'd still appreciate some early feedback at this stage though!

krehermann · 2024-04-18T14:20:59Z

pkg/types/core/relayerset.go

+
+type RelayerSet interface {
+	Get(ctx context.Context, relayID types.RelayID) (Relayer, error)
+	GetAll(ctx context.Context) ([]Relayer, error)


prefer List to GetAll.

additionally if you want to complete, you can pass a vardiac filter List(ctx, ids ...types.RelayID) and return everything when len(ids) == 0

krehermann

lgtm up to nit about api naming

pkg/types/relayer.go

cedric-cordenier · 2024-04-19T12:41:44Z

Corresponding PRs:

krehermann · 2024-04-19T13:45:42Z

pkg/types/relayer.go

+)
+
+func (i *RelayID) UnmarshalString(s string) error {
+	idxs := idRegex.FindStringIndex(s)


this looks overly complex. can't it be simple string split on . into 2 pieces?

krehermann · 2024-04-19T13:47:27Z

pkg/types/relayer.go

+}
+
+var idRegex = regexp.MustCompile(
+	fmt.Sprintf("^((%s)|(%s)|(%s)|(%s))\\.", NetworkEVM, NetworkCosmos, NetworkSolana, NetworkStarkNet),


as said below, i think the regex is more complex than needed. however, if you need to keep to it, then it should be constructed from SupportNetwork rather than individual consts

@krehermann Do you mind if I defer this? This is just the existing implementation copied from core to common

krehermann · 2024-04-19T13:51:34Z

pkg/types/relayer.go

does this file make sense here? if we are mirroring the core types up from internal, shouldn't we do the same for the other building blocks?

imho it would make the core side easier to read.

currently it is ~types.NetworkEVM and core.Keystore etc.

relayer.EVM and core.Keystone would be more consistent. or relayer.EVMNetwork

I don't disagree, but I was trying to just do what I needed to get the RelayerSet interface into common as I promised Dimitris I'd do that by EOW. There's already been a bit of scope creep and I want to avoid further creep here, but happy to follow up with moving the remaining types into types/relayer.

ok. can you add some TODOs with ticket references for the deferred maintenance? i'm fine with postponing, but i want to make sure we do the cleanup. if there are TODOs and unassigned tickets in the backlog we can clean them during planning.

krehermann · 2024-04-19T13:54:08Z

pkg/types/core/median.go

+	"github.com/smartcontractkit/chainlink-common/pkg/types"
+)
+
+type PluginMedian interface {


why is this a core type? it seems out of place relative to the other things

It's analogous to the reporting plugin factories methods, except median-specific (dating from when median was a custom loopp type).

cedric-cordenier requested a review from ettec April 18, 2024 12:55

cedric-cordenier temporarily deployed to integration April 18, 2024 12:55 — with GitHub Actions Inactive

cedric-cordenier force-pushed the relayerset-interfaces branch from 2cc3c10 to 20c0628 Compare April 18, 2024 14:06

cedric-cordenier had a problem deploying to integration April 18, 2024 14:06 — with GitHub Actions Error

cedric-cordenier had a problem deploying to integration April 18, 2024 14:06 — with GitHub Actions Failure

cedric-cordenier force-pushed the relayerset-interfaces branch from 20c0628 to 157d875 Compare April 18, 2024 14:09

cedric-cordenier had a problem deploying to integration April 18, 2024 14:09 — with GitHub Actions Failure

cedric-cordenier temporarily deployed to integration April 18, 2024 14:10 — with GitHub Actions Inactive

cedric-cordenier changed the title ~~WIP RelayerSet interfaces~~ [chore] Move core service types to types/core to mirror package structure; Add RelayerSet Apr 18, 2024

cedric-cordenier requested a review from krehermann April 18, 2024 14:10

krehermann reviewed Apr 18, 2024

View reviewed changes

cedric-cordenier force-pushed the relayerset-interfaces branch from 157d875 to bfbd363 Compare April 18, 2024 14:32

cedric-cordenier temporarily deployed to integration April 18, 2024 14:32 — with GitHub Actions Inactive

cedric-cordenier had a problem deploying to integration April 18, 2024 14:32 — with GitHub Actions Failure

cedric-cordenier force-pushed the relayerset-interfaces branch from bfbd363 to e32d9b8 Compare April 18, 2024 14:36

cedric-cordenier temporarily deployed to integration April 18, 2024 14:36 — with GitHub Actions Inactive

cedric-cordenier had a problem deploying to integration April 18, 2024 14:36 — with GitHub Actions Failure

cedric-cordenier marked this pull request as ready for review April 18, 2024 14:42

cedric-cordenier requested review from jmank88 and krebernisak as code owners April 18, 2024 14:42

jmank88 reviewed Apr 18, 2024

View reviewed changes

pkg/types/relayer.go Outdated Show resolved Hide resolved

cedric-cordenier force-pushed the relayerset-interfaces branch from e32d9b8 to 9d32752 Compare April 18, 2024 14:58

cedric-cordenier had a problem deploying to integration April 18, 2024 14:58 — with GitHub Actions Failure

cedric-cordenier had a problem deploying to integration April 18, 2024 16:26 — with GitHub Actions Error

cedric-cordenier force-pushed the relayerset-interfaces branch from af8e582 to fdd8534 Compare April 18, 2024 16:28

cedric-cordenier temporarily deployed to integration April 18, 2024 16:28 — with GitHub Actions Inactive

cedric-cordenier had a problem deploying to integration April 18, 2024 16:28 — with GitHub Actions Failure

cedric-cordenier force-pushed the relayerset-interfaces branch from fdd8534 to 9bef5c9 Compare April 18, 2024 16:35

cedric-cordenier had a problem deploying to integration April 18, 2024 16:35 — with GitHub Actions Failure

cedric-cordenier temporarily deployed to integration April 18, 2024 16:35 — with GitHub Actions Inactive

cedric-cordenier force-pushed the relayerset-interfaces branch from 9bef5c9 to 3399470 Compare April 19, 2024 12:34

cedric-cordenier had a problem deploying to integration April 19, 2024 12:35 — with GitHub Actions Failure

cedric-cordenier temporarily deployed to integration April 19, 2024 12:35 — with GitHub Actions Inactive

cedric-cordenier had a problem deploying to integration April 19, 2024 12:35 — with GitHub Actions Failure

cedric-cordenier requested review from jmank88 and krehermann April 19, 2024 12:35

[chore] Move core types to types/core; add RelayerSet type

f23f3bd

cedric-cordenier force-pushed the relayerset-interfaces branch from 3399470 to f23f3bd Compare April 19, 2024 12:52

cedric-cordenier temporarily deployed to integration April 19, 2024 12:52 — with GitHub Actions Inactive

cedric-cordenier had a problem deploying to integration April 19, 2024 12:52 — with GitHub Actions Failure

krehermann reviewed Apr 19, 2024

View reviewed changes

krehermann approved these changes Apr 19, 2024

View reviewed changes

cedric-cordenier merged commit 71a18c1 into main Apr 19, 2024
6 of 9 checks passed

cedric-cordenier deleted the relayerset-interfaces branch April 19, 2024 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[chore] Move core service types to types/core to mirror package structure; Add RelayerSet #475

[chore] Move core service types to types/core to mirror package structure; Add RelayerSet #475

cedric-cordenier commented Apr 18, 2024 •

edited

Loading

krehermann Apr 18, 2024

krehermann left a comment

cedric-cordenier commented Apr 19, 2024

krehermann Apr 19, 2024

krehermann Apr 19, 2024

cedric-cordenier Apr 19, 2024

krehermann Apr 19, 2024 •

edited

Loading

cedric-cordenier Apr 19, 2024

krehermann Apr 19, 2024

krehermann Apr 19, 2024

cedric-cordenier Apr 19, 2024 •

edited

Loading

[chore] Move core service types to types/core to mirror package structure; Add RelayerSet #475

[chore] Move core service types to types/core to mirror package structure; Add RelayerSet #475

Conversation

cedric-cordenier commented Apr 18, 2024 • edited Loading

krehermann Apr 18, 2024

Choose a reason for hiding this comment

krehermann left a comment

Choose a reason for hiding this comment

cedric-cordenier commented Apr 19, 2024

krehermann Apr 19, 2024

Choose a reason for hiding this comment

krehermann Apr 19, 2024

Choose a reason for hiding this comment

cedric-cordenier Apr 19, 2024

Choose a reason for hiding this comment

krehermann Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

cedric-cordenier Apr 19, 2024

Choose a reason for hiding this comment

krehermann Apr 19, 2024

Choose a reason for hiding this comment

krehermann Apr 19, 2024

Choose a reason for hiding this comment

cedric-cordenier Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

cedric-cordenier commented Apr 18, 2024 •

edited

Loading

krehermann Apr 19, 2024 •

edited

Loading

cedric-cordenier Apr 19, 2024 •

edited

Loading