Full Progressive Balances Improvements

eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/ManualReferenceTestRunner.java

@@ -39,13 +39,13 @@ public class ManualReferenceTestRunner extends Eth2ReferenceTestCase {
    * <p>e.g. set to "ssz_static" to run only ssz static tests or "ssz_static/Attestation" for only
    * attestation ssz tests.
    */
-  private static final String TEST_TYPE = "sync/optimistic";
+  private static final String TEST_TYPE = "fork_choice";
 
   /** Filter test to run to those from the specified spec. One of general, minimal or mainnet */
-  private static final String SPEC = "";
+  private static final String SPEC = "minimal";
 
   /** Filter test to run only those for a specific milestone. Use values from TestFork. */
-  private static final String MILESTONE = null;
+  private static final String MILESTONE = "phase0";
 
   /** Filter tests to run only those where the display name contains this string. */
   private static final String DISPLAY_NAME = "";

eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/ForkChoiceTestExecutor.java

@@ -139,6 +139,7 @@ spec, new SignedBlockAndState(anchorBlock, anchorState)),
               .collect(Collectors.joining("\n"));
       throw new AssertionError(
           e.getMessage()
+              + "\n-------"
               + "\nJustified checkpoint: "
               + recentChainData.getJustifiedCheckpoint().orElse(null)
               + "\nFinalized checkpoint: "

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/BeaconStateUtil.java

@@ -138,7 +138,7 @@ public Optional<Bytes32> getCurrentDutyDependentRoot(
         forkChoiceStrategy, block.getRoot(), miscHelpers.computeEpochAtSlot(block.getSlot()));
   }
 
