Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BCF-2317 One relayer per chain id #9788

Merged
merged 86 commits into from
Aug 17, 2023
Merged

BCF-2317 One relayer per chain id #9788

merged 86 commits into from
Aug 17, 2023

Conversation

krehermann
Copy link
Contributor

@krehermann krehermann commented Jul 12, 2023

This change removes chainset from the core Application api. This enables 1:1 relayer:chain, which is a necessary step to encapsulate each chain in a LOOPP.

The Key ideas:

  • introduce RelayerChainInteroperator which more or less replaces chainset

    • This interface provides a clean split between access to loop.Relayer and newly introduced LegacyChains
    • The implementation still internal uses chainsets, but each chainset is explicitly scoped to one chain. This is an implementation detail that enables this refactor to use existing code and it can and will be refactored away in the future.
  • Adds Identity to relayer as (name, id) tuple eg "solana", "solana-chain-abc" which is used consistently in the api

Most of the changes are due to the many, many tentacles of chainset through tests

Note: this change will spawn one relayer per chain id even if the chain id is not used by a job. We may want to implement lazy relayer initialization.

First sketch...

Adds plumbing for Solana only to get quick feedback

Key ideas:
- Identify relayer as (name, id) tuple eg "solana", "solana-chain-abc"
- Create one relayer per identifier and plumb a map[relay_id]relayer to delegate
- Use the existing job spec to recreate the relayer_id and do the lookup

Note: this change will spawn one relayer per chain id even if the chain id is not used by a job. We may want to implement lazy relayer initialization.
@github-actions
Copy link
Contributor

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

relayID := relay.Identifier{Network: spec.Relay, ChainID: chainID}
relayer, err := d.RelayGetter.Get(relayID)
if err != nil {
return nil, fmt.Errorf("failed to get relay %s is it enabled?: %w", spec.Relay, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to the scope of this PR, but would this realistically ever happen? Is there job spec validation that should protect against this?

Comment on lines 266 to 318
{name: "all chains",
initFuncs: []chainlink.CoreRelayerChainInitFunc{chainlink.InitSolana(testctx, factory, chainlink.SolanaFactoryConfig{
Keystore: keyStore.Solana(),
SolanaConfigs: cfg.SolanaConfigs()},
),
chainlink.InitEVM(testctx, factory, chainlink.EVMFactoryConfig{
RelayerConfig: evm.RelayerConfig{
GeneralConfig: cfg,
EventBroadcaster: pg.NewNullEventBroadcaster(),
MailMon: &utils.MailboxMonitor{},
},
CSAETHKeystore: keyStore,
}),
chainlink.InitStarknet(testctx, factory, chainlink.StarkNetFactoryConfig{
Keystore: keyStore.StarkNet(),
StarknetConfigs: cfg.StarknetConfigs()},
),
chainlink.InitCosmos(testctx, factory, chainlink.CosmosFactoryConfig{
Keystore: keyStore.Cosmos(),
CosmosConfigs: cfg.CosmosConfigs(),
EventBroadcaster: pg.NewNullEventBroadcaster(),
},
),
},
expectedEVMChainCnt: 2,
expectedEVMNodeCnt: 3,
expectedEVMRelayerIds: []relay.Identifier{
{Network: relay.EVM, ChainID: relay.ChainID(evmChainID1.String())},
{Network: relay.EVM, ChainID: relay.ChainID(evmChainID2.String())},
},

expectedSolanaChainCnt: 2,
expectedSolanaNodeCnt: 2,
expectedSolanaRelayerIds: []relay.Identifier{
{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID1)},
{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID2)},
},

expectedStarknetChainCnt: 2,
expectedStarknetNodeCnt: 4,
expectedStarknetRelayerIds: []relay.Identifier{
{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID1)},
{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID2)},
},

expectedCosmosChainCnt: 2,
expectedCosmosNodeCnt: 2,
expectedCosmosRelayerIds: []relay.Identifier{
{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID1)},
{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID2)},
},
},
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/nit formatting

