[DPA-1151]: fix: handle duplicate chain id across network better for …

…ethkeys, ethTransactions & chains query. (#14791)

* feat(dataloader): introduce loadByRelayIDs

We currently have a loader `loadByIDs` but this does not take into consideration when same Id exists across different network. Eg if solana and ethereum both have chain ID 1. `loadByIDs` will either return an error or worst returns an incorrect results.

`loadByRelayIDs` rectifies this by performing lookup with ID and Network.

JIRA: https://smartcontract-it.atlassian.net/browse/DPA-1151

* fix(eth): update ethKeys & ethTransaction query

Update ethKeys & ethTransaction query to use the new dataloader `GetChainByRelayID` as the existing loader does not handle duplicate Id across different network.

* fix(chain): add network args to chain query

Allow chain lookup by network and chain Id so we can differentiate same chain id in different networks.

JIRA: https://smartcontract-it.atlassian.net/browse/DPA-1151

.changeset/honest-bugs-grin.md

@@ -0,0 +1,7 @@
+---
+"chainlink": minor
+---
+
+#updated 
+update ethkeys, ethtransactions to handle duplicate chain id in different network
+introduce network arg input to Chain graphql query to allow better lookup based on network and chain id 

core/services/chainlink/mocks/relayer_chain_interoperators.go

@@ -40,7 +40,11 @@ func (f *FakeRelayerChainInteroperators) List(filter chainlink.FilterFn) chainli
 }
 
 func (f *FakeRelayerChainInteroperators) Get(id types.RelayID) (loop.Relayer, error) {
-	panic("unimplemented")
+	r, ok := f.Relayers[id]
+	if !ok {
+		return nil, chainlink.ErrNoSuchRelayer
+	}
+	return r, nil
 }
 
 func (f *FakeRelayerChainInteroperators) GetIDToRelayerMap() (map[types.RelayID]loop.Relayer, error) {

core/web/loader/chain.go

@@ -6,6 +6,8 @@ import (
 
 	"github.com/graph-gophers/dataloader"
 
+	commonTypes "github.com/smartcontractkit/chainlink-common/pkg/types"
+
 	"github.com/smartcontractkit/chainlink/v2/common/types"
 	"github.com/smartcontractkit/chainlink/v2/core/chains"
 	"github.com/smartcontractkit/chainlink/v2/core/services/chainlink"
@@ -15,6 +17,7 @@ type chainBatcher struct {
 	app chainlink.Application
 }
 
+// Deprecated: use 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))
@@ -68,3 +71,34 @@ func (b *chainBatcher) loadByIDs(ctx context.Context, keys dataloader.Keys) []*d
 
 	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 {
+		var relay commonTypes.RelayID
+		err := relay.UnmarshalString(key.String())
+		if err != nil {
+			results = append(results, &dataloader.Result{Data: nil, Error: err})
+			continue
+		}
+
+		relayer, err := b.app.GetRelayers().Get(relay)
+		if err != nil {
+			results = append(results, &dataloader.Result{Data: nil, Error: chains.ErrNotFound})
+			continue
+		}
+
+		status, err := relayer.GetChainStatus(ctx)
+		if err != nil {
+			results = append(results, &dataloader.Result{Data: nil, Error: err})
+			continue
+		}
+
+		results = append(results, &dataloader.Result{Data: types.ChainStatusWithID{
+			ChainStatus: status,
+			RelayID:     relay,
+		}, Error: err})
+	}
+
+	return results
+}

core/web/loader/getters.go

@@ -20,6 +20,7 @@ import (
 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)
 
@@ -37,6 +38,24 @@ func GetChainByID(ctx context.Context, id string) (*commonTypes.ChainStatusWithI
 	return &chain, nil
 }
 
+// GetChainByRelayID fetches the chain by it's relayId.
+func GetChainByRelayID(ctx context.Context, id string) (*commonTypes.ChainStatusWithID, error) {
+	ldr := For(ctx)
+
+	thunk := ldr.ChainsByRelayIDLoader.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
+}
+
 // GetNodesByChainID fetches the nodes for a chain.
 func GetNodesByChainID(ctx context.Context, id string) ([]types.Node, error) {
 	ldr := For(ctx)

core/web/loader/loader.go

@@ -15,6 +15,7 @@ type Dataloader struct {
 	app chainlink.Application
 
 	ChainsByIDLoader                          *dataloader.Loader
+	ChainsByRelayIDLoader                     *dataloader.Loader
 	EthTxAttemptsByEthTxIDLoader              *dataloader.Loader
 	FeedsManagersByIDLoader                   *dataloader.Loader
 	FeedsManagerChainConfigsByManagerIDLoader *dataloader.Loader
@@ -45,6 +46,7 @@ func New(app chainlink.Application) *Dataloader {
 		app: app,
 
 		ChainsByIDLoader:                          dataloader.NewBatchedLoader(chains.loadByIDs),
+		ChainsByRelayIDLoader:                     dataloader.NewBatchedLoader(chains.loadByRelayIDs),
 		EthTxAttemptsByEthTxIDLoader:              dataloader.NewBatchedLoader(attmpts.loadByEthTransactionIDs),
 		FeedsManagersByIDLoader:                   dataloader.NewBatchedLoader(mgrs.loadByIDs),
 		FeedsManagerChainConfigsByManagerIDLoader: dataloader.NewBatchedLoader(ccfgs.loadByManagerIDs),

core/web/loader/loader_test.go

@@ -89,6 +89,79 @@ func TestLoader_Chains(t *testing.T) {
 	assert.ErrorIs(t, results[2].Error, chains.ErrNotFound)
 }
 
+func TestLoader_ChainsRelayID_HandleDuplicateIDAcrossNetworks(t *testing.T) {
+	t.Parallel()
+
+	app := coremocks.NewApplication(t)
+	ctx := InjectDataloader(testutils.Context(t), app)
+
+	one := ubig.NewI(1)
+	chain := toml.EVMConfig{ChainID: one, Chain: toml.Defaults(one)}
+	two := ubig.NewI(2)
+	chain2 := toml.EVMConfig{ChainID: two, Chain: toml.Defaults(two)}
+	config1, err := chain.TOMLString()
+	require.NoError(t, err)
+	config2, err := chain2.TOMLString()
+	require.NoError(t, err)
+
+	evm1 := commontypes.RelayID{
+		Network: relay.NetworkEVM,
+		ChainID: "1",
+	}
+	evm2 := commontypes.RelayID{
+		Network: relay.NetworkEVM,
+		ChainID: "2",
+	}
+	// check if can handle same chain ID but different network
+	solana1 := commontypes.RelayID{
+		Network: relay.NetworkSolana,
+		ChainID: "1",
+	}
+	app.On("GetRelayers").Return(&chainlinkmocks.FakeRelayerChainInteroperators{Relayers: map[commontypes.RelayID]loop.Relayer{
+		evm1: testutils2.MockRelayer{ChainStatus: commontypes.ChainStatus{
+			ID:      "1",
+			Enabled: true,
+			Config:  config1,
+		}},
+		evm2: testutils2.MockRelayer{ChainStatus: commontypes.ChainStatus{
+			ID:      "2",
+			Enabled: true,
+			Config:  config2,
+		}},
+		solana1: testutils2.MockRelayer{ChainStatus: commontypes.ChainStatus{
+			ID:      "1",
+			Enabled: true,
+			Config:  "config",
+		}},
+	}})
+
+	evm3 := commontypes.RelayID{
+		Network: relay.NetworkEVM,
+		ChainID: "3",
+	}
+
+	batcher := chainBatcher{app}
+	keys := dataloader.NewKeysFromStrings([]string{evm2.String(), evm1.String(), evm3.String()})
+	results := batcher.loadByRelayIDs(ctx, keys)
+
+	assert.Len(t, results, 3)
+
+	require.NoError(t, err)
+
+	assert.Equal(t, types.ChainStatusWithID{
+		ChainStatus: commontypes.ChainStatus{ID: "2", Enabled: true, Config: config2},
+		RelayID:     evm2,
+	}, results[0].Data.(types.ChainStatusWithID))
+
+	assert.Equal(t, types.ChainStatusWithID{
+		ChainStatus: commontypes.ChainStatus{ID: "1", Enabled: true, Config: config1},
+		RelayID:     evm1,
+	}, results[1].Data.(types.ChainStatusWithID))
+	assert.Nil(t, results[2].Data)
+	require.Error(t, results[2].Error)
+	require.ErrorIs(t, results[2].Error, chains.ErrNotFound)
+}
+
 func TestLoader_Nodes(t *testing.T) {
 	t.Parallel()
 

core/web/resolver/chain_test.go

@@ -160,6 +160,22 @@ func TestResolver_Chain(t *testing.T) {
 				}
 			}
 		`
+		queryWithNetwork = `
+			query GetChain($network: String)  {
+				chain(id: "1", network: $network) {
+					... on Chain {
+						id
+						enabled
+						config
+						network
+					}
+					... on NotFoundError {
+						code
+						message
+					}
+				}
+			}
+		`
 		configTOML = `ChainID = '1'
 AutoCreateKey = false
 BlockBackfillDepth = 100
@@ -291,6 +307,51 @@ ResendAfterThreshold = '1h0m0s'
 				},
 			},
 		},
+		{
+			name:          "should return aptos chain if network is aptos",
+			authenticated: true,
+			variables: map[string]interface{}{
+				"network": relay.NetworkAptos,
+			},
+			before: func(ctx context.Context, f *gqlTestFramework) {
+				chainConf := evmtoml.EVMConfig{
+					Chain:   chain,
+					ChainID: &chainID,
+				}
+
+				chainConfToml, err2 := chainConf.TOMLString()
+				require.NoError(t, err2)
+
+				f.App.On("GetRelayers").Return(&chainlinkmocks.FakeRelayerChainInteroperators{Relayers: map[commontypes.RelayID]loop.Relayer{
+					commontypes.RelayID{
+						Network: relay.NetworkEVM,
+						ChainID: chainID.String(),
+					}: testutils.MockRelayer{ChainStatus: commontypes.ChainStatus{
+						ID:      chainID.String(),
+						Enabled: chainConf.IsEnabled(),
+						Config:  chainConfToml,
+					}},
+					commontypes.RelayID{
+						Network: relay.NetworkAptos,
+						ChainID: chainID.String(),
+					}: testutils.MockRelayer{ChainStatus: commontypes.ChainStatus{
+						ID:      chainID.String(),
+						Enabled: chainConf.IsEnabled(),
+						Config:  chainConfToml,
+					}},
+				}})
+			},
+			query: queryWithNetwork,
+			result: fmt.Sprintf(`
+				{
+					"chain": {
+						"id": "1",
+						"enabled": true,
+						"config": %s,
+						"network": "aptos"
+					}
+				}`, configTOMLEscaped),
+		},
 	}
 
 	RunGQLTests(t, testCases)

