Ensure block height manager is restarted when BFT coordinator is

CHANGELOG.md

@@ -25,6 +25,7 @@
 ### Bug fixes
 - Upgrade Netty to version 4.1.118 to fix CVE-2025-24970 [#8275](https://github.com/hyperledger/besu/pull/8275)
 - Add missing RPC method `debug_accountRange` to `RpcMethod.java` and implemented its handler. [#8153](https://github.com/hyperledger/besu/issues/8153)
+- Fix issue with new QBFT/IBFT blocks being produced under certain circumstances. [#8308](https://github.com/hyperledger/besu/issues/8308)
 
 ## 25.2.0
 

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

@@ -115,6 +115,7 @@ public void stop() {
         LOG.debug("Interrupted while waiting for BftProcessor to stop.", e);
         Thread.currentThread().interrupt();
       }
+      eventHandler.reset();
       bftExecutors.stop();
     }
   }

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/statemachine/BaseBftController.java

@@ -77,6 +77,18 @@ protected BaseBftController(
   public void start() {
     if (started.compareAndSet(false, true)) {
       startNewHeightManager(blockchain.getChainHeadHeader());
+    } else {
+      // In normal circumstances the height manager should only be started once. If the caller
+      // has stopped the height manager (e.g. while sync completes) they must call reset() before
+      // starting the height manager again.
+      throw new IllegalStateException("Attempt to start new height manager without resetting");
+    }
+  }
+
+  @Override
+  public void reset() {
+    if (started.compareAndSet(true, false)) {
+      LOG.debug("Height manager reset");
     }
   }
 

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/statemachine/BftEventHandler.java

@@ -25,6 +25,9 @@ public interface BftEventHandler {
   /** Start. */
   void start();
 
+  /** Reset. */
+  void reset();
+
   /**
    * Handle message event.
    *

consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftControllerTest.java

@@ -14,6 +14,8 @@
  */
 package org.hyperledger.besu.consensus.ibft.statemachine;
 
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.util.Lists.newArrayList;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
@@ -478,4 +480,21 @@ private void setupRoundChange(
     when(roundChangeMessageData.decode()).thenReturn(roundChange);
     roundChangeMessage = new DefaultMessage(null, roundChangeMessageData);
   }
+
+  @Test
+  public void heightManagerCanOnlyBeStartedOnceWithoutReset() {
+    constructIbftController();
+    ibftController.start();
+    assertThatThrownBy(() -> ibftController.start())
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessage("Attempt to start new height manager without resetting");
+  }
+
+  @Test
+  public void heightManagerCanBeRestartedIfReset() {
+    constructIbftController();
+    ibftController.start();
+    ibftController.reset();
+    assertThatNoException().isThrownBy(() -> ibftController.start());
+  }
 }

consensus/qbft-core/src/main/java/org/hyperledger/besu/consensus/qbft/core/statemachine/QbftController.java

@@ -143,6 +143,18 @@ private BaseQbftBlockHeightManager getCurrentHeightManager() {
   public void start() {
     if (started.compareAndSet(false, true)) {
       startNewHeightManager(blockchain.getChainHeadHeader());
+    } else {
+      // In normal circumstances the height manager should only be started once. If the caller
+      // has stopped the height manager (e.g. while sync completes) they must call reset() before
+      // starting the height manager again.
+      throw new IllegalStateException("Attempt to start new height manager without resetting");
+    }
+  }
+
+  @Override
+  public void reset() {
+    if (started.compareAndSet(true, false)) {
+      LOG.debug("QBFT height manager reset");
     }
   }
 

consensus/qbft-core/src/main/java/org/hyperledger/besu/consensus/qbft/core/types/QbftEventHandler.java

@@ -24,6 +24,9 @@ public interface QbftEventHandler {
   /** Start. */
   void start();
 
+  /** Reset. */
+  void reset();
+
   /**
    * Handle errorMessage event.
    *

consensus/qbft-core/src/test/java/org/hyperledger/besu/consensus/qbft/core/statemachine/QbftControllerTest.java

@@ -14,6 +14,8 @@
  */
 package org.hyperledger.besu.consensus.qbft.core.statemachine;
 
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.util.Lists.newArrayList;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
@@ -484,4 +486,21 @@ private void setupRoundChange(
     when(roundChangeMessageData.decode(blockEncoder)).thenReturn(roundChange);
     roundChangeMessage = new DefaultMessage(null, roundChangeMessageData);
   }
+
+  @Test
+  public void heightManagerCanOnlyBeStartedOnceWithoutReset() {
+    constructQbftController();
+    qbftController.start();
+    assertThatThrownBy(() -> qbftController.start())
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessage("Attempt to start new height manager without resetting");
+  }
+
+  @Test
+  public void heightManagerCanBeRestartedIfReset() {
+    constructQbftController();
+    qbftController.start();
+    qbftController.reset();
+    assertThatNoException().isThrownBy(() -> qbftController.start());
+  }
 }

consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/adaptor/BftEventHandlerAdaptor.java

@@ -41,6 +41,11 @@ public void start() {
     qbftEventHandler.start();
   }
 
+  @Override
+  public void reset() {
+    qbftEventHandler.reset();
+  }
+
   @Override
   public void handleMessageEvent(final BftReceivedMessageEvent msg) {
     qbftEventHandler.handleMessageEvent(msg);

consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/adaptor/BftEventHandlerAdaptorTest.java

@@ -52,6 +52,12 @@ void startDelegatesToQbftEventHandler() {
     verify(qbftEventHandler).start();
   }
 
+  @Test
+  void resetDelegatesToQbftEventHandler() {
+    handler.reset();
+    verify(qbftEventHandler).reset();
+  }
+
   @Test
   void handleMessageEventDelegatesToQbftEventHandler() {
     handler.handleMessageEvent(bftReceivedMessageEvent);