diff --git a/beacon-chain/sync/validate_data_column_gloas.go b/beacon-chain/sync/validate_data_column_gloas.go index 1e6f5211cf6a..a6f7907df7e1 100644 --- a/beacon-chain/sync/validate_data_column_gloas.go +++ b/beacon-chain/sync/validate_data_column_gloas.go @@ -54,7 +54,9 @@ func (s *Service) validateDataColumnGloas( if msg.Topic == nil || !strings.Contains(*msg.Topic+"/", expectedSubTopic) { return blocks.VerifiedRODataColumn{}, errors.New("gloas data column on wrong subnet") } - s.queuePendingGloasColumn(roDataColumn, pid) + if err := s.queuePendingGloasColumn(roDataColumn, pid); err != nil { + return blocks.VerifiedRODataColumn{}, err + } return blocks.VerifiedRODataColumn{}, ignoreValidation(errors.New("gloas data column block not yet seen")) } @@ -124,14 +126,24 @@ func (s *Service) setSeenDataColumnRootIndex(root [fieldparams.RootLength]byte, s.seenDataColumnCache.Add(slot, key, true) } -func (s *Service) queuePendingGloasColumn(roCol blocks.RODataColumn, pid peer.ID) { +// queuePendingGloasColumn returns a non-nil error for malformed sidecars (the caller propagates it as ValidationReject). +func (s *Service) queuePendingGloasColumn(roCol blocks.RODataColumn, pid peer.ID) error { dc := roCol.DataColumnSidecarGloas() if dc == nil { - return + return errors.New("nil gloas data column sidecar") + } + cells := len(dc.Column) + if cells == 0 || len(dc.KzgProofs) != cells { + return errors.Errorf("gloas data column length mismatch: cells=%d proofs=%d", cells, len(dc.KzgProofs)) + } + cfg := params.BeaconConfig() + currentEpoch := slots.ToEpoch(s.cfg.clock.CurrentSlot()) + if cells > max(cfg.MaxBlobsPerBlockAtEpoch(currentEpoch), cfg.MaxBlobsPerBlockAtEpoch(currentEpoch+1)) { + return errors.Errorf("gloas data column cell count %d exceeds network blob limit", cells) } idx := roCol.Index() if idx >= fieldparams.NumberOfColumns { - return + return errors.Errorf("gloas data column index %d out of range", idx) } root := roCol.BlockRoot() @@ -143,16 +155,17 @@ func (s *Service) queuePendingGloasColumn(roCol blocks.RODataColumn, pid peer.ID entry := s.pendingGloasColumns[root] if entry == nil { if len(s.pendingGloasColumns) >= maxPendingGloasRoots { - return + return nil } entry = &pendingGloasEntry{slot: slot} s.pendingGloasColumns[root] = entry } if entry.columns[idx] != nil { - return + return nil } entry.columns[idx] = &pendingColumnEntry{sidecar: dc, peer: pid} + return nil } func (s *Service) processPendingGloasColumns(root [fieldparams.RootLength]byte, blk interfaces.ReadOnlySignedBeaconBlock) { diff --git a/beacon-chain/sync/validate_data_column_gloas_test.go b/beacon-chain/sync/validate_data_column_gloas_test.go index 4f94bc196c43..e0be4c6b4d48 100644 --- a/beacon-chain/sync/validate_data_column_gloas_test.go +++ b/beacon-chain/sync/validate_data_column_gloas_test.go @@ -112,6 +112,8 @@ func TestValidateDataColumnGloas(t *testing.T) { t.Run("ignores unseen block", func(t *testing.T) { params.SetupTestConfigCleanup(t) cfg := params.BeaconConfig() + cfg.DenebForkEpoch = 0 + cfg.ElectraForkEpoch = 0 cfg.FuluForkEpoch = 0 cfg.GloasForkEpoch = 0 params.OverrideBeaconConfig(cfg) @@ -134,6 +136,8 @@ func TestValidateDataColumnGloas(t *testing.T) { t.Run("validates against bid commitments", func(t *testing.T) { params.SetupTestConfigCleanup(t) cfg := params.BeaconConfig() + cfg.DenebForkEpoch = 0 + cfg.ElectraForkEpoch = 0 cfg.FuluForkEpoch = 0 cfg.GloasForkEpoch = 0 params.OverrideBeaconConfig(cfg) @@ -166,6 +170,8 @@ func TestValidateDataColumnGloas(t *testing.T) { t.Run("rejects slot mismatch", func(t *testing.T) { params.SetupTestConfigCleanup(t) cfg := params.BeaconConfig() + cfg.DenebForkEpoch = 0 + cfg.ElectraForkEpoch = 0 cfg.FuluForkEpoch = 0 cfg.GloasForkEpoch = 0 params.OverrideBeaconConfig(cfg) @@ -197,9 +203,44 @@ func TestValidateDataColumnGloas(t *testing.T) { _, err = service.validateDataColumnGloas(ctx, "aDummyPID", msg, roDataColumn, "/data_column_sidecar_%d/") require.ErrorContains(t, "slot does not match block slot", err) }) + + t.Run("rejects oversize column on queue path", func(t *testing.T) { + params.SetupTestConfigCleanup(t) + cfg := params.BeaconConfig() + cfg.DenebForkEpoch = 0 + cfg.ElectraForkEpoch = 0 + cfg.FuluForkEpoch = 0 + cfg.GloasForkEpoch = 0 + params.OverrideBeaconConfig(cfg) + + sidecar, _ := gloasFixture(t) + maxCells := params.BeaconConfig().MaxBlobCommitmentsPerBlock + sidecar.Column = make([][]byte, maxCells) + sidecar.KzgProofs = make([][]byte, maxCells) + for i := range sidecar.Column { + sidecar.Column[i] = make([]byte, 2048) + sidecar.KzgProofs[i] = make([]byte, 48) + } + + service, message := serviceAndMessage(t, testNewDataColumnSidecarsVerifier(verification.MockDataColumnsVerifier{}), sidecar, sidecar.Index) + result, err := service.validateDataColumn(ctx, "aDummyPID", message) + require.NotNil(t, err) + require.Equal(t, pubsub.ValidationReject, result) + + blockRoot := bytesutil.ToBytes32(sidecar.BeaconBlockRoot) + require.Equal(t, false, service.hasPendingGloasColumns(blockRoot)) + }) } func TestPendingGloasColumns(t *testing.T) { + params.SetupTestConfigCleanup(t) + cfg := params.BeaconConfig() + cfg.DenebForkEpoch = 0 + cfg.ElectraForkEpoch = 0 + cfg.FuluForkEpoch = 0 + cfg.GloasForkEpoch = 0 + params.OverrideBeaconConfig(cfg) + clock := startup.NewClock(time.Now(), [32]byte{}) t.Run("queue and retrieve", func(t *testing.T) { @@ -218,7 +259,7 @@ func TestPendingGloasColumns(t *testing.T) { roCol, err := blocks.NewRODataColumnGloasWithRoot(dc, root) require.NoError(t, err) - s.queuePendingGloasColumn(roCol, "peer1") + require.NoError(t, s.queuePendingGloasColumn(roCol, "peer1")) require.Equal(t, true, s.hasPendingGloasColumns(root)) entry := s.pendingGloasColumns[root] @@ -243,8 +284,8 @@ func TestPendingGloasColumns(t *testing.T) { roCol, err := blocks.NewRODataColumnGloasWithRoot(dc, root) require.NoError(t, err) - s.queuePendingGloasColumn(roCol, "peer1") - s.queuePendingGloasColumn(roCol, "peer2") + require.NoError(t, s.queuePendingGloasColumn(roCol, "peer1")) + require.NoError(t, s.queuePendingGloasColumn(roCol, "peer2")) require.Equal(t, peer.ID("peer1"), s.pendingGloasColumns[root].columns[10].peer) }) @@ -277,7 +318,76 @@ func TestPendingGloasColumns(t *testing.T) { roCol, err := blocks.NewRODataColumnGloasWithRoot(dc, root) require.NoError(t, err) - s.queuePendingGloasColumn(roCol, "peer1") + require.NotNil(t, s.queuePendingGloasColumn(roCol, "peer1")) + require.Equal(t, false, s.hasPendingGloasColumns(root)) + }) + + t.Run("oversize column rejected", func(t *testing.T) { + s := &Service{ + cfg: &config{clock: clock}, + pendingGloasColumns: make(map[[32]byte]*pendingGloasEntry), + } + // SSZ allows 4096 cells; live max is much smaller. Without admission cap, + // this 8 MiB sidecar would sit on the heap until prune. + maxCells := params.BeaconConfig().MaxBlobCommitmentsPerBlock + cells := make([][]byte, maxCells) + proofs := make([][]byte, maxCells) + for i := range cells { + cells[i] = make([]byte, 2048) + proofs[i] = make([]byte, 48) + } + root := [32]byte{0x77} + dc := ðpb.DataColumnSidecarGloas{ + Index: 0, + Slot: clock.CurrentSlot(), + BeaconBlockRoot: root[:], + Column: cells, + KzgProofs: proofs, + } + roCol, err := blocks.NewRODataColumnGloasWithRoot(dc, root) + require.NoError(t, err) + + require.NotNil(t, s.queuePendingGloasColumn(roCol, "peer1")) + require.Equal(t, false, s.hasPendingGloasColumns(root)) + }) + + t.Run("empty column rejected", func(t *testing.T) { + s := &Service{ + cfg: &config{clock: clock}, + pendingGloasColumns: make(map[[32]byte]*pendingGloasEntry), + } + root := [32]byte{0x78} + dc := ðpb.DataColumnSidecarGloas{ + Index: 0, + Slot: clock.CurrentSlot(), + BeaconBlockRoot: root[:], + Column: nil, + KzgProofs: nil, + } + roCol, err := blocks.NewRODataColumnGloasWithRoot(dc, root) + require.NoError(t, err) + + require.NotNil(t, s.queuePendingGloasColumn(roCol, "peer1")) + require.Equal(t, false, s.hasPendingGloasColumns(root)) + }) + + t.Run("column proof length mismatch rejected", func(t *testing.T) { + s := &Service{ + cfg: &config{clock: clock}, + pendingGloasColumns: make(map[[32]byte]*pendingGloasEntry), + } + root := [32]byte{0x79} + dc := ðpb.DataColumnSidecarGloas{ + Index: 0, + Slot: clock.CurrentSlot(), + BeaconBlockRoot: root[:], + Column: [][]byte{make([]byte, 2048), make([]byte, 2048)}, + KzgProofs: [][]byte{make([]byte, 48)}, + } + roCol, err := blocks.NewRODataColumnGloasWithRoot(dc, root) + require.NoError(t, err) + + require.NotNil(t, s.queuePendingGloasColumn(roCol, "peer1")) require.Equal(t, false, s.hasPendingGloasColumns(root)) }) @@ -298,7 +408,7 @@ func TestPendingGloasColumns(t *testing.T) { } roCol, err := blocks.NewRODataColumnGloasWithRoot(dc, root) require.NoError(t, err) - s.queuePendingGloasColumn(roCol, "peer1") + require.NoError(t, s.queuePendingGloasColumn(roCol, "peer1")) } require.Equal(t, maxPendingGloasRoots, len(s.pendingGloasColumns)) @@ -313,7 +423,7 @@ func TestPendingGloasColumns(t *testing.T) { } roCol, err := blocks.NewRODataColumnGloasWithRoot(dc, overflowRoot) require.NoError(t, err) - s.queuePendingGloasColumn(roCol, "peer1") + require.NoError(t, s.queuePendingGloasColumn(roCol, "peer1")) require.Equal(t, false, s.hasPendingGloasColumns(overflowRoot)) // Adding to an existing root should still work. @@ -327,7 +437,7 @@ func TestPendingGloasColumns(t *testing.T) { } roCol2, err := blocks.NewRODataColumnGloasWithRoot(dc2, existingRoot) require.NoError(t, err) - s.queuePendingGloasColumn(roCol2, "peer1") + require.NoError(t, s.queuePendingGloasColumn(roCol2, "peer1")) require.NotNil(t, s.pendingGloasColumns[existingRoot].columns[1]) }) @@ -361,7 +471,7 @@ func TestPendingGloasColumns(t *testing.T) { // Queue the sidecar. roCol, err := blocks.NewRODataColumnGloasWithRoot(sidecar, blockRoot) require.NoError(t, err) - s.queuePendingGloasColumn(roCol, "peer1") + require.NoError(t, s.queuePendingGloasColumn(roCol, "peer1")) require.Equal(t, true, s.hasPendingGloasColumns(blockRoot)) // Process with the block. @@ -404,7 +514,7 @@ func TestPendingGloasColumns(t *testing.T) { roCol, err := blocks.NewRODataColumnGloasWithRoot(sidecar, blockRoot) require.NoError(t, err) - s.queuePendingGloasColumn(roCol, "badpeer") + require.NoError(t, s.queuePendingGloasColumn(roCol, "badpeer")) s.processPendingGloasColumns(blockRoot, signedBlock) require.Equal(t, false, s.hasPendingGloasColumns(blockRoot)) diff --git a/beacon-chain/sync/validate_execution_payload_bid.go b/beacon-chain/sync/validate_execution_payload_bid.go index 4f2ff89ddd55..3293cc20da91 100644 --- a/beacon-chain/sync/validate_execution_payload_bid.go +++ b/beacon-chain/sync/validate_execution_payload_bid.go @@ -54,6 +54,13 @@ func (s *Service) validateExecutionPayloadBidGossip(ctx context.Context, pid pee return pubsub.ValidationIgnore, err } + // [IGNORE] this is the first signed bid seen with a valid signature from the given builder for this slot. + // Cache is populated only after VerifySignature below; a hit here implies a valid-sig bid was already seen. + builderKey := executionPayloadBidBuilderKey(bid.Slot(), bid.BuilderIndex()) + if s.hasSeenExecutionPayloadBidBuilder(builderKey) { + return pubsub.ValidationIgnore, nil + } + // [IGNORE] bid.slot is the current slot or the next slot. if err := v.VerifyCurrentOrNextSlot(); err != nil { return pubsub.ValidationIgnore, err @@ -86,17 +93,9 @@ func (s *Service) validateExecutionPayloadBidGossip(ctx context.Context, pid pee if err := v.VerifyFeeRecipientMatches(pref.FeeRecipient[:]); err != nil { return pubsub.ValidationReject, err } - // The spec lists signature validation later, but the "first signed bid seen - // with a valid signature" gate below depends on knowing validity first. if err := v.VerifySignature(st); err != nil { return pubsub.ValidationReject, err } - - // [IGNORE] this is the first signed bid seen with a valid signature from the given builder for this slot. - builderKey := executionPayloadBidBuilderKey(bid.Slot(), bid.BuilderIndex()) - if s.hasSeenExecutionPayloadBidBuilder(builderKey) { - return pubsub.ValidationIgnore, nil - } s.setSeenExecutionPayloadBidBuilder(bid.Slot(), builderKey) // [IGNORE] this bid is the highest value bid seen for the tuple (bid.slot, bid.parent_block_hash, bid.parent_block_root). if !s.isHighestExecutionPayloadBid(bid) { @@ -122,8 +121,6 @@ func (s *Service) validateExecutionPayloadBidGossip(ctx context.Context, pid pee if err := v.VerifyParentBlockRootSeen(s.cfg.chain.InForkchoice); err != nil { return pubsub.ValidationIgnore, err } - // [REJECT] signed_execution_payload_bid.signature is valid with respect to the bid.builder_index. - // Verified earlier to satisfy the "first valid signed bid seen" condition. msg.ValidatorData = signedBid return pubsub.ValidationAccept, nil } diff --git a/beacon-chain/sync/validate_execution_payload_bid_test.go b/beacon-chain/sync/validate_execution_payload_bid_test.go index ce5614680103..06fa1f337fcb 100644 --- a/beacon-chain/sync/validate_execution_payload_bid_test.go +++ b/beacon-chain/sync/validate_execution_payload_bid_test.go @@ -51,6 +51,26 @@ func TestValidateExecutionPayloadBidGossip_AlreadySeenBuilder(t *testing.T) { require.Equal(t, pubsub.ValidationIgnore, result) } +// Dedup must short-circuit before every later check; duplicates pay only the cache lookup. +func TestValidateExecutionPayloadBidGossip_DedupShortCircuitsAllLaterChecks(t *testing.T) { + ctx := context.Background() + s, msg, signedBid := setupExecutionPayloadBidService(t) + key := executionPayloadBidBuilderKey(signedBid.Message.Slot, signedBid.Message.BuilderIndex) + s.setSeenExecutionPayloadBidBuilder(signedBid.Message.Slot, key) + // Every subsequent verifier method would Reject/Ignore if it ran; the cache hit must skip them all. + s.newExecutionPayloadBidVerifier = testNewExecutionPayloadBidVerifier(mockExecutionPayloadBidVerifier{ + errCurrentOrNextSlot: errors.New("slot"), + errBuilderActive: errors.New("builder"), + errExecutionPayment: errors.New("payment"), + errFeeRecipientMismatch: errors.New("fee"), + errSignature: errors.New("sig"), + }) + + result, err := s.validateExecutionPayloadBidGossip(ctx, "", msg) + require.NoError(t, err) + require.Equal(t, pubsub.ValidationIgnore, result) +} + func TestValidateExecutionPayloadBidGossip_ProposerPreferencesUnseen(t *testing.T) { ctx := context.Background() s, msg, _ := setupExecutionPayloadBidService(t) diff --git a/changelog/terence_cap-pending-gloas-column-cells.md b/changelog/terence_cap-pending-gloas-column-cells.md new file mode 100644 index 000000000000..648a4a905272 --- /dev/null +++ b/changelog/terence_cap-pending-gloas-column-cells.md @@ -0,0 +1,3 @@ +### Fixed + +- Cap Gloas data column sidecar cell count by `MaxBlobsPerBlockAtEpoch` at pending-queue admission, and require `KzgProofs` length to match `Column`. diff --git a/changelog/terence_dedup-bid-before-sig-verify.md b/changelog/terence_dedup-bid-before-sig-verify.md new file mode 100644 index 000000000000..599eacf110f7 --- /dev/null +++ b/changelog/terence_dedup-bid-before-sig-verify.md @@ -0,0 +1,3 @@ +### Fixed + +- Run the `(slot, builder_index)` dedup before BLS signature verification in `validateExecutionPayloadBidGossip`. Duplicates of an already-seen bid now short-circuit without running `VerifySignature`.