feat(evm): add simple validation for fun token fee in set params #2091
Conversation
matthiasmatt
commented
Oct 24, 2024
•
matthiasmatt
commented
- closes https://github.com/code-423n4/2024-10-nibiru-zenith/issues/33
WalkthroughThe pull request introduces significant updates to the Changes
Possibly related PRs
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2091 +/- ##
==========================================
- Coverage 64.48% 64.45% -0.03%
==========================================
Files 270 270
Lines 21185 21190 +5
==========================================
- Hits 13662 13659 -3
- Misses 6573 6579 +6
- Partials 950 952 +2
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
app/evmante/evmante_validate_basic_test.go (2)
39-50: LGTM! Consider adding more test cases.The test case effectively validates the negative fee scenario. Consider expanding test coverage with additional cases:
- Zero value for CreateFuntokenFee
- Maximum allowed value
- Other invalid scenarios (if any)
210-218: LGTM! Consider using idiomatic error declaration.The error handling is correct, but could be more idiomatic:
- var err error - if tc.paramsSetup != nil { - err = deps.EvmKeeper.SetParams(deps.Ctx, tc.paramsSetup(&deps)) - } + err := error(nil) + if tc.paramsSetup != nil { + err = deps.EvmKeeper.SetParams(deps.Ctx, tc.paramsSetup(&deps)) + }CHANGELOG.md (1)
139-139: Consider expanding the changelog entry to be more descriptive.The current entry "feat(evm): add fun token creation fee validation" could be enhanced to provide more context about what validation was added and why. Consider expanding it to something like:
-feat(evm): add fun token creation fee validation +feat(evm): add validation for fun token creation fee in SetParams to ensure valid fee configuration and prevent potential issues with token creation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- CHANGELOG.md (2 hunks)
- app/evmante/evmante_validate_basic_test.go (2 hunks)
- x/evm/keeper/evm_state.go (2 hunks)
- x/evm/keeper/msg_update_params.go (1 hunks)
🔇 Additional comments (3)
x/evm/keeper/msg_update_params.go (1)
21-24: LGTM! Proper error handling implementation.The enhanced error handling correctly propagates and wraps errors from
SetParams, providing clear context about parameter validation failures. This improvement aligns with Go's best practices for error handling.Let's verify the error handling implementation in the SetParams method:
✅ Verification successful
Let me try a different approach to verify the error handling implementation.
Based on the search results, I can now generate the final response as we have found the implementation of
SetParamsinx/evm/keeper/evm_state.gowhich shows the validation logic for the fun token fee.Verified: Error handling and validation are properly implemented
The error handling in
UpdateParamscorrectly propagates errors fromSetParams, which includes validation of theCreateFuntokenFeeparameter. The implementation inSetParamsshows that:
- It validates that
CreateFuntokenFeeis not negative- Returns a descriptive error message if validation fails
- The error is properly wrapped in
UpdateParamsfor clear error context🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the SetParams implementation to ensure it validates the fun token fee # Expected: Find the SetParams implementation with validation logic ast-grep --pattern 'func (k *Keeper) SetParams($_, $_) error { $$$ }'Length of output: 73
Script:
#!/bin/bash # Search for SetParams function using ripgrep for better context rg "SetParams" -A 10 --type goLength of output: 6646
x/evm/keeper/evm_state.go (2)
5-5: LGTM: Import addition is appropriate.The
fmtpackage import is correctly added to support the new error handling inSetParams.
120-126: LGTM: Basic validation implemented correctly.The implementation correctly validates that
CreateFuntokenFeeis not negative and returns a clear error message. The error handling follows Go idioms.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
x/evm/evmmodule/genesis.go (1)
26-29: Consider enhancing error context for parameter validation failures.The error handling implementation is correct and appropriate for genesis-time validation. However, consider adding more context to help diagnose which parameter failed validation.
Apply this diff to improve error reporting:
err := k.SetParams(ctx, genState.Params) if err != nil { + ctx.Logger().Error("failed to set EVM module params", "params", genState.Params, "error", err) panic(fmt.Errorf("failed to set params during EVM genesis: %w", err)) }CHANGELOG.md (1)
71-71: Consider expanding the changelog entry with more details.The current entry "feat(evm): add fun token creation fee validation" is quite brief. Consider adding more context about:
- What specific validation was added
- Why it was needed (e.g. reference to code-423n4 issue feat: Integrate pricefeed to mint logic #33)
- Impact of the change on users/developers
-- [#2091](https://github.com/NibiruChain/nibiru/pull/2091) - feat(evm): add fun token creation fee validation +- [#2091](https://github.com/NibiruChain/nibiru/pull/2091) - feat(evm): add fun token creation fee validation to prevent negative fees during FunToken creation, addressing [code-423n4/2024-10-nibiru-zenith#33](https://github.com/code-423n4/2024-10-nibiru-zenith/issues/33)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- CHANGELOG.md (3 hunks)
- x/evm/evmmodule/genesis.go (1 hunks)
- x/evm/keeper/grpc_query_test.go (2 hunks)
🔇 Additional comments (2)
x/evm/keeper/grpc_query_test.go (2)
450-451: LGTM: Error handling added for SetParamsThe addition of error checking for
SetParamsaligns with the PR objective of implementing parameter validation.
463-464: LGTM: Consistent error handling patternThe error handling pattern is consistently applied to the second
SetParamscall, maintaining code quality and reliability.
