Remain in optimistic mode if there is no viable head

CHANGELOG.md

@@ -22,6 +22,7 @@ For information on changes in released versions of Teku, see the [releases page]
   This simplifies using the standard REST API to retrieve the initial state as just the base URL can be specified (e.g. `--initial-state https://<[email protected]`)
 - Ability to configure multiple beacon nodes for a single validator client using `--beacon-node-api-endpoints` CLI option
 - Primed cache for new justified checkpoints to reduce time required to run fork choice immediately after justification
+- Remain in optimistic mode when there are no viable branches in the block tree because blocks from every branch were marked INVALID during optimistic sync (https://github.com/ethereum/consensus-specs/pull/2955)
 
 ### Bug Fixes
 - Fixed `NullPointerException` when checking for the terminal PoW block while the EL was syncing

ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/generator/ChainBuilder.java

@@ -318,6 +318,32 @@ public SignedBlockAndState generateBlockAtSlot(final UInt64 slot, final BlockOpt
     return appendNewBlockToChain(slot, options);
   }
 
+  public List<SignedBlockAndState> finalizeCurrentChain() {
+    final UInt64 chainHeadSlot = getLatestSlot();
+    final UInt64 finalizeEpoch = spec.computeEpochAtSlot(chainHeadSlot).max(2);
+    final UInt64 finalHeadEpoch = finalizeEpoch.plus(3);
+    final UInt64 finalHeadSlot = spec.computeStartSlotAtEpoch(finalHeadEpoch);
+
+    final List<SignedBlockAndState> addedBlockAndStates = new ArrayList<>();
+    SignedBlockAndState newChainHead = null;
+    for (UInt64 slot = chainHeadSlot.plus(1);
+        slot.isLessThan(finalHeadSlot);
+        slot = slot.increment()) {
+      final BlockOptions blockOptions = BlockOptions.create();
+      streamValidAttestationsForBlockAtSlot(slot).forEach(blockOptions::addAttestation);
+      newChainHead = generateBlockAtSlot(slot, blockOptions);
+      addedBlockAndStates.add(newChainHead);
+    }
+    final Checkpoint finalizedCheckpoint = newChainHead.getState().getFinalizedCheckpoint();
+    assertThat(finalizedCheckpoint.getEpoch())
+        .describedAs("Failed to finalize epoch %s", finalizeEpoch)
+        .isEqualTo(finalizeEpoch);
+    assertThat(finalizedCheckpoint.getRoot())
+        .describedAs("Failed to finalize epoch %s", finalizeEpoch)
+        .isNotEqualTo(Bytes32.ZERO);
+    return addedBlockAndStates;
+  }
+
   /**
    * Utility for streaming valid attestations available for inclusion at the given slot. This
    * utility can be used to assign valid attestations to a generated block.

storage/src/main/java/tech/pegasys/teku/storage/protoarray/ForkChoiceStrategy.java

@@ -200,7 +200,7 @@ public ForkChoiceState getForkChoiceState(
           headExecutionBlockHash,
           justifiedExecutionHash,
           finalizedExecutionHash,
-          headNode.isOptimistic());
+          headNode.isOptimistic() || !protoArray.nodeIsViableForHead(headNode));
     } finally {
       protoArrayLock.readLock().unlock();
     }

storage/src/main/java/tech/pegasys/teku/storage/protoarray/ProtoArray.java

@@ -241,7 +241,8 @@ private Optional<ProtoNode> findHead(
 
     // Perform a sanity check that the node is indeed valid to be the head.
     if (!nodeIsViableForHead(bestNode) && !bestNode.equals(justifiedNode)) {
-      throw new RuntimeException("ProtoArray: Best node is not viable for head");
+      throw new IllegalStateException(
+          "ProtoArray: Best node " + bestNode.toLogString() + " is not viable for head");
     }
     return Optional.of(bestNode);
   }

storage/src/main/java/tech/pegasys/teku/storage/protoarray/ProtoNode.java

@@ -21,6 +21,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import org.apache.tuweni.bytes.Bytes32;
+import tech.pegasys.teku.infrastructure.logging.LogFormatter;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 import tech.pegasys.teku.spec.datastructures.blocks.BlockCheckpoints;
 import tech.pegasys.teku.spec.datastructures.forkchoice.ProtoNodeData;
@@ -270,4 +271,8 @@ public String toString() {
         .add("validationStatus", validationStatus)
         .toString();
   }
+
+  public String toLogString() {
+    return LogFormatter.formatBlock(blockSlot, blockRoot);
+  }
 }

storage/src/test/java/tech/pegasys/teku/storage/protoarray/ForkChoiceStrategyTest.java

@@ -38,6 +38,7 @@
 import tech.pegasys.teku.spec.datastructures.blocks.Eth1Data;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBlockAndState;
+import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot;
 import tech.pegasys.teku.spec.datastructures.forkchoice.ProtoNodeData;
 import tech.pegasys.teku.spec.datastructures.forkchoice.ReadOnlyForkChoiceStrategy;
 import tech.pegasys.teku.spec.datastructures.forkchoice.TestStoreFactory;
@@ -47,10 +48,13 @@
 import tech.pegasys.teku.spec.datastructures.state.AnchorPoint;
 import tech.pegasys.teku.spec.datastructures.state.Checkpoint;
 import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState;
+import tech.pegasys.teku.spec.executionlayer.ForkChoiceState;
 import tech.pegasys.teku.spec.executionlayer.PayloadStatus;
 import tech.pegasys.teku.spec.generator.ChainBuilder;
 import tech.pegasys.teku.spec.generator.ChainBuilder.BlockOptions;
 import tech.pegasys.teku.spec.util.DataStructureUtil;
+import tech.pegasys.teku.storage.client.ChainUpdater;
+import tech.pegasys.teku.storage.client.RecentChainData;
 import tech.pegasys.teku.storage.storageSystem.InMemoryStorageSystemBuilder;
 import tech.pegasys.teku.storage.storageSystem.StorageSystem;
 
@@ -453,6 +457,64 @@ void applyPendingVotes_shouldMarkEquivocation() {
     assertThat(transaction3.getVote(ZERO).getNextRoot()).isEqualTo(block1.getRoot());
   }
 
+  @Test
+  void shouldConsiderHeadOptimisticWhenItIsNotViable() {
+    // If we optimistically import blocks which include enough attestations to update the justified
+    // checkpoint, then later invalidate some non-justified blocks such that the head block no
+    // longer includes enough attestations to support that justification, then none of our blocks
+    // will be viable because they won't have the same checkpoints as our store.
+    // In that case we will use the justified checkpoint as chain head and should remain in
+    // optimistic sync mode
+
+    final StorageSystem storageSystem = InMemoryStorageSystemBuilder.buildDefault(spec);
+    final ChainUpdater chainUpdater = storageSystem.chainUpdater();
+    chainUpdater.initializeGenesisWithPayload(true);
+    final ForkChoiceStrategy protoArray = getProtoArray(storageSystem);
+    final RecentChainData recentChainData = storageSystem.recentChainData();
+    final SignedBlockAndState optimisticHead = chainUpdater.finalizeCurrentChainOptimistically();
+    ProtoNodeData firstJustifiedData =
+        protoArray.getBlockData(optimisticHead.getRoot()).orElseThrow();
+    ProtoNodeData lastUnjustifiedData =
+        protoArray.getBlockData(firstJustifiedData.getParentRoot()).orElseThrow();
+    final Checkpoint currentJustified = recentChainData.getJustifiedCheckpoint().orElseThrow();
+    while (lastUnjustifiedData.getCheckpoints().getJustifiedCheckpoint().equals(currentJustified)) {
+      firstJustifiedData = lastUnjustifiedData;
+      lastUnjustifiedData =
+          protoArray.getBlockData(lastUnjustifiedData.getParentRoot()).orElseThrow();
+    }
+    assertThat(firstJustifiedData.getCheckpoints().getJustifiedCheckpoint())
+        .isEqualTo(currentJustified);
+    assertThat(lastUnjustifiedData.getCheckpoints().getJustifiedCheckpoint())
+        .isNotEqualTo(currentJustified);
+
+    // Invalidate blocks that updated the justified checkpoint
+    protoArray.onExecutionPayloadResult(
+        firstJustifiedData.getRoot(),
+        PayloadStatus.invalid(Optional.empty(), Optional.of("No good, very bad block")),
+        true);
+    // And validate the justified checkpoint
+    protoArray.onExecutionPayloadResult(currentJustified.getRoot(), PayloadStatus.VALID, false);
+
+    // Find new chain head
+    final SlotAndBlockRoot revertHead =
+        protoArray.findHead(
+            recentChainData.getJustifiedCheckpoint().orElseThrow(),
+            recentChainData.getFinalizedCheckpoint().orElseThrow());
+    recentChainData.updateHead(revertHead.getBlockRoot(), optimisticHead.getSlot());
+
+    final ForkChoiceState forkChoiceState =
+        protoArray.getForkChoiceState(
+            recentChainData.getJustifiedCheckpoint().orElseThrow(),
+            recentChainData.getFinalizedCheckpoint().orElseThrow());
+
+    // Should have reverted to the justified checkpoint as head
+    assertThat(forkChoiceState.getHeadBlockRoot()).isEqualTo(currentJustified.getRoot());
+    // The current head block itself is fully validated
+    assertThat(protoArray.isFullyValidated(currentJustified.getRoot())).isTrue();
+    // But we consider the chain head optimistic because of the updated justified checkpoint
+    assertThat(forkChoiceState.isHeadOptimistic()).isTrue();
+  }
+
   private StorageSystem initStorageSystem() {
     return initStorageSystem(spec);
   }

storage/src/testFixtures/java/tech/pegasys/teku/storage/client/ChainUpdater.java

@@ -17,6 +17,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static tech.pegasys.teku.infrastructure.time.TimeUtilities.secondsToMillis;
 
+import java.util.List;
 import java.util.Optional;
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
@@ -30,7 +31,6 @@
 import tech.pegasys.teku.spec.datastructures.state.Checkpoint;
 import tech.pegasys.teku.spec.executionlayer.PayloadStatus;
 import tech.pegasys.teku.spec.generator.ChainBuilder;
-import tech.pegasys.teku.spec.generator.ChainBuilder.BlockOptions;
 import tech.pegasys.teku.spec.schemas.SchemaDefinitionsBellatrix;
 import tech.pegasys.teku.storage.store.UpdatableStore.StoreTransaction;
 
@@ -143,31 +143,19 @@ public SignedBlockAndState finalizeEpoch(final UInt64 epoch) {
   }
 
   public SignedBlockAndState finalizeCurrentChain() {
-    final UInt64 chainHeadSlot = recentChainData.getHeadSlot();
-    final UInt64 finalizeEpoch = spec.computeEpochAtSlot(chainHeadSlot).max(2);
-    final UInt64 finalHeadEpoch = finalizeEpoch.plus(3);
-    final UInt64 finalHeadSlot = spec.computeStartSlotAtEpoch(finalHeadEpoch);
-
-    SignedBlockAndState newChainHead = null;
-    for (UInt64 slot = chainHeadSlot.plus(1);
-        slot.isLessThan(finalHeadSlot);
-        slot = slot.increment()) {
-      final BlockOptions blockOptions = BlockOptions.create();
-      chainBuilder
-          .streamValidAttestationsForBlockAtSlot(slot)
-          .forEach(blockOptions::addAttestation);
-      newChainHead = chainBuilder.generateBlockAtSlot(slot, blockOptions);
-      saveBlock(newChainHead);
-      updateBestBlock(newChainHead);
-    }
-    final Checkpoint finalizedCheckpoint = newChainHead.getState().getFinalizedCheckpoint();
-    assertThat(finalizedCheckpoint.getEpoch())
-        .describedAs("Failed to finalize epoch %s", finalizeEpoch)
-        .isEqualTo(finalizeEpoch);
-    assertThat(finalizedCheckpoint.getRoot())
-        .describedAs("Failed to finalize epoch %s", finalizeEpoch)
-        .isNotEqualTo(Bytes32.ZERO);
-    return newChainHead;
+    final List<SignedBlockAndState> newBlocks = chainBuilder.finalizeCurrentChain();
+    newBlocks.forEach(this::saveBlock);
+    final SignedBlockAndState newHead = newBlocks.get(newBlocks.size() - 1);
+    updateBestBlock(newHead);
+    return newHead;
+  }
+
+  public SignedBlockAndState finalizeCurrentChainOptimistically() {
+    final List<SignedBlockAndState> newBlocks = chainBuilder.finalizeCurrentChain();
+    newBlocks.forEach(this::saveOptimisticBlock);
+    final SignedBlockAndState newHead = newBlocks.get(newBlocks.size() - 1);
+    updateBestBlock(newHead);
+    return newHead;
   }
 
   public SignedBlockAndState justifyEpoch(final long epoch) {