Make PoS block creation max time a config option (#4519)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

CHANGELOG.md

@@ -11,6 +11,7 @@
 ### Additions and Improvements
 - Improved RLP processing of zero-length string as 0x80 [#4283](https://github.com/hyperledger/besu/pull/4283) [#4388](https://github.com/hyperledger/besu/issues/4388)
 - Increased level of detail in JSON-RPC parameter error log messages [#4510](https://github.com/hyperledger/besu/pull/4510)
+- New unstable configuration options to set the maximum time, in milliseconds, a PoS block creation jobs is allowed to run [#4519](https://github.com/hyperledger/besu/pull/4519)
 
 ### Bug Fixes
 - Corrects emission of blockadded events when rewinding during a re-org. Fix for [#4495](https://github.com/hyperledger/besu/issues/4495)

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

@@ -1759,6 +1759,12 @@ private void validateMiningParams() {
           "Unable to mine with Stratum if mining is disabled. Either disable Stratum mining (remove --miner-stratum-enabled) "
               + "or specify mining is enabled (--miner-enabled)");
     }
+    if (unstableMiningOptions.getPosBlockCreationMaxTime() <= 0
+        || unstableMiningOptions.getPosBlockCreationMaxTime()
+            > MiningParameters.DEFAULT_POS_BLOCK_CREATION_MAX_TIME) {
+      throw new ParameterException(
+          this.commandLine, "--Xpos-block-creation-max-time must be positive and ≤ 12000");
+    }
   }
 
   protected void validateP2PInterface(final String p2pInterface) {
@@ -2093,6 +2099,7 @@ public BesuControllerBuilder getControllerBuilder() {
                 .remoteSealersTimeToLive(unstableMiningOptions.getRemoteSealersTimeToLive())
                 .powJobTimeToLive(unstableMiningOptions.getPowJobTimeToLive())
                 .maxOmmerDepth(unstableMiningOptions.getMaxOmmersDepth())
+                .posBlockCreationMaxTime(unstableMiningOptions.getPosBlockCreationMaxTime())
                 .build())
         .transactionPoolConfiguration(buildTransactionPoolConfiguration())
         .nodeKey(new NodeKey(securityModule()))

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

@@ -15,6 +15,7 @@
 package org.hyperledger.besu.cli.options.unstable;
 
 import static org.hyperledger.besu.ethereum.core.MiningParameters.DEFAULT_MAX_OMMERS_DEPTH;
+import static org.hyperledger.besu.ethereum.core.MiningParameters.DEFAULT_POS_BLOCK_CREATION_MAX_TIME;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.DEFAULT_POW_JOB_TTL;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.DEFAULT_REMOTE_SEALERS_LIMIT;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.DEFAULT_REMOTE_SEALERS_TTL;
@@ -58,6 +59,13 @@ public class MiningOptions {
       description = "Extranonce for Stratum network miners (default: ${DEFAULT-VALUE})")
   private String stratumExtranonce = "080c";
 
+  @CommandLine.Option(
+      hidden = true,
+      names = {"--Xpos-block-creation-max-time"},
+      description =
+          "Specifies the maximum time, in milliseconds, a PoS block creation jobs is allowed to run. Must be positive and ≤ 12000 (default: ${DEFAULT-VALUE} milliseconds)")
+  private final Long posBlockCreationMaxTime = DEFAULT_POS_BLOCK_CREATION_MAX_TIME;
+
   public static MiningOptions create() {
     return new MiningOptions();
   }
@@ -81,4 +89,8 @@ public Long getPowJobTimeToLive() {
   public int getMaxOmmersDepth() {
     return maxOmmersDepth;
   }
+
+  public Long getPosBlockCreationMaxTime() {
+    return posBlockCreationMaxTime;
+  }
 }

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

@@ -36,6 +36,7 @@
 import static org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis.NET;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis.PERM;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis.WEB3;
+import static org.hyperledger.besu.ethereum.core.MiningParameters.DEFAULT_POS_BLOCK_CREATION_MAX_TIME;
 import static org.hyperledger.besu.ethereum.p2p.config.DefaultDiscoveryConfiguration.GOERLI_BOOTSTRAP_NODES;
 import static org.hyperledger.besu.ethereum.p2p.config.DefaultDiscoveryConfiguration.GOERLI_DISCOVERY_URL;
 import static org.hyperledger.besu.ethereum.p2p.config.DefaultDiscoveryConfiguration.MAINNET_BOOTSTRAP_NODES;
@@ -226,7 +227,7 @@ public void callingVersionDisplayBesuInfoVersion() {
 
   // Testing default values
   @Test
-  public void callingBesuCommandWithoutOptionsMustSyncWithDefaultValues() throws Exception {
+  public void callingBesuCommandWithoutOptionsMustSyncWithDefaultValues() {
     parseCommand();
 
     final int maxPeers = 25;
@@ -5327,4 +5328,35 @@ public void logWarnIfFastSyncMinPeersUsedWithFullSync() {
     assertThat(commandErrorOutput.toString(UTF_8))
         .contains("--fast-sync-min-peers can't be used with FULL sync-mode");
   }
+
+  @Test
+  public void posBlockCreationMaxTimeDefaultValue() {
+    parseCommand();
+    assertThat(getPosBlockCreationMaxTimeValue()).isEqualTo(DEFAULT_POS_BLOCK_CREATION_MAX_TIME);
+  }
+
+  @Test
+  public void posBlockCreationMaxTimeOption() {
+    parseCommand("--Xpos-block-creation-max-time", "7000");
+    assertThat(getPosBlockCreationMaxTimeValue()).isEqualTo(7000L);
+  }
+
+  private long getPosBlockCreationMaxTimeValue() {
+    final ArgumentCaptor<MiningParameters> miningArg =
+        ArgumentCaptor.forClass(MiningParameters.class);
+
+    verify(mockControllerBuilder).miningParameters(miningArg.capture());
+    verify(mockControllerBuilder).build();
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+    return miningArg.getValue().getPosBlockCreationMaxTime();
+  }
+
+  @Test
+  public void posBlockCreationMaxTimeOutOfAllowedRange() {
+    parseCommand("--Xpos-block-creation-max-time", "17000");
+    assertThat(commandErrorOutput.toString(UTF_8))
+        .contains("--Xpos-block-creation-max-time must be positive and ≤ 12000");
+  }
 }

besu/src/test/resources/everything_config.toml

@@ -136,6 +136,8 @@ miner-stratum-host="0.0.0.0"
 miner-stratum-port=8008
 Xminer-remote-sealers-limit=1000
 Xminer-remote-sealers-hashrate-ttl=10
+Xpos-block-creation-max-time=5
+
 # Pruning
 pruning-enabled=true
 pruning-blocks-retained=1024

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java

@@ -214,7 +214,7 @@ private void tryToBuildBetterBlock(
       final PayloadIdentifier payloadIdentifier,
       final MergeBlockCreator mergeBlockCreator) {
 
-    long remainingTime = miningParameters.getPosBlockCreationTimeout();
+    long remainingTime = miningParameters.getPosBlockCreationMaxTime();
     final Supplier<Block> blockCreator =
         () -> mergeBlockCreator.createBlock(Optional.empty(), random, timestamp);
     // start working on a full block and update the payload value and candidate when it's ready

consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java

@@ -215,7 +215,7 @@ public void shouldRetryBlockCreationIfStillHaveTime() {
   @Test
   public void shouldStopRetryBlockCreationIfTimeExpired() {
     when(mergeContext.getFinalized()).thenReturn(Optional.empty());
-    doReturn(1L).when(miningParameters).getPosBlockCreationTimeout();
+    doReturn(1L).when(miningParameters).getPosBlockCreationMaxTime();
     reset(ethContext, ethScheduler);
     when(ethContext.getScheduler()).thenReturn(ethScheduler);
     when(ethScheduler.scheduleComputationTask(any()))

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/MiningParameters.java

@@ -34,7 +34,7 @@ public class MiningParameters {
 
   public static final int DEFAULT_MAX_OMMERS_DEPTH = 8;
 
-  public static final long DEFAULT_POS_BLOCK_CREATION_TIMEOUT_MS = Duration.ofSeconds(7).toMillis();
+  public static final long DEFAULT_POS_BLOCK_CREATION_MAX_TIME = Duration.ofSeconds(12).toMillis();
 
   private final Optional<Address> coinbase;
   private final Optional<AtomicLong> targetGasLimit;
@@ -51,7 +51,7 @@ public class MiningParameters {
   private final long remoteSealersTimeToLive;
   private final long powJobTimeToLive;
   private final int maxOmmerDepth;
-  private final long posBlockCreationTimeout;
+  private final long posBlockCreationMaxTime;
 
   private MiningParameters(
       final Address coinbase,
@@ -69,7 +69,7 @@ private MiningParameters(
       final long remoteSealersTimeToLive,
       final long powJobTimeToLive,
       final int maxOmmerDepth,
-      final long posBlockCreationTimeout) {
+      final long posBlockCreationMaxTime) {
     this.coinbase = Optional.ofNullable(coinbase);
     this.targetGasLimit = Optional.ofNullable(targetGasLimit).map(AtomicLong::new);
     this.minTransactionGasPrice = minTransactionGasPrice;
@@ -85,7 +85,7 @@ private MiningParameters(
     this.remoteSealersTimeToLive = remoteSealersTimeToLive;
     this.powJobTimeToLive = powJobTimeToLive;
     this.maxOmmerDepth = maxOmmerDepth;
-    this.posBlockCreationTimeout = posBlockCreationTimeout;
+    this.posBlockCreationMaxTime = posBlockCreationMaxTime;
   }
 
   public Optional<Address> getCoinbase() {
@@ -148,8 +148,8 @@ public int getMaxOmmerDepth() {
     return maxOmmerDepth;
   }
 
-  public long getPosBlockCreationTimeout() {
-    return posBlockCreationTimeout;
+  public long getPosBlockCreationMaxTime() {
+    return posBlockCreationMaxTime;
   }
 
   @Override
@@ -170,7 +170,7 @@ public boolean equals(final Object o) {
         && remoteSealersTimeToLive == that.remoteSealersTimeToLive
         && remoteSealersLimit == that.remoteSealersLimit
         && powJobTimeToLive == that.powJobTimeToLive
-        && posBlockCreationTimeout == that.posBlockCreationTimeout;
+        && posBlockCreationMaxTime == that.posBlockCreationMaxTime;
   }
 
   @Override
@@ -189,7 +189,7 @@ public int hashCode() {
         remoteSealersLimit,
         remoteSealersTimeToLive,
         powJobTimeToLive,
-        posBlockCreationTimeout);
+        posBlockCreationMaxTime);
   }
 
   @Override
@@ -225,8 +225,8 @@ public String toString() {
         + remoteSealersTimeToLive
         + ", powJobTimeToLive="
         + powJobTimeToLive
-        + ", posBlockCreationTimeout="
-        + posBlockCreationTimeout
+        + ", posBlockCreationMaxTime="
+        + posBlockCreationMaxTime
         + '}';
   }
 
@@ -247,8 +247,7 @@ public static class Builder {
     private long remoteSealersTimeToLive = DEFAULT_REMOTE_SEALERS_TTL;
     private long powJobTimeToLive = DEFAULT_POW_JOB_TTL;
     private int maxOmmerDepth = DEFAULT_MAX_OMMERS_DEPTH;
-
-    private long posBlockCreationTimeout = DEFAULT_POS_BLOCK_CREATION_TIMEOUT_MS;
+    private long posBlockCreationMaxTime = DEFAULT_POS_BLOCK_CREATION_MAX_TIME;
 
     public Builder() {
       // zero arg
@@ -273,7 +272,7 @@ public Builder(final MiningParameters existing) {
       this.remoteSealersTimeToLive = existing.getRemoteSealersTimeToLive();
       this.powJobTimeToLive = existing.getPowJobTimeToLive();
       this.maxOmmerDepth = existing.getMaxOmmerDepth();
-      this.posBlockCreationTimeout = existing.getPosBlockCreationTimeout();
+      this.posBlockCreationMaxTime = existing.getPosBlockCreationMaxTime();
     }
 
     public Builder coinbase(final Address address) {
@@ -351,8 +350,8 @@ public Builder maxOmmerDepth(final int maxOmmerDepth) {
       return this;
     }
 
-    public Builder posBlockCreationTimeout(final long posBlockCreationTimeoutMillis) {
-      this.posBlockCreationTimeout = posBlockCreationTimeoutMillis;
+    public Builder posBlockCreationMaxTime(final long posBlockCreationMaxTime) {
+      this.posBlockCreationMaxTime = posBlockCreationMaxTime;
       return this;
     }
 
@@ -373,7 +372,7 @@ public MiningParameters build() {
           remoteSealersTimeToLive,
           powJobTimeToLive,
           maxOmmerDepth,
-          posBlockCreationTimeout);
+          posBlockCreationMaxTime);
     }
   }
 }