Skip to content

Conversation

@0x00101010
Copy link

What type of PR is this?

Feature

What does this PR do? Why is it needed?

commit cell proof computation related proto and generated go files, see ethereum/execution-apis#630

Which issues(s) does this PR fix?

Fixes #14129

Other notes for review

Acknowledgements

@0x00101010 0x00101010 marked this pull request as ready for review February 28, 2025 23:52
@0x00101010 0x00101010 requested a review from a team as a code owner February 28, 2025 23:52
@0x00101010 0x00101010 requested review from potuz, rkapka and terencechain and removed request for a team February 28, 2025 23:52
Comment on lines 247 to 267
message BlobsBundleV2 {
// The KZG commitments of the blobs.
repeated bytes kzg_commitments = 1 [
(ethereum.eth.ext.ssz_size) = "?,48",
(ethereum.eth.ext.ssz_max) = "max_blob_commitments.size"
];

// The cell_proofs for all blobs
// TODO: update ssz_max when it's defined in the spec
repeated bytes cell_proofs = 2 [
(ethereum.eth.ext.ssz_size) = "?,48",
(ethereum.eth.ext.ssz_max) = "max_blob_commitments.size"
];

// The blobs itself.
repeated bytes blobs = 3 [
(ethereum.eth.ext.ssz_size) = "?,blob.size",
(ethereum.eth.ext.ssz_max) = "max_blob_commitments.size"
];
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

In general, it’s best to define new protobuf messages in their own files, such as execution_engine_eip7594.proto or something similar. Keeping everything in a single file increases the risk of conflicts, especially with multiple feature branches in development

Copy link
Author

Choose a reason for hiding this comment

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

Good to know, will update

@0x00101010 0x00101010 force-pushed the cell-proof-computation-proto branch from d95cb37 to d6bbe00 Compare March 3, 2025 23:25
@0x00101010 0x00101010 requested a review from terencechain March 3, 2025 23:32
Copy link
Collaborator

@terencechain terencechain left a comment

Choose a reason for hiding this comment

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

Lgtm. Thanks for considering my feedback. Will let @nalepae approve and merge this one

…le does not exists.

Rerun ` hack/update-go-pbs.sh` and `hack/update-go-ssz.sh `.
@nalepae nalepae merged commit 0b5064b into OffchainLabs:peerDAS Mar 4, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants