feat(evm): Tx to create FunToken mapping from ERC20, contract embeds, and ERC20 queries. #1950
Conversation
Squashed commit of the following: commit 51071dad60c12e02093d92d1311d33b70b81304f Author: Unique-Divine <[email protected]> Date: Mon Jun 10 12:32:00 2024 -0500 refactor(e2e-evm): unused vars and better type hint commit 6729b97 Author: Unique-Divine <[email protected]> Date: Fri Jun 7 22:46:14 2024 -0500 test(e2e-evm): more type safety commit 25c1557 Author: Unique-Divine <[email protected]> Date: Fri Jun 7 22:01:21 2024 -0500 Squashed commit of the following: commit 021d161d176112cf24e28780ad64f61155f70ea2 Author: Unique-Divine <[email protected]> Date: Fri Jun 7 22:00:10 2024 -0500 test(e2e): (1) Generated smart contract types for ethers. (2) TypeScript support. (3) Formatter commit af7e7b3 Author: Unique-Divine <[email protected]> Date: Fri Jun 7 16:11:45 2024 -0500 chore: another issue ticket commit 36745fd Author: Unique-Divine <[email protected]> Date: Fri Jun 7 16:07:20 2024 -0500 chore: add issue number for TODO comment commit 8a76c0e Author: Unique-Divine <[email protected]> Date: Fri Jun 7 15:54:07 2024 -0500 refactor(evm): Remove dead code and document non-EVM ante handler commit e4e11df Author: Unique-Divine <[email protected]> Date: Fri Jun 7 15:52:38 2024 -0500 refactor: remove dead code commit cad00c0 Merge: dc5f4dd 359e310 Author: Unique-Divine <[email protected]> Date: Fri Jun 7 15:41:53 2024 -0500 Merge branch 'main' into ud/ante-test commit dc5f4dd Author: Unique-Divine <[email protected]> Date: Fri Jun 7 15:28:53 2024 -0500 refactor: ante handler and evm cleanup commit f73cdc3 Merge: d3a6ea9 290c372 Author: Unique-Divine <[email protected]> Date: Wed Jun 5 20:59:39 2024 -0500 Merge branch 'test/evm-grpc-query' of https://github.com/NibiruChain/nibiru into test/evm-grpc-query commit d3a6ea9 Merge: 376596d 70ee1bf Author: Unique-Divine <[email protected]> Date: Wed Jun 5 20:59:30 2024 -0500 Merge branch 'main' into test/evm-grpc-query commit 376596d Author: Unique-Divine <[email protected]> Date: Wed Jun 5 20:58:40 2024 -0500 Squashed commit of the following: commit b5687130ff5f3d020a3b14d219fec3a816579c30 Author: Unique-Divine <[email protected]> Date: Wed Jun 5 20:57:44 2024 -0500 chore: run tidy commit 1f1f938 Merge: 3e3cc83 bbcc6f8 Author: Unique-Divine <[email protected]> Date: Wed Jun 5 19:16:30 2024 -0500 Merge branch 'main' into ud/fix-race-condition commit 3e3cc83 Author: Unique-Divine <[email protected]> Date: Wed Jun 5 19:15:40 2024 -0500 chore: changelog commit 3876ccb Author: Unique-Divine <[email protected]> Date: Wed Jun 5 19:04:00 2024 -0500 refactor: more consistent test names commit aaa0a19 Author: Unique-Divine <[email protected]> Date: Wed Jun 5 18:53:09 2024 -0500 test(oracle): Fix missing tear down step for oracle integration test commit 8c3c35e Author: Unique-Divine <[email protected]> Date: Wed Jun 5 17:55:56 2024 -0500 chore: add test comands to justfile commit 4916282 Merge: 64ed0a2 e7e708d Author: Unique-Divine <[email protected]> Date: Fri May 31 09:35:33 2024 -0500 Merge branch 'main' into ud/fix-race-condition commit 64ed0a2 Author: Unique-Divine <[email protected]> Date: Fri May 31 01:44:55 2024 -0500 fix(gosdk): tests parallel race condition commit 290c372 Merge: 0d1c894 70ee1bf Author: Unique Divine <[email protected]> Date: Wed Jun 5 20:05:19 2024 -0500 Merge branch 'main' into test/evm-grpc-query commit 0d1c894 Merge: 9170835 ad173e9 Author: Unique Divine <[email protected]> Date: Wed Jun 5 19:34:38 2024 -0500 Merge branch 'main' into test/evm-grpc-query commit 9170835 Author: Oleg Nikonychev <[email protected]> Date: Wed Jun 5 13:55:14 2024 +0400 fix: removed hardcoded gas value in grpc_query test commit 4337858 Author: Oleg Nikonychev <[email protected]> Date: Wed Jun 5 13:14:34 2024 +0400 chore: refactored eth util methods commit 7df84e2 Merge: 8918498 bbcc6f8 Author: Oleg Nikonychev <[email protected]> Date: Wed Jun 5 12:23:51 2024 +0400 chore: resolve conflicts commit 8918498 Merge: 3fd45ce e7e708d Author: Oleg Nikonychev <[email protected]> Date: Mon Jun 3 21:56:39 2024 +0400 Merge branch 'main' into test/evm-grpc-query commit 3fd45ce Author: Oleg Nikonychev <[email protected]> Date: Mon Jun 3 21:56:23 2024 +0400 chore: changelog update commit 3348876 Author: Oleg Nikonychev <[email protected]> Date: Mon Jun 3 21:53:54 2024 +0400 test(evm): grpc_query full coverage
WalkthroughThe recent changes introduce comprehensive updates to the Ethereum Virtual Machine (EVM) functionality, primarily focusing on facilitating the creation of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant EVM Module
participant Bank Module
User->>Client: Request creation of FunToken from ERC20 token
Client->>EVM Module: Initiate CreateFunToken RPC
EVM Module->>EVM Module: Validate inputs and deploy FunToken contract
EVM Module->>Bank Module: Store ERC20 token metadata
Bank Module-->>EVM Module: Confirmation of metadata storage
EVM Module-->>Client: Response with FunToken details
Client-->>User: FunToken creation successful
Assessment against linked issues
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 Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1950 +/- ##
==========================================
+ Coverage 64.83% 65.34% +0.50%
==========================================
Files 253 253
Lines 16065 16092 +27
==========================================
+ Hits 10416 10515 +99
+ Misses 4892 4805 -87
- Partials 757 772 +15
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
x/evm/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (23)
- CHANGELOG.md (1 hunks)
- app/server/config/server_config.go (2 hunks)
- app/server/start.go (1 hunks)
- eth/rpc/rpcapi/eth_api_test.go (3 hunks)
- proto/eth/evm/v1/tx.proto (2 hunks)
- x/evm/const.go (3 hunks)
- x/evm/deps.go (2 hunks)
- x/evm/embeds/ERC20Minter.sol (1 hunks)
- x/evm/embeds/ERC20MinterCompiled.json (1 hunks)
- x/evm/embeds/README.md (1 hunks)
- x/evm/embeds/embeds.go (1 hunks)
- x/evm/embeds/embeds_test.go (1 hunks)
- x/evm/evmtest/tx.go (4 hunks)
- x/evm/json_tx_args.go (1 hunks)
- x/evm/keeper/erc20.go (1 hunks)
- x/evm/keeper/erc20_test.go (1 hunks)
- x/evm/keeper/funtoken_state_test.go (3 hunks)
- x/evm/keeper/grpc_query_test.go (17 hunks)
- x/evm/keeper/keeper_test.go (1 hunks)
- x/evm/keeper/msg_ethereum_tx_test.go (2 hunks)
- x/evm/keeper/msg_server.go (7 hunks)
- x/evm/keeper/precompiles.go (1 hunks)
- x/evm/msg.go (3 hunks)
Files not summarized due to errors (1)
- x/evm/embeds/ERC20MinterCompiled.json: Error: Message exceeds token limit
Files skipped from review due to trivial changes (3)
- x/evm/embeds/README.md
- x/evm/keeper/keeper_test.go
- x/evm/keeper/msg_ethereum_tx_test.go
Additional context used
GitHub Check: codecov/patch
x/evm/const.go
[warning] 53-53: x/evm/const.go#L53
Added line #L53 was not covered by tests
[warning] 58-58: x/evm/const.go#L58
Added line #L58 was not covered by tests
[warning] 77-81: x/evm/const.go#L77-L81
Added lines #L77 - L81 were not covered by tests
[warning] 83-83: x/evm/const.go#L83
Added line #L83 was not covered by testsx/evm/embeds/embeds.go
[warning] 33-33: x/evm/embeds/embeds.go#L33
Added line #L33 was not covered by tests
[warning] 85-87: x/evm/embeds/embeds.go#L85-L87
Added lines #L85 - L87 were not covered by tests
[warning] 96-96: x/evm/embeds/embeds.go#L96
Added line #L96 was not covered by tests
[warning] 101-101: x/evm/embeds/embeds.go#L101
Added line #L101 was not covered by tests
[warning] 114-114: x/evm/embeds/embeds.go#L114
Added line #L114 was not covered by tests
[warning] 130-130: x/evm/embeds/embeds.go#L130
Added line #L130 was not covered by tests
[warning] 136-136: x/evm/embeds/embeds.go#L136
Added line #L136 was not covered by tests
[warning] 143-143: x/evm/embeds/embeds.go#L143
Added line #L143 was not covered by tests
[warning] 145-146: x/evm/embeds/embeds.go#L145-L146
Added lines #L145 - L146 were not covered by tests
[warning] 151-151: x/evm/embeds/embeds.go#L151
Added line #L151 was not covered by tests
[warning] 164-164: x/evm/embeds/embeds.go#L164
Added line #L164 was not covered by testsx/evm/evmtest/tx.go
[warning] 157-160: x/evm/evmtest/tx.go#L157-L160
Added lines #L157 - L160 were not covered by tests
[warning] 176-178: x/evm/evmtest/tx.go#L176-L178
Added lines #L176 - L178 were not covered by tests
[warning] 181-184: x/evm/evmtest/tx.go#L181-L184
Added lines #L181 - L184 were not covered by tests
[warning] 186-186: x/evm/evmtest/tx.go#L186
Added line #L186 was not covered by tests
[warning] 189-189: x/evm/evmtest/tx.go#L189
Added line #L189 was not covered by tests
[warning] 191-192: x/evm/evmtest/tx.go#L191-L192
Added lines #L191 - L192 were not covered by tests
[warning] 195-195: x/evm/evmtest/tx.go#L195
Added line #L195 was not covered by tests
[warning] 202-202: x/evm/evmtest/tx.go#L202
Added line #L202 was not covered by tests
[warning] 204-210: x/evm/evmtest/tx.go#L204-L210
Added lines #L204 - L210 were not covered by tests
[warning] 216-216: x/evm/evmtest/tx.go#L216
Added line #L216 was not covered by tests
[warning] 218-221: x/evm/evmtest/tx.go#L218-L221
Added lines #L218 - L221 were not covered by tests
[warning] 227-227: x/evm/evmtest/tx.go#L227
Added line #L227 was not covered by tests
[warning] 236-236: x/evm/evmtest/tx.go#L236
Added line #L236 was not covered by tests
[warning] 242-242: x/evm/evmtest/tx.go#L242
Added line #L242 was not covered by tests
[warning] 245-245: x/evm/evmtest/tx.go#L245
Added line #L245 was not covered by tests
Gitleaks
x/evm/keeper/grpc_query_test.go
954-954: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (61)
x/evm/embeds/embeds_test.go (1)
1-1: Add a package-level comment.Consider adding a package-level comment to describe the purpose of the
embeds_testpackage.// Package embeds_test contains tests for the embeds package. package embeds_testx/evm/embeds/ERC20Minter.sol (2)
1-1: Add a file-level comment.Consider adding a file-level comment to describe the purpose and functionality of the
ERC20Mintercontract.// ERC20Minter.sol // This contract extends OpenZeppelin's ERC20 implementation to include minting and burning functionalities.
31-34: Ensure proper role management.The constructor mentions granting roles, but there is no actual role management code. Ensure that role management is properly implemented if required.
constructor(string memory name, string memory symbol, uint8 decimals_) ERC20(name, symbol) { _setupDecimals(decimals_); // Ensure proper role management if required }x/evm/deps.go (1)
34-35: Add documentation for new methods.The new methods
GetDenomMetaDataandSetDenomMetaDatain theBankKeeperinterface lack documentation. Add comments to describe their purpose and usage.// GetDenomMetaData returns the metadata for a given denomination. GetDenomMetaData(ctx sdk.Context, denom string) (metadata bank.Metadata, isFound bool) // SetDenomMetaData sets the metadata for a given denomination. SetDenomMetaData(ctx sdk.Context, denomMetaData bank.Metadata)x/evm/const.go (2)
53-53: Add test coverage forPrefixAccStateEthAddr.The function
PrefixAccStateEthAddris not covered by tests. Ensure that test cases are added to cover this function.Tools
GitHub Check: codecov/patch
[warning] 53-53: x/evm/const.go#L53
Added line #L53 was not covered by tests
58-58: Add test coverage forStateKey.The function
StateKeyis not covered by tests. Ensure that test cases are added to cover this function.Tools
GitHub Check: codecov/patch
[warning] 58-58: x/evm/const.go#L58
Added line #L58 was not covered by testsx/evm/keeper/funtoken_state_test.go (3)
Line range hint
11-33: LGTM!The
TestInsertAndGetfunction correctly tests the insertion and retrieval ofFunToken.
Line range hint
34-62: LGTM!The
TestCollectfunction correctly tests the collection ofFunTokenbyBankDenomandERC20Addr.
Line range hint
63-95: LGTM!The
TestDeletefunction correctly tests the deletion ofFunToken.x/evm/keeper/erc20_test.go (1)
16-95: LGTM!The
TestCreateFunTokenFromERC20function correctly tests the creation ofFunTokenfrom ERC20.x/evm/keeper/precompiles.go (6)
17-32: LGTM!The
AvailablePrecompilesfunction correctly initializes an empty map for precompiled contracts and includes clear TODO comments.
Line range hint
35-43: LGTM!The
WithPrecompilesfunction correctly sets the available precompiled contracts and includes error handling for duplicate initialization.
Line range hint
45-60: LGTM!The
Precompilesfunction correctly filters and returns the active precompiled contracts.
Line range hint
62-97: LGTM!The
AddEVMExtensionsfunction correctly adds the precompiles and includes error handling for duplicates.
Line range hint
99-103: LGTM!The
IsAvailablePrecompilefunction correctly checks for the availability of a precompile address.
Line range hint
105-122: LGTM!The
GetAvailablePrecompileAddrsfunction correctly returns the list of available precompile addresses and includes an optimized approach for performance.x/evm/json_tx_args.go (1)
34-34: Ensure backward compatibility for "data" and "input" fields.The addition of both "data" and "input" fields for backward compatibility is a good practice. Ensure all relevant parts of the codebase handle both fields correctly.
Verification successful
Ensure backward compatibility for "data" and "input" fields.
The
GetDatamethod injson_tx_args.gocorrectly checks for both "input" and "data" fields, ensuring backward compatibility. The usage ofJsonTxArgsin other files aligns with this implementation.
json_tx_args.go: TheGetDatamethod ensures backward compatibility by checking both fields.- Other files (
call_tx.go,sign_tx.go,backend.go,eth_api.go): The usage ofJsonTxArgsdoes not contradict the backward compatibility ensured by theGetDatamethod.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that both "data" and "input" fields are handled correctly in the codebase. # Test: Search for functions handling JsonTxArgs. Expect: Correct handling of both fields. rg --type python $'JsonTxArgs'Length of output: 60
Script:
#!/bin/bash # Description: Verify that both "data" and "input" fields are handled correctly in the Go codebase. # Test: Search for functions handling JsonTxArgs. Expect: Correct handling of both fields. rg 'JsonTxArgs' --type goLength of output: 5310
Script:
#!/bin/bash # Description: Extract and review code sections handling "data" and "input" fields in JsonTxArgs. # Extract methods from json_tx_args.go echo "Methods in json_tx_args.go:" rg --multiline --multiline-dotall --type go 'func \(([^)]+)\) (ToTransaction|ToMessage|GetData)\(' x/evm/json_tx_args.go # Extract usage in other files echo -e "\nUsage in other files:" rg --multiline --multiline-dotall --type go 'JsonTxArgs' eth/rpc/backend/ eth/rpc/rpcapi/Length of output: 3620
Script:
#!/bin/bash # Description: Extract method implementations and relevant code sections for JsonTxArgs. # Extract method implementations from json_tx_args.go echo "Method implementations in json_tx_args.go:" rg --multiline --multiline-dotall --type go 'func \(([^)]+)\) (ToTransaction|ToMessage|GetData)\(' x/evm/json_tx_args.go -A 20 # Extract relevant code sections from other files echo -e "\nRelevant code sections in other files:" rg --multiline --multiline-dotall --type go 'JsonTxArgs' eth/rpc/backend/ eth/rpc/rpcapi/ -A 10Length of output: 27229
proto/eth/evm/v1/tx.proto (2)
24-27: Ensure correct implementation ofCreateFunTokenRPC method.The
CreateFunTokenRPC method has been added. Ensure that the corresponding server implementation handles the method correctly.
185-200: ValidateMsgCreateFunTokenfields.Ensure that all fields in the
MsgCreateFunTokenmessage are correctly validated and used in the server implementation.Verification successful
To ensure that the fields in the
MsgCreateFunTokenmessage are correctly validated and used, we need to examine theValidateBasicfunction and the server implementation. Let's look into the details of these functions.
To ensure the server implementation is correctly using the
MsgCreateFunTokenfields, let's search for theCreateFunTokenfunction directly in themsg_server.gofile.
To locate the server implementation for
CreateFunToken, let's list all functions in themsg_server.gofile.
To correct the syntax error and list all functions in the
msg_server.gofile, let's rerun the script with the correct syntax.
To ensure we capture the relevant usage of
MsgCreateFunTokenin the server implementation, let's search forMsgCreateFunTokendirectly in themsg_server.gofile.
Validate
MsgCreateFunTokenfields are correctly handled.The
MsgCreateFunTokenfields are validated in theValidateBasicfunction withinx/evm/msg.go, ensuring proper validation forsender,from_erc20, andfrom_bank_denom. The server implementation inx/evm/keeper/msg_server.goreferencesMsgCreateFunToken, indicating its fields are used in the server logic.
x/evm/msg.go:ValidateBasicfunction validates all fields.x/evm/keeper/msg_server.go: ReferencesMsgCreateFunToken.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the validation and usage of MsgCreateFunToken fields. # Test: Search for the usage of MsgCreateFunToken fields. Expect: Correct validation and usage of all fields. rg --type go $'MsgCreateFunToken'Length of output: 5693
Script:
#!/bin/bash # Extract the ValidateBasic function for MsgCreateFunToken ast-grep --lang go --pattern 'func (m *MsgCreateFunToken) ValidateBasic() error { $$$ }' # Extract the server implementation for MsgCreateFunToken ast-grep --lang go --pattern 'func (srv *MsgServer) CreateFunToken(ctx context.Context, msg *MsgCreateFunToken) (*MsgCreateFunTokenResponse, error) { $$$ }'Length of output: 1097
Script:
#!/bin/bash # Search for the CreateFunToken function in msg_server.go rg 'func (srv \*MsgServer) CreateFunToken(ctx context.Context, msg \*MsgCreateFunToken) \(\*MsgCreateFunTokenResponse, error\)' --context 10 --type go --glob 'x/evm/keeper/msg_server.go'Length of output: 186
Script:
#!/bin/bash # List all functions in msg_server.go ast-grep --lang go --pattern 'func $_($_, $_) $_ { $$$ }' --path 'x/evm/keeper/msg_server.go'Length of output: 281
Script:
#!/bin/bash # List all functions in msg_server.go ast-grep --lang go --pattern 'func $_($_, $_) $_ { $$$ }' x/evm/keeper/msg_server.goLength of output: 86
Script:
#!/bin/bash # Search for MsgCreateFunToken usage in msg_server.go rg 'MsgCreateFunToken' --type go --glob 'x/evm/keeper/msg_server.go'Length of output: 292
x/evm/keeper/erc20.go (1)
26-52: Ensure correct handling of ERC20 metadata inFindERC20Metadata.The
FindERC20Metadatafunction retrieves ERC20 metadata. Ensure that the function handles all possible errors and edge cases correctly.Verification successful
Verification successful for handling of ERC20 metadata in
FindERC20Metadata.The
FindERC20Metadatafunction is adequately tested for various edge cases and errors inerc20_test.go. The tests ensure that the function handles non-existent contract addresses and validates the metadata for deployed contracts correctly.
erc20_test.go: Lines covering test cases forFindERC20Metadata.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of ERC20 metadata in FindERC20Metadata. # Test: Search for the usage of FindERC20Metadata. Expect: Correct handling of all possible errors and edge cases. rg --type go $'FindERC20Metadata'Length of output: 544
Script:
#!/bin/bash # Extract the relevant error handling code from erc20_test.go to verify edge cases and errors rg --context 10 'FindERC20Metadata' x/evm/keeper/erc20_test.goLength of output: 1689
eth/rpc/rpcapi/eth_api_test.go (2)
21-21: Importembedspackage: Ensure usage of embedded contracts.The import statement for the
embedspackage indicates the usage of embedded contracts within the tests. Ensure that the embedded contracts are correctly utilized and tested.
46-46: Ensure correct initialization and usage ofcontractData.The
contractDatafield is added to theTestSuitestruct. Verify that it is correctly initialized and used within the test cases.x/evm/msg.go (6)
36-36: AddedMsgCreateFunTokenmessage type: Ensure proper implementation.The
MsgCreateFunTokenmessage type is added. Ensure that it is correctly implemented and integrated within the EVM module.
185-185: Ensure clarity of theTxData.Validateerror message.The error message for
TxData.Validatefailure should provide enough context for debugging.
476-481: Verify correctness ofGetSignersmethod forMsgCreateFunToken.Ensure that the
GetSignersmethod correctly returns the expected signers for theMsgCreateFunTokenmessage.
483-485: Verify clarity and usefulness of error messages inerrMsgCreateFunTokenValidate.Ensure that the
errMsgCreateFunTokenValidatefunction provides clear and useful error messages for debugging.
487-503: Verify thoroughness of validation checks inValidateBasicforMsgCreateFunToken.Ensure that the
ValidateBasicmethod performs all necessary validation checks for theMsgCreateFunTokenmessage.
505-508: Verify correctness ofGetSignBytesmethod forMsgCreateFunToken.Ensure that the
GetSignBytesmethod correctly returns the sign bytes for theMsgCreateFunTokenmessage.app/server/config/server_config.go (2)
59-60: RenamedDefaultGasCaptoDefaultEthCallGasLimit: Verify consistency.The constant
DefaultGasCapis renamed toDefaultEthCallGasLimit. Ensure that this change is consistently applied across the codebase.
249-249: UpdatedGasCapfield to useDefaultEthCallGasLimit: Verify correctness.Ensure that the
GasCapfield in theDefaultJSONRPCConfigcorrectly uses the newDefaultEthCallGasLimitconstant.x/evm/keeper/msg_server.go (6)
17-17: Importgethcommonpackage: Ensure consistent usage.The import statement for the
gethcommonpackage indicates a shift from usingcommon. Ensure consistent usage across the codebase.
150-150: UpdatedcontractAddrto usegethcommon.Address: Verify correctness.Ensure that the
contractAddrvariable, now usinggethcommon.Address, is correctly used throughout the function.
225-225: UpdatedtxConfigto usegethcommon.BytesToHash: Verify correctness.Ensure that the
txConfiginitialization, now usinggethcommon.BytesToHash, is correctly used throughout the function.
Line range hint
275-321: UpdatedGetHashFnto returngethcommon.Hash: Verify correctness.Ensure that the
GetHashFnfunction, now returninggethcommon.Hash, is correctly used throughout the function.
390-390: UpdatedactivePrecompilesto usegethcommon.Address: Verify correctness.Ensure that the
activePrecompilesinitialization, now usinggethcommon.Address, is correctly used throughout the function.
508-537: Verify correctness ofCreateFunTokenmethod inKeeper.Ensure that the
CreateFunTokenmethod is correctly implemented and integrated within the EVM module.x/evm/keeper/grpc_query_test.go (1)
19-19: Ensure that the new import is used correctly.The new import from the
embedspackage should be utilized correctly in the test cases. Verify the usage to ensure it aligns with the intended functionality.Verification successful
The new import is used correctly.
The
embedspackage import is utilized appropriately within the test cases to load a smart contract and use its bytecode in the test scenario.
grpc_query_test.go: Correct usage ofembeds.SmartContract_FunToken.Load().Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `embeds` package in the test cases. # Test: Search for the usage of the `embeds` package. Expect: Valid usage in test cases. rg --type go 'embeds' x/evm/keeper/grpc_query_test.goLength of output: 170
Script:
#!/bin/bash # Description: Extract the surrounding lines of the `embeds.SmartContract_FunToken.Load()` usage in the grpc_query_test.go file to understand its context. # Extract 5 lines before and after the usage of `embeds.SmartContract_FunToken.Load()` rg --type go -A 5 -B 5 'embeds.SmartContract_FunToken.Load()' x/evm/keeper/grpc_query_test.goLength of output: 504
x/evm/embeds/ERC20MinterCompiled.json (23)
1-4: Metadata section looks good.The metadata section correctly includes the format, contract name, and source name.
5-25: Constructor definition looks good.The constructor correctly includes inputs for
name,symbol, anddecimals_.
26-50: Event definitions look good.The
ApprovalandOwnershipTransferredevents are correctly defined.
51-95: Event definitions look good.The
Transferevent is correctly defined.
96-118: Functionallowancelooks good.The
allowancefunction correctly includes inputs forownerandspenderand outputs auint256.
119-142: Functionapprovelooks good.The
approvefunction correctly includes inputs forspenderandamountand outputs abool.
143-161: FunctionbalanceOflooks good.The
balanceOffunction correctly includes an input foraccountand outputs auint256.
162-192: FunctionburnandburnCoinslook good.The
burnandburnCoinsfunctions correctly include inputs foramountandfromrespectively, and have no outputs.
193-210: FunctionburnFromlooks good.The
burnFromfunction correctly includes inputs foraccountandamountand has no outputs.
211-223: Functiondecimalslooks good.The
decimalsfunction correctly outputs auint8.
224-247: FunctiondecreaseAllowancelooks good.The
decreaseAllowancefunction correctly includes inputs forspenderandsubtractedValueand outputs abool.
248-271: FunctionincreaseAllowancelooks good.The
increaseAllowancefunction correctly includes inputs forspenderandaddedValueand outputs abool.
272-289: Functionmintlooks good.The
mintfunction correctly includes inputs fortoandamountand has no outputs.
290-302: Functionnamelooks good.The
namefunction correctly outputs astring.
303-315: Functionownerlooks good.The
ownerfunction correctly outputs anaddress.
316-322: FunctionrenounceOwnershiplooks good.The
renounceOwnershipfunction has no inputs or outputs.
323-335: Functionsymbollooks good.The
symbolfunction correctly outputs astring.
336-348: FunctiontotalSupplylooks good.The
totalSupplyfunction correctly outputs auint256.
349-372: Functiontransferlooks good.The
transferfunction correctly includes inputs fortoandamountand outputs abool.
373-401: FunctiontransferFromlooks good.The
transferFromfunction correctly includes inputs forfrom,to, andamountand outputs abool.
402-414: FunctiontransferOwnershiplooks good.The
transferOwnershipfunction correctly includes an input fornewOwnerand has no outputs.
417-418: Bytecode section looks good.The bytecode section includes the compiled bytecode and deployed bytecode.
419-420: Link references section looks good.The link references section includes
linkReferencesanddeployedLinkReferences.CHANGELOG.md (1)
79-79: Ensure clarity and completeness of the changelog entry.The changelog entry for PR #1950 is clear and concise. It accurately describes the feature added: "Tx to create FunToken mapping from ERC20, contract embeds, and ERC20 queries."
| func TestLoadContracts(t *testing.T) { | ||
| for _, tc := range []embeds.SmartContractFixture{ | ||
| embeds.SmartContract_FunToken, | ||
| embeds.SmartContract_ERC20Minter, | ||
| } { | ||
| t.Run(tc.Name, func(t *testing.T) { | ||
| _, err := tc.Load() | ||
| assert.NoError(t, err) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Consider adding more test cases.
The test function TestLoadContracts currently only checks for the absence of errors when loading contracts. Consider adding more test cases to validate the loaded contract data.
func TestLoadContracts(t *testing.T) {
for _, tc := range []embeds.SmartContractFixture{
embeds.SmartContract_FunToken,
embeds.SmartContract_ERC20Minter,
} {
t.Run(tc.Name, func(t *testing.T) {
contract, err := tc.Load()
assert.NoError(t, err)
assert.NotNil(t, contract)
// Add more assertions to validate contract data
})
}
}| function burnCoins(address from, uint256 amount) public virtual onlyOwner { | ||
| _burn(from, amount); | ||
| } |
There was a problem hiding this comment.
Clarify the requirements for burnCoins.
The burnCoins function's requirements comment mentions MINTER_ROLE, but the function uses onlyOwner. Clarify or correct the comment.
/**
* @dev Destroys `amount` tokens from `from`.
*
* See {ERC20-_burn}.
*
* Requirements:
*
* - the caller must be the owner.
*/
function burnCoins(address from, uint256 amount) public virtual onlyOwner {
_burn(from, amount);
}
x/evm/const.go
Outdated
| func ModuleAddressEVM() gethcommon.Address { | ||
| if len(evmModuleAddr) == 0 { | ||
| evmModuleAddr = gethcommon.BytesToAddress( | ||
| authtypes.NewModuleAddress(ModuleName).Bytes(), | ||
| ) |
There was a problem hiding this comment.
Tip
Codebase Verification
Add test coverage for ModuleAddressEVM.
The function ModuleAddressEVM is not covered by tests. Ensure that test cases are added to cover this function.
- No instances of
ModuleAddressEVMwere found in test files.
Analysis chain
Add test coverage for ModuleAddressEVM.
The function ModuleAddressEVM is not covered by tests. Ensure that test cases are added to cover this function.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test coverage for ModuleAddressEVM.
# Test: Search for tests covering ModuleAddressEVM. Expect: At least one test case.
rg --type go --pattern 'ModuleAddressEVM'
Length of output: 453
Script:
#!/bin/bash
# Description: Verify test coverage for ModuleAddressEVM.
# Test: Search for tests covering ModuleAddressEVM. Expect: At least one test case.
rg 'ModuleAddressEVM' --type go
Length of output: 273
Script:
#!/bin/bash
# Description: Verify test coverage for ModuleAddressEVM.
# Test: Search within test files for tests covering ModuleAddressEVM. Expect: At least one test case.
rg 'ModuleAddressEVM' --type go --glob '*_test.go'
Length of output: 50
Tools
GitHub Check: codecov/patch
[warning] 77-81: x/evm/const.go#L77-L81
Added lines #L77 - L81 were not covered by tests
| func init() { | ||
| out, err := SmartContract_ERC20Minter.Load() | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| EmbeddedContractERC20Minter = out | ||
| } |
There was a problem hiding this comment.
Add test coverage for the init function.
The init function initializes the EmbeddedContractERC20Minter but lacks test coverage.
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] 33-33: x/evm/embeds/embeds.go#L33
Added line #L33 was not covered by tests
| @@ -182,31 +233,33 @@ | |||
| ) | |||
| require.NoError(t, err) | |||
| nonce = deps.StateDB().GetNonce(deps.Sender.EthAddr) | |||
| txArgs = evm.JsonTxArgs{ | |||
| txArgs := evm.JsonTxArgs{ | |||
There was a problem hiding this comment.
Add test coverage for txArgs in DeployAndExecuteERC20Transfer.
Ensure to add test cases to validate the construction of txArgs in the DeployAndExecuteERC20Transfer function.
// Example test case for txArgs
func TestTxArgsInDeployAndExecuteERC20Transfer(t *testing.T) {
deps := setupTestDeps(t)
_, err := DeployContract(deps, embeds.SmartContract_FunToken, t)
require.NoError(t, err)
ethTxMsg, predecessors := DeployAndExecuteERC20Transfer(deps, t)
require.NotNil(t, ethTxMsg, "Ethereum transaction message should not be nil")
require.NotEmpty(t, predecessors, "Predecessors should not be empty")
expectedTxArgs := evm.JsonTxArgs{
From: &deps.Sender.EthAddr,
To: ðTxMsg.To,
Nonce: (*hexutil.Uint64)(ðTxMsg.Nonce),
Data: (*hexutil.Bytes)(ðTxMsg.Data),
}
require.Equal(t, expectedTxArgs, ethTxMsg, "TxArgs should match the expected value")
}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.
| txArgs := evm.JsonTxArgs{ | |
| // Example test case for txArgs | |
| func TestTxArgsInDeployAndExecuteERC20Transfer(t *testing.T) { | |
| deps := setupTestDeps(t) | |
| _, err := DeployContract(deps, embeds.SmartContract_FunToken, t) | |
| require.NoError(t, err) | |
| ethTxMsg, predecessors := DeployAndExecuteERC20Transfer(deps, t) | |
| require.NotNil(t, ethTxMsg, "Ethereum transaction message should not be nil") | |
| require.NotEmpty(t, predecessors, "Predecessors should not be empty") | |
| expectedTxArgs := evm.JsonTxArgs{ | |
| From: &deps.Sender.EthAddr, | |
| To: ðTxMsg.To, | |
| Nonce: (*hexutil.Uint64)(ðTxMsg.Nonce), | |
| Data: (*hexutil.Bytes)(ðTxMsg.Data), | |
| } | |
| require.Equal(t, expectedTxArgs, ethTxMsg, "TxArgs should match the expected value") | |
| } |
Tools
GitHub Check: codecov/patch
[warning] 236-236: x/evm/evmtest/tx.go#L236
Added line #L236 was not covered by tests
| From: &deps.Sender.EthAddr, | ||
| To: &contractAddress, | ||
| Nonce: (*hexutil.Uint64)(&nonce), | ||
| Data: (*hexutil.Bytes)(&input), | ||
| } | ||
| ethTxMsg, err = GenerateAndSignEthTxMsg(txArgs, deps) | ||
| ethTxMsg, err := GenerateAndSignEthTxMsg(txArgs, deps) |
There was a problem hiding this comment.
Add test coverage for GenerateAndSignEthTxMsg in DeployAndExecuteERC20Transfer.
Ensure to add test cases to validate the GenerateAndSignEthTxMsg function call in DeployAndExecuteERC20Transfer.
// Example test case for GenerateAndSignEthTxMsg
func TestGenerateAndSignEthTxMsgInDeployAndExecuteERC20Transfer(t *testing.T) {
deps := setupTestDeps(t)
_, err := DeployContract(deps, embeds.SmartContract_FunToken, t)
require.NoError(t, err)
ethTxMsg, predecessors := DeployAndExecuteERC20Transfer(deps, t)
require.NotNil(t, ethTxMsg, "Ethereum transaction message should not be nil")
require.NotEmpty(t, predecessors, "Predecessors should not be empty")
signedTxMsg, err := GenerateAndSignEthTxMsg(ethTxMsg, deps)
require.NoError(t, err, "GenerateAndSignEthTxMsg should not return an error")
require.NotNil(t, signedTxMsg, "Signed Ethereum transaction message should not be nil")
}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.
| ethTxMsg, err := GenerateAndSignEthTxMsg(txArgs, deps) | |
| // Example test case for GenerateAndSignEthTxMsg | |
| func TestGenerateAndSignEthTxMsgInDeployAndExecuteERC20Transfer(t *testing.T) { | |
| deps := setupTestDeps(t) | |
| _, err := DeployContract(deps, embeds.SmartContract_FunToken, t) | |
| require.NoError(t, err) | |
| ethTxMsg, predecessors := DeployAndExecuteERC20Transfer(deps, t) | |
| require.NotNil(t, ethTxMsg, "Ethereum transaction message should not be nil") | |
| require.NotEmpty(t, predecessors, "Predecessors should not be empty") | |
| signedTxMsg, err := GenerateAndSignEthTxMsg(ethTxMsg, deps) | |
| require.NoError(t, err, "GenerateAndSignEthTxMsg should not return an error") | |
| require.NotNil(t, signedTxMsg, "Signed Ethereum transaction message should not be nil") | |
| } |
Tools
GitHub Check: codecov/patch
[warning] 242-242: x/evm/evmtest/tx.go#L242
Added line #L242 was not covered by tests
| require.NoError(t, err) | ||
|
|
||
| resp, err := deps.Chain.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg) | ||
| require.NoError(t, err) | ||
| require.Empty(t, resp.VmError) | ||
|
|
||
| contractAddress := crypto.CreateAddress(deps.Sender.EthAddr, nonce) |
There was a problem hiding this comment.
Add test coverage for contractAddress in DeployContract.
Ensure to add test cases to validate the construction of contractAddress in the DeployContract function.
// Example test case for contractAddress
func TestContractAddress(t *testing.T) {
deps := setupTestDeps(t)
contract := embeds.SmartContract_FunToken
args := []any{"arg1", "arg2"}
result, err := DeployContract(deps, contract, t, args...)
require.NoError(t, err, "DeployContract should not return an error")
expectedAddress := crypto.CreateAddress(deps.Sender.EthAddr, result.Nonce)
require.Equal(t, expectedAddress, result.ContractAddr, "Contract address should match the expected value")
}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.
| contractAddress := crypto.CreateAddress(deps.Sender.EthAddr, nonce) | |
| // Example test case for contractAddress | |
| func TestContractAddress(t *testing.T) { | |
| deps := setupTestDeps(t) | |
| contract := embeds.SmartContract_FunToken | |
| args := []any{"arg1", "arg2"} | |
| result, err := DeployContract(deps, contract, t, args...) | |
| require.NoError(t, err, "DeployContract should not return an error") | |
| expectedAddress := crypto.CreateAddress(deps.Sender.EthAddr, result.Nonce) | |
| require.Equal(t, expectedAddress, result.ContractAddr, "Contract address should match the expected value") | |
| } |
Tools
GitHub Check: codecov/patch
[warning] 202-202: x/evm/evmtest/tx.go#L202
Added line #L202 was not covered by tests
| require.NoError(t, err) | ||
|
|
||
| resp, err = deps.Chain.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg) | ||
| resp, err := deps.Chain.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg) |
There was a problem hiding this comment.
Add test coverage for EthereumTx in DeployAndExecuteERC20Transfer.
Ensure to add test cases to validate the EthereumTx function call in DeployAndExecuteERC20Transfer.
// Example test case for EthereumTx
func TestEthereumTxInDeployAndExecuteERC20Transfer(t *testing.T) {
deps := setupTestDeps(t)
_, err := DeployContract(deps, embeds.SmartContract_FunToken, t)
require.NoError(t, err)
ethTxMsg, predecessors := DeployAndExecuteERC20Transfer(deps, t)
require.NotNil(t, ethTxMsg, "Ethereum transaction message should not be nil")
require.NotEmpty(t, predecessors, "Predecessors should not be empty")
resp, err := deps.Chain.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg)
require.NoError(t, err, "EthereumTx should not return an error")
require.Empty(t, resp.VmError, "VM error should be empty")
}Committable suggestion was skipped due to low confidence.
Tools
GitHub Check: codecov/patch
[warning] 245-245: x/evm/evmtest/tx.go#L245
Added line #L245 was not covered by tests
| return DeployContractResult{ | ||
| TxResp: resp, | ||
| EthTxMsg: ethTxMsg, | ||
| ContractData: contractData, | ||
| Nonce: nonce, | ||
| ContractAddr: contractAddress, | ||
| }, nil |
There was a problem hiding this comment.
Add test coverage for DeployContractResult.
Ensure to add test cases to validate the DeployContractResult structure.
// Example test case for DeployContractResult
func TestDeployContractResult(t *testing.T) {
deps := setupTestDeps(t)
contract := embeds.SmartContract_FunToken
args := []any{"arg1", "arg2"}
result, err := DeployContract(deps, contract, t, args...)
require.NoError(t, err, "DeployContract should not return an error")
expectedResult := DeployContractResult{
TxResp: result.TxResp,
EthTxMsg: result.EthTxMsg,
ContractData: result.ContractData,
Nonce: result.Nonce,
ContractAddr: result.ContractAddr,
}
require.Equal(t, expectedResult, result, "DeployContractResult should match the expected value")
}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.
| return DeployContractResult{ | |
| TxResp: resp, | |
| EthTxMsg: ethTxMsg, | |
| ContractData: contractData, | |
| Nonce: nonce, | |
| ContractAddr: contractAddress, | |
| }, nil | |
| return DeployContractResult{ | |
| TxResp: resp, | |
| EthTxMsg: ethTxMsg, | |
| ContractData: contractData, | |
| Nonce: nonce, | |
| ContractAddr: contractAddress, | |
| }, nil | |
| ``` | |
| // Example test case for DeployContractResult | |
| func TestDeployContractResult(t *testing.T) { | |
| deps := setupTestDeps(t) | |
| contract := embeds.SmartContract_FunToken | |
| args := []any{"arg1", "arg2"} | |
| result, err := DeployContract(deps, contract, t, args...) | |
| require.NoError(t, err, "DeployContract should not return an error") | |
| expectedResult := DeployContractResult{ | |
| TxResp: result.TxResp, | |
| EthTxMsg: result.EthTxMsg, | |
| ContractData: result.ContractData, | |
| Nonce: result.Nonce, | |
| ContractAddr: result.ContractAddr, | |
| } | |
| require.Equal(t, expectedResult, result, "DeployContractResult should match the expected value") | |
| } |
Tools
GitHub Check: codecov/patch
[warning] 204-210: x/evm/evmtest/tx.go#L204-L210
Added lines #L204 - L210 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- x/evm/const.go (3 hunks)
- x/evm/evm_test.go (2 hunks)
Additional context used
GitHub Check: codecov/patch
x/evm/const.go
[warning] 53-53: x/evm/const.go#L53
Added line #L53 was not covered by tests
[warning] 58-58: x/evm/const.go#L58
Added line #L58 was not covered by tests
Additional comments not posted (2)
x/evm/evm_test.go (2)
Line range hint
8-114: LGTM!The test cases for
TestFunTokencover various scenarios and are well-structured.
116-124: LGTM!The test cases for
TestModuleAddressEVMcover the validation of theModuleAddressEVMfunction and are well-structured.
| @@ -49,12 +50,12 @@ | |||
| var KeyPrefixBzAccState = KeyPrefixAccState.Prefix() | |||
|
|
|||
| // PrefixAccStateEthAddr returns a prefix to iterate over a given account storage. | |||
| func PrefixAccStateEthAddr(address common.Address) []byte { | |||
| func PrefixAccStateEthAddr(address gethcommon.Address) []byte { | |||
There was a problem hiding this comment.
Add test coverage for PrefixAccStateEthAddr.
The function PrefixAccStateEthAddr is not covered by tests. Ensure that test cases are added to cover this function.
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] 53-53: x/evm/const.go#L53
Added line #L53 was not covered by tests
| return append(KeyPrefixBzAccState, address.Bytes()...) | ||
| } | ||
|
|
||
| // StateKey defines the full key under which an account state is stored. | ||
| func StateKey(address common.Address, key []byte) []byte { | ||
| func StateKey(address gethcommon.Address, key []byte) []byte { |
There was a problem hiding this comment.
Add test coverage for StateKey.
The function StateKey is not covered by tests. Ensure that test cases are added to cover this function.
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] 58-58: x/evm/const.go#L58
Added line #L58 was not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 7
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- x/evm/const.go (3 hunks)
- x/evm/keeper/erc20.go (1 hunks)
- x/evm/keeper/erc20_from_coin.go (1 hunks)
- x/evm/keeper/erc20_test.go (1 hunks)
- x/evm/keeper/msg_server.go (7 hunks)
Files skipped from review as they are similar to previous changes (3)
- x/evm/const.go
- x/evm/keeper/erc20.go
- x/evm/keeper/msg_server.go
Additional comments not posted (3)
x/evm/keeper/erc20_test.go (3)
41-41: Make assertions more descriptive.Adding more descriptive messages to assertions can help with debugging test failures.
- s.NoError(err, info) + s.NoError(err, "Failed to find ERC20 metadata: %v", info)
146-146: Make assertions more descriptive.Adding more descriptive messages to assertions can help with debugging test failures.
- s.NoError(err, info) + s.NoError(err, "Failed to find ERC20 metadata: %v", info)
207-207: Make assertions more descriptive.Adding more descriptive messages to assertions can help with debugging test failures.
- s.NoError(err, info) + s.NoError(err, "Failed to find ERC20 metadata: %v", info)
| ) (funtoken evm.FunToken, err error) { | ||
| // 1 | Coin already registered with FunToken? | ||
| if funtokens := k.FunTokens.Collect( | ||
| ctx, k.FunTokens.Indexes.BankDenom.ExactMatch(ctx, bankDenom), | ||
| ); len(funtokens) > 0 { | ||
| return funtoken, fmt.Errorf("Funtoken mapping already created for bank denom \"%s\"", bankDenom) |
There was a problem hiding this comment.
Improve error handling and logging.
Consider adding more detailed error messages and logging to help with debugging.
- return funtoken, fmt.Errorf("Funtoken mapping already created for bank denom \"%s\"", bankDenom)
+ return funtoken, fmt.Errorf("Funtoken mapping already created for bank denom \"%s\"", bankDenom)Additionally, consider using a logger to log these errors.
import "log"
log.Printf("Funtoken mapping already created for bank denom \"%s\"", bankDenom)| // 2 | Check for denom metadata in bank state | ||
| bankCoin, isAlreadyCoin := k.bankKeeper.GetDenomMetaData(ctx, bankDenom) | ||
| if !isAlreadyCoin { | ||
| return funtoken, fmt.Errorf("Bank coin denom should have bank metadata for denom \"%s\"", bankDenom) | ||
| } |
There was a problem hiding this comment.
Use constants for error messages.
Using constants for error messages can enhance maintainability and readability.
const (
ErrMetadataNotFound = "Bank coin denom should have bank metadata for denom \"%s\""
)
return funtoken, fmt.Errorf(ErrMetadataNotFound, bankDenom)| if err != nil { | ||
| err = errors.Wrap(err, "failed to pack ABI args") | ||
| return |
There was a problem hiding this comment.
Improve error handling and logging.
Consider adding more detailed error messages and logging to help with debugging.
- err = errors.Wrap(err, "failed to pack ABI args")
+ err = errors.Wrap(err, "failed to pack ABI args for ERC20 contract deployment")
returnAdditionally, consider using a logger to log these errors.
import "log"
log.Printf("Failed to pack ABI args for ERC20 contract deployment: %v", err)| err = errors.Wrap(err, "failed to pack ABI args") | ||
| return | ||
| } | ||
| bytecodeForCall := append(erc20Embed.Bytecode, packedArgs...) |
There was a problem hiding this comment.
Break down complex expressions for readability.
Breaking down complex expressions can improve readability and maintainability.
bytecodeForCall := append(erc20Embed.Bytecode, packedArgs...)Consider breaking it down:
bytecode := erc20Embed.Bytecode
packedArgs, err := erc20Embed.ABI.Pack(methodName, callArgs...)
if err != nil {
err = errors.Wrap(err, "failed to pack ABI args for ERC20 contract deployment")
return
}
bytecodeForCall := append(bytecode, packedArgs...)| func (s *Suite) TestCreateFunTokenFromERC20() { | ||
| deps := evmtest.NewTestDeps() | ||
|
|
||
| // Compute contract address. FindERC20 should fail | ||
| nonce := deps.StateDB().GetNonce(deps.Sender.EthAddr) | ||
| contractAddress := crypto.CreateAddress(deps.Sender.EthAddr, nonce) | ||
| _, err := deps.K.FindERC20Metadata(deps.Ctx, contractAddress) | ||
| s.Error(err) | ||
|
|
||
| s.T().Log("Case 1: Deploy and invoke ERC20 for info") | ||
| { | ||
| metadata := keeper.ERC20Metadata{ | ||
| Name: "erc20name", | ||
| Symbol: "TOKEN", | ||
| Decimals: 18, | ||
| } | ||
| deployResp, err := evmtest.DeployContract( | ||
| &deps, embeds.SmartContract_ERC20Minter, s.T(), | ||
| metadata.Name, metadata.Symbol, metadata.Decimals, | ||
| ) | ||
| s.NoError(err) | ||
| s.Equal(contractAddress, deployResp.ContractAddr) | ||
|
|
||
| info, err := deps.K.FindERC20Metadata(deps.Ctx, deployResp.ContractAddr) | ||
| s.NoError(err, info) | ||
| s.Equal(metadata, info) | ||
| } | ||
|
|
||
| s.T().Log("Case 2: Deploy and invoke ERC20 for info") | ||
| { | ||
| metadata := keeper.ERC20Metadata{ | ||
| Name: "gwei", | ||
| Symbol: "GWEI", | ||
| Decimals: 9, | ||
| } | ||
| deployResp, err := evmtest.DeployContract( | ||
| &deps, embeds.SmartContract_ERC20Minter, s.T(), | ||
| metadata.Name, metadata.Symbol, metadata.Decimals, | ||
| ) | ||
| s.NoError(err) | ||
| s.NotEqual(contractAddress, deployResp.ContractAddr) | ||
|
|
||
| info, err := deps.K.FindERC20Metadata(deps.Ctx, deployResp.ContractAddr) | ||
| s.NoError(err, info) | ||
| s.Equal(metadata, info) | ||
| } | ||
|
|
||
| s.T().Log("happy: CreateFunToken for the ERC20") | ||
|
|
||
| erc20Addr := eth.NewHexAddr(contractAddress) | ||
| queryCodeReq := &evm.QueryCodeRequest{ | ||
| Address: erc20Addr.String(), | ||
| } | ||
| _, err = deps.K.Code(deps.Ctx, queryCodeReq) | ||
| s.NoError(err) | ||
|
|
||
| createFuntokenResp, err := deps.K.CreateFunToken( | ||
| deps.GoCtx(), | ||
| &evm.MsgCreateFunToken{ | ||
| FromErc20: erc20Addr, | ||
| Sender: deps.Sender.NibiruAddr.String(), | ||
| }, | ||
| ) | ||
| s.NoError(err, "erc20 %s", erc20Addr) | ||
| s.Equal( | ||
| createFuntokenResp.FuntokenMapping, | ||
| evm.FunToken{ | ||
| Erc20Addr: erc20Addr, | ||
| BankDenom: fmt.Sprintf("erc20/%s", erc20Addr.String()), | ||
| IsMadeFromCoin: false, | ||
| }) | ||
|
|
||
| s.T().Log("sad: CreateFunToken for the ERC20: already registered") | ||
| _, err = deps.K.CreateFunToken( | ||
| deps.GoCtx(), | ||
| &evm.MsgCreateFunToken{ | ||
| FromErc20: erc20Addr, | ||
| Sender: deps.Sender.NibiruAddr.String(), | ||
| }, | ||
| ) | ||
| s.ErrorContains(err, "Funtoken mapping already created") | ||
|
|
||
| s.T().Log("sad: CreateFunToken for the ERC20: invalid sender") | ||
| _, err = deps.K.CreateFunToken( | ||
| deps.GoCtx(), | ||
| &evm.MsgCreateFunToken{ | ||
| FromErc20: erc20Addr, | ||
| }, | ||
| ) | ||
| s.ErrorContains(err, "invalid sender") | ||
| } |
There was a problem hiding this comment.
Add more edge cases.
Consider adding more edge cases to test scenarios such as invalid ERC20 addresses and edge values for token decimals.
s.T().Log("Case 3: Deploy ERC20 with invalid address")
{
invalidAddress := gethcommon.Address{}
_, err := deps.K.FindERC20Metadata(deps.Ctx, invalidAddress)
s.Error(err)
}
s.T().Log("Case 4: Deploy ERC20 with edge decimals")
{
metadata := keeper.ERC20Metadata{
Name: "edgecase",
Symbol: "EDGE",
Decimals: 255,
}
deployResp, err := evmtest.DeployContract(
&deps, embeds.SmartContract_ERC20Minter, s.T(),
metadata.Name, metadata.Symbol, metadata.Decimals,
)
s.NoError(err)
s.NotEqual(contractAddress, deployResp.ContractAddr)
}| func (s *Suite) TestDeployERC20ForBankCoin() { | ||
| deps := evmtest.NewTestDeps() | ||
|
|
||
| // Compute contract address. FindERC20 should fail | ||
| nonce := deps.StateDB().GetNonce(deps.Sender.EthAddr) | ||
| contractAddress := crypto.CreateAddress(deps.Sender.EthAddr, nonce) | ||
| _, err := deps.K.FindERC20Metadata(deps.Ctx, contractAddress) | ||
| s.Error(err) | ||
|
|
||
| s.T().Log("Case 1: Deploy and invoke ERC20 for info") | ||
| bankDenom := "sometoken" | ||
| bankMetadata := bank.Metadata{ | ||
| DenomUnits: []*bank.DenomUnit{ | ||
| { | ||
| Denom: bankDenom, | ||
| Exponent: 0, | ||
| }, | ||
| }, | ||
| Base: bankDenom, | ||
| Display: bankDenom, | ||
| Name: bankDenom, | ||
| Symbol: "TOKEN", | ||
| } | ||
| erc20Addr, err := deps.K.DeployERC20ForBankCoin( | ||
| deps.Ctx, bankMetadata, | ||
| ) | ||
| s.NoError(err) | ||
| s.NotEqual(contractAddress, erc20Addr, | ||
| "address derived from before call should differ since the contract deployment succeeds") | ||
|
|
||
| s.T().Log("Expect ERC20 metadata on contract") | ||
| metadata := keeper.ERC20Metadata{ | ||
| Name: bankDenom, | ||
| Symbol: "TOKEN", | ||
| Decimals: 0, | ||
| } | ||
| info, err := deps.K.FindERC20Metadata(deps.Ctx, erc20Addr) | ||
| s.NoError(err, info) | ||
| s.Equal(metadata, info) | ||
| } |
There was a problem hiding this comment.
Add more edge cases.
Consider adding more edge cases to test scenarios such as invalid bank metadata and edge values for token decimals.
s.T().Log("Case 2: Deploy ERC20 with invalid bank metadata")
{
invalidMetadata := bank.Metadata{}
_, err := deps.K.DeployERC20ForBankCoin(deps.Ctx, invalidMetadata)
s.Error(err)
}
s.T().Log("Case 3: Deploy ERC20 with edge decimals")
{
bankMetadata := bank.Metadata{
DenomUnits: []*bank.DenomUnit{
{
Denom: bankDenom,
Exponent: 255,
},
},
Base: bankDenom,
Display: bankDenom,
Name: bankDenom,
Symbol: "EDGE",
}
erc20Addr, err := deps.K.DeployERC20ForBankCoin(deps.Ctx, bankMetadata)
s.NoError(err)
s.NotEqual(contractAddress, erc20Addr)
}| func (s *Suite) TestCreateFunTokenFromCoin() { | ||
| deps := evmtest.NewTestDeps() | ||
|
|
||
| // Compute contract address. FindERC20 should fail | ||
| nonce := deps.StateDB().GetNonce(deps.Sender.EthAddr) | ||
| contractAddress := crypto.CreateAddress(deps.Sender.EthAddr, nonce) | ||
| _, err := deps.K.FindERC20Metadata(deps.Ctx, contractAddress) | ||
| s.Error(err) | ||
|
|
||
| s.T().Log("Setup: Create a coin in the bank state") | ||
| bankDenom := "sometoken" | ||
| bankMetadata := bank.Metadata{ | ||
| DenomUnits: []*bank.DenomUnit{ | ||
| { | ||
| Denom: bankDenom, | ||
| Exponent: 0, | ||
| }, | ||
| }, | ||
| Base: bankDenom, | ||
| Display: bankDenom, | ||
| Name: bankDenom, | ||
| Symbol: "TOKEN", | ||
| } | ||
| deps.Chain.BankKeeper.SetDenomMetaData(deps.Ctx, bankMetadata) | ||
|
|
||
| s.T().Log("happy: CreateFunToken for the bank coin") | ||
| createFuntokenResp, err := deps.K.CreateFunToken( | ||
| deps.GoCtx(), | ||
| &evm.MsgCreateFunToken{ | ||
| FromBankDenom: bankDenom, | ||
| Sender: deps.Sender.NibiruAddr.String(), | ||
| }, | ||
| ) | ||
| s.NoError(err, "bankDenom %s", bankDenom) | ||
| erc20 := createFuntokenResp.FuntokenMapping.Erc20Addr | ||
| erc20Addr := erc20.ToAddr() | ||
| s.Equal( | ||
| createFuntokenResp.FuntokenMapping, | ||
| evm.FunToken{ | ||
| Erc20Addr: erc20, | ||
| BankDenom: bankDenom, | ||
| IsMadeFromCoin: true, | ||
| }) | ||
|
|
||
| s.T().Log("Expect ERC20 to be deployed") | ||
| queryCodeReq := &evm.QueryCodeRequest{ | ||
| Address: erc20Addr.String(), | ||
| } | ||
| _, err = deps.K.Code(deps.Ctx, queryCodeReq) | ||
| s.NoError(err) | ||
|
|
||
| s.T().Log("Expect ERC20 metadata on contract") | ||
| metadata := keeper.ERC20Metadata{ | ||
| Name: bankDenom, | ||
| Symbol: "TOKEN", | ||
| Decimals: 0, | ||
| } | ||
| info, err := deps.K.FindERC20Metadata(deps.Ctx, erc20Addr) | ||
| s.NoError(err, info) | ||
| s.Equal(metadata, info) | ||
|
|
||
| s.T().Log("sad: CreateFunToken for the bank coin: already registered") | ||
| _, err = deps.K.CreateFunToken( | ||
| deps.GoCtx(), | ||
| &evm.MsgCreateFunToken{ | ||
| FromBankDenom: bankDenom, | ||
| Sender: deps.Sender.NibiruAddr.String(), | ||
| }, | ||
| ) | ||
| s.ErrorContains(err, "Funtoken mapping already created") | ||
| } |
There was a problem hiding this comment.
Add more edge cases.
Consider adding more edge cases to test scenarios such as invalid bank denoms and edge values for token decimals.
s.T().Log("Case 2: Create FunToken with invalid bank denom")
{
invalidDenom := ""
_, err := deps.K.CreateFunToken(
deps.GoCtx(),
&evm.MsgCreateFunToken{
FromBankDenom: invalidDenom,
Sender: deps.Sender.NibiruAddr.String(),
},
)
s.Error(err)
}
s.T().Log("Case 3: Create FunToken with edge decimals")
{
bankMetadata := bank.Metadata{
DenomUnits: []*bank.DenomUnit{
{
Denom: bankDenom,
Exponent: 255,
},
},
Base: bankDenom,
Display: bankDenom,
Name: bankDenom,
Symbol: "EDGE",
}
deps.Chain.BankKeeper.SetDenomMetaData(deps.Ctx, bankMetadata)
createFuntokenResp, err := deps.K.CreateFunToken(
deps.GoCtx(),
&evm.MsgCreateFunToken{
FromBankDenom: bankDenom,
Sender: deps.Sender.NibiruAddr.String(),
},
)
s.NoError(err)
erc20 := createFuntokenResp.FuntokenMapping.Erc20Addr
erc20Addr := erc20.ToAddr()
s.Equal(
createFuntokenResp.FuntokenMapping,
evm.FunToken{
Erc20Addr: erc20,
BankDenom: bankDenom,
IsMadeFromCoin: true,
})
}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- x/common/testutil/cli/network.go (1 hunks)
- x/oracle/integration/app_test.go (2 hunks)
Additional comments not posted (5)
x/oracle/integration/app_test.go (3)
120-120: Ensure proper error handling and logging.Proper error handling is crucial for debugging and maintaining code quality. Ensure that errors are logged with sufficient context.
Confirm that error contexts are logged throughout the codebase.
126-127: Ensure proper error handling and logging.Proper error handling is crucial for debugging and maintaining code quality. Ensure that errors are logged with sufficient context.
Confirm that error contexts are logged throughout the codebase.
143-143: Ensure proper error handling and logging.Proper error handling is crucial for debugging and maintaining code quality. Ensure that errors are logged with sufficient context.
Confirm that error contexts are logged throughout the codebase.
x/common/testutil/cli/network.go (2)
652-654: Ensure proper error handling and logging.Proper error handling is crucial for debugging and maintaining code quality. Ensure that errors are logged with sufficient context.
Confirm that error contexts are logged throughout the codebase.
662-663: Review the sleep statement for potential issues.The sleep statement is used to mitigate address-in-use errors. Ensure this is the best approach and consider alternative solutions if necessary.
Confirm if the sleep statement is necessary and if there are alternative solutions.
| // TODO: Is there a cleaner way to do this with a synchronous check? | ||
| // https://github.com/NibiruChain/nibiru/issues/1955 | ||
|
|
There was a problem hiding this comment.
Consider addressing the TODO comment.
The TODO comment suggests there might be a better way to handle the cleanup process. Addressing this could improve the robustness of the cleanup function.
Would you like assistance in addressing this TODO comment?
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/evm/keeper/grpc_query_test.go (18 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/evm/keeper/grpc_query_test.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- app/server/config/server_config.go (2 hunks)
- x/evm/keeper/msg_server.go (7 hunks)
Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- app/server/config/server_config.go
- x/evm/keeper/msg_server.go
Purpose / Abstract
CreateFunTokenFromERC20#1941CreateFunTokenFromCoin#1942Summary of Changes
serves as the ABI embed for queries of ERC20 metadata
FunTokenmapping from an existingERC20.
DeployContracthelper function from theevmtestpackage toaccept variadic arguments that will be passed to the constructor of the EVM
contract. This is useful for deploying more complex contract types in tests.
Summary by CodeRabbit
New Features
Refactor
gethcommontypes instead ofcommonfor Ethereum addresses and hashes.Bug Fixes
eth_call/estimateGasoperations.