Simplify blob sidecar availability checker (#8840)

Author: tbenr

CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Breaking Changes
 
 ### Additions and Improvements
+- Optimized blobs validation pipeline
 
 ### Bug Fixes
 - Updated the gas change check for block building so that warnings only get raised if the change is off spec.

beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/historical/HistoricalBatchFetcher.java

@@ -402,6 +402,7 @@ private void validateBlobSidecars(final SignedBeaconBlock block) {
     final List<BlobSidecar> blobSidecars =
         blobSidecarsBySlotToImport.getOrDefault(
             block.getSlotAndBlockRoot(), Collections.emptyList());
+
     LOG.trace("Validating {} blob sidecars for block {}", blobSidecars.size(), block.getRoot());
     final BlobSidecarsAndValidationResult validationResult =
         blobSidecarManager.createAvailabilityCheckerAndValidateImmediately(block, blobSidecars);

beacon/sync/src/test/java/tech/pegasys/teku/beacon/sync/historical/HistoricalBatchFetcherTest.java

@@ -117,7 +117,7 @@ public void setup() {
             .map(SignedBlockAndState::getBlock)
             .collect(Collectors.toList());
     lastBlockInBatch = chainBuilder.getLatestBlockAndState().getBlock();
-    firstBlockInBatch = blockBatch.get(0);
+    firstBlockInBatch = blockBatch.getFirst();
     blobSidecarsBatch =
         chainBuilder
             .streamBlobSidecars(10, 20)
@@ -202,7 +202,7 @@ public void run_returnAllBlocksAndBlobSidecarsOnFirstRequest() {
             earliestBlobSidecarSlotCaptor.capture());
     assertThat(blockCaptor.getValue()).containsExactlyElementsOf(blockBatch);
     assertThat(blobSidecarCaptor.getValue()).isEqualTo(blobSidecarsBatch);
-    assertThat(earliestBlobSidecarSlotCaptor.getValue()).contains(blockBatch.get(0).getSlot());
+    assertThat(earliestBlobSidecarSlotCaptor.getValue()).contains(blockBatch.getFirst().getSlot());
   }
 
   @Test

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

@@ -98,12 +98,6 @@ public SafeFuture<BlobSidecarsAndValidationResult> getAvailabilityCheckResult()
         return SafeFuture.completedFuture(validateImmediately(block, blobsAndProofs));
       }
 
-      @Override
-      public BlobSidecarsAndValidationResult validateImmediately(
-          final List<BlobSidecar> blobSidecars) {
-        throw new UnsupportedOperationException("Not available in fork choice reference tests");
-      }
-
       private BlobSidecarsAndValidationResult validateImmediately(
           final SignedBeaconBlock block, final BlobsAndProofs blobsAndProofs) {
         final List<KZGCommitment> kzgCommitments =

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

@@ -39,6 +39,10 @@ public class BlobSidecar
         SignedBeaconBlockHeader,
         SszBytes32Vector> {
 
+  private volatile boolean kzgValidated = false;
+  private volatile boolean kzgCommitmentInclusionProofValidated = false;
+  private volatile boolean signatureValidated = false;
+
   BlobSidecar(final BlobSidecarSchema blobSidecarSchema, final TreeNode backingTreeNode) {
     super(blobSidecarSchema, backingTreeNode);
   }
@@ -139,6 +143,30 @@ public String toLogString() {
         getKZGProof().toAbbreviatedString());
   }
 
+  public boolean isKzgValidated() {
+    return kzgValidated;
+  }
+
+  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;
+  }
+
   @Override
   public BlobSidecarSchema getSchema() {
     return (BlobSidecarSchema) super.getSchema();

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

@@ -380,16 +380,22 @@ 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) {
+  public boolean verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock(
+      final List<BlobSidecar> blobSidecars, final SignedBeaconBlock signedBeaconBlock) {
+    return blobSidecars.stream()
+        .allMatch(
+            blobSidecar ->
+                verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock(
+                    blobSidecar, signedBeaconBlock));
+  }
+
+  public boolean verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock(
+      final BlobSidecar blobSidecar, final SignedBeaconBlock signedBeaconBlock) {
     throw new UnsupportedOperationException("No Blob Sidecars before Deneb");
   }
 
   public void verifyBlobSidecarCompleteness(
-      final List<BlobSidecar> verifiedBlobSidecars,
-      final List<KZGCommitment> kzgCommitmentsFromBlock)
+      final List<BlobSidecar> verifiedBlobSidecars, final SignedBeaconBlock signedBeaconBlock)
       throws IllegalArgumentException {
     throw new UnsupportedOperationException("No Blob Sidecars before Deneb");
   }

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

@@ -15,9 +15,7 @@
 
 import static tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsAndValidationResult.NOT_REQUIRED_RESULT_FUTURE;
 
-import java.util.List;
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
-import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar;
 import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadHeader;
 import tech.pegasys.teku.spec.datastructures.execution.NewPayloadRequest;
 import tech.pegasys.teku.spec.logic.versions.bellatrix.block.OptimisticExecutionPayloadExecutor;
@@ -35,12 +33,6 @@ public boolean initiateDataAvailabilityCheck() {
         public SafeFuture<BlobSidecarsAndValidationResult> getAvailabilityCheckResult() {
           return NOT_REQUIRED_RESULT_FUTURE;
         }
-
-        @Override
-        public BlobSidecarsAndValidationResult validateImmediately(
-            final List<BlobSidecar> blobSidecars) {
-          return BlobSidecarsAndValidationResult.NOT_REQUIRED;
-        }
       };
 
   BlobSidecarsAvailabilityChecker NOT_REQUIRED = NOOP;
@@ -55,7 +47,4 @@ public BlobSidecarsAndValidationResult validateImmediately(
   boolean initiateDataAvailabilityCheck();
 
   SafeFuture<BlobSidecarsAndValidationResult> getAvailabilityCheckResult();
-
-  /** Perform the data availability check immediately on the provided blob sidecars */
-  BlobSidecarsAndValidationResult validateImmediately(List<BlobSidecar> blobSidecars);
 }

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

@@ -84,10 +84,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,77 +115,71 @@ 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());
-        });
+    blobSidecars.stream()
+        .filter(blobSidecar -> !blobSidecar.isKzgValidated())
+        .forEach(
+            blobSidecar -> {
+              blobs.add(blobSidecar.getBlob().getBytes());
+              kzgCommitments.add(blobSidecar.getKZGCommitment());
+              kzgProofs.add(blobSidecar.getKZGProof());
+            });
+
+    if (blobs.isEmpty()) {
+      return true;
+    }
+
+    final boolean result = kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, kzgProofs);
 
