Local state index #553
Local state index #553
Conversation
# Conflicts: # storage/pebble/ledger.go
# Conflicts: # models/receipt.go # models/transaction.go
# Conflicts: # bootstrap/bootstrap.go # go.mod # go.sum # models/receipt.go # models/transaction.go # tests/go.mod # tests/go.sum # tests/helpers.go
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
services/state/validator.go (2)
18-20: Reminder: Address the TODO comment.The TODO comment suggests introducing a new state of the block: indexed, executed, and validated. Please ensure that this is addressed in a future iteration.
Do you want me to open a GitHub issue to track this task?
55-102: LGTM with a suggestion!The
ValidateBlockmethod looks good. It correctly validates the register updates against the on-chain state and returns anErrStateMismatcherror if any mismatch is found. This ensures the integrity of the blockchain state and prevents inconsistencies in the ledger.One suggestion for improvement:
Consider wrapping the error returned on line 85 with more context about the validation failure. For example:
-return fmt.Errorf("invalid request for register values: %w", err) +return fmt.Errorf("failed to validate block: invalid request for register values: %w", err)This will provide more context about the error when it is propagated up the call stack.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- bootstrap/bootstrap.go (12 hunks)
- services/requester/client_handler.go (1 hunks)
- services/requester/local_client.go (1 hunks)
- services/state/engine.go (1 hunks)
- services/state/state.go (1 hunks)
- services/state/validator.go (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- bootstrap/bootstrap.go
- services/requester/local_client.go
- services/state/engine.go
- services/state/state.go
Additional comments not posted (15)
services/state/validator.go (3)
1-16: LGTM!The package declaration and import statements look good. The necessary dependencies are being imported correctly.
21-53: LGTM!The
RegisterValidatorstruct and its associated methods look good. The struct correctly implements theatree.Ledgerinterface, and theSetValuemethod correctly sets the register value and tracks the update.
66-68: LGTM!The early return statement if there are no updates to validate is a valid optimization. It avoids unnecessary processing when the
updatesmap is empty.services/requester/client_handler.go (12)
27-35: LGTM!The
ClientHandlerstruct has all the necessary fields to handle EVM operations using both remote and local clients. The field names and types are appropriate.
37-62: LGTM!The
NewClientHandlerfunction correctly initializes a newClientHandlerinstance with all the necessary dependencies. The error handling for creating a new remote client is also appropriate.
64-67: LGTM!The
SendRawTransactionmethod correctly uses the remote client to send the raw transaction. The logic is straightforward and there are no apparent issues.
69-84: LGTM!The
GetBalancemethod correctly uses thehandleCallfunction to execute theGetBalancemethod on both local and remote clients concurrently. The error handling logic inhandleCallensures that the correct result is returned. The logic is clear and there are no apparent issues.
86-102: LGTM!The
Callmethod correctly uses thehandleCallfunction to execute theCallmethod on both local and remote clients concurrently. The error handling logic inhandleCallensures that the correct result is returned. The logic is clear and there are no apparent issues.
104-120: LGTM!The
EstimateGasmethod correctly uses thehandleCallfunction to execute theEstimateGasmethod on both local and remote clients concurrently. The error handling logic inhandleCallensures that the correct result is returned. The logic is clear and there are no apparent issues.
122-137: LGTM!The
GetNoncemethod correctly uses thehandleCallfunction to execute theGetNoncemethod on both local and remote clients concurrently. The error handling logic inhandleCallensures that the correct result is returned. The logic is clear and there are no apparent issues.
139-154: LGTM!The
GetCodemethod correctly uses thehandleCallfunction to execute theGetCodemethod on both local and remote clients concurrently. The error handling logic inhandleCallensures that the correct result is returned. The logic is clear and there are no apparent issues.
156-162: LGTM!The
GetLatestEVMHeightmethod correctly uses the remote client to retrieve the latest EVM height from the network. The warning comment about not using this height to query the state is clear and helpful.
164-180: LGTM!The
GetStorageAtmethod correctly uses thehandleCallfunction to execute theGetStorageAtmethod on both local and remote clients concurrently. The error handling logic inhandleCallensures that the correct result is returned. The logic is clear and there are no apparent issues.
182-201: LGTM!The
localClientmethod correctly creates a new local client at a given height. It retrieves the block using the block indexer, creates a new block state with the necessary dependencies, and returns a new local client. The error handling for retrieving the block and creating the block state is also appropriate. The logic is clear and there are no apparent issues.
203-281: LGTM!The
handleCallfunction correctly implements the error handling logic to return the correct result when executing local and remote calls concurrently. It logs the execution times and compares the results and errors appropriately. The logic for handling different scenarios, such as both calls succeeding with different results, one call failing while the other succeeds, and both calls failing with different errors, is clear and well-implemented. There are no apparent issues with the logic or syntax.
…o gregor/local-state/progress
Improve error handling on the local state
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
services/requester/remote_client.go (1)
Line range hint
104-194: Consider refactoring the function into smaller, focused functions.The
NewRemotefunction is quite lengthy and performs multiple responsibilities, such as verifying the COA account balance, creating an emulator configuration, setting up a cache, and creating a COA resource. Consider extracting these responsibilities into separate functions for better readability and maintainability.services/requester/client_handler_test.go (1)
40-42: Refactor repeated logger initialization into a helper function.The logger initialization code is duplicated across multiple test functions. Consider refactoring it into a helper function to reduce code duplication and enhance maintainability.
Add a helper function at the beginning of the file:
func newTestLogger(buf *bytes.Buffer) zerolog.Logger { return zerolog.New(buf).With().Timestamp().Logger() }Then, replace the logger initialization in each test function:
-var buf bytes.Buffer -logger := zerolog.New(&buf).With().Timestamp().Logger() +var buf bytes.Buffer +logger := newTestLogger(&buf)Also applies to: 62-64, 84-86, 106-108, 128-130, 150-152, 172-174, 195-197, 218-220, 241-243
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sum
Files selected for processing (5)
- go.mod (1 hunks)
- services/requester/client_handler.go (1 hunks)
- services/requester/client_handler_test.go (1 hunks)
- services/requester/remote_client.go (25 hunks)
- tests/go.mod (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- go.mod
- services/requester/client_handler.go
- tests/go.mod
Additional comments not posted (14)
services/requester/remote_client.go (14)
Line range hint
85-102: LGTM!The
RemoteClientstruct is well-defined with appropriate fields for its functionality.
Line range hint
196-253: LGTM!The
SendRawTransactionfunction performs appropriate validations on the EVM transaction, handles errors, and correctly builds and sends the Flow transaction.
Line range hint
255-303: LGTM!The
buildTransactionfunction correctly creates a Flow transaction with the provided script and arguments, sets the necessary transaction fields, and signs it with the configured COA account. The use of concurrency to retrieve the latest block and signer network information is a good optimization.
Line range hint
305-345: LGTM!The
GetBalancefunction correctly retrieves the balance of an EVM address at a specific EVM height by executing a Cadence script. It performs the necessary type conversions and handles errors appropriately.
Line range hint
347-394: LGTM!The
GetNoncefunction correctly retrieves the nonce of an EVM address at a specific EVM height by executing a Cadence script. It performs the necessary type conversions and handles errors appropriately.
Line range hint
396-421: LGTM!The
stateAtfunction correctly retrieves the EVM state at a specific EVM height by creating a remote ledger and a new state database. It performs the necessary height conversion and handles errors appropriately.
Line range hint
423-431: LGTM!The
GetStorageAtfunction correctly retrieves the storage value of an EVM address at a specific EVM height and storage hash. It uses thestateAtfunction to retrieve the state database and handles errors appropriately.
Line range hint
433-482: LGTM!The
Callfunction correctly executes an EVM call at a specific EVM height by executing a Cadence script. It performs the necessary type conversions and handles errors appropriately.
Line range hint
484-538: LGTM!The
EstimateGasfunction correctly estimates the gas consumption of an EVM transaction at a specific EVM height by executing a Cadence script. It performs the necessary type conversions and handles errors appropriately.
Line range hint
540-588: LGTM!The
GetCodefunction correctly retrieves the code of an EVM address at a specific EVM height by executing a Cadence script. It performs the necessary type conversions and handles errors appropriately.
Line range hint
590-614: LGTM!The
GetLatestEVMHeightfunction correctly retrieves the latest EVM height by executing a Cadence script. It handles errors appropriately and returns the result as a uint64.
Line range hint
616-641: LGTM!The
getSignerNetworkInfofunction correctly retrieves the signer account information from the network and returns the key index and sequence number of the signer. It handles errors appropriately and returns informative error messages.
Line range hint
643-661: LGTM!The
replaceAddressesfunction correctly replaces the import statements and COA address in the provided script based on the network configuration. It retrieves the system contracts for the configured Flow network ID and replaces the addresses appropriately.
663-669: LGTM!The
evmToCadenceHeightfunction correctly converts an EVM height to a Cadence height using theblocksstorage. It handles errors appropriately and returns informative error messages.
| func containsLogMessage(logOutput, message string) bool { | ||
| return bytes.Contains([]byte(logOutput), []byte(message)) | ||
| } |
There was a problem hiding this comment.
Remove the unused helper function containsLogMessage.
The function containsLogMessage is defined but not used anywhere in the code. Removing unused code helps improve maintainability and readability.
Apply this diff to remove the unused function:
-// Helper function to check if log output contains a specific message
-func containsLogMessage(logOutput, message string) bool {
- return bytes.Contains([]byte(logOutput), []byte(message))
-}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 containsLogMessage(logOutput, message string) bool { | |
| return bytes.Contains([]byte(logOutput), []byte(message)) | |
| } |
Improve client result compare
Make state index more robust and able to sync
Closes: #322
Includes:
#550
#546
#542
#541
#540
#537
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
Release Notes
New Features
ClientHandlerfor managing EVM operations, enhancing local and remote client interactions.Statefield in theBootstrapstruct for state indexing management.BlockStatefor robust transaction execution and state management.Bug Fixes
Documentation
Tests
handleCallfunction to validate local and remote call handling.Chores
go.modfile for improved functionality and stability.