fix(precompile-funtoken.go): Fixes a bug where the err != nil check is missing in the bankBalance precompile method #2116
Conversation
…s missing in the bankBalance precompile method
WalkthroughThe changes in this pull request encompass updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
evm-e2e/justfile (1)
27-28: Add documentation for the new recipeWhile the recipe is functionally correct, consider adding a comment explaining its purpose, similar to other recipes in the file. Also note that this command is already run during
install, so you might want to clarify when developers should use this separate recipe instead.Consider adding documentation like this:
+# Regenerates TypeScript types from smart contract ABIs gen-types: npx hardhat typechainx/evm/precompile/funtoken.go (1)
Line range hint
196-198: Consider implementing the TODO items for event emissionThese TODO comments indicate missing event emissions for balance changes. Implementing these events would improve transaction observability and make it easier to track balance changes.
Would you like me to help implement these event emissions or create a GitHub issue to track this enhancement?
CHANGELOG.md (1)
109-109: Consider enhancing the changelog entry with more details.While the entry correctly documents the fix, it would be helpful to add more context about:
- The potential impact of the missing error check
- Why this fix is important for the bankBalance precompile method
- Any related issues or bugs this fixes
Example enhancement:
- - [#2116](https://github.com/NibiruChain/nibiru/pull/2116) - fix(precompile-funtoken.go): Fixes a bug where the err != nil check is missing in the bankBalance precompile method + - [#2116](https://github.com/NibiruChain/nibiru/pull/2116) - fix(precompile-funtoken.go): Fixes a bug where the err != nil check is missing in the bankBalance precompile method. This could lead to incorrect balance reporting or potential panics if the underlying bank query fails. The fix ensures proper error handling and consistent behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)evm-e2e/justfile(1 hunks)x/evm/precompile/funtoken.go(3 hunks)
🔇 Additional comments (4)
evm-e2e/justfile (1)
26-26: LGTM!
The added empty line maintains consistent spacing between recipe blocks.
x/evm/precompile/funtoken.go (3)
121-121: LGTM: Consistent error wrapping
The error wrapping with ErrInvalidArgs aligns with the error handling pattern used throughout the codebase.
463-464: LGTM: Consistent error handling
The error wrapping with ErrInvalidArgs maintains consistency with the error handling pattern used throughout the precompile methods.
380-383: LGTM: Fixed missing error check
This change addresses the critical bug where errors from parseArgsBankBalance were not being handled. The implementation is consistent with error handling patterns in other methods like balance and whoAmI.
Let's verify the consistency of error handling across similar methods:
✅ Verification successful
Let me verify if all methods that parse arguments have consistent error handling by checking for parse functions and their error handling.
Let me try a different pattern to find parse functions and their error handling.
LGTM: Error handling is consistent across all parse functions
The verification confirms that the error handling in bankBalance method is consistent with the codebase's patterns. All methods that use parse functions properly check and wrap errors using ErrInvalidArgs, including:
sendToBankusingparseArgsSendToBankbalanceusingparseArgsBalancebankBalanceusingparseArgsBankBalancewhoAmIusingparseArgsWhoAmI- Wasm-related methods using their respective parse functions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify error handling consistency in precompile methods
# Expected: All precompile methods should check and wrap errors from their parse functions
ast-grep --pattern 'if err != nil {
err = ErrInvalidArgs(err)
return
}'
Length of output: 1525
Script:
#!/bin/bash
# Find all parse functions and their callers to verify error handling
ast-grep --pattern 'parse$_Args($$$)'
Length of output: 39
Script:
#!/bin/bash
# Find all parse functions and their usage
rg "parse.*Args.*\(" -A 3 x/evm/precompile/
Length of output: 4345
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2116 +/- ##
==========================================
- Coverage 64.68% 64.66% -0.02%
==========================================
Files 273 273
Lines 21610 21616 +6
==========================================
+ Hits 13978 13979 +1
- Misses 6655 6659 +4
- Partials 977 978 +1
|
Purpose / Abstract
This change makes the error description prefixes more consistent in the precompile/funtoken.go and fixes a bug where there's a missing
if err != nilcheck.Summary by CodeRabbit
Release Notes
New Features
gen-typesfor generating TypeScript types for Ethereum smart contracts.Bug Fixes
Improvements