Reimplement proposer boost changes (#10368)

Author: tbenr

eth-tests/src/main/java/tech/pegasys/teku/ethtests/finder/ReferenceTestFinder.java

@@ -71,18 +71,11 @@ private static Stream<TestDefinition> findTestTypes(final Path specDirectory) th
                       new PyspecTestFinder(
                           List.of(),
                           List.of(
-                              // TODO: https://github.com/Consensys/teku/issues/10320 ignoring tests
-                              // which fail because of the proposer boost changes added in
-                              // https://github.com/ethereum/consensus-specs/pull/4807
-                              "minimal - fork_choice/get_head - voting_source_beyond_two_epoch",
-                              "minimal - fork_choice/on_block - justified_update_always_if_better",
-                              "minimal - fork_choice/on_block - justified_update_not_realized_finality",
                               // TODO-GLOAS: Limit what tests we run for Gloas while it is
                               // under development. This is temporary and should be removed once we
                               // are up-to-date with Gloas specs (see
                               // https://github.com/Consensys/teku-internal/issues/221)
-                              "gloas - minimal - fork_choice/",
-                              "gloas - mainnet - fork_choice/")),
+                              "gloas - minimal - fork_choice/", "gloas - mainnet - fork_choice/")),
                       new MerkleProofTestFinder())
                   .flatMap(unchecked(finder -> finder.findTests(fork, spec, testsPath)));
             });

ethereum/spec/src/main/java/tech/pegasys/teku/spec/Spec.java

@@ -13,6 +13,7 @@
 
 package tech.pegasys.teku.spec;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static tech.pegasys.teku.infrastructure.time.TimeUtilities.millisToSeconds;
 import static tech.pegasys.teku.spec.SpecMilestone.DENEB;
@@ -632,6 +633,38 @@ public int getBeaconProposerIndex(final BeaconState state, final UInt64 slot) {
     return atState(state).beaconStateAccessors().getBeaconProposerIndex(state, slot);
   }
 
+  /**
+   * Returns the proposer index for the given slot, advancing the state via slot processing if
+   * necessary. On FULU+, the ProposerLookahead allows resolving the proposer without slot
+   * processing for slots in the current or next epoch. On earlier forks, slot processing is avoided
+   * only when the slot is in the same epoch as the state.
+   *
+   * @param state a beacon state at or before the requested slot
+   * @param slot the slot to get the proposer index for (must be >= state.slot)
+   */
+  public int getProposerIndexAtSlot(final BeaconState state, final UInt64 slot)
+      throws SlotProcessingException, EpochProcessingException {
+    checkArgument(
+        state.getSlot().isLessThanOrEqualTo(slot),
+        "State slot %s must not be ahead of requested slot %s",
+        state.getSlot(),
+        slot);
+
+    final BeaconState actualState;
+
+    if (canCalculateProposerIndexAtSlot(state, slot)) {
+      actualState = state;
+    } else {
+      actualState = processSlots(state, slot);
+    }
+
+    return getBeaconProposerIndex(actualState, slot);
+  }
+
+  public boolean canCalculateProposerIndexAtSlot(final BeaconState state, final UInt64 slot) {
+    return atState(state).beaconStateAccessors().canCalculateProposerIndexAtSlot(state, slot);
+  }
+
   public UInt64 getCommitteeCountPerSlot(final BeaconState state, final UInt64 epoch) {
     return atState(state).beaconStateAccessors().getCommitteeCountPerSlot(state, epoch);
   }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/helpers/BeaconStateAccessors.java

