feat(evm): evm tx indexer service implemented #2044
Conversation
WalkthroughThe changes introduce a new Ethereum Virtual Machine (EVM) transaction indexer service, including a command-line interface for indexing transactions within specified block ranges. The codebase has been refactored to replace the previous key-value indexer with the new EVM indexer. Additionally, various components, such as the JSON-RPC server and backend, have been updated to integrate this new indexing functionality, improving overall structure and error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant EVMIndexerService
participant RPCClient
participant Database
User->>CLI: Execute evm-tx-index command
CLI->>EVMIndexerService: Start indexing process
EVMIndexerService->>Database: Open indexer database
EVMIndexerService->>RPCClient: Retrieve block data
RPCClient-->>EVMIndexerService: Return block data
EVMIndexerService->>Database: Index transactions
EVMIndexerService->>CLI: Output completion message
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2044 +/- ##
==========================================
- Coverage 65.76% 65.58% -0.18%
==========================================
Files 264 264
Lines 16636 16653 +17
==========================================
- Hits 10940 10922 -18
- Misses 4808 4846 +38
+ Partials 888 885 -3
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Outside diff range and nitpick comments (6)
eth/indexer/evm_tx_indexer.go (2)
Line range hint
50-127: LGTM with a suggestion to improve test coverage!The
IndexBlockmethod implementation follows the steps described in the comment and handles various scenarios appropriately. It uses appropriate error handling, logging, and a batch to write the indexed transaction results for better performance.However, the static analysis hints indicate that some lines in the method are not covered by tests. Please consider adding test coverage for the following lines:
- Line 72: Error handling when decoding the transaction fails.
- Line 82: Error handling when parsing the transaction result fails.
- Lines 109-114: Logging when the Ethereum transaction index doesn't match the expected value.
Do you want me to generate the test cases or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 72-72: eth/indexer/evm_tx_indexer.go#L72
Added line #L72 was not covered by tests
211-219: LGTM with a suggestion to improve test coverage!The
CloseDBAndExitmethod is responsible for closing the indexer's database and logging the action when the indexer is stopped. It logs an informational message before closing the database, which is a good practice for tracking the indexer's lifecycle. The method also handles the error returned by theClosemethod of the database and returns it wrapped with an appropriate error message.However, the static analysis hints indicate that some lines in the method are not covered by tests. Please consider adding test coverage for the following lines:
- Lines 212-216: The entire
CloseDBAndExitmethod.- Line 218: The return statement when no error occurs.
Do you want me to generate the test cases or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 212-216: eth/indexer/evm_tx_indexer.go#L212-L216
Added lines #L212 - L216 were not covered by tests
[warning] 218-218: eth/indexer/evm_tx_indexer.go#L218
Added line #L218 was not covered by testsx/common/testutil/testnetwork/validator_node.go (1)
178-178: Consider adding a test case for the error scenario.The static analysis hint indicates that line 178, which logs an error message if the
Stopmethod of theEthTxIndexerServicereturns an error, is not covered by tests. To improve the overall test coverage and ensure the correct handling of errors during shutdown, consider adding a test case that simulates an error scenario and verifies that the appropriate error message is logged.Do you want me to generate a test case for this scenario or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 178-178: x/common/testutil/testnetwork/validator_node.go#L178
Added line #L178 was not covered by testsapp/server/evm_tx_indexer_service.go (1)
50-51: Simplify initialization ofchainHeightStorageThe use of
atomic.StoreInt64for initializingchainHeightStorageis unnecessary at this point since no other goroutines are accessing it yet.Suggestion: Directly assign the initial value to
chainHeightStorage.var chainHeightStorage int64 -atomic.StoreInt64(&chainHeightStorage, status.SyncInfo.LatestBlockHeight) +chainHeightStorage = status.SyncInfo.LatestBlockHeightapp/server/evm_tx_indexer_cli.go (1)
76-78: Improve error message clarity whenfromBlockexceeds maximum available heightWhen
fromBlockis greater than the maximum available block height, the error message could be more informative to help the user understand the issue.Apply this diff to enhance the error message:
if fromBlock > maxAvailableHeight { - return fmt.Errorf("maximum available block is: %d", maxAvailableHeight) + return fmt.Errorf("minBlockNumber (%d) cannot be greater than the maximum available block height (%d)", fromBlock, maxAvailableHeight) }app/server/start.go (1)
430-430: Nitpick: Clarify terminology in commentThe terminology in the comment can be improved for clarity and consistency.
Apply this diff to refine the comment:
- // If grpc is enabled, configure grpc rpcClient for grpc gateway and json-rpc. + // If gRPC is enabled, configure gRPC client for gRPC gateway and JSON-RPC.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- CHANGELOG.md (1 hunks)
- app/server/evm_tx_indexer_cli.go (1 hunks)
- app/server/evm_tx_indexer_service.go (1 hunks)
- app/server/json_rpc.go (3 hunks)
- app/server/start.go (7 hunks)
- app/server/util.go (1 hunks)
- cmd/nibid/cmd/root.go (1 hunks)
- contrib/scripts/localnet.sh (1 hunks)
- e2e/evm/test/utils.ts (1 hunks)
- eth/indexer/evm_tx_indexer.go (7 hunks)
- eth/indexer/evm_tx_indexer_test.go (2 hunks)
- eth/rpc/backend/backend.go (3 hunks)
- eth/rpc/backend/tx_info.go (4 hunks)
- eth/rpc/backend/tx_info_test.go (3 hunks)
- x/common/testutil/testnetwork/start_node.go (3 hunks)
- x/common/testutil/testnetwork/validator_node.go (3 hunks)
Files skipped from review due to trivial changes (1)
- app/server/util.go
Additional context used
Shellcheck
contrib/scripts/localnet.sh
[warning] 169-169: Quotes/backslashes in this variable will not be respected.
(SC2090)
GitHub Check: codecov/patch
eth/indexer/evm_tx_indexer.go
[warning] 72-72: eth/indexer/evm_tx_indexer.go#L72
Added line #L72 was not covered by tests
[warning] 82-82: eth/indexer/evm_tx_indexer.go#L82
Added line #L82 was not covered by tests
[warning] 109-114: eth/indexer/evm_tx_indexer.go#L109-L114
Added lines #L109 - L114 were not covered by tests
[warning] 212-216: eth/indexer/evm_tx_indexer.go#L212-L216
Added lines #L212 - L216 were not covered by tests
[warning] 218-218: eth/indexer/evm_tx_indexer.go#L218
Added line #L218 was not covered by testsx/common/testutil/testnetwork/start_node.go
[warning] 139-139: x/common/testutil/testnetwork/start_node.go#L139
Added line #L139 was not covered by testsx/common/testutil/testnetwork/validator_node.go
[warning] 178-178: x/common/testutil/testnetwork/validator_node.go#L178
Added line #L178 was not covered by tests
Additional comments not posted (31)
e2e/evm/test/utils.ts (1)
50-50: LGTM!The addition of the
console.logstatement to log thetxResponseobject is a useful change for debugging purposes. It provides visibility into the transaction response without altering the existing logic or control flow of the function.This change aligns with the PR objective of implementing an EVM transaction indexer service, as logging the transaction response can help with monitoring and indexing transactions.
eth/rpc/backend/backend.go (1)
29-29: LGTM!The field renaming aligns with the naming convention and improves code readability. The underlying data type remains unchanged.
app/server/json_rpc.go (2)
24-32: LGTM!The variable renaming from
tmWsClienttotmWsClientForRPCApiimproves clarity by specifying the purpose of the WebSocket client connection for the JSON-RPC API. The change is consistent with the AI-generated summary and does not introduce any issues.
112-113: LGTM!The variable renaming from
tmWsClienttotmWsClientForRPCWsimproves clarity by specifying the purpose of the WebSocket client connection for the WebSocket server. The change is consistently applied in theNewWebsocketsServerfunction call and does not introduce any issues.x/common/testutil/testnetwork/start_node.go (5)
9-9: LGTM!The import statement is valid and follows the common practice of using an alias for the package name.
136-143: LGTM!The changes to the EVM indexer initialization are valid and improve the test network setup:
- Using an in-memory database is a good choice for the test environment.
- The enhanced error handling provides clearer error messages.
- Assigning the
evmTxIndexerServiceto the validator ensures its accessibility.Tools
GitHub Check: codecov/patch
[warning] 139-139: x/common/testutil/testnetwork/start_node.go#L139
Added line #L139 was not covered by tests
145-146: LGTM!Passing the
val.EthTxIndexerto theStartJSONRPCfunction is necessary to enable the JSON-RPC server to handle queries related to EVM transactions and blocks.
165-165: LGTM!Passing the
val.EthTxIndexerto theNewBackendfunction is necessary for the Ethereum RPC backend to handle queries and requests related to EVM transactions and blocks.
139-139: Skipping the static analysis hint.The lack of test coverage for the error handling block at line 139 is not a critical issue in the context of a test network setup. Simulating an error condition during the indexer initialization to cover this line may not be a high priority.
Tools
GitHub Check: codecov/patch
[warning] 139-139: x/common/testutil/testnetwork/start_node.go#L139
Added line #L139 was not covered by testseth/rpc/backend/tx_info_test.go (3)
105-105: LGTM!The change in the expected transaction index from
1to0is a valid correction and aligns with the PR objective of introducing theEVMTxIndexerservice.
146-146: LGTM!The changes in the expected transaction index from
1to0are valid corrections and align with the PR objective of introducing theEVMTxIndexerservice.Also applies to: 152-152
182-182: LGTM!The change in the expected transaction index from
1to0is a valid correction and aligns with the updates made to the test cases and the PR objective of introducing theEVMTxIndexerservice.eth/indexer/evm_tx_indexer_test.go (2)
26-26: LGTM!The test function name change from
TestKVIndexertoTestEVMTxIndexeraccurately reflects the focus on testing the Ethereum transaction indexer. The change is appropriate and maintains the overall structure of the test.
164-164: LGTM!The change in the indexer instantiation from
indexer.NewKVIndexertoindexer.NewEVMTxIndexeraligns with the transition to an Ethereum transaction indexer. The rest of the test case logic remains intact, suggesting that the new indexer is expected to exhibit the same behavior as the previous one. The change is appropriate and maintains the integrity of the test.cmd/nibid/cmd/root.go (1)
139-141: LGTM!The addition of the
server.NewEVMTxIndexCmd()command to theinitRootCmdfunction is a valuable enhancement to the CLI tool. It provides users with the ability to manually trigger the indexing of EVM transactions, which is particularly useful in scenarios where the indexer was disabled for a period or if the node was not stopped gracefully, potentially leading to synchronization issues.The command integrates well with the existing command structure and aligns with the purpose of the
initRootCmdfunction. The comment above the command clearly indicates its purpose, making it easy for users to understand its functionality.Great work on adding this command to improve the usability and robustness of the EVM transaction indexer!
contrib/scripts/localnet.sh (1)
168-170: LGTM!The code segment correctly enables the EVM indexer in the
config/app.tomlfile, which is consistent with the PR objective of implementing the EVM indexer service.The static analysis warning about quotes/backslashes in the variable not being respected is a false positive because the
sedcommand is not using any variables that contain quotes or backslashes.Tools
Shellcheck
[warning] 169-169: Quotes/backslashes in this variable will not be respected.
(SC2090)
eth/indexer/evm_tx_indexer.go (6)
31-31: LGTM!The type assertion ensures that the
EVMTxIndexertype implements theeth.EVMTxIndexerinterface, which is a good practice.
33-34: LGTM!The
EVMTxIndexertype definition is clear and concise, with fields for the database, logger, and client context, which are necessary for indexing transactions.
40-42: LGTM!The
NewEVMTxIndexerfunction correctly initializes and returns a new instance of theEVMTxIndexertype.
46-50: LGTM!The comment provides a clear and concise overview of the indexing process performed by the
IndexBlockmethod, which is helpful for understanding its purpose and functionality.
136-142: LGTM!The
LastIndexedBlockandFirstIndexedBlockmethods are simple and clear wrappers around theLoadLastBlockandLoadFirstBlockfunctions, respectively, passing the indexer's database as an argument.
146-170: LGTM!The
GetByTxHashandGetByBlockAndIndexmethods retrieve indexed Ethereum transaction results by transaction hash and block number + transaction index, respectively. They handle errors appropriately and return clear error messages when the requested transaction is not found. TheGetByBlockAndIndexmethod promotes code reuse by calling theGetByTxHashmethod internally after retrieving the transaction hash.x/common/testutil/testnetwork/validator_node.go (3)
15-16: LGTM!The import statement for the
appserverpackage is valid and correctly imports the package from the specified path.
90-94: LGTM!The new fields added to the
Validatorstruct are correctly defined with their respective types. These fields are likely used to integrate the EVM transaction indexer functionality into the validator node, as mentioned in the PR objectives.
167-182: LGTM!The added code block correctly handles the shutdown of the
EthTxIndexerServiceif it exists. Calling theStopmethod ensures a graceful shutdown of the indexer service, and logging the success or failure message helps in monitoring the shutdown process. This aligns with the updated shutdown logic mentioned in the PR objectives.Tools
GitHub Check: codecov/patch
[warning] 178-178: x/common/testutil/testnetwork/validator_node.go#L178
Added line #L178 was not covered by testseth/rpc/backend/tx_info.go (3)
Line range hint
305-317: LGTM! The changes optimize transaction retrieval performance.The updated
GetTxByEthHashfunction prioritizes using theevmTxIndexerto retrieve transactions by Ethereum hash, which is more efficient compared to querying the Tendermint indexer. Falling back to the Tendermint indexer query provides backward compatibility.The error wrapping using
errorsmod.Wrapfadds useful context to any errors that occur during the retrieval process.
Line range hint
323-339: LGTM! The changes optimize transaction retrieval performance.The updated
GetTxByTxIndexfunction prioritizes using theevmTxIndexerto retrieve transactions by block height and transaction index, which is more efficient compared to querying the Tendermint indexer. Falling back to the Tendermint indexer query provides backward compatibility.The error wrapping using
errorsmod.Wrapfadds useful context to any errors that occur during the retrieval process.
Line range hint
341-363: LGTM! The function provides a generic way to query the Tendermint indexer with validation checks.The
queryTendermintTxIndexerfunction offers a flexible way to query the Tendermint transaction indexer using a provided query and a callback function to extract the desired transaction from the results.The function includes important validation checks using
rpc.TxIsValidEnoughto ensure the transaction is valid. It also handles cases where the transaction result code is not 0 by decoding the transaction, which is necessary when the transaction exceeds the block gas limit.The usage of
rpc.ParseTxIndexerResultensures that the transaction indexer result is parsed into a structured format for further processing.CHANGELOG.md (1)
123-125: LGTM!The changes look good:
- The duplicate entry for PR refactor(rpc-backend): remove unnecessary interface code #2039 is correctly removed.
- New entries for PRs feat(evm): evm tx indexer service implemented #2044 and test(evm): backend tests with test network and real txs #2045 are added in the right place under the "Nibiru EVM" section and follow the existing format.
- The descriptions for the new PRs are accurate and provide a good summary of the changes.
app/server/evm_tx_indexer_service.go (1)
70-73: Add type assertion check for message dataThe code assumes that
msg.Datais of typetypes.EventDataNewBlockHeader. If the type assertion fails, it could cause a panic.Suggestion: Include a type assertion check to prevent potential panics.
case msg := <-blockHeadersChan: - eventDataHeader := msg.Data.(types.EventDataNewBlockHeader) + eventDataHeader, ok := msg.Data.(types.EventDataNewBlockHeader) + if !ok { + service.Logger.Error("Unexpected message data type", "data", msg.Data) + continue + }Likely invalid or redundant comment.
app/server/start.go (1)
20-20: Approved: Added rpcclient importThe addition of
rpcclientimport is necessary for the integration with CometBFT's RPC client.
| block, err := service.rpcClient.Block(ctx, &i) | ||
| if err != nil { | ||
| service.Logger.Error("failed to fetch block", "height", i, "err", err) | ||
| break | ||
| } | ||
| blockResult, err := service.rpcClient.BlockResults(ctx, &i) | ||
| if err != nil { | ||
| service.Logger.Error("failed to fetch block result", "height", i, "err", err) | ||
| break | ||
| } |
There was a problem hiding this comment.
Continue indexing after transient RPC errors
When an error occurs while fetching a block or block result, the current implementation breaks out of the loop. This could halt the indexing process indefinitely due to a transient error.
Suggestion: Modify the error handling to log the error and continue with the next block instead of breaking the loop.
block, err := service.rpcClient.Block(ctx, &i)
if err != nil {
service.Logger.Error("failed to fetch block", "height", i, "err", err)
- break
+ continue
}
blockResult, err := service.rpcClient.BlockResults(ctx, &i)
if err != nil {
service.Logger.Error("failed to fetch block result", "height", i, "err", err)
- break
+ continue
}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.
| block, err := service.rpcClient.Block(ctx, &i) | |
| if err != nil { | |
| service.Logger.Error("failed to fetch block", "height", i, "err", err) | |
| break | |
| } | |
| blockResult, err := service.rpcClient.BlockResults(ctx, &i) | |
| if err != nil { | |
| service.Logger.Error("failed to fetch block result", "height", i, "err", err) | |
| break | |
| } | |
| block, err := service.rpcClient.Block(ctx, &i) | |
| if err != nil { | |
| service.Logger.Error("failed to fetch block", "height", i, "err", err) | |
| continue | |
| } | |
| blockResult, err := service.rpcClient.BlockResults(ctx, &i) | |
| if err != nil { | |
| service.Logger.Error("failed to fetch block result", "height", i, "err", err) | |
| continue | |
| } |
| go func(ctx context.Context) { | ||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| service.Logger.Info("Stopping indexer goroutine") | ||
| err := service.evmTxIndexer.CloseDBAndExit() | ||
| if err != nil { | ||
| service.Logger.Error("Error closing indexer DB", "err", err) | ||
| } | ||
| return | ||
| case msg := <-blockHeadersChan: | ||
| eventDataHeader := msg.Data.(types.EventDataNewBlockHeader) | ||
| currentChainHeight := eventDataHeader.Header.Height | ||
| chainHeight := atomic.LoadInt64(&chainHeightStorage) | ||
| if currentChainHeight > chainHeight { | ||
| atomic.StoreInt64(&chainHeightStorage, currentChainHeight) | ||
| // notify | ||
| select { | ||
| case newBlockSignal <- struct{}{}: | ||
| default: | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }(ctx) |
There was a problem hiding this comment.
Ensure proper resource cleanup on service stop
The goroutine listening for new blocks calls service.evmTxIndexer.CloseDBAndExit() when the context is canceled. This could lead to race conditions or multiple calls to close the database if there are other goroutines or if OnStop also attempts to close the database.
Suggestion: Move the database closure to the OnStop method to ensure it is called only once during the service shutdown process.
Apply the following changes:
--- a/app/server/evm_tx_indexer_service.go
+++ b/app/server/evm_tx_indexer_service.go
@@ -69,10 +69,7 @@
case <-ctx.Done():
service.Logger.Info("Stopping indexer goroutine")
- err := service.evmTxIndexer.CloseDBAndExit()
- if err != nil {
- service.Logger.Error("Error closing indexer DB", "err", err)
- }
+ // Context canceled, exiting goroutine
return
case msg := <-blockHeadersChan:Then, in the OnStop method:
func (service *EVMTxIndexerService) OnStop() {
service.Logger.Info("Stopping EVMTxIndexerService")
if service.cancelFunc != nil {
service.Logger.Info("Calling EVMIndexerService CancelFunc")
service.cancelFunc()
}
+ err := service.evmTxIndexer.CloseDBAndExit()
+ if err != nil {
+ service.Logger.Error("Error closing indexer DB", "err", err)
+ }
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| go func(ctx context.Context) { | |
| for { | |
| select { | |
| case <-ctx.Done(): | |
| service.Logger.Info("Stopping indexer goroutine") | |
| err := service.evmTxIndexer.CloseDBAndExit() | |
| if err != nil { | |
| service.Logger.Error("Error closing indexer DB", "err", err) | |
| } | |
| return | |
| case msg := <-blockHeadersChan: | |
| eventDataHeader := msg.Data.(types.EventDataNewBlockHeader) | |
| currentChainHeight := eventDataHeader.Header.Height | |
| chainHeight := atomic.LoadInt64(&chainHeightStorage) | |
| if currentChainHeight > chainHeight { | |
| atomic.StoreInt64(&chainHeightStorage, currentChainHeight) | |
| // notify | |
| select { | |
| case newBlockSignal <- struct{}{}: | |
| default: | |
| } | |
| } | |
| } | |
| } | |
| }(ctx) | |
| go func(ctx context.Context) { | |
| for { | |
| select { | |
| case <-ctx.Done(): | |
| service.Logger.Info("Stopping indexer goroutine") | |
| // Context canceled, exiting goroutine | |
| return | |
| case msg := <-blockHeadersChan: | |
| eventDataHeader := msg.Data.(types.EventDataNewBlockHeader) | |
| currentChainHeight := eventDataHeader.Header.Height | |
| chainHeight := atomic.LoadInt64(&chainHeightStorage) | |
| if currentChainHeight > chainHeight { | |
| atomic.StoreInt64(&chainHeightStorage, currentChainHeight) | |
| // notify | |
| select { | |
| case newBlockSignal <- struct{}{}: | |
| default: | |
| } | |
| } | |
| } | |
| } | |
| }(ctx) |
| for { | ||
| chainHeight := atomic.LoadInt64(&chainHeightStorage) | ||
| if chainHeight <= lastIndexedHeight { | ||
| // nothing to index. wait for signal of new block | ||
| select { | ||
| case <-newBlockSignal: | ||
| case <-time.After(NewBlockWaitTimeout): | ||
| } | ||
| continue | ||
| } | ||
| for i := lastIndexedHeight + 1; i <= chainHeight; i++ { | ||
| block, err := service.rpcClient.Block(ctx, &i) | ||
| if err != nil { | ||
| service.Logger.Error("failed to fetch block", "height", i, "err", err) | ||
| break | ||
| } | ||
| blockResult, err := service.rpcClient.BlockResults(ctx, &i) | ||
| if err != nil { | ||
| service.Logger.Error("failed to fetch block result", "height", i, "err", err) | ||
| break | ||
| } | ||
| if err := service.evmTxIndexer.IndexBlock(block.Block, blockResult.TxsResults); err != nil { | ||
| service.Logger.Error("failed to index block", "height", i, "err", err) | ||
| } | ||
| lastIndexedHeight = blockResult.Height | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle context cancellation in the indexer loop
The indexer loop does not currently check for context cancellation. If the service is stopping, the loop may continue running, preventing a graceful shutdown.
Suggestion: Add a context check at the beginning of the loop to exit when the context is canceled.
for {
+ select {
+ case <-ctx.Done():
+ service.Logger.Info("Context canceled, stopping indexer loop")
+ return
+ default:
+ }
+
chainHeight := atomic.LoadInt64(&chainHeightStorage)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.
| for { | |
| chainHeight := atomic.LoadInt64(&chainHeightStorage) | |
| if chainHeight <= lastIndexedHeight { | |
| // nothing to index. wait for signal of new block | |
| select { | |
| case <-newBlockSignal: | |
| case <-time.After(NewBlockWaitTimeout): | |
| } | |
| continue | |
| } | |
| for i := lastIndexedHeight + 1; i <= chainHeight; i++ { | |
| block, err := service.rpcClient.Block(ctx, &i) | |
| if err != nil { | |
| service.Logger.Error("failed to fetch block", "height", i, "err", err) | |
| break | |
| } | |
| blockResult, err := service.rpcClient.BlockResults(ctx, &i) | |
| if err != nil { | |
| service.Logger.Error("failed to fetch block result", "height", i, "err", err) | |
| break | |
| } | |
| if err := service.evmTxIndexer.IndexBlock(block.Block, blockResult.TxsResults); err != nil { | |
| service.Logger.Error("failed to index block", "height", i, "err", err) | |
| } | |
| lastIndexedHeight = blockResult.Height | |
| } | |
| } | |
| } | |
| for { | |
| select { | |
| case <-ctx.Done(): | |
| service.Logger.Info("Context canceled, stopping indexer loop") | |
| return | |
| default: | |
| } | |
| chainHeight := atomic.LoadInt64(&chainHeightStorage) | |
| if chainHeight <= lastIndexedHeight { | |
| // nothing to index. wait for signal of new block | |
| select { | |
| case <-newBlockSignal: | |
| case <-time.After(NewBlockWaitTimeout): | |
| } | |
| continue | |
| } | |
| for i := lastIndexedHeight + 1; i <= chainHeight; i++ { | |
| block, err := service.rpcClient.Block(ctx, &i) | |
| if err != nil { | |
| service.Logger.Error("failed to fetch block", "height", i, "err", err) | |
| break | |
| } | |
| blockResult, err := service.rpcClient.BlockResults(ctx, &i) | |
| if err != nil { | |
| service.Logger.Error("failed to fetch block result", "height", i, "err", err) | |
| break | |
| } | |
| if err := service.evmTxIndexer.IndexBlock(block.Block, blockResult.TxsResults); err != nil { | |
| service.Logger.Error("failed to index block", "height", i, "err", err) | |
| } | |
| lastIndexedHeight = blockResult.Height | |
| } | |
| } | |
| } |
| toBlock = maxAvailableHeight | ||
| } | ||
| } |
There was a problem hiding this comment.
Add check for toBlock less than minAvailableHeight
Currently, there is no validation to ensure that toBlock is not less than the minimum available block height. This could cause the indexing process to malfunction if toBlock is set to a height lower than what's available.
Consider adding this check:
if toBlock < minAvailableHeight {
return fmt.Errorf("maxBlockNumber (%d) cannot be less than the minimum available block height (%d)", toBlock, minAvailableHeight)
}| err = evmTxIndexer.CloseDBAndExit() | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
Inconsistent error handling for CloseDBAndExit in app/server/evm_tx_indexer_cli.go.
- Consider adding error logging or additional context when returning the error to ensure proper cleanup handling.
Analysis chain
Ensure proper cleanup when closing the indexer
The error returned by evmTxIndexer.CloseDBAndExit() is currently being returned without additional context. Confirm whether any additional cleanup is necessary or if error logging is sufficient to handle potential issues during the closing process.
Run the following script to search for other usages of CloseDBAndExit() and verify error handling practices:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of CloseDBAndExit in the codebase to verify error handling.
# Search for CloseDBAndExit method calls and display surrounding context.
rg --type go -C 3 $'CloseDBAndExit\(\)'
Length of output: 1298
| @@ -437,7 +442,7 @@ | |||
| } | |||
|
|
|||
| clientCtx = clientCtx.WithGRPCClient(grpcClient) | |||
| ctx.Logger.Debug("gRPC client assigned to client context", "address", grpcAddress) | |||
| ctx.Logger.Debug("gRPC rpcClient assigned to rpcClient context", "address", grpcAddress) | |||
There was a problem hiding this comment.
Improve clarity of debug log message
The debug log message uses "rpcClient context," which might be unclear. Clarify the message for better understanding.
Apply this diff to enhance the log message:
- ctx.Logger.Debug("gRPC rpcClient assigned to rpcClient context", "address", grpcAddress)
+ ctx.Logger.Debug("gRPC client assigned to client context", "address", grpcAddress)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.
| ctx.Logger.Debug("gRPC rpcClient assigned to rpcClient context", "address", grpcAddress) | |
| ctx.Logger.Debug("gRPC client assigned to client context", "address", grpcAddress) |
NOTES
EVMTxIndexeris a service which indexes EVM transactions and blocks to allow queries likeeth_getTransactionByHash,eth_getTransactionByBlockNumberAndIndexand similar on the full nodes / archive nodes.The indexer gets block/tx info from the block store and state db.
EVMTxIndexeris enabled by the configuration parameter of theapp.toml, see:EVM Indexer Background Service
EVMTxIndexerServiceis starting in background along with the JSON RPC server. It listens for the new blocks, filters EVM txs from block results and passes toEVMTxIndexerfor indexing.EVM Indexer CMD for Filling Gaps
If the node had evm indexer disabled for a time being or was not gracefully stopped and there is a chance that evm indexer is not in sync, there is a CLI command
nibid evm-tx-indexis available.The default run of the CMD is:
this will index all blocks that are not yet indexed.
Alternatively, you can index a supply a specific block range:
The CMD checks for the first (base) latest block available on the node, so if the node is pruning, then the base block will be the first available block for evm indexing.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Chores