Accept empty header set in range headers validation (hyperledger#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 hyperledger#3336 Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
gfukushima · Jul 28, 2022 · 962c805 · 962c805
1 parent 3913174
commit 962c805
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 38 deletions.
diff --git 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
@@ -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<BlockHeader> headersToImport;
 
   public RangeHeaders(
       final SyncTargetRange checkpointRange, final List<BlockHeader> 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<BlockHeader> getHeadersToImport() {
     return headersToImport;
   }
 
-  public BlockHeader getFirstHeaderToImport() {
-    return headersToImport.get(0);
+  public Optional<BlockHeader> getFirstHeaderToImport() {
+    return headersToImport.size() > 0 ? Optional.of(headersToImport.get(0)) : Optional.empty();
   }
 
   @Override

diff --git a/...rc/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersValidationStep.java b/...rc/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersValidationStep.java
@@ -43,30 +43,35 @@ public RangeHeadersValidationStep(
   @Override
   public Stream<BlockHeader> 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/.../src/test/java/org/hyperledger/besu/ethereum/eth/sync/RangeHeadersValidationStepTest.java b/.../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<BlockHeader> result = validationStep.apply(emptyRangeHeaders);
+    assertThat(result).isEmpty();
+
+    verifyNoMoreInteractions(
+        protocolSchedule,
+        protocolSpec,
+        protocolContext,
+        headerValidator,
+        validationPolicy,
+        syncTarget);
+  }
 }