From 07cadd5b7139c8bac727ca81be52546a3af78df0 Mon Sep 17 00:00:00 2001 From: Fredrik Svantes Date: Fri, 24 Sep 2021 13:20:10 +0200 Subject: [PATCH 1/7] Adding missing space (#4407) Simply adding a space after "tracking" in "Set strategy for handling performance tracking." + "Valid values: ${COMPLETION-CANDIDATES}" --- .../java/tech/pegasys/teku/cli/options/ValidatorOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java index 903e5931b82..1a4d6eda491 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java @@ -60,7 +60,7 @@ public class ValidatorOptions { names = {"--validators-performance-tracking-mode"}, paramLabel = "", description = - "Set strategy for handling performance tracking." + "Set strategy for handling performance tracking. " + "Valid values: ${COMPLETION-CANDIDATES}", arity = "1") private ValidatorPerformanceTrackingMode validatorPerformanceTrackingMode = From d368fd44ec43eb93923dd4c150a6649d82798e43 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Sat, 25 Sep 2021 11:36:23 +1000 Subject: [PATCH 2/7] Schedule Altair fork for MainNet (#4410) --- CHANGELOG.md | 1 + .../test/java/tech/pegasys/teku/spec/SpecFactoryTest.java | 6 +++--- .../pegasys/teku/cli/options/Eth2NetworkOptionsTest.java | 4 ++-- .../tech/pegasys/teku/util/config/configs/mainnet.yaml | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 322c2c3e0f0..9b28f3cda23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ For information on changes in released versions of Teku, see the [releases page] ## Unreleased Changes ### Additions and Improvements +- Sets Altair fork epoch for MainNet to 74240 - Upgraded to BLST 0.3.5. - `/teku/v1/admin/readiness` endpoint now accepts a `target_peer_count` param to require a minimum number of peers before the node is considered ready. - Support for building on JDK 17. diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/SpecFactoryTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/SpecFactoryTest.java index c793e0508cf..f42bd83f575 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/SpecFactoryTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/SpecFactoryTest.java @@ -31,12 +31,12 @@ public class SpecFactoryTest { - private static final Set ALTAIR_NETWORKS = Set.of("pyrmont", "prater"); + private static final Set ALTAIR_NETWORKS = Set.of("pyrmont", "prater", "mainnet"); @Test - public void defaultFactoryShouldOnlySupportPhase0_mainnet() { + public void defaultFactoryShouldScheduleAltairForMainNet() { final Spec spec = SpecFactory.create("mainnet"); - assertThat(spec.getForkSchedule().getSupportedMilestones()).containsExactly(PHASE0); + assertThat(spec.getForkSchedule().getSupportedMilestones()).containsExactly(PHASE0, ALTAIR); } @ParameterizedTest(name = "{0}") diff --git a/teku/src/test/java/tech/pegasys/teku/cli/options/Eth2NetworkOptionsTest.java b/teku/src/test/java/tech/pegasys/teku/cli/options/Eth2NetworkOptionsTest.java index 064c7790400..bb9f6c961a1 100644 --- a/teku/src/test/java/tech/pegasys/teku/cli/options/Eth2NetworkOptionsTest.java +++ b/teku/src/test/java/tech/pegasys/teku/cli/options/Eth2NetworkOptionsTest.java @@ -24,11 +24,11 @@ class Eth2NetworkOptionsTest extends AbstractBeaconNodeCommandTest { @Test - void shouldNotEnableAltairByDefault() { + void shouldEnableAltairByDefault() { final TekuConfiguration config = getTekuConfigurationFromArguments(); final Spec spec = config.eth2NetworkConfiguration().getSpec(); assertThat(spec.getForkSchedule().getHighestSupportedMilestone()) - .isEqualTo(SpecMilestone.PHASE0); + .isEqualTo(SpecMilestone.ALTAIR); } @Test diff --git a/util/src/main/resources/tech/pegasys/teku/util/config/configs/mainnet.yaml b/util/src/main/resources/tech/pegasys/teku/util/config/configs/mainnet.yaml index b9342206abb..160aac53b6c 100644 --- a/util/src/main/resources/tech/pegasys/teku/util/config/configs/mainnet.yaml +++ b/util/src/main/resources/tech/pegasys/teku/util/config/configs/mainnet.yaml @@ -26,7 +26,7 @@ GENESIS_DELAY: 604800 # Altair ALTAIR_FORK_VERSION: 0x01000000 -ALTAIR_FORK_EPOCH: 18446744073709551615 +ALTAIR_FORK_EPOCH: 74240 # Oct 27, 2021, 10:56:23am UTC # Merge MERGE_FORK_VERSION: 0x02000000 MERGE_FORK_EPOCH: 18446744073709551615 From eea27d05d9dbdb4513faff63ceffc96da6a31d44 Mon Sep 17 00:00:00 2001 From: Regio Michelin Date: Mon, 27 Sep 2021 10:01:45 +1000 Subject: [PATCH 3/7] Failed to export slashing protection data. (#4406) SyncDataAcessor does a validation if the received `Path` is a path or a file before checking the OS support to atomic file moves. Also added new testcases validating positive cases for the export and import features. --- CHANGELOG.md | 1 + .../teku/data/SlashingProtectionExporter.java | 3 +- .../teku/data/SlashingProtectionImporter.java | 3 +- .../data/SlashingProtectionExporterTest.java | 14 ++++++ .../data/SlashingProtectionImporterTest.java | 44 +++++++++++++++++++ .../infrastructure/io/SyncDataAccessor.java | 11 ++--- 6 files changed, 69 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b28f3cda23..ea3d370fd97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,3 +27,4 @@ For information on changes in released versions of Teku, see the [releases page] ### Bug Fixes - Fix `NoSuchElementException` reported at startup by validator performance tracking module. - Fix `IllegalStateException` reported on altair networks, when none of your validators are in a sync committee for the current epoch. +- Fix failed to export/import slashing protection data. diff --git a/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionExporter.java b/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionExporter.java index 97fded2a065..63244417fd1 100644 --- a/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionExporter.java +++ b/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionExporter.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -45,7 +46,7 @@ public class SlashingProtectionExporter { public SlashingProtectionExporter(final SubCommandLogger log, final String path) { this.log = log; - this.syncDataAccessor = SyncDataAccessor.createWithoutAtomicMove(); + syncDataAccessor = SyncDataAccessor.create(Paths.get(path)); } public void initialise(final Path slashProtectionPath) { diff --git a/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionImporter.java b/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionImporter.java index cfb468a2560..35935393f4a 100644 --- a/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionImporter.java +++ b/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionImporter.java @@ -22,6 +22,7 @@ import java.io.File; import java.io.IOException; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -47,7 +48,7 @@ public class SlashingProtectionImporter { public SlashingProtectionImporter(final SubCommandLogger log, final String path) { this.log = log; - syncDataAccessor = SyncDataAccessor.createWithoutAtomicMove(); + syncDataAccessor = SyncDataAccessor.create(Paths.get(path)); } public void initialise(final File inputFile) throws IOException { diff --git a/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionExporterTest.java b/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionExporterTest.java index da8fec4a5b8..11564b21bf8 100644 --- a/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionExporterTest.java +++ b/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionExporterTest.java @@ -155,6 +155,20 @@ public void shouldPrintIfFileCannotBeRead(@TempDir Path tempDir) assertThat(stringArgs.getValue()).startsWith("Failed to read from file"); } + @Test + public void shouldExportSlashProtection(@TempDir Path tempDir) + throws IOException, URISyntaxException { + final Path exportedFile = tempDir.resolve("exportedFile.json").toAbsolutePath(); + final SlashingProtectionExporter exporter = + new SlashingProtectionExporter(logger, tempDir.toString()); + + exporter.readSlashProtectionFile(usingResourceFile("slashProtection.yml", tempDir)); + + assertThat(Files.exists(exportedFile)).isFalse(); + exporter.saveToFile(exportedFile.toString()); + assertThat(Files.exists(exportedFile)).isTrue(); + } + private File usingResourceFile(final String resourceFileName, final Path tempDir) throws URISyntaxException, IOException { final Path tempFile = tempDir.resolve(pubkey + ".yml").toAbsolutePath(); diff --git a/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionImporterTest.java b/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionImporterTest.java index f58a8698629..59705383bf1 100644 --- a/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionImporterTest.java +++ b/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionImporterTest.java @@ -22,13 +22,19 @@ import java.io.File; import java.io.IOException; import java.net.URISyntaxException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.mockito.ArgumentCaptor; import tech.pegasys.teku.data.slashinginterchange.Metadata; import tech.pegasys.teku.infrastructure.logging.SubCommandLogger; public class SlashingProtectionImporterTest { private final ArgumentCaptor stringArgs = ArgumentCaptor.forClass(String.class); + private final String pubkey = + "b845089a1457f811bfc000588fbb4e713669be8ce060ea6be3c6ece09afc3794106c91ca73acda5e5457122d58723bed"; @Test public void shouldFailWithParseError() throws URISyntaxException, IOException { @@ -55,6 +61,34 @@ public void shouldFailIfMetadataNotPresent() throws IOException, URISyntaxExcept assertThat(errorString).contains("does not appear to have metadata"); } + @Test + public void shouldExportAndImportFile(@TempDir Path tempDir) + throws IOException, URISyntaxException { + final SubCommandLogger logger = mock(SubCommandLogger.class); + final Path exportedFile = tempDir.resolve("exportedFile.json").toAbsolutePath(); + + final SlashingProtectionExporter exporter = + new SlashingProtectionExporter(logger, tempDir.toString()); + final File ruleFile = usingResourceFile("slashProtection.yml", tempDir); + exporter.readSlashProtectionFile(ruleFile); + final String originalFileContent = Files.readString(ruleFile.toPath()); + + assertThat(Files.exists(ruleFile.toPath())).isTrue(); + assertThat(Files.exists(exportedFile)).isFalse(); + exporter.saveToFile(exportedFile.toString()); + ruleFile.delete(); + assertThat(Files.exists(exportedFile)).isTrue(); + assertThat(Files.exists(ruleFile.toPath())).isFalse(); + + SlashingProtectionImporter importer = + new SlashingProtectionImporter(logger, exportedFile.toString()); + importer.initialise(new File(exportedFile.toString())); + importer.updateLocalRecords(tempDir); + assertThat(Files.exists(ruleFile.toPath())).isTrue(); + + assertThat(originalFileContent).isEqualTo(Files.readString(ruleFile.toPath())); + } + private String loadAndGetErrorText(final String resourceFile) throws URISyntaxException, IOException { final SubCommandLogger logger = mock(SubCommandLogger.class); @@ -65,4 +99,14 @@ private String loadAndGetErrorText(final String resourceFile) verify(logger).exit(eq(1), stringArgs.capture()); return stringArgs.getValue(); } + + private File usingResourceFile(final String resourceFileName, final Path tempDir) + throws URISyntaxException, IOException { + final Path tempFile = tempDir.resolve(pubkey + ".yml").toAbsolutePath(); + Files.copy( + new File(Resources.getResource(resourceFileName).toURI()).toPath(), + tempFile, + StandardCopyOption.REPLACE_EXISTING); + return tempFile.toFile(); + } } diff --git a/infrastructure/io/src/main/java/tech/pegasys/teku/infrastructure/io/SyncDataAccessor.java b/infrastructure/io/src/main/java/tech/pegasys/teku/infrastructure/io/SyncDataAccessor.java index 5194a018610..e6a2b5904bf 100644 --- a/infrastructure/io/src/main/java/tech/pegasys/teku/infrastructure/io/SyncDataAccessor.java +++ b/infrastructure/io/src/main/java/tech/pegasys/teku/infrastructure/io/SyncDataAccessor.java @@ -41,14 +41,15 @@ public class SyncDataAccessor { this.atomicFileMoveSupport = atomicFileMoveSupport; } - public static SyncDataAccessor createWithoutAtomicMove() { - return new SyncDataAccessor(false); - } - public static SyncDataAccessor create(final Path path) { boolean atomicFileMoveSupport = false; - final Path tmpFile = path.resolve("syncWriteTest.tmp"); + final Path tmpFile; + if (Files.isDirectory(path.toAbsolutePath())) { + tmpFile = path.toAbsolutePath().resolve("syncWriteTest.tmp"); + } else { + tmpFile = path.toAbsolutePath().getParent().resolve("syncWriteTest.tmp"); + } try { atomicSyncedWrite(tmpFile, Bytes32.ZERO); From 4c8392558bc8a04a7774372367174f658eb2675e Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Mon, 27 Sep 2021 15:51:48 +1000 Subject: [PATCH 4/7] Fix "Failed to produce sync_signature" errors (#4414) * Ignore duplicate sync committee messages instead of rejecting * Avoid create multiple sync committee messages for the same slot --- CHANGELOG.md | 1 + .../SyncCommitteeMessageValidator.java | 3 ++- .../client/SyncCommitteeScheduler.java | 14 +++++++++++ .../client/SyncCommitteeSchedulerTest.java | 25 +++++++++++++++++++ 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea3d370fd97..87f4c2b575b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,3 +28,4 @@ For information on changes in released versions of Teku, see the [releases page] - Fix `NoSuchElementException` reported at startup by validator performance tracking module. - Fix `IllegalStateException` reported on altair networks, when none of your validators are in a sync committee for the current epoch. - Fix failed to export/import slashing protection data. +- Fix incorrect gossip validation when processing duplicate sync committee messages which could lead to an error being reported instead of ignoring the duplicate. diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/synccommittee/SyncCommitteeMessageValidator.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/synccommittee/SyncCommitteeMessageValidator.java index 0316ee2dcd9..af32cc889af 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/synccommittee/SyncCommitteeMessageValidator.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/synccommittee/SyncCommitteeMessageValidator.java @@ -16,6 +16,7 @@ import static java.util.stream.Collectors.toList; import static tech.pegasys.teku.statetransition.validation.InternalValidationResult.ACCEPT; import static tech.pegasys.teku.statetransition.validation.InternalValidationResult.IGNORE; +import static tech.pegasys.teku.statetransition.validation.InternalValidationResult.ignore; import static tech.pegasys.teku.statetransition.validation.InternalValidationResult.reject; import static tech.pegasys.teku.util.config.Constants.VALID_SYNC_COMMITTEE_MESSAGE_SET_SIZE; @@ -187,7 +188,7 @@ private SafeFuture validateWithState( return reject("Rejecting sync committee message because the signature is invalid"); } if (!seenIndices.addAll(uniquenessKeys)) { - return reject( + return ignore( "Ignoring sync committee message as a duplicate was processed during validation"); } return ACCEPT; diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/SyncCommitteeScheduler.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/SyncCommitteeScheduler.java index cfeb2ea2139..f71fb4667d0 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/SyncCommitteeScheduler.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/SyncCommitteeScheduler.java @@ -15,6 +15,8 @@ import java.util.Optional; import java.util.stream.Stream; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.tuweni.bytes.Bytes32; import org.hyperledger.besu.plugin.services.MetricsSystem; import tech.pegasys.teku.infrastructure.unsigned.UInt64; @@ -35,6 +37,7 @@ * subnet subscriptions are renewed if the reconnection was because the beacon chain restarted. */ public class SyncCommitteeScheduler implements ValidatorTimingChannel { + private static final Logger LOG = LogManager.getLogger(); private final MetricsSystem metricsSystem; private final Spec spec; @@ -43,6 +46,7 @@ public class SyncCommitteeScheduler implements ValidatorTimingChannel { private Optional currentSyncCommitteePeriod = Optional.empty(); private Optional nextSyncCommitteePeriod = Optional.empty(); + private UInt64 lastProductionSlot; public SyncCommitteeScheduler( final MetricsSystem metricsSystem, @@ -109,6 +113,16 @@ private SyncCommitteePeriod createSyncCommitteePeriod( @Override public void onAttestationCreationDue(final UInt64 slot) { + // Check slot being null for the edge case of genesis slot (i.e. slot 0) + if (lastProductionSlot != null && slot.compareTo(lastProductionSlot) <= 0) { + LOG.debug( + "Not producing sync committee message for slot {} because last production slot {} is beyond that.", + slot, + lastProductionSlot); + return; + } + + lastProductionSlot = slot; getDutiesForSlot(slot).ifPresent(duties -> duties.onProductionDue(slot)); } diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/SyncCommitteeSchedulerTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/SyncCommitteeSchedulerTest.java index bfa4963fb19..6ca23a135dc 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/SyncCommitteeSchedulerTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/SyncCommitteeSchedulerTest.java @@ -98,6 +98,31 @@ void shouldPerformProductionForEachSlotWhenAttestationCreationDue() { }); } + @Test + void shouldNotPerformProductionMultipleTimesForSameSlot() { + scheduler.onSlot(UInt64.ONE); + + getRequestedDutiesForSyncCommitteePeriod(0).complete(Optional.of(duties)); + + scheduler.onAttestationCreationDue(UInt64.ONE); + scheduler.onAttestationCreationDue(UInt64.ONE); + + verify(duties, times(1)).performProductionDuty(UInt64.ONE); + } + + @Test + void shouldNotPerformProductionForEarlierSlot() { + scheduler.onSlot(UInt64.ONE); + + getRequestedDutiesForSyncCommitteePeriod(0).complete(Optional.of(duties)); + + scheduler.onAttestationCreationDue(UInt64.valueOf(2)); + scheduler.onAttestationCreationDue(UInt64.ONE); + + verify(duties).performProductionDuty(UInt64.valueOf(2)); + verify(duties, never()).performProductionDuty(UInt64.ONE); + } + @Test void shouldPerformAggregationForEachSlotWhenAttestationAggregationDue() { scheduler.onSlot(UInt64.ONE); From 189bb1ec1ca71fab6c5b247f3dde2fbd82c48727 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Tue, 28 Sep 2021 12:12:43 +1000 Subject: [PATCH 5/7] Add additional verification of snappy data sizes (#4419) --- CHANGELOG.md | 1 + .../compression/snappy/SnappyFrameDecoder.java | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87f4c2b575b..9d11c6bc970 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,3 +29,4 @@ For information on changes in released versions of Teku, see the [releases page] - Fix `IllegalStateException` reported on altair networks, when none of your validators are in a sync committee for the current epoch. - Fix failed to export/import slashing protection data. - Fix incorrect gossip validation when processing duplicate sync committee messages which could lead to an error being reported instead of ignoring the duplicate. +- Added additional verification of message lengths when decoding snappy messages. diff --git a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/core/encodings/compression/snappy/SnappyFrameDecoder.java b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/core/encodings/compression/snappy/SnappyFrameDecoder.java index d0b5354aa7f..cb2af4f4c6e 100644 --- a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/core/encodings/compression/snappy/SnappyFrameDecoder.java +++ b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/core/encodings/compression/snappy/SnappyFrameDecoder.java @@ -17,6 +17,7 @@ import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; +import io.netty.handler.codec.compression.DecompressionException; import io.netty.handler.codec.compression.Snappy; import java.util.Optional; import tech.pegasys.teku.networking.eth2.rpc.core.encodings.AbstractByteBufDecoder; @@ -43,7 +44,12 @@ private enum ChunkType { } private static final int SNAPPY_IDENTIFIER_LEN = 6; + // See https://github.com/google/snappy/blob/1.1.9/framing_format.txt#L95 private static final int MAX_UNCOMPRESSED_DATA_SIZE = 65536 + 4; + // See https://github.com/google/snappy/blob/1.1.9/framing_format.txt#L82 + private static final int MAX_DECOMPRESSED_DATA_SIZE = 65536; + // See https://github.com/google/snappy/blob/1.1.9/framing_format.txt#L82 + private static final int MAX_COMPRESSED_CHUNK_SIZE = 16777216 - 1; private final Snappy snappy = new Snappy(); private final boolean validateChecksums; @@ -164,13 +170,21 @@ protected Optional decodeOneImpl(ByteBuf in) throws CompressionExceptio throw new CompressionException("Received COMPRESSED_DATA tag before STREAM_IDENTIFIER"); } + if (chunkLength > MAX_COMPRESSED_CHUNK_SIZE) { + throw new DecompressionException( + "Received COMPRESSED_DATA that contains" + + " chunk that exceeds " + + MAX_COMPRESSED_CHUNK_SIZE + + " bytes"); + } + if (inSize < 4 + chunkLength) { return Optional.empty(); } in.skipBytes(4); int checksum = in.readIntLE(); - ByteBuf uncompressed = Unpooled.buffer(); + ByteBuf uncompressed = Unpooled.buffer(chunkLength, MAX_DECOMPRESSED_DATA_SIZE); try { if (validateChecksums) { int oldWriterIndex = in.writerIndex(); From 6a6283a975af85cac2955ea3cdfbc248923fb965 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Tue, 28 Sep 2021 13:34:11 +1000 Subject: [PATCH 6/7] Fix incompatibilies when importing, exporting and repairing slashing protection records. (#4421) --- .../teku/data/SlashingProtectionImporter.java | 49 +++++++++------ .../slashinginterchange/SigningHistory.java | 19 ++++-- .../data/SlashingProtectionExporterTest.java | 30 ++++++++++ .../data/SlashingProtectionImporterTest.java | 60 +++++++++++++++++++ .../slashProtectionNoAttestations.yml | 4 ++ .../slashProtectionWithGenesisRoot.yml | 2 +- 6 files changed, 141 insertions(+), 23 deletions(-) create mode 100644 data/dataexchange/src/test/resources/slashProtectionNoAttestations.yml diff --git a/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionImporter.java b/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionImporter.java index 35935393f4a..905d16ea374 100644 --- a/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionImporter.java +++ b/data/dataexchange/src/main/java/tech/pegasys/teku/data/SlashingProtectionImporter.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; import tech.pegasys.teku.data.signingrecord.ValidatorSigningRecord; @@ -99,23 +100,33 @@ private List summariseCompleteInterchangeFormat( } private SigningHistory signingHistoryConverter(final SigningHistory signingHistory) { - final Optional lastSlot = - signingHistory.signedBlocks.stream().map(SignedBlock::getSlot).max(UInt64::compareTo); - final Optional sourceEpoch = - signingHistory.signedAttestations.stream() - .map(SignedAttestation::getSourceEpoch) - .max(UInt64::compareTo); - final Optional targetEpoch = - signingHistory.signedAttestations.stream() - .map(SignedAttestation::getTargetEpoch) - .max(UInt64::compareTo); - final ValidatorSigningRecord record = - new ValidatorSigningRecord( - metadata.genesisValidatorsRoot, - lastSlot.orElse(UInt64.ZERO), - sourceEpoch.orElse(ValidatorSigningRecord.NEVER_SIGNED), - targetEpoch.orElse(ValidatorSigningRecord.NEVER_SIGNED)); - return new SigningHistory(signingHistory.pubkey, record); + try { + final Optional lastSlot = + signingHistory.signedBlocks.stream() + .map(SignedBlock::getSlot) + .filter(Objects::nonNull) + .max(UInt64::compareTo); + final Optional sourceEpoch = + signingHistory.signedAttestations.stream() + .map(SignedAttestation::getSourceEpoch) + .filter(Objects::nonNull) + .max(UInt64::compareTo); + final Optional targetEpoch = + signingHistory.signedAttestations.stream() + .map(SignedAttestation::getTargetEpoch) + .filter(Objects::nonNull) + .max(UInt64::compareTo); + final ValidatorSigningRecord record = + new ValidatorSigningRecord( + metadata.genesisValidatorsRoot, + lastSlot.orElse(UInt64.ZERO), + sourceEpoch.orElse(ValidatorSigningRecord.NEVER_SIGNED), + targetEpoch.orElse(ValidatorSigningRecord.NEVER_SIGNED)); + return new SigningHistory(signingHistory.pubkey, record); + } catch (NullPointerException e) { + System.out.println(signingHistory.pubkey); + throw e; + } } public void updateLocalRecords(final Path slashingProtectionPath) { @@ -134,10 +145,12 @@ private void updateLocalRecord(final SigningHistory signingHistory) { try { existingRecord = syncDataAccessor.read(outputFile).map(ValidatorSigningRecord::fromBytes); } catch (IOException e) { - log.exit(1, "Failed to read existing file: " + outputFile.toString()); + log.exit(1, "Failed to read existing file: " + outputFile); } } if (existingRecord.isPresent() + && existingRecord.get().getGenesisValidatorsRoot() != null + && metadata.genesisValidatorsRoot != null && metadata.genesisValidatorsRoot.compareTo(existingRecord.get().getGenesisValidatorsRoot()) != 0) { log.exit( diff --git a/data/dataexchange/src/main/java/tech/pegasys/teku/data/slashinginterchange/SigningHistory.java b/data/dataexchange/src/main/java/tech/pegasys/teku/data/slashinginterchange/SigningHistory.java index 6aef0a8eca9..a741c7952ea 100644 --- a/data/dataexchange/src/main/java/tech/pegasys/teku/data/slashinginterchange/SigningHistory.java +++ b/data/dataexchange/src/main/java/tech/pegasys/teku/data/slashinginterchange/SigningHistory.java @@ -13,6 +13,8 @@ package tech.pegasys.teku.data.slashinginterchange; +import static tech.pegasys.teku.data.signingrecord.ValidatorSigningRecord.isNeverSigned; + import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.MoreObjects; @@ -50,9 +52,12 @@ public SigningHistory(final BLSPubKey pubkey, final ValidatorSigningRecord recor this.signedBlocks = new ArrayList<>(); signedBlocks.add(new SignedBlock(record.getBlockSlot(), null)); this.signedAttestations = new ArrayList<>(); - signedAttestations.add( - new SignedAttestation( - record.getAttestationSourceEpoch(), record.getAttestationTargetEpoch(), null)); + if (!isNeverSigned(record.getAttestationSourceEpoch()) + || !isNeverSigned(record.getAttestationTargetEpoch())) { + signedAttestations.add( + new SignedAttestation( + record.getAttestationSourceEpoch(), record.getAttestationTargetEpoch(), null)); + } } @Override @@ -83,15 +88,21 @@ public ValidatorSigningRecord toValidatorSigningRecord( final Optional maybeRecord, final Bytes32 genesisValidatorsRoot) { final UInt64 lastSignedBlockSlot = - signedBlocks.stream().map(SignedBlock::getSlot).max(UInt64::compareTo).orElse(UInt64.ZERO); + signedBlocks.stream() + .map(SignedBlock::getSlot) + .filter(Objects::nonNull) + .max(UInt64::compareTo) + .orElse(UInt64.ZERO); final UInt64 lastSignedAttestationSourceEpoch = signedAttestations.stream() .map(SignedAttestation::getSourceEpoch) + .filter(Objects::nonNull) .max(UInt64::compareTo) .orElse(null); final UInt64 lastSignedAttestationTargetEpoch = signedAttestations.stream() .map(SignedAttestation::getTargetEpoch) + .filter(Objects::nonNull) .max(UInt64::compareTo) .orElse(null); diff --git a/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionExporterTest.java b/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionExporterTest.java index 11564b21bf8..b3409341d4d 100644 --- a/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionExporterTest.java +++ b/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionExporterTest.java @@ -39,6 +39,7 @@ import tech.pegasys.teku.cli.OSUtils; import tech.pegasys.teku.data.signingrecord.ValidatorSigningRecord; import tech.pegasys.teku.data.slashinginterchange.Metadata; +import tech.pegasys.teku.data.slashinginterchange.SignedBlock; import tech.pegasys.teku.data.slashinginterchange.SigningHistory; import tech.pegasys.teku.data.slashinginterchange.SlashingProtectionInterchangeFormat; import tech.pegasys.teku.infrastructure.logging.SubCommandLogger; @@ -169,6 +170,35 @@ public void shouldExportSlashProtection(@TempDir Path tempDir) assertThat(Files.exists(exportedFile)).isTrue(); } + @Test + void shouldHaveNoSignedAttestationsWhenNoAttestationsSigned(@TempDir Path tempDir) + throws Exception { + final Path exportedFile = tempDir.resolve("exportedFile.json").toAbsolutePath(); + final SlashingProtectionExporter exporter = + new SlashingProtectionExporter(logger, tempDir.toString()); + + final UInt64 blockSlot = UInt64.ONE; + final ValidatorSigningRecord signingRecord = + new ValidatorSigningRecord(validatorsRoot) + .maySignBlock(validatorsRoot, blockSlot) + .orElseThrow(); + final Path recordFile = tempDir.resolve(pubkey + ".yml"); + Files.write(recordFile, signingRecord.toBytes().toArrayUnsafe()); + exporter.readSlashProtectionFile(recordFile.toFile()); + + assertThat(exportedFile).doesNotExist(); + exporter.saveToFile(exportedFile.toString()); + assertThat(exportedFile).exists(); + + final SlashingProtectionInterchangeFormat exportedRecords = + jsonProvider.jsonToObject( + Files.readString(exportedFile), SlashingProtectionInterchangeFormat.class); + assertThat(exportedRecords.data).hasSize(1); + final SigningHistory signingHistory = exportedRecords.data.get(0); + assertThat(signingHistory.signedBlocks).containsExactly(new SignedBlock(blockSlot, null)); + assertThat(signingHistory.signedAttestations).isEmpty(); + } + private File usingResourceFile(final String resourceFileName, final Path tempDir) throws URISyntaxException, IOException { final Path tempFile = tempDir.resolve(pubkey + ".yml").toAbsolutePath(); diff --git a/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionImporterTest.java b/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionImporterTest.java index 59705383bf1..24327bc6042 100644 --- a/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionImporterTest.java +++ b/data/dataexchange/src/test/java/tech/pegasys/teku/data/SlashingProtectionImporterTest.java @@ -14,8 +14,10 @@ package tech.pegasys.teku.data; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import com.google.common.io.Resources; @@ -25,11 +27,14 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import org.apache.tuweni.bytes.Bytes; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.mockito.ArgumentCaptor; +import tech.pegasys.teku.data.signingrecord.ValidatorSigningRecord; import tech.pegasys.teku.data.slashinginterchange.Metadata; import tech.pegasys.teku.infrastructure.logging.SubCommandLogger; +import tech.pegasys.teku.infrastructure.unsigned.UInt64; public class SlashingProtectionImporterTest { private final ArgumentCaptor stringArgs = ArgumentCaptor.forClass(String.class); @@ -89,6 +94,61 @@ public void shouldExportAndImportFile(@TempDir Path tempDir) assertThat(originalFileContent).isEqualTo(Files.readString(ruleFile.toPath())); } + @Test + void shouldImportFileOverRepairedRecords(@TempDir Path tempDir) throws Exception { + final SubCommandLogger logger = mock(SubCommandLogger.class); + final Path initialRecords = tempDir.resolve("initial"); + final Path repairedRecords = tempDir.resolve("repaired"); + assertThat(initialRecords.toFile().mkdirs()).isTrue(); + assertThat(repairedRecords.toFile().mkdirs()).isTrue(); + + final Path exportedFile = tempDir.resolve("exportedFile.json").toAbsolutePath(); + final File initialRuleFile = + usingResourceFile("slashProtectionWithGenesisRoot.yml", initialRecords); + final File repairedRuleFile = + usingResourceFile("slashProtectionWithGenesisRoot.yml", repairedRecords); + + final SlashingProtectionExporter exporter = + new SlashingProtectionExporter(logger, tempDir.toString()); + exporter.readSlashProtectionFile(repairedRuleFile); + final String originalFileContent = Files.readString(initialRuleFile.toPath()); + + assertThat(exportedFile).doesNotExist(); + exporter.saveToFile(exportedFile.toString()); + assertThat(exportedFile).exists(); + + final SlashingProtectionRepairer repairer = + SlashingProtectionRepairer.create(logger, repairedRecords, true); + final UInt64 repairedSlot = UInt64.valueOf(1566); + final UInt64 repairedEpoch = UInt64.valueOf(7668); + repairer.updateRecords(repairedSlot, repairedEpoch); + verify(logger, never()).error(any()); + verify(logger, never()).error(any(), any()); + assertThat(Files.readString(repairedRuleFile.toPath())).isNotEqualTo(originalFileContent); + + SlashingProtectionImporter importer = + new SlashingProtectionImporter(logger, exportedFile.toString()); + importer.initialise(exportedFile.toFile()); + importer.updateLocalRecords(repairedRecords); + + // Should wind up with a file that contains the slot and epochs from the repair, combined with + // the genesis root from the initial file + final ValidatorSigningRecord initialRecord = loadSigningRecord(initialRuleFile); + final ValidatorSigningRecord importedRecord = loadSigningRecord(repairedRuleFile); + assertThat(importedRecord) + .isEqualTo( + new ValidatorSigningRecord( + initialRecord.getGenesisValidatorsRoot(), + repairedSlot, + repairedEpoch, + repairedEpoch)); + } + + private ValidatorSigningRecord loadSigningRecord(final File repairedRuleFile) throws IOException { + return ValidatorSigningRecord.fromBytes( + Bytes.wrap(Files.readAllBytes(repairedRuleFile.toPath()))); + } + private String loadAndGetErrorText(final String resourceFile) throws URISyntaxException, IOException { final SubCommandLogger logger = mock(SubCommandLogger.class); diff --git a/data/dataexchange/src/test/resources/slashProtectionNoAttestations.yml b/data/dataexchange/src/test/resources/slashProtectionNoAttestations.yml new file mode 100644 index 00000000000..2c06848eb03 --- /dev/null +++ b/data/dataexchange/src/test/resources/slashProtectionNoAttestations.yml @@ -0,0 +1,4 @@ +--- +lastSignedBlockSlot: 327 +lastSignedAttestationSourceEpoch: 51 +lastSignedAttestationTargetEpoch: 1741 diff --git a/data/dataexchange/src/test/resources/slashProtectionWithGenesisRoot.yml b/data/dataexchange/src/test/resources/slashProtectionWithGenesisRoot.yml index 5b9f23fe414..cccb2de4277 100644 --- a/data/dataexchange/src/test/resources/slashProtectionWithGenesisRoot.yml +++ b/data/dataexchange/src/test/resources/slashProtectionWithGenesisRoot.yml @@ -1,5 +1,5 @@ --- +genesisValidatorsRoot: "0x6e2c5d8a89dfe121a92c8812bea69fe9f84ae48f63aafe34ef7e18c7eac9af70" lastSignedBlockSlot: 327 lastSignedAttestationSourceEpoch: 51 lastSignedAttestationTargetEpoch: 1741 -genesisValidatorsRoot: "0x6e2c5d8a89dfe121a92c8812bea69fe9f84ae48f63aafe34ef7e18c7eac9af70" From c846d7f793a3103842fcf88c8b3ec27d02b17aca Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Tue, 28 Sep 2021 13:51:31 +1000 Subject: [PATCH 7/7] Update to 1.1.0 version of reference tests. (#4420) --- build.gradle | 2 +- .../java/tech/pegasys/teku/reference/Eth2ReferenceTestCase.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index b61825e9e17..f223c1f4c56 100644 --- a/build.gradle +++ b/build.gradle @@ -225,7 +225,7 @@ allprojects { } } -def refTestVersion = 'v1.1.0-beta.4' +def refTestVersion = 'v1.1.0' def blsRefTestVersion = 'v0.1.0' def refTestBaseUrl = 'https://github.com/ethereum/eth2.0-spec-tests/releases/download' def blsRefTestBaseUrl = 'https://github.com/ethereum/bls12-381-tests/releases/download' diff --git a/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/Eth2ReferenceTestCase.java b/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/Eth2ReferenceTestCase.java index 194d4b3a1af..d32aad115e8 100644 --- a/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/Eth2ReferenceTestCase.java +++ b/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/Eth2ReferenceTestCase.java @@ -46,6 +46,7 @@ public abstract class Eth2ReferenceTestCase { .putAll(OperationsTestExecutor.OPERATIONS_TEST_TYPES) .putAll(SanityTests.SANITY_TEST_TYPES) .putAll(SszTestExecutorDeprecated.SSZ_TEST_TYPES) + .put("merkle/single_proof", TestExecutor.IGNORE_TESTS) .build(); private final ImmutableMap PHASE_0_TEST_TYPES =