Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add getBlobSidecars route to Beacon API #2159

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MaanavKhaitan
Copy link
Contributor

@MaanavKhaitan MaanavKhaitan commented Nov 16, 2024

What

Testing

Verified that the correct blob sidecar is returned

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a new BlobBackend interface for handling blob sidecars.
    • Added BlobSidecarsAtSlot method for retrieving blob sidecars by slot and indices.
    • Enhanced GetBlobsFromStore method in AvailabilityStore to fetch blob sidecars.
  • Improvements

    • Updated type definitions for blob sidecars to improve type safety.
    • Enhanced concurrency in methods for persisting and retrieving blob sidecars.
    • Added new methods to the BlobSidecar interface for improved data access.
  • Bug Fixes

    • Corrected validation rules for indices in blob sidecar requests.
  • Documentation

    • Updated comments for clarity and accuracy regarding blob sidecars.

Copy link
Contributor

coderabbitai bot commented Nov 16, 2024

Walkthrough

The pull request introduces significant modifications across multiple files to enhance the handling of blob sidecars within the application. Key changes include the reordering of type parameters in function calls, the addition of new methods and interfaces for blob sidecars, and updates to existing method signatures to accommodate new generic structures. Notably, a new BlobBackend interface is introduced, and several methods are added to improve data retrieval and processing related to blob sidecars. These changes collectively enhance the type safety and functionality of the backend components.

Changes

File Path Change Summary
beacond/cmd/defaults.go Modified DefaultComponents function to reorder type parameters in components.ProvideNodeAPIBackend.
beacond/cmd/types.go Updated NodeAPIBackend type alias to include *BlobSidecar.
mod/beacon/blockchain/types.go Corrected comment for BlobSidecars interface from "blobs sidecars" to "blob sidecars".
mod/da/pkg/store/store.go Added constant SLOT_COMMITMENTS_KEY and modified Persist method for parallel processing; introduced GetBlobsFromStore method.
mod/da/pkg/store/types.go Added Get method to IndexDB interface for data retrieval.
mod/da/pkg/types/commitments.go Introduced SlotCommitments type with associated methods for KZG commitments.
mod/da/pkg/types/sidecar.go Added GetIndex and GetInclusionProof methods to BlobSidecar struct.
mod/node-api/backend/backend.go Updated type parameters for BlobSidecarsT and BlobSidecarT in Backend struct and related methods.
mod/node-api/backend/blob.go Introduced BlobSidecarsAtSlot method to retrieve blob sidecars by slot.
mod/node-api/backend/block.go Updated method signatures for BlockHeaderAtSlot, BlockRootAtSlot, and BlockRewardsAtSlot with additional type parameters.
mod/node-api/backend/genesis.go Updated GenesisValidatorsRoot method signature to include an extra underscore in type parameters.
mod/node-api/backend/randao.go Modified RandaoAtEpoch method signature to reflect new type parameters.
mod/node-api/backend/state.go Updated StateFromSlotForProof, StateRootAtSlot, and StateForkAtSlot method signatures with additional underscores.
mod/node-api/backend/types.go Added new interfaces and methods for blob sidecars, including BlobSidecar and BlobSidecars.
mod/node-api/backend/validator.go Updated method signatures for ValidatorByID, ValidatorsByIDs, and ValidatorBalancesByIDs to include new type parameters.
mod/node-api/handlers/beacon/backend.go Introduced BlobBackend interface and updated Backend interface to include it.
mod/node-api/handlers/beacon/blob.go Added GetBlobSidecars method to handle requests for blob sidecars.
mod/node-api/handlers/beacon/routes.go Changed handler for /eth/v1/beacon/blob_sidecars/:block_id from h.NotImplemented to h.GetBlobSidecars.
mod/node-api/handlers/beacon/types/request.go Updated validation rules for Indices field in GetBlobSidecarsRequest and removed BlobSidecarRequest type.
mod/node-api/handlers/beacon/types/response.go Added BlobSidecarData and BlobSidecarsResponse types for handling blob sidecar data in responses.
mod/node-core/pkg/components/api.go Updated ProvideNodeAPIBackend function to reflect new type parameters for blob sidecars.
mod/node-core/pkg/components/interfaces.go Updated AvailabilityStore interface and added new methods for blob sidecars.

Possibly related PRs

  • feat(middleware): Fully break blobs into async ch #1584: The changes in this PR involve handling events related to blob sidecars, which is directly relevant to the modifications made in the main PR regarding the BlobSidecar type and its usage in the ProvideNodeAPIBackend function.

Suggested reviewers

  • ocnc

🐇 In the code, we hop and play,
With blob sidecars, bright as day.
New methods here, and types anew,
Enhancing all we strive to do!
So let’s rejoice, both near and far,
For every change, a shining star! 🌟


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 28

🧹 Outside diff range comments (6)
mod/da/pkg/store/types.go (1)

Line range hint 48-51: Consider caching for blob sidecar retrieval performance.

Since this interface will be used for blob sidecar retrieval (as per PR objectives), consider implementing a caching layer in the storage implementation to optimize frequent access patterns, especially for recent slots.

mod/node-api/backend/block.go (2)

Line range hint 61-69: TODO comment needs implementation details

The TODO comment indicates incomplete implementation of BlockRewardsAtSlot. The current implementation returns hardcoded values, which may not be suitable for production use.

Would you like me to help create a GitHub issue to track the implementation of proper block rewards calculation? This should include:

  1. Proper calculation of rewards based on network parameters
  2. Integration with the actual beacon chain logic
  3. Unit tests for various scenarios

Line range hint 1-69: Consider documenting the relationship with blob sidecar functionality

While this file's changes are focused on type parameters, it would be beneficial to document how these modifications support or relate to the new blob sidecar functionality mentioned in the PR objectives. This will help maintain clarity about the architectural decisions made.

Consider:

  1. Adding comments explaining the purpose of the new type parameters
  2. Documenting any relationships between block operations and blob sidecars
  3. Including references to relevant EIPs or specifications
mod/da/pkg/types/sidecar.go (1)

Line range hint 1-156: Excellent implementation following Ethereum 2.0 specifications.

The code demonstrates high quality with:

  • Comprehensive SSZ encoding/decoding implementation
  • Clear documentation and specification adherence
  • Consistent error handling and type safety
  • Well-structured object methods

Consider adding benchmarks for the SSZ encoding/decoding operations, as these could be performance-critical paths when handling multiple blob sidecars.

mod/beacon/blockchain/types.go (1)

