-
Notifications
You must be signed in to change notification settings - Fork 2
fix(mcms): improve error messages when proposal ctx is not correctly provided [OPT-325] #661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances error handling and validation for the MCMSv2 CLI and EVM analyzer by adding checks to ensure proposal context and EVM registry are properly configured, providing clearer error messages to guide developers when required dependencies are missing.
Changes:
- Added validation to ensure
proposalCtxProvideris not nil before use - Enhanced error message for missing EVM registry to include configuration guidance
- Added check to verify proposal context returned by provider is not nil
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| experimental/analyzer/evm_analyzer.go | Improved error message for missing EVM registry to include setup instructions |
| engine/cld/legacy/cli/mcmsv2/mcms_v2.go | Added nil checks for proposalCtxProvider and proposalCtx with descriptive error messages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
nit: @ecPablo you might update these bot suggestions I think it make it a little more grammatical there. |
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
thanks @ajaskolski should be good to go now :) |
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|




This pull request introduces improved error handling and validation for the proposal analysis context and EVM registry initialization across the MCMSv2 CLI and EVM analyzer. These changes help prevent misconfiguration and provide clearer guidance to developers when required dependencies are missing.
Validation and error messaging improvements:
buildMCMSv2AnalyzeProposalCmdto ensureproposalCtxProvideris not nil, returning a descriptive error if missing.newCfgv2to verify that the proposal context returned by the provider is not nil, with a clear error message for initialization issues.EVM registry initialization guidance:
AnalyzeEVMTransactionto instruct developers to provide an EVM registry via theProposalContextProviderwith theWithEVMABIMappings()option if it is missing.