feat(evm): app config and json-rpc #1871
Conversation
WalkthroughThe recent changes to the codebase primarily focus on enabling JSON-RPC by default, refining Ethereum-related functionalities, and enhancing various configurations and error handling mechanisms. This includes updates to flag definitions, server configurations, EVM state management, and query validation processes. Additionally, several functions and constants have been renamed for clarity and accuracy, and test cases have been refactored for better organization and coverage. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (3)
app/server/json_rpc.go (1)
22-114: LGTM! TheStartJSONRPCfunction is well-implemented. Consider adding comments to explain the purpose of key sections for better readability.app/server/flags.go (1)
68-93: LGTM! TheAddTxFlagsfunction is well-implemented. Consider removing the commented-out code for better readability.contrib/scripts/localnet.sh (1)
Line range hint
150-150: Quote the variables to prevent word splitting.- rm -rf "$CHAIN_DIR" + rm -rf "$CHAIN_DIR" - if ! mkdir -p "$CHAIN_DIR" 2>/dev/null; then + if ! mkdir -p "$CHAIN_DIR" 2>/dev/null; then - cat $CHAIN_DIR/config/genesis.json | jq "$1" >$CHAIN_DIR/config/tmp_genesis.json + cat "$CHAIN_DIR/config/genesis.json" | jq "$1" >"$CHAIN_DIR/config/tmp_genesis.json" - mv $CHAIN_DIR/config/tmp_genesis.json $CHAIN_DIR/config/genesis.json + mv "$CHAIN_DIR/config/tmp_genesis.json" "$CHAIN_DIR/config/genesis.json"Also applies to: 154-154, 158-158, 165-165, 182-182, 184-184
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- app/server/config/server_config.go (1 hunks)
- app/server/flags.go (1 hunks)
- app/server/json_rpc.go (1 hunks)
- app/server/start.go (1 hunks)
- app/server/util.go (1 hunks)
- cmd/nibid/cmd/root.go (5 hunks)
- contrib/scripts/localnet.sh (2 hunks)
Additional Context Used
ShellCheck (14)
contrib/scripts/localnet.sh (14)
[error] 26-26: This expression is constant. Did you forget a $ somewhere?
[warning] 75-75: FLAG_SPOT appears unused. Verify use (or export if used externally).
[warning] 109-109: Quotes/backslashes will be treated literally. Use an array.
[warning] 150-150: Quotes/backslashes in this variable will not be respected.
[info] 150-150: Double quote to prevent globbing and word splitting.
[warning] 154-154: Quotes/backslashes in this variable will not be respected.
[info] 154-154: Double quote to prevent globbing and word splitting.
[warning] 158-158: Quotes/backslashes in this variable will not be respected.
[info] 158-158: Double quote to prevent globbing and word splitting.
[warning] 165-165: Quote this to prevent word splitting.
[info] 182-182: Double quote to prevent globbing and word splitting.
[info] 182-182: Double quote to prevent globbing and word splitting.
[info] 184-184: Double quote to prevent globbing and word splitting.
[info] 184-184: Double quote to prevent globbing and word splitting.
Additional comments not posted (28)
app/server/util.go (3)
21-56: LGTM! TheAddCommandsfunction correctly adds the necessary commands and flags.
58-84: LGTM! TheConnectTmWSfunction correctly handles WebSocket client creation and reconnection logic.
86-100: LGTM! TheListenfunction correctly starts a network listener and applies connection limits if specified.app/server/flags.go (3)
10-19: LGTM! The Tendermint/cosmos-sdk flags are correctly defined.
21-28: LGTM! The GRPC-related flags are correctly defined.
36-54: LGTM! The JSON-RPC flags are correctly defined.contrib/scripts/localnet.sh (3)
6-6: LGTM! The chain initialization command is correctly updated to use the new chain ID.Also applies to: 134-134
Line range hint
75-75: Verify the usage of theFLAG_SPOTvariable as it appears to be unused.
Line range hint
182-182: LGTM! The network start command is correctly implemented.Also applies to: 184-184
cmd/nibid/cmd/root.go (3)
78-78: Verify the function call tosrvconfig.AppConfigas it appears to be incorrectly formatted.
121-126: LGTM! TheinitRootCmdfunction correctly initializes the root command by adding various subcommands.
Line range hint
147-149: LGTM! TheaddModuleInitFlagsfunction correctly adds module initialization flags to the start command.app/server/config/server_config.go (2)
245-245: LGTM! The default configuration values are correctly defined.
245-245: LGTM! The TLS configuration is correctly defined.app/server/start.go (14)
77-213: The command definition and flag bindings are comprehensive and cover various configurations for the server. No issues found.
103-115: ThePreRunEfunction correctly binds flags to the context's Viper and retrieves pruning options. No issues found.
116-153: TheRunEfunction handles the main logic for starting the server, including unlocking the keyring, starting ABCI with or without Tendermint, and handling errors. No issues found.
215-229: The database opening logic correctly handles opening the database and errors. No issues found.
240-249: The server configuration logic correctly retrieves and validates the server configuration. No issues found.
256-276: The server start logic correctly starts the ABCI server and handles signals for graceful shutdown. No issues found.
284-294: The database opening logic correctly handles opening the database and errors. No issues found.
303-312: The server configuration logic correctly retrieves and validates the server configuration. No issues found.
316-361: The Tendermint node initialization logic correctly handles loading the node key, creating the node, and starting the node. No issues found.
363-589: The service registration logic correctly handles registering transaction, Tendermint, and node services, starting telemetry, and handling JSON-RPC and gRPC services. No issues found.
591-595: TheOpenIndexerDBfunction correctly opens the custom eth indexer database. No issues found.
597-608: TheopenTraceWriterfunction correctly opens a trace writer file. No issues found.
610-615: ThestartTelemetryfunction correctly starts telemetry if enabled in the configuration. No issues found.
617-660: TheWaitForQuitSignalsfunction correctly waits for quit signals, and thewrapCPUProfilefunction correctly wraps a callback with CPU profiling. No issues found.
| @@ -242,7 +242,7 @@ func GetAPINamespaces() []string { | |||
| // DefaultJSONRPCConfig returns an EVM config with the JSON-RPC API enabled by default | |||
| func DefaultJSONRPCConfig() *JSONRPCConfig { | |||
| return &JSONRPCConfig{ | |||
| Enable: false, | |||
| Enable: true, | |||
There was a problem hiding this comment.
Fix the validation logic for the FeeHistoryCap field to allow positive values.
- if c.FeeHistoryCap <= 0 {
+ if c.FeeHistoryCap < 0 {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.
| Enable: true, | |
| Enable: true, |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1871 +/- ##
==========================================
- Coverage 67.13% 67.12% -0.01%
==========================================
Files 212 212
Lines 13467 13463 -4
==========================================
- Hits 9041 9037 -4
Misses 3780 3780
Partials 646 646
|
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (8)
contrib/scripts/localnet.sh (8)
Line range hint
26-26: This expression is constant. Did you forget a$somewhere?- if [ console_log_text_color ]; then + if $(console_log_text_color); then
Line range hint
75-75: TheFLAG_SPOTvariable appears unused. Verify its use or remove it if not needed.
Line range hint
150-150: Double quote to prevent globbing and word splitting.- if $BINARY init $CHAIN_ID --chain-id $CHAIN_ID --overwrite; then + if "$BINARY" init "$CHAIN_ID" --chain-id "$CHAIN_ID" --overwrite; then
Line range hint
154-154: Double quote to prevent globbing and word splitting.- $BINARY config keyring-backend test + "$BINARY" config keyring-backend test
Line range hint
158-158: Double quote to prevent globbing and word splitting.- $BINARY config chain-id $CHAIN_ID + "$BINARY" config chain-id "$CHAIN_ID"
Line range hint
165-165: Quote this to prevent word splitting.- echo "$MNEMONIC" | $BINARY keys add $val_key_name --recover + echo "$MNEMONIC" | "$BINARY" keys add "$val_key_name" --recover
Line range hint
182-182: Double quote to prevent globbing and word splitting.- cat $CHAIN_DIR/config/genesis.json | jq "$1" >$CHAIN_DIR/config/tmp_genesis.json + cat "$CHAIN_DIR/config/genesis.json" | jq "$1" >"$CHAIN_DIR/config/tmp_genesis.json"
Line range hint
184-184: Double quote to prevent globbing and word splitting.- mv $CHAIN_DIR/config/tmp_genesis.json $CHAIN_DIR/config/genesis.json + mv "$CHAIN_DIR/config/tmp_genesis.json" "$CHAIN_DIR/config/genesis.json"
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/server/start.go (1 hunks)
- contrib/scripts/localnet.sh (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/server/start.go
Additional Context Used
ShellCheck (14)
contrib/scripts/localnet.sh (14)
[error] 26-26: This expression is constant. Did you forget a $ somewhere?
[warning] 75-75: FLAG_SPOT appears unused. Verify use (or export if used externally).
[warning] 109-109: Quotes/backslashes will be treated literally. Use an array.
[warning] 150-150: Quotes/backslashes in this variable will not be respected.
[info] 150-150: Double quote to prevent globbing and word splitting.
[warning] 154-154: Quotes/backslashes in this variable will not be respected.
[info] 154-154: Double quote to prevent globbing and word splitting.
[warning] 158-158: Quotes/backslashes in this variable will not be respected.
[info] 158-158: Double quote to prevent globbing and word splitting.
[warning] 165-165: Quote this to prevent word splitting.
[info] 182-182: Double quote to prevent globbing and word splitting.
[info] 182-182: Double quote to prevent globbing and word splitting.
[info] 184-184: Double quote to prevent globbing and word splitting.
[info] 184-184: Double quote to prevent globbing and word splitting.
Additional comments not posted (2)
contrib/scripts/localnet.sh (2)
Line range hint
150-150: Genesis parameters configuration looks good.
Line range hint
150-150: Gentx and network start sections look good.
onikonychev
changed the title
There was a problem hiding this comment.
Actionable comments posted: 13
Out of diff range and nitpick comments (3)
app/appconst/appconst.go (1)
Line range hint
35-39: EnsureRuntimeVersionhandles empty or missing values gracefully.If
GitCommitorBuildDateare empty, the formatted string might be misleading. Consider adding default values or handling empty cases explicitly.func RuntimeVersion() string { return fmt.Sprintf( "Version %s (%s)\nCompiled at %s using Go %s (%s)", AppVersion, GitCommit, if BuildDate == "" { "unknown" } else { BuildDate }, GoVersion, GoArch, ) }x/evm/evmmodule/genesis.go (1)
Line range hint
28-28: Implement theExportGenesisfunction.The
ExportGenesisfunction currently has a TODO comment. Do you want me to help implement this function or open a GitHub issue to track this task?x/evm/keeper/keeper.go (1)
6-6: Remove unused import.The import statement for
github.com/NibiruChain/nibiru/x/evmis commented out and should be removed if not needed.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.modis excluded by!**/*.mod,!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
Files selected for processing (14)
- CHANGELOG.md (1 hunks)
- app/appconst/appconst.go (1 hunks)
- eth/chain_id.go (3 hunks)
- eth/chain_id_test.go (1 hunks)
- eth/rpc/rpcapi/web3_api.go (1 hunks)
- x/common/error.go (1 hunks)
- x/evm/const.go (2 hunks)
- x/evm/evmmodule/genesis.go (1 hunks)
- x/evm/keeper/evm_state.go (1 hunks)
- x/evm/keeper/grpc_query.go (2 hunks)
- x/evm/keeper/keeper.go (3 hunks)
- x/evm/query.go (1 hunks)
- x/evm/query_test.go (1 hunks)
- x/tokenfactory/keeper/msg_server.go (1 hunks)
Additional comments not posted (25)
eth/rpc/rpcapi/web3_api.go (1)
21-21: EnsureRuntimeVersionis correctly formatted and handles edge cases.Verify that
appconst.RuntimeVersionhandles empty or missing values gracefully, as it directly impacts the output ofClientVersion.x/evm/keeper/evm_state.go (2)
18-43: TheEvmStatestruct definition looks well-organized and correct.
47-87: TheNewEvmStatefunction correctly initializes theEvmStatestruct.x/evm/query.go (2)
14-30: TheUnpackInterfacesfunctions correctly unpack interfaces forQueryTraceTxRequestandQueryTraceBlockRequest.
32-41: TheValidatefunctions correctly validate various query requests, ensuring non-nil requests and valid addresses.Also applies to: 44-54, 57-70, 72-83, 85-95, 98-109, 112-116, 119-127, 130-138
x/common/error.go (1)
187-187: TheErrNilGrpcMsgfunction correctly returns a gRPC error for nil messages.x/tokenfactory/keeper/msg_server.go (7)
Line range hint
18-35: TheCreateDenomfunction correctly handles the creation of a new denomination, including validation and storage.
Line range hint
37-56: TheChangeAdminfunction correctly handles the change of admin for a denomination, including validation and storage.
Line range hint
58-74: TheUpdateModuleParamsfunction correctly handles the update of module parameters, including validation and storage.
Line range hint
76-104: TheMintfunction correctly handles the minting of new tokens, including validation and storage.
Line range hint
106-134: TheBurnfunction correctly handles the burning of tokens, including validation and storage.
Line range hint
136-158: TheSetDenomMetadatafunction correctly handles the setting of denomination metadata, including validation and storage.
Line range hint
160-182: TheBurnNativefunction correctly handles the burning of native tokens, including validation and storage.x/evm/keeper/grpc_query.go (12)
Line range hint
14-22: TheAccountfunction is a placeholder and correctly returns a not implemented error.
Line range hint
24-32: TheCosmosAccountfunction is a placeholder and correctly returns a not implemented error.
Line range hint
34-42: TheValidatorAccountfunction is a placeholder and correctly returns a not implemented error.
94-101: TheBalancefunction correctly handles the balance retrieval process, including validation.
Line range hint
103-111: TheBaseFeefunction is a placeholder and correctly returns a not implemented error.
Line range hint
113-121: TheStoragefunction is a placeholder and correctly returns a not implemented error.
Line range hint
123-131: TheCodefunction is a placeholder and correctly returns a not implemented error.
Line range hint
133-141: TheParamsfunction is a placeholder and correctly returns a not implemented error.
Line range hint
143-151: TheEthCallfunction is a placeholder and correctly returns a not implemented error.
Line range hint
153-155: TheEstimateGasfunction is a placeholder and correctly returns a not implemented error.
Line range hint
157-165: TheEstimateGasForEvmCallTypefunction is a placeholder and correctly returns a not implemented error.
Line range hint
167-175: TheTraceTxandTraceBlockfunctions are placeholders and correctly return not implemented errors.Also applies to: 177-185
eth/chain_id.go
Outdated
| // IsValidChainID returns false if the given chain identifier is incorrectly | ||
| // formatted. | ||
| func IsValidChainID(chainID string) bool { |
There was a problem hiding this comment.
Consider using a constant for the maximum chain ID length.
To improve maintainability, consider defining a constant for the maximum chain ID length.
const MaxChainIDLength = 48
func IsValidChainID(chainID string) bool {
if len(chainID) > MaxChainIDLength {
return false
}
return nibiruEvmChainId.MatchString(chainID)
}
eth/chain_id.go
Outdated
| return nil, ErrInvalidChainID.Wrapf( | ||
| `chain-id input "%s" cannot exceed 48 chars`, chainID) |
There was a problem hiding this comment.
Consider using a constant for the maximum chain ID length.
To improve maintainability, consider defining a constant for the maximum chain ID length.
const MaxChainIDLength = 48
func ParseChainID(chainID string) (*big.Int, error) {
chainID = strings.TrimSpace(chainID)
if len(chainID) > MaxChainIDLength {
return nil, ErrInvalidChainID.Wrapf(
`chain-id input "%s" cannot exceed %d chars`, chainID, MaxChainIDLength)
}
// other code...
}
eth/chain_id.go
Outdated
| return nil, ErrInvalidChainID.Wrapf( | ||
| `chain-id input "%s", matches "%v"`, chainID, matches) |
There was a problem hiding this comment.
Improve error message clarity.
Consider improving the clarity of the error message by providing more context.
if matches == nil || len(matches) != 4 || matches[1] == "" {
return nil, ErrInvalidChainID.Wrapf(
`chain-id input "%s" does not match expected format`, chainID)
}
eth/chain_id.go
Outdated
| return nil, ErrInvalidChainID.Wrapf( | ||
| `epoch "%s" must be base-10 integer format`, matches[2]) |
There was a problem hiding this comment.
Improve error message clarity.
Consider improving the clarity of the error message by providing more context.
if !ok {
return nil, ErrInvalidChainID.Wrapf(
`epoch "%s" must be a valid base-10 integer`, matches[2])
}
x/evm/const.go
Outdated
| // StoreKey: Persistent storage key for ethereum storage data, account code | ||
| // (StateDB) or block related data for the Eth Web3 API. |
There was a problem hiding this comment.
Ensure the comment accurately describes the StoreKey constant.
The comment should accurately describe the StoreKey constant to improve clarity.
// StoreKey: Persistent storage key for EVM module data, including account code and block-related data for the Eth Web3 API.
StoreKey = ModuleName
x/evm/const.go
Outdated
| // KVStore transient prefix namespaces for the EVM Module. Transient stores only | ||
| // remain for current block, and have more gas efficient read and write access. |
There was a problem hiding this comment.
Ensure the comment accurately describes the transient store namespaces.
The comment should accurately describe the transient store namespaces to improve clarity.
// KVStore transient prefix namespaces for the EVM Module. Transient stores only
// persist for the current block and offer more gas-efficient read and write access.
x/evm/keeper/keeper.go
Outdated
| // EvmState isolates the key-value stores (collections) for the x/evm module. | ||
| EvmState EvmState |
There was a problem hiding this comment.
Ensure the comment accurately describes the EvmState field.
The comment should accurately describe the EvmState field to improve clarity.
// EvmState manages the key-value stores (collections) for the x/evm module.
EvmState EvmState
x/evm/keeper/keeper.go
Outdated
| // GetEvmGasBalance: Implements `evm.EVMKeeper` from | ||
| // "github.com/NibiruChain/nibiru/app/ante/evm": Load account's balance of gas | ||
| // tokens for EVM execution | ||
| func (k *Keeper) GetEvmGasBalance(ctx sdk.Context, addr gethcommon.Address) *big.Int { | ||
| cosmosAddr := sdk.AccAddress(addr.Bytes()) | ||
| evmParams := k.GetParams(ctx) | ||
| evmDenom := evmParams.GetEvmDenom() | ||
| // if node is pruned, params is empty. Return invalid value | ||
| if evmDenom == "" { | ||
| return big.NewInt(-1) | ||
| } | ||
| coin := k.bankKeeper.GetBalance(ctx, cosmosAddr, evmDenom) | ||
| return coin.Amount.BigInt() | ||
| } |
There was a problem hiding this comment.
Ensure proper error handling for GetEvmGasBalance.
The GetEvmGasBalance function should handle potential errors from k.bankKeeper.GetBalance and return a more informative error message if the balance retrieval fails.
func (k *Keeper) GetEvmGasBalance(ctx sdk.Context, addr gethcommon.Address) (*big.Int, error) {
cosmosAddr := sdk.AccAddress(addr.Bytes())
evmParams := k.GetParams(ctx)
evmDenom := evmParams.GetEvmDenom()
// if node is pruned, params is empty. Return invalid value
if evmDenom == "" {
return nil, fmt.Errorf("EVM denomination is empty")
}
coin := k.bankKeeper.GetBalance(ctx, cosmosAddr, evmDenom)
if coin.Amount.IsNil() {
return nil, fmt.Errorf("failed to retrieve balance for address %s", cosmosAddr.String())
}
return coin.Amount.BigInt(), nil
}
eth/chain_id_test.go
Outdated
| func TestParseChainID_Happy(t *testing.T) { | ||
| testCases := []struct { | ||
| name string | ||
| chainID string | ||
| expError bool | ||
| expInt *big.Int | ||
| name string | ||
| chainID string | ||
| expInt *big.Int | ||
| }{ | ||
| { | ||
| "valid chain-id, single digit", "nibiru_1-1", false, big.NewInt(1), | ||
| chainID: "nibiru_1-1", | ||
| name: "valid chain-id, single digit", | ||
| expInt: big.NewInt(1), | ||
| }, | ||
| { | ||
| "valid chain-id, multiple digits", "aragonchain_256-1", false, big.NewInt(256), | ||
| chainID: "aragonchain_256-1", | ||
| name: "valid chain-id, multiple digits", | ||
| expInt: big.NewInt(256), | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| chainIDEpoch, err := ParseChainID(tc.chainID) | ||
| require.NoError(t, err, tc.name) | ||
| var errMsg string = "" | ||
| if err != nil { | ||
| errMsg = err.Error() | ||
| } | ||
| assert.NoError(t, err, tc.name, errMsg) | ||
| require.Equal(t, tc.expInt, chainIDEpoch, tc.name) | ||
| require.True(t, IsValidChainID(tc.chainID)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Add more descriptive test case names.
Consider adding more descriptive names to the test cases within TestParseChainID_Happy to improve readability and maintainability.
func TestParseChainID_Happy(t *testing.T) {
testCases := []struct {
name string
chainID string
expInt *big.Int
}{
{
name: "valid chain-id, single digit",
chainID: "nibiru_1-1",
expInt: big.NewInt(1),
},
{
name: "valid chain-id, multiple digits",
chainID: "aragonchain_256-1",
expInt: big.NewInt(256),
},
}
for _, tc := range testCases {
chainIDEpoch, err := ParseChainID(tc.chainID)
require.NoError(t, err, tc.name)
assert.NoError(t, err, tc.name)
require.Equal(t, tc.expInt, chainIDEpoch, tc.name)
require.True(t, IsValidChainID(tc.chainID), tc.name)
}
}
eth/chain_id_test.go
Outdated
| chainID: "aragonchain-1-1", | ||
| name: "invalid chain-id, double dash", | ||
| }, | ||
| { | ||
| "invalid chain-id, double underscore", "aragonchain_1_1", true, nil, | ||
| chainID: "aragonchain_1_1", | ||
| name: "invalid chain-id, double underscore", | ||
| }, | ||
| { | ||
| "invalid chain-id, dash only", "-", true, nil, | ||
| chainID: "-", | ||
| name: "invalid chain-id, dash only", | ||
| }, | ||
| { | ||
| "invalid chain-id, undefined identifier and EIP155", "-1", true, nil, | ||
| chainID: "-1", | ||
| name: "invalid chain-id, undefined identifier and EIP155", | ||
| }, | ||
| { | ||
| "invalid chain-id, undefined identifier", "_1-1", true, nil, | ||
| chainID: "_1-1", | ||
| name: "invalid chain-id, undefined identifier", | ||
| }, | ||
| { | ||
| "invalid chain-id, uppercases", "NIBIRU_1-1", true, nil, | ||
| chainID: "NIBIRU_1-1", | ||
| name: "invalid chain-id, uppercases", | ||
| }, | ||
| { | ||
| "invalid chain-id, mixed cases", "Nibiru_1-1", true, nil, | ||
| chainID: "Nibiru_1-1", | ||
| name: "invalid chain-id, mixed cases", | ||
| }, | ||
| { | ||
| "invalid chain-id, special chars", "$&*#!_1-1", true, nil, | ||
| chainID: "$&*#!_1-1", | ||
| name: "invalid chain-id, special chars", | ||
| }, | ||
| { | ||
| "invalid eip155 chain-id, cannot start with 0", "nibiru_001-1", true, nil, | ||
| chainID: "nibiru_001-1", | ||
| name: "invalid eip155 chain-id, cannot start with 0", | ||
| }, | ||
| { | ||
| "invalid eip155 chain-id, cannot invalid base", "nibiru_0x212-1", true, nil, | ||
| chainID: "nibiru_0x212-1", | ||
| name: "invalid eip155 chain-id, cannot invalid base", | ||
| }, | ||
| { | ||
| "invalid eip155 chain-id, cannot invalid base", "nibiru_1-0x212", true, nil, | ||
| chainID: "nibiru_1-0x212", | ||
| name: "invalid eip155 chain-id, cannot invalid base", | ||
| }, | ||
| { | ||
| "invalid eip155 chain-id, non-integer", "nibiru_nibiru_9000-1", true, nil, | ||
| chainID: "nibiru_nibiru_9000-1", | ||
| name: "invalid eip155 chain-id, non-integer", | ||
| }, | ||
| { | ||
| "invalid epoch, undefined", "nibiru_-", true, nil, | ||
| chainID: "nibiru_-", | ||
| name: "invalid epoch, undefined", | ||
| }, | ||
| { | ||
| "blank chain ID", " ", true, nil, | ||
| chainID: " ", | ||
| name: "blank chain ID", | ||
| }, | ||
| { | ||
| "empty chain ID", "", true, nil, | ||
| chainID: "", | ||
| name: "empty chain ID", | ||
| }, | ||
| { | ||
| "empty content for chain id, eip155 and epoch numbers", "_-", true, nil, | ||
| chainID: "_-", | ||
| name: "empty content for chain id, eip155 and epoch numbers", | ||
| }, | ||
| { | ||
| "long chain-id", "nibiru_" + strings.Repeat("1", 45) + "-1", true, nil, | ||
| chainID: "nibiru_" + strings.Repeat("1", 45) + "-1", | ||
| name: "long chain-id", | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| chainIDEpoch, err := ParseChainID(tc.chainID) | ||
| if tc.expError { | ||
| require.Error(t, err, tc.name) | ||
| require.Nil(t, chainIDEpoch) | ||
|
|
||
| require.False(t, IsValidChainID(tc.chainID), tc.name) | ||
| } else { | ||
| require.NoError(t, err, tc.name) | ||
| require.Equal(t, tc.expInt, chainIDEpoch, tc.name) | ||
| require.True(t, IsValidChainID(tc.chainID)) | ||
| } | ||
| require.Error(t, err, tc.name) | ||
| require.Nil(t, chainIDEpoch) | ||
| require.False(t, IsValidChainID(tc.chainID), tc.name) |
There was a problem hiding this comment.
Add more descriptive test case names.
Consider adding more descriptive names to the test cases within TestParseChainID_Sad to improve readability and maintainability.
func TestParseChainID_Sad(t *testing.T) {
testCases := []struct {
name string
chainID string
}{
{
name: "invalid chain-id, double dash",
chainID: "aragonchain-1-1",
},
{
name: "invalid chain-id, double underscore",
chainID: "aragonchain_1_1",
},
{
name: "invalid chain-id, dash only",
chainID: "-",
},
{
name: "invalid chain-id, undefined identifier and EIP155",
chainID: "-1",
},
{
name: "invalid chain-id, undefined identifier",
chainID: "_1-1",
},
{
name: "invalid chain-id, uppercases",
chainID: "NIBIRU_1-1",
},
{
name: "invalid chain-id, mixed cases",
chainID: "Nibiru_1-1",
},
{
name: "invalid chain-id, special chars",
chainID: "$&*#!_1-1",
},
{
name: "invalid eip155 chain-id, cannot start with 0",
chainID: "nibiru_001-1",
},
{
name: "invalid eip155 chain-id, cannot invalid base",
chainID: "nibiru_0x212-1",
},
{
name: "invalid eip155 chain-id, cannot invalid base",
chainID: "nibiru_1-0x212",
},
{
name: "invalid eip155 chain-id, non-integer",
chainID: "nibiru_nibiru_9000-1",
},
{
name: "invalid epoch, undefined",
chainID: "nibiru_-",
},
{
name: "blank chain ID",
chainID: " ",
},
{
name: "empty chain ID",
chainID: "",
},
{
name: "empty content for chain id, eip155 and epoch numbers",
chainID: "_-",
},
{
name: "long chain-id",
chainID: "nibiru_" + strings.Repeat("1", 45) + "-1",
},
}
for _, tc := range testCases {
chainIDEpoch, err := ParseChainID(tc.chainID)
require.Error(t, err, tc.name)
require.Nil(t, chainIDEpoch)
require.False(t, IsValidChainID(tc.chainID), tc.name)
}
}
There was a problem hiding this comment.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- CHANGELOG.md (1 hunks)
- cmd/nibid/cmd/root.go (5 hunks)
- x/evm/evmmodule/genesis.go (1 hunks)
- x/evm/keeper/keeper.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- x/evm/evmmodule/genesis.go
- x/evm/keeper/keeper.go
Additional comments not posted (3)
cmd/nibid/cmd/root.go (3)
8-10: Ensure the newly added imports are necessary and correctly used in the code.The imports for
serverandsrvconfigare added. Verify their usage in the code to ensure they are necessary and correctly implemented.
Line range hint
78-83: Verify the correctness of the custom application template and configuration integration.The custom application template and configuration are added in the
PersistentPreRunEfunction. Ensure this integration is correct and does not introduce any issues.
121-126: Verify the correctness of the added server commands and their impact on the existing functionality.The server commands are added in the
initRootCmdfunction. Ensure these commands are correctly added and do not conflict with existing commands.
No description provided.