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(ssz): Move sidecars onto peters library and interface cleanup #1778

Merged
merged 24 commits into from
Jul 25, 2024

Conversation

itsdevbear
Copy link
Contributor

@itsdevbear itsdevbear commented Jul 25, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a new dependency for improved SSZ functionality in the project.
    • Enhanced BlobSidecar and BeaconBlockHeader types with methods for improved serialization and deserialization.
    • Added methods to the BlobSidecars type for flexible SSZ encoding and decoding.
    • Modified middleware to better handle dynamic blob sidecar structures.
  • Bug Fixes

    • Updated control flow in related logic to ensure correct instantiation of structures.
  • Tests

    • Refactored tests to accommodate changes in serialization methods, enhancing readability and maintainability.

@itsdevbear itsdevbear requested a review from ocnc as a code owner July 25, 2024 16:30
Copy link
Contributor

coderabbitai bot commented Jul 25, 2024

Warning

Rate limit exceeded

@itsdevbear has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 47 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between aa9b1fa and 0839400.

Walkthrough

The changes introduce significant enhancements to the beacond project, focusing on improving serialization and deserialization of data structures related to Ethereum 2.0. Key modifications include updating dependencies, enriching types with new SSZ encoding capabilities, and refactoring interface constraints for greater flexibility. These updates enhance clarity, performance, and maintainability, aligning the code more closely with Ethereum specifications and best practices.

Changes

Files Change Summary
go.mod Added dependency on github.com/itsdevbear/ssz, replacing github.com/karalabe/ssz with a specific version.
mod/da/pkg/types/sidecar.go, mod/da/pkg/types/sidecars.go Enhanced BlobSidecar and BeaconBlockHeader with new SSZ methods for serialization; removed max size tag in BlobSidecars.
mod/runtime/pkg/middleware/middleware.go, mod/runtime/pkg/middleware/types.go Updated type constraints for interfaces to support dynamic SSZ marshaling and unmarshaling.
mod/primitives/pkg/constraints/encoding.go Restructured SSZ serialization interfaces, creating distinct marshaler and unmarshaller interfaces.
mod/consensus-types/pkg/types/v2/header.go Introduced BeaconBlockHeader structure with serialization methods for enhanced functionality.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant BlobSidecar
    participant SSZCodec

    Client->>BlobSidecar: Create BlobSidecar
    BlobSidecar->>SSZCodec: DefineSSZ()
    BlobSidecar->>SSZCodec: MarshalSSZ()
    SSZCodec-->>BlobSidecar: Serialized Data
    BlobSidecar-->>Client: Return BlobSidecar
Loading

🐰 In a code-filled burrow, hopping with glee,
New methods and changes as bright as can be!
With SSZ in hand, our data's so spry,
Refactoring flows as the bytes dance and fly.
Hooray for the progress, let’s munch on some greens,
In this world of code, we’re crafting new dreams! 🌟


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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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: 0

Outside diff range comments (1)
mod/da/pkg/types/sidecar.go (1)

Line range hint 89-99:
Reminder: Implement KZG inclusion proof depth calculation.

The TODO comment indicates that the KZG inclusion proof depth calculation needs to be implemented.

Do you want me to generate the code for the KZG inclusion proof depth calculation or open a GitHub issue to track this task?

Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between 201979a and 485f069.

Files selected for processing (5)
  • beacond/go.mod (1 hunks)
  • mod/da/pkg/types/sidecar.go (5 hunks)
  • mod/da/pkg/types/sidecar_test.go (8 hunks)
  • mod/da/pkg/types/sidecars.go (2 hunks)
  • mod/da/pkg/types/sidecars_test.go (8 hunks)
Additional comments not posted (24)
mod/da/pkg/types/sidecars_test.go (2)

Line range hint 36-52:
LGTM!

The use of BuildBlobSidecar simplifies the creation of BlobSidecar instances and improves readability.


Line range hint 87-109:
LGTM!

The use of BuildBlobSidecar simplifies the creation of BlobSidecar instances and improves readability.

mod/da/pkg/types/sidecars.go (6)

94-97: LGTM!

The DefineSSZ method correctly defines the SSZ encoding for the BlobSidecars object.


100-105: LGTM!

The SizeSSZ method correctly calculates the size of the BlobSidecars object in SSZ encoding.


108-121: LGTM!

The MarshalSSZ method correctly marshals the BlobSidecars object to SSZ format and includes useful debug logging.


124-129: LGTM!

The UnmarshalSSZ method correctly unmarshals the BlobSidecars object from SSZ format and includes useful debug logging.


132-134: LGTM!

The MarshalSSZTo method correctly marshals the BlobSidecars object to the provided buffer in SSZ format.


137-139: LGTM!

The HashTreeRoot method correctly returns the hash tree root of the BlobSidecars object.

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

Line range hint 41-57:
LGTM!

The use of BuildBlobSidecar simplifies the creation of BlobSidecar instances and improves readability.


87-115: LGTM!

The use of BuildBlobSidecar simplifies the creation of BlobSidecar instances and improves readability.


Line range hint 139-157:
LGTM!

The use of BuildBlobSidecar simplifies the creation of BlobSidecar instances and improves readability.

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

68-70: LGTM!

The changes to instantiate BeaconBlockHeader within BuildBlobSidecar are correct.


