Creation of blob sidecars (with inclusion proof) when publishing #7721

StefanBratanov · 2023-11-16T15:43:08Z

PR Description

Add computeBlobSidecar in MiscHelpersDeneb
Create a new sidecar selector in BlockOperationSelectorFactory
Create createBlobSidecars method in BlockFactory and use it in AbstractBlockPublisher

Fixed Issue(s)

helps with block publishing flow for #7654

Documentation

I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

I thought about adding a changelog entry, and added one if I deemed necessary.

beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDeneb.java

              return operationSelector
-                  .createBlobSidecarsSelector()
+                  .createBlobSidecarsSelectorOld()


beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDeneb.java

              return operationSelector
-                  .createBlobSidecarsSelector()
+                  .createBlobSidecarsSelectorOld()


...src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java

-                  .isEqualTo(blobsBundle.getCommitments().get(0));
-              assertThat(blobSidecar.getKZGProof()).isEqualTo(blobsBundle.getProofs().get(0));
-            });
+        safeJoin(factory.createBlobSidecarsSelectorOld().apply(block));


...alidator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java

...spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDeneb.java

zilm13 · 2023-11-17T14:04:32Z

ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java

@@ -1181,6 +1207,49 @@ public BeaconBlockBody randomBlindedBeaconBlockBody() {
    return randomBlindedBeaconBlockBody(randomUInt64());
  }

+  public BeaconBlockBody randomBlindedBeaconBlockBodyWithCommitments(
+      final UInt64 slot, final SszList<SszKZGCommitment> commitments) {
+    BeaconBlockBodySchema<?> schema =


zilm13 · 2023-11-17T14:08:02Z

ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java

+            })
+        .join();
+  }
+
  public BeaconBlockBody randomBlindedBeaconBlockBody(UInt64 slotNum) {


We could call yours from this one with randomBlobKzgCommitments() at commitments.

The problem is for pre-Deneb spec that would fail since there would be no BlobKzgCommitmentsSchema

Ahh, got it. Ok

zilm13 · 2023-11-17T14:11:48Z

ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java

@@ -1410,8 +1484,7 @@ public BeaconBlockBody randomBeaconBlockBodyWithCommitments(final int count) {
                    .syncAggregate(randomSyncAggregateIfRequiredBySchema(schema))
                    .executionPayload(SafeFuture.completedFuture(randomExecutionPayload()))
                    .blsToExecutionChanges(randomSignedBlsToExecutionChangesList())
-                    .blobKzgCommitments(
-                        SafeFuture.completedFuture(randomBlobKzgCommitments(count))))
+                    .blobKzgCommitments(SafeFuture.completedFuture(commitments)))
        .join();
  }



you could use #randomBeaconBlockBodyWithCommitments( final SszList<SszKZGCommitment> commitments) in randomFullBeaconBlockBody()

So this suggestion will fail preDeneb too. Ok

zilm13 · 2023-11-17T15:54:28Z

...spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDeneb.java

+      final SignedBeaconBlock signedBeaconBlock,
+      final UInt64 index,
+      final Blob blob,
+      final SszKZGCommitment commitment,


I don't like this interface a bit, because we already have commitment in block and passed index, so passing commitment separately is redundant and could cause error, maybe omit this field?

I like that idea. Makes things cleaner.

zilm13 · 2023-11-17T16:29:08Z

...n/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java

+
+    final BlockAndBlobs blockAndBlobs = assertBlobSidecarsCreated(false, spec);
+
+    final List<BlobSidecar> blobSidecars = blockAndBlobs.blobSidecars();


could you please add assert not empty here, I'm a bit paranoid

These assertions are done in the assertBlobSidecarsCreated method following the pattern from other unit tests in this class.

zilm13 · 2023-11-17T16:30:54Z

...n/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java

+
+    final BlockAndBlobs blockAndBlobs = assertBlobSidecarsCreated(true, spec);
+
+    final List<BlobSidecar> blobSidecars = blockAndBlobs.blobSidecars();


and here please (check non empty)

...src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java

StefanBratanov · 2023-11-18T11:04:34Z

Thanks for feedback @zilm13 Added required changes, so you can have another look when you can.

ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java

zilm13

Looks great, good refactor of computeKzgCommitmentInclusionProof
Just some final nits
Also #7721 (comment) is still actual

...n/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java

StefanBratanov · 2023-11-20T10:59:23Z

Looks great, good refactor of computeKzgCommitmentInclusionProof Just some final nits Also #7721 (comment) is still actual

Thanks. I renamed the method to createBlockAndBlobSidecars in 6a7f1b0 to be more correct. Does it make sense?

zilm13 · 2023-11-20T11:20:36Z

@StefanBratanov yeah, looks good

StefanBratanov marked this pull request as draft November 16, 2023 15:43

github-advanced-security bot found potential problems Nov 16, 2023

View reviewed changes

StefanBratanov force-pushed the creation_blob_sidecars branch from aad6a7f to ecfe8c4 Compare November 17, 2023 08:48

StefanBratanov marked this pull request as ready for review November 17, 2023 11:51

StefanBratanov force-pushed the creation_blob_sidecars branch from 1ff3ff7 to 01f2428 Compare November 17, 2023 11:51

github-advanced-security bot found potential problems Nov 17, 2023

View reviewed changes

...alidator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java Fixed Show fixed Hide fixed

StefanBratanov force-pushed the creation_blob_sidecars branch from 7ef48e4 to cfaf024 Compare November 17, 2023 13:52

zilm13 reviewed Nov 17, 2023

View reviewed changes

StefanBratanov commented Nov 20, 2023

View reviewed changes

ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java Outdated Show resolved Hide resolved

StefanBratanov added 13 commits November 20, 2023 10:42

Creation of blob sidecars (with inclusion proof) when publishing

4579828

use bounded blob sidecar index

1921ead

add tests

9375c2d

improve assertions

c25469e

fix unit test

a2165ab

fix property tests

568e1e4

better property test fix

dd12cbe

add license

ac3d157

some comments changing and remove unused

2947b55

better fuzzing

2f82486

refactor

407062c

changes as per feedback

f54e15a

add final

d7d01da

zilm13 approved these changes Nov 20, 2023

View reviewed changes

...n/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java Show resolved Hide resolved

...n/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java Show resolved Hide resolved

resolve conflict on rebase

00a7246

StefanBratanov force-pushed the creation_blob_sidecars branch from eadb9f3 to 00a7246 Compare November 20, 2023 10:44

feedback

6a7f1b0

StefanBratanov merged commit f3300d8 into Consensys:master Nov 20, 2023
15 checks passed

StefanBratanov deleted the creation_blob_sidecars branch November 20, 2023 11:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Creation of blob sidecars (with inclusion proof) when publishing #7721

Creation of blob sidecars (with inclusion proof) when publishing #7721

StefanBratanov commented Nov 16, 2023 •

edited

Loading

zilm13 Nov 17, 2023

zilm13 Nov 17, 2023

StefanBratanov Nov 18, 2023

zilm13 Nov 20, 2023

zilm13 Nov 17, 2023

zilm13 Nov 20, 2023

zilm13 Nov 17, 2023

StefanBratanov Nov 18, 2023

zilm13 Nov 17, 2023

StefanBratanov Nov 18, 2023 •

edited

Loading

zilm13 Nov 17, 2023

StefanBratanov commented Nov 18, 2023

zilm13 left a comment

StefanBratanov commented Nov 20, 2023 •

edited

Loading

zilm13 commented Nov 20, 2023


		final BlockAndBlobs blockAndBlobs = assertBlobSidecarsCreated(false, spec);

		final List<BlobSidecar> blobSidecars = blockAndBlobs.blobSidecars();

Creation of blob sidecars (with inclusion proof) when publishing #7721

Creation of blob sidecars (with inclusion proof) when publishing #7721

Conversation

StefanBratanov commented Nov 16, 2023 • edited Loading

PR Description

Fixed Issue(s)

Documentation

Changelog

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanBratanov Nov 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanBratanov commented Nov 18, 2023

zilm13 left a comment

Choose a reason for hiding this comment

StefanBratanov commented Nov 20, 2023 • edited Loading

zilm13 commented Nov 20, 2023

StefanBratanov commented Nov 16, 2023 •

edited

Loading

StefanBratanov Nov 18, 2023 •

edited

Loading

StefanBratanov commented Nov 20, 2023 •

edited

Loading