From 962c805cae2258729e4443386ce625ab391cc45f Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Thu, 28 Jul 2022 16:53:55 -0600 Subject: [PATCH] Accept empty header set in range headers validation (#4189) If a response to the get header P2P request only returns the header that is the start of the range we may need to trim it to an empty response, this is breaking the validation response. One client has started returning only the range header start in some circumstances (which is a valid response per spec). Because we sometimes request an overlapping header this results in an empty stream once we cut the duplicates. fixes #3336 Signed-off-by: Danno Ferrin --- CHANGELOG.md | 7 ++- .../ethereum/eth/sync/range/RangeHeaders.java | 14 ++--- .../range/RangeHeadersValidationStep.java | 51 ++++++++++--------- .../sync/RangeHeadersValidationStepTest.java | 18 +++++++ 4 files changed, 52 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c82a904be7..c8a285d99a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,12 @@ - Deprecation warning for Ropsten, Rinkeby, Kiln [#4173](https://github.com/hyperledger/besu/pull/4173) ### Bug Fixes - +- Stop producing stack traces when a get headers response only contains the range start header [#4189](https://github.com/hyperledger/besu/pull/4189) ## 22.7.0-RC3 -Known/Outstanding issues: -Besu requires a restart post-merge to re-enable remote transaction processing [#3890](https://github.com/hyperledger/besu/issues/3890) +### Known/Outstanding issues: +- Besu requires a restart post-merge to re-enable remote transaction processing [#3890](https://github.com/hyperledger/besu/issues/3890) ### Additions and Improvements - Engine API: Change expiration time for JWT tokens to 60s [#4168](https://github.com/hyperledger/besu/pull/4168) @@ -28,7 +28,6 @@ Besu requires a restart post-merge to re-enable remote transaction processing [# - https://hyperledger.jfrog.io/artifactory/besu-binaries/besu/22.7.0-RC3/besu-22.7.0-RC3.tar.gz / sha256: `6a1ee89c82db9fa782d34733d8a8c726670378bcb71befe013da48d7928490a6` - https://hyperledger.jfrog.io/artifactory/besu-binaries/besu/22.7.0-RC3/besu-22.7.0-RC3.zip / sha256: `5de22445ab2a270cf33e1850cd28f1946442b7104738f0d1ac253a009c53414e` - ## 22.7.0-RC2 ### Additions and Improvements diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeaders.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeaders.java index 250104cac59..bff5ceeffac 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeaders.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeaders.java @@ -14,29 +14,21 @@ */ package org.hyperledger.besu.ethereum.eth.sync.range; -import static com.google.common.base.Preconditions.checkArgument; - import org.hyperledger.besu.ethereum.core.BlockHeader; import java.util.List; import java.util.Objects; +import java.util.Optional; import com.google.common.base.MoreObjects; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class RangeHeaders { - private static final Logger LOG = LoggerFactory.getLogger(RangeHeaders.class); private final SyncTargetRange range; private final List headersToImport; public RangeHeaders( final SyncTargetRange checkpointRange, final List headersToImport) { - if (headersToImport.isEmpty()) { - LOG.debug(String.format("Headers list empty. Range: %s", checkpointRange.toString())); - } - checkArgument(!headersToImport.isEmpty(), "Must have at least one header to import"); this.range = checkpointRange; this.headersToImport = headersToImport; } @@ -49,8 +41,8 @@ public List getHeadersToImport() { return headersToImport; } - public BlockHeader getFirstHeaderToImport() { - return headersToImport.get(0); + public Optional getFirstHeaderToImport() { + return headersToImport.size() > 0 ? Optional.of(headersToImport.get(0)) : Optional.empty(); } @Override diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersValidationStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersValidationStep.java index 0ed34491737..79ed3c53108 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersValidationStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersValidationStep.java @@ -43,30 +43,35 @@ public RangeHeadersValidationStep( @Override public Stream apply(final RangeHeaders rangeHeaders) { final BlockHeader rangeStart = rangeHeaders.getRange().getStart(); - final BlockHeader firstHeaderToImport = rangeHeaders.getFirstHeaderToImport(); - if (isValid(rangeStart, firstHeaderToImport)) { - return rangeHeaders.getHeadersToImport().stream(); - } else { - final String rangeEndDescription; - if (rangeHeaders.getRange().hasEnd()) { - final BlockHeader rangeEnd = rangeHeaders.getRange().getEnd(); - rangeEndDescription = - String.format("#%d (%s)", rangeEnd.getNumber(), rangeEnd.getBlockHash()); - } else { - rangeEndDescription = "chain head"; - } - final String errorMessage = - String.format( - "Invalid range headers. Headers downloaded between #%d (%s) and %s do not connect at #%d (%s)", - rangeStart.getNumber(), - rangeStart.getHash(), - rangeEndDescription, - firstHeaderToImport.getNumber(), - firstHeaderToImport.getHash()); - throw new InvalidBlockException( - errorMessage, firstHeaderToImport.getNumber(), firstHeaderToImport.getHash()); - } + return rangeHeaders + .getFirstHeaderToImport() + .map( + firstHeader -> { + if (isValid(rangeStart, firstHeader)) { + return rangeHeaders.getHeadersToImport().stream(); + } else { + final String rangeEndDescription; + if (rangeHeaders.getRange().hasEnd()) { + final BlockHeader rangeEnd = rangeHeaders.getRange().getEnd(); + rangeEndDescription = + String.format("#%d (%s)", rangeEnd.getNumber(), rangeEnd.getBlockHash()); + } else { + rangeEndDescription = "chain head"; + } + final String errorMessage = + String.format( + "Invalid range headers. Headers downloaded between #%d (%s) and %s do not connect at #%d (%s)", + rangeStart.getNumber(), + rangeStart.getHash(), + rangeEndDescription, + firstHeader.getNumber(), + firstHeader.getHash()); + throw new InvalidBlockException( + errorMessage, firstHeader.getNumber(), firstHeader.getHash()); + } + }) + .orElse(Stream.empty()); } private boolean isValid(final BlockHeader expectedParent, final BlockHeader firstHeaderToImport) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/RangeHeadersValidationStepTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/RangeHeadersValidationStepTest.java index 89b5930c29f..478adbfdc62 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/RangeHeadersValidationStepTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/RangeHeadersValidationStepTest.java @@ -35,6 +35,7 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; +import java.util.List; import java.util.stream.Stream; import org.junit.Before; @@ -107,4 +108,21 @@ public void shouldThrowExceptionWhenValidationFails() { + firstHeader.getHash() + ")"); } + + @Test + public void acceptResponseWithNoHeaders() { + var emptyRangeHeaders = + new RangeHeaders(new SyncTargetRange(syncTarget, rangeStart, rangeEnd), List.of()); + + final Stream result = validationStep.apply(emptyRangeHeaders); + assertThat(result).isEmpty(); + + verifyNoMoreInteractions( + protocolSchedule, + protocolSpec, + protocolContext, + headerValidator, + validationPolicy, + syncTarget); + } }