Don't start BFT mining coordinators until initial sync has completed (h…

…yperledger#5861) * Don't start BFT mining coordinators until initial sync has completed Signed-off-by: Matthew Whitehead <matthew1001@gmail.com> * Fix unit tests Signed-off-by: Matthew Whitehead <matthew1001@gmail.com> * Fix 'enable' logic Signed-off-by: Matthew Whitehead <matthew1001@gmail.com> --------- Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
garyschulte · Sep 20, 2023 · d068c31 · d068c31
1 parent 68d1562
commit d068c31
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 5 deletions.
diff --git a/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java
@@ -72,6 +72,7 @@
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.p2p.config.SubProtocolConfiguration;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
+import org.hyperledger.besu.plugin.services.BesuEvents;
 import org.hyperledger.besu.util.Subscribers;
 
 import java.util.HashMap;
@@ -231,7 +232,30 @@ protected MiningCoordinator createMiningCoordinator(
             blockCreatorFactory,
             blockchain,
             bftEventQueue);
-    ibftMiningCoordinator.enable();
+
+    if (syncState.isInitialSyncPhaseDone()) {
+      LOG.info("Starting IBFT mining coordinator");
+      ibftMiningCoordinator.enable();
+      ibftMiningCoordinator.start();
+    } else {
+      LOG.info("IBFT mining coordinator not starting while initial sync in progress");
+    }
+
+    syncState.subscribeCompletionReached(
+        new BesuEvents.InitialSyncCompletionListener() {
+          @Override
+          public void onInitialSyncCompleted() {
+            LOG.info("Starting IBFT mining coordinator following initial sync");
+            ibftMiningCoordinator.enable();
+            ibftMiningCoordinator.start();
+          }
+
+          @Override
+          public void onInitialSyncRestart() {
+            // Nothing to do. The mining coordinator won't be started until
+            // sync has completed.
+          }
+        });
 
     return ibftMiningCoordinator;
   }

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java
@@ -82,6 +82,7 @@
 import org.hyperledger.besu.ethereum.p2p.config.SubProtocolConfiguration;
 import org.hyperledger.besu.ethereum.transaction.TransactionSimulator;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
+import org.hyperledger.besu.plugin.services.BesuEvents;
 import org.hyperledger.besu.util.Subscribers;
 
 import java.util.HashMap;
@@ -271,7 +272,30 @@ protected MiningCoordinator createMiningCoordinator(
             blockCreatorFactory,
             blockchain,
             bftEventQueue);
-    miningCoordinator.enable();
+
+    if (syncState.isInitialSyncPhaseDone()) {
+      LOG.info("Starting QBFT mining coordinator");
+      miningCoordinator.enable();
+      miningCoordinator.start();
+    } else {
+      LOG.info("QBFT mining coordinator not starting while initial sync in progress");
+    }
+
+    syncState.subscribeCompletionReached(
+        new BesuEvents.InitialSyncCompletionListener() {
+          @Override
+          public void onInitialSyncCompleted() {
+            LOG.info("Starting QBFT mining coordinator following initial sync");
+            miningCoordinator.enable();
+            miningCoordinator.start();
+          }
+
+          @Override
+          public void onInitialSyncRestart() {
+            // Nothing to do. The mining coordinator won't be started until
+            // sync has completed.
+          }
+        });
 
     return miningCoordinator;
   }

diff --git a/...ommon/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java b/...ommon/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java
@@ -65,6 +65,7 @@ public void start() {
   }
 
   private void startActiveMiningCoordinator() {
+    activeMiningCoordinator.enable();
     activeMiningCoordinator.start();
     if (activeMiningCoordinator instanceof BlockAddedObserver) {
       ((BlockAddedObserver) activeMiningCoordinator).removeObserver();

diff --git a/...in/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinator.java b/...in/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinator.java
@@ -46,7 +46,9 @@ private enum State {
     /** Running state. */
     RUNNING,
     /** Stopped state. */
-    STOPPED
+    STOPPED,
+    /** Paused state. */
+    PAUSED,
   }
 
   private static final Logger LOG = LoggerFactory.getLogger(BftMiningCoordinator.class);
@@ -61,7 +63,7 @@ private enum State {
   private final BftExecutors bftExecutors;
 
   private long blockAddedObserverId;
-  private final AtomicReference<State> state = new AtomicReference<>(State.IDLE);
+  private final AtomicReference<State> state = new AtomicReference<>(State.PAUSED);
 
   /**
    * Instantiates a new Bft mining coordinator.
@@ -122,7 +124,13 @@ public void awaitStop() throws InterruptedException {
 
   @Override
   public boolean enable() {
-    return true;
+    // Return true if we're already running or idle, or successfully switch to idle
+    if (state.get() == State.RUNNING
+        || state.get() == State.IDLE
+        || state.compareAndSet(State.PAUSED, State.IDLE)) {
+      return true;
+    }
+    return false;
   }
 
   @Override

diff --git a/...ava/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinatorTest.java b/...ava/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinatorTest.java
@@ -75,6 +75,7 @@ public void stopsMining() {
     bftMiningCoordinator.stop();
     verify(bftProcessor, never()).stop();
 
+    bftMiningCoordinator.enable();
     bftMiningCoordinator.start();
     bftMiningCoordinator.stop();
     verify(bftProcessor).stop();