Ignore excessive blobSidecars (Consensys#7616)

StefanBratanov · Oct 20, 2023 · 5800509 · 5800509
1 parent 0d4f9eb
commit 5800509
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 8 deletions.
diff --git a/...ition/src/main/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTracker.java b/...ition/src/main/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTracker.java
@@ -56,6 +56,13 @@ public class BlockBlobSidecarsTracker {
 
   private final Optional<Map<UInt64, Long>> maybeDebugTimings;
 
+  /**
+   * {@link BlockBlobSidecarsTracker#add} and {@link BlockBlobSidecarsTracker#setBlock} methods are
+   * assumed to be called from BlobSidecarPool in a synchronized context
+   *
+   * @param slotAndBlockRoot slot and block root to create tracker for
+   * @param maxBlobsPerBlock max number of blobs per block for the slot
+   */
   public BlockBlobSidecarsTracker(
       final SlotAndBlockRoot slotAndBlockRoot, final UInt64 maxBlobsPerBlock) {
     this.slotAndBlockRoot = slotAndBlockRoot;
@@ -128,6 +135,13 @@ public boolean add(final BlobSidecar blobSidecar) {
       return false;
     }
 
+    if (isExcessiveBlobSidecar(blobSidecar)) {
+      LOG.debug(
+          "Ignoring BlobSidecar {} - index is too high with respect to block commitments",
+          blobSidecar::toLogString);
+      return false;
+    }
+
     boolean addedNew = blobSidecars.put(blobSidecar.getIndex(), blobSidecar) == null;
 
     if (addedNew) {
@@ -160,6 +174,7 @@ public boolean setBlock(final SignedBeaconBlock block) {
     maybeDebugTimings.ifPresent(
         debugTimings -> debugTimings.put(BLOCK_ARRIVAL_TIMING_IDX, System.currentTimeMillis()));
 
+    pruneExcessiveBlobSidecars();
     checkCompletion();
 
     return true;
@@ -180,6 +195,17 @@ private void checkCompletion() {
     }
   }
 
+  private void pruneExcessiveBlobSidecars() {
+    getBlockKzgCommitmentsCount()
+        .ifPresent(count -> blobSidecars.tailMap(UInt64.valueOf(count), true).clear());
+  }
+
+  private boolean isExcessiveBlobSidecar(final BlobSidecar blobSidecar) {
+    return getBlockKzgCommitmentsCount()
+        .map(count -> blobSidecar.getIndex().isGreaterThanOrEqualTo(count))
+        .orElse(false);
+  }
+
   public boolean isCompleted() {
     return blobSidecarsComplete.isDone();
   }

diff --git a/...n/src/test/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTrackerTest.java b/...n/src/test/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTrackerTest.java
@@ -167,14 +167,14 @@ void add_shouldWorkTillCompletionWhenAddingBlobsBeforeBlockIsSet() {
     blockBlobSidecarsTracker.add(toAdd);
 
     // we don't know the block, missing blobs are max blobs minus the blob we already have
-    final Set<BlobIdentifier> potentialMissingBlocks =
+    final Set<BlobIdentifier> potentialMissingBlobs =
         UInt64.range(UInt64.valueOf(1), maxBlobsPerBlock.plus(1))
             .map(index -> new BlobIdentifier(slotAndBlockRoot.getBlockRoot(), index))
             .collect(Collectors.toSet());
 
     SafeFutureAssert.assertThatSafeFuture(completionFuture).isNotCompleted();
     assertThat(blockBlobSidecarsTracker.getMissingBlobSidecars())
-        .containsExactlyInAnyOrderElementsOf(potentialMissingBlocks);
+        .containsExactlyInAnyOrderElementsOf(potentialMissingBlobs);
     assertThat(blockBlobSidecarsTracker.getBlobSidecars())
         .containsExactlyInAnyOrderEntriesOf(added);
 
@@ -345,4 +345,55 @@ void getMissingBlobSidecars_shouldRespectMaxBlobsPerBlock() {
     assertThat(blockBlobSidecarsTracker.getMissingBlobSidecars())
         .containsExactlyInAnyOrderElementsOf(knownMissing);
   }
+
+  @Test
+  void shouldNotIgnoreExcessiveBlobSidecarWhenBlockIsUnknownAndWePruneItLater() {
+    final BlockBlobSidecarsTracker blockBlobSidecarsTracker =
+        new BlockBlobSidecarsTracker(slotAndBlockRoot, maxBlobsPerBlock);
+
+    final BlobSidecar legitBlobSidecar = createBlobSidecar(UInt64.valueOf(2));
+    final BlobSidecar excessiveBlobSidecar1 = createBlobSidecar(maxBlobsPerBlock);
+    final BlobSidecar excessiveBlobSidecar2 = createBlobSidecar(maxBlobsPerBlock.plus(1));
+
+    final List<BlobSidecar> toAddAltered =
+        List.of(legitBlobSidecar, excessiveBlobSidecar1, excessiveBlobSidecar2);
+
+    toAddAltered.forEach(
+        blobSidecar -> assertThat(blockBlobSidecarsTracker.add(blobSidecar)).isTrue());
+
+    assertThat(blockBlobSidecarsTracker.getBlobSidecars().values())
+        .containsExactlyInAnyOrderElementsOf(toAddAltered);
+
+    blockBlobSidecarsTracker.setBlock(block);
+
+    assertThat(blockBlobSidecarsTracker.getBlobSidecars().values())
+        .containsExactlyInAnyOrderElementsOf(List.of(legitBlobSidecar));
+  }
+
+  @Test
+  void add_shouldIgnoreExcessiveBlobSidecarWhenBlockIsKnown() {
+    final BlockBlobSidecarsTracker blockBlobSidecarsTracker =
+        new BlockBlobSidecarsTracker(slotAndBlockRoot, maxBlobsPerBlock);
+
+    blockBlobSidecarsTracker.setBlock(block);
+
+    final BlobSidecar legitBlobSidecar = createBlobSidecar(UInt64.valueOf(2));
+    final BlobSidecar excessiveBlobSidecar1 = createBlobSidecar(maxBlobsPerBlock);
+    final BlobSidecar excessiveBlobSidecar2 = createBlobSidecar(maxBlobsPerBlock.plus(1));
+
+    assertThat(blockBlobSidecarsTracker.add(legitBlobSidecar)).isTrue();
+    assertThat(blockBlobSidecarsTracker.add(excessiveBlobSidecar1)).isFalse();
+    assertThat(blockBlobSidecarsTracker.add(excessiveBlobSidecar2)).isFalse();
+
+    assertThat(blockBlobSidecarsTracker.getBlobSidecars().size()).isEqualTo(1);
+  }
+
+  private BlobSidecar createBlobSidecar(final UInt64 index) {
+    return dataStructureUtil
+        .createRandomBlobSidecarBuilder()
+        .blockRoot(slotAndBlockRoot.getBlockRoot())
+        .slot(slotAndBlockRoot.getSlot())
+        .index(index)
+        .build();
+  }
 }
diff --git a/...nsition/src/test/java/tech/pegasys/teku/statetransition/util/BlobSidecarPoolImplTest.java b/...nsition/src/test/java/tech/pegasys/teku/statetransition/util/BlobSidecarPoolImplTest.java
@@ -207,12 +207,7 @@ public void onNewBlobSidecar_shouldIgnoreBlobsForAlreadyImportedBlocks() {
   @Test
   public void onNewBlobSidecarOnNewBlock_addTrackerWithBothBlockAndBlobSidecar() {
     final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(currentSlot);
-    final BlobSidecar blobSidecar =
-        dataStructureUtil
-            .createRandomBlobSidecarBuilder()
-            .slot(currentSlot)
-            .blockRoot(block.getRoot())
-            .build();
+    final BlobSidecar blobSidecar = dataStructureUtil.randomBlobSidecarsForBlock(block).get(0);
 
     blobSidecarPool.onNewBlobSidecar(blobSidecar);
     blobSidecarPool.onNewBlock(block);