Implementing support for emptyBlockPeriodSeconds in QBFT (Issue #3810)

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

@@ -286,12 +286,18 @@ protected MiningCoordinator createMiningCoordinator(
     protocolContext
         .getBlockchain()
         .observeBlockAdded(
-            o ->
-                miningParameters.setBlockPeriodSeconds(
-                    qbftForksSchedule
-                        .getFork(o.getBlock().getHeader().getNumber() + 1)
-                        .getValue()
-                        .getBlockPeriodSeconds()));
+            o -> {
+              miningParameters.setBlockPeriodSeconds(
+                  qbftForksSchedule
+                      .getFork(o.getBlock().getHeader().getNumber() + 1)
+                      .getValue()
+                      .getBlockPeriodSeconds());
+              miningParameters.setEmptyBlockPeriodSeconds(
+                  qbftForksSchedule
+                      .getFork(o.getBlock().getHeader().getNumber() + 1)
+                      .getValue()
+                      .getEmptyBlockPeriodSeconds());
+            });
 
     if (syncState.isInitialSyncPhaseDone()) {
       miningCoordinator.enable();
@@ -419,8 +425,9 @@ private static MinedBlockObserver blockLogger(
     return block ->
         LOG.info(
             String.format(
-                "%s #%,d / %d tx / %d pending / %,d (%01.1f%%) gas / (%s)",
+                "%s %s #%,d / %d tx / %d pending / %,d (%01.1f%%) gas / (%s)",
                 block.getHeader().getCoinbase().equals(localAddress) ? "Produced" : "Imported",
+                block.getBody().getTransactions().size() == 0 ? "empty block" : "block",
                 block.getHeader().getNumber(),
                 block.getBody().getTransactions().size(),
                 transactionPool.count(),

besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java

@@ -141,15 +141,18 @@ public abstract class CommandTestAbstract {
   private static final Logger TEST_LOGGER = LoggerFactory.getLogger(CommandTestAbstract.class);
 
   protected static final int POA_BLOCK_PERIOD_SECONDS = 5;
+  protected static final int POA_EMPTY_BLOCK_PERIOD_SECONDS = 50;
   protected static final JsonObject VALID_GENESIS_QBFT_POST_LONDON =
       (new JsonObject())
           .put(
               "config",
               new JsonObject()
                   .put("londonBlock", 0)
+                  .put("qbft", new JsonObject().put("blockperiodseconds", POA_BLOCK_PERIOD_SECONDS))
                   .put(
                       "qbft",
-                      new JsonObject().put("blockperiodseconds", POA_BLOCK_PERIOD_SECONDS)));
+                      new JsonObject()
+                          .put("emptyblockperiodseconds", POA_EMPTY_BLOCK_PERIOD_SECONDS)));
   protected static final JsonObject VALID_GENESIS_IBFT2_POST_LONDON =
       (new JsonObject())
           .put(

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

@@ -37,6 +37,13 @@ public interface BftConfigOptions {
    */
   int getBlockPeriodSeconds();
 
+  /**
+   * Gets empty block period seconds.
+   *
+   * @return the empty block period seconds
+   */
+  int getEmptyBlockPeriodSeconds();
+
   /**
    * Gets request timeout seconds.
    *

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

@@ -40,6 +40,9 @@ public class BftFork implements Fork {
   /** The constant BLOCK_PERIOD_SECONDS_KEY. */
   public static final String BLOCK_PERIOD_SECONDS_KEY = "blockperiodseconds";
 
+  /** The constant EMPTY_BLOCK_PERIOD_SECONDS_KEY. */
+  public static final String EMPTY_BLOCK_PERIOD_SECONDS_KEY = "emptyblockperiodseconds";
+
   /** The constant BLOCK_REWARD_KEY. */
   public static final String BLOCK_REWARD_KEY = "blockreward";
 
@@ -82,6 +85,16 @@ public OptionalInt getBlockPeriodSeconds() {
     return JsonUtil.getPositiveInt(forkConfigRoot, BLOCK_PERIOD_SECONDS_KEY);
   }
 
+  /**
+   * Gets empty block period seconds.
+   *
+   * @return the empty block period seconds
+   */
+  public OptionalInt getEmptyBlockPeriodSeconds() {
+    // It can be 0 to disable custom empty block periods
+    return JsonUtil.getInt(forkConfigRoot, EMPTY_BLOCK_PERIOD_SECONDS_KEY);
+  }
+
   /**
    * Gets block reward wei.
    *

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

@@ -34,6 +34,8 @@ public class JsonBftConfigOptions implements BftConfigOptions {
 
   private static final long DEFAULT_EPOCH_LENGTH = 30_000;
   private static final int DEFAULT_BLOCK_PERIOD_SECONDS = 1;
+  // 0 keeps working as before, increase to activate it
+  private static final int DEFAULT_EMPTY_BLOCK_PERIOD_SECONDS = 0;
   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
@@ -66,6 +68,12 @@ public int getBlockPeriodSeconds() {
         bftConfigRoot, "blockperiodseconds", DEFAULT_BLOCK_PERIOD_SECONDS);
   }
 
+  @Override
+  public int getEmptyBlockPeriodSeconds() {
+    return JsonUtil.getInt(
+        bftConfigRoot, "emptyblockperiodseconds", DEFAULT_EMPTY_BLOCK_PERIOD_SECONDS);
+  }
+
   @Override
   public int getRequestTimeoutSeconds() {
     return JsonUtil.getInt(bftConfigRoot, "requesttimeoutseconds", DEFAULT_ROUND_EXPIRY_SECONDS);
@@ -133,6 +141,9 @@ public Map<String, Object> asMap() {
     if (bftConfigRoot.has("blockperiodseconds")) {
       builder.put("blockPeriodSeconds", getBlockPeriodSeconds());
     }
+    if (bftConfigRoot.has("emptyblockperiodseconds")) {
+      builder.put("emptyblockperiodseconds", getEmptyBlockPeriodSeconds());
+    }
     if (bftConfigRoot.has("requesttimeoutseconds")) {
       builder.put("requestTimeoutSeconds", getRequestTimeoutSeconds());
     }

config/src/test/java/org/hyperledger/besu/config/JsonBftConfigOptionsTest.java

@@ -17,6 +17,7 @@
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import org.hyperledger.besu.datatypes.Address;
@@ -30,6 +31,7 @@ public class JsonBftConfigOptionsTest {
 
   private static final int EXPECTED_DEFAULT_EPOCH_LENGTH = 30_000;
   private static final int EXPECTED_DEFAULT_BLOCK_PERIOD = 1;
+  private static final int EXPECTED_EMPTY_DEFAULT_BLOCK_PERIOD = 0;
   private static final int EXPECTED_DEFAULT_REQUEST_TIMEOUT = 1;
   private static final int EXPECTED_DEFAULT_GOSSIPED_HISTORY_LIMIT = 1000;
   private static final int EXPECTED_DEFAULT_MESSAGE_QUEUE_LIMIT = 1000;
@@ -61,25 +63,51 @@ public void shouldGetBlockPeriodFromConfig() {
     assertThat(config.getBlockPeriodSeconds()).isEqualTo(5);
   }
 
+  @Test
+  public void shouldGetEmptyBlockPeriodFromConfig() {
+    final BftConfigOptions config = fromConfigOptions(singletonMap("emptyblockperiodseconds", 60));
+    assertThat(config.getEmptyBlockPeriodSeconds()).isEqualTo(60);
+  }
+
   @Test
   public void shouldFallbackToDefaultBlockPeriod() {
     final BftConfigOptions config = fromConfigOptions(emptyMap());
     assertThat(config.getBlockPeriodSeconds()).isEqualTo(EXPECTED_DEFAULT_BLOCK_PERIOD);
   }
 
+  @Test
+  public void shouldFallbackToEmptyDefaultBlockPeriod() {
+    final BftConfigOptions config = fromConfigOptions(emptyMap());
+    assertThat(config.getEmptyBlockPeriodSeconds()).isEqualTo(EXPECTED_EMPTY_DEFAULT_BLOCK_PERIOD);
+  }
+
   @Test
   public void shouldGetDefaultBlockPeriodFromDefaultConfig() {
     assertThat(JsonBftConfigOptions.DEFAULT.getBlockPeriodSeconds())
         .isEqualTo(EXPECTED_DEFAULT_BLOCK_PERIOD);
   }
 
+  @Test
+  public void shouldGetDefaultEmptyBlockPeriodFromDefaultConfig() {
+
+    assertThat(JsonBftConfigOptions.DEFAULT.getEmptyBlockPeriodSeconds())
+        .isEqualTo(EXPECTED_EMPTY_DEFAULT_BLOCK_PERIOD);
+  }
+
   @Test
   public void shouldThrowOnNonPositiveBlockPeriod() {
     final BftConfigOptions config = fromConfigOptions(singletonMap("blockperiodseconds", -1));
     assertThatThrownBy(() -> config.getBlockPeriodSeconds())
         .isInstanceOf(IllegalArgumentException.class);
   }
 
+  @Test
+  public void shouldNotThrowOnNonPositiveEmptyBlockPeriod() {
+    // can be 0 to be compatible with older versions
+    final BftConfigOptions config = fromConfigOptions(singletonMap("emptyblockperiodseconds", 0));
+    assertThatCode(() -> config.getEmptyBlockPeriodSeconds()).doesNotThrowAnyException();
+  }
+
   @Test
   public void shouldGetRequestTimeoutFromConfig() {
     final BftConfigOptions config = fromConfigOptions(singletonMap("requesttimeoutseconds", 5));

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

@@ -24,14 +24,20 @@
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 /** Class for starting and keeping organised block timers */
 public class BlockTimer {
 
+  private static final Logger LOG = LoggerFactory.getLogger(BlockTimer.class);
   private final ForksSchedule<? extends BftConfigOptions> forksSchedule;
   private final BftExecutors bftExecutors;
   private Optional<ScheduledFuture<?>> currentTimerTask;
   private final BftEventQueue queue;
   private final Clock clock;
+  private long blockPeriodSeconds;
+  private long emptyBlockPeriodSeconds;
 
   /**
    * Construct a BlockTimer with primed executor service ready to start timers
@@ -51,6 +57,8 @@ public BlockTimer(
     this.bftExecutors = bftExecutors;
     this.currentTimerTask = Optional.empty();
     this.clock = clock;
+    this.blockPeriodSeconds = 0;
+    this.emptyBlockPeriodSeconds = 0;
   }
 
   /** Cancels the current running round timer if there is one */
@@ -76,16 +84,61 @@ public synchronized boolean isRunning() {
    */
   public synchronized void startTimer(
       final ConsensusRoundIdentifier round, final BlockHeader chainHeadHeader) {
-    cancelTimer();
-
-    final long now = clock.millis();
 
     // 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);
+
+    startTimer(round, expiryTime);
+  }
+
+  /**
+   * Checks if the empty block timer is expired
+   *
+   * @param chainHeadHeader The header of the chain head
+   * @param currentTimeInMillis The current time
+   * @return a boolean value
+   */
+  public synchronized boolean checkEmptyBlockExpired(
+      final BlockHeader chainHeadHeader, final long currentTimeInMillis) {
+    final long emptyBlockPeriodExpiryTime =
+        (chainHeadHeader.getTimestamp() + emptyBlockPeriodSeconds) * 1000;
+
+    if (currentTimeInMillis > emptyBlockPeriodExpiryTime) {
+      LOG.info("Empty Block expired");
+      return true;
+    }
+    LOG.info("Empty Block NOT expired");
+    return false;
+  }
+
+  /**
+   * Resets the empty block timer
+   *
+   * @param roundIdentifier The current round identifier
+   * @param chainHeadHeader The header of the chain head
+   * @param currentTimeInMillis The current time
+   */
+  public void resetTimerForEmptyBlock(
+      final ConsensusRoundIdentifier roundIdentifier,
+      final BlockHeader chainHeadHeader,
+      final long currentTimeInMillis) {
+    final long emptyBlockPeriodExpiryTime =
+        (chainHeadHeader.getTimestamp() + emptyBlockPeriodSeconds) * 1000;
+    final long nextBlockPeriodExpiryTime = currentTimeInMillis + blockPeriodSeconds * 1000;
+
+    startTimer(roundIdentifier, Math.min(emptyBlockPeriodExpiryTime, nextBlockPeriodExpiryTime));
+  }
+
+  private synchronized void startTimer(
+      final ConsensusRoundIdentifier round, final long expiryTime) {
+    cancelTimer();
+    final long now = clock.millis();
+
     if (expiryTime > now) {
       final long delay = expiryTime - now;
 
@@ -98,4 +151,29 @@ public synchronized void startTimer(
       queue.add(new BlockTimerExpiry(round));
     }
   }
+
+  private synchronized void setBlockTimes(final ConsensusRoundIdentifier round) {
+    final BftConfigOptions currentConfigOptions =
+        forksSchedule.getFork(round.getSequenceNumber()).getValue();
+    this.blockPeriodSeconds = currentConfigOptions.getBlockPeriodSeconds();
+    this.emptyBlockPeriodSeconds = currentConfigOptions.getEmptyBlockPeriodSeconds();
+  }
+
+  /**
+   * Retrieves the Block Period Seconds
+   *
+   * @return the Block Period Seconds
+   */
+  public synchronized long getBlockPeriodSeconds() {
+    return blockPeriodSeconds;
+  }
+
+  /**
+   * Retrieves the Empty Block Period Seconds
+   *
+   * @return the Empty Block Period Seconds
+   */
+  public synchronized long getEmptyBlockPeriodSeconds() {
+    return emptyBlockPeriodSeconds;
+  }
 }

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

@@ -29,8 +29,10 @@
  * ForksSchedule}*.
  */
 public class MutableBftConfigOptions implements BftConfigOptions {
+
   private long epochLength;
   private int blockPeriodSeconds;
+  private int emptyBlockPeriodSeconds;
   private int requestTimeoutSeconds;
   private int gossipedHistoryLimit;
   private int messageQueueLimit;
@@ -48,6 +50,7 @@ public class MutableBftConfigOptions implements BftConfigOptions {
   public MutableBftConfigOptions(final BftConfigOptions bftConfigOptions) {
     this.epochLength = bftConfigOptions.getEpochLength();
     this.blockPeriodSeconds = bftConfigOptions.getBlockPeriodSeconds();
+    this.emptyBlockPeriodSeconds = bftConfigOptions.getEmptyBlockPeriodSeconds();
     this.requestTimeoutSeconds = bftConfigOptions.getRequestTimeoutSeconds();
     this.gossipedHistoryLimit = bftConfigOptions.getGossipedHistoryLimit();
     this.messageQueueLimit = bftConfigOptions.getMessageQueueLimit();
@@ -68,6 +71,11 @@ public int getBlockPeriodSeconds() {
     return blockPeriodSeconds;
   }
 
+  @Override
+  public int getEmptyBlockPeriodSeconds() {
+    return emptyBlockPeriodSeconds;
+  }
+
   @Override
   public int getRequestTimeoutSeconds() {
     return requestTimeoutSeconds;
@@ -131,6 +139,15 @@ public void setBlockPeriodSeconds(final int blockPeriodSeconds) {
     this.blockPeriodSeconds = blockPeriodSeconds;
   }
 
+  /**
+   * Sets empty block period seconds.
+   *
+   * @param emptyBlockPeriodSeconds the empty block period seconds
+   */
+  public void setEmptyBlockPeriodSeconds(final int emptyBlockPeriodSeconds) {
+    this.emptyBlockPeriodSeconds = emptyBlockPeriodSeconds;
+  }
+
   /**
    * Sets request timeout seconds.
    *