fix: added deprecated proposal, compatible history #622
Conversation
WalkthroughThe recent changes introduce a new governance message type, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Governance
participant Legacy
User->>Governance: Request Update with MsgUpdateParams
Governance->>Legacy: Validate Authority and Parameters
Legacy-->>Governance: Return Validation Result
Governance-->>User: Confirm Update
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (2)
proto/fx/gov/v1/legacy.proto (2)
12-17: Make the deprecation warning more prominent.The
MsgUpdateParamsmessage is marked as deprecated. Consider making the deprecation warning more prominent to ensure developers avoid using it.-// Deprecated: Do not use. +// DEPRECATED: Do not use this message. It will be removed in future versions.
19-67: Make the deprecation warning more prominent and review comments for clarity.The
LegacyParamsmessage is marked as deprecated. Consider making the deprecation warning more prominent to ensure developers avoid using it. Additionally, review the comments for clarity and consistency.-// Deprecated: Do not use. +// DEPRECATED: Do not use this message. It will be removed in future versions. -// The ratio representing the proportion of the deposit value that must be -// paid at proposal submission. +// The ratio representing the proportion of the deposit value that must be +// paid at proposal submission.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
x/gov/legacy/legacy.pb.gois excluded by!**/*.pb.go
Files selected for processing (6)
- CHANGELOG.md (1 hunks)
- app/encoding_test.go (1 hunks)
- app/interface_registry.json (3 hunks)
- proto/fx/gov/v1/legacy.proto (1 hunks)
- x/gov/legacy/legacy_msg.go (1 hunks)
- x/gov/types/codec.go (2 hunks)
Files skipped from review due to trivial changes (1)
- CHANGELOG.md
Additional comments not posted (8)
x/gov/legacy/legacy_msg.go (3)
11-12: LGTM!The
Routemethod correctly returns the module name.
23-26: LGTM!The
GetSignersmethod correctly returns the expected signers.
28-33: LGTM!The
ValidateBasicmethod correctly validates theAuthorityfield and returns an appropriate error if validation fails.x/gov/types/codec.go (1)
32-32: LGTM!The
RegisterInterfacesmethod correctly registers the new message typeMsgUpdateParamsfrom thelegacypackage, enhancing the module's interoperability and compatibility with legacy message formats.app/encoding_test.go (1)
47-47: LGTM!The modified assertion correctly reflects the addition of the new interface, ensuring the test accurately validates the current state of the system.
app/interface_registry.json (3)
198-198: LGTM!The addition of
"/fx.gov.v1.MsgUpdateParams"in theTypeURLMapsection is correct and consistent with the existing entries.
415-415: LGTM!The addition of
"/fx.gov.v1.MsgUpdateParams"in theMsgssection is correct and consistent with the existing entries.
503-503: LGTM!The addition of
"/fx.gov.v1.MsgUpdateParams"in theProposalMsgssection is correct and consistent with the existing entries.
Comments failed to post (2)
x/gov/legacy/legacy_msg.go
14-15: Return a non-empty string for the message type.
The
Typemethod should return a meaningful string representing the message type instead of an empty string.- return "" + return "update_params"Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// Type returns the MsgUpdateParams message type. func (m *MsgUpdateParams) Type() string { return "update_params" }
17-21: Return the correct sign bytes.
The
GetSignBytesmethod should return the raw bytes that need to be signed, not an empty byte array.- return []byte{} + bz, err := json.Marshal(m) + if err != nil { + panic(err) + } + return sdk.MustSortJSON(bz)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// GetSignBytes returns the raw bytes for a MsgUpdateParams message that // the expected signer needs to sign. func (m *MsgUpdateParams) GetSignBytes() []byte { bz, err := json.Marshal(m) if err != nil { panic(err) } return sdk.MustSortJSON(bz) }
Summary by CodeRabbit
New Features
MsgUpdateParams, to enhance governance parameter updates.Bug Fixes
Documentation