Improve isDataAvailable checks

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/helpers/MiscHelpers.java

@@ -343,18 +343,27 @@ public boolean isExecutionEnabled(final BeaconState genericState, final BeaconBl
     return false;
   }
 
-  /**
-   * Performs data availability by validating the blobs against the kzg commitments from the block.
-   * It will also check the slot and blockRoot consistency.
-   */
-  public boolean isDataAvailable(
-      final UInt64 slot,
-      final Bytes32 beaconBlockRoot,
-      final List<KZGCommitment> kzgCommitments,
-      final List<BlobSidecar> blobSidecars) {
+  public boolean isDataAvailable(final List<BlobSidecar> blobSidecars, final BeaconBlock block) {
     return false;
   }
 
+  public boolean verifyBlobKzgProofBatch(final List<BlobSidecar> blobSidecars) {
+    return false;
+  }
+
+  public void validateBlobSidecarsBatchAgainstBlock(
+      final List<BlobSidecar> blobSidecars,
+      final BeaconBlock block,
+      final List<KZGCommitment> kzgCommitmentsFromBlock) {
+    throw new UnsupportedOperationException("Blob Sidecars before Deneb");
+  }
+
+  public void verifyBlobSidecarCompleteness(
+      final List<BlobSidecar> verifiedBlobSidecars,
+      final List<KZGCommitment> kzgCommitmentsFromBlock) {
+    throw new UnsupportedOperationException("No KZGCommitments before Deneb");
+  }
+
   public VersionedHash kzgCommitmentToVersionedHash(final KZGCommitment kzgCommitment) {
     throw new UnsupportedOperationException("No KZGCommitments before Deneb");
   }

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

@@ -16,10 +16,10 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static tech.pegasys.teku.spec.config.SpecConfigDeneb.VERSIONED_HASH_VERSION_KZG;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import org.apache.tuweni.bytes.Bytes;
-import org.apache.tuweni.bytes.Bytes32;
 import tech.pegasys.teku.infrastructure.crypto.Hash;
 import tech.pegasys.teku.infrastructure.ssz.SszList;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
@@ -31,8 +31,10 @@
 import tech.pegasys.teku.spec.config.SpecConfigDeneb;
 import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.Blob;
 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.blocks.blockbody.versions.deneb.BeaconBlockBodyDeneb;
+import tech.pegasys.teku.spec.datastructures.type.SszKZGCommitment;
 import tech.pegasys.teku.spec.logic.versions.capella.helpers.MiscHelpersCapella;
 import tech.pegasys.teku.spec.logic.versions.deneb.types.VersionedHash;
 
@@ -58,33 +60,121 @@ private static KZG initKZG(final SpecConfigDeneb config) {
   }
 
   /**
-   * <a
+   * Performs complete data availability check <a
    * href="https://github.com/ethereum/consensus-specs/blob/dev/specs/deneb/fork-choice.md#is_data_available">is_data_available</a>
+   *
+   * <p>In Deneb we don't need to retrieve data, everything is already available via blob sidecars.
    */
   @Override
-  public boolean isDataAvailable(
-      final UInt64 slot,
-      final Bytes32 beaconBlockRoot,
-      final List<KZGCommitment> kzgCommitments,
-      final List<BlobSidecar> blobSidecars) {
+  public boolean isDataAvailable(final List<BlobSidecar> blobSidecars, final BeaconBlock block) {
+
+    final List<KZGCommitment> kzgCommitmentsFromBlock =
+        BeaconBlockBodyDeneb.required(block.getBody()).getBlobKzgCommitments().stream()
+            .map(SszKZGCommitment::getKZGCommitment)
+            .toList();
+
+    validateBlobSidecarsBatchAgainstBlock(blobSidecars, block, kzgCommitmentsFromBlock);
+
+    if (!verifyBlobKzgProofBatch(blobSidecars)) {
+      return false;
+    }
+
+    verifyBlobSidecarCompleteness(blobSidecars, kzgCommitmentsFromBlock);
+
+    return true;
+  }
+
+  /**
+   * Performs a verifyBlobKzgProofBatch on the given blob sidecars
+   *
+   * @param blobSidecars blob sidecars to verify, can be a partial set
+   * @return true if all blob sidecars are valid
+   */
+  @Override
+  public boolean verifyBlobKzgProofBatch(final List<BlobSidecar> blobSidecars) {
+    final List<Bytes> blobs = new ArrayList<>();
+    final List<KZGProof> kzgProofs = new ArrayList<>();
+    final List<KZGCommitment> kzgCommitments = new ArrayList<>();
+
     blobSidecars.forEach(
         blobSidecar -> {
+          blobs.add(blobSidecar.getBlob().getBytes());
+          kzgProofs.add(blobSidecar.getKZGProof());
+          kzgCommitments.add(blobSidecar.getKZGCommitment());
+        });
+
+    return kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, kzgProofs);
+  }
+
+  /**
+   * Validates blob sidecars against block by matching all fields they have in common
+   *
+   * @param blobSidecars blob sidecars to validate
+   * @param block block to validate blob sidecar against
+   * @param kzgCommitmentsFromBlock kzg commitments from block. They could be extracted from block
+   *     but since we already have them we can avoid extracting them again.
+   */
+  @Override
+  public void validateBlobSidecarsBatchAgainstBlock(
+      final List<BlobSidecar> blobSidecars,
+      final BeaconBlock block,
+      final List<KZGCommitment> kzgCommitmentsFromBlock) {
+
+    final String slotAndBlockRoot = block.getSlotAndBlockRoot().toLogString();
+
+    blobSidecars.forEach(
+        blobSidecar -> {
+          final UInt64 blobIndex = blobSidecar.getIndex();
+
           checkArgument(
-              slot.equals(blobSidecar.getSlot()),
-              "Blob sidecar slot %s does not match block slot %s",
-              blobSidecar.getSlot(),
-              slot);
+              blobSidecar.getBlockRoot().equals(block.getRoot()),
+              "Block and blob sidecar slot mismatch for %s, blob index %s",
+              slotAndBlockRoot,
+              blobIndex);
+
+          checkArgument(
+              blobSidecar.getSlot().equals(block.getSlot()),
+              "Block and blob sidecar slot mismatch for %s, blob index %s",
+              slotAndBlockRoot,
+              blobIndex);
+
+          checkArgument(
+              blobSidecar.getProposerIndex().equals(block.getProposerIndex()),
+              "Block and blob sidecar proposer index mismatch for %s, blob index %s",
+              slotAndBlockRoot,
+              blobIndex);
           checkArgument(
-              beaconBlockRoot.equals(blobSidecar.getBlockRoot()),
-              "Blob sidecar block root %s does not match block root %s",
-              blobSidecar.getBlockRoot(),
-              beaconBlockRoot);
+              blobSidecar.getBlockParentRoot().equals(block.getParentRoot()),
+              "Block and blob sidecar parent block mismatch for %s, blob index %s",
+              slotAndBlockRoot,
+              blobIndex);
+
+          final KZGCommitment kzgCommitmentFromBlock;
+
+          try {
+            kzgCommitmentFromBlock = kzgCommitmentsFromBlock.get(blobIndex.intValue());
+          } catch (IndexOutOfBoundsException e) {
+            throw new IllegalArgumentException(
+                String.format(
+                    "Blob sidecar index out of bound with respect to block %s, blob index %s",
+                    slotAndBlockRoot, blobIndex));
+          }
+
+          checkArgument(
+              blobSidecar.getKZGCommitment().equals(kzgCommitmentFromBlock),
+              "Block and blob sidecar kzg commitments mismatch for %s, blob index %s",
+              slotAndBlockRoot,
+              blobIndex);
         });