103-110: LGTM!

The DefineSSZ method correctly defines the SSZ encoding for the BlobSidecar object.


113-120: LGTM!

The SizeSSZ method correctly calculates the size of the BlobSidecar object in SSZ encoding.


123-126: LGTM!

The MarshalSSZ method correctly marshals the BlobSidecar object to SSZ format.


129-134: LGTM!

The UnmarshalSSZ method correctly unmarshals the BlobSidecar object from SSZ format.


137-139: LGTM!

The MarshalSSZTo method correctly marshals the BlobSidecar object to the provided buffer in SSZ format.


142-144: LGTM!

The HashTreeRoot method correctly computes the SSZ hash tree root of the BlobSidecar object.


151-153: LGTM!

The SizeSSZ method correctly calculates the size of the BeaconBlockHeader object in SSZ encoding.


156-165: LGTM!

The DefineSSZ method correctly defines the SSZ encoding for the BeaconBlockHeader object.


173-176: LGTM!

The MarshalSSZ method correctly marshals the BeaconBlockHeader object to SSZ format.


179-181: LGTM!

The UnmarshalSSZ method correctly unmarshals the BeaconBlockHeader object from SSZ format.


184-186: LGTM!

The HashTreeRoot method correctly computes the SSZ hash tree root of the BeaconBlockHeader object.

beacond/go.mod (1)

14-14: LGTM!

The dependency replacement from github.com/karalabe/ssz to github.com/itsdevbear/ssz is correctly specified.

Copy link

codecov bot commented Jul 25, 2024

Codecov Report

Attention: Patch coverage is 33.94495% with 72 lines in your changes missing coverage. Please review.

Project coverage is 22.51%. Comparing base (201979a) to head (0839400).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1778      +/-   ##
==========================================
+ Coverage   22.37%   22.51%   +0.13%     
==========================================
  Files         316      317       +1     
  Lines       13020    13118      +98     
  Branches       27       27              
==========================================
+ Hits         2913     2953      +40     
- Misses      10012    10070      +58     
  Partials       95       95              
Files Coverage Δ
mod/storage/pkg/beacondb/index/validator.go 0.00% <ø> (ø)
mod/da/pkg/blob/processor.go 0.00% <0.00%> (ø)
mod/da/pkg/types/sidecar.go 91.80% <94.87%> (+34.66%) ⬆️
mod/runtime/pkg/encoding/encoding.go 0.00% <0.00%> (ø)
mod/da/pkg/types/sidecars.go 24.48% <0.00%> (-10.81%) ⬇️
mod/consensus-types/pkg/types/v2/header.go 0.00% <0.00%> (ø)

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: 0

Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between 485f069 and 611417b.

Files selected for processing (10)
  • mod/beacon/blockchain/types.go (3 hunks)
  • mod/da/pkg/types/internal/temporary.go (1 hunks)
  • mod/da/pkg/types/sidecar.go (5 hunks)
  • mod/da/pkg/types/sidecars.go (2 hunks)
  • mod/primitives/pkg/constraints/encoding.go (2 hunks)
  • mod/runtime/pkg/encoding/encoding.go (2 hunks)
  • mod/runtime/pkg/middleware/middleware.go (2 hunks)
  • mod/runtime/pkg/middleware/types.go (1 hunks)
  • mod/runtime/pkg/p2p/noop.go (1 hunks)
  • mod/runtime/pkg/p2p/noop_blob.go (1 hunks)
Additional comments not posted (51)
mod/runtime/pkg/p2p/noop.go (3)

34-35: LGTM!

The change to the type parameter ensures that NoopGossipHandler can handle both marshaling and unmarshaling.


Line range hint 39-42:
LGTM!

The Publish function correctly uses the MarshalSSZ method, aligning with the new type constraint.


Line range hint 45-63:
LGTM!

The Request function correctly uses the UnmarshalSSZ method and handles instance creation appropriately.

mod/runtime/pkg/p2p/noop_blob.go (4)

33-34: LGTM!

The change to the type parameter ensures that NoopBlobHandler can handle both marshaling and unmarshaling.


Line range hint 39-43:
LGTM!

The NewNoopBlobHandler function correctly uses the updated type parameter, ensuring consistency.


Line range hint 46-50:
LGTM!

The Publish function correctly uses the MarshalSSZ method, aligning with the new type constraint.


Line range hint 53-59:
LGTM!

The Request function correctly uses the UnmarshalBlobSidecarsFromABCIRequest method, aligning with the new type constraint.

mod/runtime/pkg/middleware/types.go (1)

51-54: LGTM!

The change to the type parameter ensures a more flexible or dynamic implementation of the BlobSidecarsT type.

mod/primitives/pkg/constraints/encoding.go (7)

23-25: LGTM!

The SSZMarshaler interface is well-defined and straightforward, focusing on marshaling data into byte slices.


27-29: LGTM!

The SSZUnmarshaler interface is well-defined and straightforward, focusing on unmarshaling data from byte slices.


31-33: LGTM!

The SSZRootable interface is well-defined and straightforward, focusing on obtaining a hash tree root.


35-38: LGTM!

The SSZMarshalerUnmarshaler interface correctly combines both marshaling and unmarshaling capabilities.


40-44: LGTM!

The SSZMarshalerUnmarshalerRootable interface correctly extends SSZMarshalerUnmarshaler by including SSZRootable functionality.