core/web/resolver/eth_key.go

@@ -6,9 +6,12 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/graph-gophers/graphql-go"
 
+	commonTypes "github.com/smartcontractkit/chainlink-common/pkg/types"
+
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/legacyevm"
 	"github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey"
+	"github.com/smartcontractkit/chainlink/v2/core/services/relay"
 	"github.com/smartcontractkit/chainlink/v2/core/web/loader"
 )
 
@@ -37,7 +40,8 @@ func NewETHKeys(keys []ETHKey) []*ETHKeyResolver {
 }
 
 func (r *ETHKeyResolver) Chain(ctx context.Context) (*ChainResolver, error) {
-	chain, err := loader.GetChainByID(ctx, r.key.state.EVMChainID.String())
+	relayID := commonTypes.NewRelayID(relay.NetworkEVM, r.key.state.EVMChainID.String())
+	chain, err := loader.GetChainByRelayID(ctx, relayID.Name())
 	if err != nil {
 		return nil, err
 	}

core/web/resolver/eth_transaction.go

@@ -6,8 +6,11 @@ import (
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/graph-gophers/graphql-go"
 
+	commonTypes "github.com/smartcontractkit/chainlink-common/pkg/types"
+
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr"
+	"github.com/smartcontractkit/chainlink/v2/core/services/relay"
 	"github.com/smartcontractkit/chainlink/v2/core/utils/stringutils"
 	"github.com/smartcontractkit/chainlink/v2/core/web/loader"
 )
@@ -98,7 +101,8 @@ func (r *EthTransactionResolver) Hex(ctx context.Context) string {
 
 // Chain resolves the node's chain object field.
 func (r *EthTransactionResolver) Chain(ctx context.Context) (*ChainResolver, error) {
-	chain, err := loader.GetChainByID(ctx, string(r.EVMChainID()))
+	relayID := commonTypes.NewRelayID(relay.NetworkEVM, string(r.EVMChainID()))
+	chain, err := loader.GetChainByRelayID(ctx, relayID.Name())
 	if err != nil {
 		return nil, err
 	}

core/web/resolver/query.go

@@ -10,6 +10,8 @@ import (
 	"github.com/graph-gophers/graphql-go"
 	"github.com/pkg/errors"
 
+	"github.com/smartcontractkit/chainlink-common/pkg/types"
+
 	commonTypes "github.com/smartcontractkit/chainlink/v2/common/types"
 	"github.com/smartcontractkit/chainlink/v2/core/bridges"
 	"github.com/smartcontractkit/chainlink/v2/core/chains"
@@ -64,12 +66,29 @@ func (r *Resolver) Bridges(ctx context.Context, args struct {
 }
 
 // Chain retrieves a chain by id.
-func (r *Resolver) Chain(ctx context.Context, args struct{ ID graphql.ID }) (*ChainPayloadResolver, error) {
+func (r *Resolver) Chain(ctx context.Context,
+	args struct {
+		ID      graphql.ID
+		Network *string
+	}) (*ChainPayloadResolver, error) {
 	if err := authenticateUser(ctx); err != nil {
 		return nil, err
 	}
 
-	id, err := loader.GetChainByID(ctx, string(args.ID))
+	// fall back to original behaviour if network is not provided
+	if args.Network == nil {
+		id, err := loader.GetChainByID(ctx, string(args.ID))
+		if err != nil {
+			if errors.Is(err, chains.ErrNotFound) {
+				return NewChainPayload(commonTypes.ChainStatusWithID{}, chains.ErrNotFound), nil
+			}
+			return nil, err
+		}
+		return NewChainPayload(*id, nil), nil
+	}
+
+	relayID := types.NewRelayID(*args.Network, string(args.ID))
+	id, err := loader.GetChainByRelayID(ctx, relayID.Name())
 	if err != nil {
 		if errors.Is(err, chains.ErrNotFound) {
 			return NewChainPayload(commonTypes.ChainStatusWithID{}, chains.ErrNotFound), nil

core/web/schema/schema.graphql

@@ -10,7 +10,7 @@ schema {
 type Query {
     bridge(id: ID!): BridgePayload!
     bridges(offset: Int, limit: Int): BridgesPayload!
-    chain(id: ID!): ChainPayload!
+    chain(id: ID!, network: String): ChainPayload!
     chains(offset: Int, limit: Int): ChainsPayload!
     configv2: ConfigV2Payload!
     csaKeys: CSAKeysPayload!