-
Notifications
You must be signed in to change notification settings - Fork 193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(evm): Combine both account queries into "/eth.evm.v1.Query/EthAccount", accepting both nibi-prefixed Bech32 addresses and Ethereum-type hexadecimal addresses as input. #1986
Conversation
…cts manipulate in wei units
commit e171a5e Merge: 4419067 3c73f03 Author: Unique-Divine <[email protected]> Date: Mon Aug 5 06:01:44 2024 -0500 Merge branch 'main' into ud/attonibi commit 4419067 Author: Unique-Divine <[email protected]> Date: Mon Aug 5 03:04:46 2024 +0200 test(statedb_test.go): more thorough test cases commit 18995b6 Author: Unique-Divine <[email protected]> Date: Mon Aug 5 01:25:14 2024 +0200 test(statedb): complete the wei-based account migration. Remove all mocks commit 9cd1edf Author: Unique-Divine <[email protected]> Date: Sun Aug 4 04:32:31 2024 +0200 chore: wei unit migration commit cd9642b Author: Unique-Divine <[email protected]> Date: Sun Aug 4 04:27:33 2024 +0200 math functions for unibi and wei commit a1c5782 Author: Unique-Divine <[email protected]> Date: Sun Aug 4 04:26:58 2024 +0200 refactor(statedb): separate Account and AccountWei to have state objects manipulate in wei units commit e106002 Author: Unique-Divine <[email protected]> Date: Tue Jul 30 02:25:34 2024 +0100 changelog commit 3335463 Author: Unique-Divine <[email protected]> Date: Tue Jul 30 02:23:33 2024 +0100 refactor: use pebbledb as the test db commit 1f0d8d0 Author: Unique-Divine <[email protected]> Date: Tue Jul 30 01:51:38 2024 +0100 refactor: remove unused vars. improve error clarity for testnetwork/New
…count", accepting both nibi-prefixed Bech32 addresses and Ethereum-type hexadecimal addresses as input.
WalkthroughThe recent updates to the NibiruChain project significantly enhance Ethereum Virtual Machine (EVM) integration and optimize account management. Key features include the introduction of atto denominations for wei, streamlined account queries supporting both Ethereum hexadecimal and Nibiru Bech32 addresses, and improved error handling. These changes emphasize better interoperability, refine balance representation, and enhance the testing framework for greater reliability and clarity. 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 (
|
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: 6
Outside diff range, codebase verification and nitpick comments (6)
x/evm/keeper/statedb_test.go (1)
21-23
: Consider using sub-tests for better organization.Using sub-tests with
t.Run
can improve the readability and organization of the test cases.func (s *Suite) TestStateDBBalance() { deps := evmtest.NewTestDeps() s.Run("Initial balance check", func() { db := deps.StateDB() s.Equal("0", db.GetBalance(deps.Sender.EthAddr).String()) }) s.Run("Fund account and check balance", func() { // existing test code for funding account and checking balance }) s.Run("EVM transfer and balance check", func() { // existing test code for EVM transfer and checking balance }) s.Run("Bank transfer and balance check", func() { // existing test code for bank transfer and checking balance }) }eth/rpc/backend/account_info.go (1)
178-180
: Improve error message clarity.The error message can be more descriptive to aid in debugging.
- return nil, fmt.Errorf("invalid balance: %s", res.BalanceWei) + return nil, fmt.Errorf("invalid balance retrieved from BalanceWei: %s", res.BalanceWei)x/evm/statedb/state_object.go (3)
43-49
: Ensure direct tests forToWei
method.The
ToWei
method is not directly tested in the codebase. While the underlyingNativeToWei
function is well-tested, it's important to have direct tests for theToWei
method to ensure comprehensive coverage.
- Add direct tests for the
ToWei
method in the appropriate test files.Analysis chain
LGTM! Ensure conversion logic is verified.
The
ToWei
method correctly converts the balance from native units to wei.However, ensure that the conversion logic is thoroughly tested.
Let's search for direct tests of the
ToWei
method to ensure that it is thoroughly tested.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `ToWei` conversion logic is thoroughly tested. # Test: Search for tests related to the `ToWei` method. Expect: Thorough testing. rg --type go 'ToWei'Length of output: 2008
Script:
#!/bin/bash # Description: Search for direct tests of the `ToWei` method. # Test: Search for direct invocations of the `ToWei` method in test files. rg --type go 'ToWei()' --glob '*_test.go'Length of output: 1236
126-136
: Ensure explicit tests fornewObject
initialization logic.The current tests in
statedb_test.go
do not explicitly cover thenewObject
method or the initialization ofstateObject
. To ensure robustness, add direct tests fornewObject
to verify the initialization logic.
- Add tests to cover the initialization of
stateObject
innewObject
.- Verify that all fields in
stateObject
are correctly initialized.Analysis chain
Ensure proper initialization of
stateObject
.The method
newObject
correctly initializes thestateObject
with the newAccountWei
structure.However, ensure that the initialization logic is thoroughly tested.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `newObject` initialization logic is thoroughly tested. # Test: Search for tests related to the `newObject` method. Expect: Thorough testing. rg --type go 'newObject'Length of output: 332
Script:
#!/bin/bash # Description: Verify that the `newObject` initialization logic is thoroughly tested. # Step 1: Find all test files in the `x/evm/statedb` directory. test_files=$(fd --type f --extension go --glob '*_test.go' x/evm/statedb) # Step 2: Search for references to `stateObject` initialization in the test files. for file in $test_files; do echo "Searching in $file" rg 'stateObject' "$file" doneLength of output: 273
Script:
#!/bin/bash # Description: Verify that the `newObject` initialization logic is thoroughly tested by inspecting the `statedb_test.go` file. # Step 1: Print the contents of the `statedb_test.go` file to inspect for any indirect tests covering `newObject` or `stateObject` initialization. cat x/evm/statedb/statedb_test.goLength of output: 17551
51-61
: EnsureToNative
method is thoroughly tested.The
ToNative
method does not appear to be directly tested in the codebase. Consider adding specific test cases to ensure the conversion logic is thoroughly verified.
- File:
x/evm/statedb/state_object.go
- Method:
ToNative
Analysis chain
LGTM! Ensure conversion logic is verified.
The
ToNative
method correctly converts the balance from wei to native units.However, ensure that the conversion logic is thoroughly tested.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `ToNative` conversion logic is thoroughly tested. # Test: Search for tests related to the `ToNative` method. Expect: Thorough testing. rg --type go 'ToNative'Length of output: 760
Script:
#!/bin/bash # Description: Verify that the `ToNative` conversion logic is thoroughly tested by searching for test files and functions. # Search for Go test files that might include tests for the `ToNative` method. fd --type f --extension go --exec grep -l 'func Test' {} + # Search for test functions that might involve `AccountWei` and `Account`. rg --type go 'AccountWei|Account' --glob '*_test.go'Length of output: 47421
Script:
#!/bin/bash # Description: Verify that the `ToNative` conversion logic is thoroughly tested by searching for specific test functions in the statedb_test.go file. # Search for test functions in the statedb_test.go file. rg --type go 'func Test' x/evm/statedb/statedb_test.go # Search for usages of `ToNative` in the statedb_test.go file. rg --type go 'ToNative' x/evm/statedb/statedb_test.goLength of output: 141
x/evm/keeper/grpc_query_test.go (1)
101-108
: Ensure proper error handling in setup functions.The setup functions in
grpc_query_test.go
do not consistently handle errors. Ensure that all setup functions handle errors appropriately, similar to the ones usings.Require().NoError(err)
.
setup: func(deps *evmtest.TestDeps) { ... }
functions should include error handling.Analysis chain
Ensure proper error handling in setup function.
The setup function funds an account with tokens. Ensure that errors are properly handled and logged.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper error handling in setup functions. # Test: Search for error handling in setup functions. Expect: Proper error handling and logging. rg --type go 'setup: func\(deps \*evmtest.TestDeps\) \{.*err := .*; s.Require\(\).NoError\(err\)' --context 5Length of output: 111
Script:
#!/bin/bash # Description: Verify proper error handling in setup functions in grpc_query_test.go. # Test: Search for any error handling in setup functions within grpc_query_test.go. rg --type go 'setup: func\(deps \*evmtest.TestDeps\) \{.*err := .*' -A 10 x/evm/keeper/grpc_query_test.goLength of output: 107
Script:
#!/bin/bash # Description: Find all setup functions in grpc_query_test.go for manual inspection. # Test: Search for all setup functions within grpc_query_test.go. rg --type go 'setup: func\(deps \*evmtest.TestDeps\)' -A 10 x/evm/keeper/grpc_query_test.goLength of output: 3185
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
x/evm/query.pb.go
is excluded by!**/*.pb.go
x/evm/query.pb.gw.go
is excluded by!**/*.pb.gw.go
Files selected for processing (33)
- CHANGELOG.md (1 hunks)
- app/evmante/evmante_can_transfer.go (1 hunks)
- app/evmante/evmante_can_transfer_test.go (2 hunks)
- app/evmante/evmante_gas_consume_test.go (2 hunks)
- app/evmante/evmante_handler_test.go (2 hunks)
- app/evmante/evmante_verify_eth_acc.go (1 hunks)
- app/evmante/evmante_verify_eth_acc_test.go (1 hunks)
- eth/eth_account.go (4 hunks)
- eth/eth_account_test.go (1 hunks)
- eth/rpc/backend/account_info.go (3 hunks)
- eth/rpc/backend/evm_query_client_test.go (2 hunks)
- eth/rpc/backend/mocks/evm_query_client.go (1 hunks)
- eth/rpc/rpcapi/eth_api_test.go (2 hunks)
- eth/safe_math_test.go (3 hunks)
- eth/state_encoder_test.go (2 hunks)
- proto/eth/evm/v1/query.proto (3 hunks)
- x/evm/const.go (2 hunks)
- x/evm/evm_test.go (2 hunks)
- x/evm/evmtest/eth.go (1 hunks)
- x/evm/evmtest/test_deps.go (1 hunks)
- x/evm/evmtest/tx.go (1 hunks)
- x/evm/keeper/grpc_query.go (5 hunks)
- x/evm/keeper/grpc_query_test.go (11 hunks)
- x/evm/keeper/keeper.go (1 hunks)
- x/evm/keeper/keeper_test.go (1 hunks)
- x/evm/keeper/msg_ethereum_tx_test.go (3 hunks)
- x/evm/keeper/msg_server.go (3 hunks)
- x/evm/keeper/statedb.go (4 hunks)
- x/evm/keeper/statedb_test.go (1 hunks)
- x/evm/query.go (2 hunks)
- x/evm/statedb/state_object.go (5 hunks)
- x/evm/statedb/statedb.go (4 hunks)
- x/evm/statedb/statedb_test.go (13 hunks)
Files skipped from review due to trivial changes (4)
- eth/rpc/backend/mocks/evm_query_client.go
- eth/safe_math_test.go
- eth/state_encoder_test.go
- x/evm/keeper/keeper_test.go
Additional context used
GitHub Check: break-check
proto/eth/evm/v1/query.proto
[failure] 94-94:
Field "2" with name "balance_wei" on message "QueryEthAccountResponse" changed option "json_name" from "codeHash" to "balanceWei".
[failure] 94-94:
Field "2" on message "QueryEthAccountResponse" changed name from "code_hash" to "balance_wei".
[failure] 96-96:
Field "3" with name "code_hash" on message "QueryEthAccountResponse" changed option "json_name" from "nonce" to "codeHash".
[failure] 96-96:
Field "3" with name "code_hash" on message "QueryEthAccountResponse" changed type from "uint64" to "string".
[failure] 96-96:
Field "3" on message "QueryEthAccountResponse" changed name from "nonce" to "code_hash".
Additional comments not posted (80)
eth/eth_account_test.go (1)
1-40
: LGTM! Consider adding edge cases.The function is well-structured and covers multiple scenarios. Consider adding edge cases such as invalid addresses or edge cases for address conversion.
app/evmante/evmante_gas_consume_test.go (2)
28-28
: LGTM!The balance calculation now uses
evm.NativeToWei
, ensuring it is expressed in Wei, the correct unit for Ethereum transactions.
49-49
: LGTM!The balance calculation now uses
evm.NativeToWei
, ensuring it is expressed in Wei, the correct unit for Ethereum transactions.app/evmante/evmante_verify_eth_acc.go (1)
72-74
: Verify the correctness of the new balance representation.The change from
acct.Balance
toacct.BalanceEvmDenom
suggests a shift to a different denomination for balance verification. Ensure thatacct.BalanceEvmDenom
is correctly populated and represents the intended balance accurately.app/evmante/evmante_verify_eth_acc_test.go (1)
24-24
: LGTM! Verify the correctness of the conversion function.The change to convert the balance to Wei using
evm.NativeToWei(happyGasLimit())
is appropriate and ensures accurate balance representation. Verify thatevm.NativeToWei
correctly converts the balance to Wei.Verification successful
Verified! The
NativeToWei
function correctly converts "unibi" to "wei" units by multiplying the input amount by 10^12, adhering to the documented constraint equation.
x/evm/const.go
: Verified the correct implementation ofNativeToWei
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `evm.NativeToWei` correctly converts the balance to Wei. # Test: Search for the definition and usage of `NativeToWei`. Expect: Correct conversion logic. rg --type go 'NativeToWei'Length of output: 1778
Script:
#!/bin/bash # Description: Extract the implementation of the `NativeToWei` function from `x/evm/const.go`. # Extract the implementation of `NativeToWei`. rg --type go --context 10 'func NativeToWei' x/evm/const.goLength of output: 859
eth/eth_account.go (4)
15-17
: LGTM!The function
EthAddrToNibiruAddr
correctly converts an Ethereum address to a Nibiru address.
19-21
: LGTM!The function
NibiruAddrToEthAddr
correctly converts a Nibiru address to an Ethereum address.
44-48
: LGTM!The updated types in the
EthAccountI
interface ensure consistent use of Ethereum-related data types and improve type safety.
58-66
: LGTM!The updated methods in the
EthAccount
struct ensure consistent use of Ethereum-related data types and improve type safety.x/evm/keeper/statedb_test.go (1)
27-42
: Verify the correctness of balance checks.Ensure that the expected balance values are correct and consistent with the operations performed.
Verification successful
Balance checks are consistent with the operations performed by
FundAccount
.The
FundAccount
function mints coins and sends them to the specified account, which aligns with the balance checks in the test file. No directSetBalance
operations were found in the test file.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of balance checks in the test. # Test: Search for balance checks in the test file. Expect: Correct balance values. rg --type go -A 3 's\.Equal' x/evm/keeper/statedb_test.goLength of output: 798
Script:
#!/bin/bash # Description: Verify the correctness of balance checks in the test. # Search for the FundAccount function definition to understand its operations. rg 'func FundAccount' --type go # Search for other balance-modifying operations in the test file. rg 'db.SetBalance' x/evm/keeper/statedb_test.go rg 'BankKeeper.SetBalance' x/evm/keeper/statedb_test.goLength of output: 410
Script:
#!/bin/bash # Description: Verify the correctness of balance checks in the test. # Search for the implementation of the FundAccount function in the specified file. rg 'func FundAccount' -A 10 x/common/testutil/testapp/testapp.go # Search for other balance-modifying operations in the test file. rg 'SetBalance' x/evm/keeper/statedb_test.goLength of output: 430
x/evm/evmtest/eth.go (1)
34-35
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
EthAddrToNibiruAddr
match the new implementation.Verification successful
Function usage verified and consistent.
The function
eth.EthAddrToNibiruAddr
has been correctly implemented and consistently used across the codebase.
x/evm/evmtest/eth.go
x/evm/keeper/grpc_query.go
x/evm/keeper/statedb_test.go
eth/rpc/rpcapi/eth_api_test.go
eth/eth_account_test.go
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `EthAddrToNibiruAddr` match the new implementation. # Test: Search for the function usage. Expect: Only occurrences of the new implementation. rg --type go -A 5 $'eth.EthAddrToNibiruAddr'Length of output: 2140
app/evmante/evmante_can_transfer.go (1)
90-95
: LGTM! But verify the error message in the codebase.The code changes are approved.
However, ensure that the new error message is correctly formatted and used.
Verification successful
Ensure consistent usage of the new error message format.
The error message format appears correct in the provided context. To confirm, let's verify that this format is used consistently across the codebase and is not duplicated or incorrectly formatted elsewhere.
The error message format is correctly implemented and used consistently.
- The error message "failed to transfer %s wei (balance=%s) from address %s using the EVM block context transfer function" is correctly formatted.
- It is used only once in the codebase, ensuring consistency.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new error message in the codebase. # Test: Search for the error message usage. Expect: Correctly formatted error message. rg --type go -A 5 'failed to transfer'Length of output: 430
Script:
#!/bin/bash # Description: Ensure consistent usage of the new error message format across the codebase. # Test: Search for all instances of the error message format. Expect: No duplicates or incorrect formats. rg --type go 'failed to transfer'Length of output: 179
app/evmante/evmante_can_transfer_test.go (1)
24-31
: Improved abstraction and maintainability.The use of
testapp.FundAccount
enhances abstraction by utilizing a higher-level utility function instead of directly manipulating the state database's balance. This promotes better encapsulation and improves the readability and maintainability of the test code.app/evmante/evmante_handler_test.go (2)
30-33
: Improved balance management and consistency.The introduction of
balanceMicronibi
improves the clarity and correctness of the balance management within the test setup. The use ofevm.NativeToWei
ensures consistent conversion of values.
38-41
: Consistent gas limit conversion.Updating the
MaxGas
parameter to usemaxGasMicronibi
ensures consistent conversion of gas limits using theevm.NativeToWei
function, enhancing maintainability and reducing potential errors.x/evm/query.go (1)
34-54
: Enhanced utility and readability.The modifications to the
Validate
method improve its utility by providing clearer feedback on the address format (isBech32
). The use of a switch statement to handle address validation results enhances the readability and maintainability of the code.x/evm/keeper/keeper.go (1)
85-94
: LGTM! The named return value improves readability.The function logic is correct and handles the case where
evmDenom
is empty.x/evm/const.go (3)
94-105
: LGTM! The function correctly converts "unibi" to "wei".The function logic is correct and follows the established conversion relationship.
107-118
: LGTM! The function correctly converts "wei" to "unibi".The function logic is correct and follows the established conversion relationship.
120-152
: LGTM! The function correctly processes wei amounts and handles edge cases.The function logic is correct and handles edge cases appropriately. The error messages are clear and informative.
x/evm/evm_test.go (2)
141-200
: LGTM! The test cases cover a range of scenarios and ensure robust error messaging.The function logic is correct and ensures accurate conversions and error handling.
132-139
: LGTM! The updates reinforce the correctness of the account retrieval functionality.The assertions are clear and informative.
eth/rpc/backend/account_info.go (2)
114-114
: LGTM! But verify the usage ofBalanceWei
.The code changes are approved.
However, ensure that all usages of
BalanceWei
are consistent and correct across the codebase.Verification successful
Verified: Usage of
BalanceWei
is consistent and correct across the codebase.The conversion from string to
sdkmath.Int
is handled properly, and error cases are managed appropriately.
eth/rpc/backend/account_info.go
: Proper conversion and error handling.- Various other files: Consistent usage and representation.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `BalanceWei` in the codebase. # Test: Search for the usage of `BalanceWei`. Expect: Consistent and correct usage. rg --type go -A 5 $'BalanceWei'Length of output: 13098
155-158
: LGTM! But verify the usage ofBalanceWei
.The code changes are approved.
However, ensure that all usages of
BalanceWei
are consistent and correct across the codebase.x/evm/keeper/statedb.go (4)
70-76
: LGTM! But verify the usage ofamountEvmDenom
.The code changes are approved.
However, ensure that all usages of
amountEvmDenom
are consistent and correct across the codebase.
129-129
: LGTM! But verify the usage ofBalanceEvmDenom
.The code changes are approved.
However, ensure that all usages of
BalanceEvmDenom
are consistent and correct across the codebase.Verification successful
Verified: The usage of
BalanceEvmDenom
is consistent and correct across the codebase.
statedb.go
: Used for setting and getting account balances.grpc_query.go
: Used for converting and displaying balances.state_object.go
: Used for initializing and converting balances.evmante_verify_eth_acc.go
: Used for verifying account balances.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `BalanceEvmDenom` in the codebase. # Test: Search for the usage of `BalanceEvmDenom`. Expect: Consistent and correct usage. rg --type go -A 5 $'BalanceEvmDenom'Length of output: 3442
216-217
: LGTM! But verify the usage ofBalanceEvmDenom
.The code changes are approved.
However, ensure that all usages of
BalanceEvmDenom
are consistent and correct across the codebase.Verification successful
Verification Successful:
BalanceEvmDenom
Usage is Consistent and CorrectThe
BalanceEvmDenom
field is used consistently and correctly across the codebase. The initialization, conversion to/from Wei, and balance checks are properly handled.
- Files and Lines:
x/evm/keeper/grpc_query.go
: Lines related to balance and balance conversion.x/evm/keeper/statedb.go
: Lines related to balance initialization and setting.x/evm/statedb/state_object.go
: Lines related to balance conversion and initialization.app/evmante/evmante_verify_eth_acc.go
: Line related to balance verification.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `BalanceEvmDenom` in the codebase. # Test: Search for the usage of `BalanceEvmDenom`. Expect: Consistent and correct usage. rg --type go -A 5 $'BalanceEvmDenom'Length of output: 3442
33-33
: LGTM! But verify the usage ofBalanceEvmDenom
.The code changes are approved.
However, ensure that all usages of
BalanceEvmDenom
are consistent and correct across the codebase.Verification successful
Usages of
BalanceEvmDenom
are consistent and correct across the codebase.The codebase shows consistent and expected usage of
BalanceEvmDenom
in various contexts, including balance conversions and initializations.
x/evm/keeper/grpc_query.go
: Used for string conversion and balance conversion to Wei.x/evm/keeper/statedb.go
: Assigned and retrieved.x/evm/statedb/state_object.go
: Various operations including conversion to/from Wei, initialization, and checks.app/evmante/evmante_verify_eth_acc.go
: Used in a function call for balance verification.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `BalanceEvmDenom` in the codebase. # Test: Search for the usage of `BalanceEvmDenom`. Expect: Consistent and correct usage. rg --type go -A 5 $'BalanceEvmDenom'Length of output: 3442
x/evm/keeper/msg_ethereum_tx_test.go (1)
191-191
: LGTM! But verify the usage offundedAmount
.The code changes are approved.
However, ensure that all usages of
fundedAmount
are consistent and correct across the codebase.Verification successful
LGTM! But verify the usage of
fundedAmount
.The code changes are approved.
fundedAmount
is used consistently and correctly within themsg_ethereum_tx_test.go
file.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `fundedAmount` in the codebase. # Test: Search for the usage of `fundedAmount`. Expect: Consistent and correct usage. rg --type go -A 5 $'fundedAmount'Length of output: 1315
x/evm/statedb/state_object.go (5)
21-24
: Ensure proper initialization ofBalanceEvmDenom
.The new field
BalanceEvmDenom
should be properly initialized to avoid nil pointer dereference issues.Verification successful
Initialization of
BalanceEvmDenom
is handled correctly.The new field
BalanceEvmDenom
is properly initialized in the codebase to avoid nil pointer dereference issues.
- In
x/evm/statedb/state_object.go
, it is set to a newbig.Int
if it isnil
.- In
x/evm/keeper/statedb.go
, it is set usingk.GetEvmGasBalance(ctx, addr)
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `BalanceEvmDenom` is properly initialized in the codebase. # Test: Search for the initialization of `BalanceEvmDenom`. Expect: Proper initialization. rg --type go 'BalanceEvmDenom ='Length of output: 250
144-144
: LGTM! Ensure empty state logic is verified.The
empty
method correctly checks the conditions for an emptystateObject
.However, ensure that the empty state logic is thoroughly tested.
Verification successful
The
empty
method is indirectly tested in thestatedb_test.go
file.The tests in
statedb_test.go
create accounts, commit changes, and check if the accounts are empty, which invokes theempty
method.
x/evm/statedb/statedb_test.go
: Lines whereempty
method is indirectly tested.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `empty` state logic is thoroughly tested. # Test: Search for tests related to the `empty` method. Expect: Thorough testing. rg --type go 'empty'Length of output: 67290
Script:
#!/bin/bash # Description: Search for tests related to the `empty` method in the `x/evm/statedb` directory. # Test: Search for the `empty` method and include context lines to identify any indirect tests. rg --type go --context 5 'empty' x/evm/statedb/Length of output: 11415
247-247
: LGTM! Ensure balance retrieval logic is verified.The
Balance
method correctly returns the balance in wei.However, ensure that the balance retrieval logic is thoroughly tested.
Verification successful
To ensure that the
Balance
retrieval logic inx/evm/statedb/state_object.go
is thoroughly tested, let's narrow down the search to find specific tests for this method.
Balance retrieval logic is thoroughly tested.
The
Balance
method inx/evm/statedb/state_object.go
is covered by multiple test cases inx/evm/statedb/statedb_test.go
andx/evm/keeper/statedb_test.go
, ensuring that its functionality is well-tested.
x/evm/statedb/statedb_test.go
x/evm/keeper/statedb_test.go
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `Balance` retrieval logic is thoroughly tested. # Test: Search for tests related to the `Balance` method. Expect: Thorough testing. rg --type go 'Balance'Length of output: 42963
Script:
#!/bin/bash # Description: Verify that the `Balance` retrieval logic in `x/evm/statedb/state_object.go` is thoroughly tested. # Test: Search for tests related to the `Balance` method in `x/evm/statedb/state_object.go`. Expect: Thorough testing. rg --type go 'Balance' -g '*statedb*test.go'Length of output: 2409
34-37
: Ensure proper initialization ofBalanceWei
.The new field
BalanceWei
should be properly initialized to avoid nil pointer dereference issues.
173-173
: LGTM! Ensure balance update logic is verified.The
SetBalance
method correctly updates the balance and appends the change to the journal.However, ensure that the balance update logic is thoroughly tested.
Verification successful
Balance update logic is tested indirectly
The
SetBalance
method is indirectly tested through theSubBalance
method in thex/evm/statedb/statedb_test.go
file. This ensures that the balance update logic is covered in the tests.
x/evm/statedb/state_object.go
:SetBalance
method is invoked bySubBalance
.x/evm/statedb/statedb_test.go
:SubBalance
method is tested.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `SetBalance` logic is thoroughly tested. # Test: Search for tests related to the `SetBalance` method. Expect: Thorough testing. rg --type go 'SetBalance'Length of output: 341
Script:
#!/bin/bash # Description: Search for Go test files and check for tests related to balance updates. # Find all Go test files in the repository. test_files=$(fd --type f --extension go --exec grep -l 'func Test') # Search for balance-related tests in the found test files. for file in $test_files; do echo "Checking file: $file" rg 'balance|SetBalance' $file doneLength of output: 31252
Script:
#!/bin/bash # Description: Verify if the `SetBalance` method is tested in `x/evm/statedb/statedb_test.go`. # Search for invocations of the `SetBalance` method within the `x/evm/statedb/statedb_test.go` file. rg 'SetBalance' ./x/evm/statedb/statedb_test.goLength of output: 47
Script:
#!/bin/bash # Description: Trace invocations of `SetBalance` within the `stateObject` class to any test files. # Search for invocations of `SetBalance` within the `stateObject` class in `x/evm/statedb/state_object.go`. rg 'SetBalance' ./x/evm/statedb/state_object.go -A 5 # Search for invocations of the methods that call `SetBalance` in the test files. methods=$(rg 'SetBalance' ./x/evm/statedb/state_object.go -o -r '$' -A 5 | grep -oP 'func \(\w+ \*stateObject\) \K\w+') for method in $methods; do echo "Checking for method: $method in test files" rg $method ./x/evm/statedb/statedb_test.go doneLength of output: 990
eth/rpc/rpcapi/eth_api_test.go (2)
73-73
: LGTM! Ensure address conversion logic is verified.The
SetupSuite
method has been updated to use theEthAddrToNibiruAddr
function from theeth
package.However, ensure that the address conversion logic is thoroughly tested.
Verification successful
Address conversion logic is thoroughly tested.
The
EthAddrToNibiruAddr
function is covered by tests, particularly ineth/eth_account_test.go
, ensuring that the address conversion logic is thoroughly tested.
eth/eth_account_test.go:func (s *Suite) TestEthAddrToNibiruAddr()
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the address conversion logic is thoroughly tested. # Test: Search for tests related to the `SetupSuite` method. Expect: Thorough testing. rg --type go 'SetupSuite'Length of output: 1078
Script:
#!/bin/bash # Description: Verify that the address conversion logic is thoroughly tested. # Test: Search for tests related to the `EthAddrToNibiruAddr` function. Expect: Thorough testing. rg --type go 'EthAddrToNibiruAddr'Length of output: 688
18-18
: LGTM! Ensure import refactoring is verified.The import statement has been updated to import from
eth
instead ofevmtest
.However, ensure that the import refactoring is thoroughly tested.
proto/eth/evm/v1/query.proto (6)
16-17
: LGTM!The updated description for the
EthAccount
method accurately reflects the new functionality.
85-85
: LGTM!The updated description for the
address
field inQueryEthAccountRequest
accurately reflects the new functionality.
91-91
: LGTM!The updated description for the
balance
field inQueryEthAccountResponse
accurately reflects that it represents the balance of unibi (micronibi).
100-102
: LGTM!The new
eth_address
field is a valuable addition, providing the hexadecimal-encoded string representing the 20-byte address of a Nibiru EVM account.
104-106
: LGTM!The new
bech32_address
field is a valuable addition, providing the nibi-prefixed address of the account that can receive bank transfers.
93-94
: Ensure consistency in field names.The new
balance_wei
field is a valuable addition, but ensure that the field names are consistent with the rest of the codebase.Verification successful
Field name
balance_wei
is consistent with the naming conventions used in theproto/eth/evm/v1/query.proto
file. The field names in the file use a mix of snake_case and camelCase, andbalance_wei
aligns with the snake_case convention seen in other fields likecode_hash
,eth_address
, andbech32_address
.
proto/eth/evm/v1/query.proto
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of the `balance_wei` field name. # Test: Search for the usage of `balance_wei` in the codebase. Expect: Consistent field names. rg --type proto -A 5 $'balance_wei'Length of output: 64
Script:
#!/bin/bash # Description: Verify consistency of the `balance_wei` field name. # Find all .proto files and search for the usage of `balance_wei` fd -e proto -x rg 'balance_wei' {}Length of output: 161
Script:
#!/bin/bash # Description: List all field names in the `proto/eth/evm/v1/query.proto` file to verify naming consistency. # Extract all field names from the specified .proto file rg '^\s*\w+\s+\w+\s*=\s*\d+;' proto/eth/evm/v1/query.protoLength of output: 980
Tools
GitHub Check: break-check
[failure] 94-94:
Field "2" with name "balance_wei" on message "QueryEthAccountResponse" changed option "json_name" from "codeHash" to "balanceWei".
[failure] 94-94:
Field "2" on message "QueryEthAccountResponse" changed name from "code_hash" to "balance_wei".eth/rpc/backend/evm_query_client_test.go (4)
292-294
: LGTM!The updated return value in
RegisterAccount
aligns with the new structure of theQueryEthAccountResponse
message.
305-305
: LGTM!The updated return value in
RegisterBalance
aligns with the new structure of theQueryBalanceResponse
message.
312-312
: LGTM!The updated return value in
RegisterBalanceInvalid
aligns with the new structure of theQueryBalanceResponse
message.
319-319
: LGTM!The updated return value in
RegisterBalanceNegative
aligns with the new structure of theQueryBalanceResponse
message.x/evm/statedb/statedb.go (4)
220-220
: LGTM!The addition of a blank line in
getStateObject
improves readability without affecting functionality.
267-267
: LGTM!The updated logic in
createObject
aligns with the shift towards using Wei as the standard for balance representation.
353-353
: LGTM!The updated logic in
Suicide
aligns with the shift towards using Wei as the standard for balance representation.
461-461
: LGTM!The updated logic in
Commit
likely enhances compatibility or performance by converting the account object to a native format before committing it to the state.x/evm/statedb/statedb_test.go (15)
31-34
: LGTM!The
TestSuite
function is correctly structured and uses thesuite.Run
method from thetestify
package.
40-56
: LGTM!The
CollectContractStorage
function is correctly implemented, with proper error handling and usage of theForEachStorage
method.
59-131
: LGTM!The
TestAccount
function contains well-structured test cases that cover various scenarios, including non-existent accounts, empty accounts, and account suicides. The assertions are clear and consistent.
Line range hint
136-152
: LGTM!The
TestAccountOverride
function correctly verifies that the balance is not lost while the nonce is reset when an account is overridden.
Line range hint
155-173
: LGTM!The
TestDBError
function contains well-structured test cases that verify error handling in the state database. The assertions are clear and consistent.
Line range hint
177-212
: LGTM!The
TestBalance
function contains well-structured test cases that verify balance-related functionalities. The assertions are clear and consistent.
Line range hint
217-272
: LGTM!The
TestState
function contains well-structured test cases that verify state-related functionalities. The assertions are clear and consistent.
Line range hint
278-314
: LGTM!The
TestCode
function contains well-structured test cases that verify code-related functionalities. The assertions are clear and consistent.
Line range hint
319-398
: LGTM!The
TestRevertSnapshot
function contains well-structured test cases that verify snapshot-related functionalities. The assertions are clear and consistent.
403-422
: LGTM!The
TestNestedSnapshot
function correctly verifies that nested snapshots work as expected.
425-431
: LGTM!The
TestInvalidSnapshotId
function correctly verifies that using an invalid snapshot ID results in a panic.
Line range hint
434-496
: LGTM!The
TestAccessList
function contains well-structured test cases that verify access list-related functionalities. The assertions are clear and consistent.
507-554
: LGTM!The
TestLog
function contains well-structured test cases that verify log-related functionalities. The assertions are clear and consistent.
Line range hint
557-585
: LGTM!The
TestRefund
function contains well-structured test cases that verify refund-related functionalities. The assertions are clear and consistent.
Line range hint
590-625
: LGTM!The
TestIterateStorage
function contains well-structured test cases that verify storage iteration functionalities. The assertions are clear and consistent.x/evm/keeper/msg_server.go (4)
504-520
: LGTM!The
ParseWeiAsMultipleOfMicronibi
function is correctly implemented, with proper error handling and conversion logic.
Line range hint
23-80
: LGTM!The
EthereumTx
function is correctly structured and uses appropriate error handling and event emission logic.
Line range hint
82-118
: LGTM!The
ApplyEvmTx
function is correctly structured and uses appropriate error handling and event emission logic.
Line range hint
120-134
: LGTM!The
ApplyEvmMsgWithEmptyTxConfig
function is correctly structured and uses appropriate error handling.x/evm/keeper/grpc_query.go (2)
40-76
: LGTM!The
EthAccount
function has been modified to handle both Ethereum hexadecimal and Bech32 address formats, improving its flexibility and usability.
Line range hint
120-138
: LGTM!The
Balance
function has been modified to clarify that the balance is reported in wei, improving the documentation and understanding of the balance retrieval process.x/evm/keeper/grpc_query_test.go (7)
32-33
: New callback function added toTestCase
struct.The
onTestEnd
callback function allows for cleanup or additional checks after each test case execution. This enhances the test framework's flexibility.
71-73
: Function renamed and types updated.The function
TestQueryNibiruAccount
has been renamed toTestQueryEvmAccount
. The input and output types have been updated toevm.QueryEthAccountRequest
andevm.QueryEthAccountResponse
, respectively. This aligns with the shift in focus from Nibiru to Ethereum accounts.
85-94
: Test case for querying Ethereum address.The test case ensures that an account funded with tokens can be queried using an Ethereum address. The expected response includes balance, balance in wei, code hash, nonce, Ethereum address, and Bech32 address.
110-119
: Test case for querying Nibiru Bech32 address.The test case ensures that an account funded with tokens can be queried using a Nibiru Bech32 address. The expected response includes balance, balance in wei, code hash, nonce, Ethereum address, and Bech32 address.
132-134
: Test case for invalid Ethereum address.The test case checks the validation of an invalid Ethereum address. The expected response includes zero balance in wei, code hash, and nonce. The error message should indicate an invalid Ethereum address.
141-153
: Test case for non-existing account.The test case ensures that querying a non-existing account returns zero balance, balance in wei, code hash, nonce, Ethereum address, and Bech32 address.
76-82
: Ensure proper error handling in setup function.The setup function funds an account with tokens. Ensure that errors are properly handled and logged.
Verification successful
Proper error handling is present in the setup functions.
The setup functions in the
x/evm/keeper/grpc_query_test.go
file include proper error handling using constructs likes.Require().NoError(err)
ands.NoError(err)
. This satisfies the requirement for error handling in the setup functions.
x/evm/keeper/grpc_query_test.go
: Lines 76-82, 98-104, 120-126Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper error handling in setup functions. # Test: Search for error handling in setup functions. Expect: Proper error handling and logging. rg --type go 'setup: func\(deps \*evmtest.TestDeps\) \{.*err := .*; s.Require\(\).NoError\(err\)' --context 5Length of output: 111
Script:
#!/bin/bash # Description: Verify proper error handling in setup functions. # Search for setup functions and check for any form of error handling. rg --type go 'setup: func\(deps \*evmtest.TestDeps\) \{.*err := .*' --context 10Length of output: 82
Script:
#!/bin/bash # Description: Identify all setup functions in Go files for manual inspection of error handling. # Search for all setup functions in Go files. rg --type go 'setup: func\(deps \*evmtest.TestDeps\)' --context 10Length of output: 9420
CHANGELOG.md (2)
98-98
: LGTM!The change to use atto denomination for wei units in the EVM is well-documented and clear.
99-99
: LGTM!The change to combine account queries into a single endpoint is well-documented and clear.
x/evm/evmtest/test_deps.go
Outdated
func (deps TestDeps) StateDB() *statedb.StateDB { | ||
return statedb.New(deps.Ctx, &deps.Chain.EvmKeeper, | ||
statedb.NewEmptyTxConfig( |
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.
Potential issue with value receiver.
Changing the receiver type to a value receiver means the method will operate on a copy of the TestDeps
instance. If modifications to the struct's fields are expected, consider reverting to a pointer receiver.
- func (deps TestDeps) StateDB() *statedb.StateDB {
+ func (deps *TestDeps) StateDB() *statedb.StateDB {
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 (deps TestDeps) StateDB() *statedb.StateDB { | |
return statedb.New(deps.Ctx, &deps.Chain.EvmKeeper, | |
statedb.NewEmptyTxConfig( | |
func (deps *TestDeps) StateDB() *statedb.StateDB { | |
return statedb.New(deps.Ctx, &deps.Chain.EvmKeeper, | |
statedb.NewEmptyTxConfig( |
x/evm/evmtest/tx.go
Outdated
func TransferWei( | ||
deps *TestDeps, | ||
to gethcommon.Address, | ||
amountWei *big.Int, | ||
) error { | ||
ethAcc := deps.Sender | ||
var innerTxData []byte = nil | ||
var accessList gethcore.AccessList = nil | ||
ethTxMsg, err := NewEthTxMsgFromTxData( | ||
deps, | ||
gethcore.LegacyTxType, | ||
innerTxData, | ||
deps.StateDB().GetNonce(ethAcc.EthAddr), | ||
&to, | ||
amountWei, | ||
gethparams.TxGas, | ||
accessList, | ||
) | ||
if err != nil { | ||
return fmt.Errorf("error while transferring wei: %w", err) | ||
} | ||
|
||
_, err = deps.Chain.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg) | ||
if err != nil { | ||
return fmt.Errorf("error while transferring wei: %w", err) | ||
} | ||
return 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.
Ensure proper error handling and return value.
The function TransferWei
correctly handles errors during transaction creation and execution. However, the final return statement should return nil
instead of err
to clearly indicate success when no errors occur.
- return err
+ return 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.
func TransferWei( | |
deps *TestDeps, | |
to gethcommon.Address, | |
amountWei *big.Int, | |
) error { | |
ethAcc := deps.Sender | |
var innerTxData []byte = nil | |
var accessList gethcore.AccessList = nil | |
ethTxMsg, err := NewEthTxMsgFromTxData( | |
deps, | |
gethcore.LegacyTxType, | |
innerTxData, | |
deps.StateDB().GetNonce(ethAcc.EthAddr), | |
&to, | |
amountWei, | |
gethparams.TxGas, | |
accessList, | |
) | |
if err != nil { | |
return fmt.Errorf("error while transferring wei: %w", err) | |
} | |
_, err = deps.Chain.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg) | |
if err != nil { | |
return fmt.Errorf("error while transferring wei: %w", err) | |
} | |
return err | |
} | |
func TransferWei( | |
deps *TestDeps, | |
to gethcommon.Address, | |
amountWei *big.Int, | |
) error { | |
ethAcc := deps.Sender | |
var innerTxData []byte = nil | |
var accessList gethcore.AccessList = nil | |
ethTxMsg, err := NewEthTxMsgFromTxData( | |
deps, | |
gethcore.LegacyTxType, | |
innerTxData, | |
deps.StateDB().GetNonce(ethAcc.EthAddr), | |
&to, | |
amountWei, | |
gethparams.TxGas, | |
accessList, | |
) | |
if err != nil { | |
return fmt.Errorf("error while transferring wei: %w", err) | |
} | |
_, err = deps.Chain.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg) | |
if err != nil { | |
return fmt.Errorf("error while transferring wei: %w", err) | |
} | |
return nil | |
} |
x/evm/keeper/msg_ethereum_tx_test.go
Outdated
err := testapp.FundAccount( | ||
deps.Chain.BankKeeper, | ||
deps.Ctx, | ||
deps.Sender.NibiruAddr, | ||
sdk.NewCoins(sdk.NewInt64Coin("unibi", amount)), | ||
sdk.NewCoins(sdk.NewInt64Coin("unibi", fundedAmount)), |
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.
Fix the type conversion.
The type conversion of fundedAmount
should be handled carefully to avoid potential issues.
- sdk.NewCoins(sdk.NewInt64Coin("unibi", fundedAmount)),
+ sdk.NewCoins(sdk.NewInt64Coin("unibi", sdkmath.NewInt(fundedAmount))),
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.
sdk.NewCoins(sdk.NewInt64Coin("unibi", fundedAmount)), | |
sdk.NewCoins(sdk.NewInt64Coin("unibi", sdkmath.NewInt(fundedAmount))), |
x/evm/keeper/msg_ethereum_tx_test.go
Outdated
@@ -229,7 +229,7 @@ | |||
&evm.EventTransfer{ | |||
Sender: ethAcc.EthAddr.String(), | |||
Recipient: to.String(), | |||
Amount: strconv.FormatInt(amount, 10), | |||
Amount: strconv.FormatInt(fundedAmount, 10), |
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.
Fix the type conversion.
The type conversion of fundedAmount
should be handled carefully to avoid potential issues.
- Amount: strconv.FormatInt(fundedAmount, 10),
+ Amount: strconv.FormatInt(int64(fundedAmount), 10),
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.
Amount: strconv.FormatInt(fundedAmount, 10), | |
Amount: strconv.FormatInt(int64(fundedAmount), 10), |
x/evm/keeper/msg_ethereum_tx_test.go
Outdated
@@ -208,7 +208,7 @@ | |||
innerTxData, | |||
deps.StateDB().GetNonce(ethAcc.EthAddr), | |||
&to, | |||
big.NewInt(amount), | |||
big.NewInt(fundedAmount), |
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.
Fix the type conversion.
The type conversion of fundedAmount
should be handled carefully to avoid potential issues.
- big.NewInt(fundedAmount),
+ big.NewInt(int64(fundedAmount)),
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.
big.NewInt(fundedAmount), | |
big.NewInt(int64(fundedAmount)), |
x/evm/keeper/msg_server.go
Outdated
// access list preparation is moved from ante handler to here, because it's | ||
// needed when `ApplyMessage` is called under contexts where ante handlers | ||
// are not run, for example `eth_call` and `eth_estimateGas`. | ||
stateDB.PrepareAccessList( | ||
msg.From(), | ||
msg.To(), | ||
evmObj.ActivePrecompiles(params.Rules{}), | ||
msg.AccessList(), | ||
) | ||
|
||
msgWei, err := ParseWeiAsMultipleOfMicronibi(msg.Value()) | ||
if err != nil { | ||
return nil, err | ||
// return nil, fmt.Errorf("cannot use \"value\" in wei that can't be converted to unibi. %s is not divisible by 10^12", msg.Value()) |
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 the commented-out error message.
The error message in line 437 is commented out and should be removed if not needed.
- // return nil, fmt.Errorf("cannot use \"value\" in wei that can't be converted to unibi. %s is not divisible by 10^12", msg.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.
// access list preparation is moved from ante handler to here, because it's | |
// needed when `ApplyMessage` is called under contexts where ante handlers | |
// are not run, for example `eth_call` and `eth_estimateGas`. | |
stateDB.PrepareAccessList( | |
msg.From(), | |
msg.To(), | |
evmObj.ActivePrecompiles(params.Rules{}), | |
msg.AccessList(), | |
) | |
msgWei, err := ParseWeiAsMultipleOfMicronibi(msg.Value()) | |
if err != nil { | |
return nil, err | |
// return nil, fmt.Errorf("cannot use \"value\" in wei that can't be converted to unibi. %s is not divisible by 10^12", msg.Value()) | |
// access list preparation is moved from ante handler to here, because it's | |
// needed when `ApplyMessage` is called under contexts where ante handlers | |
// are not run, for example `eth_call` and `eth_estimateGas`. | |
stateDB.PrepareAccessList( | |
msg.From(), | |
msg.To(), | |
evmObj.ActivePrecompiles(params.Rules{}), | |
msg.AccessList(), | |
) | |
msgWei, err := ParseWeiAsMultipleOfMicronibi(msg.Value()) | |
if err != nil { | |
return nil, 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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- app/evmante/evmante_verify_eth_acc.go (2 hunks)
- eth/rpc/backend/mocks/evm_query_client.go (1 hunks)
- eth/rpc/rpcapi/eth_api_test.go (7 hunks)
- x/evm/keeper/gas_fees.go (2 hunks)
- x/evm/tx_data.go (1 hunks)
Files skipped from review due to trivial changes (1)
- x/evm/tx_data.go
Files skipped from review as they are similar to previous changes (3)
- app/evmante/evmante_verify_eth_acc.go
- eth/rpc/backend/mocks/evm_query_client.go
- eth/rpc/rpcapi/eth_api_test.go
Additional comments not posted (2)
x/evm/keeper/gas_fees.go (2)
Line range hint
61-67
:
LGTM!The
GasToRefund
function is straightforward and correctly calculates the gas to refund.
Line range hint
85-100
:
LGTM!The
CheckSenderBalance
function correctly validates the sender's balance using the updated*big.Int
type. The changes align with the new type and ensure correct error handling.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1986 +/- ##
==========================================
+ Coverage 65.72% 65.75% +0.03%
==========================================
Files 261 261
Lines 16528 16529 +1
==========================================
+ Hits 10863 10869 +6
+ Misses 4852 4850 -2
+ Partials 813 810 -3
|
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 (2)
- e2e/evm/test/basic_queries.test.ts (2 hunks)
- e2e/evm/test/contract_send_nibi.test.ts (4 hunks)
Additional comments not posted (5)
e2e/evm/test/basic_queries.test.ts (2)
8-8
: LGTM! Verify the system's ability to handle large transactions.The increase in
amountToSend
fromtoBigInt(1e3)
totoBigInt(5e12) * toBigInt(1e6)
suggests a focus on testing the system's capability to handle large transactions. Ensure that the system can handle such large transactions without issues.
22-22
: LGTM! Verify the impact of the new wait parameters on test reliability.The modification to
await txResponse.wait(1, 10e3)
aims to ensure that the transaction is confirmed within a defined timeframe. Ensure that this change improves the test's reliability under varying network conditions.e2e/evm/test/contract_send_nibi.test.ts (3)
14-14
: LGTM! Verify the system's ability to handle large transactions.The increase in
transferValue
from100e6
totoBigInt(5e12) * toBigInt(6)
suggests a focus on testing the system's capability to handle large transactions. Ensure that the system can handle such large transactions without issues.
33-33
: LGTM! Verify the system's ability to handle large transactions.The increase in
transferValue
from100e6
totoBigInt(100e12) * toBigInt(6)
suggests a focus on testing the system's capability to handle large transactions. Ensure that the system can handle such large transactions without issues.
52-52
: LGTM! Verify the system's ability to handle large transactions.The increase in
transferValue
from100e6
totoBigInt(100e12) * toBigInt(6)
suggests a focus on testing the system's capability to handle large transactions. Ensure that the system can handle such large transactions without issues.
…nto ud/attonibi
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 (8)
- CHANGELOG.md (1 hunks)
- e2e/evm/test/contract_send_nibi.test.ts (1 hunks)
- proto/eth/evm/v1/query.proto (3 hunks)
- x/evm/evm_test.go (1 hunks)
- x/evm/keeper/gas_fees.go (1 hunks)
- x/evm/keeper/grpc_query.go (1 hunks)
- x/evm/keeper/grpc_query_test.go (2 hunks)
- x/evm/statedb/state_object.go (1 hunks)
Files skipped from review due to trivial changes (2)
- e2e/evm/test/contract_send_nibi.test.ts
- x/evm/keeper/gas_fees.go
Files skipped from review as they are similar to previous changes (1)
- x/evm/evm_test.go
Additional context used
GitHub Check: break-check
proto/eth/evm/v1/query.proto
[failure] 92-92:
Field "1" with name "balance" on message "QueryEthAccountResponse" changed option "json_name" from "balanceWei" to "balance".
[failure] 92-92:
Field "1" on message "QueryEthAccountResponse" changed name from "balance_wei" to "balance".
[failure] 94-94:
Field "2" with name "balance_wei" on message "QueryEthAccountResponse" changed option "json_name" from "codeHash" to "balanceWei".
[failure] 94-94:
Field "2" on message "QueryEthAccountResponse" changed name from "code_hash" to "balance_wei".
[failure] 96-96:
Field "3" with name "code_hash" on message "QueryEthAccountResponse" changed option "json_name" from "nonce" to "codeHash".
[failure] 96-96:
Field "3" with name "code_hash" on message "QueryEthAccountResponse" changed type from "uint64" to "string".
[failure] 96-96:
Field "3" on message "QueryEthAccountResponse" changed name from "nonce" to "code_hash".
Additional comments not posted (14)
x/evm/statedb/state_object.go (1)
148-150
: Improved readability inisEmpty
method.The reformatting enhances the readability of the return statement, making the conditions more clear.
proto/eth/evm/v1/query.proto (3)
16-17
: Updated description forEthAccount
RPC method.The updated description clarifies that the
EthAccount
method now queries a Nibiru account using either its EVM address or Bech32 Nibiru address.
85-85
: Updated description foraddress
field inQueryEthAccountRequest
.The updated description clarifies that the address can be either an Ethereum hex address or a nibi Bech32 address.
91-106
: EnhancedQueryEthAccountResponse
message.The
QueryEthAccountResponse
message has been enhanced with new fieldsbalance_wei
,eth_address
, andbech32_address
. These additions improve the detail and usability of the response.Tools
GitHub Check: break-check
[failure] 92-92:
Field "1" with name "balance" on message "QueryEthAccountResponse" changed option "json_name" from "balanceWei" to "balance".
[failure] 92-92:
Field "1" on message "QueryEthAccountResponse" changed name from "balance_wei" to "balance".
[failure] 94-94:
Field "2" with name "balance_wei" on message "QueryEthAccountResponse" changed option "json_name" from "codeHash" to "balanceWei".
[failure] 94-94:
Field "2" on message "QueryEthAccountResponse" changed name from "code_hash" to "balance_wei".
[failure] 96-96:
Field "3" with name "code_hash" on message "QueryEthAccountResponse" changed option "json_name" from "nonce" to "codeHash".
[failure] 96-96:
Field "3" with name "code_hash" on message "QueryEthAccountResponse" changed type from "uint64" to "string".
[failure] 96-96:
Field "3" on message "QueryEthAccountResponse" changed name from "nonce" to "code_hash".x/evm/keeper/grpc_query.go (3)
40-46
: Updated comments forEthAccount
method.The updated comments provide a clear description of the parameters and functionality of the
EthAccount
method.
50-63
: Improved address validation and conversion.The updated logic for address validation and conversion enhances the flexibility of the method by supporting both Bech32 and Ethereum hexadecimal address formats.
67-76
: Enhanced response structure inEthAccount
method.The response structure has been enhanced to include both Ethereum and Bech32 addresses, as well as the balance in both native denomination and wei. This improves the clarity and usability of the response.
x/evm/keeper/grpc_query_test.go (5)
71-73
: LGTM! The renaming and type updates are appropriate.The function name and input/output types have been updated to reflect Ethereum accounts.
76-94
: Comprehensive test case for funding account and querying Ethereum address.The test case covers the scenario of funding an account and querying it using an Ethereum address. The expected response is well-defined.
101-119
: Comprehensive test case for funding account and querying Nibiru Bech32 address.The test case covers the scenario of funding an account and querying it using a Nibiru Bech32 address. The expected response is well-defined.
Line range hint
132-136
:
Valid test case for invalid Ethereum address.The test case covers the scenario of querying an invalid Ethereum address. The expected error message is well-defined.
142-154
: Valid test case for non-existing account.The test case covers the scenario of querying a non-existing account. The expected response is well-defined.
CHANGELOG.md (2)
101-101
: LGTM! The change is well-documented.The change description is clear and follows the changelog guidelines.
100-100
: LGTM! The change is well-documented.The change description is clear and follows the changelog guidelines.
Purpose / Abstract
This makes it easier to tell what your hex address is on the EVM using your nibi address and vice versa. It's simpler to use and understand than having two different queries.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation