cl: validate Fulu cell proof widths

[domiwei]

Summary

• reject Fulu and later BlobsBundleV2 responses when any cell proof is not exactly 48 bytes
• validate all proof widths before fixed-array conversion or blob cache/body mutation
• preserve the existing pre-Fulu validation order

Why

The Osaka Engine API defines each BlobsBundleV2.proofs element as a 48-byte KZGProof. Block production previously checked only proof cardinality for Fulu and later. A short proof could therefore panic during []byte to common.Bytes48 conversion, while an overlong proof could be silently truncated.

Tests

• TDD repro: 47-byte proof panicked before the fix; 49-byte proof escaped validation
• focused 47/48/49-byte and pre-Fulu ordering tests
• go test ./cl/beacon/handler -count=1
• focused go test -race
• make lint (twice, isolated cache)
• make erigon integration

This is the first of the follow-up hardening changes found while reviewing #23299. Broader Engine response and KZG semantic validation remain separate PRs.

[awskii]

All three new tests corrupt proofs[0] only, so nothing pins that the loop at block_production.go:1208 covers the whole slice.

Narrow it to for _, proof := range bundles.Proofs[:len(bundles.Blobs)] — the same shape as the pre-Fulu check 13 lines below at 1221 — and the suite stays green: with one blob that slice is Proofs[:1], which is exactly the element this helper corrupts. A Fulu bundle whose short proof sits at column 1..127 then reaches common.Bytes48(bundles.Proofs[i*NumberOfColumns+j]) at 1241 and panics the block-production goroutine, which is the case the PR exists to prevent.

Corrupting the last column instead closes that:

Suggested change

	proofs[0] = make([]byte, proofLength)
	proofs[len(proofs)-1] = make([]byte, proofLength)

Taking the index as a parameter and running both ends would be better still.

[awskii]

The unguarded bundles deref just above your new check — block_production.go:1197 and :1206 — is fixed separately in #23586. No need to pull it into this branch.

[Copilot]

Pull request overview

Validates Fulu+ cell proof widths before conversion or state mutation, preventing panics and truncation.

Changes:

• Rejects cell proofs not exactly 48 bytes.
• Adds 47/48/49-byte proof tests.
• Verifies pre-Fulu validation order remains unchanged.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
cl/beacon/handler/block_production.go	Validates Fulu+ proof widths before processing bundles.
cl/beacon/handler/block_production_test.go	Covers invalid, valid, and pre-Fulu validation behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[yperbasis]

Please address the unresolved regression-test coverage gap before merging. The new tests only alter proofs[0], so an implementation that validates only the first proof would still pass while a malformed later column can still panic during fixed-array conversion. Parameterize the proof index and cover at least the first and last columns.

The production validation otherwise looks correct. The nil BlobsBundle case remains separate in #23586.

[domiwei]

@yperbasis Addressed in 579dd3bbb4 and cbd13742a6.

The invalid-proof test now parameterizes the proof index and covers both the first and final proof with 47-byte and 49-byte values. I also strengthened the fixture to use two blobs, so the final case exercises the last proof of the second blob rather than only the first blob’s columns. The exact 48-byte and pre-Fulu cases remain covered.

Could you please re-review when convenient?

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[yperbasis]

Non-blocking notes, mostly pre-existing or adjacent to this change:

• produceBeaconBody dereferences bundles without a nil check (line 1197), so a remote EL replying without blobsBundle panics the process — same trust boundary, already fixed by #23586.
• Bundle validation never bounds the blob count by MaxBlobsPerBlock, so a bad EL response could make the proposer sign an over-full block that peers then reject.
• The Fulu arm of publishBlindedBlocks (line 1761) lacks the same proof validation. It is dead code behind the version >= Fulu early return, but its nolint points to #17943, which is closed, so the gap is now untracked.
• The Fulu happy-path test asserts only NoError/NotNil and uses two identical commitments (they collide on one cache key). Distinct commitments plus asserting each cached 128-proof slice would catch a wrong proof-indexing regression that currently stays green.
• The 48-byte proof width check now lives in two fork-gated arms (new upfront loop at line 1208, pre-Fulu clause at line 1221); one unconditional loop after the cardinality check would cover both.
• Commitment/blob width checks still run inside the loop that mutates blobBundles, so a bundle failing on a later blob leaves earlier blobs cached from a failed production; hoisting them next to the new upfront loop would complete validate-before-mutate.
• TestProduceBeaconBodyPreservesPreFuluValidationOrder pins which error message wins when both widths are bad, so any harmless reordering of independent checks turns it red; corrupting only the commitment would pin the ordering without freezing message precedence.
• GetDataColumnSidecars/GetDataColumnSidecarsGloas index Proofs[columnIndex] without checking that each entry has NumberOfColumns proofs; a wrong-width cache entry would panic during broadcast.
• In the tests, 2*int(NumberOfColumns) and the fixture blob count 2 repeat across three sites; a named const would keep the "last proof of the last blob" index honest.