add batch validation path for column to speedup gossip validaiton by agnxsh · Pull Request #8314 · status-im/nimbus-eth2

agnxsh · 2026-04-21T22:47:55Z

No description provided.

github-actions · 2026-04-21T23:11:55Z

Unit Test Results

0 tests 0 ✔️ 0s ⏱️
0 suites 0 💤
0 files 0 ❌

Results for commit ce5b14f.

♻️ This comment has been updated with latest results.

tersec · 2026-04-22T00:51:42Z

+    return
+
+  withBlck(blckOpt.get()):
+    when (consensusFork >= ConsensusFork.Fulu) and


This is just exactly consensusFork == ConsensusFork.Fulu.

tersec · 2026-04-22T01:05:17Z

+    sidecars.add(s[])
+
+  data_column_kzg_batch_verifications.inc()
+  let batchRes = verify_data_column_sidecar_kzg_proofs(sidecars)


let batchRes = verify_data_column_sidecar_kzg_proofs(pending.mapIt(it[]))

and can remove all of

var sidecars = newSeqOfCap[fulu.DataColumnSidecar](pending.len) for s in pending: sidecars.add(s[])

which clarifies the logic. But I don't understand why it needs this at all ref fulu.DataColumnSidecar to fulu.DataColumnSidecar conversion at all, since

speedup block processing with batch verification #8302 appears to deliberately add overloads for both openArray[fulu.DataColumnSidecar] and openArray[ref fulu.DataColumnSidecar]:

nimbus-eth2/beacon_chain/spec/peerdas_helpers.nim

Lines 591 to 609 in 6286825

proc verify_data_column_sidecar_kzg_proofs*[

T: fulu.DataColumnSidecar | gloas.DataColumnSidecar |

ref fulu.DataColumnSidecar | ref gloas.DataColumnSidecar](

sidecars: openArray[T],

kzg_commitments: KzgCommitments): Result[void, cstring] =

## Batch verify KZG proofs across multiple DataColumnSidecars against a

## single shared `kzg_commitments` array (e.g. gloas, where commitments

## come from the bid). Accepts either values or refs.

verifyKzgBatchBody(sidecars, kzg_commitments.len, kzg_commitments[i])

proc verify_data_column_sidecar_kzg_proofs*[

T: fulu.DataColumnSidecar | ref fulu.DataColumnSidecar](

sidecars: openArray[T]): Result[void, cstring] =

## Batch verify KZG proofs across multiple fulu DataColumnSidecars using

## each sidecar's own `kzg_commitments`. Equivalent to verifying each

## sidecar individually: a sidecar carrying commitments that don't match

## its cells/proofs is rejected regardless of its position in the batch.

verifyKzgBatchBody(

sidecars, sidecar.kzg_commitments.len, sidecar.kzg_commitments[i])

tersec · 2026-04-22T01:09:50Z

-          self.blockProcessor.enqueueBlock(MsgSource.gossip, forkyBlck, cres)
-        else:
-          raiseAssert "Wrong fork for columns: " & $consensusFork
+  let batchThreshold = max(self.dataColumnQuarantine[].custodyColumns.len, 1)


When is self.dataColumnQuarantine[].custodyColumns.len < 4? But if it's not, then this max doesn't do anything. What is it accomplishing?

tersec · 2026-04-22T01:16:45Z

+  self.pendingGloasColumnSidecars.add(newClone(dataColumnSidecar))
+
+  let batchThreshold =
+    max(self.gloasColumnQuarantine[].custodyColumns.len, 1)


Same question about the purpose of max here: this only matters if self.gloasColumnQuarantine[].custodyColumns.len is 0, but that should never happen. If it does, there are bigger problems.

i.e. a network with 0 custody columns also means not doing anything with gloasColumnQuarantine or columnQuarantine at all, not just tweaking around the edges.

tersec · 2026-04-22T01:29:21Z

+
+    data_column_kzg_batch_verifications.inc()
+    let batchRes = verify_data_column_sidecar_kzg_proofs(
+      sidecars, commitmentsOpt.get())


let batchRes = verify_data_column_sidecar_kzg_proofs( pending.mapIt(it[]), commitmentsOpt.get())

makes the logic clearer here by connecting it to its inputs more directly and simply. But it raises the same question of why do this ref to non-ref conversion at all, per https://github.com/status-im/nimbus-eth2/pull/8314/changes#r3121047564