69-78: LGTM!

The SSZMarshallableStatic interface is well-defined, focusing on marshaling, unmarshaling, and obtaining the size and hash tree root of an object.


80-89: LGTM!

The SSZMarshallableDynamic interface is well-defined, focusing on marshaling, unmarshaling, and obtaining the size and hash tree root of an object, with a dynamic size calculation.

mod/runtime/pkg/encoding/encoding.go (2)

33-33: LGTM!

The change to the type parameter for BlobSidecarsT from constraints.SSZMarshallable to constraints.SSZUnmarshaler enhances the function's flexibility in handling incoming data formats.


107-111: LGTM!

The change to the type parameter for T to BlobSidecarsT defined as constraints.SSZUnmarshaler simplifies the return type and standardizes the type used across both functions.

mod/da/pkg/types/internal/temporary.go (9)

30-32: LGTM!

The BeaconBlockHeader struct correctly embeds types.BeaconBlockHeader.


34-39: LGTM!

The SizeSSZ method correctly returns the size of the BeaconBlockHeader object in SSZ encoding.


41-51: LGTM!

The DefineSSZ method correctly defines the SSZ encoding for the BeaconBlockHeader object.


53-56: LGTM!

The MarshalSSZTo method correctly marshals the BeaconBlockHeader object to SSZ format.


58-62: LGTM!

The MarshalSSZ method correctly marshals the BeaconBlockHeader object to SSZ format.


64-67: LGTM!

The UnmarshalSSZ method correctly unmarshals the BeaconBlockHeader object from SSZ format.


69-72: LGTM!

The HashTreeRoot method correctly computes the SSZ hash tree root of the BeaconBlockHeader object.


74-77: LGTM!

The GetSlot method correctly retrieves the slot of the BeaconBlockHeader.


79-82: LGTM!

The GetProposerIndex method correctly retrieves the proposer index of the BeaconBlockHeader.

mod/da/pkg/types/sidecars.go (7)

92-96: Verify the necessity of the hardcoded value 6 in SSZ encoding.

The DefineSSZ method uses the hardcoded value 6, which might be a remnant of the removed ssz-max:"6" tag. Ensure that this value is still necessary and correct.


98-104: LGTM!

The SizeSSZ method correctly calculates the size of the BlobSidecars object in SSZ encoding.


106-115: LGTM!

The MarshalSSZ method correctly marshals the BlobSidecars object to SSZ format and handles errors appropriately.


117-120: LGTM!

The UnmarshalSSZ method correctly unmarshals the BlobSidecars object from SSZ format and handles errors appropriately.


122-126: LGTM!

The MarshalSSZTo method correctly marshals the BlobSidecars object to the provided buffer in SSZ format and handles errors appropriately.


128-131: LGTM!

The HashTreeRoot method correctly computes the hash tree root of the BlobSidecars object using ssz.HashSequential.


33-33: LGTM!

The removal of the ssz-max:"6" tag from the Sidecars field allows for more dynamic management of the slice.

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

102-111: Verify the necessity of the hardcoded value 8 in SSZ encoding.

The DefineSSZ method uses the hardcoded value 8 for the depth of the InclusionProof field. Ensure that this value is correct and necessary.


113-121: LGTM!

The SizeSSZ method correctly calculates the size of the BlobSidecar object in SSZ encoding.


123-127: LGTM!

The MarshalSSZ method correctly marshals the BlobSidecar object to SSZ format and handles errors appropriately.


129-135: LGTM!

The UnmarshalSSZ method correctly unmarshals the BlobSidecar object from SSZ format and handles errors appropriately.


137-141: LGTM!

The MarshalSSZTo method correctly marshals the BlobSidecar object to the provided buffer in SSZ format and handles errors appropriately.


143-145: LGTM!

The HashTreeRoot method correctly computes the hash tree root of the BlobSidecar object using ssz.HashSequential.


Line range hint 147-153:
LGTM!

The DefineSSZ method correctly defines the SSZ encoding for the BeaconBlockHeader object.


Line range hint 155-161:
LGTM!

The SizeSSZ method correctly calculates the size of the BeaconBlockHeader object in SSZ encoding.


Line range hint 163-167:
LGTM!

The MarshalSSZ method correctly marshals the BeaconBlockHeader object to SSZ format and handles errors appropriately.


Line range hint 169-173:
LGTM!

The UnmarshalSSZ method correctly unmarshals the BeaconBlockHeader object from SSZ format and handles errors appropriately.


Line range hint 175-177:
LGTM!

The HashTreeRoot method correctly computes the hash tree root of the BeaconBlockHeader object using ssz.HashSequential.


62-71: LGTM!

The BuildBlobSidecar function correctly instantiates the new BeaconBlockHeader type and initializes all fields appropriately.

mod/runtime/pkg/middleware/middleware.go (2)

43-43: LGTM!

The change in the type declaration for the BlobSidecarsT generic parameter enhances the middleware's capability to manage dynamic data structures or varying sizes of blob sidecars.


104-104: LGTM!

The change in the type declaration for the BlobSidecarsT generic parameter enhances the middleware's capability to manage dynamic data structures or varying sizes of blob sidecars.

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

51-51: Approved: Enhanced serialization and deserialization capabilities.

