-
Notifications
You must be signed in to change notification settings - Fork 193
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
refactor: simplify account retrieval #2141
Conversation
WalkthroughThe pull request introduces changes to the Nibiru EVM module's account retrieval mechanism. The modifications focus on simplifying the account retrieval operation in the Changes
Sequence DiagramsequenceDiagram
participant Client
participant EVMKeeper
participant StateDB
Client->>EVMKeeper: Query EVM Account
EVMKeeper->>StateDB: getAccountWithoutBalance()
alt Account Exists
StateDB-->>EVMKeeper: Return Account
EVMKeeper-->>Client: Return Account Details
else Account Not Found
StateDB-->>EVMKeeper: Return nil
EVMKeeper-->>Client: Return "account not found" error
end
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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2141 +/- ##
==========================================
- Coverage 65.10% 65.08% -0.03%
==========================================
Files 274 274
Lines 21732 21725 -7
==========================================
- Hits 14148 14139 -9
- Misses 6610 6613 +3
+ Partials 974 973 -1
|
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
🧹 Nitpick comments (2)
x/evm/keeper/statedb.go (1)
187-190
: Refine doc-comment for private function.The doc-comment is slightly terse: “load nonce and codehash without balance.” Consider clarifying that this function:
- Queries the account from the
accountKeeper
.- Deems the account nonexistent (i.e., returns
nil
) when not found or of the wrong type.x/evm/keeper/grpc_query.go (1)
211-211
: Check for contractness outside the method.Retrieving the code via
k.getAccountWithoutBalance
(which returns nil if not found) is consistent with the new approach of streamlined account checking. Ensure external logic verifies whether the account is nil before referencing its fields.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md
(1 hunks)x/evm/keeper/grpc_query.go
(2 hunks)x/evm/keeper/grpc_query_test.go
(2 hunks)x/evm/keeper/statedb.go
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-tests
🔇 Additional comments (7)
x/evm/keeper/statedb.go (1)
28-28
: Ensure calling code gracefully handles nil return.Changing from a publicly exposed function to a private helper is consistent with the intent to simplify the account retrieval logic. However, confirm downstream usage (like
GetAccount
) properly checks fornil
returns to avoid potential nil-pointer references.x/evm/keeper/grpc_query.go (1)
67-70
: Great approach for nonexistent accounts.Raising an explicit error instead of returning a silent, empty account clarifies the gRPC contract. This helps users differentiate between genuinely empty data versus a non-existent account scenario.
x/evm/keeper/grpc_query_test.go (4)
147-147
: Test coverage for null response.It's good that you updated the test to expect
nil
for a non-existent address. This addresses the new error condition thoroughly.
150-150
: Align test message with the code.The new error string "account not found for" is consistent with the code changes in
grpc_query.go
. This ensures robust test coverage for the not-found scenario.
159-159
: Duplicate scenario for nonexistent account.Testing another input scenario for an unknown address further improves coverage and ensures consistent error messages across different input formats (hex vs. bech32).
162-162
: Consistent with updated logic.This line ensures our test suite enforces the expectation that nonexistent accounts always trigger an error. No concerns here.
CHANGELOG.md (1)
57-57
: Accurate changelog entry.Tying PR #2141 to the actual code changes in the EVM account retrieval logic is precise. The mention highlights the improvement to the
nibid q evm account
command, which is consistent with the rest of the changes.
Purpose / Abstract
Simplifies the account retrieval operation in
nibid q evm account
.Summary by CodeRabbit
Release Notes
Refactor
nibid q evm account
commandBug Fixes
Documentation