change approach

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

@@ -801,7 +801,8 @@ void shouldCreateBlobSidecarsForBlockContents() {
               assertThat(blobSidecar.getSszKZGCommitment())
                   .isEqualTo(expectedCommitments.get(index));
               // verify the merkle proof
-              assertThat(miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)).isTrue();
+              assertThat(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecar))
+                  .isTrue();
             });
   }
 
@@ -921,7 +922,8 @@ void shouldCreateBlobSidecarsForBlindedBlock(final boolean useLocalFallback) {
               assertThat(blobSidecar.getSszKZGCommitment())
                   .isEqualTo(expectedCommitments.get(index));
               // verify the merkle proof
-              assertThat(miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)).isTrue();
+              assertThat(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecar))
+                  .isTrue();
             });
   }
 

data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/events/EventSubscriber.java

@@ -45,7 +45,7 @@ public class EventSubscriber {
   private final AtomicBoolean processingQueue;
   private final AsyncRunner asyncRunner;
   private final AtomicLong excessiveQueueingDisconnectionTime = new AtomicLong(Long.MAX_VALUE);
-  private volatile AtomicInteger successiveFailureCounter = new AtomicInteger(0);
+  private final AtomicInteger successiveFailureCounter = new AtomicInteger(0);
 
   public EventSubscriber(
       final List<String> eventTypes,

eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/StubBlobSidecarManager.java

@@ -95,11 +95,7 @@ public SafeFuture<BlobSidecarsAndValidationResult> getAvailabilityCheckResult()
         if (blobsAndProofs == null) {
           return SafeFuture.completedFuture(BlobSidecarsAndValidationResult.NOT_REQUIRED);
         }
-        try {
-          return SafeFuture.completedFuture(validateImmediately(block, blobsAndProofs));
-        } catch (RuntimeException e) {
-          return SafeFuture.completedFuture(BlobSidecarsAndValidationResult.notAvailable(e));
-        }
+        return SafeFuture.completedFuture(validateImmediately(block, blobsAndProofs));
       }
 
       private BlobSidecarsAndValidationResult validateImmediately(
@@ -111,8 +107,12 @@ private BlobSidecarsAndValidationResult validateImmediately(
                 .map(SszKZGCommitment::getKZGCommitment)
                 .toList();
         final List<Bytes> blobs = blobsAndProofs.blobs.stream().map(Blob::getBytes).toList();
-        if (!kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, blobsAndProofs.proofs)) {
-          throw new RuntimeException("Invalid KZG proof");
+        try {
+          if (!kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, blobsAndProofs.proofs)) {
+            return BlobSidecarsAndValidationResult.invalidResult(Collections.emptyList());
+          }
+        } catch (final Exception ex) {
+          return BlobSidecarsAndValidationResult.invalidResult(Collections.emptyList(), ex);
         }
         return BlobSidecarsAndValidationResult.validResult(Collections.emptyList());
       }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blobs/versions/deneb/BlobSidecar.java

@@ -39,7 +39,8 @@ public class BlobSidecar
         SignedBeaconBlockHeader,
         SszBytes32Vector> {
 
-  private volatile boolean kzgAndInclusionProofValidated = false;
+  private volatile boolean kzgValidated = false;
+  private volatile boolean kzgCommitmentInclusionProofValidated = false;
   private volatile boolean signatureValidated = false;
 
   BlobSidecar(final BlobSidecarSchema blobSidecarSchema, final TreeNode backingTreeNode) {
@@ -142,18 +143,26 @@ public String toLogString() {
         getKZGProof().toAbbreviatedString());
   }
 
-  public boolean isKzgAndInclusionProofValidated() {
-    return kzgAndInclusionProofValidated;
+  public boolean isKzgValidated() {
+    return kzgValidated;
   }
 
-  public void markKzgAndInclusionProofAsValidated() {
-    kzgAndInclusionProofValidated = true;
+  public boolean isKzgCommitmentInclusionProofValidated() {
+    return kzgCommitmentInclusionProofValidated;
   }
 
   public boolean isSignatureValidated() {
     return signatureValidated;
   }
 
+  public void markKzgAsValidated() {
+    kzgValidated = true;
+  }
+
+  public void markKzgCommitmentInclusionProofAsValidated() {
+    kzgCommitmentInclusionProofValidated = true;
+  }
+
   public void markSignatureAsValidated() {
     signatureValidated = true;
   }

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

@@ -380,20 +380,6 @@ public boolean verifyBlobKzgProofBatch(final KZG kzg, final List<BlobSidecar> bl
     return false;
   }
 
-  public void validateBlobSidecarsBatchAgainstBlock(
-      final List<BlobSidecar> blobSidecars,
-      final BeaconBlock block,
-      final List<KZGCommitment> kzgCommitmentsFromBlock) {
-    throw new UnsupportedOperationException("No Blob Sidecars before Deneb");
-  }
-
-  public void verifyBlobSidecarCompleteness(
-      final List<BlobSidecar> verifiedBlobSidecars,
-      final List<KZGCommitment> kzgCommitmentsFromBlock)
-      throws IllegalArgumentException {
-    throw new UnsupportedOperationException("No Blob Sidecars 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/blobs/BlobSidecarsAndValidationResult.java

@@ -43,6 +43,18 @@ public static BlobSidecarsAndValidationResult validResult(final List<BlobSidecar
         BlobSidecarsValidationResult.VALID, blobSidecars, Optional.empty());
   }
 
+  public static BlobSidecarsAndValidationResult invalidResult(
+      final List<BlobSidecar> blobSidecars) {
+    return new BlobSidecarsAndValidationResult(
+        BlobSidecarsValidationResult.INVALID, blobSidecars, Optional.empty());
+  }
+
+  public static BlobSidecarsAndValidationResult invalidResult(
+      final List<BlobSidecar> blobSidecars, final Throwable cause) {
+    return new BlobSidecarsAndValidationResult(
+        BlobSidecarsValidationResult.INVALID, blobSidecars, Optional.of(cause));
+  }
+
   public static BlobSidecarsAndValidationResult notAvailable(final Throwable cause) {
     return new BlobSidecarsAndValidationResult(
         BlobSidecarsValidationResult.NOT_AVAILABLE, Collections.emptyList(), Optional.of(cause));
@@ -77,6 +89,18 @@ public boolean isNotRequired() {
     return validationResult.equals(BlobSidecarsValidationResult.NOT_REQUIRED);
   }
 
+  public boolean isInvalid() {
+    return validationResult.equals(BlobSidecarsValidationResult.INVALID);
+  }
+
+  public boolean isNotAvailable() {
+    return validationResult.equals(BlobSidecarsValidationResult.NOT_AVAILABLE);
+  }
+
+  public boolean isFailure() {
+    return isInvalid() || isNotAvailable();
+  }
+
   public boolean isSuccess() {
     return isValid() || isNotRequired();
   }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsValidationResult.java

@@ -16,5 +16,6 @@
 public enum BlobSidecarsValidationResult {
   NOT_REQUIRED,
   NOT_AVAILABLE,
+  INVALID,
   VALID
 }

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

@@ -13,14 +13,12 @@
 
 package tech.pegasys.teku.spec.logic.versions.deneb.helpers;
 
-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.NoSuchElementException;
 import java.util.Optional;
-import java.util.stream.IntStream;
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
 import tech.pegasys.teku.infrastructure.crypto.Hash;
@@ -35,7 +33,6 @@
 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.blobs.versions.deneb.BlobSidecarSchema;
-import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
 import tech.pegasys.teku.spec.datastructures.blocks.blockbody.BeaconBlockBody;
 import tech.pegasys.teku.spec.datastructures.blocks.blockbody.versions.deneb.BeaconBlockBodySchemaDeneb;
@@ -84,10 +81,20 @@ public MiscHelpersDeneb(
    */
   @Override
   public boolean verifyBlobKzgProof(final KZG kzg, final BlobSidecar blobSidecar) {
-    return kzg.verifyBlobKzgProof(
-        blobSidecar.getBlob().getBytes(),
-        blobSidecar.getKZGCommitment(),
-        blobSidecar.getKZGProof());
+    if (blobSidecar.isKzgValidated()) {
+      return true;
+    }
+    final boolean result =
+        kzg.verifyBlobKzgProof(
+            blobSidecar.getBlob().getBytes(),
+            blobSidecar.getKZGCommitment(),
+            blobSidecar.getKZGProof());
+
+    if (result) {
+      blobSidecar.markKzgAsValidated();
+    }
+
+    return result;
   }
 
   /**
@@ -105,90 +112,28 @@ public boolean verifyBlobKzgProofBatch(final KZG kzg, final List<BlobSidecar> bl
     final List<KZGCommitment> kzgCommitments = new ArrayList<>();
     final List<KZGProof> kzgProofs = new ArrayList<>();
 
-    blobSidecars.forEach(
-        blobSidecar -> {
-          blobs.add(blobSidecar.getBlob().getBytes());
-          kzgCommitments.add(blobSidecar.getKZGCommitment());
-          kzgProofs.add(blobSidecar.getKZGProof());
-        });
-
-    return kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, kzgProofs);
-  }
-
-  /**
-   * Validates blob sidecars against block. We need to check block root and kzg commitment, it's
-   * enough to guarantee BlobSidecars belong to block
-   *
-   * @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(
-              blobSidecar.getBlockRoot().equals(block.getRoot()),
-              "Block and blob sidecar root 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));
-          }
+    blobSidecars.stream()
+        .filter(blobSidecar -> !blobSidecar.isKzgValidated())
+        .forEach(
+            blobSidecar -> {
+              blobs.add(blobSidecar.getBlob().getBytes());
+              kzgCommitments.add(blobSidecar.getKZGCommitment());
+              kzgProofs.add(blobSidecar.getKZGProof());
+            });
 
-          checkArgument(
-              blobSidecar.getKZGCommitment().equals(kzgCommitmentFromBlock),
-              "Block and blob sidecar kzg commitments mismatch for %s, blob index %s",
-              slotAndBlockRoot,
-              blobIndex);
-        });
-  }
+    if (blobs.isEmpty()) {
+      return true;
+    }
 
-  /**
-   * Verifies that blob sidecars are complete and with expected indexes
-   *
-   * @param completeVerifiedBlobSidecars blob sidecars to verify, It is assumed that it is an
-   *     ordered list based on BlobSidecar index
-   * @param kzgCommitmentsFromBlock kzg commitments from block.
-   */
-  @Override
-  public void verifyBlobSidecarCompleteness(
-      final List<BlobSidecar> completeVerifiedBlobSidecars,
-      final List<KZGCommitment> kzgCommitmentsFromBlock)
-      throws IllegalArgumentException {
-    checkArgument(
-        completeVerifiedBlobSidecars.size() == kzgCommitmentsFromBlock.size(),
-        "Blob sidecars are not complete");
+    final boolean result = kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, kzgProofs);
 
-    IntStream.range(0, completeVerifiedBlobSidecars.size())
-        .forEach(
-            index -> {
-              final BlobSidecar blobSidecar = completeVerifiedBlobSidecars.get(index);
-              final UInt64 blobIndex = blobSidecar.getIndex();
+    if (result) {
+      blobSidecars.stream()
+          .filter(blobSidecar -> !blobSidecar.isKzgValidated())
+          .forEach(BlobSidecar::markKzgAsValidated);
+    }
 
-              checkArgument(
-                  blobIndex.longValue() == index,
-                  "Blob sidecar index mismatch, expected %s, got %s",
-                  index,
-                  blobIndex);
-            });
+    return result;
   }
 
   /**
@@ -262,12 +207,22 @@ public BlobSidecar constructBlobSidecar(
         index, blob, commitment, proof, signedBeaconBlock.asHeader(), kzgCommitmentInclusionProof);
   }
 
-  public boolean verifyBlobSidecarMerkleProof(final BlobSidecar blobSidecar) {
-    return predicates.isValidMerkleBranch(
-        blobSidecar.getSszKZGCommitment().hashTreeRoot(),
-        blobSidecar.getKzgCommitmentInclusionProof(),
-        SpecConfigDeneb.required(specConfig).getKzgCommitmentInclusionProofDepth(),
-        getBlobSidecarKzgCommitmentGeneralizedIndex(blobSidecar.getIndex()),
-        blobSidecar.getBlockBodyRoot());
+  public boolean verifyBlobKzgCommitmentInclusionProof(final BlobSidecar blobSidecar) {
+    if (blobSidecar.isKzgCommitmentInclusionProofValidated()) {
+      return true;
+    }
+    final boolean result =
+        predicates.isValidMerkleBranch(
+            blobSidecar.getSszKZGCommitment().hashTreeRoot(),
+            blobSidecar.getKzgCommitmentInclusionProof(),
+            SpecConfigDeneb.required(specConfig).getKzgCommitmentInclusionProofDepth(),
+            getBlobSidecarKzgCommitmentGeneralizedIndex(blobSidecar.getIndex()),
+            blobSidecar.getBlockBodyRoot());
+
+    if (result) {
+      blobSidecar.markKzgCommitmentInclusionProofAsValidated();
+    }
+
+    return result;
   }
 }

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

@@ -119,7 +119,7 @@ void fuzzConstructBlobSidecarAndVerifyMerkleProof(
     try {
       final BlobSidecar blobSidecar =
           miscHelpers.constructBlobSidecar(signedBeaconBlock, index, blob, proof);
-      miscHelpers.verifyBlobSidecarMerkleProof(blobSidecar);
+      miscHelpers.verifyBlobKzgCommitmentInclusionProof(blobSidecar);
     } catch (Exception e) {
       assertThat(e).isInstanceOf(IllegalArgumentException.class);
     }