-    return kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, kzgProofs);
+    if (result) {
+      blobSidecars.stream()
+          .filter(blobSidecar -> !blobSidecar.isKzgValidated())
+          .forEach(BlobSidecar::markKzgAsValidated);
+    }
+
+    return result;
   }
 
-  /**
-   * 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));
-          }
-
-          checkArgument(
-              blobSidecar.getKZGCommitment().equals(kzgCommitmentFromBlock),
-              "Block and blob sidecar kzg commitments mismatch for %s, blob index %s",
-              slotAndBlockRoot,
-              blobIndex);
-        });
+  public boolean verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock(
+      final BlobSidecar blobSidecar, final SignedBeaconBlock signedBeaconBlock) {
+    if (blobSidecar.isSignatureValidated()) {
+      return true;
+    }
+
+    final boolean result =
+        blobSidecar
+            .getSignedBeaconBlockHeader()
+            .hashTreeRoot()
+            .equals(signedBeaconBlock.hashTreeRoot());
+
+    if (result) {
+      blobSidecar.markSignatureAsValidated();
+    }
+
+    return result;
   }
 
   /**
    * 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.
+   * @param signedBeaconBlock block with commitments
    */
   @Override
   public void verifyBlobSidecarCompleteness(
       final List<BlobSidecar> completeVerifiedBlobSidecars,
-      final List<KZGCommitment> kzgCommitmentsFromBlock)
+      final SignedBeaconBlock signedBeaconBlock)
       throws IllegalArgumentException {
+    int commitmentCount =
+        signedBeaconBlock
+            .getBeaconBlock()
+            .map(BeaconBlock::getBody)
+            .flatMap(BeaconBlockBody::getOptionalBlobKzgCommitments)
+            .orElseThrow()
+            .size();
     checkArgument(
-        completeVerifiedBlobSidecars.size() == kzgCommitmentsFromBlock.size(),
-        "Blob sidecars are not complete");
+        completeVerifiedBlobSidecars.size() == commitmentCount, "Blob sidecars are not complete");
 
     IntStream.range(0, completeVerifiedBlobSidecars.size())
         .forEach(
@@ -262,12 +266,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;
   }
 }