-
Notifications
You must be signed in to change notification settings - Fork 201
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
feat(evm-cli): Add the account query to the EVM command. Cover the CLI with tests. #2002
Conversation
WalkthroughThe changes enhance the Ethereum Virtual Machine (EVM) module by introducing new command functionalities, improving test coverage, and refining code documentation. Key modifications include the addition of account query capabilities, restructuring the handling of fungible tokens, and implementing a comprehensive testing framework for command-line interactions. Updates also involve comment organization and field ordering in response structures across various components. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant EVM Module
participant Keeper
User->>CLI: Execute account query command
CLI->>EVM Module: Process query command
EVM Module->>Keeper: Retrieve account information
Keeper-->>EVM Module: Return account details
EVM Module-->>CLI: Send account details response
CLI-->>User: Display account information
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2002 +/- ##
==========================================
+ Coverage 65.73% 65.90% +0.17%
==========================================
Files 261 263 +2
Lines 16405 16562 +157
==========================================
+ Hits 10784 10916 +132
- Misses 4809 4824 +15
- Partials 812 822 +10
|
@@ -36,87 +36,72 @@ func GetTxCmd() *cobra.Command { | |||
return txCmd | |||
} | |||
|
|||
// CmdCreateFunTokenFromBankCoin broadcast MsgCreateFunToken | |||
func CmdCreateFunTokenFromBankCoin() *cobra.Command { | |||
// CmdCreateFunToken broadcast MsgCreateFunToken |
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.
Now mirroring the behavior of the gRPC tx message
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- CHANGELOG.md (1 hunks)
- app/evmante/evmante_sigverify.go (1 hunks)
- x/evm/cli/cli_setup_test.go (1 hunks)
- x/evm/cli/cli_test.go (1 hunks)
- x/evm/cli/query.go (3 hunks)
- x/evm/cli/tx.go (4 hunks)
- x/evm/evmtest/eth.go (1 hunks)
- x/evm/keeper/grpc_query.go (1 hunks)
- x/evm/keeper/grpc_query_test.go (2 hunks)
Files skipped from review due to trivial changes (2)
- app/evmante/evmante_sigverify.go
- x/evm/keeper/grpc_query.go
Additional comments not posted (14)
x/evm/evmtest/eth.go (1)
39-46
: New functionNewEthPrivAccs
is well-implemented.The function efficiently creates a slice of
EthPrivKeyAcc
instances by callingNewEthPrivAcc
multiple times. This addition enhances functionality by allowing batch creation of accounts.x/evm/cli/cli_setup_test.go (2)
1-54
: Test suite setup is well-structured.The test suite uses the testify package effectively to set up and run tests. The
SetupSuite
method initializes the keyring and client context correctly, and theTestSuite
function runs the suite as expected.
56-115
: Test case structure is clear and comprehensive.The
TestCase
struct and its methodsRunTxCmd
andRunQueryCmd
provide a clear framework for executing transaction and query commands. The use of context and error handling is appropriate.x/evm/cli/tx.go (3)
29-30
: Command consolidation improves usability.The consolidation of commands into
CmdCreateFunToken
simplifies the CLI by reducing the number of commands and using flags to specify the source. This change enhances user experience.
39-90
: Flag-based argument handling is a good practice.The use of flags
--bank-denom
and--erc20
inCmdCreateFunToken
improves flexibility and clarity. The error handling for flag requirements is well-implemented.
Line range hint
94-120
: Command renaming aligns with best practices.Renaming
ConvertCoinToEvm
toCmdConvertCoinToEvm
aligns with naming conventions and improves consistency. The command logic remains clear and functional.x/evm/cli/query.go (1)
Line range hint
41-76
: LGTM!The
CmdQueryFunToken
function is well-implemented with appropriate error handling and command structure.x/evm/cli/cli_test.go (4)
23-50
: LGTM!The
TestCmdConvertCoinToEvm
function is well-structured and covers both success and error scenarios effectively.
52-87
: LGTM!The
TestCmdCreateFunToken
function is comprehensive and effectively covers various scenarios, including errors.
89-139
: LGTM!The
TestCmdQueryAccount
function is well-designed, covering different address formats and offline mode effectively.
141-173
: LGTM!The
TestCmdQueryFunToken
function is thorough and effectively addresses both success and error scenarios.x/evm/keeper/grpc_query_test.go (2)
Line range hint
141-158
: LGTM!The test case for a nonexistent account with a hex address input is well-structured and correctly verifies the expected response.
159-177
: LGTM!The test case for a nonexistent account with a Bech32 address input is well-structured and correctly verifies the expected response.
CHANGELOG.md (1)
111-111
: Changelog entry is well-formed.The new entry for the account query feature in the EVM command is correctly added under the "Unreleased" section. It adheres to the specified format and guidelines.
isBech32, err := req.Validate() | ||
fmt.Printf("TODO: UD-DEBUG: req.String(): %v\n", req.String()) | ||
fmt.Printf("TODO: UD-DEBUG: err: %v\n", err) |
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.
Remove debug print statements.
The debug print statements should be removed to clean up the code for production.
Apply this diff to remove the debug statements:
- fmt.Printf("TODO: UD-DEBUG: req.String(): %v\n", req.String())
- fmt.Printf("TODO: UD-DEBUG: err: %v\n", err)
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.
isBech32, err := req.Validate() | |
fmt.Printf("TODO: UD-DEBUG: req.String(): %v\n", req.String()) | |
fmt.Printf("TODO: UD-DEBUG: err: %v\n", err) | |
isBech32, err := req.Validate() |
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/evm/cli/cli_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/evm/cli/cli_test.go
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
Purpose / Abstract
For easier account lookup
The
--offline
flagIt helps with this issue the gRPC query can fail and you want to use the command for the address conversion anyway.
Summary by CodeRabbit
New Features
Bug Fixes
Tests