From 5f7c001f8760884dc4192acb38792f0ae8eaaac8 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Thu, 13 Jun 2024 14:00:38 +0100 Subject: [PATCH] changes --- .../forkchoice/ForkChoice.java | 11 ++----- .../statetransition/util/DebugDataDumper.java | 9 ++--- .../util/DebugDataFileDumper.java | 33 ++++++++----------- .../forkchoice/ForkChoiceTest.java | 32 ++++++++++++++---- .../util/DebugDataFileDumperTest.java | 20 +++++++---- 5 files changed, 58 insertions(+), 47 deletions(-) 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 index 07f882bac4d..601d05a0f36 100644 --- 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 @@ -18,7 +18,6 @@ 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.spec.logic.versions.deneb.blobs.BlobSidecarsValidationResult.NOT_AVAILABLE; import static tech.pegasys.teku.statetransition.forkchoice.StateRootCollector.addParentStateRoots; import com.google.common.annotations.VisibleForTesting; @@ -546,18 +545,12 @@ private BlockImportResult importBlockAndState( switch (blobSidecarsAndValidationResult.getValidationResult()) { case NOT_AVAILABLE -> { - debugDataDumper.saveFailedDataAvailabilityBlobSidecars( - blobSidecarsAndValidationResult.getBlobSidecars(), - NOT_AVAILABLE.toString(), - blobSidecarsAndValidationResult.getCause()); return BlockImportResult.failedDataAvailabilityCheckNotAvailable( blobSidecarsAndValidationResult.getCause()); } case INVALID -> { - debugDataDumper.saveFailedDataAvailabilityBlobSidecars( - blobSidecarsAndValidationResult.getBlobSidecars(), - INVALID.toString(), - blobSidecarsAndValidationResult.getCause()); + debugDataDumper.saveInvalidBlobSidecars( + blobSidecarsAndValidationResult.getBlobSidecars(), block); return BlockImportResult.failedDataAvailabilityCheckInvalid( blobSidecarsAndValidationResult.getCause()); } 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 index 3d2e90be4e5..e8af45dba06 100644 --- 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 @@ -46,10 +46,8 @@ public void saveInvalidBlock( final Optional failureCause) {} @Override - public void saveFailedDataAvailabilityBlobSidecars( - final List blobSidecars, - final String failureReason, - final Optional failureCause) {} + public void saveInvalidBlobSidecars( + final List blobSidecars, final SignedBeaconBlock block) {} }; void saveGossipMessageDecodingError( @@ -67,6 +65,5 @@ void saveGossipRejectedMessage( void saveInvalidBlock( SignedBeaconBlock block, String failureReason, Optional failureCause); - void saveFailedDataAvailabilityBlobSidecars( - List blobSidecars, String failureReason, Optional failureCause); + void saveInvalidBlobSidecars(List 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 index a92bd104950..ccdd1da1710 100644 --- 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 @@ -43,8 +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 FAILED_DATA_AVAILABILITY_BLOB_SIDECARS_DIR = - "failed_data_availability_blob_sidecars"; + private static final String INVALID_BLOB_SIDECARS_DIR = "invalid_blob_sidecars"; private boolean enabled; private final Path directory; @@ -138,13 +137,18 @@ public void saveInvalidBlock( } @Override - public void saveFailedDataAvailabilityBlobSidecars( - final List blobSidecars, - final String failureReason, - final Optional failureCause) { + public void saveInvalidBlobSidecars( + final List 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(); @@ -152,19 +156,10 @@ public void saveFailedDataAvailabilityBlobSidecars( final UInt64 index = blobSidecar.getIndex(); final String fileName = String.format("%s_%s_%s.ssz", slot, blockRoot.toUnprefixedHexString(), index); - final boolean success = - saveBytesToFile( - "failed data availability blob sidecar", - Path.of(FAILED_DATA_AVAILABILITY_BLOB_SIDECARS_DIR).resolve(fileName), - blobSidecar.sszSerialize()); - if (success) { - LOG.warn( - "Blob sidecar at slot {} with block root {} has failed the data availability check, reason: {}, cause: {}", - slot, - blockRoot, - failureReason, - failureCause.orElse(null)); - } + saveBytesToFile( + "blob sidecar", + Path.of(INVALID_BLOB_SIDECARS_DIR).resolve(fileName), + blobSidecar.sszSerialize()); }); } 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 index 3f22a4be697..70d9ca7e103 100644 --- 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 @@ -91,7 +91,6 @@ import tech.pegasys.teku.spec.logic.common.statetransition.results.BlockImportResult.FailureReason; import tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsAndValidationResult; import tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsAvailabilityChecker; -import tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsValidationResult; import tech.pegasys.teku.spec.util.DataStructureUtil; import tech.pegasys.teku.statetransition.blobs.BlobSidecarManager; import tech.pegasys.teku.statetransition.forkchoice.ForkChoice.OptimisticHeadSubscriber; @@ -246,9 +245,31 @@ void onBlock_shouldFailIfBlobsAreNotAvailable() { verify(blobSidecarManager).createAvailabilityChecker(blockAndState.getBlock()); verify(blobSidecarsAvailabilityChecker).initiateDataAvailabilityCheck(); verify(blobSidecarsAvailabilityChecker).getAvailabilityCheckResult(); - verify(debugDataDumper) - .saveFailedDataAvailabilityBlobSidecars( - any(), eq(BlobSidecarsValidationResult.NOT_AVAILABLE.toString()), eq(Optional.empty())); + } + + @Test + void onBlock_shouldFailIfBlobsAreInvalid() { + setupWithSpec(TestSpecFactory.createMinimalDeneb()); + final SignedBlockAndState blockAndState = chainBuilder.generateBlockAtSlot(ONE); + storageSystem.chainUpdater().advanceCurrentSlotToAtLeast(blockAndState.getSlot()); + final List 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 @@ -268,9 +289,6 @@ void onBlock_consensusValidationShouldNotResolveWhenDataAvailabilityFails() { verify(blobSidecarManager).createAvailabilityChecker(blockAndState.getBlock()); verify(blobSidecarsAvailabilityChecker).initiateDataAvailabilityCheck(); verify(blobSidecarsAvailabilityChecker).getAvailabilityCheckResult(); - verify(debugDataDumper) - .saveFailedDataAvailabilityBlobSidecars( - any(), eq(BlobSidecarsValidationResult.NOT_AVAILABLE.toString()), eq(Optional.empty())); } @Test 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 index 2da2c9c38f1..04b5510af69 100644 --- 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 @@ -94,11 +94,20 @@ void saveInvalidBlockToFile_shouldSaveToFile(@TempDir final Path tempDir) { } @Test - void saveFailedDataAvailabilityBlobSidecars_shouldSaveToFiles(@TempDir final Path tempDir) { + void saveInvalidBlobSidecars_shouldSaveToFiles(@TempDir final Path tempDir) { final DebugDataFileDumper dumper = new DebugDataFileDumper(tempDir); - final List blobSidecars = dataStructureUtil.randomBlobSidecars(3); - dumper.saveFailedDataAvailabilityBlobSidecars( - blobSidecars, "reason", Optional.of(new Throwable())); + final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(); + final List 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 -> { @@ -108,8 +117,7 @@ void saveFailedDataAvailabilityBlobSidecars_shouldSaveToFiles(@TempDir final Pat blobSidecar.getSlot(), blobSidecar.getBlockRoot().toUnprefixedHexString(), blobSidecar.getIndex()); - final Path expectedFile = - tempDir.resolve("failed_data_availability_blob_sidecars").resolve(fileName); + final Path expectedFile = tempDir.resolve("invalid_blob_sidecars").resolve(fileName); checkBytesSavedToFile(expectedFile, blobSidecar.sszSerialize()); }); }