Implementing support for emptyBlockPeriodSeconds in QBFT (Issue #3810) (

#6965) Implemented support for emptyBlockPeriodSeconds in QBFT (Issue #3810) Introduces experimental xemptyblockperiodseconds genesis config option for producing empty blocks at a specific interval independently of the value of the existing blockperiodseconds setting. #3810 --------- Signed-off-by: Antonio Mota <[email protected]> Signed-off-by: amsmota <[email protected]>
hyperledger · Sep 24, 2024 · aed6bb0 · aed6bb0
1 parent e0518c6
commit aed6bb0
Show file tree

Hide file tree

Showing 20 changed files with 388 additions and 154 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,8 @@
 ### Additions and Improvements
 - Remove privacy test classes support [#7569](https://github.com/hyperledger/besu/pull/7569)
 - Add Blob Transaction Metrics [#7622](https://github.com/hyperledger/besu/pull/7622)
+- Implemented support for emptyBlockPeriodSeconds in QBFT [#6965](https://github.com/hyperledger/besu/pull/6965)
+
 
 ### Bug fixes
 - Fix mounted data path directory permissions for besu user [#7575](https://github.com/hyperledger/besu/pull/7575)

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java
@@ -288,12 +288,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();
@@ -422,8 +428,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(),

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java
@@ -138,15 +138,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("xemptyblockperiodseconds", POA_EMPTY_BLOCK_PERIOD_SECONDS)));
   protected static final JsonObject VALID_GENESIS_IBFT2_POST_LONDON =
       (new JsonObject())
           .put(

diff --git a/config/src/main/java/org/hyperledger/besu/config/BftConfigOptions.java b/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 block period milliseconds. For TESTING only. If set then blockperiodseconds is ignored.
    *

diff --git a/config/src/main/java/org/hyperledger/besu/config/BftFork.java b/config/src/main/java/org/hyperledger/besu/config/BftFork.java
@@ -41,6 +41,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 = "xemptyblockperiodseconds";
+
   /** The constant BLOCK_PERIOD_MILLISECONDS_KEY. */
   public static final String BLOCK_PERIOD_MILLISECONDS_KEY = "xblockperiodmilliseconds";
 
@@ -86,6 +89,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 period milliseconds. Experimental for test scenarios only.
    *

diff --git a/config/src/main/java/org/hyperledger/besu/config/JsonBftConfigOptions.java b/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_BLOCK_PERIOD_MILLISECONDS = 0; // Experimental for test only
   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
@@ -67,6 +69,12 @@ public int getBlockPeriodSeconds() {
         bftConfigRoot, "blockperiodseconds", DEFAULT_BLOCK_PERIOD_SECONDS);
   }
 
+  @Override
+  public int getEmptyBlockPeriodSeconds() {
+    return JsonUtil.getInt(
+        bftConfigRoot, "xemptyblockperiodseconds", DEFAULT_EMPTY_BLOCK_PERIOD_SECONDS);
+  }
+
   @Override
   public long getBlockPeriodMilliseconds() {
     return JsonUtil.getLong(

diff --git a/config/src/test/java/org/hyperledger/besu/config/JsonBftConfigOptionsTest.java b/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("xemptyblockperiodseconds", 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("xemptyblockperiodseconds", 0));
+    assertThatCode(() -> config.getEmptyBlockPeriodSeconds()).doesNotThrowAnyException();
+  }
+
   @Test
   public void shouldGetRequestTimeoutFromConfig() {
     final BftConfigOptions config = fromConfigOptions(singletonMap("requesttimeoutseconds", 5));

diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BlockTimer.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BlockTimer.java
@@ -37,6 +37,8 @@ public class BlockTimer {
   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
@@ -56,6 +58,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 */
@@ -83,13 +87,11 @@ public synchronized void startTimer(
       final ConsensusRoundIdentifier round, final BlockHeader chainHeadHeader) {
     cancelTimer();
 
-    final long now = clock.millis();
     final long expiryTime;
 
     // Experimental option for test scenarios only. Not for production use.
     final long blockPeriodMilliseconds =
         forksSchedule.getFork(round.getSequenceNumber()).getValue().getBlockPeriodMilliseconds();
-
     if (blockPeriodMilliseconds > 0) {
       // Experimental mode for setting < 1 second block periods e.g. for CI/CD pipelines
       // running tests against Besu
@@ -99,12 +101,60 @@ public synchronized void startTimer(
           blockPeriodMilliseconds);
     } else {
       // 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;
       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.debug("Empty Block expired");
+      return true;
+    }
+    LOG.debug("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;
 
@@ -117,4 +167,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;
+  }
 }
diff --git a/...mmon/src/main/java/org/hyperledger/besu/consensus/common/bft/MutableBftConfigOptions.java b/...mmon/src/main/java/org/hyperledger/besu/consensus/common/bft/MutableBftConfigOptions.java
@@ -31,6 +31,7 @@
 public class MutableBftConfigOptions implements BftConfigOptions {
   private long epochLength;
   private int blockPeriodSeconds;
+  private int emptyBlockPeriodSeconds;
   private long blockPeriodMilliseconds;
   private int requestTimeoutSeconds;
   private int gossipedHistoryLimit;
@@ -49,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.blockPeriodMilliseconds = bftConfigOptions.getBlockPeriodMilliseconds();
     this.requestTimeoutSeconds = bftConfigOptions.getRequestTimeoutSeconds();
     this.gossipedHistoryLimit = bftConfigOptions.getGossipedHistoryLimit();
@@ -70,6 +72,11 @@ public int getBlockPeriodSeconds() {
     return blockPeriodSeconds;
   }
 
+  @Override
+  public int getEmptyBlockPeriodSeconds() {
+    return emptyBlockPeriodSeconds;
+  }
+
   @Override
   public long getBlockPeriodMilliseconds() {
     return blockPeriodMilliseconds;
@@ -138,6 +145,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 block period milliseconds. Experimental for test scenarios. Not for use on production
    * systems.