From d93a55fae361367d643755127c651e787c7fc5ae Mon Sep 17 00:00:00 2001 From: Graham Goh Date: Thu, 17 Oct 2024 13:23:37 +0900 Subject: [PATCH] fix(nodes): use loadByRelayIDs for nodes and node query Nodes and node query only ever supported EVM, migrating to use `loadByRelayIDs` and make sure to set the network to EVM. IF we do not migrate, if there is a duplicate in id between EVM and other network, it will return incorrect results worst case or an error best case. JIRA: https://smartcontract-it.atlassian.net/browse/DPA-1151 --- .changeset/honest-bugs-grin.md | 5 +++ core/web/loader/chain.go | 56 ---------------------------------- core/web/loader/getters.go | 19 ------------ core/web/loader/loader.go | 2 -- core/web/resolver/node.go | 4 ++- core/web/resolver/query.go | 2 ++ 6 files changed, 10 insertions(+), 78 deletions(-) create mode 100644 .changeset/honest-bugs-grin.md diff --git a/.changeset/honest-bugs-grin.md b/.changeset/honest-bugs-grin.md new file mode 100644 index 00000000000..e794330cb0e --- /dev/null +++ b/.changeset/honest-bugs-grin.md @@ -0,0 +1,5 @@ +--- +"chainlink": minor +--- + +#updated handle duplicate chain id in different network better diff --git a/core/web/loader/chain.go b/core/web/loader/chain.go index fe2cd086ee0..5becc4d5478 100644 --- a/core/web/loader/chain.go +++ b/core/web/loader/chain.go @@ -2,7 +2,6 @@ package loader import ( "context" - "slices" "github.com/graph-gophers/dataloader" commonTypes "github.com/smartcontractkit/chainlink-common/pkg/types" @@ -15,61 +14,6 @@ type chainBatcher struct { app chainlink.Application } -// DEPRECATED: loadByChainIDs is deprecated and we should be using loadByRelayIDs. -func (b *chainBatcher) loadByIDs(ctx context.Context, keys dataloader.Keys) []*dataloader.Result { - // Create a map for remembering the order of keys passed in - keyOrder := make(map[string]int, len(keys)) - // Collect the keys to search for - var chainIDs []string - for ix, key := range keys { - chainIDs = append(chainIDs, key.String()) - keyOrder[key.String()] = ix - } - - var cs []types.ChainStatusWithID - relayersMap, err := b.app.GetRelayers().GetIDToRelayerMap() - if err != nil { - return []*dataloader.Result{{Data: nil, Error: err}} - } - - for k, v := range relayersMap { - s, err := v.GetChainStatus(ctx) - if err != nil { - return []*dataloader.Result{{Data: nil, Error: err}} - } - - if slices.Contains(chainIDs, s.ID) { - cs = append(cs, types.ChainStatusWithID{ - ChainStatus: s, - RelayID: k, - }) - } - } - - // todo: future improvements to handle multiple chains with same id - if len(cs) > len(keys) { - b.app.GetLogger().Warn("Found multiple chain with same id") - return []*dataloader.Result{{Data: nil, Error: chains.ErrMultipleChainFound}} - } - - results := make([]*dataloader.Result, len(keys)) - for _, c := range cs { - ix, ok := keyOrder[c.ID] - // if found, remove from index lookup map, so we know elements were found - if ok { - results[ix] = &dataloader.Result{Data: c, Error: nil} - delete(keyOrder, c.ID) - } - } - - // fill array positions without any nodes - for _, ix := range keyOrder { - results[ix] = &dataloader.Result{Data: nil, Error: chains.ErrNotFound} - } - - return results -} - func (b *chainBatcher) loadByRelayIDs(ctx context.Context, keys dataloader.Keys) []*dataloader.Result { results := make([]*dataloader.Result, 0, len(keys)) for _, key := range keys { diff --git a/core/web/loader/getters.go b/core/web/loader/getters.go index 1a60707a850..46333ebc904 100644 --- a/core/web/loader/getters.go +++ b/core/web/loader/getters.go @@ -19,25 +19,6 @@ import ( // ErrInvalidType indicates that results loaded is not the type expected var ErrInvalidType = errors.New("invalid type") -// GetChainByID fetches the chain by it's id. -// DEPRECATED: use GetChainByRelayID. -func GetChainByID(ctx context.Context, id string) (*commonTypes.ChainStatusWithID, error) { - ldr := For(ctx) - - thunk := ldr.ChainsByIDLoader.Load(ctx, dataloader.StringKey(id)) - result, err := thunk() - if err != nil { - return nil, err - } - - chain, ok := result.(commonTypes.ChainStatusWithID) - if !ok { - return nil, ErrInvalidType - } - - return &chain, nil -} - // GetChainByRelayID fetches the chain by it's relayId. func GetChainByRelayID(ctx context.Context, id string) (*commonTypes.ChainStatusWithID, error) { ldr := For(ctx) diff --git a/core/web/loader/loader.go b/core/web/loader/loader.go index 4ce2d4d51f2..e7ef9eb1fe2 100644 --- a/core/web/loader/loader.go +++ b/core/web/loader/loader.go @@ -14,7 +14,6 @@ type loadersKey struct{} type Dataloader struct { app chainlink.Application - ChainsByIDLoader *dataloader.Loader ChainsByRelayIDLoader *dataloader.Loader EthTxAttemptsByEthTxIDLoader *dataloader.Loader FeedsManagersByIDLoader *dataloader.Loader @@ -45,7 +44,6 @@ func New(app chainlink.Application) *Dataloader { return &Dataloader{ app: app, - ChainsByIDLoader: dataloader.NewBatchedLoader(chains.loadByIDs), ChainsByRelayIDLoader: dataloader.NewBatchedLoader(chains.loadByRelayIDs), EthTxAttemptsByEthTxIDLoader: dataloader.NewBatchedLoader(attmpts.loadByEthTransactionIDs), FeedsManagersByIDLoader: dataloader.NewBatchedLoader(mgrs.loadByIDs), diff --git a/core/web/resolver/node.go b/core/web/resolver/node.go index 8e01b7056be..39b971f04df 100644 --- a/core/web/resolver/node.go +++ b/core/web/resolver/node.go @@ -6,6 +6,7 @@ import ( "github.com/graph-gophers/graphql-go" "github.com/pelletier/go-toml/v2" + "github.com/smartcontractkit/chainlink/v2/core/services/relay" "github.com/smartcontractkit/chainlink-common/pkg/types" @@ -88,7 +89,8 @@ func (r *NodeResolver) Order() *int32 { // Chain resolves the node's chain object field. func (r *NodeResolver) Chain(ctx context.Context) (*ChainResolver, error) { - chain, err := loader.GetChainByID(ctx, r.status.ChainID) + relayID := types.NewRelayID(relay.NetworkEVM, r.status.ChainID) + chain, err := loader.GetChainByRelayID(ctx, relayID.Name()) if err != nil { return nil, err } diff --git a/core/web/resolver/query.go b/core/web/resolver/query.go index 6f42238abd4..dcebff10938 100644 --- a/core/web/resolver/query.go +++ b/core/web/resolver/query.go @@ -278,6 +278,7 @@ func (r *Resolver) Features(ctx context.Context) (*FeaturesPayloadResolver, erro } // Node retrieves a node by ID (Name) +// only supports EVM chains func (r *Resolver) Node(ctx context.Context, args struct{ ID graphql.ID }) (*NodePayloadResolver, error) { if err := authenticateUser(ctx); err != nil { return nil, err @@ -378,6 +379,7 @@ func (r *Resolver) JobProposal(ctx context.Context, args struct { } // Nodes retrieves a paginated list of nodes. +// only supports EVM chains func (r *Resolver) Nodes(ctx context.Context, args struct { Offset *int32 Limit *int32