Suggested change
{name: "all chains",
initFuncs: []chainlink.CoreRelayerChainInitFunc{chainlink.InitSolana(testctx, factory, chainlink.SolanaFactoryConfig{
Keystore: keyStore.Solana(),
SolanaConfigs: cfg.SolanaConfigs()},
),
chainlink.InitEVM(testctx, factory, chainlink.EVMFactoryConfig{
RelayerConfig: evm.RelayerConfig{
GeneralConfig: cfg,
EventBroadcaster: pg.NewNullEventBroadcaster(),
MailMon: &utils.MailboxMonitor{},
},
CSAETHKeystore: keyStore,
}),
chainlink.InitStarknet(testctx, factory, chainlink.StarkNetFactoryConfig{
Keystore: keyStore.StarkNet(),
StarknetConfigs: cfg.StarknetConfigs()},
),
chainlink.InitCosmos(testctx, factory, chainlink.CosmosFactoryConfig{
Keystore: keyStore.Cosmos(),
CosmosConfigs: cfg.CosmosConfigs(),
EventBroadcaster: pg.NewNullEventBroadcaster(),
},
),
},
expectedEVMChainCnt: 2,
expectedEVMNodeCnt: 3,
expectedEVMRelayerIds: []relay.Identifier{
{Network: relay.EVM, ChainID: relay.ChainID(evmChainID1.String())},
{Network: relay.EVM, ChainID: relay.ChainID(evmChainID2.String())},
},
expectedSolanaChainCnt: 2,
expectedSolanaNodeCnt: 2,
expectedSolanaRelayerIds: []relay.Identifier{
{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID1)},
{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID2)},
},
expectedStarknetChainCnt: 2,
expectedStarknetNodeCnt: 4,
expectedStarknetRelayerIds: []relay.Identifier{
{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID1)},
{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID2)},
},
expectedCosmosChainCnt: 2,
expectedCosmosNodeCnt: 2,
expectedCosmosRelayerIds: []relay.Identifier{
{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID1)},
{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID2)},
},
},
}
{name: "all chains",
initFuncs: []chainlink.CoreRelayerChainInitFunc{
chainlink.InitSolana(testctx, factory, chainlink.SolanaFactoryConfig{
Keystore: keyStore.Solana(),
SolanaConfigs: cfg.SolanaConfigs(),
}),
chainlink.InitEVM(testctx, factory, chainlink.EVMFactoryConfig{
RelayerConfig: evm.RelayerConfig{
GeneralConfig: cfg,
EventBroadcaster: pg.NewNullEventBroadcaster(),
MailMon: &utils.MailboxMonitor{},
},
CSAETHKeystore: keyStore,
}),
chainlink.InitStarknet(testctx, factory, chainlink.StarkNetFactoryConfig{
Keystore: keyStore.StarkNet(),
StarknetConfigs: cfg.StarknetConfigs(),
}),
chainlink.InitCosmos(testctx, factory, chainlink.CosmosFactoryConfig{
Keystore: keyStore.Cosmos(),
CosmosConfigs: cfg.CosmosConfigs(),
EventBroadcaster: pg.NewNullEventBroadcaster(),
}),
},
expectedEVMChainCount: 2,
expectedEVMNodeCount: 3,
expectedEVMRelayerIds: []relay.Identifier{
{Network: relay.EVM, ChainID: relay.ChainID(evmChainID1.String())},
{Network: relay.EVM, ChainID: relay.ChainID(evmChainID2.String())},
},
expectedSolanaChainCount: 2,
expectedSolanaNodeCount: 2,
expectedSolanaRelayerIds: []relay.Identifier{
{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID1)},
{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID2)},
},
expectedStarknetChainCount: 2,
expectedStarknetNodeCount: 4,
expectedStarknetRelayerIds: []relay.Identifier{
{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID1)},
{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID2)},
},
expectedCosmosChainCount: 2,
expectedCosmosNodeCount: 2,
expectedCosmosRelayerIds: []relay.Identifier{
{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID1)},
{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID2)},
},
},

type Chains interface {
// ChainStatuser is a generic interface for chain configuration.
type ChainStatuser interface {
// must return [ErrNotFound] if the id is not found
ChainStatus(ctx context.Context, id string) (types.ChainStatus, error)
ChainStatuses(ctx context.Context, offset, limit int) ([]types.ChainStatus, int, error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope for this PR, but documentation here would be helpful to me. How are offset and limit related to Chain Statuses? When concrete types implicitly satisfy this interface, what are some basic rules for how offset and limit can be used?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the eventual goal to migrate all the functionality in chain_set to chain_kv? Worth adding a README.md here (i.e. core/chains/README.md) and quickly documenting the different use cases?

patrickhuie19
patrickhuie19 previously approved these changes Aug 16, 2023
Copy link
Contributor

@patrickhuie19 patrickhuie19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legend 🚀

Had a few comments around documentation that I would find helpful, but certainly not blocking.

patrickhuie19
patrickhuie19 previously approved these changes Aug 16, 2023
@cl-sonarqube-production
Copy link

SonarQube Quality Gate

Quality Gate failed

Failed condition 77.8% 77.8% Coverage on New Code (is less than 80%)

See analysis details on SonarQube

@krehermann krehermann added this pull request to the merge queue Aug 16, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 16, 2023
@krehermann krehermann added this pull request to the merge queue Aug 16, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 16, 2023
@krehermann krehermann added this pull request to the merge queue Aug 16, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 17, 2023
@krehermann krehermann added this pull request to the merge queue Aug 17, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 17, 2023
@krehermann krehermann added this pull request to the merge queue Aug 17, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 17, 2023
@jmank88 jmank88 added this pull request to the merge queue Aug 17, 2023
Merged via the queue into develop with commit 77f0074 Aug 17, 2023
92 of 93 checks passed
@jmank88 jmank88 deleted the BCF-2317_relayer_chain_id branch August 17, 2023 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants