Remove chain selectors params from agg/indexer cs#1893
Conversation
carte7000
requested review from
AmrMohamedRezk,
agusaldasoro,
asoliman92,
b-gopalswami,
Copilot,
makramkd,
ogtownsend and
winder
There was a problem hiding this comment.
Pull request overview
This PR removes explicit chain selector parameters from the v1_7_0 aggregator/indexer config changesets, shifting selection logic to topology (aggregator) and environment chains (indexer) while improving how undeployed verifier addresses are handled.
Changes:
- Remove
ChainSelectorsfromGenerateIndexerConfigInputandGenerateAggregatorConfigInput. - For aggregator config generation, derive chain selectors from the provided committee topology (and validate it).
- For indexer config generation, ignore per-chain “missing addresses” errors and fail if no addresses exist across all environment chains for a qualifier.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| deployment/v1_7_0/changesets/generate_indexer_config.go | Removes selector filtering and adds logic to skip missing-address errors, with new “no deployed addresses” behavior. |
| deployment/v1_7_0/changesets/generate_indexer_config_test.go | Updates tests to match new indexer input shape and behavior. |
| deployment/v1_7_0/changesets/generate_aggregator_config.go | Replaces selector input with topology-driven chain selection and validation. |
| deployment/v1_7_0/changesets/generate_aggregator_config_test.go | Updates aggregator tests to build selectors from topology and adds validation cases. |
| deployment/v1_7_0/adapters/indexer_config.go | Introduces a typed “missing addresses” error and a registry helper to detect adapters. |
| ccv/chains/evm/deployment/v1_7_0/adapters/indexer_config_adapter.go | Switches EVM adapter to return the new typed missing-address error. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| addrs, err := adapter.ResolveVerifierAddresses(ds, sel, qualifier, kind) | ||
| if err != nil { | ||
| var missingErr *adapters.MissingIndexerVerifierAddressesError | ||
| if errors.As(err, &missingErr) { |
There was a problem hiding this comment.
Because GenerateIndexerConfig now always uses env.BlockChains.ListChainSelectors(), environments containing chain families without a registered IndexerConfigAdapter will fail hard at registry.GetByChain(sel). This is a regression from being able to restrict selectors via input. Consider treating “no adapter registered for chain family” as a non-fatal condition (skip that selector), ideally via a typed error from GetByChain / a HasAdapterForChain helper, so mixed-family environments can still generate indexer config for supported chains.
| addrs, err := adapter.ResolveVerifierAddresses(ds, sel, qualifier, kind) | ||
| if err != nil { | ||
| var missingErr *adapters.MissingIndexerVerifierAddressesError | ||
| if errors.As(err, &missingErr) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
The new behavior that ignores adapters.MissingIndexerVerifierAddressesError via errors.As isn’t exercised by the current tests (the mock adapter returns (nil, nil) on missing data). Add a test case where the adapter returns MissingIndexerVerifierAddressesError for one selector to ensure the changeset correctly skips that chain and still succeeds when other chains provide addresses.
|
No description provided.