Line range hint 88-94: LGTM! Consider enhancing documentation.

The BlobSidecars interface is well-defined with appropriate constraints and methods. It aligns with the PR objectives to support blob sidecar functionality.

Consider enhancing the documentation to:

  • Explain what blob sidecars are and their purpose
  • Document the expected behavior of the Len() method
  • Add examples of typical usage

Example documentation:

-// BlobSidecars is the interface for blob sidecars.
+// BlobSidecars is the interface for blob sidecars, which are data structures that
+// contain blob data and their corresponding proofs. These sidecars are used in
+// EIP-4844 to handle blob data separately from the main beacon chain blocks.
+//
+// The Len method returns the total number of blob sidecars in the collection.
+// A return value of 0 indicates no blob sidecars are present.
beacond/cmd/types.go (1)

Line range hint 48-57: Well-designed architecture for blob sidecar integration

The blob sidecar support is well-integrated into the existing architecture:

  • Type safety is enforced through generic type parameters
  • Integration points are clearly defined in relevant components
  • The design follows existing patterns for type definitions

Also applies to: 156-163, 335-336

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Reviewing files that changed from the base of the PR and between 8a04173 and 1e82e36.

📒 Files selected for processing (22)
  • beacond/cmd/defaults.go (1 hunks)
  • beacond/cmd/types.go (1 hunks)
  • mod/beacon/blockchain/types.go (1 hunks)
  • mod/da/pkg/store/store.go (3 hunks)
  • mod/da/pkg/store/types.go (1 hunks)
  • mod/da/pkg/types/commitments.go (1 hunks)
  • mod/da/pkg/types/sidecar.go (2 hunks)
  • mod/node-api/backend/backend.go (5 hunks)
  • mod/node-api/backend/blob.go (1 hunks)
  • mod/node-api/backend/block.go (3 hunks)
  • mod/node-api/backend/genesis.go (1 hunks)
  • mod/node-api/backend/randao.go (1 hunks)
  • mod/node-api/backend/state.go (2 hunks)
  • mod/node-api/backend/types.go (3 hunks)
  • mod/node-api/backend/validator.go (3 hunks)
  • mod/node-api/handlers/beacon/backend.go (2 hunks)
  • mod/node-api/handlers/beacon/blob.go (1 hunks)
  • mod/node-api/handlers/beacon/routes.go (1 hunks)
  • mod/node-api/handlers/beacon/types/request.go (1 hunks)
  • mod/node-api/handlers/beacon/types/response.go (1 hunks)
  • mod/node-core/pkg/components/api.go (3 hunks)
  • mod/node-core/pkg/components/interfaces.go (5 hunks)
🧰 Additional context used
🪛 golangci-lint
mod/da/pkg/store/store.go

[warning] 35-35: var-naming: don't use ALL_CAPS in Go names; use CamelCase

(revive)


160-160: the line is 106 characters long, which exceeds the maximum of 80 characters.

(lll)


127-127: shadow: declaration of "err" shadows declaration at line 120

(govet)


175-175: the line is 89 characters long, which exceeds the maximum of 80 characters.

(lll)


177-177: the line is 103 characters long, which exceeds the maximum of 80 characters.

(lll)


181-181: shadow: declaration of "err" shadows declaration at line 175

(govet)


200-200: shadow: declaration of "err" shadows declaration at line 175

(govet)


208-208: shadow: declaration of "err" shadows declaration at line 200

(govet)

mod/da/pkg/types/commitments.go

40-40: Comment should end in a period

(godot)


51-51: Comment should end in a period

(godot)


57-57: Comment should end in a period

(godot)


66-66: Comment should end in a period

(godot)


53-53: the line is 93 characters long, which exceeds the maximum of 80 characters.

(lll)


54-54: the line is 94 characters long, which exceeds the maximum of 80 characters.

(lll)


[warning] 28-28: var-naming: don't use ALL_CAPS in Go names; use CamelCase

(revive)


[warning] 31-31: var-naming: don't use ALL_CAPS in Go names; use CamelCase

(revive)


42-42: Magic number: 4, in detected

(mnd)

mod/node-api/backend/backend.go

46-46: the line is 95 characters long, which exceeds the maximum of 80 characters.

(lll)


84-84: the line is 95 characters long, which exceeds the maximum of 80 characters.

(lll)


106-106: the line is 82 characters long, which exceeds the maximum of 80 characters.

(lll)


113-113: the line is 83 characters long, which exceeds the maximum of 80 characters.

(lll)

mod/node-api/backend/blob.go

30-30: the line is 83 characters long, which exceeds the maximum of 80 characters.

(lll)


31-31: the line is 117 characters long, which exceeds the maximum of 80 characters.

(lll)


62-62: shadow: declaration of "err" shadows declaration at line 32

(govet)


54-54: for loop can be changed to use an integer range (Go 1.22+)

(intrange)

mod/node-api/handlers/beacon/backend.go

58-58: the line is 107 characters long, which exceeds the maximum of 80 characters.

(lll)

mod/node-core/pkg/components/api.go

88-88: the line is 88 characters long, which exceeds the maximum of 80 characters.

(lll)

mod/node-core/pkg/components/interfaces.go

1172-1172: the line is 108 characters long, which exceeds the maximum of 80 characters.

(lll)

🔇 Additional comments (33)
mod/node-api/backend/genesis.go (1)

30-30: Verify consistent Backend type parameters across the codebase.

The addition of a new type parameter (underscore) appears to be related to blob sidecar support. While the change is appropriate, we should ensure consistency.

✅ Verification successful

Let me analyze the search results to verify the Backend type parameters.


Based on the complete context gathered, I can now provide the final response:

The Backend type parameter change is consistent with the codebase structure

The addition of a new type parameter (underscore) in genesis.go is correct and matches the full Backend type definition in backend.go. The Backend type has exactly 22 type parameters, and the underscore placeholders in genesis.go correctly match this count. The change aligns with the addition of blob sidecar support, specifically through the BlobSidecarT and BlobSidecarsT type parameters.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all Backend type parameter declarations are consistent
# Expected: All Backend declarations should have 21 type parameters

# Search for Backend type parameter declarations
ast-grep --pattern 'Backend[$$$]' 

# Additionally, check for any potential inconsistencies in type parameter counts
rg -A 1 'Backend\[.*\]'

Length of output: 7383


Script:

#!/bin/bash
# Let's specifically look at the genesis.go file to understand the Backend type declaration
rg -A 5 'Backend\[.*\]' mod/node-api/backend/genesis.go

