fulu block proposals with datacolumn broadcast #15628

james-prysm · 2025-08-24T02:57:50Z

What type of PR is this?

Feature

What does this PR do? Why is it needed?

adding relevent changes for handling fulu changes in grpc and rest submit block endpoints.

refactors broadcasting to its own functions
updates consensus check for rest api endpoint.

tested via kurtosis with disable get blob cdd44dd

Which issues(s) does this PR fix?

Fixes #

part of #14129

Other notes for review

Acknowledgements

I have read CONTRIBUTING.md.
I have included a uniquely named changelog fragment file.
I have added a description to this PR with sufficient context for reviewers to understand this PR.

…ast for rest api to properly send datasidecars

…max blobs

nalepae

General comments about errors wrapping: https://github.com/uber-go/guide/blob/master/style.md#error-wrapping

To avoid the failed to xxx: failed to yyy: failed to zzz

beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go

beacon-chain/rpc/helpers/broadcast.go

beacon-chain/rpc/eth/beacon/handlers.go

beacon-chain/blockchain/kzg/validation_test.go

beacon-chain/rpc/eth/beacon/handlers.go

beacon-chain/rpc/helpers/broadcast.go

nalepae · 2025-08-27T21:38:13Z

beacon-chain/rpc/helpers/broadcast.go

+}
+
+// BroadcastDataColumnSidecars broadcasts data column sidecars concurrently and optionally receives them.
+func BroadcastDataColumnSidecars(


I think a warning should be added in the godoc of this function because it transforms sidecars into verified sidecars.
Like:
"WARNING: This function should be called only with trusted sidecars, since it will upgrade them as verified without any further check."

Same for the BroadcastBlobSidecars.
Maybe Kasey could have a better idea.

I thought about it i'm going to refactor this and have the helper as part of the broadcaster it makes more sense

Co-authored-by: Manu NALEPA <[email protected]>

…needs to be reused

james-prysm · 2025-08-28T04:38:38Z

beacon-chain/rpc/eth/beacon/handlers.go

 		return s.broadcastSeenBlockSidecars(ctx, b, gb.GetDeneb().Blobs, gb.GetDeneb().KzgProofs)
 	default:
 		// other forks before Deneb do not support blob sidecars
+		// forks after fulu do not support blob sidecars, instead support data columns, no need to rebroadcast


I don't think we need to broadcast datacolumns when the block is equivocated anymore right

But in that case should we broadcast data columns of the equivocated block?

I don't think so but I guess I can ask around, it's not required by the beacon api as far as I can tell

rkapka · 2025-08-28T11:36:17Z

beacon-chain/blockchain/kzg/validation.go

+	"github.com/pkg/errors"
 )

