Conversation
|
Code coverage report:
WARNING: go tool cover failed for coverage_target.out |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR relocates the concrete EVM chain configuration types (Info/Node) out of the generic integration/pkg/blockchain package and into the EVM accessor package, updating executables and devenv utilities to use evm.Info while keeping blockchain.Infos[T] as a generic map helper.
Changes:
- Moved EVM chain config structs from
integration/pkg/blockchainintointegration/pkg/accessors/evm(evm.Info,evm.Node). - Updated verifier/executor entrypoints and devenv config generation to use
evm.Infoin generic config wrappers. - Moved EVM source reader implementation under the
evmaccessor package and updated constructor call sites accordingly.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| verifier/cmd/token/main.go | Switch token verifier wiring from blockchain.Info to evm.Info. |
| verifier/cmd/committee/main.go | Switch committee verifier factory generics and deprecated wrapper to evm.Info. |
| integration/pkg/constructors/committee_verifier.go | Use evm.NewEVMSourceReader instead of sourcereader.NewEVMSourceReader. |
| integration/pkg/blockchain/info_test.go | Adjust tests to use a local generic test struct rather than removed blockchain.Info. |
| integration/pkg/blockchain/info.go | Remove concrete Info/Node, leaving only generic Infos[T] helpers. |
| integration/pkg/accessors/evm/multi_node_client.go | Move multinode client helpers into package evm and update signatures to Infos[evm.Info]. |
| integration/pkg/accessors/evm/info.go | New home for EVM Info/Node and helper methods. |
| integration/pkg/accessors/evm/factory_constructor.go | Update accessor factory constructor to use local multinode helper + Infos[evm.Info]. |
| integration/pkg/accessors/evm/factory.go | Update EVM accessor factory to store Infos[evm.Info] and construct source readers in-package. |
| integration/pkg/accessors/evm/evm_source_reader.go | Move EVM source reader implementation into package evm and rename struct to SourceReader. |
| integration/pkg/accessors/evm/evm_factory.go | Move EVM factory into package evm and update generics to Infos[evm.Info]. |
| cmd/executor/main.go | Use evm.CreateMultiNodeClientFromInfo and decode blockchain infos as Infos[evm.Info]. |
| build/devenv/services/tokenVerifier.go | Update token verifier config generation to accept Infos[evm.Info]. |
| build/devenv/services/executor.go | Update executor config generation + filtering to use Infos[evm.Info]. |
| build/devenv/services/common.go | Update conversion from CTF outputs to return Infos[evm.Info]. |
| build/devenv/services/chainconfig/evm.go | Update chainconfig loader to produce *evm.Info values. |
| build/devenv/go.mod | Add indirect chainlink-framework deps and pin ugorji/go/codec to v1.2.12 via replace. |
| build/devenv/go.sum | Record sums for newly added deps and the pinned codec version. |
Comments suppressed due to low confidence (2)
integration/pkg/accessors/evm/evm_source_reader.go:27
- The comment reads "SourceReader implements the SourceReader interface", which is tautological and ambiguous given the
chainaccess.SourceReaderinterface name. Update it to explicitly referencechainaccess.SourceReader(or remove it) to avoid confusion.
integration/pkg/accessors/evm/multi_node_client.go:25 - If
GetBlockchainByChainSelectorreturns an error, the function currently just logs it and continues with a zero-valueInfo, which can mask the real cause (and lead to a misleading "no nodes found" error). Return the lookup error immediately instead of proceeding.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
20
to
32
| func TestHelper_GetBlockchainByChainSelector_NilMapEntriesTreatedAsNotFound(t *testing.T) { | ||
| validInfo := Info{ChainID: "123", Type: "evm", Family: "evm", UniqueChainName: "chain-123"} | ||
| validInfo := TestInfo{ChainID: "123", Type: "evm", Family: "evm", UniqueChainName: "chain-123"} | ||
| selector := protocol.ChainSelector(999) | ||
| tests := []struct { | ||
| name string | ||
| infos map[string]Info | ||
| infos Infos[TestInfo] | ||
| wantErr bool | ||
| }{ | ||
| { | ||
| name: "returns error when key exists but value is nil", | ||
| infos: map[string]Info{"999": {}}, | ||
| infos: Infos[TestInfo]{"999": {}}, | ||
| wantErr: false, // With generic types, this isn't something we can test for. | ||
| }, |
There was a problem hiding this comment.
This test name and the first sub-case are now misleading: map values are not pointers and GetBlockchainByChainSelector only checks key existence, so there is no longer a meaningful "nil map entry" scenario here. Consider renaming the test and removing/reworking the first case so it reflects the current behavior being validated.
| func ConvertBlockchainOutputsToInfo(outputs []*ctfblockchain.Output) (ccvblockchain.Infos[ccvblockchain.Info], error) { | ||
| infos := make(map[string]ccvblockchain.Info) | ||
| func ConvertBlockchainOutputsToInfo(outputs []*ctfblockchain.Output) (ccvblockchain.Infos[evm.Info], error) { | ||
| infos := make(map[string]evm.Info) |
There was a problem hiding this comment.
Infos[T] is a named map type, so infos := make(map[string]evm.Info) cannot be returned as ccvblockchain.Infos[evm.Info] (will not compile). Define infos as make(ccvblockchain.Infos[evm.Info]) (or cast on return) so the return type matches.
Suggested change
| infos := make(map[string]evm.Info) | |
| infos := make(ccvblockchain.Infos[evm.Info]) |
makramkd
approved these changes
Mar 27, 2026
carte7000
approved these changes
Mar 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Attempt to better compartmentalize EVM specific code by moving several evm specific types, functions and services out of the general integration package and into the evm accessor package.