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: cell proof computation related proto and generated go files #15003

Open
wants to merge 2 commits into
base: peerDAS
Choose a base branch
from

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, terencechain and rkapka and removed request for a team February 28, 2025 23:52
@nalepae nalepae added the peerDAS label Mar 3, 2025
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
Member

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
Member

@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 `.
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