@@ -229,15 +229,18 @@ public int getBeaconProposerIndex(final BeaconState state, final UInt64 requeste
 
   protected void validateStateCanCalculateProposerIndexAtSlot(
       final BeaconState state, final UInt64 requestedSlot) {
-    final UInt64 epoch = miscHelpers.computeEpochAtSlot(requestedSlot);
-    final UInt64 stateEpoch = getCurrentEpoch(state);
     checkArgument(
-        epoch.equals(stateEpoch),
-        "get_beacon_proposer_index is only used for requesting a slot in the current epoch. Requested slot %s (in epoch %s), state slot %s (in epoch %s)",
+        canCalculateProposerIndexAtSlot(state, requestedSlot),
+        "get_beacon_proposer_index is only used for requesting a slot in the current epoch (or next epoch in FULU). Requested slot %s, state slot %s",
         requestedSlot,
-        epoch,
-        state.getSlot(),
-        stateEpoch);
+        state.getSlot());
+  }
+
+  public boolean canCalculateProposerIndexAtSlot(
+      final BeaconState state, final UInt64 requestedSlot) {
+    final UInt64 epoch = miscHelpers.computeEpochAtSlot(requestedSlot);
+    final UInt64 stateEpoch = getCurrentEpoch(state);
+    return epoch.equals(stateEpoch);
   }
 
   public UInt64 getFinalityDelay(final BeaconState state) {

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/fulu/helpers/BeaconStateAccessorsFulu.java

@@ -70,17 +70,11 @@ public int getBeaconProposerIndex(final BeaconState state, final UInt64 requeste
   }
 
   @Override
-  protected void validateStateCanCalculateProposerIndexAtSlot(
+  public boolean canCalculateProposerIndexAtSlot(
       final BeaconState state, final UInt64 requestedSlot) {
     final UInt64 epoch = miscHelpers.computeEpochAtSlot(requestedSlot);
     final UInt64 stateEpoch = getCurrentEpoch(state);
-    checkArgument(
-        stateEpoch.equals(epoch) || stateEpoch.increment().equals(epoch),
-        "get_beacon_proposer_index is only used for requesting a slot in the current or next epoch. Requested slot %s (in epoch %s), state slot %s (in epoch %s)",
-        requestedSlot,
-        epoch,
-        state.getSlot(),
-        stateEpoch);
+    return epoch.equals(stateEpoch) || stateEpoch.increment().equals(epoch);
   }
 
   public List<Integer> getBeaconProposerIndices(final BeaconState state, final UInt64 epoch) {

ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/versions/fulu/helpers/BeaconStateAccessorsFuluTest.java

@@ -136,7 +136,7 @@ void getBeaconProposerIndex_shouldThrowIfNotCurrentOrNextEpoch(final int slot) {
                 stateAccessorsFulu.getBeaconProposerIndex(
                     blockAndState.getState(), UInt64.valueOf(slot)))
         .hasMessageContaining(
-            "get_beacon_proposer_index is only used for requesting a slot in the current or next epoch");
+            "get_beacon_proposer_index is only used for requesting a slot in the current epoch (or next epoch in FULU)");
   }
 
   private List<Integer> getProposerLookaheadFromState(final BeaconStateFulu beaconStateFulu) {

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

@@ -15,7 +15,6 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static tech.pegasys.teku.infrastructure.logging.P2PLogger.P2P_LOG;
-import static tech.pegasys.teku.infrastructure.time.TimeUtilities.secondsToMillis;
 import static tech.pegasys.teku.statetransition.forkchoice.StateRootCollector.addParentStateRoots;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -66,6 +65,8 @@
 import tech.pegasys.teku.spec.executionlayer.PayloadStatus;
 import tech.pegasys.teku.spec.logic.common.statetransition.availability.AvailabilityChecker;
 import tech.pegasys.teku.spec.logic.common.statetransition.availability.DataAndValidationResult;
+import tech.pegasys.teku.spec.logic.common.statetransition.exceptions.EpochProcessingException;
+import tech.pegasys.teku.spec.logic.common.statetransition.exceptions.SlotProcessingException;
 import tech.pegasys.teku.spec.logic.common.statetransition.exceptions.StateTransitionException;
 import tech.pegasys.teku.spec.logic.common.statetransition.results.BlockImportResult;
 import tech.pegasys.teku.spec.logic.common.statetransition.results.BlockImportResult.FailureReason;
@@ -662,8 +663,8 @@ private BlockImportResult importBlockAndState(
         blobSidecars,
         earliestBlobSidecarsSlot);
 
-    final boolean shouldApplyProposerBoost = shouldApplyProposerBoost(block, transaction);
-    if (shouldApplyProposerBoost) {
+    final boolean shouldUpdateProposerBoostRoot = shouldUpdateProposerBoostRoot(block, transaction);
+    if (shouldUpdateProposerBoostRoot) {
       transaction.setProposerBoostRoot(block.getRoot());
     }
 
@@ -693,7 +694,8 @@ private BlockImportResult importBlockAndState(
     } else {
       result = BlockImportResult.optimisticallySuccessful(block);
     }
-    updateForkChoiceForImportedBlock(block, shouldApplyProposerBoost, result, forkChoiceStrategy);
+    updateForkChoiceForImportedBlock(
+        block, shouldUpdateProposerBoostRoot, result, forkChoiceStrategy);
     notifyForkChoiceUpdatedAndOptimisticSyncingChanged(Optional.empty());
     return result;
   }
@@ -746,20 +748,42 @@ private ExecutionPayloadImportResult importExecutionPayloadAndState(
   }
 
   // from consensus-specs/fork-choice:
-  private boolean shouldApplyProposerBoost(
+  private boolean shouldUpdateProposerBoostRoot(
       final SignedBeaconBlock block, final StoreTransaction transaction) {
-    // get_current_slot(store) == block.slot
-    if (!spec.getCurrentSlot(transaction).equals(block.getSlot())) {
+    // is_first_block
+    if (transaction.getProposerBoostRoot().isPresent()) {
       return false;
     }
-    // is_before_attesting_interval
-    final UInt64 timeIntoSlotMillis =
-        getMillisIntoSlot(transaction, spec.getSlotDurationMillis(block.getSlot()));
-    if (!timeIntoSlotMillis.isLessThan(spec.getAttestationDueMillis(block.getSlot()))) {
+    // is_timely
+    if (recentChainData.isBlockLate(block.getRoot())) {
       return false;
     }
-    // is_first_block
-    return transaction.getProposerBoostRoot().isEmpty();
+
+    // in the edge cases when the chain head is not present or the head state future is not
+    // immediately available, the proposer boost root will be set to maintain backwards
+    // compatibility (see: https://github.com/ethereum/consensus-specs/pull/4807/)
+    return recentChainData
+        .getChainHead()
+        .map(
+            chainHead -> {
+              final SafeFuture<BeaconState> headStateFuture = chainHead.getState();
+              if (!headStateFuture.isCompletedNormally()) {
+                return true;
+              }
+              final BeaconState headState = headStateFuture.join();
+              final UInt64 currentSlot = spec.getCurrentSlot(transaction);
+              try {
+                return block.getProposerIndex().intValue()
+                    == spec.getProposerIndexAtSlot(headState, currentSlot);
+              } catch (final SlotProcessingException | EpochProcessingException ex) {
+                throw new RuntimeException(
+                    String.format(
+                        "Can't progress head state from slot %s to slot %s",
+                        headState.getSlot(), currentSlot),
+                    ex);
+              }
+            })
+        .orElse(true);
   }
 
   private Optional<List<BlobSidecar>> extractBlobSidecarsFromValidationResults(
@@ -811,13 +835,6 @@ private Optional<UInt64> computeEarliestBlobSidecarsSlot(
             earliestAvailabilityWindowSlotBeforeBlock.max(earliestAffectedSlot));
   }
 
-  private UInt64 getMillisIntoSlot(final StoreTransaction transaction, final int millisPerSlot) {
-    return transaction
-        .getTimeInMillis()
-        .minus(secondsToMillis(transaction.getGenesisTime()))
-        .mod(millisPerSlot);
-  }
-
   private void onExecutionPayloadResult(
       final Bytes32 blockRoot, final PayloadStatus payloadResult) {
     final SafeFuture<PayloadValidationResult> transitionValidatedStatus;
@@ -858,7 +875,7 @@ private void onExecutionPayloadResult(
 
   private void updateForkChoiceForImportedBlock(
       final SignedBeaconBlock block,
-      final boolean shouldApplyProposerBoost,
+      final boolean shouldUpdateProposerBoostRoot,
       final BlockImportResult result,
       final ForkChoiceStrategy forkChoiceStrategy) {
 
@@ -872,7 +889,7 @@ private void updateForkChoiceForImportedBlock(
       }
     }
 
-    if (!result.isBlockOnCanonicalChain() && shouldApplyProposerBoost) {
+    if (!result.isBlockOnCanonicalChain() && shouldUpdateProposerBoostRoot) {
       // This is likely a reorging block that requires a full processHead to update the head.
       // Running processHead here will ensure:
       //

ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceTest.java

@@ -845,7 +845,7 @@ void onBlock_shouldNotifyOptimisticSyncChangeOnlyWhenImportingOnCanonicalHead()
   }
 
   @Test
-  void onBlock_shouldApplyProposerBoostToFirstBlock() {
+  void onBlock_shouldUpdateProposerBoostRootToFirstBlock() {
     final ChainBuilder forkChain = chainBuilder.fork();
 
     final SignedBlockAndState block = chainBuilder.generateNextBlock();

storage/src/main/java/tech/pegasys/teku/storage/client/LateBlockReorgLogic.java

@@ -56,6 +56,7 @@ public LateBlockReorgLogic(final Spec spec, final RecentChainData recentChainDat
     this.recentChainData = recentChainData;
   }
 
+  // record_block_timeliness
   public void setBlockTimelinessFromArrivalTime(
       final SignedBeaconBlock block, final UInt64 arrivalTimeMillis) {
     if (blockTimeliness.get(block.getRoot()) != null) {

storage/src/main/java/tech/pegasys/teku/storage/client/RecentChainData.java

@@ -778,6 +778,10 @@ public void setBlockTimelinessIfEmpty(final SignedBeaconBlock block) {
     lateBlockReorgLogic.setBlockTimelinessFromArrivalTime(block, store.getTimeInMillis());
   }
 
+  public boolean isBlockLate(final Bytes32 root) {
+    return lateBlockReorgLogic.isBlockLate(root);
+  }
+
   public Optional<UInt64> getCustodyGroupCount() {
     return store.getCustodyGroupCount();
   }