The change from constraints.SSZMarshallable to constraints.SSZMarshalerUnmarshalerRootable improves the interface by adding unmarshaling and rootable capabilities, which are beneficial for cryptographic operations or proofs.


65-65: Approved: Enhanced serialization and deserialization capabilities.

The change from constraints.SSZMarshallable to constraints.SSZMarshalerUnmarshalerRootable improves the interface by adding unmarshaling and rootable capabilities, which are beneficial for cryptographic operations or proofs.


74-74: Approved: Enhanced serialization and deserialization capabilities.

The change from constraints.SSZMarshallable to constraints.SSZMarshalerUnmarshalerRootable improves the interface by adding unmarshaling and rootable capabilities, which are beneficial for cryptographic operations or proofs.


81-81: Approved: Enhanced serialization and deserialization capabilities.

The change from constraints.SSZMarshallable to constraints.SSZMarshalerUnmarshaler improves the interface by adding unmarshaling capabilities, which are beneficial for more complex data handling.

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: 1

Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between 611417b and aa9b1fa.

Files ignored due to path filters (1)
  • mod/da/go.sum is excluded by !**/*.sum
Files selected for processing (8)
  • mod/consensus-types/pkg/types/v2/header.go (1 hunks)
  • mod/da/go.mod (4 hunks)
  • mod/da/pkg/blob/processor.go (2 hunks)
  • mod/da/pkg/types/sidecar.go (4 hunks)
  • mod/da/pkg/types/sidecars.go (2 hunks)
  • mod/primitives/pkg/constraints/encoding.go (2 hunks)
  • mod/runtime/pkg/middleware/middleware.go (2 hunks)
  • mod/runtime/pkg/middleware/types.go (1 hunks)
Additional comments not posted (38)
mod/runtime/pkg/middleware/types.go (1)

51-51: Approved: Enhanced type constraint for BlobSidecarsT.

The change from constraints.SSZMarshallable to constraints.SSZMarshalerUnmarshaler improves the interface by supporting both marshaling and unmarshaling.

Ensure that all implementations of BlockchainService are updated to match this new constraint.

mod/primitives/pkg/constraints/encoding.go (6)

23-25: Approved: New SSZMarshaler interface.

The SSZMarshaler interface is correctly defined for marshaling data.


27-29: Approved: New SSZUnmarshaler interface.

The SSZUnmarshaler interface is correctly defined for unmarshaling data.


31-33: Approved: New SSZRootable interface.

The SSZRootable interface is correctly defined for obtaining a hash tree root.


35-38: Approved: New SSZMarshalerUnmarshaler interface.

The SSZMarshalerUnmarshaler interface correctly combines the SSZMarshaler and SSZUnmarshaler interfaces.


40-44: Approved: New SSZMarshalerUnmarshalerRootable interface.

The SSZMarshalerUnmarshalerRootable interface correctly combines the SSZMarshaler, SSZUnmarshaler, and SSZRootable interfaces.


58-61: Approved: Renamed SSZMarshallable interface.

The SSZMarshallable interface has been correctly renamed to SSZMarshaler and retains its functionality.

mod/da/pkg/types/sidecars.go (6)

33-33: Approved: Removed ssz-max:"6" tag.

The removal of the ssz-max:"6" tag allows for more flexible handling of the Sidecars slice.


92-96: Approved: New DefineSSZ method.

The DefineSSZ method correctly defines the SSZ encoding for the BlobSidecars object.


98-104: Approved: New SizeSSZ method.

The SizeSSZ method correctly computes the size of the BlobSidecars object in SSZ encoding.


106-110: Approved: New MarshalSSZ method.

The MarshalSSZ method correctly serializes the BlobSidecars object into SSZ format.


112-116: Approved: New MarshalSSZTo method.

The MarshalSSZTo method correctly marshals the BlobSidecars object into the provided buffer in SSZ format.


118-121: Approved: New UnmarshalSSZ method.

The UnmarshalSSZ method correctly deserializes the BlobSidecars object from SSZ format.

mod/da/pkg/blob/processor.go (2)

117-119: LGTM! But verify the method usage in the codebase.

The change to pass the Slot value directly instead of converting it is approved. This simplifies the code and may improve performance.

However, ensure that all calls to ProcessSidecars are compatible with this change.


93-95: LGTM! But verify the method usage in the codebase.

The change to pass the Slot value directly instead of converting it is approved. This simplifies the code and may improve performance.

However, ensure that all calls to VerifySidecars are compatible with this change.

Verification successful

Verify the method usage in the codebase.

The change to pass the Slot value directly instead of converting it is approved. This simplifies the code and may improve performance.

  • Ensure that all calls to VerifySidecars are compatible with this change:
    • mod/da/pkg/blob/processor.go
    • mod/da/pkg/blob/verifier.go
    • mod/da/pkg/da/service.go

All calls to VerifySidecars are compatible with the change.

The change to pass the Slot value directly instead of converting it is confirmed to be compatible with all calls to VerifySidecars.

  • mod/da/pkg/blob/processor.go directly passes sidecars.Sidecars[0].BeaconBlockHeader.Slot.
  • mod/da/pkg/blob/verifier.go and mod/da/pkg/da/service.go do not involve the Slot value directly and are unaffected by the change.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all calls to `VerifySidecars` are compatible with the change.

# Test: Search for the method usage. Expect: Only occurrences of the new usage.
rg --type go -A 5 $'VerifySidecars'

Length of output: 3591


Script:

#!/bin/bash
# Description: Verify the specific lines in each file where `VerifySidecars` is called to ensure compatibility with the change.

# Test: Check the usage in `processor.go`
rg --type go -A 5 'VerifySidecars' mod/da/pkg/blob/processor.go

# Test: Check the usage in `verifier.go`
rg --type go -A 5 'VerifySidecars' mod/da/pkg/blob/verifier.go

# Test: Check the usage in `service.go`
rg --type go -A 5 'VerifySidecars' mod/da/pkg/da/service.go

Length of output: 1328

mod/da/go.mod (4)

45-45: Indirect dependency acknowledged: fastssz.

The reintroduction of github.com/ferranbt/fastssz as an indirect dependency is acknowledged. Ensure that this does not cause any conflicts with the new karalabe/ssz dependency.

However, verify that the indirect dependency does not cause any conflicts.


8-8: Dependency update approved: consensus-types.

The update to github.com/berachain/beacon-kit/mod/consensus-types is approved. Ensure that the new version is compatible with the existing codebase.

However, verify that all usages of this dependency are compatible with the new version.


70-70: Indirect dependency acknowledged: go-bitfield.

The addition of github.com/prysmaticlabs/go-bitfield as an indirect dependency is acknowledged. Ensure that this does not cause any conflicts with the existing dependencies.

However, verify that the indirect dependency does not cause any conflicts.

Verification successful

Indirect dependency acknowledged: go-bitfield.

The addition of github.com/prysmaticlabs/go-bitfield as an indirect dependency is acknowledged. The verification process did not reveal any conflicts with existing dependencies.

  • mod/da/go.mod: Contains go-bitfield as an indirect dependency.
  • mod/da/go.sum: Contains the corresponding checksum entries for go-bitfield.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the indirect dependency `go-bitfield` does not cause any conflicts with existing dependencies.

# Test: Search for the dependency usage. Expect: No conflicts.
rg --type go -A 5 $'go-bitfield'

Length of output: 31


Script:

#!/bin/bash
# Description: Verify the presence of `go-bitfield` in go.mod and go.sum and ensure there are no conflicts.

# Search for `go-bitfield` in go.mod
echo "Searching for go-bitfield in go.mod"
rg 'go-bitfield' go.mod

# Search for `go-bitfield` in go.sum
echo "Searching for go-bitfield in go.sum"
rg 'go-bitfield' go.sum

Length of output: 303


Script:

#!/bin/bash
# Description: Locate go.mod and go.sum files and verify the presence of `go-bitfield`.

# Locate go.mod file
echo "Locating go.mod file"
fd go.mod

# Locate go.sum file
echo "Locating go.sum file"
fd go.sum

Length of output: 1097


Script:

#!/bin/bash
# Description: Verify the presence of `go-bitfield` in go.mod and go.sum files located in the previous step.

# Assuming go.mod is located at mod/da/go.mod and go.sum is located at mod/da/go.sum
# Search for `go-bitfield` in go.mod
echo "Searching for go-bitfield in mod/da/go.mod"
rg 'go-bitfield' mod/da/go.mod

# Search for `go-bitfield` in go.sum
echo "Searching for go-bitfield in mod/da/go.sum"
rg 'go-bitfield' mod/da/go.sum

Length of output: 583


16-16: Dependency switch approved: karalabe/ssz.

The switch from github.com/ferranbt/fastssz to github.com/karalabe/ssz is approved. Ensure that the new dependency is compatible with the existing codebase.

However, verify that all usages of serialization and deserialization logic are compatible with the new dependency.

Verification successful

Verify serialization and deserialization logic compatibility with karalabe/ssz.

The new dependency github.com/karalabe/ssz is used in multiple files. To ensure compatibility, we need to verify how serialization and deserialization methods are implemented with this new dependency.


Verify serialization and deserialization logic compatibility with karalabe/ssz.

The new dependency github.com/karalabe/ssz is used in multiple files. To ensure compatibility, we need to verify how serialization and deserialization methods are implemented with this new dependency.


Dependency switch approved: karalabe/ssz.

The switch from github.com/ferranbt/fastssz to github.com/karalabe/ssz has been verified. The new dependency is correctly used for serialization and deserialization in the following files:

  • mod/consensus-types/pkg/types/v2/fork_data.go
  • mod/consensus-types/pkg/types/v2/header.go
  • mod/consensus-types/pkg/types/v2/deposit_message.go
  • mod/consensus-types/pkg/types/v2/signing_data.go
  • mod/da/pkg/types/sidecars.go
  • mod/da/pkg/types/sidecar.go

No compatibility issues were found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all usages of serialization and deserialization logic are compatible with the new dependency.

# Test: Search for the dependency usage. Expect: No compatibility issues.
rg --type go -A 5 $'karalabe/ssz'

Length of output: 2662


Script:

#!/bin/bash
# Description: Extract methods using the `karalabe/ssz` dependency to verify serialization and deserialization logic.

# Searching for functions using `karalabe/ssz` in the identified files.
files=(
  "mod/consensus-types/pkg/types/v2/fork_data.go"
  "mod/consensus-types/pkg/types/v2/header.go"
  "mod/consensus-types/pkg/types/v2/deposit_message.go"
  "mod/consensus-types/pkg/types/v2/signing_data.go"
  "mod/da/pkg/types/sidecars.go"
  "mod/da/pkg/types/sidecar.go"
)

for file in "${files[@]}"; do
  echo "File: $file"
  rg --type go --no-heading --line-number --context 5 'karalabe/ssz' "$file"
done

Length of output: 4185


Script:

#!/bin/bash
# Description: Extract methods using the `karalabe/ssz` dependency to verify serialization and deserialization logic.

# Searching for functions using `karalabe/ssz` in the identified files.
files=(
  "mod/consensus-types/pkg/types/v2/fork_data.go"
  "mod/consensus-types/pkg/types/v2/header.go"
  "mod/consensus-types/pkg/types/v2/deposit_message.go"
  "mod/consensus-types/pkg/types/v2/signing_data.go"
  "mod/da/pkg/types/sidecars.go"
  "mod/da/pkg/types/sidecar.go"
)

for file in "${files[@]}"; do
  echo "File: $file"
  rg --type go --no-heading --line-number --context 5 'ssz' "$file" | grep -E 'func|ssz'
done

Length of output: 6329

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

127-130: New method approved: MarshalSSZ.

The addition of the MarshalSSZ method for SSZ encoding is approved. Ensure that this method is used correctly in the codebase.

However, verify that the method is used correctly.

Verification successful

The MarshalSSZ method is used correctly in the codebase, as evidenced by its usage in various implementation files and the presence of comprehensive tests that validate its functionality. The tests ensure that the method behaves as expected, which confirms its correct usage.

New method approved: MarshalSSZ.

The addition of the MarshalSSZ method for SSZ encoding is approved. The method is used correctly in the codebase, with appropriate tests validating its functionality.

  • mod/da/pkg/types/sidecar.go
  • mod/da/pkg/store/store.go
  • mod/da/pkg/types/sidecar_test.go
  • mod/da/pkg/types/sidecars.go
  • mod/da/pkg/types/sidecars_test.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `MarshalSSZ` method is used correctly in the codebase.

# Test: Search for the method usage. Expect: Correct usage.
rg --type go -A 5 $'MarshalSSZ'

Length of output: 152939


117-124: New method approved: SizeSSZ.

The addition of the SizeSSZ method for SSZ encoding is approved. Ensure that this method is used correctly in the codebase.

However, verify that the method is used correctly.

Verification successful

The SizeSSZ method for BlobSidecar is used correctly within its own file.

  • The method is used to allocate a buffer of the appropriate size for SSZ encoding within mod/da/pkg/types/sidecar.go.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `SizeSSZ` method is used correctly in the codebase.

# Test: Search for the method usage. Expect: Correct usage.
rg --type go -A 5 $'SizeSSZ'

Length of output: 82708


Script:

#!/bin/bash
# Description: Verify the correct usage of the `BlobSidecar.SizeSSZ` method in the codebase.

# Test: Search for the `BlobSidecar` type and its method usage. Expect: Correct usage.
rg --type go -A 5 'BlobSidecar' | rg 'SizeSSZ'

Length of output: 526


105-114: New method approved: DefineSSZ.

The addition of the DefineSSZ method for SSZ encoding is approved. Ensure that this method is used correctly in the codebase.

However, verify that the method is used correctly.


146-149: New method approved: HashTreeRoot.

The addition of the HashTreeRoot method for SSZ encoding is approved. Ensure that this method is used correctly in the codebase.

However, verify that the method is used correctly.


146-149: New method approved: HashTreeRoot.

The addition of the HashTreeRoot method for SSZ encoding is approved. Ensure that this method is used correctly in the codebase.

However, verify that the method is used correctly.


133-138: New method approved: UnmarshalSSZ.

The addition of the UnmarshalSSZ method for SSZ encoding is approved. Ensure that this method is used correctly in the codebase.

However, verify that the method is used correctly.

Verification successful

Verification complete: UnmarshalSSZ method usage

The UnmarshalSSZ method for BlobSidecar is used correctly and is adequately tested in the codebase.

  • Test files: The method is invoked and validated in test files, ensuring its correctness.
  • Non-test files: No issues or incorrect usages were found in the non-test files.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `UnmarshalSSZ` method is used correctly in the codebase.

# Test: Search for the method usage. Expect: Correct usage.
rg --type go -A 5 $'UnmarshalSSZ'

Length of output: 87457


Script:

#!/bin/bash
# Description: Verify the correct usage of the `UnmarshalSSZ` method in the context of `BlobSidecar`.

# Test: Search for the method usage specific to `BlobSidecar`. Expect: Correct usage.
rg --type go -A 5 'BlobSidecar.*UnmarshalSSZ'

Length of output: 583


Script:

#!/bin/bash
# Description: Verify the correct usage of the `UnmarshalSSZ` method by searching for instances of `BlobSidecar`.

# Test: Search for instances of `BlobSidecar` and inspect surrounding code for `UnmarshalSSZ` calls. Expect: Correct usage.
rg --type go -A 10 'BlobSidecar'

Length of output: 113583


127-130: New method approved: MarshalSSZ.

The addition of the MarshalSSZ method for SSZ encoding is approved. Ensure that this method is used correctly in the codebase.

However, verify that the method is used correctly.

Verification successful

The MarshalSSZ method is used correctly in the codebase.

The provided output shows that the method is used in various contexts, including tests and mocks, with appropriate error handling and validation.

  • mod/runtime/pkg/p2p/noop_blob.go
  • mod/runtime/pkg/p2p/noop.go
  • mod/runtime/pkg/p2p/noop_block.go
  • mod/storage/pkg/beacondb/encoding/ssz_value.go
  • mod/storage/pkg/beacondb/index/validator.go
  • mod/node-api/backend/mocks/deposit.mock.go
  • mod/primitives/pkg/math/bool.go
  • mod/primitives/pkg/math/u16.go
  • mod/primitives/pkg/math/u8_test.go
  • mod/primitives/pkg/math/u256_legacy.go
  • mod/primitives/pkg/math/u64_test.go
  • mod/primitives/pkg/math/u32_test.go
  • mod/primitives/pkg/math/u16_test.go
  • mod/primitives/pkg/math/u256.go
  • mod/primitives/pkg/math/u256_legacy_test.go
  • mod/primitives/pkg/encoding/ssz/vector.go
  • mod/primitives/pkg/math/u8.go
  • mod/primitives/pkg/encoding/ssz/list.go
  • mod/primitives/pkg/math/u64.go
  • mod/primitives/pkg/math/u32.go
  • mod/primitives/pkg/encoding/ssz/merkle/merkleizer.go
  • mod/primitives/pkg/encoding/ssz/container.go
  • mod/primitives/pkg/encoding/ssz/byte.go
  • mod/primitives/pkg/math/u256_test.go
  • mod/primitives/pkg/encoding/ssz/schema/id.go
  • mod/primitives/pkg/math/bool_test.go
  • mod/primitives/pkg/encoding/ssz/merkle/merkleizer_test.go
  • mod/primitives/pkg/constraints/encoding.go
  • mod/primitives/pkg/bytes/b256.go
  • mod/primitives/pkg/bytes/b4.go
  • mod/primitives/pkg/bytes/b32_test.go
  • mod/primitives/pkg/bytes/b4_test.go
  • mod/primitives/pkg/bytes/b48_test.go
  • mod/primitives/pkg/bytes/b8_test.go
  • mod/primitives/pkg/bytes/b96_test.go
  • mod/primitives/pkg/bytes/b96.go
  • mod/primitives/pkg/bytes/b8.go
  • mod/primitives/pkg/bytes/b48.go
  • mod/primitives/pkg/bytes/b32.go
  • mod/primitives/pkg/bytes/b20.go
  • mod/consensus-types/pkg/types/block_deneb.ssz.go
  • mod/consensus-types/pkg/types/deposit_test.go
  • mod/consensus-types/pkg/types/header.ssz.go
  • mod/consensus-types/pkg/types/validator_test.go
  • mod/consensus-types/pkg/types/payload_test.go
  • mod/consensus-types/pkg/types/slashing_info.ssz.go
  • mod/consensus-types/pkg/types/validator.ssz.go
  • mod/consensus-types/pkg/types/v2/header.go
  • mod/consensus-types/pkg/types/v2/signing_data_test.go
  • mod/consensus-types/pkg/types/slashing_info_test.go
  • mod/consensus-types/pkg/types/v2/fork_data.go
  • mod/consensus-types/pkg/types/v2/deposit_message.go
  • mod/consensus-types/pkg/types/payload_header.ssz.go
  • mod/consensus-types/pkg/types/fork_test.go
  • mod/consensus-types/pkg/types/payload_header_test.go
  • mod/consensus-types/pkg/types/header_test.go
  • mod/consensus-types/pkg/types/v2/signing_data.go
  • mod/consensus-types/pkg/types/fork.ssz.go
  • mod/consensus-types/pkg/types/v2/deposit_message_test.go
  • mod/consensus-types/pkg/types/v2/fork_data_test.go
  • mod/consensus-types/pkg/types/mocks/beacon_block_body.mock.go
  • mod/consensus-types/pkg/types/mocks/raw_beacon_block_body.mock.go
  • mod/consensus-types/pkg/types/payload.ssz.go
  • mod/consensus-types/pkg/types/mocks/read_only_beacon_block.mock.go
  • mod/consensus-types/pkg/types/mocks/read_only_beacon_block_body.mock.go
  • mod/consensus-types/pkg/types/eth1data.ssz.go
  • mod/consensus-types/pkg/types/deposit.ssz.go
  • mod/consensus-types/pkg/types/body_test.go
  • mod/consensus-types/pkg/types/body_denebplus_test.go
  • mod/consensus-types/pkg/state/deneb/deneb.ssz.go
  • mod/consensus-types/pkg/state/deneb/deneb_test.go
  • mod/consensus-types/pkg/types/mocks/inner_execution_payload.mock.go
  • mod/consensus-types/pkg/types/mocks/execution_payload_body.mock.go
  • mod/consensus-types/pkg/types/block_test.go
  • mod/consensus-types/pkg/types/eth1data_test.go
  • mod/consensus-types/pkg/types/attestation_data.ssz.go
  • mod/consensus-types/pkg/types/mocks/raw_beacon_block.mock.go
  • mod/consensus-types/pkg/state/deneb/deneb_test.go
  • mod/consensus-types/pkg/types/mocks/inner_execution_payload_header.mock.go
  • mod/consensus-types/pkg/types/body_deneb.ssz.go
  • mod/consensus-types/pkg/types/block_denebplus_test.go
  • mod/da/pkg/store/store.go
  • mod/da/pkg/types/sidecars.go
  • mod/da/pkg/types/sidecar_test.go
  • mod/da/pkg/types/sidecar.go
  • mod/engine-primitives/pkg/engine-primitives/withdrawal_ssz_test.go
  • mod/engine-primitives/pkg/engine-primitives/withdrawal.ssz.go
  • mod/consensus-types/pkg/types/attestation_data_test.go
  • mod/da/pkg/types/sidecars_test.go
  • mod/cli/pkg/commands/genesis/types/validators.ssz.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `MarshalSSZ` method is used correctly in the codebase.

# Test: Search for the method usage. Expect: Correct usage.
rg --type go -A 5 $'MarshalSSZ'

Length of output: 152939


25-25: Dependency update approved: typesv2.

The update to use typesv2 for BeaconBlockHeader is approved. Ensure that the new version is compatible with the existing codebase.

However, verify that all usages of this dependency are compatible with the new version.


117-124: New method approved: SizeSSZ.

The addition of the SizeSSZ method for SSZ encoding is approved. Ensure that this method is used correctly in the codebase.

However, verify that the method is used correctly.


133-138: New method approved: UnmarshalSSZ.

The addition of the UnmarshalSSZ method for SSZ encoding is approved. Ensure that this method is used correctly in the codebase.

However, verify that the method is used correctly.

mod/runtime/pkg/middleware/middleware.go (2)

43-43: LGTM!

The update to the BlobSidecarsT type from constraints.SSZMarshallable to constraints.SSZMarshalerUnmarshaler enhances the middleware's flexibility in handling serialization and deserialization.


104-104: LGTM!

The update to the BlobSidecarsT type from constraints.SSZMarshallable to constraints.SSZMarshalerUnmarshaler in the NewABCIMiddleware function aligns with the type declaration change and enhances the middleware's flexibility.

mod/consensus-types/pkg/types/v2/header.go (7)

1-20: LGTM!

The licensing information in the header is standard and correct.


23-28: LGTM!

The import statements are correct and necessary for the implementation.


41-53: LGTM!

The BeaconBlockHeader type declaration is correct and well-structured.


59-74: LGTM!

The NewBeaconBlockHeader function implementation is correct and follows best practices.


76-87: LGTM!

The New function implementation is correct and follows best practices.


93-126: LGTM!

The SSZ-related functions for encoding and decoding the BeaconBlockHeader type are correctly implemented and follow best practices.


132-182: LGTM!

The getter and setter functions for the BeaconBlockHeader type are correctly implemented and follow best practices.

Comment on lines +105 to +114
// DefineSSZ defines the SSZ encoding for the BlobSidecar object.
func (b *BlobSidecar) DefineSSZ(codec *ssz.Codec) {
ssz.DefineUint64(codec, &b.Index)
ssz.DefineStaticBytes(codec, &b.Blob)
ssz.DefineStaticBytes(codec, &b.KzgCommitment)
ssz.DefineStaticBytes(codec, &b.KzgProof)
ssz.DefineStaticObject(codec, &b.BeaconBlockHeader)
//nolint:mnd // depth of 8
ssz.DefineCheckedArrayOfStaticBytes(codec, &b.InclusionProof, 8)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Method DefineSSZ is not being utilized correctly.

The DefineSSZ method for the BlobSidecar structure is defined but not used anywhere in the codebase. Ensure that this method is invoked where necessary to define the SSZ encoding for BlobSidecar objects.

  • File: mod/da/pkg/types/sidecar.go
  • Lines: 105-114
Analysis chain

New method approved: DefineSSZ.

The addition of the DefineSSZ method for SSZ encoding is approved. Ensure that this method is used correctly in the codebase.

However, verify that the method is used correctly.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `DefineSSZ` method is used correctly in the codebase.

# Test: Search for the method usage. Expect: Correct usage.
rg --type go -A 5 $'DefineSSZ'

Length of output: 3605


Script:

#!/bin/bash
# Description: Verify that the `DefineSSZ` method of `BlobSidecar` is used correctly in the codebase.

# Search for instantiations of `BlobSidecar`.
rg --type go 'BlobSidecar' -A 5

# Search for method calls on `BlobSidecar` instances.
rg --type go '\.DefineSSZ' -A 5

Length of output: 78613


Script:

#!/bin/bash
# Description: Verify indirect usage or integration points for the `DefineSSZ` method of `BlobSidecar`.

# Search for methods or functions that handle `BlobSidecar` objects and might call `DefineSSZ`.
rg --type go 'BlobSidecar' -A 10 | grep -E 'func|method'

Length of output: 7386

@itsdevbear itsdevbear enabled auto-merge (squash) July 25, 2024 17:31
@itsdevbear itsdevbear changed the title feat(ssz): Move sidecars onto karalabe feat(ssz): Move sidecars onto peters library and interface cleanup Jul 25, 2024
@itsdevbear itsdevbear merged commit c651574 into main Jul 25, 2024
18 checks passed
@itsdevbear itsdevbear deleted the hack-sidecars-over branch July 25, 2024 17:42
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