Fix failing test and better management of pre London fee market in layered txpool

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

Author: fab-10

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

@@ -168,8 +168,9 @@ public ConfigurationOverviewBuilder setHighSpecEnabled() {
   }
 
   /**
-   * Sets experimental layered txpool enabled.
+   * Sets the txpool implementation in use.
    *
+   * @param implementation the txpool implementation
    * @return the builder
    */
   public ConfigurationOverviewBuilder setTxPoolImplementation(

besu/src/main/java/org/hyperledger/besu/cli/converter/DurationMillisConverter.java

@@ -0,0 +1,44 @@
+/*
+ * Copyright Hyperledger Besu Contributors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.cli.converter;
+
+import org.hyperledger.besu.cli.converter.exception.DurationConversionException;
+
+import java.time.Duration;
+
+import picocli.CommandLine;
+
+/** The Percentage Cli type converter. */
+public class DurationMillisConverter
+    implements CommandLine.ITypeConverter<Duration>, TypeFormatter<Duration> {
+
+  @Override
+  public Duration convert(final String value) throws DurationConversionException {
+    try {
+      final long millis = Long.parseLong(value);
+      if (millis < 0) {
+        throw new DurationConversionException(millis);
+      }
+      return Duration.ofMillis(Long.parseLong(value));
+    } catch (NullPointerException | IllegalArgumentException e) {
+      throw new DurationConversionException(value);
+    }
+  }
+
+  @Override
+  public String format(final Duration value) {
+    return Long.toString(value.toMillis());
+  }
+}

besu/src/main/java/org/hyperledger/besu/cli/converter/TypeFormatter.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright Hyperledger Besu Contributors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.cli.converter;
+
+/**
+ * This interface can be used to give a converter the capability to format the converted value back
+ * to its CLI form
+ *
+ * @param <V> the type of the CLI converted runtime value
+ */
+public interface TypeFormatter<V> {
+  /**
+   * Format a converted value back to its CLI form
+   *
+   * @param value the converted value
+   * @return the textual CLI form of the value
+   */
+  String format(V value);
+}

besu/src/main/java/org/hyperledger/besu/cli/converter/exception/DurationConversionException.java

@@ -0,0 +1,39 @@
+/*
+ * Copyright Hyperledger Besu Contributors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.cli.converter.exception;
+
+import static java.lang.String.format;
+
+/** The custom Duration conversion exception. */
+public final class DurationConversionException extends Exception {
+
+  /**
+   * Instantiates a new Duration conversion exception for malformed value.
+   *
+   * @param value the value
+   */
+  public DurationConversionException(final String value) {
+    super(format("'%s' is not a long", value));
+  }
+
+  /**
+   * Instantiates a new Duration conversion exception for negative value.
+   *
+   * @param millis the value
+   */
+  public DurationConversionException(final long millis) {
+    super(format("negative value '%d' is not allowed", millis));
+  }
+}

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

@@ -112,12 +112,21 @@ public static String format(final Wei value) {
     return format(value.toUInt256());
   }
 
+  /**
+   * Format any object to string. This implementation tries to find an existing format method, in
+   * this class, that matches the type of the passed object, and if not found just invoke, to string
+   * on the passed object
+   *
+   * @param value the object
+   * @return the string
+   */
   public static String format(final Object value) {
     Method formatMethod;
     try {
       formatMethod = OptionParser.class.getMethod("format", value.getClass());
     } catch (NoSuchMethodException e) {
       try {
+        // maybe a primitive version of the method exists
         formatMethod =
             OptionParser.class.getMethod(
                 "format", MethodType.methodType(value.getClass()).unwrap().returnType());

besu/src/main/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptions.java

@@ -23,7 +23,6 @@
 import org.hyperledger.besu.cli.converter.FractionConverter;
 import org.hyperledger.besu.cli.converter.PercentageConverter;
 import org.hyperledger.besu.cli.options.CLIOptions;
-import org.hyperledger.besu.cli.options.OptionParser;
 import org.hyperledger.besu.cli.util.CommandLineUtils;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration;
@@ -32,16 +31,11 @@
 import org.hyperledger.besu.util.number.Percentage;
 
 import java.io.File;
-import java.lang.annotation.Annotation;
-import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
-import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
-import java.util.Objects;
 
 import picocli.CommandLine;
 
+/** The Transaction pool Cli stable options. */
 public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfiguration> {
   private static final String TX_POOL_IMPLEMENTATION = "--tx-pool";
   private static final String TX_POOL_DISABLE_LOCALS = "--tx-pool-disable-locals";
@@ -126,7 +120,7 @@ static class Layered {
         description =
             "Max amount of memory space, in bytes, that any layer within the transaction pool could occupy (default: ${DEFAULT-VALUE})",
         arity = "1")
-    long txPoolLayerMaxCapacity =
+    Long txPoolLayerMaxCapacity =
         TransactionPoolConfiguration.DEFAULT_PENDING_TRANSACTIONS_LAYER_MAX_CAPACITY_BYTES;
 
     @CommandLine.Option(
@@ -135,15 +129,16 @@ static class Layered {
         description =
             "Max number of pending transactions that are prioritized and thus kept sorted (default: ${DEFAULT-VALUE})",
         arity = "1")
-    int txPoolMaxPrioritized = TransactionPoolConfiguration.DEFAULT_MAX_PRIORITIZED_TRANSACTIONS;
+    Integer txPoolMaxPrioritized =
+        TransactionPoolConfiguration.DEFAULT_MAX_PRIORITIZED_TRANSACTIONS;
 
     @CommandLine.Option(
         names = {TX_POOL_MAX_FUTURE_BY_SENDER},
         paramLabel = MANDATORY_INTEGER_FORMAT_HELP,
         description =
             "Max number of future pending transactions allowed for a single sender (default: ${DEFAULT-VALUE})",
         arity = "1")
-    int txPoolMaxFutureBySender = TransactionPoolConfiguration.DEFAULT_MAX_FUTURE_BY_SENDER;
+    Integer txPoolMaxFutureBySender = TransactionPoolConfiguration.DEFAULT_MAX_FUTURE_BY_SENDER;
   }
 
   @CommandLine.ArgGroup(
@@ -163,7 +158,7 @@ static class Legacy {
         description =
             "Maximum retention period of pending transactions in hours (default: ${DEFAULT-VALUE})",
         arity = "1")
-    int pendingTxRetentionPeriod = TransactionPoolConfiguration.DEFAULT_TX_RETENTION_HOURS;
+    Integer pendingTxRetentionPeriod = TransactionPoolConfiguration.DEFAULT_TX_RETENTION_HOURS;
 
     @CommandLine.Option(
         names = {TX_POOL_LIMIT_BY_ACCOUNT_PERCENTAGE},
@@ -181,7 +176,7 @@ static class Legacy {
         description =
             "Maximum number of pending transactions that will be kept in the transaction pool (default: ${DEFAULT-VALUE})",
         arity = "1")
-    int txPoolMaxSize = TransactionPoolConfiguration.DEFAULT_MAX_PENDING_TRANSACTIONS;
+    Integer txPoolMaxSize = TransactionPoolConfiguration.DEFAULT_MAX_PENDING_TRANSACTIONS;
   }
 
   private TransactionPoolOptions() {}
@@ -222,34 +217,24 @@ public static TransactionPoolOptions fromConfig(final TransactionPoolConfigurati
     return options;
   }
 
+  /**
+   * Validate that there are no inconsistencies in the specified options. For example that the
+   * options are valid for the selected implementation.
+   *
+   * @param commandLine the full commandLine to check all the options specified by the user
+   */
   public void validate(final CommandLine commandLine) {
     CommandLineUtils.failIfOptionDoesntMeetRequirement(
         commandLine,
         "Could not use legacy transaction pool options with layered implementation",
         !txPoolImplementation.equals(LAYERED),
-        allOptionsOfGroup(Legacy.class));
+        CommandLineUtils.getCLIOptionNames(Legacy.class));
 
     CommandLineUtils.failIfOptionDoesntMeetRequirement(
         commandLine,
         "Could not use layered transaction pool options with legacy implementation",
         !txPoolImplementation.equals(LEGACY),
-        allOptionsOfGroup(Layered.class));
-  }
-
-  private List<String> allOptionsOfGroup(final Class<?> group) {
-    return Arrays.stream(group.getDeclaredFields())
-        .filter(f -> Modifier.isStatic(f.getModifiers()))
-        .map(
-            f -> {
-              try {
-                return f.get(null);
-              } catch (IllegalAccessException e) {
-                throw new RuntimeException(e);
-              }
-            })
-        .filter(o -> o instanceof String)
-        .map(String.class::cast)
-        .toList();
+        CommandLineUtils.getCLIOptionNames(Layered.class));
   }
 
   @Override
@@ -273,39 +258,6 @@ public TransactionPoolConfiguration toDomainObject() {
 
   @Override
   public List<String> getCLIOptions() {
-    return getCLIOptions(getClass(), this, new TransactionPoolOptions());
-  }
-
-  private List<String> getCLIOptions(
-      final Class<?> startClass, final Object currOptions, final Object defaults) {
-    List<String> cliOpts = new ArrayList<>();
-    Field[] fields = startClass.getDeclaredFields();
-    for (Field field : fields) {
-      Annotation optionAnnotation = field.getAnnotation(CommandLine.Option.class);
-      if (optionAnnotation != null) {
-        try {
-          var optVal = field.get(currOptions);
-          if (!Objects.equals(optVal, field.get(defaults))) {
-            var optAnn = CommandLine.Option.class.cast(optionAnnotation);
-            String optName = optAnn.names()[0];
-            cliOpts.add(optName);
-            cliOpts.add(OptionParser.format(optVal));
-          }
-        } catch (IllegalAccessException e) {
-          throw new RuntimeException(e);
-        }
-      } else {
-        Annotation groupAnnotation = field.getAnnotation(CommandLine.ArgGroup.class);
-        if (groupAnnotation != null) {
-          try {
-            cliOpts.addAll(
-                getCLIOptions(field.getType(), field.get(currOptions), field.get(defaults)));
-          } catch (IllegalAccessException e) {
-            throw new RuntimeException(e);
-          }
-        }
-      }
-    }
-    return cliOpts;
+    return CommandLineUtils.getCLIOptions(this, new TransactionPoolOptions());
   }
 }

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

@@ -14,18 +14,18 @@
  */
 package org.hyperledger.besu.cli.options.unstable;
 
+import org.hyperledger.besu.cli.converter.DurationMillisConverter;
 import org.hyperledger.besu.cli.options.CLIOptions;
-import org.hyperledger.besu.cli.options.OptionParser;
+import org.hyperledger.besu.cli.util.CommandLineUtils;
 import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
 
 import java.time.Duration;
-import java.util.Arrays;
 import java.util.List;
 
 import picocli.CommandLine;
 
-/** The Transaction pool Cli options. */
+/** The Transaction pool Cli unstable options. */
 public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfiguration.Unstable> {
   private static final String TX_MESSAGE_KEEP_ALIVE_SEC_FLAG =
       "--Xincoming-tx-messages-keep-alive-seconds";
@@ -46,12 +46,13 @@ public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfigu
   @CommandLine.Option(
       names = {ETH65_TX_ANNOUNCED_BUFFERING_PERIOD_FLAG},
       paramLabel = "<LONG>",
+      converter = DurationMillisConverter.class,
       hidden = true,
       description =
           "The period for which the announced transactions remain in the buffer before being requested from the peers in milliseconds (default: ${DEFAULT-VALUE})",
       arity = "1")
-  private long eth65TrxAnnouncedBufferingPeriod =
-      TransactionPoolConfiguration.Unstable.ETH65_TRX_ANNOUNCED_BUFFERING_PERIOD.toMillis();
+  private Duration eth65TrxAnnouncedBufferingPeriod =
+      TransactionPoolConfiguration.Unstable.ETH65_TRX_ANNOUNCED_BUFFERING_PERIOD;
 
   private TransactionPoolOptions() {}
 
@@ -74,25 +75,20 @@ public static TransactionPoolOptions fromConfig(
       final TransactionPoolConfiguration.Unstable config) {
     final TransactionPoolOptions options = TransactionPoolOptions.create();
     options.txMessageKeepAliveSeconds = config.getTxMessageKeepAliveSeconds();
-    options.eth65TrxAnnouncedBufferingPeriod =
-        config.getEth65TrxAnnouncedBufferingPeriod().toMillis();
+    options.eth65TrxAnnouncedBufferingPeriod = config.getEth65TrxAnnouncedBufferingPeriod();
     return options;
   }
 
   @Override
   public TransactionPoolConfiguration.Unstable toDomainObject() {
     return ImmutableTransactionPoolConfiguration.Unstable.builder()
         .txMessageKeepAliveSeconds(txMessageKeepAliveSeconds)
-        .eth65TrxAnnouncedBufferingPeriod(Duration.ofMillis(eth65TrxAnnouncedBufferingPeriod))
+        .eth65TrxAnnouncedBufferingPeriod(eth65TrxAnnouncedBufferingPeriod)
         .build();
   }
 
   @Override
   public List<String> getCLIOptions() {
-    return Arrays.asList(
-        TX_MESSAGE_KEEP_ALIVE_SEC_FLAG,
-        OptionParser.format(txMessageKeepAliveSeconds),
-        ETH65_TX_ANNOUNCED_BUFFERING_PERIOD_FLAG,
-        OptionParser.format(eth65TrxAnnouncedBufferingPeriod));
+    return CommandLineUtils.getCLIOptions(this, new TransactionPoolOptions());
   }
 }