+func bytesToBlob(blob []byte) *GoKZG.Blob {


nitpick: Why do the other functions use a named return value and this one uses a variable?

that one is a pointer the other ones are not

rkapka · 2025-08-28T11:44:55Z

beacon-chain/blockchain/kzg/validation.go

+	for blobIndex, blobData := range blobs {
+		// Convert blob to kzg.Blob type
+		var blob Blob
+		copy(blob[:], blobData)


Can't we avoid a copy here?

rkapka · 2025-08-28T11:45:52Z

beacon-chain/blockchain/kzg/validation.go

+			allIndices = append(allIndices, columnIndex)
+
+			var proof Bytes48
+			copy(proof[:], cellProofs[cellProofIndex])


Can't we avoid a copy here?

beacon-chain/blockchain/kzg/validation.go

rkapka · 2025-08-28T11:50:01Z

beacon-chain/blockchain/kzg/validation.go

+		copy(ckzgBlobs[i][:], blobs[i])
+		copy(ckzgCommitments[i][:], commitments[i])
+		copy(ckzgProofs[i][:], proofs[i])


We never modify any of these objects - is copying necessary?

rkapka · 2025-08-28T11:51:51Z

beacon-chain/rpc/eth/beacon/handlers.go

 		return s.broadcastSeenBlockSidecars(ctx, b, gb.GetDeneb().Blobs, gb.GetDeneb().KzgProofs)
 	default:
 		// other forks before Deneb do not support blob sidecars
+		// forks after fulu do not support blob sidecars, instead support data columns, no need to rebroadcast


But in that case should we broadcast data columns of the equivocated block?

rkapka · 2025-08-28T11:58:35Z

beacon-chain/rpc/eth/beacon/handlers.go

+		// For Fulu blocks, proofs are cell proofs (blobs * numberOfColumns)
+		expectedProofsCount := uint64(len(blobs)) * numberOfColumns
+		if uint64(len(proofs)) != expectedProofsCount || len(blobs) != len(commitments) {
+			return fmt.Errorf("number of blobs (%d), cell proofs (%d), and commitments (%d) do not match (expected %d cell proofs)", len(blobs), len(proofs), len(commitments), expectedProofsCount)


This error is a little misleading because number of blobs (%d), cell proofs (%d), and commitments (%d) do not match sounds like they should all be equal, while in reality there should be more cell proofs than blobs/commitments.

Maybe this should be split into two separate errors?

actually I think this error is probably fine with what you posted , i think people knowing this context will know

beacon-chain/rpc/eth/beacon/handlers.go

rkapka · 2025-08-28T12:04:50Z

beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go

 func (vs *Server) ProposeBeaconBlock(ctx context.Context, req *ethpb.GenericSignedBeaconBlock) (*ethpb.ProposeResponse, error) {
+	var (
+		blobSidecars       []*ethpb.BlobSidecar
+		dataColumnSideCars []*ethpb.DataColumnSidecar


nitpick: SideCars --> Sidecars

beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go

Co-authored-by: Radosław Kapka <[email protected]>

* propose block changes from peerdas branch * breaking out broadcast code into its own helper, changing fulu broadcast for rest api to properly send datasidecars * renamed validate blobsidecars to validate blobs, and added check for max blobs * gofmt * adding in batch verification for blobs" * changelog * adding kzg tests, moving new kzg functions to validation.go * linting and other small fixes * fixing linting issues and adding some proposer tests * missing dependencies * fixing test * fixing more tests * gaz * removed return on broadcast data columns * more cleanup and unit test adjustments * missed removal of unneeded field * adding data column receiver initialization * Update beacon-chain/rpc/eth/beacon/handlers.go Co-authored-by: Manu NALEPA <[email protected]> * partial review feedback from manu * gaz * reverting some code to peerdas as I don't believe the broadcast code needs to be reused * missed removal of build dependency * fixing tests and adding another test based on manu's suggestion * fixing linting * Update beacon-chain/rpc/eth/beacon/handlers.go Co-authored-by: Radosław Kapka <[email protected]> * Update beacon-chain/blockchain/kzg/validation.go Co-authored-by: Radosław Kapka <[email protected]> * radek's review changes * adding missed test --------- Co-authored-by: Manu NALEPA <[email protected]> Co-authored-by: Radosław Kapka <[email protected]>

james-prysm force-pushed the block-proposal-fulu branch 2 times, most recently from fac3a33 to cea9503 Compare August 25, 2025 03:18

james-prysm added api fulu peer-das labels Aug 25, 2025

james-prysm force-pushed the block-proposal-fulu branch 5 times, most recently from 93695ee to 65876ce Compare August 25, 2025 16:35

propose block changes from peerdas branch

99cf1ef

james-prysm force-pushed the block-proposal-fulu branch from 2936798 to 99cf1ef Compare August 25, 2025 16:52

breaking out broadcast code into its own helper, changing fulu broadc…

63c3fb1

…ast for rest api to properly send datasidecars

james-prysm force-pushed the block-proposal-fulu branch from 826da72 to 63c3fb1 Compare August 25, 2025 20:38

james-prysm and others added 7 commits August 25, 2025 15:39

Merge branch 'develop' into block-proposal-fulu

6a18d77

renamed validate blobsidecars to validate blobs, and added check for …

ac13ad5

…max blobs

gofmt

aff6169

adding in batch verification for blobs"

50ad7df

changelog

87da155

adding kzg tests, moving new kzg functions to validation.go

c53fd25

linting and other small fixes

8b993a4

james-prysm mentioned this pull request Aug 26, 2025

BPO validation on block submission #15620

Closed

3 tasks

james-prysm added 8 commits August 25, 2025 22:08

fixing linting issues and adding some proposer tests

a21f153

missing dependencies

f6a7550

fixing test

ceca53d

fixing more tests

0f471c1

gaz

4434e53

removed return on broadcast data columns

c72a137

more cleanup and unit test adjustments

c46ac42

missed removal of unneeded field

adad25f

adding data column receiver initialization

0fd759a

james-prysm force-pushed the block-proposal-fulu branch from 2594828 to 0fd759a Compare August 26, 2025 18:21

Merge branch 'develop' into block-proposal-fulu

07d4389

james-prysm requested a review from nalepae August 26, 2025 19:34

james-prysm marked this pull request as ready for review August 26, 2025 19:38

james-prysm requested a review from rkapka August 26, 2025 19:38

nalepae reviewed Aug 27, 2025

View reviewed changes

james-prysm and others added 6 commits August 27, 2025 20:39

Update beacon-chain/rpc/eth/beacon/handlers.go

e3884cf

Co-authored-by: Manu NALEPA <[email protected]>

partial review feedback from manu

409d8d3

Merge branch 'develop' into block-proposal-fulu

1174c8a

gaz

85dd4e5

reverting some code to peerdas as I don't believe the broadcast code …

ca27f91

…needs to be reused

missed removal of build dependency

d4dcd5f

james-prysm commented Aug 28, 2025

View reviewed changes

james-prysm added 2 commits August 27, 2025 23:44

fixing tests and adding another test based on manu's suggestion

a463332

fixing linting

05fb71c

rkapka reviewed Aug 28, 2025

View reviewed changes

james-prysm and others added 4 commits August 28, 2025 08:25

Update beacon-chain/rpc/eth/beacon/handlers.go

96ad094

Co-authored-by: Radosław Kapka <[email protected]>

Update beacon-chain/blockchain/kzg/validation.go

9d885fd

Co-authored-by: Radosław Kapka <[email protected]>

radek's review changes

6504c6e

adding missed test

8c59358

rkapka approved these changes Aug 28, 2025

View reviewed changes

james-prysm added this pull request to the merge queue Aug 28, 2025

Merged via the queue into develop with commit d339e09 Aug 28, 2025
17 checks passed

james-prysm deleted the block-proposal-fulu branch August 28, 2025 16:37

fulu block proposals with datacolumn broadcast #15628

fulu block proposals with datacolumn broadcast #15628

Uh oh!

Conversation

james-prysm commented Aug 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nalepae left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

james-prysm commented Aug 24, 2025 •

edited

Loading