-    final List<Bytes> blobs =
-        blobSidecars.stream().map(BlobSidecar::getBlob).map(Blob::getBytes).toList();
-    final List<KZGProof> proofs = blobSidecars.stream().map(BlobSidecar::getKZGProof).toList();
+  }
 
-    return kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, proofs);
+  @Override
+  public void verifyBlobSidecarCompleteness(
+      final List<BlobSidecar> verifiedBlobSidecars,
+      final List<KZGCommitment> kzgCommitmentsFromBlock) {
+    checkArgument(
+        verifiedBlobSidecars.size() == kzgCommitmentsFromBlock.size(),
+        "Blob sidecars are not complete");
   }
 
   /**

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

@@ -20,16 +20,14 @@
 import net.jqwik.api.ForAll;
 import net.jqwik.api.From;
 import net.jqwik.api.Property;
-import org.apache.tuweni.bytes.Bytes32;
-import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 import tech.pegasys.teku.kzg.KZGCommitment;
 import tech.pegasys.teku.spec.config.SpecConfigDeneb;
 import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar;
+import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock;
 import tech.pegasys.teku.spec.propertytest.suppliers.SpecSupplier;
 import tech.pegasys.teku.spec.propertytest.suppliers.blobs.versions.deneb.BlobSidecarSupplier;
-import tech.pegasys.teku.spec.propertytest.suppliers.type.Bytes32Supplier;
+import tech.pegasys.teku.spec.propertytest.suppliers.blocks.versions.deneb.BeaconBlockSupplier;
 import tech.pegasys.teku.spec.propertytest.suppliers.type.KZGCommitmentSupplier;
-import tech.pegasys.teku.spec.propertytest.suppliers.type.UInt64Supplier;
 
 public class MiscHelpersDenebPropertyTest {
   private final SpecConfigDeneb specConfig =
@@ -42,12 +40,10 @@ public class MiscHelpersDenebPropertyTest {
 
   @Property(tries = 100)
   void fuzzIsDataAvailable(
-      @ForAll(supplier = UInt64Supplier.class) final UInt64 slot,
-      @ForAll(supplier = Bytes32Supplier.class) final Bytes32 beaconBlockRoot,
-      @ForAll final List<@From(supplier = KZGCommitmentSupplier.class) KZGCommitment> commitments,
-      @ForAll final List<@From(supplier = BlobSidecarSupplier.class) BlobSidecar> blobSidecars) {
+      @ForAll final List<@From(supplier = BlobSidecarSupplier.class) BlobSidecar> blobSidecars,
+      @ForAll(supplier = BeaconBlockSupplier.class) final BeaconBlock block) {
     try {
-      miscHelpers.isDataAvailable(slot, beaconBlockRoot, commitments, blobSidecars);
+      miscHelpers.isDataAvailable(blobSidecars, block);
     } catch (Exception e) {
       assertThat(e).isInstanceOf(IllegalArgumentException.class);
     }

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

@@ -14,14 +14,24 @@
 package tech.pegasys.teku.spec.logic.versions.deneb.helpers;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static tech.pegasys.teku.spec.config.SpecConfigDeneb.VERSIONED_HASH_VERSION_KZG;
 
+import java.util.List;
 import org.apache.tuweni.bytes.Bytes32;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 import tech.pegasys.teku.kzg.KZGCommitment;
 import tech.pegasys.teku.spec.Spec;
 import tech.pegasys.teku.spec.TestSpecFactory;
+import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar;
+import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
+import tech.pegasys.teku.spec.datastructures.blocks.blockbody.versions.deneb.BeaconBlockBodyDeneb;
+import tech.pegasys.teku.spec.datastructures.type.SszKZGCommitment;
 import tech.pegasys.teku.spec.logic.versions.deneb.types.VersionedHash;
+import tech.pegasys.teku.spec.util.DataStructureUtil;
 
 class MiscHelpersDenebTest {
 
@@ -34,6 +44,7 @@ class MiscHelpersDenebTest {
   private final Spec spec = TestSpecFactory.createMinimalDeneb();
   private final MiscHelpersDeneb miscHelpersDeneb =
       new MiscHelpersDeneb(spec.getGenesisSpecConfig().toVersionDeneb().orElseThrow());
+  private final DataStructureUtil dataStructureUtil = new DataStructureUtil(spec);
 
   @Test
   public void versionedHash() {
@@ -43,4 +54,95 @@ public void versionedHash() {
                 "0x85d1edf1ee88f68260e750abb2c766398ad1125d4e94e1de04034075ccbd2bb79c5689b952ef15374fd03ca2b2475371"));
     assertThat(actual).isEqualTo(VERSIONED_HASH);
   }
+
+  @Test
+  void validateBlobSidecarsAgainstBlock_shouldNotThrowOnValidBlobSidecar() {
+    final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock();
+    final List<BlobSidecar> blobSidecars = dataStructureUtil.randomBlobSidecarsForBlock(block);
+
+    // make sure we are testing something
+    assertThat(blobSidecars).isNotEmpty();
+
+    miscHelpersDeneb.validateBlobSidecarsBatchAgainstBlock(
+        blobSidecars,
+        block.getMessage(),
+        BeaconBlockBodyDeneb.required(block.getMessage().getBody()).getBlobKzgCommitments().stream()
+            .map(SszKZGCommitment::getKZGCommitment)
+            .toList());
+  }
+
+  @ParameterizedTest
+  @EnumSource(value = BlobSidecarsAlteration.class)
+  void validateBlobSidecarsAgainstBlock_shouldThrowOnBlobSidecarNotMatching(
+      final BlobSidecarsAlteration blobSidecarsAlteration) {
+
+    final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock();
+
+    final List<KZGCommitment> kzgCommitments =
+        BeaconBlockBodyDeneb.required(block.getMessage().getBody()).getBlobKzgCommitments().stream()
+            .map(SszKZGCommitment::getKZGCommitment)
+            .toList();
+
+    // make sure we are testing something
+    assertThat(kzgCommitments).isNotEmpty();
+
+    final int indexToBeAltered =
+        Math.toIntExact(dataStructureUtil.randomPositiveLong(kzgCommitments.size()));
+
+    // let's create blobs with only one altered with the given alteration
+    final List<BlobSidecar> blobSidecars =
+        dataStructureUtil.randomBlobSidecarsForBlock(
+            block,
+            (index, randomBlobSidecarBuilder) -> {
+              if (!index.equals(indexToBeAltered)) {
+                return randomBlobSidecarBuilder;
+              }
+
+              return switch (blobSidecarsAlteration) {
+                case BLOB_SIDECAR_PROPOSER_INDEX -> randomBlobSidecarBuilder.proposerIndex(
+                    block.getProposerIndex().plus(1));
+                case BLOB_SIDECAR_INDEX -> randomBlobSidecarBuilder.index(
+                    UInt64.valueOf(kzgCommitments.size())); // out of bounds
+                case BLOB_SIDECAR_PARENT_ROOT -> randomBlobSidecarBuilder.blockParentRoot(
+                    block.getParentRoot().not());
+                case BLOB_SIDECAR_KZG_COMMITMENT -> randomBlobSidecarBuilder.kzgCommitment(
+                    BeaconBlockBodyDeneb.required(block.getMessage().getBody())
+                        .getBlobKzgCommitments()
+                        .get(index)
+                        .getBytes()
+                        .not());
+              };
+            });
+
+    assertThatThrownBy(
+            () ->
+                miscHelpersDeneb.validateBlobSidecarsBatchAgainstBlock(
+                    blobSidecars,
+                    block.getMessage(),
+                    BeaconBlockBodyDeneb.required(block.getMessage().getBody())
+                        .getBlobKzgCommitments()
+                        .stream()
+                        .map(SszKZGCommitment::getKZGCommitment)
+                        .toList()))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageStartingWith(
+            switch (blobSidecarsAlteration) {
+              case BLOB_SIDECAR_PROPOSER_INDEX -> "Block and blob sidecar proposer index mismatch";
+              case BLOB_SIDECAR_INDEX -> "Blob sidecar index out of bound with respect to block";
+              case BLOB_SIDECAR_PARENT_ROOT -> "Block and blob sidecar parent block mismatch";
+              case BLOB_SIDECAR_KZG_COMMITMENT -> "Block and blob sidecar kzg commitments mismatch";
+            })
+        .hasMessageEndingWith(
+            "blob index %s",
+            blobSidecarsAlteration == BlobSidecarsAlteration.BLOB_SIDECAR_INDEX
+                ? kzgCommitments.size() // out of bounds altered index
+                : indexToBeAltered);
+  }
+
+  private enum BlobSidecarsAlteration {
+    BLOB_SIDECAR_PROPOSER_INDEX,
+    BLOB_SIDECAR_INDEX,
+    BLOB_SIDECAR_PARENT_ROOT,
+    BLOB_SIDECAR_KZG_COMMITMENT,
+  }
 }

ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/propertytest/suppliers/blocks/versions/deneb/BeaconBlockSupplier.java

@@ -0,0 +1,25 @@
+/*
+ * Copyright Consensys Software Inc., 2023
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+
+package tech.pegasys.teku.spec.propertytest.suppliers.blocks.versions.deneb;
+
+import tech.pegasys.teku.spec.SpecMilestone;
+import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock;
+import tech.pegasys.teku.spec.propertytest.suppliers.DataStructureUtilSupplier;
+import tech.pegasys.teku.spec.util.DataStructureUtil;
+
+public class BeaconBlockSupplier extends DataStructureUtilSupplier<BeaconBlock> {
+  public BeaconBlockSupplier() {
+    super(DataStructureUtil::randomBeaconBlock, SpecMilestone.DENEB);
+  }
+}