feat(node-api): Allow querying beacon blocks by execution numbers #1824
Conversation
WalkthroughThe changes significantly enhance the beacondb framework, focusing on improved handling of blocks, timestamps, and execution identifiers. Key updates include an increase in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Backend
participant BlockStore
participant ExecutionStorage
User->>Backend: Request data by ExecutionID
Backend->>BlockStore: GetSlotByExecutionNumber(ExecutionID)
BlockStore->>ExecutionStorage: Fetch slot for ExecutionID
ExecutionStorage-->>BlockStore: Return Slot
BlockStore-->>Backend: Return Slot
Backend-->>User: Response with Slot data
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 (
|
![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
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (28)
- mod/beacon/block_store/config.go (1 hunks)
- mod/beacon/block_store/service.go (1 hunks)
- mod/beacon/block_store/types.go (1 hunks)
- mod/consensus-types/pkg/types/block.go (1 hunks)
- mod/node-api/backend/backend.go (3 hunks)
- mod/node-api/backend/types.go (1 hunks)
- mod/node-api/handlers/beacon/backend.go (1 hunks)
- mod/node-api/handlers/proof/backend.go (1 hunks)
- mod/node-api/handlers/proof/block_proposer.go (1 hunks)
- mod/node-api/handlers/proof/execution_fee_recipient.go (1 hunks)
- mod/node-api/handlers/proof/execution_number.go (1 hunks)
- mod/node-api/handlers/proof/handler.go (2 hunks)
- mod/node-api/handlers/proof/routes.go (1 hunks)
- mod/node-api/handlers/proof/types/request.go (1 hunks)
- mod/node-api/handlers/proof/types/response.go (3 hunks)
- mod/node-api/handlers/types/request.go (1 hunks)
- mod/node-api/handlers/utils/constants.go (1 hunks)
- mod/node-api/handlers/utils/id.go (2 hunks)
- mod/node-core/pkg/components/storage/types.go (1 hunks)
- mod/node-core/pkg/components/store.go (1 hunks)
- mod/storage/pkg/beacondb/kvstore.go (1 hunks)
- mod/storage/pkg/block/keys.go (1 hunks)
- mod/storage/pkg/block/store.go (3 hunks)
- mod/storage/pkg/block/types.go (1 hunks)
- mod/storage/pkg/deposit/store.go (2 hunks)
- mod/storage/pkg/encoding/ssz.go (2 hunks)
- mod/storage/pkg/encoding/u64.go (1 hunks)
- mod/storage/pkg/pruner/pruner.go (1 hunks)
Additional comments not posted (56)
mod/storage/pkg/block/keys.go (1)
1-33: LGTM!The constants for key prefixes and map names are well-defined and follow Go naming conventions.
mod/node-api/handlers/types/request.go (1)
31-33: LGTM!The new
TimestampIDRequesttype is well-defined and includes validation.mod/storage/pkg/block/types.go (1)
34-34: LGTM!The new
GetTimestampmethod enhances theBeaconBlockinterface's functionality by allowing timestamp retrieval.mod/node-api/handlers/utils/constants.go (4)
1-20: License Header: ApprovedThe license header is correctly formatted and provides the necessary legal information.
21-23: Package Declaration and Imports: ApprovedThe package declaration and imports are appropriate for the functionality provided in this file.
25-31: Constants Definition: ApprovedThe constants for state IDs and timestamp ID prefix are well-defined and appropriately named.
33-36: Slot Constants: ApprovedThe use of
iotafor defining slot constants is appropriate and efficient.mod/storage/pkg/encoding/u64.go (3)
Line range hint
1-20:
License Header: ApprovedThe license header is correctly formatted and provides the necessary legal information.
21-25: Package Declaration and Imports: ApprovedThe package declaration and imports are appropriate for the functionality provided in this file.
28-39: Global Variables: ApprovedThe global variables
U64KeyandU64Valueare well-defined and the comments provide clear explanations of their purpose and usage.mod/node-api/handlers/proof/backend.go (3)
Line range hint
1-20:
License Header: ApprovedThe license header is correctly formatted and provides the necessary legal information.
21-25: Package Declaration and Imports: ApprovedThe package declaration and imports are appropriate for the functionality provided in this file.
28-31: Backend Interface: ApprovedThe change to include the
GetSlotByTimestampmethod enhances the flexibility of the API by allowing timestamp-based slot retrieval. The interface definitions are clear and well-structured.mod/node-api/handlers/proof/types/request.go (3)
26-29: LGTM!The
BlockProposerRequeststruct now correctly embedstypes.TimestampIDRequestand aligns with the new endpoint path.
32-35: LGTM!The
ExecutionNumberRequeststruct now correctly embedstypes.TimestampIDRequestand aligns with the new endpoint path.
38-41: LGTM!The
ExecutionFeeRecipientRequeststruct now correctly embedstypes.TimestampIDRequestand aligns with the new endpoint path.mod/beacon/block_store/config.go (1)
24-24: LGTM! But verify the impact on the system.The
DefaultAvailabilityWindowhas been increased from1000to8192. This change is likely aimed at enhancing the availability and performance of the block service.However, ensure that this change does not negatively impact memory usage or other system resources.
mod/node-api/handlers/proof/routes.go (3)
37-39: LGTM!The route for
GetBlockProposernow correctly uses:timestamp_idinstead of:block_id.
42-44: LGTM!The route for
GetExecutionNumbernow correctly uses:timestamp_idinstead of:block_id.
47-49: LGTM!The route for
GetExecutionFeeRecipientnow correctly uses:timestamp_idinstead of:block_id.mod/node-core/pkg/components/store.go (2)
27-27: Verify the correctness of the new import path.The import path for the encoding package has been changed. Ensure that the new path
github.com/berachain/beacon-kit/mod/storage/pkg/encodingis correct and consistent with the rest of the codebase.Verification successful
The new import path is correct.
The import path for the encoding package
github.com/berachain/beacon-kit/mod/storage/pkg/encodingis correct and consistent with the package structure in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new import path for the encoding package. # Test: Search for the package declaration. Expect: The package declaration should match the new import path. rg --type go 'package encoding' mod/storage/pkg/encodingLength of output: 156
Line range hint
31-36:
LGTM! But verify the function implementation.The function
ProvideKVStorelooks good. Ensure that the function correctly uses the new import path and that there are no errors introduced by the change.Verification successful
LGTM! But verify the function implementation.
The function
ProvideKVStorelooks good. The function correctly uses the new import paths for theencodingandbeacondbpackages, and there are no errors introduced by the change.
mod/node-core/pkg/components/store.go: Import statements and function implementation are correct.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the function implementation for ProvideKVStore. # Test: Search for the function implementation. Expect: The function implementation should match the new import path. rg --type go 'func ProvideKVStore' mod/node-core/pkg/components/store.goLength of output: 95
Script:
#!/bin/bash # Description: Extract the full implementation of the ProvideKVStore function. # Use ripgrep to search for the ProvideKVStore function and include 20 lines of context to capture the full implementation. rg --type go 'func ProvideKVStore' -A 20 mod/node-core/pkg/components/store.goLength of output: 378
Script:
#!/bin/bash # Description: Verify the import statements for encoding and beacondb packages. # Use ripgrep to search for the import statements in the store.go file. rg --type go 'import' -A 10 mod/node-core/pkg/components/store.goLength of output: 392
mod/beacon/block_store/types.go (1)
39-39: Verify the correctness of the new parameter type.The
Setmethod in theBlockStoreinterface has changed the parameter type forindexfromuint64tomath.Slot. Ensure that the new parameter type is correct and consistent with the rest of the codebase.mod/node-api/handlers/proof/block_proposer.go (1)
40-42: Verify the correctness of the new method.The
Handlerfunction has changed the method used to resolve block identifiers fromresolveBlockIDtoresolveTimestampID. Ensure that the new method is correct and consistent with the rest of the codebase.Verification successful
The new method
resolveTimestampIDis correctly implemented and consistent with the rest of the codebase.
- The method
resolveTimestampIDis implemented inmod/node-api/handlers/proof/handler.go.- It is referenced in multiple files, including
block_proposer.go,execution_fee_recipient.go, andexecution_number.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new method used to resolve block identifiers. # Test: Search for the method implementation. Expect: The method implementation should match the new method. rg --type go 'func resolveTimestampID' mod/node-api/handlers/proof/block_proposer.goLength of output: 86
Script:
#!/bin/bash # Description: Verify the new method used to resolve block identifiers. # Test 1: Search for the method implementation across the entire codebase. rg --type go 'func resolveTimestampID' # Test 2: Search for any references to the method across the entire codebase. rg --type go 'resolveTimestampID'Length of output: 492
Script:
#!/bin/bash # Description: Verify the new method used to resolve block identifiers. # Test: Search for the method implementation within the handler.go file. rg --type go 'func resolveTimestampID' mod/node-api/handlers/proof/handler.goLength of output: 79
Script:
#!/bin/bash # Description: Verify the new method used to resolve block identifiers. # Test: Search for the method implementation within the context of a struct or interface. rg --type go 'resolveTimestampID' -A 10Length of output: 3593
mod/node-api/handlers/proof/execution_number.go (1)
41-43: Verify the correctness of the change fromresolveBlockIDtoresolveTimestampID.Ensure that the
resolveTimestampIDmethod correctly handles the timestamp-based resolution and that all relevant parts of the codebase are updated to use this new method.mod/node-api/handlers/proof/execution_fee_recipient.go (1)
41-43: Verify the correctness of the change fromresolveBlockIDtoresolveTimestampID.Ensure that the
resolveTimestampIDmethod correctly handles the timestamp-based resolution and that all relevant parts of the codebase are updated to use this new method.mod/storage/pkg/pruner/pruner.go (1)
91-91: Improved readability of error logging.The consolidation of the error logging statement into a single line improves readability and maintainability without altering functionality.
mod/beacon/block_store/service.go (1)
94-97: Enhanced error logging and simplified slot handling.The simplification of the
slothandling and the enhancement of the error logging mechanism improve readability and provide more context in error messages, aiding in debugging.mod/node-api/handlers/proof/handler.go (1)
76-102: Well-defined method with proper error handling.The new method
resolveTimestampIDis well-defined, with proper error handling to retrieve the slot, beacon state, and block header associated with a given timestamp ID.mod/storage/pkg/deposit/store.go (2)
30-30: LGTM!The import path for the
encodingpackage has been updated to reflect a more general path, enhancing modularity and clarity.
Line range hint
59-63:
LGTM!The prefix used for the
NewPrefixfunction has been updated to use theKeyDepositPrefixconstant. This enhances readability and maintainability by avoiding hardcoded values.mod/storage/pkg/encoding/ssz.go (2)
31-31: LGTM!The
Empty()method has been replaced with a new constraintconstraints.Empty[T]. This change enhances type safety and potentially improves performance by leveraging compile-time checks.
73-73: LGTM!An additional constraint
constraints.Versionablehas been introduced, and theVersion()method has been removed. This ensures that any type used withSSZInterfaceCodecmust support versioning, enhancing compatibility and correctness in serialization and deserialization processes.mod/node-api/handlers/utils/id.go (2)
53-53: LGTM!The type of the
GetSlotByRootmethod's parameter has been changed from a fixed-size byte array to a more abstractcommon.Roottype. This enhances type safety and flexibility in handling different root types.
67-91: LGTM!The new
SlotFromTimestampIDfunction allows querying of slots based on a timestamp ID. This expands the existing capabilities of the module by enabling users to retrieve slots using timestamp identifiers.mod/node-api/handlers/proof/types/response.go (3)
31-31: LGTM! Documentation update forBlockProposerResponse.The change correctly updates the endpoint identifier to use
timestamp_id.
51-51: LGTM! Documentation update forExecutionNumberResponse.The change correctly updates the endpoint identifier to use
timestamp_id.
69-69: LGTM! Documentation update forExecutionFeeRecipientResponse.The change correctly updates the endpoint identifier to use
timestamp_id.mod/storage/pkg/block/store.go (7)
47-49: LGTM! Addition oftimestampsmap inKVStore.The new
timestampsmap enhances the functionality of theKVStoreby allowing it to track block timestamps.
77-82: LGTM! Initialization oftimestampsmap inNewStore.The change ensures that the
timestampsmap is properly set up when a new store is created.
89-92: LGTM! Update toGetfunction parameter type.The change improves type safety by using the more specific
math.Slottype for theslotparameter.
98-119: LGTM! Update toSetfunction to store block root and timestamp.The change ensures that all relevant data is maintained cohesively by storing both the block root and the timestamp.
124-130: LGTM! Update toGetSlotByRootfunction parameter type.The change improves type safety and abstraction by using the more semantically meaningful
common.Roottype for therootparameter.
132-139: LGTM! Addition ofGetSlotByTimestampfunction.The new function enhances the store's querying capabilities by allowing retrieval based on timestamps.
144-182: LGTM! Update toPrunefunction to handle removal of blocks, roots, and timestamps.The change ensures that all related data is cleaned up efficiently by removing blocks, roots, and timestamps.
mod/node-api/backend/types.go (3)
84-84: LGTM! Update toGetmethod parameter type inBlockStoreinterface.The change improves type safety by using the more specific
math.Slottype for theslotparameter.
86-86: LGTM! Update toGetSlotByRootmethod parameter type inBlockStoreinterface.The change improves type safety and abstraction by using the more semantically meaningful
common.Roottype for therootparameter.
87-89: LGTM! Addition ofGetSlotByTimestampmethod inBlockStoreinterface.The new method enhances the interface's querying capabilities by allowing retrieval based on timestamps.
mod/node-api/backend/backend.go (3)
124-127: LGTM!The
AttachNodemethod correctly sets the node on the backend for querying historical heights.
138-141: LGTM!The parameter type change in
GetSlotByRootfrom[32]bytetocommon.Rootenhances type safety and readability.
145-151: LGTM!The
GetSlotByTimestampmethod is correctly implemented and complements the existingGetSlotByRootmethod.mod/consensus-types/pkg/types/block.go (1)
238-242: LGTM!The
GetTimestampmethod is correctly implemented and follows the existing method structure.mod/node-core/pkg/components/storage/types.go (2)
50-51: LGTM!The parameter type change in
Getfromuint64tomath.Slotenhances type safety.
52-53: LGTM!The parameter type change in
GetSlotByRootfrom[32]bytetocommon.Rootenhances type safety and readability.mod/storage/pkg/beacondb/kvstore.go (2)
31-31: Verify the usage of theencodingpackage within the file.Ensure that the usage of the
encodingpackage remains consistent with the previous implementation and that there are no errors introduced due to the import path change.Verification successful
Verify the definitions of
SSZInterfaceCodecandSSZValueCodecwithin theencodingpackage.Ensure that these types are correctly defined and used in the
encodingpackage to avoid any potential errors due to the import path change.
The import and usage of the
encodingpackage are correct and consistent.The
SSZInterfaceCodecandSSZValueCodectypes are correctly defined and used within theencodingpackage, and there are no errors introduced due to the import path change.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `encoding` package within the file. # Test: Search for the usage of the `encoding` package. Expect: The usage should be consistent with the previous implementation. rg --type go 'encoding.' mod/storage/pkg/beacondb/kvstore.goLength of output: 394
Script:
#!/bin/bash # Description: Verify the definitions of `SSZInterfaceCodec` and `SSZValueCodec` within the `encoding` package. # Test: Search for the definitions of `SSZInterfaceCodec` and `SSZValueCodec` in the `encoding` package. rg 'SSZInterfaceCodec' mod/storage/pkg/encoding rg 'SSZValueCodec' mod/storage/pkg/encodingLength of output: 1599
31-31: Verify the correctness of the new import path.The import path for the
encodingpackage has been updated. Ensure that the new pathgithub.com/berachain/beacon-kit/mod/storage/pkg/encodingis correct and aligns with the project's structure.Verification successful
The new import path
github.com/berachain/beacon-kit/mod/storage/pkg/encodingis correct.The definitions for
SSZInterfaceCodecandSSZValueCodecare found inmod/storage/pkg/encoding/ssz.go, confirming that the new import path is accurate.
mod/storage/pkg/encoding/ssz.gocontains the necessary definitions forSSZInterfaceCodecandSSZValueCodec.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and correctness of the new import path. # Test: Check if the new import path exists. Expect: The path should exist. if ! fd 'encoding.go' | grep -q 'mod/storage/pkg/encoding'; then echo "Error: The new import path 'mod/storage/pkg/encoding' does not exist." exit 1 fi echo "The new import path 'mod/storage/pkg/encoding' exists."Length of output: 215
Script:
#!/bin/bash # Description: Search for the encoding.go file in the entire project to locate the correct path. # Find the location of the encoding.go file. fd 'encoding.go'Length of output: 96
Script:
#!/bin/bash # Description: Search for the usage of the encoding package within kvstore.go to determine the correct import path. # Find the usage of the encoding package in kvstore.go rg 'encoding\.' mod/storage/pkg/beacondb/kvstore.go -A 5Length of output: 1352
Script:
#!/bin/bash # Description: Search for the definitions of SSZInterfaceCodec and SSZValueCodec in the found encoding.go files. # Search for SSZInterfaceCodec and SSZValueCodec in mod/primitives/pkg/constraints/encoding.go rg 'SSZInterfaceCodec|SSZValueCodec' mod/primitives/pkg/constraints/encoding.go # Search for SSZInterfaceCodec and SSZValueCodec in mod/runtime/pkg/encoding/encoding.go rg 'SSZInterfaceCodec|SSZValueCodec' mod/runtime/pkg/encoding/encoding.goLength of output: 157
Script:
#!/bin/bash # Description: Search for the definitions of SSZInterfaceCodec and SSZValueCodec in the entire project. # Search for SSZInterfaceCodec in the entire project rg 'SSZInterfaceCodec' # Search for SSZValueCodec in the entire project rg 'SSZValueCodec'Length of output: 2374
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
mod/node-api/engines/go.sumis excluded by!**/*.sum
Files selected for processing (4)
- mod/node-api/backend/types.go (1 hunks)
- mod/node-api/engines/echo/vaildator.go (4 hunks)
- mod/node-api/engines/go.mod (1 hunks)
- mod/node-api/handlers/utils/id.go (2 hunks)
Additional comments not posted (17)
mod/node-api/engines/go.mod (6)
16-16: Approved: Addition ofgithub.com/berachain/beacon-kit/mod/chain-spec.This dependency is likely related to blockchain specifications and seems necessary for the module's functionality.
17-17: Approved: Addition ofgithub.com/berachain/beacon-kit/mod/primitives.This dependency provides foundational data types or utilities, which are essential for the module's operations.
21-21: Approved: Addition ofgithub.com/ethereum/go-ethereum.This dependency is significant for providing Ethereum blockchain functionalities, indicating support for Ethereum-related operations.
28-28: Approved: Addition ofgithub.com/holiman/uint256.This dependency is particularly relevant in blockchain applications where large numerical values are common.
29-29: Approved: Addition ofgithub.com/klauspost/cpuid/v2.This dependency enhances performance optimizations based on the underlying hardware by detecting CPU features.
37-37: Approved: Addition ofgithub.com/prysmaticlabs/gohashtree.This dependency is useful for efficient data verification and integrity checks in blockchain implementations.
mod/node-api/handlers/utils/id.go (4)
48-66: Approved: Changes toSlotFromBlockID.The changes enhance type safety and flexibility by using the
common.Roottype instead of a fixed-size byte array.
67-91: Approved: Addition ofSlotFromTimestampID.This new function allows querying slots using a timestamp ID, expanding the module's capabilities to include timestamp-based queries.
93-97: Approved: Addition ofIsTimestampPrefix.This function correctly checks if the given timestampID is prefixed with the timestamp prefix.
99-104: Approved: Repositioning ofU64FromString.The function has been correctly repositioned within the file without any change to its functionality.
mod/node-api/engines/echo/vaildator.go (4)
Line range hint
60-69: Approved: Enhancements toConstructValidator.The function has been enhanced with new validators, including
ValidateTimestampID, improving the validator's capabilities.
98-111: Approved: Addition ofValidateTimestampID.This new function correctly checks if the provided timestamp ID is valid by comparing it against a set of allowed values.
Line range hint
83-96: Approved: Changes toValidateBlockID.The function has been modified to integrate with new validators, improving its capabilities.
Line range hint
155-172: Approved: Changes tovalidateStateBlockIDs.The function has been streamlined to handle empty fields directly within the validation logic for state block IDs, simplifying the validation process.
mod/node-api/backend/types.go (3)
84-84: Improved Type Safety and Abstraction.The change from
[32]bytetocommon.Rootenhances type safety and abstraction.
85-87: New Functionality: Query by Timestamp.The addition of
GetSlotByTimestampenhances theBlockStoreinterface by allowing queries based on timestamps.
88-88: Enhanced Type Safety withmath.Slot.The change from
uint64tomath.Slotimproves type safety and consistency within the interface.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- mod/node-api/backend/mocks/block_store.mock.go (5 hunks)
Additional comments not posted (5)
mod/node-api/backend/mocks/block_store.mock.go (5)
6-8: Imports look good.The new imports for
bytesandmathare appropriate for the updated function signatures and new functionalities.
Line range hint
26-48: Function signature update improves type safety.The update to use
bytes.B32instead of[32]byteenhances type safety and clarity. The function implementation correctly handles the return values.
Line range hint
59-76: Helper methods updated for consistency.The helper methods for
GetSlotByRoothave been updated to reflect the new parameter typebytes.B32, ensuring consistency.
81-106: New functionGetSlotByTimestampadded.The new function
GetSlotByTimestampfollows the pattern of existing mock functions and correctly handles the return values.
109-132: Helper methods forGetSlotByTimestampadded.The helper methods for
GetSlotByTimestampare consistent with the new function and follow the existing patterns for defining mock expectations and return values.
calbera
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (29)
- contracts/src/eip4788/BeaconVerifier.sol (2 hunks)
- contracts/src/eip4788/SSZ.sol (1 hunks)
- contracts/src/eip4788/interfaces/IBeaconVerifier.sol (1 hunks)
- contracts/test/eip4788/SSZ.t.sol (1 hunks)
- mod/consensus-types/pkg/types/block.go (1 hunks)
- mod/consensus-types/pkg/types/block_test.go (2 hunks)
- mod/node-api/backend/backend.go (3 hunks)
- mod/node-api/backend/mocks/block_store.mock.go (5 hunks)
- mod/node-api/backend/types.go (1 hunks)
- mod/node-api/engines/echo/vaildator.go (4 hunks)
- mod/node-api/handlers/proof/backend.go (1 hunks)
- mod/node-api/handlers/proof/block_proposer.go (1 hunks)
- mod/node-api/handlers/proof/execution_fee_recipient.go (1 hunks)
- mod/node-api/handlers/proof/execution_number.go (1 hunks)
- mod/node-api/handlers/proof/handler.go (2 hunks)
- mod/node-api/handlers/proof/routes.go (1 hunks)
- mod/node-api/handlers/proof/types/request.go (1 hunks)
- mod/node-api/handlers/proof/types/response.go (3 hunks)
- mod/node-api/handlers/types/request.go (1 hunks)
- mod/node-api/handlers/utils/constants.go (1 hunks)
- mod/node-api/handlers/utils/id.go (2 hunks)
- mod/node-core/pkg/components/block_store.go (2 hunks)
- mod/node-core/pkg/components/deposit_store.go (2 hunks)
- mod/node-core/pkg/components/storage/provider.go (1 hunks)
- mod/node-core/pkg/components/store.go (2 hunks)
- mod/storage/pkg/block/keys.go (1 hunks)
- mod/storage/pkg/block/store.go (2 hunks)
- mod/storage/pkg/block/types.go (1 hunks)
- mod/storage/pkg/deposit/store.go (2 hunks)
Additional comments not posted (66)
mod/node-api/handlers/types/request.go (2)
Line range hint
14-16:
LGTM! TheBlockIDRequeststructure is well-defined.The
BlockIDfield is appropriately annotated with parameter and validation tags.
31-33: LGTM! TheExecutionIDRequeststructure is well-defined.The
ExecutionIDfield is appropriately annotated with parameter and validation tags.mod/storage/pkg/block/keys.go (2)
23-27: LGTM! The block key prefixes are well-defined.The constants for
BlockKeyPrefix,RootsKeyPrefix, andExecutionNumbersKeyPrefixare appropriately defined.
29-33: LGTM! The map names are well-defined.The constants for
BlocksMapName,RootsMapName, andExecutionNumbersMapNameare appropriately defined.mod/storage/pkg/block/types.go (1)
34-34: LGTM! TheGetExecutionNumbermethod is well-defined.The method is appropriately added to the
BeaconBlockinterface.mod/node-api/handlers/utils/constants.go (4)
1-20: LGTM!The license information is standard and appears correct.
21-21: LGTM!The package declaration is correct.
23-23: LGTM!The import statement is necessary and correct.
25-36: LGTM!The constants are well-defined and follow a consistent naming convention.
mod/node-api/handlers/proof/backend.go (3)
Line range hint
1-20:
LGTM!The license information is standard and appears correct.
22-22: LGTM!The package declaration is correct.
23-25: LGTM!The import statement reformatting improves readability and aligns with Go's conventions.
mod/node-api/handlers/proof/types/request.go (3)
Line range hint
1-20:
LGTM!The license information is standard and appears correct.
Line range hint
22-22:
LGTM!The package declaration is correct.
24-24: LGTM!The import statement is necessary and correct.
mod/node-core/pkg/components/storage/provider.go (3)
30-32: LGTM! Struct definition is clear and concise.The
KVStoreProviderstruct correctly embedsstore.KVStoreWithBatchto extend its functionality.
34-39: LGTM! Constructor function is well-defined.The
NewKVStoreProviderfunction correctly initializes and returns aKVStoreProvider.
41-44: LGTM! Function is simple and effective.The
OpenKVStorefunction correctly returns the embeddedKVStoreWithBatch.mod/node-api/handlers/proof/routes.go (3)
37-37: LGTM! Route path updated to useexecution_id.The route for
GetBlockProposerhas been correctly updated to useexecution_id.
42-42: LGTM! Route path updated to useexecution_id.The route for
GetExecutionNumberhas been correctly updated to useexecution_id.
47-47: LGTM! Route path updated to useexecution_id.The route for
GetExecutionFeeRecipienthas been correctly updated to useexecution_id.mod/node-core/pkg/components/store.go (2)
27-27: LGTM! Import path simplified.The import path for
encodinghas been correctly simplified, improving modularity.
37-37: LGTM! Function signature streamlined.The function signature for
ProvideKVStorehas been correctly streamlined, improving readability.contracts/test/eip4788/SSZ.t.sol (1)
16-25: LGTM! Ensure comprehensive test coverage.The changes correctly update the function to handle
uint256and useSSZ.toLittleEndian. Ensure that the new behavior is adequately covered by tests.mod/node-api/handlers/proof/block_proposer.go (1)
40-42: LGTM! Ensure comprehensive test coverage.The changes correctly update the function to use
resolveExecutionIDinstead ofresolveBlockID. Ensure that the new method is adequately tested.mod/node-api/handlers/proof/execution_number.go (1)
41-43: LGTM! Ensure comprehensive test coverage.The changes correctly update the function to use
resolveExecutionIDinstead ofresolveBlockID. Ensure that the new method is adequately tested.mod/node-api/handlers/proof/execution_fee_recipient.go (1)
41-43: LGTM! But verify the implementation ofresolveExecutionID.The code changes are approved.
However, ensure that the
resolveExecutionIDmethod is correctly implemented and handles errors appropriately.Verification successful
The implementation of
resolveExecutionIDis correct and handles errors appropriately.The method
resolveExecutionIDinmod/node-api/handlers/proof/handler.gocorrectly implements error handling for each potential error source.
SlotFromExecutionIDcall (line 86)StateFromSlotForProofcall (line 91)BlockHeaderAtSlotcall (line 96)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `resolveExecutionID`. # Test: Search for the method implementation. Expect: Correct implementation with proper error handling. ast-grep --lang go --pattern $'func (h *Handler[$_, $_, $_, $_, $_, $_]) resolveExecutionID($_) ($_ , $_, $_, error) { $$$ }'Length of output: 1926
mod/node-core/pkg/components/block_store.go (1)
29-29: LGTM! But verify the implementation ofstorage.NewKVStoreProvider.The code changes are approved.
However, ensure that the
storage.NewKVStoreProvidermethod is correctly implemented and integrated.Verification successful
Let's correct the shell script to search for the
NewKVStoreProviderfunction implementation in the specified directory.
To ensure thorough verification, let's check the definition of
KVStoreProviderand its usage in the codebase.
The implementation of
storage.NewKVStoreProvideris correctly integrated.The
KVStoreProviderstruct and its initialization viaNewKVStoreProviderare properly defined and used in the codebase.
KVStoreProviderstruct is defined inmod/node-core/pkg/components/storage/provider.go.NewKVStoreProviderfunction initializesKVStoreProvidercorrectly.- Usage of
KVStoreProvideris consistent inmod/node-core/pkg/components/deposit_store.goandmod/node-core/pkg/components/block_store.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `storage.NewKVStoreProvider`. # Test: Search for the method implementation. Expect: Correct implementation with proper integration. ast-grep --lang go --pattern $'func NewKVStoreProvider($_) $_ { $$$ }' --path mod/node-core/pkg/components/storageLength of output: 305
Script:
#!/bin/bash # Description: Verify the implementation of `storage.NewKVStoreProvider`. # Test: Search for the method implementation. Expect: Correct implementation with proper integration. ast-grep --lang go --pattern $'func NewKVStoreProvider($_) $_ { $$$ }' mod/node-core/pkg/components/storageLength of output: 499
Script:
#!/bin/bash # Description: Verify the definition and usage of `KVStoreProvider`. # Search for the definition of the `KVStoreProvider` struct. ast-grep --lang go --pattern $'type KVStoreProvider struct { $$$ }' mod/node-core/pkg/components/storage # Search for the usage of `KVStoreProvider` in the codebase. rg 'KVStoreProvider' -A 5Length of output: 2468
mod/node-core/pkg/components/deposit_store.go (1)
28-28: LGTM! But verify the implementation ofstorage.NewKVStoreProvider.The code changes are approved.
However, ensure that the
storage.NewKVStoreProvidermethod is correctly implemented and integrated.Verification successful
Let's correct the shell script and re-run it to verify the implementation of
storage.NewKVStoreProvider.
The implementation of
storage.NewKVStoreProvideris correctly integrated.The
NewKVStoreProviderfunction is properly implemented inmod/node-core/pkg/components/storage/provider.goat line 35, returning a new instance ofKVStoreProviderinitialized withKVStoreWithBatch.
mod/node-core/pkg/components/storage/provider.go:35Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `storage.NewKVStoreProvider`. # Test: Search for the method implementation. Expect: Correct implementation with proper integration. ast-grep --lang go --pattern $'func NewKVStoreProvider($_) $_ { $$$ }' --path mod/node-core/pkg/components/storageLength of output: 305
Script:
#!/bin/bash # Description: Verify the implementation of `storage.NewKVStoreProvider`. # Test: Search for the method implementation. Expect: Correct implementation with proper integration. ast-grep --lang go --pattern $'func NewKVStoreProvider($_) $_ { $$$ }' mod/node-core/pkg/components/storageLength of output: 499
mod/storage/pkg/deposit/store.go (2)
30-30: Import path updated for better modularity.The import path for the
encodingpackage has been modified, likely to improve the package structure.
50-50: Prefix updated for better readability and consistency.The prefix for the
NewPrefixfunction has been updated from a hardcoded byte array to the constantKeyDepositPrefix, enhancing readability and consistency.mod/node-api/handlers/proof/handler.go (1)
74-102: New methodresolveExecutionIDadded to enhance functionality.The new method
resolveExecutionIDretrieves the slot associated with a given execution ID, along with the corresponding beacon state and block header. It is well-structured, ensuring clear error reporting and early exit on failures.contracts/src/eip4788/interfaces/IBeaconVerifier.sol (1)
63-68: Parameter type updated touint256for better handling of larger values.The type of the
blockNumberparameter has been changed fromuint64touint256, enhancing the function's ability to handle larger values and aligning with broader Ethereum standards.mod/node-api/handlers/proof/types/response.go (3)
31-31: Documentation Update Approved.The update to replace
block_idwithexecution_idin the comment improves clarity and aligns with the new API usage.
51-51: Documentation Update Approved.The update to replace
block_idwithexecution_idin the comment improves clarity and aligns with the new API usage.
69-69: Documentation Update Approved.The update to replace
block_idwithexecution_idin the comment improves clarity and aligns with the new API usage.mod/node-api/handlers/utils/id.go (3)
48-66: Improved Type Safety and Flexibility Approved.The change of the parameter type from a fixed-size byte array to
common.Rootenhances type safety and flexibility.
67-91: New Function Addition Approved.The
SlotFromExecutionIDfunction correctly parses the execution number and handles errors appropriately, enhancing the module's capabilities.
99-104: Function Repositioning Approved.The
U64FromStringfunction's repositioning within the file does not affect its functionality.contracts/src/eip4788/SSZ.sol (1)
Line range hint
1-104:
Function Removal Approved. Verify Impact on Codebase.The removal of the
uint64HashTreeRootfunction simplifies the library, but ensure it does not negatively impact the codebase.Verification successful
Function Removal Verified. No Impact on Codebase.
The removal of the
uint64HashTreeRootfunction has been verified, and there are no occurrences of this function in the codebase. Therefore, its removal does not impact the functionality of the library.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing the `uint64HashTreeRoot` function. # Test: Search for usage of `uint64HashTreeRoot`. Expect: No occurrences. rg --type solidity -A 5 $'uint64HashTreeRoot'Length of output: 77
Script:
#!/bin/bash # Description: Verify the impact of removing the `uint64HashTreeRoot` function. # Test: Search for usage of `uint64HashTreeRoot`. Expect: No occurrences. rg --type-add 'solidity:*.sol' -A 5 $'uint64HashTreeRoot'Length of output: 56
mod/node-api/backend/mocks/block_store.mock.go (4)
25-52: Review the newGetSlotByExecutionNumbermethod.The method correctly mocks the functionality of retrieving a slot by execution number. Ensure that all return paths are handled correctly.
53-76: Review the newBlockStore_GetSlotByExecutionNumber_Calltype.The type correctly supports the chaining of mock call methods for
GetSlotByExecutionNumber.
Line range hint
82-123:
Review changes toGetSlotByRootmethod.The method correctly handles the new
bytes.B32type and maintains existing functionality.
6-8: Ensure correct usage of new imports.The new imports for
bytesandmathfrom thebeacon-kitpackage should be used correctly in the file.Verification successful
Imports are correctly used.
The new imports for
bytesandmathfrom thebeacon-kitpackage are used correctly in the file:
math.U64is used in multiple function signatures and implementations.bytes.B32is used in function signatures and implementations.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new imports. # Test: Search for the usage of `bytes` and `math` in the file. rg 'bytes|math' mod/node-api/backend/mocks/block_store.mock.goLength of output: 1858
mod/storage/pkg/block/store.go (6)
37-43: Review the addition ofexecutionNumbersfield.The
executionNumbersfield is correctly integrated into theKVStorestruct.
54-74: Review changes toNewStorefunction.The function correctly initializes all fields, including the new
executionNumbersfield.
79-84: Review changes toGetmethod.The method correctly handles the new
math.Slottype and maintains existing functionality.
88-112: Review changes toSetmethod.The method correctly handles the new fields and maintains existing functionality.
125-133: Review the newGetSlotByExecutionNumbermethod.The method correctly retrieves slots based on execution numbers.
30-32: Ensure correct usage of new imports.The new imports for
commonandmathfrom thebeacon-kitpackage should be used correctly in the file.Verification successful
Verified: New imports are used correctly.
The new imports for
commonandmathfrom thebeacon-kitpackage are used appropriately in the file.
math.Slotis used in multiple places.math.U64is used in function signatures.common.Rootis used in function parameters.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new imports. # Test: Search for the usage of `common` and `math` in the file. rg 'common|math' mod/storage/pkg/block/store.goLength of output: 659
mod/node-api/engines/echo/vaildator.go (4)
Line range hint
61-75:
Review changes toConstructValidatorfunction.The function correctly registers the new
ValidateExecutionIDvalidator.
98-111: Review the newValidateExecutionIDfunction.The function correctly validates execution IDs based on the allowed values.
Line range hint
91-96:
Review changes toValidateBlockIDfunction.The function correctly integrates with the new validators and maintains existing functionality.
30-32: Ensure correct usage of new import.The new import for the
utilspackage should be used correctly in the file.Verification successful
Verify definitions in the
utilspackage.The constants
StateIDHead,StateIDGenesis,StateIDFinalized,StateIDJustified, and the functionIsExecutionNumberPrefixshould be correctly defined in theutilspackage.
Verification successful.
The
utilspackage is correctly imported and used in the file. The constantsStateIDHead,StateIDGenesis,StateIDFinalized,StateIDJustified, and the functionIsExecutionNumberPrefixare properly defined and utilized.
StateIDHeaddefined inmod/node-api/handlers/utils/constants.goStateIDGenesisdefined inmod/node-api/handlers/utils/constants.goStateIDFinalizeddefined inmod/node-api/handlers/utils/constants.goStateIDJustifieddefined inmod/node-api/handlers/utils/constants.goIsExecutionNumberPrefixdefined inmod/node-api/handlers/utils/id.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new import. # Test: Search for the usage of `utils` in the file. rg 'utils' mod/node-api/engines/echo/vaildator.goLength of output: 297
Script:
#!/bin/bash # Description: Verify definitions in the utils package. # Test: Search for the definitions of the constants and function in the utils package. rg 'StateIDHead|StateIDGenesis|StateIDFinalized|StateIDJustified|IsExecutionNumberPrefix' mod/node-api/handlers/utilsLength of output: 826
mod/node-api/backend/types.go (3)
84-84: Improved type safety and clarity.The update to use
common.Rootinstead of[32]byteenhances type safety and clarity.
85-87: New method added for execution number-based queries.The addition of
GetSlotByExecutionNumberbroadens the interface's capabilities and aligns with the PR objectives.
84-84: Improved semantic representation and type safety.The update to use
math.Slotinstead ofuint64enhances the semantic representation of slots and improves type safety.mod/consensus-types/pkg/types/block_test.go (2)
45-45: Execution number added toExecutionPayload.The addition of the
Numberfield enhances the functionality of theExecutionPayloadby allowing it to carry an execution number.
104-104: New assertion added to validate execution number.The new assertion ensures that the block correctly reflects the execution number, enhancing the robustness of the test suite.
mod/node-api/backend/backend.go (3)
124-127: New method added for attaching node.The addition of the
AttachNodemethod sets the node on the backend for querying historical heights, providing clearer context for developers.
138-141: Improved type safety and clarity.The update to use
common.Rootinstead of[32]byteenhances type safety and clarity.
145-151: New method added for execution number-based queries.The addition of
GetSlotByExecutionNumberexpands the backend's querying capabilities and aligns with the PR objectives.mod/consensus-types/pkg/types/block.go (2)
237-237: LGTM!The
GetHeaderfunction is well-implemented and does not have any issues.
238-242: LGTM! But verify the initialization ofExecutionPayload.The code changes are approved.
However, ensure that the
ExecutionPayloadis always initialized to avoid potentialnilpointer dereference.contracts/src/eip4788/BeaconVerifier.sol (2)
124-124: LGTM! But verify the consistency ofblockNumbertype change.The code changes are approved.
However, ensure that the change in
blockNumbertype fromuint64touint256is consistent across the codebase.Verification successful
Let's generate a new shell script to verify the consistency of the
blockNumbertype change across the codebase.
The change in
blockNumbertype fromuint64touint256is consistent across the codebase.
contracts/src/eip4788/BeaconVerifier.solcontracts/src/eip4788/interfaces/IBeaconVerifier.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of `blockNumber` type change in `BeaconVerifier`. # Test: Search for the usage of `blockNumber` in `BeaconVerifier`. Expect: All instances of `blockNumber` are of type `uint256`. rg --type solidity -A 5 'blockNumber' | grep -i 'uint256'Length of output: 88
Script:
#!/bin/bash # Description: Verify the consistency of `blockNumber` type change in `BeaconVerifier`. # Test: Search for the usage of `blockNumber` in Solidity files. Expect: All instances of `blockNumber` are of type `uint256`. rg 'blockNumber' -g '*.sol' -A 5 | grep -i 'uint256'Length of output: 513
190-199: LGTM! But verify the consistency ofblockNumbertype change and hashing mechanism.The code changes are approved.
However, ensure that the change in
blockNumbertype fromuint64touint256and the update in hashing mechanism are consistent across the codebase.Verification successful
The change in
blockNumbertype touint256and the update in the hashing mechanism toSSZ.toLittleEndianare consistent across the codebase.
- All instances of
blockNumberare of typeuint256.- The
executionNumberRootusesSSZ.toLittleEndian.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of `blockNumber` type change and hashing mechanism in `BeaconVerifier`. # Test: Search for the usage of `blockNumber` and `executionNumberRoot` in `BeaconVerifier`. Expect: All instances of `blockNumber` are of type `uint256` and `executionNumberRoot` uses `SSZ.toLittleEndian`. rg --type solidity -A 5 'blockNumber' | grep -i 'uint256' rg --type solidity -A 5 'executionNumberRoot' | grep -i 'SSZ.toLittleEndian'Length of output: 196
Script:
#!/bin/bash # Description: Verify the consistency of `blockNumber` type change and hashing mechanism in `BeaconVerifier`. # Test: Search for the usage of `blockNumber` and `executionNumberRoot` in Solidity files. Expect: All instances of `blockNumber` are of type `uint256` and `executionNumberRoot` uses `SSZ.toLittleEndian`. rg 'blockNumber' -A 5 --glob '*.sol' | grep -i 'uint256' rg 'executionNumberRoot' -A 5 --glob '*.sol' | grep -i 'SSZ.toLittleEndian'Length of output: 703
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- mod/node-core/pkg/components/storage/types.go (1 hunks)
Additional comments not posted (2)
mod/node-core/pkg/components/storage/types.go (2)
50-50: Improved type safety inGetmethod.The update from
uint64tomath.Slotenhances type safety by ensuring that only appropriate slot values are processed.
52-52: Enhanced code clarity inGetSlotByRootmethod.The update from
[32]bytetocommon.Rootimproves code clarity and maintainability by using a more descriptive and flexible representation of root values.
executionIDinstead of beacon API specblockIDfor proof endpointsSummary by CodeRabbit
Summary by CodeRabbit
New Features
GetTimestampandGetSlotByTimestampfor improved timestamp and slot retrieval.resolveExecutionIDmethod for fetching slots based on execution IDs.GetSlotByTimestampin the BlockStore mock implementation.Improvements
DefaultAvailabilityWindowfor enhanced service performance.uint64withmath.Slot.PruneandSetmethods from the BlockStore interface to streamline functionality.Bug Fixes
listenAndStoremethod to include slot values for better debugging.Documentation
BlockIDtoExecutionID.