Round change upon f+1 RC messages (experimental option)

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/bftsoak/BftMiningSoakTest.java

@@ -41,7 +41,7 @@ public class BftMiningSoakTest extends ParameterizedBftTestBase {
 
   private static final long ONE_MINUTE = Duration.of(1, ChronoUnit.MINUTES).toMillis();
 
-  private static final long THREE_MINUTES = Duration.of(1, ChronoUnit.MINUTES).toMillis();
+  private static final long THREE_MINUTES = Duration.of(3, ChronoUnit.MINUTES).toMillis();
 
   private static final long TEN_SECONDS = Duration.of(10, ChronoUnit.SECONDS).toMillis();
 

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

@@ -68,6 +68,7 @@
 import org.hyperledger.besu.cli.options.unstable.NetworkingOptions;
 import org.hyperledger.besu.cli.options.unstable.P2PTLSConfigOptions;
 import org.hyperledger.besu.cli.options.unstable.PrivacyPluginOptions;
+import org.hyperledger.besu.cli.options.unstable.QBFTOptions;
 import org.hyperledger.besu.cli.options.unstable.RPCOptions;
 import org.hyperledger.besu.cli.options.unstable.SynchronizerOptions;
 import org.hyperledger.besu.cli.presynctasks.PreSynchronizationTaskRunner;
@@ -303,6 +304,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   private final EvmOptions unstableEvmOptions = EvmOptions.create();
   private final IpcOptions unstableIpcOptions = IpcOptions.create();
   private final ChainPruningOptions unstableChainPruningOptions = ChainPruningOptions.create();
+  private final QBFTOptions unstableQbftOptions = QBFTOptions.create();
 
   // stable CLI options
   final DataStorageOptions dataStorageOptions = DataStorageOptions.create();
@@ -1162,6 +1164,7 @@ private void handleUnstableOptions() {
             .put("EVM Options", unstableEvmOptions)
             .put("IPC Options", unstableIpcOptions)
             .put("Chain Data Pruning Options", unstableChainPruningOptions)
+            .put("QBFT options", unstableQbftOptions)
             .build();
 
     UnstableOptionsSubCommand.createUnstableOptions(commandLine, unstableOptions);
@@ -1806,6 +1809,7 @@ public BesuControllerBuilder setupControllerBuilder() {
         .isRevertReasonEnabled(isRevertReasonEnabled)
         .isParallelTxProcessingEnabled(
             dataStorageConfiguration.getUnstable().isParallelTxProcessingEnabled())
+        .isEarlyRoundChangeEnabled(unstableQbftOptions.isEarlyRoundChangeEnabled())
         .storageProvider(storageProvider)
         .gasLimitCalculator(
             miningParametersSupplier.get().getTargetGasLimit().isPresent()

besu/src/main/java/org/hyperledger/besu/cli/options/unstable/QBFTOptions.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright contributors to Besu.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.cli.options.unstable;
+
+import picocli.CommandLine;
+
+public class QBFTOptions {
+
+  public static QBFTOptions create() {
+    return new QBFTOptions();
+  }
+
+  @CommandLine.Option(
+      names = {"--Xqbft-enable-early-round-change"},
+      description =
+          "Enable early round change upon receiving f+1 valid future Round Change messages from different validators (experimental)",
+      hidden = true)
+  private boolean enableEarlyRoundChange = false;
+
+  public boolean isEarlyRoundChangeEnabled() {
+    return enableEarlyRoundChange;
+  }
+}

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

@@ -206,6 +206,8 @@ public abstract class BesuControllerBuilder implements MiningParameterOverrides
   /** whether parallel transaction processing is enabled or not */
   protected boolean isParallelTxProcessingEnabled;
 
+  protected boolean isEarlyRoundChangeEnabled = false;
+
   /** Instantiates a new Besu controller builder. */
   protected BesuControllerBuilder() {}
 
@@ -529,6 +531,11 @@ public BesuControllerBuilder isParallelTxProcessingEnabled(
     return this;
   }
 
+  public BesuControllerBuilder isEarlyRoundChangeEnabled(final boolean isEarlyRoundChangeEnabled) {
+    this.isEarlyRoundChangeEnabled = isEarlyRoundChangeEnabled;
+    return this;
+  }
+
   /**
    * Build besu controller.
    *

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

@@ -249,23 +249,28 @@ protected MiningCoordinator createMiningCoordinator(
 
     final MessageFactory messageFactory = new MessageFactory(nodeKey);
 
-    final BftEventHandler qbftController =
-        new QbftController(
-            blockchain,
+    QbftBlockHeightManagerFactory qbftBlockHeightManagerFactory =
+        new QbftBlockHeightManagerFactory(
             finalState,
-            new QbftBlockHeightManagerFactory(
+            new QbftRoundFactory(
                 finalState,
-                new QbftRoundFactory(
-                    finalState,
-                    protocolContext,
-                    bftProtocolSchedule,
-                    minedBlockObservers,
-                    messageValidatorFactory,
-                    messageFactory,
-                    bftExtraDataCodec().get()),
+                protocolContext,
+                bftProtocolSchedule,
+                minedBlockObservers,
                 messageValidatorFactory,
                 messageFactory,
-                new ValidatorModeTransitionLogger(qbftForksSchedule)),
+                bftExtraDataCodec().get()),
+            messageValidatorFactory,
+            messageFactory,
+            new ValidatorModeTransitionLogger(qbftForksSchedule));
+
+    qbftBlockHeightManagerFactory.isEarlyRoundChangeEnabled(isEarlyRoundChangeEnabled);
+
+    final BftEventHandler qbftController =
+        new QbftController(
+            blockchain,
+            finalState,
+            qbftBlockHeightManagerFactory,
             gossiper,
             duplicateMessageTracker,
             futureMessageBuffer,

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftHelpers.java

@@ -43,6 +43,16 @@ public static int calculateRequiredValidatorQuorum(final int validatorCount) {
     return Util.fastDivCeiling(2 * validatorCount, 3);
   }
 
+  /**
+   * Calculate required future RC messages count quorum for a round change.
+   *
+   * @param validatorCount the validator count
+   * @return the int
+   */
+  public static int calculateRequiredFutureRCQuorum(final int validatorCount) {
+    return validatorCount - Util.fastDivCeiling(2 * validatorCount, 3) + 1;
+  }
+
   /**
    * Prepare message count for quorum.
    *

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/RoundTimer.java

@@ -83,8 +83,7 @@ public synchronized void startTimer(final ConsensusRoundIdentifier round) {
     // Once we are up to round 2 start logging round expiries
     if (round.getRoundNumber() >= 2) {
       LOG.info(
-          "BFT round {} expired. Moved to round {} which will expire in {} seconds",
-          round.getRoundNumber() - 1,
+          "Moved to round {} which will expire in {} seconds",
           round.getRoundNumber(),
           (expiryTime / 1000));
     }

consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java

@@ -68,6 +68,7 @@ public class QbftBlockHeightManager implements BaseQbftBlockHeightManager {
 
   private Optional<PreparedCertificate> latestPreparedCertificate = Optional.empty();
   private Optional<QbftRound> currentRound = Optional.empty();
+  private boolean isEarlyRoundChangeEnabled = false;
 
   /**
    * Instantiates a new Qbft block height manager.
@@ -213,37 +214,50 @@ private void logValidatorChanges(final QbftRound qbftRound) {
   @Override
   public void roundExpired(final RoundExpiry expire) {
     if (currentRound.isEmpty()) {
-      LOG.error(
+      LOG.info(
           "Received Round timer expiry before round is created timerRound={}", expire.getView());
       return;
     }
 
     QbftRound qbftRound = currentRound.get();
     if (!expire.getView().equals(qbftRound.getRoundIdentifier())) {
-      LOG.trace(
+      LOG.info(
           "Ignoring Round timer expired which does not match current round. round={}, timerRound={}",
           qbftRound.getRoundIdentifier(),
           expire.getView());
       return;
     }
 
+    doRoundChange(qbftRound.getRoundIdentifier().getRoundNumber() + 1);
+  }
+
+  private synchronized void doRoundChange(final int newRoundNumber) {
+
+    if (currentRound.isPresent()
+        && currentRound.get().getRoundIdentifier().getRoundNumber() >= newRoundNumber) {
+      return;
+    }
     LOG.debug(
-        "Round has expired, creating PreparedCertificate and notifying peers. round={}",
-        qbftRound.getRoundIdentifier());
+        "Round has expired or changing based on RC quorum, creating PreparedCertificate and notifying peers. round={}",
+        currentRound.get().getRoundIdentifier());
     final Optional<PreparedCertificate> preparedCertificate =
-        qbftRound.constructPreparedCertificate();
+        currentRound.get().constructPreparedCertificate();
 
     if (preparedCertificate.isPresent()) {
       latestPreparedCertificate = preparedCertificate;
     }
 
-    startNewRound(qbftRound.getRoundIdentifier().getRoundNumber() + 1);
-    qbftRound = currentRound.get();
+    startNewRound(newRoundNumber);
+    if (currentRound.isEmpty()) {
+      LOG.info("Failed to start round ");
+      return;
+    }
+    QbftRound qbftRoundNew = currentRound.get();
 
     try {
       final RoundChange localRoundChange =
           messageFactory.createRoundChange(
-              qbftRound.getRoundIdentifier(), latestPreparedCertificate);
+              qbftRoundNew.getRoundIdentifier(), latestPreparedCertificate);
 
       // Its possible the locally created RoundChange triggers the transmission of a NewRound
       // message - so it must be handled accordingly.
@@ -252,7 +266,7 @@ public void roundExpired(final RoundExpiry expire) {
       LOG.warn("Failed to create signed RoundChange message.", e);
     }
 
-    transmitter.multicastRoundChange(qbftRound.getRoundIdentifier(), latestPreparedCertificate);
+    transmitter.multicastRoundChange(qbftRoundNew.getRoundIdentifier(), latestPreparedCertificate);
   }
 
   @Override
@@ -333,24 +347,55 @@ public void handleRoundChangePayload(final RoundChange message) {
     final Optional<Collection<RoundChange>> result =
         roundChangeManager.appendRoundChangeMessage(message);
 
-    if (result.isPresent()) {
-      LOG.debug(
-          "Received sufficient RoundChange messages to change round to targetRound={}",
-          targetRound);
-      if (messageAge == MessageAge.FUTURE_ROUND) {
-        startNewRound(targetRound.getRoundNumber());
-      }
+    if (!isEarlyRoundChangeEnabled) {
+      if (result.isPresent()) {
+        LOG.debug(
+            "Received sufficient RoundChange messages to change round to targetRound={}",
+            targetRound);
+        if (messageAge == MessageAge.FUTURE_ROUND) {
+          startNewRound(targetRound.getRoundNumber());
+        }
 
-      final RoundChangeArtifacts roundChangeMetadata = RoundChangeArtifacts.create(result.get());
+        final RoundChangeArtifacts roundChangeMetadata = RoundChangeArtifacts.create(result.get());
 
-      if (finalState.isLocalNodeProposerForRound(targetRound)) {
-        if (currentRound.isEmpty()) {
-          startNewRound(0);
+        if (finalState.isLocalNodeProposerForRound(targetRound)) {
+          if (currentRound.isEmpty()) {
+            startNewRound(0);
+          }
+          currentRound
+              .get()
+              .startRoundWith(roundChangeMetadata, TimeUnit.MILLISECONDS.toSeconds(clock.millis()));
         }
+      }
+    } else {
+
+      if (currentRound.isEmpty()) {
+        startNewRound(0);
+      }
+      int currentRoundNumber = currentRound.get().getRoundIdentifier().getRoundNumber();
+      // If this node is proposer for the current round, check if quorum is achieved for RC messages
+      // aiming this round
+      if (targetRound.getRoundNumber() == currentRoundNumber
+          && finalState.isLocalNodeProposerForRound(targetRound)
+          && result.isPresent()) {
+
+        final RoundChangeArtifacts roundChangeMetadata = RoundChangeArtifacts.create(result.get());
+
         currentRound
             .get()
             .startRoundWith(roundChangeMetadata, TimeUnit.MILLISECONDS.toSeconds(clock.millis()));
       }
+
+      // check if f+1 RC messages for future rounds are received
+      QbftRound qbftRound = currentRound.get();
+      Optional<Integer> nextHigherRound =
+          roundChangeManager.futureRCQuorumReceived(qbftRound.getRoundIdentifier());
+      if (nextHigherRound.isPresent()) {
+        LOG.info(
+            "Received sufficient RoundChange messages to change round to targetRound={}",
+            nextHigherRound.get());
+        doRoundChange(nextHigherRound.get());
+      }
     }
   }
 
@@ -391,6 +436,10 @@ private MessageAge determineAgeOfPayload(final int messageRoundNumber) {
     return MessageAge.PRIOR_ROUND;
   }
 
+  public void isEarlyRoundChangeEnabled(final boolean isEarlyRoundChangeEnabled) {
+    this.isEarlyRoundChangeEnabled = isEarlyRoundChangeEnabled;
+  }
+
   /** The enum Message age. */
   public enum MessageAge {
     /** Prior round message age. */

consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerFactory.java

@@ -34,6 +34,7 @@ public class QbftBlockHeightManagerFactory {
   private final MessageValidatorFactory messageValidatorFactory;
   private final MessageFactory messageFactory;
   private final ValidatorModeTransitionLogger validatorModeTransitionLogger;
+  private boolean isEarlyRoundChangeEnabled = false;
 
   /**
    * Instantiates a new Qbft block height manager factory.
@@ -75,22 +76,39 @@ public BaseQbftBlockHeightManager create(final BlockHeader parentHeader) {
     }
   }
 
+  public void isEarlyRoundChangeEnabled(final boolean isEarlyRoundChangeEnabled) {
+    this.isEarlyRoundChangeEnabled = isEarlyRoundChangeEnabled;
+  }
+
   private BaseQbftBlockHeightManager createNoOpBlockHeightManager(final BlockHeader parentHeader) {
     return new NoOpBlockHeightManager(parentHeader);
   }
 
   private BaseQbftBlockHeightManager createFullBlockHeightManager(final BlockHeader parentHeader) {
-    return new QbftBlockHeightManager(
-        parentHeader,
-        finalState,
+
+    RoundChangeManager roundChangeManager =
         new RoundChangeManager(
             BftHelpers.calculateRequiredValidatorQuorum(finalState.getValidators().size()),
             messageValidatorFactory.createRoundChangeMessageValidator(
                 parentHeader.getNumber() + 1L, parentHeader),
-            finalState.getLocalAddress()),
-        roundFactory,
-        finalState.getClock(),
-        messageValidatorFactory,
-        messageFactory);
+            finalState.getLocalAddress());
+
+    QbftBlockHeightManager qbftBlockHeightManager =
+        new QbftBlockHeightManager(
+            parentHeader,
+            finalState,
+            roundChangeManager,
+            roundFactory,
+            finalState.getClock(),
+            messageValidatorFactory,
+            messageFactory);
+
+    if (isEarlyRoundChangeEnabled) {
+      roundChangeManager.setRCQuorum(
+          BftHelpers.calculateRequiredFutureRCQuorum(finalState.getValidators().size()));
+      qbftBlockHeightManager.isEarlyRoundChangeEnabled(true);
+    }
+
+    return qbftBlockHeightManager;
   }
 }