Add is_optimistic field to /eth/v1/node/syncing (#5347)

Also, adjusting syncing status to cater for bellatrix and optimistic blocks. fixes #5287 Signed-off-by: Paul Harris <[email protected]>
Consensys · Apr 14, 2022 · 4116197 · 4116197
1 parent 3efa75f
commit 4116197
Show file tree

Hide file tree

Showing 8 changed files with 87 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,8 +19,10 @@ For information on changes in released versions of Teku, see the [releases page]
 
 ### Additions and Improvements
 - Improved performance when regenerating non-finalized states that had to be dropped from memory.
+- Performance optimizations for Gnosis beacon chain
 - Performance optimizations for Gnosis beacon chain.
 - Improved performance when processing epoch transitions.
+- Added `is_optimistic` field to `/eth/v1/node/syncing` response.
 
 ### Bug Fixes
 - Added stricter limits on attestation pool size. 

diff --git a/...egration-test/java/tech/pegasys/teku/beaconrestapi/v1/node/GetSyncingIntegrationTest.java b/...egration-test/java/tech/pegasys/teku/beaconrestapi/v1/node/GetSyncingIntegrationTest.java
@@ -22,6 +22,7 @@
 import org.junit.jupiter.api.Test;
 import tech.pegasys.teku.api.response.v1.node.Syncing;
 import tech.pegasys.teku.api.response.v1.node.SyncingResponse;
+import tech.pegasys.teku.beacon.sync.events.SyncState;
 import tech.pegasys.teku.beacon.sync.events.SyncingStatus;
 import tech.pegasys.teku.beaconrestapi.AbstractDataBackedRestAPIIntegrationTest;
 import tech.pegasys.teku.beaconrestapi.handlers.v1.node.GetSyncing;
@@ -33,6 +34,7 @@ public class GetSyncingIntegrationTest extends AbstractDataBackedRestAPIIntegrat
   public void shouldGetSyncStatusWhenSyncing() throws IOException {
     startRestAPIAtGenesis();
     when(syncService.getSyncStatus()).thenReturn(getSyncStatus(true, 1, 10, 15));
+    when(syncService.getCurrentSyncState()).thenReturn(SyncState.SYNCING);
 
     final Response response = get();
     assertThat(response.code()).isEqualTo(SC_OK);
@@ -46,6 +48,7 @@ public void shouldGetSyncStatusWhenSyncing() throws IOException {
   public void shouldGetSyncStatusWhenNotSyncing() throws IOException {
     startRestAPIAtGenesis();
     when(syncService.getSyncStatus()).thenReturn(getSyncStatus(false, 6, 11, 16));
+    when(syncService.getCurrentSyncState()).thenReturn(SyncState.IN_SYNC);
 
     final Response response = get();
     assertThat(response.code()).isEqualTo(SC_OK);

diff --git a/...rces/tech/pegasys/teku/beaconrestapi/beacon/migrated/schema/GetSyncingStatusResponse.json b/...rces/tech/pegasys/teku/beaconrestapi/beacon/migrated/schema/GetSyncingStatusResponse.json
@@ -21,6 +21,9 @@
         },
         "is_syncing" : {
           "type" : "boolean"
+        },
+        "is_optimistic" : {
+          "type" : "boolean"
         }
       }
     }

diff --git a/...src/integration-test/resources/tech/pegasys/teku/beaconrestapi/beacon/schema/Syncing.json b/...src/integration-test/resources/tech/pegasys/teku/beaconrestapi/beacon/schema/Syncing.json
@@ -14,6 +14,10 @@
     "is_syncing" : {
       "type" : "boolean",
       "description" : "Set to true if the node is syncing, false if the node is synced."
+    },
+    "is_optimistic" : {
+      "type" : "boolean",
+      "description" : "Set to true if the node is optimistically fetching blocks."
     }
   }
 }
