Don't start BFT mining coordinators until initial sync has completed (#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>

besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java

@@ -72,6 +72,7 @@ import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
 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 @@ public class IbftBesuControllerBuilder extends BftBesuControllerBuilder {
             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;
   }

besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java

@@ -82,6 +82,7 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 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 @@ public class QbftBesuControllerBuilder extends BftBesuControllerBuilder {
             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;
   }

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java

@@ -65,6 +65,7 @@ public class MigratingMiningCoordinator implements MiningCoordinator, BlockAdded
   }
 
   private void startActiveMiningCoordinator() {
+    activeMiningCoordinator.enable();
     activeMiningCoordinator.start();
     if (activeMiningCoordinator instanceof BlockAddedObserver) {
       ((BlockAddedObserver) activeMiningCoordinator).removeObserver();

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinator.java

@@ -46,7 +46,9 @@ public class BftMiningCoordinator implements MiningCoordinator, BlockAddedObserv
     /** Running state. */
     RUNNING,
     /** Stopped state. */
-    STOPPED
+    STOPPED,
+    /** Paused state. */
+    PAUSED,
   }
 
   private static final Logger LOG = LoggerFactory.getLogger(BftMiningCoordinator.class);
@@ -61,7 +63,7 @@ public class BftMiningCoordinator implements MiningCoordinator, BlockAddedObserv
   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 class BftMiningCoordinator implements MiningCoordinator, BlockAddedObserv
 
   @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

consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinatorTest.java

@@ -75,6 +75,7 @@ public class BftMiningCoordinatorTest {
     bftMiningCoordinator.stop();
     verify(bftProcessor, never()).stop();
 
+    bftMiningCoordinator.enable();
     bftMiningCoordinator.start();
     bftMiningCoordinator.stop();
     verify(bftProcessor).stop();