refactor(evm): funtoken events, cli commands and queries #1968
Conversation
WalkthroughThe changes encompass a thorough refactoring of the EVM module, emphasizing the management of fungible token events, CLI commands, and queries. Key updates include renaming functions and messages for clarity, enhancing CLI command structures for better usability, and aligning tests with the new configurations. This refactoring seeks to enhance the overall interaction with fungible tokens within the system, making operations more intuitive and efficient. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1968 +/- ##
=======================================
Coverage 65.95% 65.95%
=======================================
Files 258 258
Lines 16384 16384
=======================================
Hits 10806 10806
Misses 4772 4772
Partials 806 806
|
![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, codebase verification and nitpick comments (1)
x/evm/keeper/grpc_query.go (1)
Line range hint
744-763: Review of theFunTokenfunctionThe
FunTokenfunction has been successfully updated to handle the newQueryFunTokenRequestand return aQueryFunTokenResponse. The logic within the function appears to handle two types of token lookups: by cosmos denom and by ERC20 address, which is a good practice for flexibility.Suggestions:
- Error Handling: Ensure that the error message returned when no token mapping is found is clear and helpful for debugging.
- Performance: Consider the implications of the
ExactMatchandCollectcalls on performance, especially in scenarios with large datasets.- Testing: It's crucial to add unit tests for this function to cover both paths (lookup by denom and ERC20 address) and the error scenario.
Would you like assistance in writing comprehensive unit tests for this function?
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
x/evm/events.pb.gois excluded by!**/*.pb.gox/evm/query.pb.gois excluded by!**/*.pb.gox/evm/query.pb.gw.gois excluded by!**/*.pb.gw.go
Files selected for processing (10)
- CHANGELOG.md (1 hunks)
- eth/rpc/backend/mocks/evm_query_client.go (2 hunks)
- proto/eth/evm/v1/events.proto (1 hunks)
- proto/eth/evm/v1/query.proto (2 hunks)
- x/evm/cli/query.go (1 hunks)
- x/evm/cli/tx.go (4 hunks)
- x/evm/keeper/erc20_test.go (11 hunks)
- x/evm/keeper/grpc_query.go (2 hunks)
- x/evm/keeper/grpc_query_test.go (4 hunks)
- x/evm/keeper/msg_server.go (2 hunks)
Additional context used
Learnings (1)
x/evm/cli/query.go (1)
Learnt from: Unique-Divine PR: NibiruChain/nibiru#1869 File: x/evm/cli/cli.go:44-47 Timestamp: 2024-05-10T14:23:51.765Z Learning: The loop in `GetQueryCmd` function of `x/evm/cli/cli.go` is a placeholder for future subcommands.
Gitleaks
x/evm/keeper/grpc_query_test.go
969-969: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (13)
proto/eth/evm/v1/events.proto (1)
50-55: Renaming and Field Addition in Protobuf MessageThe renaming of
EventFunTokenFromBankCointoEventFunTokenCreatedand the addition of the fieldis_made_from_coinare consistent with the PR objectives to enhance event clarity. The field addition is properly defined with a standard scalar type (bool), which is appropriate for the intended usage.x/evm/cli/query.go (2)
15-34: New CLI Command StructureThe
GetQueryCmdfunction properly sets up a new CLI command for querying the module. The use ofcobra.Commandis standard and the function is well-structured with clear separation of concerns. The inclusion of the newGetCmdFunTokencommand is correctly implemented.
36-71: Implementation ofGetCmdFunTokenCommandThe
GetCmdFunTokencommand is well-implemented with appropriate command usage, description, and argument handling. The function's error handling is robust, ensuring that any issues with the client context or query execution are properly returned. The use ofclientCtx.PrintPrototo print the response is a good practice, ensuring that the output is formatted according to the protobuf definition.x/evm/cli/tx.go (2)
Line range hint
29-67: Updated and New Commands for Token CreationThe modifications to
CmdCreateFunTokenFromBankCoinand the addition ofCmdCreateFunTokenFromERC20are correctly implemented. The changes in command usage and description are consistent with the PR objectives. The error handling and transaction broadcasting logic are robust and follow best practices.
105-106: Modification toSendFunTokenToEvmCommandThe changes to the
SendFunTokenToEvmcommand, including the updated command usage and description, are appropriate and align with the PR objectives. The implementation ensures that the command correctly handles the transaction process, including error handling and broadcasting.proto/eth/evm/v1/query.proto (2)
79-80: Renaming of RPC MethodThe renaming of the RPC method from
TokenMappingtoFunTokenis correctly implemented, with the appropriate HTTP GET path updated. This change enhances clarity and aligns with the renaming strategy described in the PR.
309-323: Renaming of Request and Response MessagesThe renaming of
QueryTokenMappingRequestandQueryTokenMappingResponsetoQueryFunTokenRequestandQueryFunTokenResponse, respectively, is correctly implemented. The field definitions and options within these messages are appropriate and consistent with the intended functionality.eth/rpc/backend/mocks/evm_query_client.go (1)
Line range hint
380-401: Renamed function signature and parameters are correctly implemented.The function
TokenMappinghas been correctly renamed toFunToken, and the input and output types have been updated fromQueryTokenMappingRequesttoQueryFunTokenRequestand fromQueryTokenMappingResponsetoQueryFunTokenResponserespectively. This change is consistent with the PR's objectives to refactor token-related functionalities.x/evm/keeper/erc20_test.go (2)
14-14: Added import for enhanced testing utilities.The import
github.com/NibiruChain/nibiru/x/common/testutilhas been added to utilize additional testing utilities. This is a positive change as it likely provides more robust testing functions which are utilized in the test cases.
44-44: Enhanced assertions and event checks in test cases.The replacement of
s.NoErrorwiths.Require().NoErrorin various test cases is a good practice as it ensures that the tests will halt immediately if an error is encountered, which is crucial for critical operations. Additionally, the checks for specific event attributes ensure that the system's event emissions are as expected, enhancing test reliability.Also applies to: 63-63, 78-78, 97-97, 110-110, 124-124, 182-182, 244-244, 260-260, 274-274
x/evm/keeper/msg_server.go (1)
510-515: Correct implementation of new event emission inCreateFunToken.The new event
EventFunTokenCreatedis correctly implemented with additional details such asCreator,BankDenom,Erc20ContractAddress, andIsMadeFromCoin. This change enhances the detail available in event logs, aiding in better monitoring and debugging processes.x/evm/keeper/grpc_query_test.go (1)
Line range hint
915-992: Renamed Test Function and Associated TypesThe test function
TestQueryTokenMappinghas been renamed toTestQueryFunToken, and the associated request and response types have been updated accordingly. This change aligns with the PR's objective to focus onFunToken.
- The renaming of the function and types is consistent and correctly implemented across the test cases.
- The method call
deps.K.TokenMappinghas been updated todeps.K.FunTokenwhich is correct and consistent with the renaming.CHANGELOG.md (1)
88-88: Review of Changelog EntryThe entry under "State Machine Breaking" for the Nibiru EVM section is well-placed and follows the established format of the changelog. It's important to ensure that the linked PR provides a detailed description of the changes and their impact, as this entry will be crucial for users understanding the changes in the system.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
x/evm/query.pb.gois excluded by!**/*.pb.gox/evm/query.pb.gw.gois excluded by!**/*.pb.gw.go
Files selected for processing (5)
- eth/rpc/backend/mocks/evm_query_client.go (2 hunks)
- proto/eth/evm/v1/query.proto (2 hunks)
- x/evm/cli/query.go (1 hunks)
- x/evm/keeper/grpc_query.go (2 hunks)
- x/evm/keeper/grpc_query_test.go (4 hunks)
Files skipped from review as they are similar to previous changes (3)
- eth/rpc/backend/mocks/evm_query_client.go
- x/evm/cli/query.go
- x/evm/keeper/grpc_query.go
Additional context used
Gitleaks
x/evm/keeper/grpc_query_test.go
969-969: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (6)
proto/eth/evm/v1/query.proto (3)
317-323: Ensure consistency of renamed message type.The message type
QueryTokenMappingResponsehas been renamed toQueryFunTokenMappingResponse. Verify that this new message type name is consistently used throughout the codebase.Verification successful
The message type
QueryFunTokenMappingResponseis used consistently throughout the codebase.The search results confirm that the new message type name has been applied correctly, with no remaining instances of the old name
QueryTokenMappingResponse.
- Verified in files such as
x/evm/query.pb.go,proto/eth/evm/v1/query.proto,eth/rpc/backend/mocks/evm_query_client.go,x/evm/keeper/grpc_query_test.go, andx/evm/keeper/grpc_query.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new message type name `QueryFunTokenMappingResponse` is used consistently. # Test: Search for the old and new message type names. Expect: Only occurrences of the new message type name. rg --type proto $'QueryTokenMappingResponse' rg --type proto $'QueryFunTokenMappingResponse'Length of output: 150
Script:
#!/bin/bash # Description: Verify that the new message type name `QueryFunTokenMappingResponse` is used consistently. # Test: Search for the old and new message type names. Expect: Only occurrences of the new message type name. rg 'QueryTokenMappingResponse' rg 'QueryFunTokenMappingResponse'Length of output: 4106
79-80: Ensure consistency of renamed RPC method.The RPC method
TokenMappinghas been renamed toFunTokenMapping. Verify that this new method name is consistently used throughout the codebase.Verification successful
Ensure consistency of renamed RPC method.
The RPC method
TokenMappinghas been renamed toFunTokenMapping. Verify that this new method name is consistently used throughout the codebase.
Renamed RPC method verified successfully.
The RPC method
TokenMappinghas been renamed toFunTokenMappingand the new method name is consistently used throughout the codebase.
- Verified in multiple files including
query.pb.gw.go,query.pb.go,grpc_query.go,grpc_query_test.go,query.proto, andevm_query_client.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new RPC method name `FunTokenMapping` is used consistently. # Test: Search for the old and new method names. Expect: Only occurrences of the new method name. rg --type proto $'TokenMapping' rg --type proto $'FunTokenMapping'Length of output: 124
Script:
#!/bin/bash # Description: Verify that the new RPC method name `FunTokenMapping` is used consistently. # Test: Search for the old and new method names. Expect: Only occurrences of the new method name. rg 'TokenMapping' rg 'FunTokenMapping'Length of output: 20107
309-312: Ensure consistency of renamed message type.The message type
QueryTokenMappingRequesthas been renamed toQueryFunTokenMappingRequest. Verify that this new message type name is consistently used throughout the codebase.Verification successful
The new message type name
QueryFunTokenMappingRequestis used consistently.The old message type name
QueryTokenMappingRequestwas not found, and the new message type nameQueryFunTokenMappingRequestis used in relevant places in the codebase.
rpc FunTokenMapping(QueryFunTokenMappingRequest) returns (QueryFunTokenMappingResponse) {message QueryFunTokenMappingRequest {Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new message type name `QueryFunTokenMappingRequest` is used consistently. # Test: Search for the old and new message type names. Expect: Only occurrences of the new message type name. rg --type proto $'QueryTokenMappingRequest' rg --type proto $'QueryFunTokenMappingRequest'Length of output: 148
Script:
#!/bin/bash # Description: Verify that the new message type name `QueryFunTokenMappingRequest` is used consistently. # Find all .proto files and search for the old message type name fd -e proto -x rg 'QueryTokenMappingRequest' {} # Find all .proto files and search for the new message type name fd -e proto -x rg 'QueryFunTokenMappingRequest' {}Length of output: 232
x/evm/keeper/grpc_query_test.go (3)
915-915: Ensure consistency of renamed test function.The test function
TestQueryTokenMappinghas been renamed toTestQueryFunToken. Verify that this new test function name is consistently used throughout the codebase.Verification successful
Ensure consistency of renamed test function.
The test function
TestQueryTokenMappinghas been renamed toTestQueryFunToken. The search results confirm that the new test function name is used consistently throughout the codebase, and no references to the old name were found.
- The new test function name
TestQueryFunTokenis present inx/evm/keeper/grpc_query_test.go.- No occurrences of the old test function name
TestQueryTokenMappingwere found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new test function name `TestQueryFunToken` is used consistently. # Test: Search for the old and new test function names. Expect: Only occurrences of the new test function name. rg --type go $'TestQueryTokenMapping' rg --type go $'TestQueryFunToken'Length of output: 139
Line range hint
917-971:
Ensure consistency of renamed response type.The response type
QueryTokenMappingResponsehas been renamed toQueryFunTokenMappingResponse. Verify that this new response type name is consistently used throughout the codebase.Verification successful
The response type
QueryTokenMappingResponsehas been consistently renamed toQueryFunTokenMappingResponse.The new response type name is used consistently throughout the codebase, and there are no occurrences of the old response type name.
- Verified in files:
x/evm/query.pb.gox/evm/keeper/grpc_query_test.gox/evm/keeper/grpc_query.goeth/rpc/backend/mocks/evm_query_client.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new response type name `QueryFunTokenMappingResponse` is used consistently. # Test: Search for the old and new response type names. Expect: Only occurrences of the new response type name. rg --type go $'QueryTokenMappingResponse' rg --type go $'QueryFunTokenMappingResponse'Length of output: 3937
916-922: Ensure consistency of renamed request type.The request type
QueryTokenMappingRequesthas been renamed toQueryFunTokenMappingRequest. Verify that this new request type name is consistently used throughout the codebase.Verification successful
The new request type name
QueryFunTokenMappingRequestis used consistently throughout the codebase.The old request type name
QueryTokenMappingRequestdoes not appear in the search results, indicating a complete and consistent renaming.
- Files verified:
x/evm/keeper/grpc_query.gox/evm/query.pb.gox/evm/query.pb.gw.gox/evm/keeper/grpc_query_test.goeth/rpc/backend/mocks/evm_query_client.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new request type name `QueryFunTokenMappingRequest` is used consistently. # Test: Search for the old and new request type names. Expect: Only occurrences of the new request type name. rg --type go $'QueryTokenMappingRequest' rg --type go $'QueryFunTokenMappingRequest'Length of output: 4077
| @@ -965,10 +965,10 @@ func (s *Suite) TestQueryTokenMapping() { | |||
| ) | |||
| }, | |||
| scenario: func(deps *evmtest.TestDeps) (req In, wantResp Out) { | |||
| req = &evm.QueryTokenMappingRequest{ | |||
| req = &evm.QueryFunTokenMappingRequest{ | |||
| Token: "0xAEf9437FF23D48D73271a41a8A094DEc9ac71477", | |||
There was a problem hiding this comment.
Potential Security Issue: Detected Generic API Key
Static analysis has flagged a potential exposure of a generic API key. This could potentially lead to unauthorized access if the key is sensitive.
- "0xAEf9437FF23D48D73271a41a8A094DEc9ac71477"
+ "<REDACTED>"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.
| Token: "0xAEf9437FF23D48D73271a41a8A094DEc9ac71477", | |
| Token: "<REDACTED>", |
Tools
Gitleaks
969-969: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
EventFunTokenCreatedon any funtoken creationTokenMapping->FunTokenFunTokenSummary by CodeRabbit
New Features
Refactor
Tests