diff --git a/...conrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/node/GetSyncing.java b/...conrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/node/GetSyncing.java
@@ -30,11 +30,13 @@
 import io.javalin.plugin.openapi.annotations.OpenApi;
 import io.javalin.plugin.openapi.annotations.OpenApiContent;
 import io.javalin.plugin.openapi.annotations.OpenApiResponse;
+import java.util.Optional;
 import java.util.function.Function;
 import org.jetbrains.annotations.NotNull;
 import tech.pegasys.teku.api.DataProvider;
 import tech.pegasys.teku.api.SyncDataProvider;
 import tech.pegasys.teku.api.response.v1.node.SyncingResponse;
+import tech.pegasys.teku.beacon.sync.events.SyncState;
 import tech.pegasys.teku.beacon.sync.events.SyncingStatus;
 import tech.pegasys.teku.beaconrestapi.MigratingEndpointAdapter;
 import tech.pegasys.teku.infrastructure.json.types.SerializableTypeDefinition;
@@ -45,15 +47,16 @@
 public class GetSyncing extends MigratingEndpointAdapter {
   public static final String ROUTE = "/eth/v1/node/syncing";
 
-  private static final SerializableTypeDefinition<SyncingStatus> SYNC_RESPONSE_DATA_TYPE =
-      SerializableTypeDefinition.object(SyncingStatus.class)
-          .withField("head_slot", UINT64_TYPE, SyncingStatus::getCurrentSlot)
-          .withField("sync_distance", UINT64_TYPE, GetSyncing::getSlotsBehind)
-          .withField("is_syncing", BOOLEAN_TYPE, SyncingStatus::isSyncing)
+  private static final SerializableTypeDefinition<SyncStatusData> SYNC_RESPONSE_DATA_TYPE =
+      SerializableTypeDefinition.object(SyncStatusData.class)
+          .withField("head_slot", UINT64_TYPE, SyncStatusData::getCurrentSlot)
+          .withField("sync_distance", UINT64_TYPE, SyncStatusData::getSlotsBehind)
+          .withField("is_syncing", BOOLEAN_TYPE, SyncStatusData::isSyncing)
+          .withOptionalField("is_optimistic", BOOLEAN_TYPE, SyncStatusData::isOptimistic)
           .build();
 
-  private static final SerializableTypeDefinition<SyncingStatus> SYNCING_RESPONSE_TYPE =
-      SerializableTypeDefinition.object(SyncingStatus.class)
+  private static final SerializableTypeDefinition<SyncStatusData> SYNCING_RESPONSE_TYPE =
+      SerializableTypeDefinition.object(SyncStatusData.class)
           .name("GetSyncingStatusResponse")
           .withField("data", SYNC_RESPONSE_DATA_TYPE, Function.identity())
           .build();
@@ -99,14 +102,47 @@ public void handle(@NotNull final Context ctx) throws Exception {
 
   @Override
   public void handleRequest(RestApiRequest request) throws JsonProcessingException {
-    request.respondOk(syncProvider.getSyncingStatus());
+    request.respondOk(new SyncStatusData(syncProvider));
   }
 
-  private static UInt64 getSlotsBehind(final SyncingStatus syncingStatus) {
-    if (syncingStatus.isSyncing() && syncingStatus.getHighestSlot().isPresent()) {
-      final UInt64 highestSlot = syncingStatus.getHighestSlot().get();
-      return highestSlot.minusMinZero(syncingStatus.getCurrentSlot());
+  static class SyncStatusData {
+    private final boolean isSyncing;
+    private final Optional<Boolean> isOptimistic;
+    private final UInt64 currentSlot;
+    private final UInt64 slotsBehind;
+
+    public SyncStatusData(final SyncDataProvider syncProvider) {
+      final SyncingStatus status = syncProvider.getSyncingStatus();
+      final SyncState syncState = syncProvider.getCurrentSyncState();
+      this.isSyncing = !syncState.isInSync();
+      this.isOptimistic = Optional.of(syncState.isOptimistic());
+      this.currentSlot = status.getCurrentSlot();
+      // do this last, after isSyncing is calculated
+      this.slotsBehind = calculateSlotsBehind(status);
+    }
+
+    public boolean isSyncing() {
+      return isSyncing;
+    }
+
+    public Optional<Boolean> isOptimistic() {
+      return isOptimistic;
+    }
+
+    public UInt64 getCurrentSlot() {
+      return currentSlot;
+    }
+
+    public UInt64 getSlotsBehind() {
+      return slotsBehind;
+    }
+
+    private UInt64 calculateSlotsBehind(final SyncingStatus syncingStatus) {
+      if (isSyncing && syncingStatus.getHighestSlot().isPresent()) {
+        final UInt64 highestSlot = syncingStatus.getHighestSlot().get();
+        return highestSlot.minusMinZero(syncingStatus.getCurrentSlot());
+      }
+      return UInt64.ZERO;
     }
-    return UInt64.ZERO;
   }
 }
diff --git a/...estapi/src/test/java/tech/pegasys/teku/beaconrestapi/handlers/v1/node/GetSyncingTest.java b/...estapi/src/test/java/tech/pegasys/teku/beaconrestapi/handlers/v1/node/GetSyncingTest.java
@@ -19,6 +19,7 @@
 
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
+import tech.pegasys.teku.beacon.sync.events.SyncState;
 import tech.pegasys.teku.beaconrestapi.AbstractBeaconHandlerTest;
 import tech.pegasys.teku.infrastructure.restapi.endpoints.RestApiRequest;
 
@@ -29,6 +30,7 @@ public class GetSyncingTest extends AbstractBeaconHandlerTest {
   public void shouldGetSyncingStatusSyncing() throws Exception {
     GetSyncing handler = new GetSyncing(syncDataProvider);
     final RestApiRequest request = new RestApiRequest(context, handler.getMetadata());
+    when(syncService.getCurrentSyncState()).thenReturn(SyncState.SYNCING);
     when(syncService.getSyncStatus()).thenReturn(getSyncStatus(true, 1, 7, 10));
 
     handler.handleRequest(request);
@@ -40,6 +42,7 @@ public void shouldGetSyncStatusInSync() throws Exception {
     GetSyncing handler = new GetSyncing(syncDataProvider);
     final RestApiRequest request = new RestApiRequest(context, handler.getMetadata());
     when(syncService.getSyncStatus()).thenReturn(getSyncStatus(false, 1, 10, 11));
+    when(syncService.getCurrentSyncState()).thenReturn(SyncState.IN_SYNC);
 
     handler.handleRequest(request);
     checkResponse("10", "0", false);
@@ -48,7 +51,7 @@ public void shouldGetSyncStatusInSync() throws Exception {
   private void checkResponse(String headSlot, String syncDistance, boolean isSyncing) {
     final String expectedResponse =
         String.format(
-            "{\"data\":{\"head_slot\":\"%s\",\"sync_distance\":\"%s\",\"is_syncing\":%s}}",
+            "{\"data\":{\"head_slot\":\"%s\",\"sync_distance\":\"%s\",\"is_syncing\":%s,\"is_optimistic\":false}}",
             headSlot, syncDistance, isSyncing);
 
     verify(context).result(args.capture());

diff --git a/data/provider/src/main/java/tech/pegasys/teku/api/SyncDataProvider.java b/data/provider/src/main/java/tech/pegasys/teku/api/SyncDataProvider.java
@@ -13,11 +13,10 @@
 
 package tech.pegasys.teku.api;
 
-import tech.pegasys.teku.api.response.v1.node.Syncing;
 import tech.pegasys.teku.beacon.sync.SyncService;
+import tech.pegasys.teku.beacon.sync.events.SyncState;
 import tech.pegasys.teku.beacon.sync.events.SyncStateProvider;
 import tech.pegasys.teku.beacon.sync.events.SyncingStatus;
-import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 
 public class SyncDataProvider {
 
@@ -27,12 +26,6 @@ public SyncDataProvider(SyncService syncService) {
     this.syncService = syncService;
   }
 
-  public Syncing getSyncing() {
-    SyncingStatus syncStatus = syncService.getSyncStatus();
-    return new Syncing(
-        syncStatus.getCurrentSlot(), getSlotsBehind(syncStatus), syncStatus.isSyncing());
-  }
-
   public SyncingStatus getSyncingStatus() {
     return syncService.getSyncStatus();
   }
@@ -52,11 +45,7 @@ public boolean isSyncing() {
     return !syncService.getCurrentSyncState().isInSync();
   }
 
-  private UInt64 getSlotsBehind(final SyncingStatus syncingStatus) {
-    if (syncingStatus.isSyncing() && syncingStatus.getHighestSlot().isPresent()) {
-      final UInt64 highestSlot = syncingStatus.getHighestSlot().get();
-      return highestSlot.minusMinZero(syncingStatus.getCurrentSlot());
-    }
-    return UInt64.ZERO;
+  public SyncState getCurrentSyncState() {
+    return syncService.getCurrentSyncState();
   }
 }
diff --git a/data/serializer/src/main/java/tech/pegasys/teku/api/response/v1/node/Syncing.java b/data/serializer/src/main/java/tech/pegasys/teku/api/response/v1/node/Syncing.java
@@ -16,12 +16,14 @@
 import static tech.pegasys.teku.api.schema.SchemaConstants.PATTERN_UINT64;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.MoreObjects;
 import io.swagger.v3.oas.annotations.media.Schema;
 import java.util.Objects;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 
+@JsonInclude(JsonInclude.Include.NON_NULL)
 public class Syncing {
   @JsonProperty("head_slot")
   @Schema(type = "string", pattern = PATTERN_UINT64, description = "Beacon node's head slot")
@@ -40,14 +42,26 @@ public class Syncing {
       description = "Set to true if the node is syncing, false if the node is synced.")
   public final boolean isSyncing;
 
+  @JsonProperty("is_optimistic")
+  @Schema(
+      type = "boolean",
+      description = "Set to true if the node is optimistically fetching blocks.")
+  public final Boolean isOptimistic;
+
+  public Syncing(final UInt64 headSlot, final UInt64 syncDistance, final boolean isSyncing) {
+    this(headSlot, syncDistance, isSyncing, false);
+  }
+
   @JsonCreator
   public Syncing(
       @JsonProperty("head_slot") final UInt64 headSlot,
       @JsonProperty("sync_distance") final UInt64 syncDistance,
-      @JsonProperty("is_syncing") final boolean isSyncing) {
+      @JsonProperty("is_syncing") final boolean isSyncing,
+      @JsonProperty("is_optimistic") final boolean isOptimistic) {
     this.headSlot = headSlot;
     this.syncDistance = syncDistance;
     this.isSyncing = isSyncing;
+    this.isOptimistic = isOptimistic;
   }
 
   @Override
@@ -61,12 +75,13 @@ public boolean equals(final Object o) {
     final Syncing syncing = (Syncing) o;
     return isSyncing == syncing.isSyncing
         && Objects.equals(headSlot, syncing.headSlot)
-        && Objects.equals(syncDistance, syncing.syncDistance);
+        && Objects.equals(syncDistance, syncing.syncDistance)
+        && Objects.equals(isOptimistic, syncing.isOptimistic);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(headSlot, syncDistance, isSyncing);
+    return Objects.hash(headSlot, syncDistance, isSyncing, isOptimistic);
   }
 
   @Override
@@ -75,6 +90,7 @@ public String toString() {
         .add("headSlot", headSlot)
         .add("syncDistance", syncDistance)
         .add("isSyncing", isSyncing)
+        .add("isOptimistic", isOptimistic)
         .toString();
   }
 }