Make block txs selection max time aware of PoA transitions (hyperledg…

…er#6676) Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
kaleido-io · Jun 7, 2024 · aa114da · aa114da
1 parent d26bd6d
commit aa114da
Show file tree

Hide file tree

Showing 15 changed files with 627 additions and 161 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@
 - Transaction call object to accept both `input` and `data` field simultaneously if they are set to equal values [#6702](https://github.com/hyperledger/besu/pull/6702)
 
 ### Bug fixes
+- Make block transaction selection max time aware of PoA transitions [#6676](https://github.com/hyperledger/besu/pull/6676)
 
 ### Download Links
 

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -2149,10 +2149,10 @@ private TransactionPoolConfiguration buildTransactionPoolConfiguration() {
 
   private MiningParameters getMiningParameters() {
     if (miningParameters == null) {
-      miningOptions.setGenesisBlockPeriodSeconds(
-          getGenesisBlockPeriodSeconds(getActualGenesisConfigOptions()));
       miningOptions.setTransactionSelectionService(transactionSelectionServiceImpl);
       miningParameters = miningOptions.toDomainObject();
+      getGenesisBlockPeriodSeconds(getActualGenesisConfigOptions())
+          .ifPresent(miningParameters::setBlockPeriodSeconds);
       initMiningParametersMetrics(miningParameters);
     }
     return miningParameters;

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java
@@ -42,7 +42,6 @@
 import org.hyperledger.besu.util.number.PositiveNumber;
 
 import java.util.List;
-import java.util.OptionalInt;
 
 import org.apache.tuweni.bytes.Bytes;
 import org.slf4j.Logger;
@@ -191,7 +190,6 @@ static class Unstable {
         DEFAULT_POS_BLOCK_CREATION_REPETITION_MIN_DURATION;
   }
 
-  private OptionalInt maybeGenesisBlockPeriodSeconds;
   private TransactionSelectionService transactionSelectionService;
 
   private MiningOptions() {}
@@ -205,16 +203,6 @@ public static MiningOptions create() {
     return new MiningOptions();
   }
 
-  /**
-   * Set the optional genesis block period per seconds
-   *
-   * @param genesisBlockPeriodSeconds if the network is PoA then the block period in seconds
-   *     specified in the genesis file, otherwise empty.
-   */
-  public void setGenesisBlockPeriodSeconds(final OptionalInt genesisBlockPeriodSeconds) {
-    maybeGenesisBlockPeriodSeconds = genesisBlockPeriodSeconds;
-  }
-
   /**
    * Set the transaction selection service
    *
@@ -311,7 +299,6 @@ public void validate(
 
   static MiningOptions fromConfig(final MiningParameters miningParameters) {
     final MiningOptions miningOptions = MiningOptions.create();
-    miningOptions.setGenesisBlockPeriodSeconds(miningParameters.getGenesisBlockPeriodSeconds());
     miningOptions.setTransactionSelectionService(miningParameters.getTransactionSelectionService());
     miningOptions.isMiningEnabled = miningParameters.isMiningEnabled();
     miningOptions.iStratumMiningEnabled = miningParameters.isStratumMiningEnabled();
@@ -347,9 +334,6 @@ static MiningOptions fromConfig(final MiningParameters miningParameters) {
 
   @Override
   public MiningParameters toDomainObject() {
-    checkNotNull(
-        maybeGenesisBlockPeriodSeconds,
-        "genesisBlockPeriodSeconds must be set before using this object");
     checkNotNull(
         transactionSelectionService,
         "transactionSelectionService must be set before using this object");
@@ -370,7 +354,6 @@ public MiningParameters toDomainObject() {
     }
 
     return ImmutableMiningParameters.builder()
-        .genesisBlockPeriodSeconds(maybeGenesisBlockPeriodSeconds)
         .transactionSelectionService(transactionSelectionService)
         .mutableInitValues(updatableInitValuesBuilder.build())
         .isStratumMiningEnabled(iStratumMiningEnabled)

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java
@@ -102,6 +102,18 @@ protected MiningCoordinator createMiningCoordinator(
             miningExecutor,
             syncState,
             new CliqueMiningTracker(localAddress, protocolContext));
+
+    // Update the next block period in seconds according to the transition schedule
+    protocolContext
+        .getBlockchain()
+        .observeBlockAdded(
+            o ->
+                miningParameters.setBlockPeriodSeconds(
+                    forksSchedule
+                        .getFork(o.getBlock().getHeader().getNumber() + 1)
+                        .getValue()
+                        .getBlockPeriodSeconds()));
+
     miningCoordinator.addMinedBlockObserver(ethProtocolManager);
 
     // Clique mining is implicitly enabled.

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java
@@ -234,6 +234,17 @@ protected MiningCoordinator createMiningCoordinator(
             blockchain,
             bftEventQueue);
 
+    // Update the next block period in seconds according to the transition schedule
+    protocolContext
+        .getBlockchain()
+        .observeBlockAdded(
+            o ->
+                miningParameters.setBlockPeriodSeconds(
+                    forksSchedule
+                        .getFork(o.getBlock().getHeader().getNumber() + 1)
+                        .getValue()
+                        .getBlockPeriodSeconds()));
+
     if (syncState.isInitialSyncPhaseDone()) {
       LOG.info("Starting IBFT mining coordinator");
       ibftMiningCoordinator.enable();

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java
@@ -274,6 +274,17 @@ protected MiningCoordinator createMiningCoordinator(
             blockchain,
             bftEventQueue);
 
+    // Update the next block period in seconds according to the transition schedule
+    protocolContext
+        .getBlockchain()
+        .observeBlockAdded(
+            o ->
+                miningParameters.setBlockPeriodSeconds(
+                    qbftForksSchedule
+                        .getFork(o.getBlock().getHeader().getNumber() + 1)
+                        .getValue()
+                        .getBlockPeriodSeconds()));
+
     if (syncState.isInitialSyncPhaseDone()) {
       LOG.info("Starting QBFT mining coordinator");
       miningCoordinator.enable();

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/options/AbstractCLIOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/options/AbstractCLIOptionsTest.java
@@ -22,6 +22,7 @@
 
 import java.util.Collections;
 import java.util.List;
+import java.util.function.BiFunction;
 import java.util.function.Consumer;
 
 import org.junit.jupiter.api.Test;
@@ -113,10 +114,18 @@ protected List<String> getFieldsToIgnore() {
   protected abstract T getOptionsFromBesuCommand(final TestBesuCommand besuCommand);
 
   protected void internalTestSuccess(final Consumer<D> assertion, final String... args) {
+    internalTestSuccess((bc, conf) -> conf, assertion, args);
+  }
+
+  protected void internalTestSuccess(
+      final BiFunction<TestBesuCommand, D, D> runtimeConf,
+      final Consumer<D> assertion,
+      final String... args) {
     final TestBesuCommand cmd = parseCommand(args);
 
     final T options = getOptionsFromBesuCommand(cmd);
-    final D config = options.toDomainObject();
+    final D config = runtimeConf.apply(cmd, options.toDomainObject());
+
     assertion.accept(config);
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/options/MiningOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/options/MiningOptionsTest.java
@@ -34,7 +34,6 @@
 import java.nio.file.Path;
 import java.time.Duration;
 import java.util.Optional;
-import java.util.OptionalInt;
 
 import org.apache.tuweni.bytes.Bytes;
 import org.junit.jupiter.api.Test;
@@ -316,6 +315,7 @@ public void posBlockCreationMaxTimeOutOfAllowedRange() {
   @Test
   public void blockTxsSelectionMaxTimeDefaultValue() {
     internalTestSuccess(
+        this::runtimeConfiguration,
         miningParams ->
             assertThat(miningParams.getNonPoaBlockTxsSelectionMaxTime())
                 .isEqualTo(DEFAULT_NON_POA_BLOCK_TXS_SELECTION_MAX_TIME));
@@ -324,6 +324,7 @@ public void blockTxsSelectionMaxTimeDefaultValue() {
   @Test
   public void blockTxsSelectionMaxTimeOption() {
     internalTestSuccess(
+        this::runtimeConfiguration,
         miningParams -> assertThat(miningParams.getBlockTxsSelectionMaxTime()).isEqualTo(1700L),
         "--block-txs-selection-max-time",
         "1700");
@@ -343,6 +344,7 @@ public void blockTxsSelectionMaxTimeIncompatibleWithPoaNetworks() throws IOExcep
   @Test
   public void poaBlockTxsSelectionMaxTimeDefaultValue() {
     internalTestSuccess(
+        this::runtimeConfiguration,
         miningParams ->
             assertThat(miningParams.getPoaBlockTxsSelectionMaxTime())
                 .isEqualTo(DEFAULT_POA_BLOCK_TXS_SELECTION_MAX_TIME));
@@ -352,6 +354,7 @@ public void poaBlockTxsSelectionMaxTimeDefaultValue() {
   public void poaBlockTxsSelectionMaxTimeOption() throws IOException {
     final Path genesisFileIBFT2 = createFakeGenesisFile(VALID_GENESIS_IBFT2_POST_LONDON);
     internalTestSuccess(
+        this::runtimeConfiguration,
         miningParams ->
             assertThat(miningParams.getPoaBlockTxsSelectionMaxTime())
                 .isEqualTo(PositiveNumber.fromInt(80)),
@@ -365,6 +368,7 @@ public void poaBlockTxsSelectionMaxTimeOption() throws IOException {
   public void poaBlockTxsSelectionMaxTimeOptionOver100Percent() throws IOException {
     final Path genesisFileClique = createFakeGenesisFile(VALID_GENESIS_CLIQUE_POST_LONDON);
     internalTestSuccess(
+        this::runtimeConfiguration,
         miningParams -> {
           assertThat(miningParams.getPoaBlockTxsSelectionMaxTime())
               .isEqualTo(PositiveNumber.fromInt(200));
@@ -412,16 +416,19 @@ protected MiningOptions optionsFromDomainObject(final MiningParameters domainObj
 
   @Override
   protected MiningOptions getOptionsFromBesuCommand(final TestBesuCommand besuCommand) {
-    final var miningOptions = besuCommand.getMiningOptions();
-    miningOptions.setGenesisBlockPeriodSeconds(
-        besuCommand.getActualGenesisConfigOptions().isPoa()
-            ? OptionalInt.of(POA_BLOCK_PERIOD_SECONDS)
-            : OptionalInt.empty());
-    return miningOptions;
+    return besuCommand.getMiningOptions();
   }
 
   @Override
   protected String[] getNonOptionFields() {
-    return new String[] {"maybeGenesisBlockPeriodSeconds", "transactionSelectionService"};
+    return new String[] {"transactionSelectionService"};
+  }
+
+  private MiningParameters runtimeConfiguration(
+      final TestBesuCommand besuCommand, final MiningParameters miningParameters) {
+    if (besuCommand.getActualGenesisConfigOptions().isPoa()) {
+      miningParameters.setBlockPeriodSeconds(POA_BLOCK_PERIOD_SECONDS);
+    }
+    return miningParameters;
   }
 }