Dump blob sidecars which failed data availability (#8376)

StefanBratanov · web-flow · commit fb21b1a17353 · 2024-06-18T19:01:37.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,8 @@ the [releases page](https://github.com/Consensys/teku/releases).
 
 ### Breaking Changes
 
+- Renamed `--Xp2p-dumps-to-file-enabled` hidden CLI option to `--Xdebug-data-dumping-enabled`
+
 ### Additions and Improvements
 
 - Added metadata fields to `/eth/v1/beacon/blob_sidecars/{block_id}` Beacon API response as per https://github.com/ethereum/beacon-APIs/pull/441
diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoice.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoice.java
@@ -17,6 +17,7 @@
 import static tech.pegasys.teku.infrastructure.logging.P2PLogger.P2P_LOG;
 import static tech.pegasys.teku.infrastructure.time.TimeUtilities.secondsToMillis;
 import static tech.pegasys.teku.spec.constants.NetworkConstants.INTERVALS_PER_SLOT;
+import static tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsValidationResult.INVALID;
 import static tech.pegasys.teku.statetransition.forkchoice.StateRootCollector.addParentStateRoots;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -540,21 +541,20 @@ private BlockImportResult importBlockAndState(
           payloadResult.getFailureCause().orElseThrow());
     }
 
+    LOG.debug("blobSidecars validation result: {}", blobSidecarsAndValidationResult::toLogString);
+
     switch (blobSidecarsAndValidationResult.getValidationResult()) {
-      case VALID, NOT_REQUIRED -> LOG.debug(
-          "blobSidecars validation result: {}", blobSidecarsAndValidationResult::toLogString);
       case NOT_AVAILABLE -> {
-        LOG.debug(
-            "blobSidecars validation result: {}", blobSidecarsAndValidationResult::toLogString);
         return BlockImportResult.failedDataAvailabilityCheckNotAvailable(
             blobSidecarsAndValidationResult.getCause());
       }
       case INVALID -> {
-        LOG.error(
-            "blobSidecars validation result: {}", blobSidecarsAndValidationResult::toLogString);
+        debugDataDumper.saveInvalidBlobSidecars(
+            blobSidecarsAndValidationResult.getBlobSidecars(), block);
         return BlockImportResult.failedDataAvailabilityCheckInvalid(
             blobSidecarsAndValidationResult.getCause());
       }
+      default -> {}
     }
 
     final ForkChoiceStrategy forkChoiceStrategy = getForkChoiceStrategy();
diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/DebugDataDumper.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/DebugDataDumper.java
@@ -13,10 +13,12 @@
 
 package tech.pegasys.teku.statetransition.util;
 
+import java.util.List;
 import java.util.Optional;
 import java.util.function.Supplier;
 import org.apache.tuweni.bytes.Bytes;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
+import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
 
 public interface DebugDataDumper {
@@ -42,6 +44,10 @@ public void saveInvalidBlock(
             final SignedBeaconBlock block,
             final String failureReason,
             final Optional<Throwable> failureCause) {}
+
+        @Override
+        public void saveInvalidBlobSidecars(
+            final List<BlobSidecar> blobSidecars, final SignedBeaconBlock block) {}
       };
 
   void saveGossipMessageDecodingError(
@@ -58,4 +64,6 @@ void saveGossipRejectedMessage(
 
   void saveInvalidBlock(
       SignedBeaconBlock block, String failureReason, Optional<Throwable> failureCause);
+
+  void saveInvalidBlobSidecars(List<BlobSidecar> blobSidecars, SignedBeaconBlock block);
 }
diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/DebugDataFileDumper.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/DebugDataFileDumper.java
@@ -23,6 +23,7 @@
 import java.sql.Date;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
+import java.util.List;
 import java.util.Optional;
 import java.util.function.Supplier;
 import org.apache.logging.log4j.LogManager;
@@ -31,6 +32,7 @@
 import org.apache.tuweni.bytes.Bytes32;
 import tech.pegasys.teku.infrastructure.time.TimeProvider;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
+import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
 
 public class DebugDataFileDumper implements DebugDataDumper {
@@ -41,6 +43,7 @@ public class DebugDataFileDumper implements DebugDataDumper {
   private static final String DECODING_ERROR_SUB_DIR = "decoding_error";
   private static final String REJECTED_SUB_DIR = "rejected";
   private static final String INVALID_BLOCK_DIR = "invalid_blocks";
+  private static final String INVALID_BLOB_SIDECARS_DIR = "invalid_blob_sidecars";
 
   private boolean enabled;
   private final Path directory;
@@ -125,14 +128,41 @@ public void saveInvalidBlock(
             "invalid block", Path.of(INVALID_BLOCK_DIR).resolve(fileName), block.sszSerialize());
     if (success) {
       LOG.warn(
-          "Rejecting invalid block at slot {} with root {} because {}",
+          "Rejecting invalid block at slot {} with root {}, reason: {}, cause: {}",
           slot,
           blockRoot,
           failureReason,
           failureCause.orElse(null));
     }
   }
 
+  @Override
+  public void saveInvalidBlobSidecars(
+      final List<BlobSidecar> blobSidecars, final SignedBeaconBlock block) {
+    if (!enabled) {
+      return;
+    }
+    final String kzgCommitmentsFileName =
+        String.format(
+            "%s_%s_kzg_commitments.ssz", block.getSlot(), block.getRoot().toUnprefixedHexString());
+    saveBytesToFile(
+        "kzg commitments",
+        Path.of(INVALID_BLOB_SIDECARS_DIR).resolve(kzgCommitmentsFileName),
+        block.getMessage().getBody().getOptionalBlobKzgCommitments().orElseThrow().sszSerialize());
+    blobSidecars.forEach(
+        blobSidecar -> {
+          final UInt64 slot = blobSidecar.getSlot();
+          final Bytes32 blockRoot = blobSidecar.getBlockRoot();
+          final UInt64 index = blobSidecar.getIndex();
+          final String fileName =
+              String.format("%s_%s_%s.ssz", slot, blockRoot.toUnprefixedHexString(), index);
+          saveBytesToFile(
+              "blob sidecar",
+              Path.of(INVALID_BLOB_SIDECARS_DIR).resolve(fileName),
+              blobSidecar.sszSerialize());
+        });
+  }
+
   @VisibleForTesting
   boolean saveBytesToFile(
       final String description, final Path relativeFilePath, final Bytes bytes) {
diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceTest.java
@@ -247,6 +247,31 @@ void onBlock_shouldFailIfBlobsAreNotAvailable() {
     verify(blobSidecarsAvailabilityChecker).getAvailabilityCheckResult();
   }
 
+  @Test
+  void onBlock_shouldFailIfBlobsAreInvalid() {
+    setupWithSpec(TestSpecFactory.createMinimalDeneb());
+    final SignedBlockAndState blockAndState = chainBuilder.generateBlockAtSlot(ONE);
+    storageSystem.chainUpdater().advanceCurrentSlotToAtLeast(blockAndState.getSlot());
+    final List<BlobSidecar> blobSidecars =
+        storageSystem
+            .chainStorage()
+            .getBlobSidecarsBySlotAndBlockRoot(blockAndState.getSlotAndBlockRoot())
+            .join();
+
+    when(blobSidecarsAvailabilityChecker.getAvailabilityCheckResult())
+        .thenReturn(
+            SafeFuture.completedFuture(
+                BlobSidecarsAndValidationResult.invalidResult(blobSidecars)));
+
+    importBlockAndAssertFailure(
+        blockAndState, FailureReason.FAILED_DATA_AVAILABILITY_CHECK_INVALID);
+
+    verify(blobSidecarManager).createAvailabilityChecker(blockAndState.getBlock());
+    verify(blobSidecarsAvailabilityChecker).initiateDataAvailabilityCheck();
+    verify(blobSidecarsAvailabilityChecker).getAvailabilityCheckResult();
+    verify(debugDataDumper).saveInvalidBlobSidecars(blobSidecars, blockAndState.getBlock());
+  }
+
   @Test
   void onBlock_consensusValidationShouldNotResolveWhenDataAvailabilityFails() {
     setupWithSpec(TestSpecFactory.createMinimalDeneb());
diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/DebugDataFileDumperTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/DebugDataFileDumperTest.java
@@ -24,6 +24,7 @@
 import java.sql.Date;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
+import java.util.List;
 import java.util.Optional;
 import org.apache.tuweni.bytes.Bytes;
 import org.junit.jupiter.api.Test;
@@ -33,12 +34,13 @@
 import tech.pegasys.teku.infrastructure.time.StubTimeProvider;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 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.util.DataStructureUtil;
 
 class DebugDataFileDumperTest {
   final DataStructureUtil dataStructureUtil =
-      new DataStructureUtil(TestSpecFactory.createDefault());
+      new DataStructureUtil(TestSpecFactory.createMinimalDeneb());
   private final StubTimeProvider timeProvider = StubTimeProvider.withTimeInSeconds(10_000);
 
   @Test
@@ -91,6 +93,35 @@ void saveInvalidBlockToFile_shouldSaveToFile(@TempDir final Path tempDir) {
     checkBytesSavedToFile(expectedFile, block.sszSerialize());
   }
 
+  @Test
+  void saveInvalidBlobSidecars_shouldSaveToFiles(@TempDir final Path tempDir) {
+    final DebugDataFileDumper dumper = new DebugDataFileDumper(tempDir);
+    final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock();
+    final List<BlobSidecar> blobSidecars = dataStructureUtil.randomBlobSidecarsForBlock(block);
+    dumper.saveInvalidBlobSidecars(blobSidecars, block);
+
+    final String kzgCommitmentsFileName =
+        String.format(
+            "%s_%s_kzg_commitments.ssz", block.getSlot(), block.getRoot().toUnprefixedHexString());
+    final Path expectedKzgCommitmentsFileName =
+        tempDir.resolve("invalid_blob_sidecars").resolve(kzgCommitmentsFileName);
+    checkBytesSavedToFile(
+        expectedKzgCommitmentsFileName,
+        block.getMessage().getBody().getOptionalBlobKzgCommitments().orElseThrow().sszSerialize());
+
+    blobSidecars.forEach(
+        blobSidecar -> {
+          final String fileName =
+              String.format(
+                  "%s_%s_%s.ssz",
+                  blobSidecar.getSlot(),
+                  blobSidecar.getBlockRoot().toUnprefixedHexString(),
+                  blobSidecar.getIndex());
+          final Path expectedFile = tempDir.resolve("invalid_blob_sidecars").resolve(fileName);
+          checkBytesSavedToFile(expectedFile, blobSidecar.sszSerialize());
+        });
+  }
+
   @Test
   void saveBytesToFile_shouldNotThrowExceptionWhenNoDirectory(@TempDir final Path tempDir) {
     final DebugDataFileDumper dumper = new DebugDataFileDumper(tempDir);