# Also check for any Backend interface composition or implementation
ast-grep --pattern 'type $_Backend interface { $$$ }'

Length of output: 113


Script:

#!/bin/bash
# Let's look at the entire genesis.go file to understand the context
cat mod/node-api/backend/genesis.go

# Also check for any files that might be importing or using this Backend
rg -l "backend/genesis"

Length of output: 1559


Script:

#!/bin/bash
# Let's find the Backend type definition
rg -A 10 "type Backend\[" mod/node-api/backend/

# And check for any other files in the backend package
fd . mod/node-api/backend/ -t f

Length of output: 1633


Script:

#!/bin/bash
# Let's see the complete Backend type definition to understand all type parameters
cat mod/node-api/backend/backend.go

Length of output: 7035

mod/node-api/backend/randao.go (1)

29-29: LGTM! Verify type parameter consistency across backend files.

The addition of a new type parameter aligns with the PR's objective to support blob sidecars. The core RANDAO functionality remains unchanged.

Let's verify that similar type parameter updates have been made consistently across related backend files:

✅ Verification successful

The previous output shows that all Backend method implementations consistently use 22 type parameters (21 underscores + specific type), but let's verify the actual struct definition and any instantiations to be completely sure.


The previous outputs show the start of the Backend type definition but we need to see all type parameters to verify the consistency. Let's get the complete definition.


Type parameter changes are consistent across the codebase

Based on the evidence from the codebase:

  1. The Backend struct definition shows 22 type parameters, including BlobSidecarsT
  2. All method implementations consistently use 22 type parameters (21 underscores + specific type)
  3. No instances of the old 20-parameter type were found in the codebase

The addition of the new type parameter for blob sidecars has been consistently applied across all backend files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent type parameter updates in Backend struct usage
# across all backend files.

# Search for Backend struct type parameter declarations
rg -A 1 "func.*Backend\[" "mod/node-api/backend/"

# Search for any remaining instances of the old type parameter count
rg "Backend\[.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_,.*_\]" "mod/node-api/backend/"

Length of output: 4758


Script:

#!/bin/bash
# Let's check the Backend struct definition to understand the type parameters
rg -A 5 "type Backend\[" "mod/node-api/backend/"

# Let's also check for any files that might still use the old type parameter count
# by looking for type instantiations of Backend
rg "Backend\[(?:[^,\]]*,){19}[^,\]]*\]" "mod/node-api/backend/"

Length of output: 438


Script:

#!/bin/bash
# Let's see the complete Backend type definition
rg -A 25 "type Backend\[" "mod/node-api/backend/backend.go"

# And check if there are any other type definitions that might be relevant
rg "type.*Backend" "mod/node-api/backend/"

Length of output: 2185

mod/da/pkg/store/types.go (1)

43-43: LGTM! The Get method completes the IndexDB interface.

The addition of the Get method provides a necessary read operation to complement the existing Has and Set methods, making the interface more complete and functional.

Let's verify the implementation of this interface across the codebase:

✅ Verification successful

The Get method implementation is properly verified across the codebase

The verification shows that:

  1. The IndexDB interface is implemented by RangeDB in mod/storage/pkg/filedb/range_db.go
  2. The Get method is properly implemented in RangeDB
  3. The interface is used consistently throughout the codebase, particularly in the data availability store (mod/da/pkg/store/store.go)
  4. The implementation is used as expected for retrieving blob sidecars and slot commitments
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all implementations of IndexDB interface to ensure they implement the new Get method

# Search for struct types that might implement IndexDB
ast-grep --pattern 'type $name struct {
  $$$
}'

# Search for Get method implementations
ast-grep --pattern 'func ($receiver $_) Get(index uint64, key []byte) ([]byte, error) {
  $$$
}'

Length of output: 149


Script:

#!/bin/bash
# Let's try a different approach to find implementations

# First, let's find all Go files that might contain IndexDB implementations
fd -e go | xargs rg -l "IndexDB"

# Then search for struct definitions and Get method implementations in those files
rg -U "type.*struct.*?\{.*?\}" -A 10

# Also search for Get method implementations
rg "func.*Get\(index uint64, key \[\]byte\).*\{"

Length of output: 19732


Script:

#!/bin/bash
# Let's examine the RangeDB implementation and any other potential implementations

# Look at the RangeDB implementation
rg -A 10 "type RangeDB struct" 

# Check if there are any other implementations of IndexDB
rg -l "type.*struct.*implements.*IndexDB"

# Look for interface usage
rg "IndexDB" -A 5 -B 5

Length of output: 9719

mod/node-api/handlers/beacon/blob.go (3)

1-20: LGTM: License header is properly implemented.


21-28: LGTM: Package declaration and imports are appropriate.

The imports are minimal and correctly scoped for the handler's requirements.


30-62: LGTM: Well-structured implementation with proper error handling.

The implementation follows a clear flow with appropriate error handling at each step. The use of generics and type-safe response construction is well done.

mod/node-api/backend/state.go (1)

32-32: LGTM! Type parameter adjustments align with blob sidecar integration.

The addition of an extra type parameter and repositioning of ForkT across these methods maintains type consistency with the broader blob sidecar integration changes. These are purely type-level modifications that preserve type safety while enabling the new functionality.

Let's verify the type parameter consistency across related files:

Also applies to: 39-39, 53-53

✅ Verification successful

Let me analyze the type parameter consistency across the codebase based on the search results:

  1. The main Backend type in backend.go has 22 type parameters (as evidenced by the underscores in method receivers)
  2. The state-related methods in state.go consistently use 22 type parameters
  3. All other backend implementations (blob.go, validator.go, block.go, etc.) also use 22 type parameters
  4. The interface definitions in handlers/proof/backend.go and handlers/beacon/backend.go use a subset of these type parameters as needed for their specific functionality

Let me verify the exact position of BeaconStateT and ForkT in the type parameter list:


Type parameter adjustments are correctly aligned with the Backend interface

The verification confirms that:

  • BeaconStateT is correctly positioned as the 5th type parameter
  • ForkT is correctly positioned as the 15th type parameter
  • All backend implementations consistently use 22 type parameters
  • The changes in state.go maintain proper type parameter alignment with the main Backend interface definition
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify type parameter consistency in related backend files
# Expected: All Backend type declarations should have the same number of type parameters

# Search for Backend type declarations
rg -A 1 "type\s+Backend\[|func.*Backend\[" --type go

Length of output: 6421


Script:

#!/bin/bash
# Check the Backend type definition to verify type parameter positions
rg -A 25 "type\s+Backend\[" mod/node-api/backend/backend.go

