From e7d6be96ee60b068e8522f15ccd909f74e3d588d Mon Sep 17 00:00:00 2001 From: courtneyeh Date: Mon, 15 Apr 2024 19:52:45 +1000 Subject: [PATCH] Use NoOpDebugDataDumper when disabled in config --- .../statetransition/util/DebugDataDumper.java | 7 +- .../util/noop/NoOpDebugDataDumper.java | 2 +- .../util/DebugDataDumperTest.java | 81 ++----------------- .../beaconchain/BeaconChainController.java | 7 +- 4 files changed, 15 insertions(+), 82 deletions(-) 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 28fb6d0bb99..444f41009f3 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 @@ -41,12 +41,9 @@ public class DebugDataDumper { private boolean enabled; private final Path directory; - public DebugDataDumper(final Path directory, final boolean enabled) { - this.enabled = enabled; + public DebugDataDumper(final Path directory) { + this.enabled = true; this.directory = directory; - if (!enabled) { - return; - } final Path gossipMessagesPath = this.directory.resolve(GOSSIP_MESSAGES_DIR); createDirectory(gossipMessagesPath, GOSSIP_MESSAGES_DIR, "gossip messages"); diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/noop/NoOpDebugDataDumper.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/noop/NoOpDebugDataDumper.java index 64ae9e814bf..e2bdf63fd8a 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/noop/NoOpDebugDataDumper.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/noop/NoOpDebugDataDumper.java @@ -23,7 +23,7 @@ public class NoOpDebugDataDumper extends DebugDataDumper { public NoOpDebugDataDumper() { - super(Path.of("."), true); + super(Path.of(".")); } @Override diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/DebugDataDumperTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/DebugDataDumperTest.java index 33afc268bda..84acc883031 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/DebugDataDumperTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/DebugDataDumperTest.java @@ -20,7 +20,6 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; -import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.sql.Date; import java.text.DateFormat; @@ -44,7 +43,7 @@ class DebugDataDumperTest { @Test void saveGossipMessageDecodingError_shouldSaveToFile(@TempDir Path tempDir) { - final DebugDataDumper manager = new DebugDataDumper(tempDir, true); + final DebugDataDumper manager = new DebugDataDumper(tempDir); final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize(); final Optional arrivalTimestamp = Optional.of(timeProvider.getTimeInMillis()); manager.saveGossipMessageDecodingError( @@ -61,29 +60,9 @@ void saveGossipMessageDecodingError_shouldSaveToFile(@TempDir Path tempDir) { checkBytesSavedToFile(expectedFile, messageBytes); } - @Test - void saveGossipMessageDecodingError_shouldNotSaveToFileWhenDisabled(@TempDir Path tempDir) { - final DebugDataDumper manager = new DebugDataDumper(tempDir, false); - final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize(); - final Optional arrivalTimestamp = Optional.of(timeProvider.getTimeInMillis()); - manager.saveGossipMessageDecodingError( - "/eth/test/topic", arrivalTimestamp, messageBytes, new Throwable()); - assertThat(manager.isEnabled()).isFalse(); - - final String fileName = - String.format("%s.ssz", formatTimestamp(timeProvider.getTimeInMillis().longValue())); - final Path expectedFile = - tempDir - .resolve("gossip_messages") - .resolve("decoding_error") - .resolve("_eth_test_topic") - .resolve(fileName); - checkFileNotExist(expectedFile); - } - @Test void saveGossipRejectedMessageToFile_shouldSaveToFile(@TempDir Path tempDir) { - final DebugDataDumper manager = new DebugDataDumper(tempDir, true); + final DebugDataDumper manager = new DebugDataDumper(tempDir); final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize(); final Optional arrivalTimestamp = Optional.of(timeProvider.getTimeInMillis()); manager.saveGossipRejectedMessageToFile( @@ -100,28 +79,9 @@ void saveGossipRejectedMessageToFile_shouldSaveToFile(@TempDir Path tempDir) { checkBytesSavedToFile(expectedFile, messageBytes); } - @Test - void saveGossipRejectedMessageToFile_shouldNotSaveToFileWhenDisabled(@TempDir Path tempDir) { - final DebugDataDumper manager = new DebugDataDumper(tempDir, false); - final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize(); - final Optional arrivalTimestamp = Optional.of(timeProvider.getTimeInMillis()); - manager.saveGossipRejectedMessageToFile( - "/eth/test/topic", arrivalTimestamp, messageBytes, Optional.of("reason")); - assertThat(manager.isEnabled()).isFalse(); - - final String fileName = String.format("%s.ssz", arrivalTimestamp); - final Path expectedFile = - tempDir - .resolve("gossip_messages") - .resolve("rejected") - .resolve("_eth_test_topic") - .resolve(fileName); - checkFileNotExist(expectedFile); - } - @Test void saveInvalidBlockToFile_shouldSaveToFile(@TempDir Path tempDir) { - final DebugDataDumper manager = new DebugDataDumper(tempDir, true); + final DebugDataDumper manager = new DebugDataDumper(tempDir); final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(); manager.saveInvalidBlockToFile( block.getSlot(), @@ -136,23 +96,9 @@ void saveInvalidBlockToFile_shouldSaveToFile(@TempDir Path tempDir) { checkBytesSavedToFile(expectedFile, block.sszSerialize()); } - @Test - void saveInvalidBlockToFile_shouldNotSaveToFileWhenDisabled(@TempDir Path tempDir) { - final DebugDataDumper manager = new DebugDataDumper(tempDir, false); - final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(); - manager.saveInvalidBlockToFile( - block.getSlot(), block.getRoot(), block.sszSerialize(), "reason", Optional.empty()); - assertThat(manager.isEnabled()).isFalse(); - - final String fileName = - String.format("%s_%s.ssz", block.getSlot(), block.getRoot().toUnprefixedHexString()); - final Path expectedFile = tempDir.resolve("invalid_blocks").resolve(fileName); - checkFileNotExist(expectedFile); - } - @Test void saveBytesToFile_shouldNotThrowExceptionWhenNoDirectory(@TempDir Path tempDir) { - final DebugDataDumper manager = new DebugDataDumper(tempDir, true); + final DebugDataDumper manager = new DebugDataDumper(tempDir); assertDoesNotThrow( () -> { final boolean success = @@ -165,7 +111,7 @@ void saveBytesToFile_shouldNotThrowExceptionWhenNoDirectory(@TempDir Path tempDi @Test @DisabledOnOs(OS.WINDOWS) // Can't set permissions on Windows void saveBytesToFile_shouldNotEscalateWhenIOException(@TempDir Path tempDir) { - final DebugDataDumper manager = new DebugDataDumper(tempDir, true); + final DebugDataDumper manager = new DebugDataDumper(tempDir); final File invalidPath = tempDir.resolve("invalid").toFile(); assertThat(invalidPath.mkdirs()).isTrue(); assertThat(invalidPath.setWritable(false)).isTrue(); @@ -182,13 +128,13 @@ void saveBytesToFile_shouldNotEscalateWhenIOException(@TempDir Path tempDir) { @DisabledOnOs(OS.WINDOWS) // Can't set permissions on Windows void constructionOfDirectories_shouldDisableWhenFailedToCreate(@TempDir Path tempDir) { assertThat(tempDir.toFile().setWritable(false)).isTrue(); - final DebugDataDumper manager = new DebugDataDumper(tempDir, true); + final DebugDataDumper manager = new DebugDataDumper(tempDir); assertThat(manager.isEnabled()).isFalse(); } @Test void formatOptionalTimestamp_shouldFormatTimestamp(@TempDir Path tempDir) { - final DebugDataDumper manager = new DebugDataDumper(tempDir, true); + final DebugDataDumper manager = new DebugDataDumper(tempDir); final String formattedTimestamp = manager.formatOptionalTimestamp(Optional.of(timeProvider.getTimeInMillis()), timeProvider); assertThat(formattedTimestamp) @@ -197,7 +143,7 @@ void formatOptionalTimestamp_shouldFormatTimestamp(@TempDir Path tempDir) { @Test void formatOptionalTimestamp_shouldGenerateTimestamp(@TempDir Path tempDir) { - final DebugDataDumper manager = new DebugDataDumper(tempDir, true); + final DebugDataDumper manager = new DebugDataDumper(tempDir); final String formattedTimestamp = manager.formatOptionalTimestamp(Optional.empty(), timeProvider); assertThat(formattedTimestamp) @@ -213,17 +159,6 @@ private void checkBytesSavedToFile(final Path path, final Bytes expectedBytes) { } } - private void checkFileNotExist(final Path path) { - try { - Bytes.wrap(Files.readAllBytes(path)); - } catch (IOException e) { - if (e instanceof NoSuchFileException) { - return; - } - } - fail("File was found and bytes read"); - } - private String formatTimestamp(final long timeInMillis) { final DateFormat df = new SimpleDateFormat("yyyy-MM-dd'T'HH_mm_ss.SS"); final Date date = new Date(timeInMillis); diff --git a/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java b/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java index df664b70dae..2881a222e10 100644 --- a/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java +++ b/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java @@ -153,6 +153,7 @@ import tech.pegasys.teku.statetransition.util.FutureItems; import tech.pegasys.teku.statetransition.util.PendingPool; import tech.pegasys.teku.statetransition.util.PoolFactory; +import tech.pegasys.teku.statetransition.util.noop.NoOpDebugDataDumper; import tech.pegasys.teku.statetransition.validation.AggregateAttestationValidator; import tech.pegasys.teku.statetransition.validation.AttestationValidator; import tech.pegasys.teku.statetransition.validation.AttesterSlashingValidator; @@ -321,9 +322,9 @@ public BeaconChainController( eventChannels.getPublisher(ReceivedBlockEventsChannel.class); this.forkChoiceExecutor = new AsyncRunnerEventThread("forkchoice", asyncRunnerFactory); this.debugDataDumper = - new DebugDataDumper( - serviceConfig.getDataDirLayout().getDebugDataDirectory(), - beaconConfig.p2pConfig().isP2pDumpsToFileEnabled()); + beaconConfig.p2pConfig().isP2pDumpsToFileEnabled() + ? new DebugDataDumper(serviceConfig.getDataDirLayout().getDebugDataDirectory()) + : new NoOpDebugDataDumper(); this.futureItemsMetric = SettableLabelledGauge.create( metricsSystem,