feat(evm): tx for creating fun token from coin #1946
Conversation
WalkthroughThe submitted changes introduce a new feature to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant CosmosApp
participant EVMModule
participant Keeper
User->>Client: Initiate CreateFunTokenFromCoin Request
Client->>CosmosApp: Send MsgCreateFunTokenFromCoin
CosmosApp->>EVMModule: Forward MsgCreateFunTokenFromCoin
EVMModule->>Keeper: Call CreateFunTokenFromCoin Method
Keeper-->>EVMModule: Return MsgCreateFunTokenFromCoinResponse
EVMModule-->>CosmosApp: Response with new_contract_address
CosmosApp-->>Client: Return new_contract_address
Client-->>User: Display new_contract_address
Possibly related issues
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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 Configration 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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
x/evm/events.pb.gois excluded by!**/*.pb.gox/evm/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (7)
- CHANGELOG.md (1 hunks)
- proto/eth/evm/v1/events.proto (1 hunks)
- proto/eth/evm/v1/tx.proto (2 hunks)
- x/evm/keeper/funtoken_state.go (1 hunks)
- x/evm/keeper/msg_fun_token.go (1 hunks)
- x/evm/keeper/msg_fun_token_test.go (1 hunks)
- x/evm/msg.go (2 hunks)
Additional comments not posted (7)
x/evm/keeper/msg_fun_token.go (1)
13-38: Ensure proper error handling and event emission in CreateFunTokenFromCoin.
- Error Handling: The function correctly handles errors from
ValidateBasic, key generation, and token insertion. Each potential error point correctly returns an error, ensuring robust error handling.- Event Emission: The event emission at lines 31-35 correctly logs the creation of the token, including all relevant details such as the creator, denom, and contract address.
- Response Creation: The function returns a response with the new contract address, which aligns with the intended functionality.
Overall, the function is well-structured and follows good practices in error handling and response generation.
proto/eth/evm/v1/events.proto (1)
47-51: Review the structure of EventCreateFunTokenFromCoin.The new protobuf message
EventCreateFunTokenFromCoincorrectly defines fields fordenom,contract_address, andcreator. These fields are essential for tracking the creation of new tokens and are appropriately indexed starting from 1. This structure aligns well with the intended functionality of logging token creation events.x/evm/keeper/msg_fun_token_test.go (1)
9-62: Ensure comprehensive test coverage in TestMsgCreateFunTokenFromCoin.The test cases cover various scenarios including valid and invalid inputs for
denomandsender. The use of table-driven tests enhances maintainability and scalability of the test suite. However, consider adding more edge cases, such as testing with boundary values fordenomandsender.Also, ensure that the error messages and assertions are specific and informative, helping in quick identification of issues during test failures.
x/evm/keeper/funtoken_state.go (1)
83-87: Review the implementation of SafeInsert.The
SafeInsertmethod correctly creates aFunTokenobject and validates it before insertion. The use of defensive programming by validating theFunTokenbefore insertion enhances the robustness of the method. Ensure that theValidatemethod ofFunTokenis thoroughly tested to prevent any corrupt data from being inserted into the state.Verification successful
The
Validatemethod ofFunTokenis thoroughly tested.The test cases in
x/evm/evm_test.gocover various scenarios, including invalid bank denominations, different formats of ERC20 addresses, and mixed case compatibility. This ensures that theValidatemethod is robust and handles different edge cases effectively.
x/evm/evm_test.go: Comprehensive test cases forFunToken.Validate.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the Validate method of FunToken is thoroughly tested. # Test: Search for the Validate method usage in tests. Expect: Comprehensive test coverage. rg --type go $'FunToken.Validate'Length of output: 32
Script:
#!/bin/bash # Search for the definition of the Validate method in FunToken ast-grep --lang go --pattern 'func ($_ FunToken) Validate() $_' --json # Search for calls to the Validate method in the codebase ast-grep --lang go --pattern '$_ .Validate()' --json # Search for test files that might indirectly test the Validate method by testing SafeInsert rg --type go -l 'SafeInsert' | xargs rg -l 'func Test'Length of output: 39150
Script:
#!/bin/bash # Display the contents of the test file where FunToken.Validate is called to verify thorough testing. cat ./x/evm/evm_test.goLength of output: 2794
proto/eth/evm/v1/tx.proto (1)
24-25: Review the structure of MsgCreateFunTokenFromCoin and its response.The new service
CreateFunTokenFromCoinand its associated request and response messages are well-defined. The fields inMsgCreateFunTokenFromCoinandMsgCreateFunTokenFromCoinResponseare appropriately tagged and structured to align with the functionality of registering a Cosmos denom as an EVM ERC-20 contract. Ensure that these protobuf definitions are aligned with the actual implementation in the Go code to maintain consistency across the codebase.Also applies to: 184-196
x/evm/msg.go (1)
36-36: Approval of interface implementation for MsgCreateFunTokenFromCoin.The declaration correctly implements the
sdk.Msginterface forMsgCreateFunTokenFromCoin, ensuring it can be utilized within the Cosmos SDK messaging framework.CHANGELOG.md (1)
80-80: Changelog entry for PR #1946 is correctly documented.The entry is concise, links directly to the PR, and accurately describes the feature introduced. This aligns well with the project's standards for changelog entries.
x/evm/msg.go
Outdated
| func (msg *MsgCreateFunTokenFromCoin) ValidateBasic() error { | ||
| _, err := sdk.AccAddressFromBech32(msg.Sender) | ||
| if err != nil { | ||
| return fmt.Errorf("invalid sender: %s", msg.Sender) | ||
| } | ||
| if err := sdk.ValidateDenom(msg.Denom); err != nil { | ||
| return errValidateFunToken(err.Error()) | ||
| } |
There was a problem hiding this comment.
Enhance error reporting in ValidateBasic method.
Consider including the specific error message when validation fails for better debugging and user feedback.
- return errValidateFunToken(err.Error())
+ return fmt.Errorf("validation error for denom '%s': %s", msg.Denom, 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.
| func (msg *MsgCreateFunTokenFromCoin) ValidateBasic() error { | |
| _, err := sdk.AccAddressFromBech32(msg.Sender) | |
| if err != nil { | |
| return fmt.Errorf("invalid sender: %s", msg.Sender) | |
| } | |
| if err := sdk.ValidateDenom(msg.Denom); err != nil { | |
| return errValidateFunToken(err.Error()) | |
| } | |
| func (msg *MsgCreateFunTokenFromCoin) ValidateBasic() error { | |
| _, err := sdk.AccAddressFromBech32(msg.Sender) | |
| if err != nil { | |
| return fmt.Errorf("invalid sender: %s", msg.Sender) | |
| } | |
| if err := sdk.ValidateDenom(msg.Denom); err != nil { | |
| return fmt.Errorf("validation error for denom '%s': %s", msg.Denom, err) | |
| } |
| func (msg *MsgCreateFunTokenFromCoin) GetSigners() []sdk.AccAddress { | ||
| sender, _ := sdk.AccAddressFromBech32(msg.Sender) | ||
| return []sdk.AccAddress{sender} |
There was a problem hiding this comment.
Handle potential error when retrieving sender address in GetSigners.
Ignoring the error from sdk.AccAddressFromBechiz could lead to runtime panics if the address is invalid. It's important to handle this error appropriately.
- sender, _ := sdk.AccAddressFromBech32(msg.Sender)
+ sender, err := sdk.AccAddressFromBech32(msg.Sender)
+ if err != nil {
+ return nil, err
+ }Committable suggestion was skipped due to low confidence.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1946 +/- ##
==========================================
- Coverage 64.83% 64.80% -0.03%
==========================================
Files 253 254 +1
Lines 16065 16092 +27
==========================================
+ Hits 10416 10429 +13
- Misses 4892 4904 +12
- Partials 757 759 +2
|
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- x/evm/keeper/funtoken_state.go (1 hunks)
- x/evm/msg.go (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- x/evm/keeper/funtoken_state.go
Additional context used
Learnings (1)
x/evm/msg.go (2)
Learnt from: Unique-Divine PR: NibiruChain/nibiru#1680 File: x/perp/v2/keeper/msg_server.go:210-210 Timestamp: 2023-11-28T17:20:18.983Z Learning: The user indicated that error handling for `sdk.AccAddressFromBech32(msg.Sender)` is covered by prior `ValidateBasic` checks in the code flow.Learnt from: Unique-Divine PR: NibiruChain/nibiru#1680 File: x/perp/v2/keeper/msg_server.go:198-198 Timestamp: 2023-11-28T17:20:08.561Z Learning: The user Unique-Divine has indicated that the error check for `sdk.AccAddressFromBech32(msg.Sender)` is not necessary in the context of their code because it is covered by the `ValidateBasic` method.
GitHub Check: codecov/patch
x/evm/msg.go
[warning] 477-480: x/evm/msg.go#L477-L480
Added lines #L477 - L480 were not covered by tests
[warning] 482-483: x/evm/msg.go#L482-L483
Added lines #L482 - L483 were not covered by tests
[warning] 485-485: x/evm/msg.go#L485
Added line #L485 was not covered by tests
[warning] 488-490: x/evm/msg.go#L488-L490
Added lines #L488 - L490 were not covered by tests
Additional comments not posted (2)
x/evm/msg.go (2)
477-486: LGTM!Tools
GitHub Check: codecov/patch
[warning] 477-480: x/evm/msg.go#L477-L480
Added lines #L477 - L480 were not covered by tests
[warning] 482-483: x/evm/msg.go#L482-L483
Added lines #L482 - L483 were not covered by tests
[warning] 485-485: x/evm/msg.go#L485
Added line #L485 was not covered by tests
488-490: Acknowledged: Error handling for address retrieval.Based on previous learnings, error handling for
sdk.AccAddressFromBech32(msg.Sender)is covered byValidateBasic.Tools
GitHub Check: codecov/patch
[warning] 488-490: x/evm/msg.go#L488-L490
Added lines #L488 - L490 were not covered by tests
| func (msg *MsgCreateFunTokenFromCoin) ValidateBasic() error { | ||
| _, err := sdk.AccAddressFromBech32(msg.Sender) | ||
| if err != nil { | ||
| return fmt.Errorf("invalid sender: %s", msg.Sender) | ||
| } | ||
| if err := sdk.ValidateDenom(msg.Denom); err != nil { | ||
| return fmt.Errorf("invalid denom: %s", msg.Denom) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Reminder: Add tests for ValidateBasic.
The static analysis tool indicates that the ValidateBasic function is not covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 477-480: x/evm/msg.go#L477-L480
Added lines #L477 - L480 were not covered by tests
[warning] 482-483: x/evm/msg.go#L482-L483
Added lines #L482 - L483 were not covered by tests
[warning] 485-485: x/evm/msg.go#L485
Added line #L485 was not covered by tests
| func (k *Keeper) CreateFunTokenFromCoin( | ||
| goCtx context.Context, msg *evm.MsgCreateFunTokenFromCoin, | ||
| ) (*evm.MsgCreateFunTokenFromCoinResponse, error) { | ||
| if err := msg.ValidateBasic(); err != nil { | ||
| return nil, err | ||
| } | ||
| priv, err := ethsecp256k1.GenerateKey() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| newContractAddress := common.BytesToAddress(priv.PubKey().Address().Bytes()) | ||
| ctx := sdk.UnwrapSDKContext(goCtx) | ||
| err = k.FunTokens.SafeInsert(ctx, newContractAddress, msg.Denom, true) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| _ = ctx.EventManager().EmitTypedEvent(&evm.EventCreateFunTokenFromCoin{ | ||
| Creator: msg.Sender, | ||
| Denom: msg.Denom, | ||
| ContractAddress: newContractAddress.String(), | ||
| }) | ||
|
|
||
| return &evm.MsgCreateFunTokenFromCoinResponse{NewContractAddress: newContractAddress.String()}, nil |
There was a problem hiding this comment.
This tx should not use a random address and should instead deploy an ERC20 based on the coin metadata and use its address
CreateFunTokenFromCoin#1942Summary by CodeRabbit
New Features
Tests