Skip to content

Fix: engine nits#753

Open
julio4 wants to merge 7 commits intoethereum:mainfrom
julio4:patch-1
Open

Fix: engine nits#753
julio4 wants to merge 7 commits intoethereum:mainfrom
julio4:patch-1

Conversation

@julio4
Copy link
Copy Markdown

@julio4 julio4 commented Feb 17, 2026

This PR fixes a few nits I've found while reviewing specs.

@MysticRyuujin
Copy link
Copy Markdown
Contributor

Good fixes overall, but two issues to address:

1. Wrong anchor for verify_cell_kzg_proof_batch

The PR changes the anchor from #verify_cell_kzg_proof_batch to #verify_cell_kzg_proof_batch_impl, but the spec text is calling the public function — _impl is the internal implementation. The anchor
should stay as #verify_cell_kzg_proof_batch.

2. Grammar in engine_getPayloadV5 description

it returns as BlobsBundleV2

Should be:

it returns BlobsBundleV2


Everything else looks correct:

  • BlobBundleV1BlobsBundleV1 typo fix ✅
  • SECONDS_PER_SLOTSLOT_DURATION_MS is accurate (upstream renamed it, dev branch is 404) ✅
  • #beaconblocksbyroot-v1 and #beaconblocksbyrange-v1 anchors both exist on master
  • #executionpayload anchor is valid on master

@julio4
Copy link
Copy Markdown
Author

julio4 commented Mar 6, 2026

Good fixes overall, but two issues to address:

1. Wrong anchor for verify_cell_kzg_proof_batch

The PR changes the anchor from #verify_cell_kzg_proof_batch to #verify_cell_kzg_proof_batch_impl, but the spec text is calling the public function — _impl is the internal implementation. The anchor should stay as #verify_cell_kzg_proof_batch.

2. Grammar in engine_getPayloadV5 description

it returns as BlobsBundleV2

Should be:

it returns BlobsBundleV2

Good find thanks. Fixed

@MysticRyuujin
Copy link
Copy Markdown
Contributor

@mkalinin think you could review this one?

Comment thread src/engine/osaka.md
1. `assert len(blobsBundle.commitments) == len(blobsBundle.blobs)` and
2. `assert len(blobsBundle.proofs) == len(blobsBundle.blobs) * CELLS_PER_EXT_BLOB` and
3. `assert verify_cell_kzg_proof_batch(commitments, cell_indices, cells, blobsBundle.proofs)` (see [EIP-7594 consensus-specs](https://github.com/ethereum/consensus-specs/blob/36d80adb44c21c66379c6207a9578f9b1dcc8a2d/specs/fulu/polynomial-commitments-sampling.md#verify_cell_kzg_proof_batch))
3. `assert verify_cell_kzg_proof_batch(commitments, cell_indices, cells, blobsBundle.proofs)` (see [EIP-7594 consensus-specs](https://github.com/ethereum/consensus-specs/blob/master/specs/fulu/polynomial-commitments-sampling.md#verify_cell_kzg_proof_batch))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would be more stable to have a link anchored to a specific commit. wdyt?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I guess it's a choice to make. My intuition is that the specs should not change drastically and master branch is considered the latest stable version.

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.

3 participants