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

* Fix incorrect duration for THREE_MINUTES from 1 minute to 3 minutes Signed-off-by: Bhanu Pulluri <[email protected]> * Round change upon f+1 RC messages (experimental option) Signed-off-by: Bhanu Pulluri <[email protected]> * Update besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java Co-authored-by: Matt Whitehead <[email protected]> Signed-off-by: Bhanu Pulluri <[email protected]> * revert an unrelated fix already merged to main Signed-off-by: Bhanu Pulluri <[email protected]> * update return value description Signed-off-by: Bhanu Pulluri <[email protected]> * fix logging level for couple of logs Signed-off-by: Bhanu Pulluri <[email protected]> * Review changes , added tests Signed-off-by: Bhanu Pulluri <[email protected]> * Merge and fix controller builder test context Signed-off-by: Bhanu Pulluri <[email protected]> * minor fix to import missing class after merging main Signed-off-by: Bhanu Pulluri <[email protected]> * Add missing function header comments Signed-off-by: Bhanu Pulluri <[email protected]> --------- Signed-off-by: Bhanu Pulluri <[email protected]> Signed-off-by: Bhanu Pulluri <[email protected]> Co-authored-by: Bhanu Pulluri <[email protected]> Co-authored-by: Matt Whitehead <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Co-authored-by: Jason Frame <[email protected]> Co-authored-by: Matt Whitehead <[email protected]>
hyperledger · Jan 31, 2025 · 81e1ab9 · 81e1ab9
1 parent ac0265f
commit 81e1ab9
Show file tree

Hide file tree

Showing 12 changed files with 383 additions and 49 deletions.
diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -69,6 +69,7 @@
 import org.hyperledger.besu.cli.options.TransactionPoolOptions;
 import org.hyperledger.besu.cli.options.storage.DataStorageOptions;
 import org.hyperledger.besu.cli.options.storage.DiffBasedSubStorageOptions;
+import org.hyperledger.besu.cli.options.unstable.QBFTOptions;
 import org.hyperledger.besu.cli.presynctasks.PreSynchronizationTaskRunner;
 import org.hyperledger.besu.cli.presynctasks.PrivateDatabaseMigrationPreSyncTask;
 import org.hyperledger.besu.cli.subcommands.PasswordSubCommand;
@@ -301,6 +302,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);
@@ -1794,6 +1797,7 @@ public BesuControllerBuilder setupControllerBuilder() {
             .clock(Clock.systemUTC())
             .isRevertReasonEnabled(isRevertReasonEnabled)
             .storageProvider(storageProvider)
+            .isEarlyRoundChangeEnabled(unstableQbftOptions.isEarlyRoundChangeEnabled())
             .gasLimitCalculator(
                 miningParametersSupplier.get().getTargetGasLimit().isPresent()
                     ? new FrontierTargetingGasLimitCalculator()

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/QBFTOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/QBFTOptions.java
@@ -0,0 +1,49 @@
+/*
+ * 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;
+
+/** Handles configuration options for QBFT consensus */
+public class QBFTOptions {
+
+  /** Default constructor */
+  private QBFTOptions() {}
+
+  /**
+   * Create a new instance of QBFTOptions
+   *
+   * @return a new instance of 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;
+
+  /**
+   * Is early round change enabled boolean.
+   *
+   * @return true if early round change is enabled
+   */
+  public boolean isEarlyRoundChangeEnabled() {
+    return enableEarlyRoundChange;
+  }
+}
diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java
@@ -219,6 +219,9 @@ public abstract class BesuControllerBuilder implements MiningParameterOverrides
   /** The transaction simulator */
   protected TransactionSimulator transactionSimulator;
 
+  /** When enabled, round changes on f+1 RC messages from higher rounds */
+  protected boolean isEarlyRoundChangeEnabled = false;
+
   /** Instantiates a new Besu controller builder. */
   protected BesuControllerBuilder() {}
 
@@ -553,6 +556,17 @@ public BesuControllerBuilder isParallelTxProcessingEnabled(
     return this;
   }
 
+  /**
+   * check if early round change is enabled when f+1 RC messages from higher rounds are received
+   *
+   * @param isEarlyRoundChangeEnabled whether to enable early round change
+   * @return the besu controller
+   */
+  public BesuControllerBuilder isEarlyRoundChangeEnabled(final boolean isEarlyRoundChangeEnabled) {
+    this.isEarlyRoundChangeEnabled = isEarlyRoundChangeEnabled;
+    return this;
+  }
+
   /**
    * Build besu controller.
    *

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java
@@ -245,23 +245,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,

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java
@@ -280,6 +280,7 @@ public void initMocks() throws Exception {
     when(mockControllerBuilder.isRevertReasonEnabled(false)).thenReturn(mockControllerBuilder);
     when(mockControllerBuilder.isParallelTxProcessingEnabled(false))
         .thenReturn(mockControllerBuilder);
+    when(mockControllerBuilder.isEarlyRoundChangeEnabled(false)).thenReturn(mockControllerBuilder);
     when(mockControllerBuilder.storageProvider(any())).thenReturn(mockControllerBuilder);
     when(mockControllerBuilder.gasLimitCalculator(any())).thenReturn(mockControllerBuilder);
     when(mockControllerBuilder.requiredBlocks(any())).thenReturn(mockControllerBuilder);

diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftHelpers.java b/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 Required number of future round change messages to reach quorum for a round change.
+   */
+  public static int calculateRequiredFutureRCQuorum(final int validatorCount) {
+    return (validatorCount - 1) / 3 + 1;
+  }
+
   /**
    * Prepare message count for quorum.
    *

diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/RoundTimer.java b/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));
     }

diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BftHelpersTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BftHelpersTest.java
@@ -63,4 +63,39 @@ public void calculateRequiredValidatorQuorum15Validator() {
   public void calculateRequiredValidatorQuorum20Validator() {
     Assertions.assertThat(BftHelpers.calculateRequiredValidatorQuorum(20)).isEqualTo(14);
   }
+
+  @Test
+  public void calculateRequiredFutureRCQuorum4Validator() {
+    Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(4)).isEqualTo(2);
+  }
+
+  @Test
+  public void calculateRequiredFutureRCQuorum6Validator() {
+    Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(6)).isEqualTo(2);
+  }
+
+  @Test
+  public void calculateRequiredFutureRCQuorum7Validator() {
+    Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(7)).isEqualTo(3);
+  }
+
+  @Test
+  public void calculateRequiredFutureRCQuorum9Validator() {
+    Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(9)).isEqualTo(3);
+  }
+
+  @Test
+  public void calculateRequiredFutureRCQuorum10Validator() {
+    Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(10)).isEqualTo(4);
+  }
+
+  @Test
+  public void calculateRequiredFutureRCQuorum13Validator() {
+    Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(13)).isEqualTo(5);
+  }
+
+  @Test
+  public void calculateRequiredFutureRCQuorum15Validator() {
+    Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(15)).isEqualTo(5);
+  }
 }
diff --git a/...in/java/org/hyperledger/besu/consensus/qbft/core/statemachine/QbftBlockHeightManager.java b/...in/java/org/hyperledger/besu/consensus/qbft/core/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.
@@ -115,6 +116,39 @@ public QbftBlockHeightManager(
     finalState.getBlockTimer().startTimer(roundIdentifier, parentHeader);
   }
 
+  /**
+   * Instantiates a new Qbft block height manager. Secondary constructor with early round change
+   * option.
+   *
+   * @param parentHeader the parent header
+   * @param finalState the final state
+   * @param roundChangeManager the round change manager
+   * @param qbftRoundFactory the qbft round factory
+   * @param clock the clock
+   * @param messageValidatorFactory the message validator factory
+   * @param messageFactory the message factory
+   * @param isEarlyRoundChangeEnabled enable round change when f+1 RC messages are received
+   */
+  public QbftBlockHeightManager(
+      final BlockHeader parentHeader,
+      final BftFinalState finalState,
+      final RoundChangeManager roundChangeManager,
+      final QbftRoundFactory qbftRoundFactory,
+      final Clock clock,
+      final MessageValidatorFactory messageValidatorFactory,
+      final MessageFactory messageFactory,
+      final boolean isEarlyRoundChangeEnabled) {
+    this(
+        parentHeader,
+        finalState,
+        roundChangeManager,
+        qbftRoundFactory,
+        clock,
+        messageValidatorFactory,
+        messageFactory);
+    this.isEarlyRoundChangeEnabled = isEarlyRoundChangeEnabled;
+  }
+
   @Override
   public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifier) {
     if (currentRound.isPresent()) {
@@ -227,23 +261,36 @@ public void roundExpired(final RoundExpiry expire) {
       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 +299,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 +380,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());
+      }
     }
   }