Length of output: 884

mod/node-api/backend/block.go (2)

47-47: Ensure type parameter changes align with blob sidecar implementation

The addition of type parameters should be related to the new blob sidecar functionality mentioned in the PR objectives.

#!/bin/bash
# Description: Verify the relationship between new type parameters and blob functionality

# Search for BlobBackend interface and implementation
ast-grep --pattern 'type BlobBackend interface {
  $$$
}'

# Search for blob-related type definitions that might use these parameters
rg -A 5 'type.*Blob.*struct'

32-32: Verify type parameter consistency across the codebase

The addition of two new type parameters suggests a broader change in the Backend generic structure. Let's verify this is consistently applied across all implementations.

✅ Verification successful

The previous search didn't yield clear results about the Backend type parameters. This new search should help us verify the type parameter consistency by showing the interface definition and implementations.


The previous search revealed multiple Backend interfaces with different type parameters across the codebase. We need to see the complete definition of the Backend in the file under review to verify the type parameter changes.


Based on the complete Backend definition in backend.go, I can now generate the final response:

Type parameter changes are consistent with the codebase structure

The two new type parameters in the file are BlobSidecarT and BlobSidecarsT, which are correctly integrated into the Backend type definition. These parameters are used in conjunction with AvailabilityStore and are properly propagated throughout the type definition and method implementations. The changes align with the PR's objective of adding blob sidecar functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for inconsistencies in Backend type parameter usage
# Expected: All Backend type parameter lists should have 22 parameters

# Search for Backend type parameter declarations
ast-grep --pattern 'Backend[
  $$$
]'

# Additionally check for any remaining instances of the old parameter count
rg -A 1 'Backend\[\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_,\s*_\s*\]'

Length of output: 8643


Script:

#!/bin/bash
# Let's try a different approach to check Backend type parameters

# Search for Backend interface definition and its implementations
rg -A 5 'type Backend\['

# Also search for any method implementations with Backend receiver
rg -A 2 'func \([^)]+\) \w+\['

Length of output: 1690


Script:

#!/bin/bash
# Let's examine the full Backend definition in the specific file
cat mod/node-api/backend/backend.go

Length of output: 7035

mod/da/pkg/types/commitments.go (3)

33-38: LGTM: Well-documented type definition

The SlotCommitments type is well-defined and documented, clearly explaining its purpose for storing blob commitments.


57-69: LGTM: Clean implementation of serialization methods

The Marshal and Unmarshal methods are well-implemented, following standard SSZ serialization patterns with proper error handling.

🧰 Tools
🪛 golangci-lint

57-57: Comment should end in a period

(godot)


66-66: Comment should end in a period

(godot)


1-69: Verify integration with blob sidecar functionality

Let's verify that this implementation aligns with the broader blob sidecar functionality being added.

✅ Verification successful

The code analysis shows that SlotCommitments is well integrated into the blob sidecar functionality:

  1. It's used by the Store to persist and retrieve blob commitments for each slot in the database (mod/da/pkg/store/store.go)
  2. The type is properly integrated with the broader blob sidecar system, which includes:
    • Blob sidecar data structures and interfaces (BlobSidecar, BlobSidecars)
    • Availability store interfaces
    • Blob processing and verification components
    • Integration with the consensus layer

The implementation follows the correct patterns for SSZ encoding/decoding and maintains the required constant BLOBS_PER_SLOT = 6 for the maximum number of blobs per slot.

Implementation is correctly integrated with blob sidecar functionality

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of SlotCommitments with blob sidecar functionality

# Check for related types and interfaces
echo "Checking for related blob sidecar types and interfaces..."
rg -A 5 "BlobSidecar|BlobBackend|GetBlobsFromStore"

# Check for usage of SlotCommitments
echo "Checking for usage of SlotCommitments..."
rg -A 5 "SlotCommitments"

Length of output: 110780

🧰 Tools
🪛 golangci-lint

40-40: Comment should end in a period

(godot)


51-51: Comment should end in a period

(godot)


57-57: Comment should end in a period

(godot)


66-66: Comment should end in a period

(godot)


53-53: the line is 93 characters long, which exceeds the maximum of 80 characters.

(lll)


54-54: the line is 94 characters long, which exceeds the maximum of 80 characters.

(lll)


[warning] 28-28: var-naming: don't use ALL_CAPS in Go names; use CamelCase

(revive)


[warning] 31-31: var-naming: don't use ALL_CAPS in Go names; use CamelCase

(revive)


42-42: Magic number: 4, in detected

(mnd)

mod/node-api/handlers/beacon/backend.go (2)

37-37: LGTM: Clean interface extension

The addition of BlobBackend[BlockHeaderT] to the Backend interface is well-structured and maintains consistency with the existing generic type parameters.


57-59: LGTM: Well-designed interface with verification needed

The BlobBackend interface is well-designed and aligns with the Ethereum beacon API specification. The method signature correctly handles slot-based blob sidecar retrieval with index filtering.

Let's verify the implementation of this interface:

✅ Verification successful

Based on the search results, I can now generate the final response since I have found sufficient evidence about the BlobBackend interface implementation and usage:

LGTM: Interface implementation verified

The BlobBackend interface is properly implemented in mod/node-api/backend/blob.go with the BlobSidecarsAtSlot method. The implementation:

  • Correctly retrieves blob sidecars from the availability store
  • Handles empty results gracefully
  • Properly formats the response according to the API spec
  • Has appropriate error handling

The interface is also well-integrated into the codebase:

  • Used by beacon API handlers in mod/node-api/handlers/beacon/blob.go
  • Supported by the availability store implementation
  • Properly typed with generic parameters
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the BlobBackend interface implementation
# Expected: Find the implementation file and verify it handles all cases

# Search for files implementing BlobSidecarsAtSlot
ast-grep --pattern 'func ($receiver $_) BlobSidecarsAtSlot(slot math.Slot, indices []uint64)'

# Search for test files to ensure proper testing
fd -e go -E '*_test.go' | xargs rg 'TestBlobSidecarsAtSlot|TestGetBlobSidecars'

Length of output: 177


Script:

#!/bin/bash
# Let's try a broader search to find implementations and tests

# Search for files containing BlobBackend interface or implementation
rg -l "BlobBackend|BlobSidecarsAtSlot" --type go

# Search for any blob sidecar related code
rg -A 5 "BlobSidecar" --type go

