From 090063799cd2fd62646c383bf2d2e42d25225c40 Mon Sep 17 00:00:00 2001 From: Graham Goh Date: Thu, 31 Oct 2024 23:19:37 +0900 Subject: [PATCH] [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 | 7 ++ .../mocks/relayer_chain_interoperators.go | 6 +- core/web/loader/chain.go | 34 +++++++++ core/web/loader/getters.go | 19 +++++ core/web/loader/loader.go | 2 + core/web/loader/loader_test.go | 73 +++++++++++++++++++ core/web/resolver/chain_test.go | 61 ++++++++++++++++ core/web/resolver/eth_key.go | 6 +- core/web/resolver/eth_transaction.go | 6 +- core/web/resolver/query.go | 23 +++++- core/web/schema/schema.graphql | 2 +- .../web/sdk/internal/schema.graphql | 35 ++++++++- 12 files changed, 267 insertions(+), 7 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..7d3ff3c7848 --- /dev/null +++ b/.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 diff --git a/core/services/chainlink/mocks/relayer_chain_interoperators.go b/core/services/chainlink/mocks/relayer_chain_interoperators.go index 13fef0e3180..3ebe1411c19 100644 --- a/core/services/chainlink/mocks/relayer_chain_interoperators.go +++ b/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) { diff --git a/core/web/loader/chain.go b/core/web/loader/chain.go index c6b3206edbd..0c66f55da4b 100644 --- a/core/web/loader/chain.go +++ b/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 +} diff --git a/core/web/loader/getters.go b/core/web/loader/getters.go index 33aba17db36..fef84b5eabd 100644 --- a/core/web/loader/getters.go +++ b/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) diff --git a/core/web/loader/loader.go b/core/web/loader/loader.go index d28874b44f4..4ce2d4d51f2 100644 --- a/core/web/loader/loader.go +++ b/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), diff --git a/core/web/loader/loader_test.go b/core/web/loader/loader_test.go index 0e88f67c444..953c0ff9319 100644 --- a/core/web/loader/loader_test.go +++ b/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() diff --git a/core/web/resolver/chain_test.go b/core/web/resolver/chain_test.go index fed817df456..0199fca3513 100644 --- a/core/web/resolver/chain_test.go +++ b/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) diff --git a/core/web/resolver/eth_key.go b/core/web/resolver/eth_key.go index d986374ec21..fb03730b778 100644 --- a/core/web/resolver/eth_key.go +++ b/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 } diff --git a/core/web/resolver/eth_transaction.go b/core/web/resolver/eth_transaction.go index 1292b6bc104..aab378ffd74 100644 --- a/core/web/resolver/eth_transaction.go +++ b/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 } diff --git a/core/web/resolver/query.go b/core/web/resolver/query.go index 3fe425f290e..ae33e5688bb 100644 --- a/core/web/resolver/query.go +++ b/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 diff --git a/core/web/schema/schema.graphql b/core/web/schema/schema.graphql index 0bf9ae9e71a..568716f7b76 100644 --- a/core/web/schema/schema.graphql +++ b/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! diff --git a/deployment/environment/web/sdk/internal/schema.graphql b/deployment/environment/web/sdk/internal/schema.graphql index 71aa67598da..f459e738c46 100644 --- a/deployment/environment/web/sdk/internal/schema.graphql +++ b/deployment/environment/web/sdk/internal/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! @@ -34,6 +34,8 @@ type Query { p2pKeys: P2PKeysPayload! solanaKeys: SolanaKeysPayload! aptosKeys: AptosKeysPayload! + cosmosKeys: CosmosKeysPayload! + starknetKeys: StarkNetKeysPayload! sqlLogging: GetSQLLoggingPayload! vrfKey(id: ID!): VRFKeyPayload! vrfKeys: VRFKeysPayload! @@ -68,6 +70,8 @@ type Mutation { setSQLLogging(input: SetSQLLoggingInput!): SetSQLLoggingPayload! updateBridge(id: ID!, input: UpdateBridgeInput!): UpdateBridgePayload! updateFeedsManager(id: ID!, input: UpdateFeedsManagerInput!): UpdateFeedsManagerPayload! + enableFeedsManager(id: ID!): EnableFeedsManagerPayload! + disableFeedsManager(id: ID!): DisableFeedsManagerPayload! updateFeedsManagerChainConfig(id: ID!, input: UpdateFeedsManagerChainConfigInput!): UpdateFeedsManagerChainConfigPayload! updateJobProposalSpecDefinition(id: ID!, input: UpdateJobProposalSpecDefinitionInput!): UpdateJobProposalSpecDefinitionPayload! updateUserPassword(input: UpdatePasswordInput!): UpdatePasswordPayload! @@ -195,6 +199,13 @@ type ConfigV2Payload { user: String! effective: String! } +type CosmosKey { + id: ID! +} + +type CosmosKeysPayload { + results: [CosmosKey!]! +} type CSAKey { id: ID! publicKey: String! @@ -324,6 +335,7 @@ type FeedsManager { jobProposals: [JobProposal!]! isConnectionActive: Boolean! createdAt: Time! + disabledAt: Time chainConfigs: [FeedsManagerChainConfig!]! } @@ -492,6 +504,20 @@ type UpdateFeedsManagerChainConfigSuccess { union UpdateFeedsManagerChainConfigPayload = UpdateFeedsManagerChainConfigSuccess | NotFoundError | InputErrors + +type EnableFeedsManagerSuccess { + feedsManager: FeedsManager! +} + +union EnableFeedsManagerPayload = EnableFeedsManagerSuccess + | NotFoundError + +type DisableFeedsManagerSuccess { + feedsManager: FeedsManager! +} + +union DisableFeedsManagerPayload = DisableFeedsManagerSuccess + | NotFoundError type Job { id: ID! name: String! @@ -1012,6 +1038,13 @@ type StandardCapabilitiesSpec { command: String! config: String } +type StarkNetKey { + id: ID! +} + +type StarkNetKeysPayload { + results: [StarkNetKey!]! +} type TaskRun { id: ID! dotID: String!