chore: remove deprecated common MCMSWithTimelockState#22641
Conversation
There was a problem hiding this comment.
Moved to Keystone internal since we don't want to disrupt their state output, but they ar e the last consumers of this deprecated function
396de46 to
71c277c
Compare
CORA - Pending ReviewersAll codeowners have approved! ✅ Legend: ✅ Approved | ❌ Changes Requested | 💬 Commented | 🚫 Dismissed | ⏳ Pending | ❓ Unknown For more details, see the full review summary. |
|
✅ No conflicts with other open PRs targeting |
Drop the deprecated `MCMSWithTimelockState` and `MaybeLoadMCMSWithTimelock*` helpers from `deployment/common/changeset`. The keystone changeset was the last internal consumer, so moved the struct into deployment/keystone/changeset/internal as a private type. Other callers (vault, ccip save_existing_test) now use the canonical `cld-changesets/legacy/pkg/family/evm` package.
8d3368d to
cd1658b
Compare
|
There was a problem hiding this comment.
Pull request overview
Risk Rating: LOW
This PR removes deprecated MCMS-with-timelock helpers from deployment/common/changeset usage sites, updating remaining consumers to use either the canonical cld-changesets/legacy/pkg/family/evm loader or a keystone-specific internal state type.
Changes:
- Switch vault + CCIP test code to load MCMS-with-timelock state via
cld-changesets/legacy/pkg/family/evm. - Move keystone’s last-use MCMS-with-timelock state loader/type into
deployment/keystone/changeset/internaland update keystone callers/tests accordingly. - Remove an unused internal struct in keystone changeset internals.
Targeted areas for scrupulous human review:
- Confirm
evmstate.MaybeLoadMCMSWithTimelockChainState(fromcld-changesets/legacy) is API/behavior compatible with the previous helper for the vault validation paths. - Confirm the keystone internal MCMS state loader continues to use the intended wrapper bindings (
ccip-owner-contracts/pkg/gethwrappers) and that no external packages need to import it (Gointernal/visibility).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| deployment/vault/changeset/validation.go | Swap MCMS state loading to cld-changesets/legacy EVM helper. |
| deployment/keystone/changeset/state.go | Use keystone internal MCMS state type/loader instead of common changeset. |
| deployment/keystone/changeset/internal/state.go | Update embedded MCMS state type and loader call to local internal implementation. |
| deployment/keystone/changeset/internal/state_test.go | Update expected MCMS state type in tests. |
| deployment/keystone/changeset/internal/state_mcms.go | Move MCMS state type/loader into keystone internal package and drop deprecated helpers. |
| deployment/keystone/changeset/internal/state_mcms_test.go | Update test package name to match internal package move. |
| deployment/keystone/changeset/internal/contract_set.go | Remove unused deployContractsRequest struct. |
| deployment/ccip/changeset/save_existing_test.go | Switch MCMS state loading in test to cld-changesets/legacy helper. |
Comments suppressed due to low confidence (1)
deployment/keystone/changeset/internal/state_mcms.go:22
- This comment says the type is "public for use in product specific packages", but it now lives under an
internalpackage and cannot be imported outsidedeployment/keystone/changeset. Updating the docstring would avoid misleading guidance.




Drop the deprecated
MCMSWithTimelockStateandMaybeLoadMCMSWithTimelock*helpers fromdeployment/common/changeset.The keystone changeset was the last internal consumer, so moved the struct into deployment/keystone/changeset/internal as a private type. Other callers (vault, ccip save_existing_test) now use the canonical
cld-changesets/legacy/pkg/family/evmpackage.