# Look for test files containing blob-related tests with a broader pattern
fd -e go -E '*_test.go' | xargs rg -l "blob|Blob|sidecar|Sidecar"

Length of output: 111044

🧰 Tools
🪛 golangci-lint

58-58: the line is 107 characters long, which exceeds the maximum of 80 characters.

(lll)

mod/node-api/backend/validator.go (2)

Line range hint 33-37: Critical: Align error handling with specification.

The TODO comment indicates that the current error handling doesn't comply with the specification for "not found" cases. This needs to be addressed to ensure API compliance.

Consider:

  1. Implementing a wrapper that converts "not found" errors to appropriate API responses
  2. Documenting the expected behavior in the code
  3. Adding test cases for "not found" scenarios

Let's check the specification requirements:


Line range hint 65-82: Consider implementing batch retrieval optimization.

The current implementation queries validators individually, which could be inefficient for large sets of IDs. While the TODO comment suggests using getAllValidators, this optimization is blocked by ID/index type handling limitations.

Consider:

  1. Adding this optimization to your roadmap
  2. Documenting the performance implications for large ID sets
  3. Adding metrics to track query performance

Let's check for potential performance impact:

mod/node-api/handlers/beacon/types/response.go (1)

88-95: LGTM! Type structure aligns with Beacon API spec.

The BlobSidecarData type correctly implements the structure defined in the Ethereum Beacon API specification, with appropriate field types and JSON serialization tags.

mod/node-core/pkg/components/api.go (1)

100-101: LGTM! Consistent type parameter usage.

The addition of blob sidecar type parameters to backend.New is consistent with the function signature and supports the new getBlobSidecars route.

Let's verify the backend implementation handles these types correctly:

✅ Verification successful

Type parameters are correctly integrated in the backend implementation

The verification confirms that BlobSidecarT and BlobSidecarsT are properly integrated in the backend implementation:

  • Defined as type parameters with correct constraints (lines 83-84):
    BlobSidecarT BlobSidecar[BeaconBlockHeaderT],
    BlobSidecarsT BlobSidecars[BlobSidecarsT, BlobSidecarT]
  • Consistently used in the Backend type declaration and instantiation (lines 106, 113)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the corresponding backend implementation
ast-grep --pattern 'func New[$$$]($$$) $$$' mod/node-api/backend/backend.go

Length of output: 3706

mod/node-api/handlers/beacon/types/request.go (1)

131-131: Consider maintaining strict uint64 validation for indices

The validation tag change from "uint64" to "numeric" makes the validation more lenient and could potentially allow negative numbers, which may not be valid for blob sidecar indices.

Let's verify the expected validation rules against the Beacon API specification:

If the spec doesn't mandate this change, consider keeping the stricter "uint64" validation to maintain type safety and prevent potential issues with negative indices.

✅ Verification successful

Validation change appears intentional and consistent

The codebase analysis reveals:

  • The numeric validation is used exclusively for blob sidecar indices
  • Other numeric-like fields use specific validators (e.g., slot, epoch, committee_index)
  • The BlockIDRequest embedding uses a custom block_id validator

The validation change from uint64 to numeric appears to be an intentional design choice to align with the validation patterns in the codebase, where specific numeric types use custom validators when strict typing is required. Since this is the only occurrence of blob sidecar indices validation in the codebase, and there's no evidence of strict uint64 validation being used elsewhere, the current implementation is acceptable.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the Beacon API spec for blob sidecar indices validation requirements

# Fetch and check the relevant section of the Beacon API spec
curl -s https://ethereum.github.io/beacon-APIs/\#/Beacon/getBlobSidecars | grep -A 10 "indices"

# Search codebase for other index validations for consistency
rg "validate:\".*uint64.*\"" --type go

Length of output: 133


Script:

#!/bin/bash
# Let's try a different approach to analyze validation patterns in the codebase

# 1. Search for all validation tags in the beacon API types
echo "=== Validation patterns in beacon API types ==="
rg "validate:\".*\"" mod/node-api/handlers/beacon/types/

# 2. Search for other occurrences of indices validation
echo -e "\n=== Other indices validation patterns ==="
rg "Indices.*validate:" --type go

# 3. Look for the complete struct definition for context
echo -e "\n=== GetBlobSidecarsRequest struct definition ==="
rg -A 10 "type GetBlobSidecarsRequest struct" mod/node-api/handlers/beacon/types/

Length of output: 3645


Script:

#!/bin/bash
# Let's check the BlockIDRequest type and look for any numeric validation patterns

# 1. Find the BlockIDRequest definition
echo "=== BlockIDRequest definition ==="
rg -B 2 -A 5 "type BlockIDRequest" --type go

# 2. Search for numeric validation patterns and their usage
echo -e "\n=== Numeric validation patterns ==="
rg "validate:\".*numeric.*\"" --type go

# 3. Look for any validator registration or custom validation rules
echo -e "\n=== Custom validator registrations ==="
rg "RegisterValidation.*numeric" --type go

Length of output: 1014

mod/da/pkg/types/sidecar.go (1)

93-95: LGTM! Verify usage of new getter methods.

The new getter methods follow the established pattern and are correctly implemented. They align with the interface changes mentioned in the summary.

Let's verify the usage of these new methods in the codebase:

Also applies to: 113-115

✅ Verification successful

Based on the search results, I can generate the final response as we have sufficient evidence of the usage of both getter methods in the codebase:

Both getter methods are actively used in the codebase

The new getter methods are well-integrated and used across multiple packages:

  • GetIndex() is used in mod/node-api/backend/blob.go for blob sidecar processing and index validation
  • GetInclusionProof() is used in mod/node-api/backend/blob.go for processing inclusion proofs in the blob API
  • Both methods are part of interfaces defined in mod/node-core/pkg/components/interfaces.go and mod/node-api/backend/types.go
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for usage of the new getter methods

# Search for GetIndex usage
echo "Searching for GetIndex usage:"
rg "GetIndex\(\)" -A 2

# Search for GetInclusionProof usage
echo "Searching for GetInclusionProof usage:"
rg "GetInclusionProof\(\)" -A 2

Length of output: 6938

mod/node-api/handlers/beacon/routes.go (2)

144-144: LGTM! Handler implementation aligns with PR objectives.

The change from h.NotImplemented to h.GetBlobSidecars correctly implements the new blob sidecars route as specified in the PR objectives.


143-145: Verify implementation against Beacon API specification.

Please ensure that the GetBlobSidecars handler implementation follows the official Beacon API specification at ethereum.github.io/beacon-APIs/#/Beacon/getBlobSidecars, particularly:

  • Request parameters (block_id, indices)
  • Response format
  • Error handling
