feat!: remove EVM MCMS view generation#77
Conversation
|
👋 jkongie, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR removes EVM MCMS “view generation” code (and associated legacy APIs/tests) while preserving timelock role usage by relocating role name/ID constants into a shared internal package.
Changes:
- Deleted
pkg/contract/mcms/view/v1_0and removedGenerateMCMSWithTimelockViewusage in legacy state/tests. - Introduced
internal/mcmsroleto centralizeRBACTimelockrole IDs/names (with unit tests). - Updated timelock role-grant/deploy flows to reference
internal/mcmsroleinstead of the removed view package.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/family/evm/sequences/grant_roles_for_timelock.go | Switched role constants to internal/mcmsrole for inspector role-member checks. |
| pkg/contract/mcms/view/v1_0/mcms.go | Removed legacy MCMS/timelock/call-proxy view generation code. |
| legacy/pkg/family/evm/state.go | Removed the legacy state method that generated MCMS-with-timelock views. |
| legacy/pkg/family/evm/state_test.go | Removed tests covering the deleted view-generation method. |
| legacy/pkg/family/evm/changesets/deploy_mcms_with_timelock.go | Updated deploy/grant-role logic to use internal/mcmsrole IDs/names. |
| legacy/pkg/family/evm/changesets/deploy_mcms_test.go | Removed JSON marshal/view-generation assertions that depended on deleted view code. |
| internal/mcmsrole/role.go | Added role constants + keccak256 role ID computation. |
| internal/mcmsrole/role_test.go | Added tests validating role ID hashes and predefined role values. |
| go.mod | Promoted golang.org/x/crypto to a direct dependency (used by internal/mcmsrole). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for _, roleAndAddress := range in.RolesAndAddresses { | ||
| switch roleAndAddress.Role { | ||
| case v1_0.PROPOSER_ROLE.ID: | ||
| case mcmsrole.ProposerRole.ID: |
Delete `pkg/contract/mcms/view/v1_0` and `GenerateMCMSWithTimelockView`. Role grant and deploy logic still reference `RBACTimelock` roles, so those constants are split into `internal/mcmsrole` instead of living alongside view code.
|
BREAKING CHANGE: Delete
pkg/contract/mcms/view/v1_0andGenerateMCMSWithTimelockView.Role grant and deploy logic still reference
RBACTimelockroles, so those constants are split intointernal/mcmsroleinstead of living alongside view code.