-  public List<UInt64> getEffectiveBalances(final BeaconState state) {
+  public List<UInt64> getEffectiveActiveUnslashedBalances(final BeaconState state) {
     return BeaconStateCache.getTransitionCaches(state)
         .getEffectiveBalances()
         .get(
@@ -147,7 +147,7 @@ public List<UInt64> getEffectiveBalances(final BeaconState state) {
                 state.getValidators().stream()
                     .map(
                         validator ->
-                            predicates.isActiveValidator(validator, epoch)
+                            predicates.isActiveValidator(validator, epoch) && !validator.isSlashed()
                                 ? validator.getEffectiveBalance()
                                 : UInt64.ZERO)
                     .collect(toUnmodifiableList()));

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/ForkChoiceUtil.java

@@ -222,7 +222,7 @@ private void processOnTick(final MutableStore store, final UInt64 timeMillis) {
     // Update store time
     store.setTimeMillis(timeMillis);
 
-    UInt64 currentSlot = getCurrentSlot(store);
+    final UInt64 currentSlot = getCurrentSlot(store);
 
     if (currentSlot.isGreaterThan(previousSlot)) {
       store.removeProposerBoostRoot();
@@ -235,10 +235,11 @@ && computeSlotsSinceEpochStart(currentSlot).isZero())) {
     }
 
     // Update store.justified_checkpoint if a better checkpoint is known
-    if (store
-        .getBestJustifiedCheckpoint()
-        .getEpoch()
-        .isGreaterThan(store.getJustifiedCheckpoint().getEpoch())) {
+    if (!specConfig.getProgressiveBalancesMode().isFull()
+        && store
+            .getBestJustifiedCheckpoint()
+            .getEpoch()
+            .isGreaterThan(store.getJustifiedCheckpoint().getEpoch())) {
       store.setJustifiedCheckpoint(store.getBestJustifiedCheckpoint());
     }
 
@@ -260,21 +261,30 @@ private void updateCheckpoints(
       final Checkpoint justifiedCheckpoint,
       final Checkpoint finalizedCheckpoint,
       final boolean isBlockOptimistic) {
-    if (justifiedCheckpoint.getEpoch().isGreaterThan(store.getJustifiedCheckpoint().getEpoch())) {
-      if (justifiedCheckpoint
-          .getEpoch()
-          .isGreaterThan(store.getBestJustifiedCheckpoint().getEpoch())) {
-        store.setBestJustifiedCheckpoint(justifiedCheckpoint);
-      }
-      if (shouldUpdateJustifiedCheckpoint(
-          store, justifiedCheckpoint, store.getForkChoiceStrategy())) {
+    if (specConfig.getProgressiveBalancesMode().isFull()) {
+      if (justifiedCheckpoint.getEpoch().isGreaterThan(store.getJustifiedCheckpoint().getEpoch())) {
         store.setJustifiedCheckpoint(justifiedCheckpoint);
       }
-    }
+      if (finalizedCheckpoint.getEpoch().isGreaterThan(store.getFinalizedCheckpoint().getEpoch())) {
+        store.setFinalizedCheckpoint(finalizedCheckpoint, isBlockOptimistic);
+      }
+    } else {
+      if (justifiedCheckpoint.getEpoch().isGreaterThan(store.getJustifiedCheckpoint().getEpoch())) {
+        if (justifiedCheckpoint
+            .getEpoch()
+            .isGreaterThan(store.getBestJustifiedCheckpoint().getEpoch())) {
+          store.setBestJustifiedCheckpoint(justifiedCheckpoint);
+        }
+        if (shouldUpdateJustifiedCheckpoint(
+            store, justifiedCheckpoint, store.getForkChoiceStrategy())) {
+          store.setJustifiedCheckpoint(justifiedCheckpoint);
+        }
+      }
 
-    if (finalizedCheckpoint.getEpoch().isGreaterThan(store.getFinalizedCheckpoint().getEpoch())) {
-      store.setFinalizedCheckpoint(finalizedCheckpoint, isBlockOptimistic);
-      store.setJustifiedCheckpoint(justifiedCheckpoint);
+      if (finalizedCheckpoint.getEpoch().isGreaterThan(store.getFinalizedCheckpoint().getEpoch())) {
+        store.setFinalizedCheckpoint(finalizedCheckpoint, isBlockOptimistic);
+        store.setJustifiedCheckpoint(justifiedCheckpoint);
+      }
     }
   }
 
@@ -398,37 +408,16 @@ public void applyBlockToStore(
       blockCheckpoints = blockCheckpoints.realizeNextEpoch();
     }
 
-    final UInt64 previousJustifiedEpoch = store.getJustifiedCheckpoint().getEpoch();
-
     updateCheckpoints(
         store,
         blockCheckpoints.getJustifiedCheckpoint(),
         blockCheckpoints.getFinalizedCheckpoint(),
         isBlockOptimistic);
 
-    if (specConfig.getProgressiveBalancesMode().isFull()) {
-      // If previous epoch is justified, pull up all current tips to previous epoch
-      if (isPreviousEpochJustified(store)) {
-        blockCheckpoints = blockCheckpoints.realizeNextEpoch();
-        // Only need to pull up all existing blocks if this block updated justification
-        if (!previousJustifiedEpoch.equals(store.getJustifiedCheckpoint().getEpoch())) {
-          for (ProtoNodeData nodeData : store.getForkChoiceStrategy().getChainHeads(true)) {
-            store.pullUpBlockCheckpoints(nodeData.getRoot());
-          }
-        }
-      }
-    }
-
     // Add new block to store
     store.putBlockAndState(signedBlock, postState, blockCheckpoints);
   }
 
-  private boolean isPreviousEpochJustified(final ReadOnlyStore store) {
-    final UInt64 currentSlot = getCurrentSlot(store);
-    final UInt64 currentEpoch = miscHelpers.computeEpochAtSlot(currentSlot);
-    return store.getJustifiedCheckpoint().getEpoch().plus(1).isGreaterThanOrEqualTo(currentEpoch);
-  }
-
   private UInt64 getFinalizedCheckpointStartSlot(final ReadOnlyStore store) {
     final UInt64 finalizedEpoch = store.getFinalizedCheckpoint().getEpoch();
     return miscHelpers.computeStartSlotAtEpoch(finalizedEpoch);

ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/EpochCachePrimer.java

@@ -111,7 +111,7 @@ private void primeJustifiedState(final Checkpoint justifiedCheckpoint) {
               }
               final BeaconState justifiedState = maybeJustifiedState.get();
               spec.getBeaconStateUtil(justifiedState.getSlot())
-                  .getEffectiveBalances(justifiedState);
+                  .getEffectiveActiveUnslashedBalances(justifiedState);
             })
         .finish(error -> LOG.warn("Failed to prime justified state caches"));
   }

ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoice.java

@@ -212,7 +212,7 @@ private SafeFuture<Boolean> processHead(
                       final BeaconState justifiedState = justifiedCheckpointState.orElseThrow();
                       final List<UInt64> justifiedEffectiveBalances =
                           spec.getBeaconStateUtil(justifiedState.getSlot())
-                              .getEffectiveBalances(justifiedState);
+                              .getEffectiveActiveUnslashedBalances(justifiedState);
 
                       Bytes32 headBlockRoot =
                           transaction.applyForkChoiceScoreChanges(

ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/EpochCachePrimerTest.java

@@ -172,7 +172,7 @@ void shouldPrimeJustifiedCheckpoint() {
     final BeaconState justifiedState =
         safeJoin(recentChainData.retrieveCheckpointState(state.getCurrentJustifiedCheckpoint()))
             .orElseThrow();
-    verify(beaconStateUtil).getEffectiveBalances(justifiedState);
+    verify(beaconStateUtil).getEffectiveActiveUnslashedBalances(justifiedState);
   }
 
   private void forEachSlotInEpoch(final UInt64 epoch, final Consumer<UInt64> action) {

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

@@ -36,13 +36,15 @@
 import tech.pegasys.teku.infrastructure.exceptions.FatalServiceFailureException;
 import tech.pegasys.teku.infrastructure.logging.StatusLogger;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
+import tech.pegasys.teku.spec.Spec;
 import tech.pegasys.teku.spec.config.ProgressiveBalancesMode;
 import tech.pegasys.teku.spec.datastructures.blocks.BlockCheckpoints;
 import tech.pegasys.teku.spec.datastructures.state.Checkpoint;
 
 public class ProtoArray {
   private static final Logger LOG = LogManager.getLogger();
 
+  private final Spec spec;
   private int pruneThreshold;
 
   private UInt64 currentEpoch;
@@ -73,13 +75,15 @@ public class ProtoArray {
   private final ProtoArrayIndices indices = new ProtoArrayIndices();
 
   ProtoArray(
+      final Spec spec,
       final int pruneThreshold,
       final UInt64 currentEpoch,
       final Checkpoint justifiedCheckpoint,
       final Checkpoint finalizedCheckpoint,
       final UInt64 initialEpoch,
       final ProgressiveBalancesMode progressiveBalancesMode,
       final StatusLogger statusLog) {
+    this.spec = spec;
     this.pruneThreshold = pruneThreshold;
     this.currentEpoch = currentEpoch;
     this.justifiedCheckpoint = justifiedCheckpoint;
@@ -620,28 +624,50 @@ public boolean nodeIsViableForHead(ProtoNode node) {
     }
 
     if (progressiveBalancesMode.isFull()) {
-      final boolean correctJustified =
-          node.getJustifiedCheckpoint()
-              .getEpoch()
-              .isGreaterThanOrEqualTo(justifiedCheckpoint.getEpoch());
-      final boolean correctFinalized;
-      if (isPreviousEpochJustified()) {
-        correctFinalized =
-            node.getFinalizedCheckpoint()
-                .getEpoch()
-                .isGreaterThanOrEqualTo(finalizedCheckpoint.getEpoch());
-      } else {
-        correctFinalized = doesCheckpointMatch(node.getFinalizedCheckpoint(), finalizedCheckpoint);
+      // The voting source should be at the same height as the store's justified checkpoint
+      boolean correctJustified =
+          doesCheckpointEpochMatch(node.getJustifiedCheckpoint(), justifiedCheckpoint);
+
+      // If this is a pulled-up block from the current epoch, also check that the unrealized
+      // justification is higher than the store's justified checkpoint, and the voting source is not
+      // more than two epochs ago.
+      if (!correctJustified
+          && !node.getUnrealizedJustifiedCheckpoint().equals(node.getJustifiedCheckpoint())) {
+        correctJustified =
+            node.getUnrealizedJustifiedCheckpoint()
+                    .getEpoch()
+                    .isGreaterThanOrEqualTo(justifiedCheckpoint.getEpoch())
+                && node.getJustifiedCheckpoint()
+                    .getEpoch()
+                    .plus(2)
+                    .isGreaterThanOrEqualTo(currentEpoch);
       }
+
+      final UInt64 finalizedSlot = spec.computeStartSlotAtEpoch(finalizedCheckpoint.getEpoch());
+      final boolean correctFinalized =
+          node.getFinalizedCheckpoint().getEpoch().equals(initialEpoch)
+              || hasAncestorAtSlot(node, finalizedSlot, finalizedCheckpoint.getRoot());
       return correctJustified && correctFinalized;
     } else {
       return doesCheckpointMatch(node.getJustifiedCheckpoint(), justifiedCheckpoint)
           && doesCheckpointMatch(node.getFinalizedCheckpoint(), finalizedCheckpoint);
     }
   }
 
-  private boolean isPreviousEpochJustified() {
-    return justifiedCheckpoint.getEpoch().plus(1).isGreaterThanOrEqualTo(currentEpoch);
+  /**
+   * This is similar to the <a
+   * href="https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#get_ancestor">get_ancestor</a>
+   * function in the eth2 spec.
+   *
+   * <p>The difference is that this is checking if the ancestor at slot is the required one.
+   */
+  private boolean hasAncestorAtSlot(
+      final ProtoNode start, final UInt64 finalizedSlot, final Bytes32 requiredRoot) {
+    ProtoNode node = start;
+    while (node != null && node.getBlockSlot().isGreaterThan(finalizedSlot)) {
+      node = node.getParentIndex().map(this::getNodeByIndex).orElse(null);
+    }
+    return node != null && requiredRoot.equals(node.getBlockRoot());
   }
 
   private boolean doesCheckpointMatch(final Checkpoint actual, final Checkpoint required) {
@@ -650,6 +676,11 @@ private boolean doesCheckpointMatch(final Checkpoint actual, final Checkpoint re
             && (actual.getRoot().isZero() || actual.getRoot().equals(required.getRoot())));
   }
 
+  private boolean doesCheckpointEpochMatch(final Checkpoint actual, final Checkpoint required) {
+    return required.getEpoch().equals(initialEpoch)
+        || actual.getEpoch().equals(required.getEpoch());
+  }
+
   public Checkpoint getJustifiedCheckpoint() {
     return justifiedCheckpoint;
   }

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

@@ -18,6 +18,7 @@
 import java.util.Optional;
 import tech.pegasys.teku.infrastructure.logging.StatusLogger;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
+import tech.pegasys.teku.spec.Spec;
 import tech.pegasys.teku.spec.config.Constants;
 import tech.pegasys.teku.spec.config.ProgressiveBalancesMode;
 import tech.pegasys.teku.spec.config.SpecConfig;
@@ -26,6 +27,7 @@
 public class ProtoArrayBuilder {
 
   private StatusLogger statusLog = StatusLogger.STATUS_LOG;
+  private Spec spec;
   private int pruneThreshold = Constants.PROTOARRAY_FORKCHOICE_PRUNE_THRESHOLD;
   private UInt64 currentEpoch;
   private Checkpoint justifiedCheckpoint;
@@ -34,11 +36,13 @@ public class ProtoArrayBuilder {
   private ProgressiveBalancesMode progressiveBalancesMode;
 
   public ProtoArray build() {
+    checkNotNull(spec, "Spec must be supplied");
     checkNotNull(progressiveBalancesMode, "Progressive balances mode must be supplied");
     checkNotNull(currentEpoch, "Current epoch must be supplied");
     checkNotNull(justifiedCheckpoint, "Justified checkpoint must be supplied");
     checkNotNull(finalizedCheckpoint, "finalized checkpoint must be supplied");
     return new ProtoArray(
+        spec,
         pruneThreshold,
         currentEpoch,
         justifiedCheckpoint,
@@ -53,6 +57,11 @@ public ProtoArrayBuilder statusLog(final StatusLogger statusLog) {
     return this;
   }
 
+  public ProtoArrayBuilder spec(final Spec spec) {
+    this.spec = spec;
+    return this;
+  }
+
   public ProtoArrayBuilder pruneThreshold(final int pruneThreshold) {
     this.pruneThreshold = pruneThreshold;
     return this;

storage/src/main/java/tech/pegasys/teku/storage/store/Store.java

@@ -247,6 +247,7 @@ private static ProtoArray buildProtoArray(
     blocks.sort(Comparator.comparing(StoredBlockMetadata::getBlockSlot));
     final ProtoArray protoArray =
         ProtoArray.builder()
+            .spec(spec)
             .currentEpoch(currentEpoch)
             .initialCheckpoint(initialCheckpoint)
             .justifiedCheckpoint(justifiedCheckpoint)

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

@@ -69,6 +69,7 @@ protected BlockMetadataStore createBlockMetadataStore(
     final BeaconState latestState = chainBuilder.getLatestBlockAndState().getState();
     final ProtoArray protoArray =
         ProtoArray.builder()
+            .spec(spec)
             .currentEpoch(ZERO)
             .finalizedCheckpoint(latestState.getFinalizedCheckpoint())
             .justifiedCheckpoint(latestState.getCurrentJustifiedCheckpoint())
@@ -135,6 +136,7 @@ public void findHead_worksForChainInitializedFromNonGenesisAnchor() {
     AnchorPoint anchor = dataStructureUtil.createAnchorFromState(anchorState);
     final ProtoArray protoArray =
         ProtoArray.builder()
+            .spec(spec)
             .initialCheckpoint(Optional.of(anchor.getCheckpoint()))
             .currentEpoch(anchor.getEpoch())
             .justifiedCheckpoint(anchorState.getCurrentJustifiedCheckpoint())
@@ -161,7 +163,7 @@ public void findHead_worksForChainInitializedFromNonGenesisAnchor() {
         dataStructureUtil
             .getSpec()
             .getBeaconStateUtil(anchor.getState().getSlot())
-            .getEffectiveBalances(anchor.getState());
+            .getEffectiveActiveUnslashedBalances(anchor.getState());
     final Bytes32 head =
         forkChoiceStrategy.applyPendingVotes(
             store,
@@ -375,7 +377,7 @@ void applyScoreChanges_shouldWorkAfterRemovingNodes() {
         dataStructureUtil
             .getSpec()
             .getBeaconStateUtil(block3State.getSlot())
-            .getEffectiveBalances(block3State);
+            .getEffectiveActiveUnslashedBalances(block3State);
     final Bytes32 bestHead =
         strategy.applyPendingVotes(
             transaction,
@@ -449,7 +451,7 @@ void applyPendingVotes_shouldMarkEquivocation() {
         dataStructureUtil
             .getSpec()
             .getBeaconStateUtil(block2State.getSlot())
-            .getEffectiveBalances(block2State);
+            .getEffectiveActiveUnslashedBalances(block2State);
     final Bytes32 bestHead =
         strategy.applyPendingVotes(
             transaction3,

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

@@ -56,6 +56,7 @@ class ProtoArrayTest {
 
   private ProtoArray protoArray =
       new ProtoArrayBuilder()
+          .spec(dataStructureUtil.getSpec())
           .statusLog(statusLog)
           .currentEpoch(ZERO)
           .justifiedCheckpoint(GENESIS_CHECKPOINT)
@@ -77,6 +78,7 @@ void findHead_shouldAlwaysConsiderJustifiedNodeAsViableHead() {
     final Checkpoint finalizedCheckpoint = new Checkpoint(UInt64.ONE, justifiedRoot);
     protoArray =
         new ProtoArrayBuilder()
+            .spec(dataStructureUtil.getSpec())
             .currentEpoch(ZERO)
             .justifiedCheckpoint(justifiedCheckpoint)
             .finalizedCheckpoint(finalizedCheckpoint)