From c811fadf338fd672b019292c5e60fff8e2c75bca Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Wed, 15 Oct 2025 14:18:04 +0200 Subject: [PATCH] `VerifyDataColumnSidecar`: Check if there is no too many commitments. (#15859) * `VerifyDataColumnSidecar`: Check if there is no too many commitments. * `TestVerifyDataColumnSidecar`: Refactor using test cases. * Add changelog. --- beacon-chain/core/peerdas/p2p_interface.go | 7 ++ .../core/peerdas/p2p_interface_test.go | 64 +++++++++++-------- .../sync/data_column_sidecars_test.go | 7 ++ .../sync/initial-sync/service_test.go | 1 + beacon-chain/verification/data_column_test.go | 9 ++- changelog/manu-check-commitment-count.md | 2 + 6 files changed, 61 insertions(+), 29 deletions(-) create mode 100644 changelog/manu-check-commitment-count.md diff --git a/beacon-chain/core/peerdas/p2p_interface.go b/beacon-chain/core/peerdas/p2p_interface.go index 8295d3d8c9..46b49fdd9f 100644 --- a/beacon-chain/core/peerdas/p2p_interface.go +++ b/beacon-chain/core/peerdas/p2p_interface.go @@ -43,6 +43,13 @@ func VerifyDataColumnSidecar(sidecar blocks.RODataColumn) error { return ErrNoKzgCommitments } + // A sidecar with more commitments than the max blob count for this block is invalid. + slot := sidecar.Slot() + maxBlobsPerBlock := params.BeaconConfig().MaxBlobsPerBlock(slot) + if len(sidecar.KzgCommitments) > maxBlobsPerBlock { + return ErrTooManyCommitments + } + // The column length must be equal to the number of commitments/proofs. if len(sidecar.Column) != len(sidecar.KzgCommitments) || len(sidecar.Column) != len(sidecar.KzgProofs) { return ErrMismatchLength diff --git a/beacon-chain/core/peerdas/p2p_interface_test.go b/beacon-chain/core/peerdas/p2p_interface_test.go index f02a5becb4..3b4c20e35c 100644 --- a/beacon-chain/core/peerdas/p2p_interface_test.go +++ b/beacon-chain/core/peerdas/p2p_interface_test.go @@ -18,38 +18,46 @@ import ( ) func TestVerifyDataColumnSidecar(t *testing.T) { - t.Run("index too large", func(t *testing.T) { - roSidecar := createTestSidecar(t, 1_000_000, nil, nil, nil) - err := peerdas.VerifyDataColumnSidecar(roSidecar) - require.ErrorIs(t, err, peerdas.ErrIndexTooLarge) - }) + testCases := []struct { + name string + index uint64 + blobCount int + commitmentCount int + proofCount int + maxBlobsPerBlock uint64 + expectedError error + }{ + {name: "index too large", index: 1_000_000, expectedError: peerdas.ErrIndexTooLarge}, + {name: "no commitments", expectedError: peerdas.ErrNoKzgCommitments}, + {name: "too many commitments", blobCount: 10, commitmentCount: 10, proofCount: 10, maxBlobsPerBlock: 2, expectedError: peerdas.ErrTooManyCommitments}, + {name: "commitments size mismatch", commitmentCount: 1, maxBlobsPerBlock: 1, expectedError: peerdas.ErrMismatchLength}, + {name: "proofs size mismatch", blobCount: 1, commitmentCount: 1, maxBlobsPerBlock: 1, expectedError: peerdas.ErrMismatchLength}, + {name: "nominal", blobCount: 1, commitmentCount: 1, proofCount: 1, maxBlobsPerBlock: 1, expectedError: nil}, + } - t.Run("no commitments", func(t *testing.T) { - roSidecar := createTestSidecar(t, 0, nil, nil, nil) - err := peerdas.VerifyDataColumnSidecar(roSidecar) - require.ErrorIs(t, err, peerdas.ErrNoKzgCommitments) - }) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + params.SetupTestConfigCleanup(t) + cfg := params.BeaconConfig() + cfg.FuluForkEpoch = 0 + cfg.BlobSchedule = []params.BlobScheduleEntry{{Epoch: 0, MaxBlobsPerBlock: tc.maxBlobsPerBlock}} + params.OverrideBeaconConfig(cfg) - t.Run("KZG commitments size mismatch", func(t *testing.T) { - kzgCommitments := make([][]byte, 1) - roSidecar := createTestSidecar(t, 0, nil, kzgCommitments, nil) - err := peerdas.VerifyDataColumnSidecar(roSidecar) - require.ErrorIs(t, err, peerdas.ErrMismatchLength) - }) + column := make([][]byte, tc.blobCount) + kzgCommitments := make([][]byte, tc.commitmentCount) + kzgProof := make([][]byte, tc.proofCount) - t.Run("KZG proofs size mismatch", func(t *testing.T) { - column, kzgCommitments := make([][]byte, 1), make([][]byte, 1) - roSidecar := createTestSidecar(t, 0, column, kzgCommitments, nil) - err := peerdas.VerifyDataColumnSidecar(roSidecar) - require.ErrorIs(t, err, peerdas.ErrMismatchLength) - }) + roSidecar := createTestSidecar(t, tc.index, column, kzgCommitments, kzgProof) + err := peerdas.VerifyDataColumnSidecar(roSidecar) - t.Run("nominal", func(t *testing.T) { - column, kzgCommitments, kzgProofs := make([][]byte, 1), make([][]byte, 1), make([][]byte, 1) - roSidecar := createTestSidecar(t, 0, column, kzgCommitments, kzgProofs) - err := peerdas.VerifyDataColumnSidecar(roSidecar) - require.NoError(t, err) - }) + if tc.expectedError != nil { + require.ErrorIs(t, err, tc.expectedError) + return + } + + require.NoError(t, err) + }) + } } func TestVerifyDataColumnSidecarKZGProofs(t *testing.T) { diff --git a/beacon-chain/sync/data_column_sidecars_test.go b/beacon-chain/sync/data_column_sidecars_test.go index c7e886d348..4c533ccc31 100644 --- a/beacon-chain/sync/data_column_sidecars_test.go +++ b/beacon-chain/sync/data_column_sidecars_test.go @@ -45,6 +45,7 @@ func TestFetchDataColumnSidecars(t *testing.T) { params.SetupTestConfigCleanup(t) cfg := params.BeaconConfig().Copy() cfg.FuluForkEpoch = 0 + cfg.BlobSchedule = []params.BlobScheduleEntry{{Epoch: 0, MaxBlobsPerBlock: 10}} params.OverrideBeaconConfig(cfg) // Start the trusted setup. @@ -760,6 +761,12 @@ func TestVerifyDataColumnSidecarsByPeer(t *testing.T) { err := kzg.Start() require.NoError(t, err) + params.SetupTestConfigCleanup(t) + cfg := params.BeaconConfig() + cfg.FuluForkEpoch = 0 + cfg.BlobSchedule = []params.BlobScheduleEntry{{Epoch: 0, MaxBlobsPerBlock: 2}} + params.OverrideBeaconConfig(cfg) + t.Run("nominal", func(t *testing.T) { const ( start, stop = 0, 15 diff --git a/beacon-chain/sync/initial-sync/service_test.go b/beacon-chain/sync/initial-sync/service_test.go index fc3c06baf1..e2b4bccbb0 100644 --- a/beacon-chain/sync/initial-sync/service_test.go +++ b/beacon-chain/sync/initial-sync/service_test.go @@ -683,6 +683,7 @@ func TestFetchOriginColumns(t *testing.T) { params.SetupTestConfigCleanup(t) cfg := params.BeaconConfig().Copy() cfg.FuluForkEpoch = 0 + cfg.BlobSchedule = []params.BlobScheduleEntry{{Epoch: 0, MaxBlobsPerBlock: 10}} params.OverrideBeaconConfig(cfg) const ( diff --git a/beacon-chain/verification/data_column_test.go b/beacon-chain/verification/data_column_test.go index 8646d195ea..e4a92d41bf 100644 --- a/beacon-chain/verification/data_column_test.go +++ b/beacon-chain/verification/data_column_test.go @@ -58,7 +58,6 @@ func TestValid(t *testing.T) { t.Run("one invalid column", func(t *testing.T) { columns := GenerateTestDataColumns(t, [fieldparams.RootLength]byte{}, 1, 1) - columns[0].KzgCommitments = [][]byte{} verifier := initializer.NewDataColumnsVerifier(columns, GossipDataColumnSidecarRequirements) err := verifier.ValidFields() @@ -67,6 +66,14 @@ func TestValid(t *testing.T) { }) t.Run("nominal", func(t *testing.T) { + const maxBlobsPerBlock = 2 + + params.SetupTestConfigCleanup(t) + cfg := params.BeaconConfig() + cfg.FuluForkEpoch = 0 + cfg.BlobSchedule = []params.BlobScheduleEntry{{Epoch: 0, MaxBlobsPerBlock: maxBlobsPerBlock}} + params.OverrideBeaconConfig(cfg) + columns := GenerateTestDataColumns(t, [fieldparams.RootLength]byte{}, 1, 1) verifier := initializer.NewDataColumnsVerifier(columns, GossipDataColumnSidecarRequirements) diff --git a/changelog/manu-check-commitment-count.md b/changelog/manu-check-commitment-count.md new file mode 100644 index 0000000000..404d96eb8b --- /dev/null +++ b/changelog/manu-check-commitment-count.md @@ -0,0 +1,2 @@ +### Fixed +- `VerifyDataColumnSidecar`: Check if there is no too many commitments.