fix: add support for getting signers in all chain families on UPF proposal#779
fix: add support for getting signers in all chain families on UPF proposal#779
Conversation
🦋 Changeset detectedLatest commit: a69b6af 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the UPF conversion command’s signer discovery to work across multiple chain families by switching from an EVM-only inspector construction flow to the generic mcms/chainwrappers inspector builder, and documents the fix via a changeset.
Changes:
- Replace EVM-specific inspector creation with
adapters.Wrap(...)+chainwrappers.BuildInspectors(...)to support non-EVM chain families ingetProposalSigners. - Remove the EVM-only
getInspectorFromProposalConfighelper. - Add a changeset describing the patch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| engine/cld/commands/mcms/cmd_convert_upf.go | Switch signer lookup to use generic chain inspectors built from wrapped chains, enabling multi-chain-family support. |
| .changeset/quick-keys-fix.md | Adds release note entry for the UPF conversion multi-chain signer support fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
graham-chainlink
dismissed
their stale review
The copilot comment is worth fixing
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "chainlink-deployments-framework": patch | ||
| --- | ||
|
|
||
| fix convert UPF to support all non evm chain families in getProposalSigners |
There was a problem hiding this comment.
The changeset summary line is inconsistent with the other changesets in this repo (capitalization) and uses “non evm” instead of the common “non-EVM”. Since this becomes part of the published changelog, please adjust the wording to match existing changeset style.
| fix convert UPF to support all non evm chain families in getProposalSigners | |
| Convert UPF to support all non-EVM chain families in getProposalSigners |
| wrappedChains := adapters.Wrap(cfg.Env.BlockChains) | ||
| inspectors, err := chainwrappers.BuildInspectors(&wrappedChains, cfg.TimelockProposal.ChainMetadata, cfg.TimelockProposal.Action) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("building inspectors: %w", err) | ||
| } | ||
|
|
||
| for chainSelector, metadata := range chainMeta { | ||
| inspector, err := getInspectorFromProposalConfig(cfg, uint64(chainSelector)) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("get inspector for selector %d: %w", chainSelector, err) | ||
| inspector, ok := inspectors[chainSelector] | ||
| if !ok { | ||
| return nil, fmt.Errorf("no inspector found for chain selector %d", chainSelector) | ||
| } | ||
|
|
||
| config, err := inspector.GetConfig(ctx, metadata.MCMAddress) |
There was a problem hiding this comment.
This change switches signer discovery to chainwrappers.BuildInspectors + adapters.Wrap to support non-EVM chains, but there are no tests validating that getProposalSigners returns signers correctly for at least one non-EVM chain family (or that the inspector map is built as expected). Adding a unit/integration test here would help prevent regressions across chain families.
|
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## chainlink-deployments-framework@0.82.1 ### Patch Changes - [#779](#779) [`139427a`](139427a) Thanks [@ecPablo](https://github.com/ecPablo)! - fix convert UPF to support all non evm chain families in getProposalSigners --------- Co-authored-by: app-token-issuer-engops[bot] <144731339+app-token-issuer-engops[bot]@users.noreply.github.com>
The UPF proposal generation logic was not properly generating inspectors for all chain families. This pr fixes the issue by using the chainwrappers pkgs from mcms lib.
AI Summary
This pull request updates the
getProposalSignerslogic in the UPF conversion command to support all non-EVM chain families, not just EVM chains. The main changes involve replacing the EVM-specific inspector creation with a more generic approach that works for multiple chain types.Expanded chain support:
getProposalSignersincmd_convert_upf.goto usechainwrappers.BuildInspectorsandadapters.Wrap, allowing proposal signers to be fetched for all supported chain families, not just EVM.getInspectorFromProposalConfigfunction, eliminating the previous restriction to EVM chains.Dependency updates:
cmd_convert_upf.goto remove EVM-specific dependencies and add generic chain wrapper and adapter imports, supporting the new multi-chain logic.Changelog: