changes as per feedback

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

@@ -345,17 +345,6 @@ public Function<SignedBlockContainer, List<BlobSidecar>> createBlobSidecarsSelec
       final MiscHelpersDeneb miscHelpersDeneb =
           MiscHelpersDeneb.required(spec.atSlot(slot).miscHelpers());
 
-      final SszList<SszKZGCommitment> commitments =
-          blockContainer
-              .getSignedBlock()
-              .getMessage()
-              .getBody()
-              .getOptionalBlobKzgCommitments()
-              .orElseThrow(
-                  () ->
-                      new IllegalStateException(
-                          "BlobKzgCommitments are not available in " + blockContainer));
-
       final SszList<Blob> blobs;
       final SszList<SszKZGProof> proofs;
 
@@ -384,11 +373,10 @@ public Function<SignedBlockContainer, List<BlobSidecar>> createBlobSidecarsSelec
       return IntStream.range(0, blobs.size())
           .mapToObj(
               index ->
-                  miscHelpersDeneb.computeBlobSidecar(
+                  miscHelpersDeneb.constructBlobSidecar(
                       blockContainer.getSignedBlock(),
                       UInt64.valueOf(index),
                       blobs.get(index),
-                      commitments.get(index),
                       proofs.get(index)))
           .toList();
     };

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

@@ -70,7 +70,6 @@
 import tech.pegasys.teku.spec.logic.common.block.AbstractBlockProcessor;
 import tech.pegasys.teku.spec.logic.common.helpers.BeaconStateAccessors;
 import tech.pegasys.teku.spec.logic.common.helpers.MiscHelpers;
-import tech.pegasys.teku.spec.logic.versions.deneb.helpers.MiscHelpersDeneb;
 import tech.pegasys.teku.spec.schemas.SchemaDefinitionsBellatrix;
 import tech.pegasys.teku.spec.schemas.SchemaDefinitionsDeneb;
 import tech.pegasys.teku.spec.util.DataStructureUtil;
@@ -299,7 +298,7 @@ protected SignedBeaconBlock blindSignedBeaconBlockIfUnblinded(
     return spec.blindSignedBeaconBlock(unblindedSignedBeaconBlock);
   }
 
-  protected BlockAndBlobs assertBlobSidecarsCreated(final boolean blinded, final Spec spec) {
+  protected BlockAndBlobSidecars createBlobSidecars(final boolean blinded, final Spec spec) {
     final BlockFactory blockFactory = createBlockFactory(spec);
     final DataStructureUtil dataStructureUtil = new DataStructureUtil(spec);
 
@@ -330,16 +329,7 @@ protected BlockAndBlobs assertBlobSidecarsCreated(final boolean blinded, final S
 
     final List<BlobSidecar> blobSidecars = blockFactory.createBlobSidecars(signedBlockContainer);
 
-    if (spec.isMilestoneSupported(SpecMilestone.DENEB)) {
-      final int expectedBlobs =
-          MiscHelpersDeneb.required(spec.atSlot(signedBlockContainer.getSlot()).miscHelpers())
-              .getBlobKzgCommitmentsCount(signedBlockContainer.getSignedBlock());
-      assertThat(blobSidecars).hasSize(expectedBlobs);
-    } else {
-      assertThat(blobSidecars).isEmpty();
-    }
-
-    return new BlockAndBlobs(signedBlockContainer, blobSidecars);
+    return new BlockAndBlobSidecars(signedBlockContainer, blobSidecars);
   }
 
   protected void prepareDefaultPayload(final Spec spec) {
@@ -501,5 +491,6 @@ private SszList<SszKZGCommitment> getCommitmentsFromBuilderPayload() {
         .orElseThrow(() -> new IllegalStateException("BuilderPayload was not prepared"));
   }
 
-  protected record BlockAndBlobs(SignedBlockContainer block, List<BlobSidecar> blobSidecars) {}
+  protected record BlockAndBlobSidecars(
+      SignedBlockContainer block, List<BlobSidecar> blobSidecars) {}
 }

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

@@ -98,18 +98,20 @@ void shouldCreateValidBlobSidecarsForBlockContents() {
     final Spec spec = TestSpecFactory.createMinimalDeneb();
     final BlobsBundle blobsBundle = prepareBlobsBundle(spec, 3);
 
-    final BlockAndBlobs blockAndBlobs = assertBlobSidecarsCreated(false, spec);
+    final BlockAndBlobSidecars blockAndBlobSidecars = createBlobSidecars(false, spec);
 
-    final List<BlobSidecar> blobSidecars = blockAndBlobs.blobSidecars();
+    final List<BlobSidecar> blobSidecars = blockAndBlobSidecars.blobSidecars();
     final SszList<SszKZGCommitment> expectedCommitments =
-        blockAndBlobs
+        blockAndBlobSidecars
             .block()
             .getSignedBlock()
             .getMessage()
             .getBody()
             .getOptionalBlobKzgCommitments()
             .orElseThrow();
 
+    assertThat(blobSidecars).hasSize(expectedCommitments.size());
+
     IntStream.range(0, blobSidecars.size())
         .forEach(
             index -> {
@@ -134,18 +136,20 @@ void shouldCreateValidBlobSidecarsForBlindedBlock() {
     final tech.pegasys.teku.spec.datastructures.builder.BlobsBundle blobsBundle =
         prepareBuilderPayload(spec, blobsCount).getOptionalBlobsBundle().orElseThrow();
 
-    final BlockAndBlobs blockAndBlobs = assertBlobSidecarsCreated(true, spec);
+    final BlockAndBlobSidecars blockAndBlobSidecars = createBlobSidecars(true, spec);
 
-    final List<BlobSidecar> blobSidecars = blockAndBlobs.blobSidecars();
+    final List<BlobSidecar> blobSidecars = blockAndBlobSidecars.blobSidecars();
     final SszList<SszKZGCommitment> expectedCommitments =
-        blockAndBlobs
+        blockAndBlobSidecars
             .block()
             .getSignedBlock()
             .getMessage()
             .getBody()
             .getOptionalBlobKzgCommitments()
             .orElseThrow();
 
+    assertThat(blobSidecars).hasSize(expectedCommitments.size());
+
     IntStream.range(0, blobSidecars.size())
         .forEach(
             index -> {

beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryPhase0Test.java

@@ -165,12 +165,18 @@ void unblindSignedBlock_shouldUnblindBlockWhenBellatrixIsActive() {
 
   @Test
   void shouldCreateEmptyBlobSidecarsForBlock() {
-    assertBlobSidecarsCreated(false, TestSpecFactory.createMinimalPhase0());
+    final BlockAndBlobSidecars blockAndBlobSidecars =
+        createBlobSidecars(false, TestSpecFactory.createMinimalPhase0());
+
+    assertThat(blockAndBlobSidecars.blobSidecars()).isEmpty();
   }
 
   @Test
   void shouldCreateEmptyBlobSidecarsForBlindedBlock() {
-    assertBlobSidecarsCreated(true, TestSpecFactory.createMinimalPhase0());
+    final BlockAndBlobSidecars blockAndBlobSidecars =
+        createBlobSidecars(true, TestSpecFactory.createMinimalPhase0());
+
+    assertThat(blockAndBlobSidecars.blobSidecars()).isEmpty();
   }
 
   @Override

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

@@ -492,6 +492,7 @@ void shouldIncludeKzgCommitmentsInBlindedBlock() {
   }
 
   @Test
+  // TODO: replace this test with the needed selector to produce BlockContents
   void shouldCreateBlobSidecarsForBlockFromCachedPayloadResult() {
     final BeaconBlock block = dataStructureUtil.randomBeaconBlock();
 
@@ -565,7 +566,7 @@ void shouldCreateBlobSidecarsForBlindedBlock() {
     prepareCachedBuilderPayload(
         signedBlindedBeaconBlock.getSlot(),
         dataStructureUtil.randomExecutionPayload(),
-        blobsBundle);
+        Optional.of(blobsBundle));
 
     final List<BlobSidecar> blobSidecars =
         factory.createBlobSidecarsSelector().apply(signedBlindedBeaconBlock);
@@ -680,11 +681,16 @@ private void prepareCachedPayloadResult(
   private void prepareCachedBuilderPayload(
       final UInt64 slot,
       final ExecutionPayload executionPayload,
-      final tech.pegasys.teku.spec.datastructures.builder.BlobsBundle blobsBundle) {
-    BuilderPayload builderPayload =
-        SchemaDefinitionsDeneb.required(spec.atSlot(slot).getSchemaDefinitions())
-            .getExecutionPayloadAndBlobsBundleSchema()
-            .create(executionPayload, blobsBundle);
+      final Optional<tech.pegasys.teku.spec.datastructures.builder.BlobsBundle> blobsBundle) {
+    final BuilderPayload builderPayload =
+        blobsBundle
+            .map(
+                bundle ->
+                    (BuilderPayload)
+                        SchemaDefinitionsDeneb.required(spec.atSlot(slot).getSchemaDefinitions())
+                            .getExecutionPayloadAndBlobsBundleSchema()
+                            .create(executionPayload, bundle))
+            .orElse(executionPayload);
     when(executionLayer.getCachedUnblindedPayload(slot)).thenReturn(Optional.of(builderPayload));
   }
 

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

@@ -18,6 +18,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.Optional;
 import java.util.stream.IntStream;
 import org.apache.tuweni.bytes.Bytes;
@@ -237,21 +238,43 @@ public int getBlobKzgCommitmentsCount(final SignedBeaconBlock signedBeaconBlock)
         .orElse(0);
   }
 
+  public int getBlobSidecarKzgCommitmentGeneralizedIndex(final UInt64 blobSidecarIndex) {
+    final long blobKzgCommitmentsGeneralizedIndex =
+        beaconBlockBodySchema.getBlobKzgCommitmentsGeneralizedIndex();
+    final long commitmentGeneralizedIndex =
+        beaconBlockBodySchema
+            .getBlobKzgCommitmentsSchema()
+            .getChildGeneralizedIndex(blobSidecarIndex.longValue());
+    return (int)
+        GIndexUtil.gIdxCompose(blobKzgCommitmentsGeneralizedIndex, commitmentGeneralizedIndex);
+  }
+
   public List<Bytes32> computeKzgCommitmentInclusionProof(
       final UInt64 blobSidecarIndex, final BeaconBlockBody beaconBlockBody) {
     return MerkleUtil.constructMerkleProof(
         beaconBlockBody.getBackingNode(),
         getBlobSidecarKzgCommitmentGeneralizedIndex(blobSidecarIndex));
   }
 
-  public BlobSidecar computeBlobSidecar(
+  public BlobSidecar constructBlobSidecar(
       final SignedBeaconBlock signedBeaconBlock,
       final UInt64 index,
       final Blob blob,
-      final SszKZGCommitment commitment,
       final SszKZGProof proof) {
+    final BeaconBlockBody beaconBlockBody = signedBeaconBlock.getMessage().getBody();
+    final SszKZGCommitment commitment;
+    try {
+      commitment =
+          beaconBlockBody.getOptionalBlobKzgCommitments().orElseThrow().get(index.intValue());
+    } catch (final IndexOutOfBoundsException | NoSuchElementException ex) {
+      final int commitmentsCount = getBlobKzgCommitmentsCount(signedBeaconBlock);
+      throw new IllegalArgumentException(
+          String.format(
+              "Can't create blob sidecar with index %s because there are %d commitment(s) in block",
+              index, commitmentsCount));
+    }
     final List<Bytes32> kzgCommitmentInclusionProof =
-        computeKzgCommitmentInclusionProof(index, signedBeaconBlock.getMessage().getBody());
+        computeKzgCommitmentInclusionProof(index, beaconBlockBody);
     return blobSidecarSchema.create(
         index, blob, commitment, proof, signedBeaconBlock.asHeader(), kzgCommitmentInclusionProof);
   }
@@ -264,15 +287,4 @@ public boolean verifyBlobSidecarMerkleProof(final BlobSidecar blobSidecar) {
         getBlobSidecarKzgCommitmentGeneralizedIndex(blobSidecar.getIndex()),
         blobSidecar.getBlockBodyRoot());
   }
-
-  private int getBlobSidecarKzgCommitmentGeneralizedIndex(final UInt64 blobSidecarIndex) {
-    final long blobKzgCommitmentsGeneralizedIndex =
-        beaconBlockBodySchema.getBlobKzgCommitmentsGeneralizedIndex();
-    final long commitmentGeneralizedIndex =
-        beaconBlockBodySchema
-            .getBlobKzgCommitmentsSchema()
-            .getChildGeneralizedIndex(blobSidecarIndex.longValue());
-    return (int)
-        GIndexUtil.gIdxCompose(blobKzgCommitmentsGeneralizedIndex, commitmentGeneralizedIndex);
-  }
 }

ethereum/spec/src/property-test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDenebPropertyTest.java

@@ -32,7 +32,6 @@
 import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar;
 import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
-import tech.pegasys.teku.spec.datastructures.type.SszKZGCommitment;
 import tech.pegasys.teku.spec.datastructures.type.SszKZGProof;
 import tech.pegasys.teku.spec.logic.common.helpers.Predicates;
 import tech.pegasys.teku.spec.propertytest.suppliers.SpecSupplier;
@@ -42,7 +41,6 @@
 import tech.pegasys.teku.spec.propertytest.suppliers.blocks.versions.deneb.BeaconBlockSupplier;
 import tech.pegasys.teku.spec.propertytest.suppliers.blocks.versions.deneb.SignedBeaconBlockSupplier;
 import tech.pegasys.teku.spec.propertytest.suppliers.type.KZGCommitmentSupplier;
-import tech.pegasys.teku.spec.propertytest.suppliers.type.SszKZGCommitmentSupplier;
 import tech.pegasys.teku.spec.propertytest.suppliers.type.SszKZGProofSupplier;
 import tech.pegasys.teku.spec.schemas.SchemaDefinitionsDeneb;
 
@@ -113,14 +111,17 @@ void fuzzVerifyBlobSidecarCompleteness(
   }
 
   @Property(tries = 100)
-  void fuzzComputeBlobSidecarAndVerifyMerkleProof(
+  void fuzzConstructBlobSidecarAndVerifyMerkleProof(
       @ForAll(supplier = SignedBeaconBlockSupplier.class) final SignedBeaconBlock signedBeaconBlock,
       @ForAll(supplier = BlobSidecarIndexSupplier.class) final UInt64 index,
       @ForAll(supplier = BlobSupplier.class) final Blob blob,
-      @ForAll(supplier = SszKZGCommitmentSupplier.class) final SszKZGCommitment commitment,
       @ForAll(supplier = SszKZGProofSupplier.class) final SszKZGProof proof) {
-    final BlobSidecar blobSidecar =
-        miscHelpers.computeBlobSidecar(signedBeaconBlock, index, blob, commitment, proof);
-    miscHelpers.verifyBlobSidecarMerkleProof(blobSidecar);
+    try {
+      final BlobSidecar blobSidecar =
+          miscHelpers.constructBlobSidecar(signedBeaconBlock, index, blob, proof);
+      miscHelpers.verifyBlobSidecarMerkleProof(blobSidecar);
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class);
+    }
   }
 }

ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDenebTest.java

@@ -188,29 +188,44 @@ void verifyBlobSidecarCompleteness_shouldThrowWhenBlobSidecarIndexIsWrong() {
   }
 
   @Test
-  void shouldComputeValidBlobSidecar() {
+  void shouldConstructValidBlobSidecar() {
     final SignedBeaconBlock signedBeaconBlock =
         dataStructureUtil.randomSignedBeaconBlockWithCommitments(1);
     final Blob blob = dataStructureUtil.randomBlob();
-    final SszKZGCommitment commitment =
+    final SszKZGCommitment expectedCommitment =
         BeaconBlockBodyDeneb.required(signedBeaconBlock.getMessage().getBody())
             .getBlobKzgCommitments()
             .get(0);
     final SszKZGProof proof = dataStructureUtil.randomSszKZGProof();
 
     final BlobSidecar blobSidecar =
-        miscHelpersDeneb.computeBlobSidecar(
-            signedBeaconBlock, UInt64.ZERO, blob, commitment, proof);
+        miscHelpersDeneb.constructBlobSidecar(signedBeaconBlock, UInt64.ZERO, blob, proof);
 
     assertThat(blobSidecar.getIndex()).isEqualTo(UInt64.ZERO);
     assertThat(blobSidecar.getBlob()).isEqualTo(blob);
     assertThat(blobSidecar.getSszKZGProof()).isEqualTo(proof);
-    assertThat(blobSidecar.getSszKZGCommitment()).isEqualTo(commitment);
+    assertThat(blobSidecar.getSszKZGCommitment()).isEqualTo(expectedCommitment);
     assertThat(blobSidecar.getSignedBeaconBlockHeader()).isEqualTo(signedBeaconBlock.asHeader());
     // verify the merkle proof
     assertThat(miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)).isTrue();
   }
 
+  @Test
+  void shouldThrowWhenConstructingBlobSidecarWithInvalidIndex() {
+    final SignedBeaconBlock signedBeaconBlock =
+        dataStructureUtil.randomSignedBeaconBlockWithCommitments(1);
+    final Blob blob = dataStructureUtil.randomBlob();
+    final SszKZGProof proof = dataStructureUtil.randomSszKZGProof();
+
+    assertThatThrownBy(
+            () ->
+                miscHelpersDeneb.constructBlobSidecar(
+                    signedBeaconBlock, UInt64.valueOf(1), blob, proof))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage(
+            "Can't create blob sidecar with index 1 because there are 1 commitment(s) in block");
+  }
+
   @Test
   void verifyBlobSidecarMerkleProofShouldValidate() {
     final int numberOfCommitments = 4;

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

@@ -1209,7 +1209,7 @@ public BeaconBlockBody randomBlindedBeaconBlockBody() {
 
   public BeaconBlockBody randomBlindedBeaconBlockBodyWithCommitments(
       final UInt64 slot, final SszList<SszKZGCommitment> commitments) {
-    BeaconBlockBodySchema<?> schema =
+    final BeaconBlockBodySchema<?> schema =
         spec.atSlot(slot).getSchemaDefinitions().getBlindedBeaconBlockBodySchema();
 
     return schema