Add BlobsBundle consistency checks (Consensys#8132)

momok88 · Mar 22, 2024 · b72675b · b72675b
1 parent bae5006
commit b72675b
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 7 deletions.
diff --git a/.../src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java b/.../src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java
@@ -13,7 +13,8 @@
 
 package tech.pegasys.teku.validator.coordinator;
 
-import com.google.common.base.Preconditions;
+import static com.google.common.base.Preconditions.checkState;
+
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
@@ -451,20 +452,28 @@ public Function<SignedBlockContainer, List<BlobSidecar>> createBlobSidecarsSelec
 
       final SszList<Blob> blobs;
       final SszList<SszKZGProof> proofs;
+      final SszList<SszKZGCommitment> blockCommitments =
+          block.getMessage().getBody().getOptionalBlobKzgCommitments().orElseThrow();
 
       if (blockContainer.isBlinded()) {
         // need to use the builder BlobsBundle for the blinded flow, because the
         // blobs and the proofs wouldn't be part of the BlockContainer
         final tech.pegasys.teku.spec.datastructures.builder.BlobsBundle blobsBundle =
             getCachedBuilderBlobsBundle(slot);
+
+        blobs = blobsBundle.getBlobs();
+        proofs = blobsBundle.getProofs();
+
         // consistency check because the BlobsBundle comes from an external source (a builder)
-        final SszList<SszKZGCommitment> blockCommitments =
-            block.getMessage().getBody().getOptionalBlobKzgCommitments().orElseThrow();
-        Preconditions.checkState(
+        checkState(
             blobsBundle.getCommitments().hashTreeRoot().equals(blockCommitments.hashTreeRoot()),
             "Commitments in the builder BlobsBundle don't match the commitments in the block");
-        blobs = blobsBundle.getBlobs();
-        proofs = blobsBundle.getProofs();
+        checkState(
+            blockCommitments.size() == proofs.size(),
+            "The number of proofs in BlobsBundle doesn't match the number of commitments in the block");
+        checkState(
+            blockCommitments.size() == blobs.size(),
+            "The number of blobs in BlobsBundle doesn't match the number of commitments in the block");
       } else {
         blobs = blockContainer.getBlobs().orElseThrow();
         proofs = blockContainer.getKzgProofs().orElseThrow();

diff --git a/.../test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java b/.../test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java
@@ -17,6 +17,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 import static tech.pegasys.teku.infrastructure.async.SafeFutureAssert.assertThatSafeFuture;
 import static tech.pegasys.teku.infrastructure.async.SafeFutureAssert.safeJoin;
@@ -815,7 +816,7 @@ void shouldCreateBlobSidecarsForBlockContents() {
   }
 
   @Test
-  void shouldFailCreatingBlobSidecarsIfBuilderBlobsBundleIsNotConsistent() {
+  void shouldFailCreatingBlobSidecarsIfBuilderBlobsBundleCommitmentsRootIsNotConsistent() {
     final SszList<SszKZGCommitment> commitments = dataStructureUtil.randomBlobKzgCommitments(3);
     final SignedBeaconBlock signedBlindedBeaconBlock =
         dataStructureUtil.randomSignedBlindedBeaconBlockWithCommitments(commitments);
@@ -834,6 +835,48 @@ void shouldFailCreatingBlobSidecarsIfBuilderBlobsBundleIsNotConsistent() {
             "Commitments in the builder BlobsBundle don't match the commitments in the block");
   }
 
+  @Test
+  void shouldFailCreatingBlobSidecarsIfBuilderBlobsBundleProofsIsNotConsistent() {
+    final SszList<SszKZGCommitment> commitments = dataStructureUtil.randomBlobKzgCommitments(3);
+    final SignedBeaconBlock signedBlindedBeaconBlock =
+        dataStructureUtil.randomSignedBlindedBeaconBlockWithCommitments(commitments);
+
+    final tech.pegasys.teku.spec.datastructures.builder.BlobsBundle blobsBundle =
+        spy(dataStructureUtil.randomBuilderBlobsBundle(commitments));
+    when(blobsBundle.getBlobs()).thenReturn(dataStructureUtil.randomSszBlobs(2));
+
+    prepareCachedBuilderPayload(
+        signedBlindedBeaconBlock.getSlot(),
+        dataStructureUtil.randomExecutionPayload(),
+        Optional.of(blobsBundle));
+
+    assertThatThrownBy(() -> factory.createBlobSidecarsSelector().apply(signedBlindedBeaconBlock))
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessage(
+            "The number of blobs in BlobsBundle doesn't match the number of commitments in the block");
+  }
+
+  @Test
+  void shouldFailCreatingBlobSidecarsIfBuilderBlobsBundleBlobsIsNotConsistent() {
+    final SszList<SszKZGCommitment> commitments = dataStructureUtil.randomBlobKzgCommitments(3);
+    final SignedBeaconBlock signedBlindedBeaconBlock =
+        dataStructureUtil.randomSignedBlindedBeaconBlockWithCommitments(commitments);
+
+    final tech.pegasys.teku.spec.datastructures.builder.BlobsBundle blobsBundle =
+        spy(dataStructureUtil.randomBuilderBlobsBundle(commitments));
+    when(blobsBundle.getProofs()).thenReturn(dataStructureUtil.randomSszKZGProofs(2));
+
+    prepareCachedBuilderPayload(
+        signedBlindedBeaconBlock.getSlot(),
+        dataStructureUtil.randomExecutionPayload(),
+        Optional.of(blobsBundle));
+
+    assertThatThrownBy(() -> factory.createBlobSidecarsSelector().apply(signedBlindedBeaconBlock))
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessage(
+            "The number of proofs in BlobsBundle doesn't match the number of commitments in the block");
+  }
+
   @Test
   void shouldCreateBlobSidecarsForBlindedBlock() {
     final SszList<SszKZGCommitment> commitments = dataStructureUtil.randomBlobKzgCommitments(3);

diff --git a/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java b/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java
@@ -2247,6 +2247,20 @@ public BlobsBundle randomBlobsBundle() {
     return randomBlobsBundle(Optional.empty(), randomSlot());
   }
 
+  public SszList<SszKZGProof> randomSszKZGProofs(final int count) {
+    return randomSszList(
+        getDenebSchemaDefinitions(UInt64.ZERO).getBlobsBundleSchema().getProofsSchema(),
+        this::randomSszKZGProof,
+        count);
+  }
+
+  public SszList<Blob> randomSszBlobs(final int count) {
+    return randomSszList(
+        getDenebSchemaDefinitions(UInt64.ZERO).getBlobsBundleSchema().getBlobsSchema(),
+        this::randomBlob,
+        count);
+  }
+
   public BlobsBundle randomBlobsBundle(final int count) {
     return randomBlobsBundle(Optional.of(count), randomSlot());
   }