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 6f3621c2043..dbafef1e1bd 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 @@ -760,13 +760,11 @@ private void reportInvalidBlock(final SignedBeaconBlock block, final BlockImport if (result.getFailureReason() == FailureReason.BLOCK_IS_FROM_FUTURE) { return; } - final String savedFile = - p2pDumpManager.saveInvalidBlockToFile( - block.getSlot(), block.getRoot(), block.sszSerialize()); + p2pDumpManager.saveInvalidBlockToFile(block.getSlot(), block.getRoot(), block.sszSerialize()); P2P_LOG.onInvalidBlock( block.getSlot(), block.getRoot(), - savedFile, + block.sszSerialize(), result.getFailureReason().name(), result.getFailureCause()); } diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/P2PDumpManager.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/P2PDumpManager.java index 6e6f15042eb..d790b3e7bef 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/P2PDumpManager.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/P2PDumpManager.java @@ -13,6 +13,7 @@ package tech.pegasys.teku.statetransition.util; +import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -20,7 +21,6 @@ import org.apache.logging.log4j.Logger; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; -import tech.pegasys.teku.infrastructure.logging.LoggingConfigurator; import tech.pegasys.teku.infrastructure.unsigned.UInt64; public class P2PDumpManager { @@ -30,12 +30,16 @@ public class P2PDumpManager { private static final String GOSSIP_REJECTED_DIR = "rejected_gossip_messages"; private static final String INVALID_BLOCK_DIR = "invalid_blocks"; - private final boolean isIncludeP2pWarnings = LoggingConfigurator.isIncludeP2pWarnings(); - + private final boolean enabled; private final Path directory; - public P2PDumpManager(final Path directory) { + public P2PDumpManager(final Path directory, final boolean enabled) { + this.enabled = enabled; // TODO fix directory structure this.directory = directory; + if (!enabled) { + return; + } + if (this.directory.resolve(GOSSIP_DECODING_ERROR_DIR).toFile().mkdirs()) { LOG.info( String.format( @@ -55,14 +59,14 @@ public P2PDumpManager(final Path directory) { } } - public String saveGossipMessageDecodingError( + public void saveGossipMessageDecodingError( final String topic, final String arrivalTimestamp, final Bytes originalMessage) { - if (!isIncludeP2pWarnings) { - return ""; + if (!enabled) { + return; } final String fileName = String.format("%s_%s.ssz", arrivalTimestamp, topic); final String identifiers = String.format("Topic: %s", topic); - return saveBytesToFile( + saveBytesToFile( "gossip message with decoding error", GOSSIP_DECODING_ERROR_DIR, fileName, @@ -70,29 +74,29 @@ public String saveGossipMessageDecodingError( originalMessage); } - public String saveGossipRejectedMessageToFile( + public void saveGossipRejectedMessageToFile( final String topic, final String arrivalTimestamp, final Bytes decodedMessage) { - if (!isIncludeP2pWarnings) { - return ""; + if (!enabled) { + return; } final String fileName = String.format("%s_%s.ssz", arrivalTimestamp, topic); final String identifiers = String.format("Topic: %s", topic); - return saveBytesToFile( + saveBytesToFile( "rejected gossip message", GOSSIP_REJECTED_DIR, fileName, identifiers, decodedMessage); } - public String saveInvalidBlockToFile( + public void saveInvalidBlockToFile( final UInt64 slot, final Bytes32 blockRoot, final Bytes blockSsz) { - if (!isIncludeP2pWarnings) { - return ""; + if (!enabled) { + return; } final String fileName = String.format("slot%s_root%s.ssz", slot, blockRoot.toUnprefixedHexString()); final String identifiers = String.format("Slot: %s, Block Root: %s", slot, blockRoot); - return saveBytesToFile("invalid block", INVALID_BLOCK_DIR, fileName, identifiers, blockSsz); + saveBytesToFile("invalid block", INVALID_BLOCK_DIR, fileName, identifiers, blockSsz); } - private String saveBytesToFile( + private void saveBytesToFile( final String object, final String errorDirectory, final String fileName, @@ -100,13 +104,17 @@ private String saveBytesToFile( final Bytes bytes) { final Path path = directory.resolve(errorDirectory).resolve(fileName); try { - return Files.write(path, bytes.toArray()).toString(); - + final String file = + Files.write(path, bytes.toArray()) + .toString(); // todo change to not be printing the whole path + LOG.info("Saved {} bytes to file. {}. Location: {}", object, identifiers, file); } catch (IOException e) { - final String errorMessage = - String.format("Failed to save %s bytes to file. %s", object, identifiers); - LOG.error(errorMessage, e); - return String.format("Error saving to %s", path); + LOG.error("Failed to save {}} bytes to file. {}", object, identifiers, e); } } + + @VisibleForTesting + protected boolean isEnabled() { + return enabled; + } } diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/P2PDumpManagerTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/P2PDumpManagerTest.java index f2e74a0fd09..97972751a8f 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/P2PDumpManagerTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/P2PDumpManagerTest.java @@ -18,12 +18,11 @@ import java.io.IOException; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import org.apache.tuweni.bytes.Bytes; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import tech.pegasys.teku.infrastructure.logging.LoggingConfig; -import tech.pegasys.teku.infrastructure.logging.LoggingConfigurator; import tech.pegasys.teku.infrastructure.time.StubTimeProvider; import tech.pegasys.teku.spec.TestSpecFactory; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; @@ -36,79 +35,79 @@ class P2PDumpManagerTest { @Test void saveGossipMessageDecodingError_shouldSaveToFile(@TempDir Path tempDir) { - final P2PDumpManager manager = new P2PDumpManager(tempDir); + final P2PDumpManager manager = new P2PDumpManager(tempDir, true); final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize(); final String arrivalTimestamp = timeProvider.getTimeInMillis().toString(); - final String file = - manager.saveGossipMessageDecodingError("test_topic", arrivalTimestamp, messageBytes); + manager.saveGossipMessageDecodingError("test_topic", arrivalTimestamp, messageBytes); final String fileName = String.format("%s_%s.ssz", arrivalTimestamp, "test_topic"); final Path expectedFile = tempDir.resolve("gossip_decoding_error_messages").resolve(fileName); - assertThat(file).isEqualTo(expectedFile.toString()); checkBytesSavedToFile(expectedFile, messageBytes); } @Test - void saveGossipMessageDecodingError_shouldNotSaveToFileWhenWarningsDisabled( - @TempDir Path tempDir) { - disableP2pWarnings(); - final P2PDumpManager manager = new P2PDumpManager(tempDir); + void saveGossipMessageDecodingError_shouldNotSaveToFileWhenDisabled(@TempDir Path tempDir) { + final P2PDumpManager manager = new P2PDumpManager(tempDir, false); final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize(); final String arrivalTimestamp = timeProvider.getTimeInMillis().toString(); - final String file = - manager.saveGossipMessageDecodingError("test_topic", arrivalTimestamp, messageBytes); - assertThat(file).isEqualTo(""); + manager.saveGossipMessageDecodingError("test_topic", arrivalTimestamp, messageBytes); + assertThat(manager.isEnabled()).isFalse(); + + final String fileName = String.format("%s_%s.ssz", arrivalTimestamp, "test_topic"); + final Path expectedFile = tempDir.resolve("gossip_decoding_error_messages").resolve(fileName); + checkFileNotExist(expectedFile); } @Test void saveGossipRejectedMessageToFile_shouldSaveToFile(@TempDir Path tempDir) { - final P2PDumpManager manager = new P2PDumpManager(tempDir); + final P2PDumpManager manager = new P2PDumpManager(tempDir, true); final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize(); final String arrivalTimestamp = timeProvider.getTimeInMillis().toString(); - final String file = - manager.saveGossipRejectedMessageToFile("test_topic", arrivalTimestamp, messageBytes); + manager.saveGossipRejectedMessageToFile("test_topic", arrivalTimestamp, messageBytes); final String fileName = String.format("%s_%s.ssz", arrivalTimestamp, "test_topic"); final Path expectedFile = tempDir.resolve("rejected_gossip_messages").resolve(fileName); - assertThat(file).isEqualTo(expectedFile.toString()); checkBytesSavedToFile(expectedFile, messageBytes); } @Test - void saveGossipRejectedMessageToFile_shouldNotSaveToFileWhenWarningsDisabled( - @TempDir Path tempDir) { - disableP2pWarnings(); - final P2PDumpManager manager = new P2PDumpManager(tempDir); + void saveGossipRejectedMessageToFile_shouldNotSaveToFileWhenDisabled(@TempDir Path tempDir) { + final P2PDumpManager manager = new P2PDumpManager(tempDir, false); final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize(); final String arrivalTimestamp = timeProvider.getTimeInMillis().toString(); - final String file = - manager.saveGossipRejectedMessageToFile("test_topic", arrivalTimestamp, messageBytes); - assertThat(file).isEqualTo(""); + manager.saveGossipRejectedMessageToFile("test_topic", arrivalTimestamp, messageBytes); + assertThat(manager.isEnabled()).isFalse(); + + final String fileName = String.format("%s_%s.ssz", arrivalTimestamp, "test_topic"); + final Path expectedFile = tempDir.resolve("rejected_gossip_messages").resolve(fileName); + checkFileNotExist(expectedFile); } @Test void saveInvalidBlockToFile_shouldSaveToFile(@TempDir Path tempDir) { - final P2PDumpManager manager = new P2PDumpManager(tempDir); + final P2PDumpManager manager = new P2PDumpManager(tempDir, true); final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(); - final String file = - manager.saveInvalidBlockToFile(block.getSlot(), block.getRoot(), block.sszSerialize()); + manager.saveInvalidBlockToFile(block.getSlot(), block.getRoot(), block.sszSerialize()); final String fileName = String.format( "slot%s_root%s.ssz", block.getSlot(), block.getRoot().toUnprefixedHexString()); final Path expectedFile = tempDir.resolve("invalid_blocks").resolve(fileName); - assertThat(file).isEqualTo(expectedFile.toString()); checkBytesSavedToFile(expectedFile, block.sszSerialize()); } @Test - void saveInvalidBlockToFile_shouldNotSaveToFileWhenWarningsDisabled(@TempDir Path tempDir) { - disableP2pWarnings(); - final P2PDumpManager manager = new P2PDumpManager(tempDir); + void saveInvalidBlockToFile_shouldNotSaveToFileWhenDisabled(@TempDir Path tempDir) { + final P2PDumpManager manager = new P2PDumpManager(tempDir, false); final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(); - final String file = - manager.saveInvalidBlockToFile(block.getSlot(), block.getRoot(), block.sszSerialize()); - assertThat(file).isEqualTo(""); + manager.saveInvalidBlockToFile(block.getSlot(), block.getRoot(), block.sszSerialize()); + assertThat(manager.isEnabled()).isFalse(); + + final String fileName = + String.format( + "slot%s_root%s.ssz", block.getSlot(), block.getRoot().toUnprefixedHexString()); + final Path expectedFile = tempDir.resolve("invalid_blocks").resolve(fileName); + checkFileNotExist(expectedFile); } private void checkBytesSavedToFile(final Path path, final Bytes expectedBytes) { @@ -120,12 +119,14 @@ private void checkBytesSavedToFile(final Path path, final Bytes expectedBytes) { } } - private void disableP2pWarnings() { - LoggingConfigurator.update( - LoggingConfig.builder() - .logDirectory(".") - .logFileNamePrefix("prefix") - .includeP2pWarningsEnabled(false) - .build()); + 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"); } } diff --git a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/P2PLogger.java b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/P2PLogger.java index 14749d8d587..f9a1c129a3a 100644 --- a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/P2PLogger.java +++ b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/P2PLogger.java @@ -16,6 +16,7 @@ import java.util.Optional; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.infrastructure.unsigned.UInt64; @@ -32,40 +33,40 @@ public P2PLogger(final String name) { } public void onGossipMessageDecodingError( - final String topic, final String savedFile, final Throwable error) { + final String topic, final Bytes originalMessage, final Throwable error) { if (isIncludeP2pWarnings) { log.warn( - "Failed to decode gossip message on topic {}, raw message location: {}", + "Failed to decode gossip message on topic {}, raw message: {}", topic, - savedFile, + originalMessage, error); } } public void onGossipRejected( - final String topic, final String savedFile, final Optional description) { + final String topic, final Bytes decodedMessage, final Optional description) { if (isIncludeP2pWarnings) { log.warn( - "Rejecting gossip message on topic {}, reason: {}, decoded message location: {}", + "Rejecting gossip message on topic {}, reason: {}, decoded message: {}", topic, description.orElse("failed validation"), - savedFile); + decodedMessage); } } public void onInvalidBlock( final UInt64 slot, final Bytes32 blockRoot, - final String savedFile, + final Bytes blockSsz, final String failureReason, final Optional failureCause) { if (isIncludeP2pWarnings) { log.warn( - "Rejecting invalid block at slot {} with root {} because {}. Full block data location: {}", + "Rejecting invalid block at slot {} with root {} because {}. Full block data: {}", slot, blockRoot, failureReason, - savedFile, + blockSsz, failureCause.orElse(null)); } } diff --git a/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/DataConfig.java b/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/DataConfig.java index 35f78e90ac3..e6205489dac 100644 --- a/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/DataConfig.java +++ b/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/DataConfig.java @@ -27,17 +27,14 @@ public static Path defaultDataPath() { private final Path dataBasePath; private final Optional beaconDataPath; private final Optional validatorDataPath; - private final Optional p2pDumpsDataPath; private DataConfig( final Path dataBasePath, final Optional beaconDataPath, - final Optional validatorDataPath, - final Optional p2pDumpsDataPath) { + final Optional validatorDataPath) { this.dataBasePath = dataBasePath; this.beaconDataPath = beaconDataPath; this.validatorDataPath = validatorDataPath; - this.p2pDumpsDataPath = p2pDumpsDataPath; } public static DataConfig.Builder builder() { @@ -56,16 +53,11 @@ public Optional getValidatorDataPath() { return validatorDataPath; } - public Optional getP2pDumpsDataPath() { - return p2pDumpsDataPath; - } - public static final class Builder { private Path dataBasePath = defaultDataPath(); private Optional beaconDataPath = Optional.empty(); private Optional validatorDataPath = Optional.empty(); - private Optional p2pDumpsDataPath = Optional.empty(); private Builder() {} @@ -84,16 +76,11 @@ public Builder validatorDataPath(Path validatorDataPath) { return this; } - public Builder p2pDumpsDataPath(Path p2pDumpsDataPath) { - this.p2pDumpsDataPath = Optional.ofNullable(p2pDumpsDataPath); - return this; - } - public DataConfig build() { if (dataBasePath == null) { throw new InvalidConfigurationException("data-base-path must be specified"); } - return new DataConfig(dataBasePath, beaconDataPath, validatorDataPath, p2pDumpsDataPath); + return new DataConfig(dataBasePath, beaconDataPath, validatorDataPath); } } } diff --git a/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/DataDirLayout.java b/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/DataDirLayout.java index 296eca77864..2e9a2f3018e 100644 --- a/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/DataDirLayout.java +++ b/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/DataDirLayout.java @@ -21,8 +21,7 @@ static DataDirLayout createFrom(final DataConfig dataConfig) { new SeparateServiceDataDirLayout( dataConfig.getDataBasePath(), dataConfig.getBeaconDataPath(), - dataConfig.getValidatorDataPath(), - dataConfig.getP2pDumpsDataPath()); + dataConfig.getValidatorDataPath()); return layout; } @@ -31,5 +30,5 @@ static DataDirLayout createFrom(final DataConfig dataConfig) { Path getValidatorDataDirectory(); - Path getP2pDumpDirectory(); + Path getDebugDataDirectory(); } diff --git a/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/SeparateServiceDataDirLayout.java b/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/SeparateServiceDataDirLayout.java index 4f7cf5ad57c..2fa8f14958b 100644 --- a/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/SeparateServiceDataDirLayout.java +++ b/infrastructure/serviceutils/src/main/java/tech/pegasys/teku/service/serviceutils/layout/SeparateServiceDataDirLayout.java @@ -19,20 +19,19 @@ public class SeparateServiceDataDirLayout implements DataDirLayout { static final String BEACON_DATA_DIR_NAME = "beacon"; static final String VALIDATOR_DATA_DIR_NAME = "validator"; - static final String P2P_DUMP_DIR_NAME = "p2p-dumps"; + static final String DEBUG_DIR_NAME = "debug"; private final Path beaconNodeDataDir; private final Path validatorDataDir; - private final Path p2pDumpDir; + private final Path debugDataDir; public SeparateServiceDataDirLayout( final Path baseDir, final Optional beaconDataDirectory, - final Optional validatorDataDirectory, - final Optional p2pDumpDirectory) { + final Optional validatorDataDirectory) { beaconNodeDataDir = beaconDataDirectory.orElseGet(() -> baseDir.resolve(BEACON_DATA_DIR_NAME)); validatorDataDir = validatorDataDirectory.orElseGet(() -> baseDir.resolve(VALIDATOR_DATA_DIR_NAME)); - p2pDumpDir = p2pDumpDirectory.orElseGet(() -> baseDir.resolve(P2P_DUMP_DIR_NAME)); + debugDataDir = baseDir.resolve(DEBUG_DIR_NAME); } @Override @@ -46,7 +45,7 @@ public Path getValidatorDataDirectory() { } @Override - public Path getP2pDumpDirectory() { - return p2pDumpDir; + public Path getDebugDataDirectory() { + return debugDataDir; } } diff --git a/infrastructure/serviceutils/src/test/java/tech/pegasys/teku/service/serviceutils/layout/SeparateServiceDataDirLayoutTest.java b/infrastructure/serviceutils/src/test/java/tech/pegasys/teku/service/serviceutils/layout/SeparateServiceDataDirLayoutTest.java index 896967cc908..a0f1e492678 100644 --- a/infrastructure/serviceutils/src/test/java/tech/pegasys/teku/service/serviceutils/layout/SeparateServiceDataDirLayoutTest.java +++ b/infrastructure/serviceutils/src/test/java/tech/pegasys/teku/service/serviceutils/layout/SeparateServiceDataDirLayoutTest.java @@ -30,9 +30,7 @@ class SeparateServiceDataDirLayoutTest { @BeforeEach void setUp() { - layout = - new SeparateServiceDataDirLayout( - tempDir, Optional.empty(), Optional.empty(), Optional.empty()); + layout = new SeparateServiceDataDirLayout(tempDir, Optional.empty(), Optional.empty()); } @Test diff --git a/infrastructure/serviceutils/src/testFixtures/java/tech/pegasys/techu/service/serviceutils/layout/SimpleDataDirLayout.java b/infrastructure/serviceutils/src/testFixtures/java/tech/pegasys/techu/service/serviceutils/layout/SimpleDataDirLayout.java index b1fa42f50aa..41df2354fdf 100644 --- a/infrastructure/serviceutils/src/testFixtures/java/tech/pegasys/techu/service/serviceutils/layout/SimpleDataDirLayout.java +++ b/infrastructure/serviceutils/src/testFixtures/java/tech/pegasys/techu/service/serviceutils/layout/SimpleDataDirLayout.java @@ -34,7 +34,7 @@ public Path getValidatorDataDirectory() { } @Override - public Path getP2pDumpDirectory() { + public Path getDebugDataDirectory() { return path; } } diff --git a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/P2PConfig.java b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/P2PConfig.java index 1edeac1a496..2008e6974d7 100644 --- a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/P2PConfig.java +++ b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/P2PConfig.java @@ -41,6 +41,7 @@ public class P2PConfig { Math.max(2, Runtime.getRuntime().availableProcessors() / 2); public static final int DEFAULT_BATCH_VERIFY_QUEUE_CAPACITY = 15_000; public static final int DEFAULT_BATCH_VERIFY_MAX_BATCH_SIZE = 250; + public static final boolean DEFAULT_P2P_DUMPS_TO_FILE_ENABLED = false; public static final boolean DEFAULT_BATCH_VERIFY_STRICT_THREAD_LIMIT_ENABLED = false; private final Spec spec; @@ -58,6 +59,7 @@ public class P2PConfig { private final int batchVerifyQueueCapacity; private final int batchVerifyMaxBatchSize; private final boolean batchVerifyStrictThreadLimitEnabled; + private final boolean p2pDumpsToFileEnabled; private final boolean allTopicsFilterEnabled; @@ -75,6 +77,7 @@ private P2PConfig( final int batchVerifyQueueCapacity, final int batchVerifyMaxBatchSize, final boolean batchVerifyStrictThreadLimitEnabled, + final boolean p2pDumpsToFileEnabled, boolean allTopicsFilterEnabled) { this.spec = spec; this.networkConfig = networkConfig; @@ -90,6 +93,7 @@ private P2PConfig( this.batchVerifyMaxBatchSize = batchVerifyMaxBatchSize; this.batchVerifyStrictThreadLimitEnabled = batchVerifyStrictThreadLimitEnabled; this.networkingSpecConfig = spec.getNetworkingConfig(); + this.p2pDumpsToFileEnabled = p2pDumpsToFileEnabled; this.allTopicsFilterEnabled = allTopicsFilterEnabled; } @@ -149,6 +153,10 @@ public boolean isBatchVerifyStrictThreadLimitEnabled() { return batchVerifyStrictThreadLimitEnabled; } + public boolean isP2pDumpsToFileEnabled() { + return p2pDumpsToFileEnabled; + } + public NetworkingSpecConfig getNetworkingSpecConfig() { return networkingSpecConfig; } @@ -174,6 +182,7 @@ public static class Builder { private boolean batchVerifyStrictThreadLimitEnabled = DEFAULT_BATCH_VERIFY_STRICT_THREAD_LIMIT_ENABLED; private boolean allTopicsFilterEnabled = DEFAULT_PEER_ALL_TOPIC_FILTER_ENABLED; + private boolean p2pDumpsToFileEnabled = DEFAULT_P2P_DUMPS_TO_FILE_ENABLED; private Builder() {} @@ -216,6 +225,7 @@ public P2PConfig build() { batchVerifyQueueCapacity, batchVerifyMaxBatchSize, batchVerifyStrictThreadLimitEnabled, + p2pDumpsToFileEnabled, allTopicsFilterEnabled); } @@ -318,5 +328,10 @@ public Builder allTopicsFilterEnabled(final boolean allTopicsFilterEnabled) { this.allTopicsFilterEnabled = allTopicsFilterEnabled; return this; } + + public Builder p2pDumpsToFileEnabled(final boolean p2pDumpsToFileEnabled) { + this.p2pDumpsToFileEnabled = p2pDumpsToFileEnabled; + return this; + } } } diff --git a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/gossip/topics/topichandlers/Eth2TopicHandler.java b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/gossip/topics/topichandlers/Eth2TopicHandler.java index 257d092025e..57a6955f5e3 100644 --- a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/gossip/topics/topichandlers/Eth2TopicHandler.java +++ b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/gossip/topics/topichandlers/Eth2TopicHandler.java @@ -135,12 +135,14 @@ private void processMessage( final PreparedGossipMessage message) { switch (internalValidationResult.code()) { case REJECT: - final String savedFile = - p2pDumpManager.saveGossipRejectedMessageToFile( - getTopic(), - message.getArrivalTimestamp().map(UInt64::toString).orElse("unknown"), - message.getDecodedMessage().getDecodedMessage().orElse(Bytes.EMPTY)); - P2P_LOG.onGossipRejected(getTopic(), savedFile, internalValidationResult.getDescription()); + p2pDumpManager.saveGossipRejectedMessageToFile( + getTopic(), + message.getArrivalTimestamp().map(UInt64::toString).orElse("unknown"), + message.getDecodedMessage().getDecodedMessage().orElse(Bytes.EMPTY)); + P2P_LOG.onGossipRejected( + getTopic(), + message.getDecodedMessage().getDecodedMessage().orElse(Bytes.EMPTY), + internalValidationResult.getDescription()); break; case IGNORE: LOG.trace("Ignoring message for topic: {}", this::getTopic); @@ -168,12 +170,11 @@ protected ValidationResult handleMessageProcessingError( final PreparedGossipMessage message, final Throwable err) { final ValidationResult response; if (ExceptionUtil.hasCause(err, DecodingException.class)) { - final String savedFile = - p2pDumpManager.saveGossipMessageDecodingError( - getTopic(), - message.getArrivalTimestamp().map(UInt64::toString).orElse("unknown"), - message.getOriginalMessage()); - P2P_LOG.onGossipMessageDecodingError(getTopic(), savedFile, err); + p2pDumpManager.saveGossipMessageDecodingError( + getTopic(), + message.getArrivalTimestamp().map(UInt64::toString).orElse("unknown"), + message.getOriginalMessage()); + P2P_LOG.onGossipMessageDecodingError(getTopic(), message.getOriginalMessage(), err); response = ValidationResult.Invalid; } else if (ExceptionUtil.hasCause(err, RejectedExecutionException.class)) { LOG.warn( 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 509b240483b..4d666d31a00 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 @@ -321,7 +321,9 @@ public BeaconChainController( eventChannels.getPublisher(ReceivedBlockEventsChannel.class); this.forkChoiceExecutor = new AsyncRunnerEventThread("forkchoice", asyncRunnerFactory); this.p2pDumpManager = - new P2PDumpManager(serviceConfig.getDataDirLayout().getP2pDumpDirectory()); + new P2PDumpManager( + serviceConfig.getDataDirLayout().getDebugDataDirectory(), + beaconConfig.p2pConfig().isP2pDumpsToFileEnabled()); this.futureItemsMetric = SettableLabelledGauge.create( metricsSystem, diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/DataOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/DataOptions.java index 591162603ca..0fb7573ad03 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/DataOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/DataOptions.java @@ -27,14 +27,6 @@ public abstract class DataOptions { arity = "1") private Path dataBasePath = DataConfig.defaultDataPath(); - @Option( - hidden = true, - names = {"--Xp2p-log-to-file-directory"}, - paramLabel = "", - description = "Path to base directory where P2P log hex dumps will be saved.", - arity = "1") - private Path p2pDumpsDataPath; - public DataConfig getDataConfig() { return configureDataConfig(DataConfig.builder()).build(); } @@ -48,6 +40,6 @@ public void configure(TekuConfiguration.Builder builder) { } protected DataConfig.Builder configureDataConfig(final DataConfig.Builder config) { - return config.dataBasePath(dataBasePath).p2pDumpsDataPath(p2pDumpsDataPath); + return config.dataBasePath(dataBasePath); } } diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/P2POptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/P2POptions.java index 3af1824c318..af5f56f1f40 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/P2POptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/P2POptions.java @@ -284,6 +284,16 @@ public class P2POptions { hidden = true) private int batchVerifyMaxBatchSize = P2PConfig.DEFAULT_BATCH_VERIFY_MAX_BATCH_SIZE; + @Option( + names = {"--Xp2p-dumps-to-file-enabled"}, + paramLabel = "", + showDefaultValue = Visibility.ALWAYS, + description = "Enables saving P2P dumps to file.", + hidden = true, + arity = "0..1", + fallbackValue = "true") + private boolean p2pDumpsToFileEnabled = P2PConfig.DEFAULT_P2P_DUMPS_TO_FILE_ENABLED; + @Option( names = {"--Xp2p-batch-verify-signatures-strict-thread-limit-enabled"}, paramLabel = "", @@ -350,7 +360,8 @@ public void configure(final TekuConfiguration.Builder builder) { .isGossipScoringEnabled(gossipScoringEnabled) .peerRateLimit(peerRateLimit) .allTopicsFilterEnabled(allTopicsFilterEnabled) - .peerRequestLimit(peerRequestLimit)) + .peerRequestLimit(peerRequestLimit) + .p2pDumpsToFileEnabled(p2pDumpsToFileEnabled)) .discovery( d -> { if (p2pDiscoveryBootnodes != null) { diff --git a/teku/src/main/java/tech/pegasys/teku/cli/subcommand/debug/DebugToolsCommand.java b/teku/src/main/java/tech/pegasys/teku/cli/subcommand/debug/DebugToolsCommand.java index 0419e300518..bc13f304a32 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/subcommand/debug/DebugToolsCommand.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/subcommand/debug/DebugToolsCommand.java @@ -146,8 +146,7 @@ public int generateSwaggerDocs( tempDir.toFile().deleteOnExit(); DataDirLayout dataDirLayout = - new SeparateServiceDataDirLayout( - tempDir, Optional.empty(), Optional.empty(), Optional.empty()); + new SeparateServiceDataDirLayout(tempDir, Optional.empty(), Optional.empty()); final KeyManager keyManager = new NoOpKeyManager(); final AsyncRunnerFactory asyncRunnerFactory = diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ValidatorSourceFactoryTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ValidatorSourceFactoryTest.java index 000b9c22d4b..e6687b6bdd0 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ValidatorSourceFactoryTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ValidatorSourceFactoryTest.java @@ -48,8 +48,7 @@ class ValidatorSourceFactoryTest { @Test void mutableValidatorShouldBeSlashingProtected(@TempDir final Path tempDir) { final DataDirLayout dataDirLayout = - new SeparateServiceDataDirLayout( - tempDir, Optional.empty(), Optional.empty(), Optional.empty()); + new SeparateServiceDataDirLayout(tempDir, Optional.empty(), Optional.empty()); final ValidatorSourceFactory factory = new ValidatorSourceFactory( spec,