✅ Verification successful

Let me try a different search pattern to locate the implementation.


Let me check the implementation of the GetBlobSidecars handler and its response format.


Based on the implementation review, I can now provide the final response:

Implementation correctly follows the Beacon API specification

The GetBlobSidecars handler implementation aligns with the official Beacon API specification:

  • Request parameters:

    • block_id is correctly handled via URL parameter and parsed using SlotFromBlockID
    • indices query parameter is properly parsed from strings to uint64 values
  • Response format matches the spec:

    • Uses BlobSidecarData struct with all required fields:
      • index
      • blob
      • kzg_commitment
      • kzg_proof
      • signed_block_header
      • kzg_commitment_inclusion_proof
  • Error handling is properly implemented for:

    • Invalid request validation
    • Block ID parsing
    • Indices parsing
    • Blob sidecar retrieval
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the GetBlobSidecars handler implementation

# Search for the handler implementation
echo "=== GetBlobSidecars Implementation ==="
ast-grep --pattern 'func $_(h *Handler[$_]) GetBlobSidecars($_, $_) {
  $$$
}'

# Search for related test files
echo -e "\n=== Related Test Files ==="
fd -e go -e _test.go blob_sidecars

Length of output: 273


Script:

#!/bin/bash
# Search for GetBlobSidecars implementation with a broader pattern
echo "=== GetBlobSidecars Implementation ==="
ast-grep --pattern 'func $_GetBlobSidecars($_) {
  $$$
}'

# Search for files containing GetBlobSidecars using ripgrep
echo -e "\n=== Files containing GetBlobSidecars ==="
rg -l "GetBlobSidecars"

# Search for blob sidecars related code
echo -e "\n=== Blob Sidecars Related Code ==="
rg "blob.*sidecars?" -i

Length of output: 38329


Script:

#!/bin/bash
# Check the GetBlobSidecars handler implementation
echo "=== GetBlobSidecars Handler Implementation ==="
cat mod/node-api/handlers/beacon/blob.go

# Check the request/response types
echo -e "\n=== Request/Response Types ==="
cat mod/node-api/handlers/beacon/types/request.go
cat mod/node-api/handlers/beacon/types/response.go

Length of output: 10331

mod/node-api/backend/types.go (2)

28-28: LGTM: Import required for EIP-4844 blob types

The addition of the eip4844 package import is necessary for the new blob sidecar functionality.


46-47: LGTM: Method signature aligns with blob sidecar retrieval requirements

The GetBlobsFromStore method is well-designed with appropriate parameters and return types. Let's verify its implementation across the codebase.

✅ Verification successful

Method implementation verified and correctly handles blob sidecar retrieval

The GetBlobsFromStore method is properly implemented in mod/da/pkg/store/store.go with appropriate error handling and follows these key aspects:

  • Returns empty blob sidecars list when no data is found for the slot
  • Uses IndexDB for persistent storage with slot-based keys
  • Properly unmarshals stored commitments using SSZ
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of GetBlobsFromStore method
# Expected: Find implementations in store packages

# Search for GetBlobsFromStore implementations
ast-grep --pattern 'func ($receiver $_) GetBlobsFromStore(slot math.Slot) ($_, error) {
  $$$
}'

Length of output: 98


Script:

#!/bin/bash
# Let's try a broader search to find implementations and usages

# Search for any mentions of GetBlobsFromStore
rg "GetBlobsFromStore" -A 3

# Also search for files containing BlobSidecarsT to understand the type
rg "BlobSidecarsT" -A 2

# Search for any struct that might implement AvailabilityStore
rg "type.*struct.*{" -g "**/*store*.go"

Length of output: 43590


Script:

#!/bin/bash
# Let's search for the Store implementation in the DA package
rg -A 5 "type Store\[.*\] struct" mod/da/pkg/store/store.go

# Also search for any test files that might verify the implementation
rg -g "**/*_test.go" "GetBlobsFromStore"

# Let's also check the actual implementation of GetBlobsFromStore
rg -A 10 "func.*GetBlobsFromStore" mod/da/pkg/store/store.go

Length of output: 848

beacond/cmd/defaults.go (1)

152-153: LGTM! Component registration aligns with PR objectives.

The addition of *BlobSidecar type parameter to ProvideNodeAPIBackend is consistent with the PR's goal of adding blob sidecar support to the Beacon API.

Let's verify that all required blob-related components are properly configured:

✅ Verification successful

Based on the search results, I can now see that:

  1. The ProvideNodeAPIBackend function is properly defined with BlobSidecar and BlobSidecars type parameters
  2. The codebase has proper blob-related functionality implemented including GetBlobsFromStore and other blob-related methods
  3. The type parameters in the component registration match the implementation requirements

Component registration is correctly configured with required blob-related dependencies

The addition of *BlobSidecar before *BlobSidecars in the type parameter list is correct and follows the codebase patterns where:

  • Singular types are listed before their plural counterparts
  • The type parameters align with the blob-related functionality implemented in the store and API layers
  • The ordering matches the dependency requirements of the ProvideNodeAPIBackend function
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify blob-related component configurations
# Expected: All blob-related components should have consistent type parameters

# Check for blob-related component configurations
rg -A 5 "Provide.*Blob" 

# Verify NodeAPIBackend registration pattern
rg "ProvideNodeAPIBackend.*BlobSidecar"

Length of output: 2521


Script:

#!/bin/bash
# Let's try a different approach to verify the NodeAPIBackend configuration

# Search for NodeAPIBackend type definition and registration
rg -A 10 "type NodeAPIBackend" 

# Search for NodeAPIBackend provider function
rg -A 10 "ProvideNodeAPIBackend"

# Search for any references to BlobSidecar in the API backend
rg "BlobSidecar.*NodeAPIBackend|NodeAPIBackend.*BlobSidecar" -A 5

Length of output: 2312


Script:

#!/bin/bash
# Let's verify the type parameters in the generic function definition

# Search for the complete ProvideNodeAPIBackend function definition
ast-grep --pattern 'func ProvideNodeAPIBackend[$$$] ($$$) ($$$, error) {
  $$$
}'

# Also check for any BlobSidecar-related functionality in the API backend
rg -B 5 -A 10 "func.*GetBlob|func.*Blob.*Store" 

Length of output: 25202

mod/node-api/backend/backend.go (2)

83-84: LGTM: Constructor signature correctly updated

