-
Notifications
You must be signed in to change notification settings - Fork 2
[Concept]: feat: introducing lazy chain loading #644
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 52648b1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // default - loads all chains from the networks config | ||
| chainSelectorsToLoad := cfg.Networks.ChainSelectors() | ||
|
|
||
| if loadcfg.chainSelectorsToLoad != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer care about what chains to load via chainSelectorsToLoad as we no longer load everything.
2c1b033 to
a5255df
Compare
| // Unlike EVMChains, this method returns an error if any chain fails to load. | ||
| // The error may contain multiple chain load failures wrapped together. | ||
| // Successfully loaded chains are still returned in the map. | ||
| func (l *LazyBlockChains) TryEVMChains() (map[uint64]evm.Chain, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use a generic implementation here and reduce a lot of the duplication in this file:
func tryChains[T any, PT *T](l *LazyBlockChains, family string) (map[uint64]T, error) {
l.mu.RLock()
selectors := make([]uint64, 0)
for selector, f := range l.availableChains {
if f == family {
selectors = append(selectors, selector)
}
}
l.mu.RUnlock()
chains := make(map[uint64]T)
var errs []error
for _, selector := range selectors {
chain, err := l.GetBySelector(selector)
if err != nil {
errs = append(errs, fmt.Errorf("failed to load chain %d: %w", selector, err))
continue
}
switch c := chain.(type) {
case T:
chains[selector] = c
case PT:
if c != nil {
chains[selector] = *c
}
}
}
if len(errs) > 0 {
return chains, errors.Join(errs...)
}
return chains, nil
}
// then
func (l *LazyBlockChains) EVMChains() map[uint64]evm.Chain {
chains, err := tryChains[evm.Chain](l, chainsel.FamilyEVM)
if err != nil {
l.lggr.Errorw("Failed to load one or more EVM chains", "error", err)
}
return chains
}
func (l *LazyBlockChains) SolanaChains() map[uint64]solana.Chain {
chains, err := tryChains[solana.Chain](l, chainsel.FamilySolana)
if err != nil {
l.lggr.Errorw("Failed to load one or more Solana chains", "error", err)
}
return chains
}
// etc...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! Good pick up, using generic cleaned it up alot, thanks! I had to use the type constraint for it to work
func tryChains[T any, PT interface {
*T
}](l *LazyBlockChains, family string) (map[uint64]T, error) {
chain/lazy_blockchains.go
Outdated
| chains := make(map[uint64]evm.Chain) | ||
| var errs []error | ||
|
|
||
| for _, selector := range selectors { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this must be done in parallel for the lazy loading to become a viable option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done! another good pick up, updated the other getters to be parallel too!
| blockChains, err := chains.LoadChains(ctx, lggr, cfg, chainSelectorsToLoad) | ||
| // Use lazy loading for chains - they will be initialized on first access | ||
| // All chains from the network config are made available, but only loaded when accessed | ||
| lazyBlockChains, err := chains.NewLazyBlockChains(ctx, lggr, cfg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should feature flag this initially. Nothing too complicated, just check for an environment variable -- say, CLD_LAZY_BLOCK_CHAINS=1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, added a feature toggle.
b71011b to
ec50b62
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces lazy loading of blockchain chains as an opt-in feature controlled by the CLD_LAZY_BLOCKCHAINS environment variable. Chains are now loaded on-demand when accessed rather than eagerly at initialization, improving startup performance and allowing users to avoid providing secrets for unused chains.
Key changes:
- Introduces
LazyBlockChainstype that implements on-demand chain loading with caching - Adds
BlockChainCollectioninterface implemented by bothBlockChainsandLazyBlockChains - Updates the
Environmenttype to useBlockChainCollectionfor flexibility - Adds comprehensive test coverage for the new lazy loading functionality
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| chain/lazy_blockchains.go | New implementation of lazy chain loading with thread-safe caching |
| chain/lazy_blockchains_test.go | Comprehensive test suite for lazy loading behavior |
| chain/blockchain.go | Adds BlockChainCollection interface to support both eager and lazy loading |
| engine/cld/chains/chains.go | Adds NewLazyBlockChains function and ChainLoader type alias |
| engine/cld/environment/environment.go | Adds lazy loading path with feature flag check |
| engine/cld/environment/environment_test.go | Tests for both lazy and eager loading modes |
| deployment/environment.go | Updates Environment type to use BlockChainCollection interface |
| engine/test/environment/environment_test.go | Updates type references to BlockChainCollection |
| engine/cld/legacy/cli/mcmsv2/mcms_v2.go | Updates type reference to BlockChainCollection |
| experimental/analyzer/evm_analyzer_test.go | Updates test fixtures to use empty BlockChains instances |
| .changeset/empty-words-tickle.md | Changeset documenting the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| l.mu.RLock() | ||
| if chain, ok := l.loadedChains[selector]; ok { | ||
| l.mu.RUnlock() | ||
| return chain, nil | ||
| } | ||
| l.mu.RUnlock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could have this as a helper functions that Rlocks and defers the unlock instead of having to cover the unlock in 2 different paths 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, i think since this is the only scenario, some verbosity is fine, if there are more usage, of this pattern, then i can refactor. This so far still seem readable.
|
|
||
| // EVMChains returns a map of all EVM chains, loading them lazily. | ||
| // If a chain fails to load, the error is logged and the chain is skipped. | ||
| func (l *LazyBlockChains) EVMChains() map[uint64]evm.Chain { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these chain specific functions be pushed down to the chain implementation? Just thinking that if we want to make lazy load the default loading method we will need to have these function with every new chain family we integrate therefore keeps all that we need to integrate within the same /chain package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm , can you elaborate more? Each of these functions eventually calls tryChains which is a generic chain loading function, dont think we want to move that into the chain specific package?
need to have these function with every new chain family
yeah because the design is not to have a single interface that can generically represent all chains which are too different from each other.
| EVMChains() map[uint64]evm.Chain | ||
| SolanaChains() map[uint64]solana.Chain | ||
| AptosChains() map[uint64]aptos.Chain | ||
| SuiChains() map[uint64]sui.Chain | ||
| TonChains() map[uint64]ton.Chain | ||
| TronChains() map[uint64]tron.Chain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to make this more generic? Just thinking every time we want to add a new chain family we will need to update the interface 🤔 Instead if we could have some Chains generic map that could accept any type of chain could be useful (although not sure how practical is this because of how different the <chain_family>.Chain structs are 🤔 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question, when i initially designed the blockchain type, this was one of the question, then we come to a conclusion where we shouldnt try to make it generic since each chain is so different.
I also remember there was a doc somewhere in Chainlink channel from Connor about the attempt to consolidate the chains, basically it says dont since they are so different.
ec50b62 to
3ee45b9
Compare
Chains are loaded lazily instead of eager, this means chains will only be loaded when it is being used, this means users are not forced to provide any secrets for chains if they are not used upfront and avoid loading huge amount of chains that are never used. This change also means we can deprecate `ChainOverrides` feature as we no longer have to tell CLD what chains to load. This lazy loading is hidden under the feature flag CLD_LAZY_BLOCKCHAINS
3ee45b9 to
52648b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert.IsType(t, &fchain.LazyBlockChains{}, env.BlockChains, "Expected LazyBlockChains instance") | ||
| } | ||
|
|
||
| func Test_Load_WithEagerBlockchains(t *testing.T) { |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test function name uses inconsistent casing: 'Blockchains' should be 'BlockChains' to match the terminology used throughout the codebase.
|


Chains can now be loaded lazily instead of eager, this means chains will only be loaded when it is being used.
Hidden under the feature toggle
CLD_LAZY_BLOCKCHAINS, off by default.Basically this solves a bunch of issues
ChainsOverridesfeature, simplifying the code