added command line flag to enable fork-choice late block reorgs

Currently not enabled, just the flag, and defaulting to false, and a test that shows it can be toggled on. partially addresses Consensys#6595 Signed-off-by: Paul Harris <paul.harris@consensys.net>
rolfyone · Dec 5, 2023 · 937c87f · 937c87f
1 parent c7b8734
commit 937c87f
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 2 deletions.
diff --git a/ethereum/networks/src/main/java/tech/pegasys/teku/networks/Eth2NetworkConfiguration.java b/ethereum/networks/src/main/java/tech/pegasys/teku/networks/Eth2NetworkConfiguration.java
@@ -50,6 +50,8 @@ public class Eth2NetworkConfiguration {
   private static final int DEFAULT_STARTUP_TIMEOUT_SECONDS = 30;
 
   public static final boolean DEFAULT_FORK_CHOICE_UPDATE_HEAD_ON_BLOCK_IMPORT_ENABLED = false;
+
+  public static final boolean DEFAULT_FORK_CHOICE_LATE_BLOCK_REORG_ENABLED = false;
   public static final boolean DEFAULT_FORK_CHOICE_PROPOSER_BOOST_UNIQUENESS_ENABLED = true;
 
   public static final int DEFAULT_ASYNC_P2P_MAX_THREADS = 10;
@@ -97,6 +99,7 @@ public class Eth2NetworkConfiguration {
   private final int asyncBeaconChainMaxThreads;
   private final int asyncBeaconChainMaxQueue;
   private final int asyncP2pMaxQueue;
+  private final boolean forkChoiceLateBlockReorgEnabled;
 
   private Eth2NetworkConfiguration(
       final Spec spec,
@@ -124,7 +127,8 @@ private Eth2NetworkConfiguration(
       final int asyncP2pMaxThreads,
       final int asyncP2pMaxQueue,
       final int asyncBeaconChainMaxThreads,
-      final int asyncBeaconChainMaxQueue) {
+      final int asyncBeaconChainMaxQueue,
+      final boolean forkChoiceLateBlockReorgEnabled) {
     this.spec = spec;
     this.constants = constants;
     this.initialState = initialState;
@@ -154,6 +158,7 @@ private Eth2NetworkConfiguration(
     this.asyncP2pMaxQueue = asyncP2pMaxQueue;
     this.asyncBeaconChainMaxThreads = asyncBeaconChainMaxThreads;
     this.asyncBeaconChainMaxQueue = asyncBeaconChainMaxQueue;
+    this.forkChoiceLateBlockReorgEnabled = forkChoiceLateBlockReorgEnabled;
   }
 
   public static Eth2NetworkConfiguration.Builder builder(final String network) {
@@ -271,6 +276,10 @@ public int getAsyncBeaconChainMaxQueue() {
     return asyncBeaconChainMaxQueue;
   }
 
+  public boolean isForkChoiceLateBlockReorgEnabled() {
+    return forkChoiceLateBlockReorgEnabled;
+  }
+
   @Override
   public String toString() {
     return constants;
@@ -306,6 +315,7 @@ public static class Builder {
         DEFAULT_FORK_CHOICE_UPDATE_HEAD_ON_BLOCK_IMPORT_ENABLED;
     private boolean forkChoiceProposerBoostUniquenessEnabled =
         DEFAULT_FORK_CHOICE_PROPOSER_BOOST_UNIQUENESS_ENABLED;
+    private boolean forkChoiceLateBlockReorgEnabled = DEFAULT_FORK_CHOICE_LATE_BLOCK_REORG_ENABLED;
 
     public void spec(Spec spec) {
       this.spec = spec;
@@ -380,7 +390,8 @@ public Eth2NetworkConfiguration build() {
           asyncP2pMaxThreads,
           asyncP2pMaxQueue,
           asyncBeaconChainMaxThreads,
-          asyncBeaconChainMaxQueue);
+          asyncBeaconChainMaxQueue,
+          forkChoiceLateBlockReorgEnabled);
     }
 
     private void validateCommandLineParameters() {
@@ -831,5 +842,10 @@ private Optional<Integer> validateAndParseEpochsStoreBlobs(final String epochsSt
           epochsStoreBlobsInt > 0, "Number of the epochs to store blobs for should be > 0");
       return Optional.of(epochsStoreBlobsInt);
     }
+
+    public Builder forkChoiceLateBlockReorgEnabled(boolean forkChoiceLateBlockReorgEnabled) {
+      this.forkChoiceLateBlockReorgEnabled = forkChoiceLateBlockReorgEnabled;
+      return this;
+    }
   }
 }
diff --git a/...tatetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoice.java b/...tatetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoice.java
@@ -100,6 +100,7 @@ public class ForkChoice implements ForkChoiceUpdatedResultSubscriber {
   private final Subscribers<OptimisticHeadSubscriber> optimisticSyncSubscribers =
       Subscribers.create(true);
   private final TickProcessor tickProcessor;
+  private final boolean forkChoiceLateBlockReorgEnabled;
   private Optional<Boolean> optimisticSyncing = Optional.empty();
 
   public ForkChoice(
@@ -113,6 +114,7 @@ public ForkChoice(
       final MergeTransitionBlockValidator transitionBlockValidator,
       final boolean forkChoiceUpdateHeadOnBlockImportEnabled,
       final boolean forkChoiceProposerBoostUniquenessEnabled,
+      final boolean forkChoiceLateBlockReorgEnabled,
       final MetricsSystem metricsSystem) {
     this.spec = spec;
     this.forkChoiceExecutor = forkChoiceExecutor;
@@ -126,6 +128,9 @@ public ForkChoice(
     this.tickProcessor = tickProcessor;
     this.forkChoiceUpdateHeadOnBlockImportEnabled = forkChoiceUpdateHeadOnBlockImportEnabled;
     this.forkChoiceProposerBoostUniquenessEnabled = forkChoiceProposerBoostUniquenessEnabled;
+    this.forkChoiceLateBlockReorgEnabled = forkChoiceLateBlockReorgEnabled;
+    LOG.debug("forkChoiceLateBlockReorgEnabled is set to {}", forkChoiceLateBlockReorgEnabled);
+
     recentChainData.subscribeStoreInitialized(this::initializeProtoArrayForkChoice);
     forkChoiceNotifier.subscribeToForkChoiceUpdatedResult(this);
   }
@@ -154,6 +159,7 @@ public ForkChoice(
         transitionBlockValidator,
         true,
         true,
+        false,
         metricsSystem);
   }
 
@@ -196,6 +202,10 @@ public SafeFuture<Boolean> processHead() {
     return processHead(Optional.empty(), false);
   }
 
+  public boolean isForkChoiceLateBlockReorgEnabled() {
+    return forkChoiceLateBlockReorgEnabled;
+  }
+
   /** Import a block to the store. */
   public SafeFuture<BlockImportResult> onBlock(
       final SignedBeaconBlock block,

diff --git a/...transition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceTest.java b/...transition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceTest.java
@@ -32,6 +32,7 @@
 import static tech.pegasys.teku.infrastructure.async.SafeFutureAssert.safeJoin;
 import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ONE;
 import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ZERO;
+import static tech.pegasys.teku.networks.Eth2NetworkConfiguration.DEFAULT_FORK_CHOICE_LATE_BLOCK_REORG_ENABLED;
 import static tech.pegasys.teku.networks.Eth2NetworkConfiguration.DEFAULT_FORK_CHOICE_PROPOSER_BOOST_UNIQUENESS_ENABLED;
 import static tech.pegasys.teku.networks.Eth2NetworkConfiguration.DEFAULT_FORK_CHOICE_UPDATE_HEAD_ON_BLOCK_IMPORT_ENABLED;
 import static tech.pegasys.teku.statetransition.forkchoice.ForkChoice.BLOCK_CREATION_TOLERANCE_MS;
@@ -165,6 +166,7 @@ private void setupWithSpec(final Spec unmockedSpec) {
             transitionBlockValidator,
             DEFAULT_FORK_CHOICE_UPDATE_HEAD_ON_BLOCK_IMPORT_ENABLED,
             DEFAULT_FORK_CHOICE_PROPOSER_BOOST_UNIQUENESS_ENABLED,
+            DEFAULT_FORK_CHOICE_LATE_BLOCK_REORG_ENABLED,
             metricsSystem);
 
     // Starting and mocks
@@ -421,6 +423,7 @@ void onBlock_shouldReorgWhenProposerWeightingMakesForkBestChain(
             transitionBlockValidator,
             DEFAULT_FORK_CHOICE_UPDATE_HEAD_ON_BLOCK_IMPORT_ENABLED,
             DEFAULT_FORK_CHOICE_PROPOSER_BOOST_UNIQUENESS_ENABLED,
+            DEFAULT_FORK_CHOICE_LATE_BLOCK_REORG_ENABLED,
             metricsSystem);
 
     final UInt64 currentSlot = recentChainData.getCurrentSlot().orElseThrow();

diff --git a/...aconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java b/...aconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java
@@ -758,6 +758,7 @@ protected void initForkChoice() {
             new MergeTransitionBlockValidator(spec, recentChainData, executionLayer),
             beaconConfig.eth2NetworkConfig().isForkChoiceUpdateHeadOnBlockImportEnabled(),
             beaconConfig.eth2NetworkConfig().isForkChoiceProposerBoostUniquenessEnabled(),
+            beaconConfig.eth2NetworkConfig().isForkChoiceLateBlockReorgEnabled(),
             metricsSystem);
     forkChoiceTrigger = new ForkChoiceTrigger(forkChoice);
   }

diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/Eth2NetworkOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/Eth2NetworkOptions.java
@@ -92,6 +92,17 @@ public class Eth2NetworkOptions {
       arity = "1")
   private String trustedSetup = null; // Depends on network configuration
 
+  @Option(
+      names = {"--Xfork-choice-late-block-reorg-enabled"},
+      paramLabel = "<BOOLEAN>",
+      description = "Allow late blocks to be reorged out if they meet the requirements.",
+      arity = "0..1",
+      fallbackValue = "true",
+      showDefaultValue = Visibility.ALWAYS,
+      hidden = true)
+  private boolean forkChoiceLateBlockReorgEnabled =
+      Eth2NetworkConfiguration.DEFAULT_FORK_CHOICE_LATE_BLOCK_REORG_ENABLED;
+
   @Option(
       names = {"--Xnetwork-altair-fork-epoch"},
       hidden = true,
@@ -328,6 +339,7 @@ private void configureEth2Network(Eth2NetworkConfiguration.Builder builder) {
         .asyncBeaconChainMaxQueue(asyncBeaconChainMaxQueue)
         .forkChoiceUpdateHeadOnBlockImportEnabled(forkChoiceUpdateHeadOnBlockImportEnabled)
         .forkChoiceProposerBoostUniquenessEnabled(forkChoiceProposerBoostUniquenessEnabled)
+        .forkChoiceLateBlockReorgEnabled(forkChoiceLateBlockReorgEnabled)
         .epochsStoreBlobs(epochsStoreBlobs);
   }
 

diff --git a/teku/src/test/java/tech/pegasys/teku/cli/options/Eth2NetworkOptionsTest.java b/teku/src/test/java/tech/pegasys/teku/cli/options/Eth2NetworkOptionsTest.java
@@ -126,6 +126,15 @@ void shouldSetFirstDescendentAsHead(final String value) {
         .isEqualTo(Boolean.valueOf(value));
   }
 
+  @ParameterizedTest
+  @ValueSource(strings = {"true", "false"})
+  void shouldSetLateBlockImportEnabled(final String value) {
+    final TekuConfiguration config =
+        getTekuConfigurationFromArguments("--Xfork-choice-late-block-reorg-enabled", value);
+    assertThat(config.eth2NetworkConfiguration().isForkChoiceLateBlockReorgEnabled())
+        .isEqualTo(Boolean.valueOf(value));
+  }
+
   @Test
   void shouldMergeTransitionsOverrideBeEmptyByDefault() {
     final TekuConfiguration config = getTekuConfigurationFromArguments();