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

Cell proof #15064

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

Cell proof #15064

wants to merge 8 commits into from

Conversation

0x00101010
Copy link

@0x00101010 0x00101010 commented Mar 17, 2025

What type of PR is this?

Feature

What does this PR do? Why is it needed?

New cell proof computation related changes as defined in:

  1. Add EIP-7594 (PeerDAS) related changes ethereum/execution-apis#630
  2. Add EIP-7594 (PeerDAS) related changes ethereum/beacon-APIs#516
  3. Add EIP-7594 (PeerDAS) related changes ethereum/builder-specs#117

Which issues(s) does this PR fix?

#14129

Fixes #

Other notes for review

Acknowledgements

@0x00101010 0x00101010 marked this pull request as ready for review March 19, 2025 22:11
@0x00101010 0x00101010 requested a review from a team as a code owner March 19, 2025 22:11
@0x00101010 0x00101010 requested review from nisdas, Inspector-Butters and james-prysm and removed request for a team March 19, 2025 22:11
@@ -689,3 +689,37 @@ func (sigBlock *SignedBeaconBlockFulu) Copy() *SignedBeaconBlockFulu {
Signature: bytesutil.SafeCopyBytes(sigBlock.Signature),
}
}

func (block *BeaconBlockFulu) Copy() *BeaconBlockFulu {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we re-use BeaconBlockElectra?


// 96 byte BLS signature from the validator that produced this block.
bytes signature = 2 [ (ethereum.eth.ext.ssz_size) = "96" ];
}

message BeaconBlockContentsFulu {
BeaconBlockElectra block = 1;
BeaconBlockFulu block = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we re-use BeaconBlockElectra?

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

message BeaconBlockFulu {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we re-use BeaconBlockElectra?

BeaconBlockBodyFulu body = 5;
}

message BeaconBlockBodyFulu {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we re-use BeaconBlockBodyElectra?

}

// Get the block body.
start := time.Now()
Copy link
Contributor

Choose a reason for hiding this comment

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

To be strictly fair this should be the first line of the function.

startTime := time.Now()
blobsCount := len(blobs)
if blobsCount == 0 {
func DataColumnSidecars(signedBlock interfaces.ReadOnlySignedBeaconBlock, cellsAndProofs []kzg.CellsAndProofs) ([]*ethpb.DataColumnSidecar, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The godoc is not correct any more.

"github.com/prysmaticlabs/prysm/v5/runtime/version"
)

// Helper function to unblind data column sidecars from a block and a blobs bundle v2.
Copy link
Contributor

Choose a reason for hiding this comment

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

Godoc of public function should start with the name of the public function itself.

return DataColumnSidecars(block, cellsAndProofs)
}

func ConstructCellsAndProofs(blobs [][]byte, cellProofs [][]byte) ([]kzg.CellsAndProofs, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this function need to be public?

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.

2 participants