diff --git a/CHANGELOG.md b/CHANGELOG.md index a9c017d9e20..9393a261a8d 100644 --- 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 index ccf50d754dd..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 @@ -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 index aebd1fc32e2..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 @@ -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 failureCause) {} + + @Override + public void saveInvalidBlobSidecars( + final List blobSidecars, final SignedBeaconBlock block) {} }; void saveGossipMessageDecodingError( @@ -58,4 +64,6 @@ void saveGossipRejectedMessage( void saveInvalidBlock( SignedBeaconBlock block, 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 a5b93ca645c..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 @@ -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,7 +128,7 @@ 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, @@ -133,6 +136,33 @@ public void saveInvalidBlock( } } + @Override + 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(); + 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 index 1098ad72f81..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 @@ -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 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 index 1cd40a66f8d..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 @@ -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 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);