-
Notifications
You must be signed in to change notification settings - Fork 4
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
fix: validate MsgUpsertFeeDenom #176
fix: validate MsgUpsertFeeDenom #176
Conversation
WalkthroughThe changes in this pull request primarily involve updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Quality Gate passedIssues Measures |
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
x/xfeemarket/types/message_upsert_fee_denom.go (1)
27-28
: Approve change with minor suggestion for error message.The update to the
Validate
method is a good improvement. It now correctly prevents both zero and negative values for theMultiplier
, which aligns with typical expectations for a multiplier in financial calculations.However, there's a minor grammatical issue in the error message that could be improved for clarity.
Consider updating the error message for better readability:
- return errors.New("multiplier cannot be less than equal 0") + return errors.New("multiplier cannot be less than or equal to 0")x/xfeemarket/post/fee.go (1)
19-19
: Approved: Clarification on fee refund policyThe updated comment accurately reflects the fee handling process, stating that excess fees are refunded to the payer. This change improves clarity for users and developers.
Consider adding a brief mention of what happens when the provided fee is less than the minimum base fee, for completeness:
- // Excess between the given fee and the on-chain min base fee is refunded to payer. + // Excess between the given fee and the on-chain min base fee is refunded to payer. + // If the given fee is less than the min base fee, the transaction will fail.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- x/xfeemarket/post/fee.go (1 hunks)
- x/xfeemarket/types/message_upsert_fee_denom.go (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
x/xfeemarket/types/message_upsert_fee_denom.go (1)
Line range hint
1-31
: Overall assessment: Changes effectively improveMsgUpsertFeeDenom
validation.The modifications to the
Validate
method ofMsgUpsertFeeDenom
align well with the PR objectives. The updated condition now prevents both zero and negative values for theMultiplier
, which is a more robust validation approach. This change enhances the overall integrity of the fee market system by ensuring that only positive multipliers are accepted.
Summary by CodeRabbit
New Features
Multiplier
field in fee denomination messages to prevent negative values.Bug Fixes