chore: port mcms helpers from chainlink/deployment#923
Conversation
🦋 Changeset detectedLatest commit: fcb9b90 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 |
ecPablo
force-pushed
the
ecpablo/port-mcms-helpers-core
branch
from
1302167 to
4dc6446
Compare
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| family, err := chainsel.GetSelectorFamily(uint64(chainSelector)) | ||
| if err != nil { | ||
| return fmt.Errorf("[ExecuteMCMSProposalV2] failed to get chain family for selector %d: %w", chainSelector, err) | ||
| } | ||
|
|
There was a problem hiding this comment.
ExecuteMCMSProposalV2 determines the chain family but only confirms transactions for EVM and Aptos; all other families (e.g., TON, TRON, Sui) will silently skip confirmation, which can leave tests in an incorrect state (transactions unconfirmed) while still returning nil. Consider switching on family with an explicit default error, or routing confirmation through a shared helper similar to engine/test/internal/mcmsutils/executor.go:360-392 so each supported family is handled intentionally (including NOOP where appropriate, e.g., Solana).
| family, err := chainsel.GetSelectorFamily(uint64(op.ChainSelector)) | ||
| if err != nil { | ||
| return fmt.Errorf("[ExecuteMCMSProposalV2] failed to get chain family for selector %d: %w", op.ChainSelector, err) | ||
| } | ||
|
|
There was a problem hiding this comment.
Same issue as the SetRoot loop: after computing family, the code only has confirmation branches for EVM and Aptos. For any other chain family present in the proposal, the operation tx will not be confirmed and the helper may still report success. Add an explicit default branch (or shared confirmation helper) so non-(EVM/Aptos/Solana) families don’t get silently skipped.
| return string(role) | ||
| } | ||
|
|
||
| type MCMSWithTimelockConfig struct { |
There was a problem hiding this comment.
We still have code using this old one?
|
|
||
| if tt.wantErr != "" { | ||
| require.Error(t, err) | ||
| assert.Contains(t, err.Error(), tt.wantErr) |
There was a problem hiding this comment.
Can use ErrorContains
|
|
||
| if tt.wantErr != "" { | ||
| require.Error(t, err) | ||
| assert.Contains(t, err.Error(), tt.wantErr) |
There was a problem hiding this comment.
Can use ErrorContains
| mcmstypes "github.com/smartcontractkit/mcms/types" | ||
| ) | ||
|
|
||
| func TransactionForChain( |
There was a problem hiding this comment.
should we add some godoc comments for this and BatchOperationForChain? Can include info such as chain supported?
|
|
||
| if tt.wantErr != "" { | ||
| require.Error(t, err) | ||
| assert.Contains(t, err.Error(), tt.wantErr) |
There was a problem hiding this comment.
Can use ErrorContains
|
|
||
| var opts mcmsInspectorOptions | ||
| WithTimelockAction(tt.action)(&opts) | ||
| assert.Equal(t, tt.wantAction, opts.TimelockAction) |
There was a problem hiding this comment.
i wonder if we can just get rid of wantAction and use action ?
| assert.Equal(t, tt.wantAction, opts.TimelockAction) | |
| assert.Equal(t, tt.action, opts.TimelockAction) |
|
|
||
| type MCMSInspectorOption func(*mcmsInspectorOptions) | ||
|
|
||
| func WithTimelockAction(action mcmstypes.TimelockAction) MCMSInspectorOption { |
There was a problem hiding this comment.
some go docs for these exported methods would be helpful?
We are porting the code in https://github.com/smartcontractkit/chainlink/blob/develop/deployment/common/proposalutils/mcms_helpers.go to CLDF in order to remove deps on core repo for some downstream consumers. The functions were splitted in separate files to keep them under semantically meaningful names. Also added unit tests and fixed some obvious issues pointed out by copilot, but in general keeping the logic as close as possible to what we have in core today.
McmsInspectorForChainin core repo -> moved toexperimental/proposalutils/inspectors.goin CLDF.BatchOperationForChainandTransactionForChainmoved toexperimental/proposalutils/operations.goin CLDFtest_helpers.goandtypes.goare moved as is from core repo to CLDFFollowup PRs in core will attempt to replace usages with these. So we can delete the
mcms_helpers.gofrom coreAI Summary
This pull request ports the
proposalutilshelpers from thechainlink/deploymentrepository into thechainlink-deployments-framework, making them part of the framework and improving proposal creation, inspection, and testing across multiple blockchain environments. The main changes include adding utility functions for proposal operations, inspectors, and test helpers, as well as defining relevant types and constants for MCMS (Many Chain Multisig) and timelock configurations.New proposal utilities and helpers:
experimental/proposalutils/inspectors.goproviding functions to build MCMS inspectors for different chains, supporting optional timelock actions and mass inspector creation.experimental/proposalutils/operations.gowith helpers to create transactions and batch operations for different chain families (EVM, Solana), abstracting over chain-specific details.Testing support:
experimental/proposalutils/test_helpers.gocontaining test helpers for signing and executing MCMS proposals and timelock proposals, as well as generating single-group configs and finding call proxy addresses for tests.Types and constants:
experimental/proposalutils/types.godefining types and constants for MCMS roles, contract types, and configuration structures (including gas boost config), supporting both legacy and new MCMS config formats.Changelog: