Implementing support for emptyBlockPeriodSeconds in QBFT (Issue hyper…

…ledger#3810)

Signed-off-by: amsmota <[email protected]>

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

@@ -2128,6 +2128,8 @@ private MiningParameters getMiningParameters() {
     if (miningParameters == null) {
       miningOptions.setGenesisBlockPeriodSeconds(
           getGenesisBlockPeriodSeconds(getActualGenesisConfigOptions()));
+      miningOptions.setGenesisEmptyBlockPeriodSeconds(
+              getGenesisEmptyBlockPeriodSeconds(getActualGenesisConfigOptions()));
       miningParameters = miningOptions.toDomainObject();
       initMiningParametersMetrics(miningParameters);
     }
@@ -2162,6 +2164,23 @@ private OptionalInt getGenesisBlockPeriodSeconds(
     return OptionalInt.empty();
   }
 
+  private OptionalInt getGenesisEmptyBlockPeriodSeconds(
+          final GenesisConfigOptions genesisConfigOptions) {
+    if (genesisConfigOptions.isClique()) {
+      return OptionalInt.of(genesisConfigOptions.getCliqueConfigOptions().getEmptyBlockPeriodSeconds());
+    }
+
+    if (genesisConfigOptions.isIbft2()) {
+      return OptionalInt.of(genesisConfigOptions.getBftConfigOptions().getEmptyBlockPeriodSeconds());
+    }
+
+    if (genesisConfigOptions.isQbft()) {
+      return OptionalInt.of(genesisConfigOptions.getQbftConfigOptions().getEmptyBlockPeriodSeconds());
+    }
+
+    return OptionalInt.empty();
+  }
+
   private boolean isPruningEnabled() {
     return pruningEnabled;
   }

besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java

@@ -190,6 +190,7 @@ static class Unstable {
   }
 
   private OptionalInt maybeGenesisBlockPeriodSeconds;
+  private OptionalInt maybeEmptyGenesisBlockPeriodSeconds;
 
   private MiningOptions() {}
 
@@ -212,6 +213,16 @@ public void setGenesisBlockPeriodSeconds(final OptionalInt genesisBlockPeriodSec
     maybeGenesisBlockPeriodSeconds = genesisBlockPeriodSeconds;
   }
 
+  /**
+   * Set the optional genesis empty block period per seconds
+   *
+   * @param genesisEmptyBlockPeriodSeconds if the network is PoA then the empty block period in seconds
+   *     specified in the genesis file, otherwise empty.
+   */
+  public void setGenesisEmptyBlockPeriodSeconds(final OptionalInt genesisEmptyBlockPeriodSeconds) {
+    maybeEmptyGenesisBlockPeriodSeconds = genesisEmptyBlockPeriodSeconds;
+  }
+
   /**
    * Validate that there are no inconsistencies in the specified options. For example that the
    * options are valid for the selected implementation.
@@ -299,6 +310,7 @@ public void validate(
   static MiningOptions fromConfig(final MiningParameters miningParameters) {
     final MiningOptions miningOptions = MiningOptions.create();
     miningOptions.setGenesisBlockPeriodSeconds(miningParameters.getGenesisBlockPeriodSeconds());
+    miningOptions.setGenesisEmptyBlockPeriodSeconds(miningParameters.getGenesisEmptyBlockPeriodSeconds());
     miningOptions.isMiningEnabled = miningParameters.isMiningEnabled();
     miningOptions.iStratumMiningEnabled = miningParameters.isStratumMiningEnabled();
     miningOptions.stratumNetworkInterface = miningParameters.getStratumNetworkInterface();
@@ -337,6 +349,10 @@ public MiningParameters toDomainObject() {
       throw new IllegalStateException(
           "genesisBlockPeriodSeconds must be set before using this object");
     }
+    if (maybeGenesisEmptyBlockPeriodSeconds == null) {
+      throw new IllegalStateException(
+              "genesisEmptyBlockPeriodSeconds must be set before using this object");
+    }
 
     final var updatableInitValuesBuilder =
         MutableInitValues.builder()
@@ -355,6 +371,7 @@ public MiningParameters toDomainObject() {
 
     return ImmutableMiningParameters.builder()
         .genesisBlockPeriodSeconds(maybeGenesisBlockPeriodSeconds)
+        .genesisEmptyBBlockPeriodSeconds(maybeGenesisEmptyBBlockPeriodSeconds)
         .mutableInitValues(updatableInitValuesBuilder.build())
         .isStratumMiningEnabled(iStratumMiningEnabled)
         .stratumNetworkInterface(stratumNetworkInterface)

config/src/main/java/org/hyperledger/besu/config/BftConfigOptions.java

@@ -40,7 +40,7 @@ public interface BftConfigOptions {
   /**
    * Gets empty block period seconds.
    *
-   * @return the block period seconds
+   * @return the empty block period seconds
    */
   int getEmptyBlockPeriodSeconds();
 

config/src/main/java/org/hyperledger/besu/config/BftFork.java

@@ -81,10 +81,11 @@ public OptionalInt getBlockPeriodSeconds() {
   /**
    * Gets empty block period seconds.
    *
-   * @return the block empty period seconds
+   * @return the empty block period seconds
    */
   public OptionalInt getEmptyBlockPeriodSeconds() {
-    return JsonUtil.getPositiveInt(forkConfigRoot, EMPTY_BLOCK_PERIOD_SECONDS_KEY);
+    // it can be 0 for emptyblockperiodseconds
+    return JsonUtil.getInt(forkConfigRoot, EMPTY_BLOCK_PERIOD_SECONDS_KEY);
   }
 
   /**

config/src/main/java/org/hyperledger/besu/config/JsonBftConfigOptions.java

@@ -34,7 +34,7 @@ public class JsonBftConfigOptions implements BftConfigOptions {
   private static final long DEFAULT_EPOCH_LENGTH = 30_000;
   private static final int DEFAULT_BLOCK_PERIOD_SECONDS = 1;
   private static final int DEFAULT_EMPTY_BLOCK_PERIOD_SECONDS = 0;
-  // 0 keeps working as before, increase to activate itby default if needed
+  // 0 keeps working as before, increase to activate it
   private static final int DEFAULT_ROUND_EXPIRY_SECONDS = 1;
   // In a healthy network this can be very small. This default limit will allow for suitable
   // protection for on a typical 20 node validator network with multiple rounds
@@ -73,7 +73,6 @@ public int getEmptyBlockPeriodSeconds() {
             bftConfigRoot, "emptyblockperiodseconds", DEFAULT_EMPTY_BLOCK_PERIOD_SECONDS);
   }
 
-
   @Override
   public int getRequestTimeoutSeconds() {
     return JsonUtil.getInt(bftConfigRoot, "requesttimeoutseconds", DEFAULT_ROUND_EXPIRY_SECONDS);

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

@@ -86,9 +86,9 @@ public synchronized void startTimer(
       final ConsensusRoundIdentifier round, final BlockHeader chainHeadHeader) {
 
     // absolute time when the timer is supposed to expire
-    final int blockPeriodSeconds =
+    final int currentBlockPeriodSeconds =
         forksSchedule.getFork(round.getSequenceNumber()).getValue().getBlockPeriodSeconds();
-    final long minimumTimeBetweenBlocksMillis = blockPeriodSeconds * 1000L;
+    final long minimumTimeBetweenBlocksMillis = currentBlockPeriodSeconds * 1000L;
     final long expiryTime = chainHeadHeader.getTimestamp() * 1_000 + minimumTimeBetweenBlocksMillis;
 
     setBlockTimes(round, false);
@@ -136,12 +136,16 @@ private synchronized void setBlockTimes(final ConsensusRoundIdentifier round, fi
             ? this.emptyBlockPeriodSeconds
             : this.blockPeriodSeconds;
 
-    LOG.debug("NEW CURRENTBLOCKPERIODSECONDS SET TO {}:  {}", isEmpty?"EMPTYBLOCKPERIODSECONDS":"BLOCKPERIODSECONDS", currentBlockPeriodSeconds);
+    LOG.debug(
+            "NEW CURRENTBLOCKPERIODSECONDS SET TO {}:  {}"
+            , isEmpty?"EMPTYBLOCKPERIODSECONDS":"BLOCKPERIODSECONDS"
+            , currentBlockPeriodSeconds);
   }
 
   public synchronized long getBlockPeriodSeconds(){
     return blockPeriodSeconds;
   }
+
   public synchronized long getEmptyBlockPeriodSeconds(){
     return emptyBlockPeriodSeconds;
   }

consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/QbftBlockHeaderValidationRulesetFactory.java

@@ -59,6 +59,7 @@ public static BlockHeaderValidator.Builder blockHeaderValidator(
                 DEFAULT_MIN_GAS_LIMIT, DEFAULT_MAX_GAS_LIMIT, baseFeeMarket))
         .addRule(new TimestampBoundedByFutureParameter(1))
         //.addRule(new TimestampMoreRecentThanParent(secondsBetweenBlocks))
+        // removed because of conflict with emptyblockperiodseconds
         .addRule(
             new ConstantFieldValidationRule<>(
                 "MixHash", BlockHeader::getMixHash, BftHelpers.EXPECTED_MIX_HASH))

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

@@ -128,7 +128,9 @@ public ConsensusRoundIdentifier getRoundIdentifier() {
    * @return a Block
    */
   public Block createBlock(final long headerTimeStampSeconds) {
-    LOG.debug("Creating proposed block. round={}", roundState.getRoundIdentifier());
+    LOG.debug(
+            "Creating proposed block. round={}",
+            roundState.getRoundIdentifier());
     return blockCreator.createBlock(headerTimeStampSeconds).getBlock();
   }
 
@@ -138,7 +140,9 @@ public Block createBlock(final long headerTimeStampSeconds) {
    * @param headerTimeStampSeconds the header time stamp seconds
    */
   public void sendProposalMessage(final Block block) {
-    LOG.trace("Creating proposed block blockHeader={}", block.getHeader());
+    LOG.trace(
+            "Creating proposed block blockHeader={}",
+            block.getHeader());
     updateStateWithProposalAndTransmit(block, emptyList(), emptyList());
   }