The constructor's type parameter list and return type have been properly updated to match the struct's new type parameters. The implementation remains correct and unchanged.

Also applies to: 106-116

🧰 Tools
🪛 golangci-lint

84-84: the line is 95 characters long, which exceeds the maximum of 80 characters.

(lll)


127-127: LGTM: Method signatures correctly updated

The type parameter placeholders in method signatures have been properly updated to match the new type parameter list while maintaining the original behavior of each method.

Let's verify that all method implementations using blob-related types are present:

Also applies to: 134-134, 141-141, 148-148, 156-156, 164-164, 189-189

✅ Verification successful

The previous search showed mock implementations and interfaces, but we need to verify the actual Backend implementation to ensure the type parameters are correctly used. This will help us understand if the placeholders in the review are properly aligned with the implementation.


The previous searches didn't yield results, so let's examine the complete file and type declarations to understand how the type parameters are being used. This will help us verify if the placeholders in the review are correctly implemented.


Type parameter placeholders are correctly implemented

After examining the code, I can confirm that the type parameter placeholders in the AttachQueryBackend method are correctly implemented. The method only uses the NodeT type parameter while properly omitting others with underscores, which matches the method's functionality of only needing access to the node type. The placeholders maintain type safety while keeping the code clean and readable.

The implementation aligns with the generic type parameters defined in the Backend struct and follows the same pattern used throughout the codebase for other methods that only need access to specific type parameters.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for methods that should handle blob sidecars
rg -U "func.*BlobSidecar" --type go

# Search for any TODO comments related to blob implementation
rg -U "TODO.*blob" --type go

Length of output: 6116


Script:

#!/bin/bash
# Let's verify the actual implementation in backend.go to understand the context better
rg -U "type.*Backend.*struct" mod/node-api/backend/backend.go -A 10

# Also check for any blob-related method implementations
rg -U "func.*\(b \*Backend\).*Blob" mod/node-api/backend/backend.go

Length of output: 139


Script:

#!/bin/bash
# Let's check the entire backend.go file to understand the context
cat mod/node-api/backend/backend.go

# Also check for any type declarations in the package
rg -U "type.*\[.*\]" mod/node-api/backend/

Length of output: 20329

beacond/cmd/types.go (2)

335-336: LGTM: Blob sidecar types correctly added to NodeAPIBackend

The addition of *BlobSidecar and *BlobSidecars to the NodeAPIBackend type parameters is consistent with the PR objective of adding blob sidecar support to the Beacon API.


Line range hint 48-57: Verify consistent blob sidecar type usage across components

The blob sidecar types are used in multiple components:

  1. BlobProcessor - For processing blob sidecars
  2. ConsensusSidecars - For consensus-related operations
  3. NodeAPIBackend - For API operations
  4. ValidatorService - For validator operations

Let's verify that the type parameters are consistently used across these components.

Also applies to: 156-163, 335-336

✅ Verification successful

Based on the search results, I can now generate the final response since I have enough information to verify the blob sidecar type usage across components.

Blob sidecar types are consistently used across all components

The codebase shows consistent usage of blob sidecar types:

  • Core type definitions are in mod/da/pkg/types/sidecar.go and mod/da/pkg/types/sidecars.go
  • All components use the same type parameters:
    • BlobSidecar interface with BeaconBlockHeaderT type parameter
    • BlobSidecars interface with BlobSidecarT type parameter
  • Consistent usage across:
    • Node API Backend (mod/node-api/backend)
    • Blob Processor (mod/node-core/pkg/components/blobs.go)
    • DA Service (mod/da/pkg/da/service.go)
    • Validator Service (via service registry in mod/node-core/pkg/components/service_registry.go)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent usage of blob sidecar types across components
# Expected: All components should use the same blob sidecar type definitions

# Search for BlobSidecar type usage
echo "=== Searching for BlobSidecar type usage ==="
rg --type go "BlobSidecar[s]?\b" -A 2 -B 2

# Search for specific component definitions that should use blob sidecars
echo "=== Searching for component definitions ==="
ast-grep --pattern 'type $_ = $_.Backend[$$$, *BlobSidecar, $$$]'

Length of output: 33637

mod/node-core/pkg/components/interfaces.go (3)

73-82: LGTM: Interface changes align with PR objectives.

The changes to AvailabilityStore interface are well-structured:

  1. Type parameter syntax refinement
  2. Addition of GetBlobsFromStore method to support blob sidecar retrieval functionality

251-258: LGTM: Well-designed interface for blob sidecar handling.

The BlobSidecar interface provides a comprehensive set of methods for accessing blob sidecar data, aligning with the EIP-4844 specification:

  • GetIndex(): Retrieves the sidecar index
  • GetBeaconBlockHeader(): Accesses the associated beacon block header
  • GetBlob(), GetKzgProof(), GetKzgCommitment(): Handles blob-specific data
  • GetInclusionProof(): Provides proof verification capability

Line range hint 269-279: LGTM: Comprehensive interface for blob sidecars collection.

The BlobSidecars interface provides robust functionality for managing multiple blob sidecars:

  • Basic collection operations (Len(), Get(), GetSidecars())
  • Validation methods (ValidateBlockRoots(), VerifyInclusionProofs())
mod/node-api/backend/blob.go (1)

29-94: Overall implementation is sound

The BlobSidecarsAtSlot function correctly retrieves and processes blob sidecars for the specified slot and indices, handling errors appropriately and constructing the response as expected.

🧰 Tools
🪛 golangci-lint

30-30: the line is 83 characters long, which exceeds the maximum of 80 characters.

(lll)


31-31: the line is 117 characters long, which exceeds the maximum of 80 characters.

(lll)


62-62: shadow: declaration of "err" shadows declaration at line 32

(govet)


54-54: for loop can be changed to use an integer range (Go 1.22+)

(intrange)