tersec · 2026-04-22T02:31:19Z

+    else:
+      raiseAssert "Wrong fork for columns: " & $consensusFork
+
+proc flushPendingColumnSidecars*(self: var Eth2Processor) =


Would be clearer to name this flushFuluPendingColumnSidecars. For legacy reasons a number of column-related functions were written before any support for Gloas existed, and something with functionName became functionNameWithGloas without renaming the original function, and there's an argument for that, but there isn't a viable one for newly written functions.

tersec · 2026-04-22T03:13:06Z

+  # Non-KZG checks passed — park the sidecar in the pending buffer. KZG
+  # proofs are verified in a single batched call once the buffer has grown
+  # to the number of columns this node custodies, i.e. the largest batch
+  # we can reasonably expect to assemble per slot.


This doesn't suffice for gossip validation:

[REJECT] The sidecar's kzg_commitments field inclusion proof is valid as verified by verify_data_column_sidecar_inclusion_proof(sidecar).

[REJECT] The sidecar's column data is valid as verified by verify_data_column_sidecar_kzg_proofs(sidecar).

tersec · 2026-04-22T03:20:50Z

-  self.blockProcessor.enqueuePayload(dataColumnSidecar.beacon_block_root)
+  # Non-KZG checks passed — park the sidecar in the pending buffer. KZG
+  # proofs are verified in a single batched call once the buffer has grown
+  # to the number of columns this node custodies.


This doesn't suffice for gossip validation:

[REJECT] The sidecar is valid as verified by verify_data_column_sidecar(sidecar, bid.blob_kzg_commitments).

[REJECT] The sidecar's column data is valid as verified by verify_data_column_sidecar_kzg_proofs(sidecar, bid.blob_kzg_commitments).

[IGNORE] The sidecar is the first sidecar for the tuple (sidecar.beacon_block_root, sidecar.index) with valid kzg proof.

tersec reviewed Apr 22, 2026

View reviewed changes

add batch validation path for column to speedup gossip validaiton

ce5b14f

agnxsh force-pushed the bnkzg2 branch from a908953 to ce5b14f Compare April 25, 2026 11:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add batch validation path for column to speedup gossip validaiton#8314

add batch validation path for column to speedup gossip validaiton#8314
agnxsh wants to merge 1 commit intounstablefrom
bnkzg2

agnxsh commented Apr 21, 2026

Uh oh!

github-actions Bot commented Apr 21, 2026 •

edited

Loading

Uh oh!

tersec Apr 22, 2026

Uh oh!

tersec Apr 22, 2026

Uh oh!

tersec Apr 22, 2026

Uh oh!

tersec Apr 22, 2026

Uh oh!

tersec Apr 22, 2026

Uh oh!

tersec Apr 22, 2026

Uh oh!

tersec Apr 22, 2026

Uh oh!

tersec Apr 22, 2026

Uh oh!

tersec Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	proc verify_data_column_sidecar_kzg_proofs*[
	T: fulu.DataColumnSidecar \| gloas.DataColumnSidecar \|
	ref fulu.DataColumnSidecar \| ref gloas.DataColumnSidecar](
	sidecars: openArray[T],
	kzg_commitments: KzgCommitments): Result[void, cstring] =
	## Batch verify KZG proofs across multiple DataColumnSidecars against a
	## single shared `kzg_commitments` array (e.g. gloas, where commitments
	## come from the bid). Accepts either values or refs.
	verifyKzgBatchBody(sidecars, kzg_commitments.len, kzg_commitments[i])

	proc verify_data_column_sidecar_kzg_proofs*[
	T: fulu.DataColumnSidecar \| ref fulu.DataColumnSidecar](
	sidecars: openArray[T]): Result[void, cstring] =
	## Batch verify KZG proofs across multiple fulu DataColumnSidecars using
	## each sidecar's own `kzg_commitments`. Equivalent to verifying each
	## sidecar individually: a sidecar carrying commitments that don't match
	## its cells/proofs is rejected regardless of its position in the batch.
	verifyKzgBatchBody(
	sidecars, sidecar.kzg_commitments.len, sidecar.kzg_commitments[i])

Conversation

agnxsh commented Apr 21, 2026

Uh oh!

github-actions Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Apr 21, 2026 •

edited

Loading