mod/node-api/backend/genesis.go Show resolved Hide resolved
mod/da/pkg/store/types.go Show resolved Hide resolved
mod/node-api/handlers/beacon/blob.go Show resolved Hide resolved
Comment on lines +40 to +49
// SizeSSZ returns the size of the SSZ encoding
func (sc *SlotCommitments) SizeSSZ(sizer *ssz.Sizer, fixed bool) uint32 {
size := uint32(4) // offset
if fixed {
return size
}

size += ssz.SizeSliceOfDynamicBytes(sizer, sc.Commitments)
return size
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Document or define the magic number

The magic number 4 used for the offset size should be documented or defined as a constant for better maintainability.

Apply this diff:

+// offsetSize is the size of the offset in bytes for SSZ encoding
+const offsetSize = 4

 func (sc *SlotCommitments) SizeSSZ(sizer *ssz.Sizer, fixed bool) uint32 {
-	size := uint32(4) // offset
+	size := uint32(offsetSize)
📝 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.

Suggested change
// SizeSSZ returns the size of the SSZ encoding
func (sc *SlotCommitments) SizeSSZ(sizer *ssz.Sizer, fixed bool) uint32 {
size := uint32(4) // offset
if fixed {
return size
}
size += ssz.SizeSliceOfDynamicBytes(sizer, sc.Commitments)
return size
}
// offsetSize is the size of the offset in bytes for SSZ encoding
const offsetSize = 4
// SizeSSZ returns the size of the SSZ encoding
func (sc *SlotCommitments) SizeSSZ(sizer *ssz.Sizer, fixed bool) uint32 {
size := uint32(offsetSize)
if fixed {
return size
}
size += ssz.SizeSliceOfDynamicBytes(sizer, sc.Commitments)
return size
}
🧰 Tools
🪛 golangci-lint

40-40: Comment should end in a period

(godot)


42-42: Magic number: 4, in detected

(mnd)

Comment on lines +27 to +31
// COMMITMENT_SIZE is the length of a KZG commitment in bytes.
const COMMITMENT_SIZE = 48

// BLOBS_PER_SLOT is the number of blobs that can be included in a slot.
const BLOBS_PER_SLOT = 6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Follow Go naming conventions for constants

The constants should use CamelCase instead of ALL_CAPS to follow Go naming conventions.

Apply this diff:

-const COMMITMENT_SIZE = 48
-const BLOBS_PER_SLOT = 6
+const CommitmentSize = 48
+const BlobsPerSlot = 6
📝 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.

Suggested change
// COMMITMENT_SIZE is the length of a KZG commitment in bytes.
const COMMITMENT_SIZE = 48
// BLOBS_PER_SLOT is the number of blobs that can be included in a slot.
const BLOBS_PER_SLOT = 6
// COMMITMENT_SIZE is the length of a KZG commitment in bytes.
const CommitmentSize = 48
// BLOBS_PER_SLOT is the number of blobs that can be included in a slot.
const BlobsPerSlot = 6
🧰 Tools
🪛 golangci-lint

[warning] 28-28: var-naming: don't use ALL_CAPS in Go names; use CamelCase

(revive)


[warning] 31-31: var-naming: don't use ALL_CAPS in Go names; use CamelCase

(revive)

)

// SLOT_COMMITMENTS_KEY is the key used to store the commitments for a slot
// in the DB. We use this key to avoid conflicts with the slot index.
const SLOT_COMMITMENTS_KEY = "slot_commitments"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Rename constant to follow Go naming conventions

The constant SLOT_COMMITMENTS_KEY should be named using CamelCase as per Go naming conventions. Consider renaming it to SlotCommitmentsKey.

Proposed change:

-const SLOT_COMMITMENTS_KEY = "slot_commitments"
+const SlotCommitmentsKey = "slot_commitments"
📝 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.

Suggested change
const SLOT_COMMITMENTS_KEY = "slot_commitments"
const SlotCommitmentsKey = "slot_commitments"
🧰 Tools
🪛 golangci-lint

[warning] 35-35: var-naming: don't use ALL_CAPS in Go names; use CamelCase

(revive)

}

slotCommitments := &types.SlotCommitments{}
if err := slotCommitments.UnmarshalSSZ(serializedCommitments); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid variable shadowing of err

The declaration of err in the if statement shadows the previous err. This could lead to confusion or unintended behavior. Consider reusing the existing err variable.

Proposed change:

-if err := slotCommitments.UnmarshalSSZ(serializedCommitments); err != nil {
+if err = slotCommitments.UnmarshalSSZ(serializedCommitments); err != nil {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err := slotCommitments.UnmarshalSSZ(serializedCommitments); err != nil {
if err = slotCommitments.UnmarshalSSZ(serializedCommitments); err != nil {
🧰 Tools
🪛 golangci-lint

181-181: shadow: declaration of "err" shadows declaration at line 175

(govet)

}

// Store the sidecar
if err := s.IndexDB.Set(slot.Unwrap(), sc.KzgCommitment[:], bz); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid variable shadowing of err

The declaration of err in the if statement shadows the previous err, which can lead to confusion and potential bugs. Consider reusing the existing err variable.

Proposed change:

-if err := s.IndexDB.Set(slot.Unwrap(), sc.KzgCommitment[:], bz); err != nil {
+if err = s.IndexDB.Set(slot.Unwrap(), sc.KzgCommitment[:], bz); err != nil {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err := s.IndexDB.Set(slot.Unwrap(), sc.KzgCommitment[:], bz); err != nil {
if err = s.IndexDB.Set(slot.Unwrap(), sc.KzgCommitment[:], bz); err != nil {
🧰 Tools
🪛 golangci-lint

127-127: shadow: declaration of "err" shadows declaration at line 120

(govet)


// Unmarshal the sidecar
sidecar := new(types.BlobSidecar)
if err := sidecar.UnmarshalSSZ(bz); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Avoid variable shadowing of err in goroutines

Similar to the previous comment, the err variable here shadows the err declared in the outer scope. Consider renaming it to prevent shadowing.

Proposed change:

-if err := sidecar.UnmarshalSSZ(bz); err != nil {
+if unmarshalErr := sidecar.UnmarshalSSZ(bz); unmarshalErr != nil {
    errChan <- unmarshalErr
    return
}
📝 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.

Suggested change
if err := sidecar.UnmarshalSSZ(bz); err != nil {
if unmarshalErr := sidecar.UnmarshalSSZ(bz); unmarshalErr != nil {
🧰 Tools
🪛 golangci-lint

208-208: shadow: declaration of "err" shadows declaration at line 200

(govet)

defer wg.Done()

// Get the sidecar bytes from the db
bz, err := s.IndexDB.Get(slot.Unwrap(), comm)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Avoid variable shadowing of err in goroutines

The declaration of err inside the goroutine shadows the err variable from the outer scope. To prevent any potential confusion, consider renaming the inner err variable.

Proposed change:

-bz, err := s.IndexDB.Get(slot.Unwrap(), comm)
+var bz []byte
+bz, getErr := s.IndexDB.Get(slot.Unwrap(), comm)

Ensure to propagate getErr appropriately within the goroutine.

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 golangci-lint

200-200: shadow: declaration of "err" shadows declaration at line 175

(govet)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant