From 3b887b603b5ac29d7092beeb054d20a3722cd509 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Mon, 21 Aug 2023 17:42:42 +0200 Subject: [PATCH] Fix failing test and better management of pre London fee market in layered txpool Signed-off-by: Fabio Di Fabio --- .../cli/ConfigurationOverviewBuilder.java | 3 +- .../converter/DurationMillisConverter.java | 44 +++++ .../besu/cli/converter/TypeFormatter.java | 19 ++ .../DurationConversionException.java | 39 ++++ .../besu/cli/options/OptionParser.java | 9 + .../stable/TransactionPoolOptions.java | 80 ++------ .../unstable/TransactionPoolOptions.java | 22 +-- .../besu/cli/util/CommandLineUtils.java | 64 ++++++ .../util/TomlConfigFileDefaultProvider.java | 3 + .../hyperledger/besu/cli/BesuCommandTest.java | 186 ------------------ .../cli/options/AbstractCLIOptionsTest.java | 19 ++ .../besu/cli/options/OptionParserTest.java | 7 + .../stable/TransactionPoolOptionsTest.java | 139 ++++++++++--- .../unstable/TransactionPoolOptionsTest.java | 64 +++--- .../controller/BesuControllerBuilderTest.java | 11 +- .../MergeBesuControllerBuilderTest.java | 11 +- .../QbftBesuControllerBuilderTest.java | 11 +- .../src/test/resources/everything_config.toml | 12 +- .../TransactionPoolConfiguration.java | 2 +- .../transactions/TransactionPoolFactory.java | 13 +- .../layered/AbstractTransactionsLayer.java | 1 + .../BaseFeePrioritizedTransactions.java | 24 +-- 22 files changed, 424 insertions(+), 359 deletions(-) create mode 100644 besu/src/main/java/org/hyperledger/besu/cli/converter/DurationMillisConverter.java create mode 100644 besu/src/main/java/org/hyperledger/besu/cli/converter/TypeFormatter.java create mode 100644 besu/src/main/java/org/hyperledger/besu/cli/converter/exception/DurationConversionException.java diff --git a/besu/src/main/java/org/hyperledger/besu/cli/ConfigurationOverviewBuilder.java b/besu/src/main/java/org/hyperledger/besu/cli/ConfigurationOverviewBuilder.java index 8ddeee286dd..5bd1d84a298 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/ConfigurationOverviewBuilder.java +++ b/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( diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/DurationMillisConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/DurationMillisConverter.java new file mode 100644 index 00000000000..2202c99b47e --- /dev/null +++ b/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, TypeFormatter { + + @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()); + } +} diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/TypeFormatter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/TypeFormatter.java new file mode 100644 index 00000000000..b16049ac8b5 --- /dev/null +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/TypeFormatter.java @@ -0,0 +1,19 @@ +/* + * 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; + +public interface TypeFormatter { + String format(V value); +} diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/exception/DurationConversionException.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/exception/DurationConversionException.java new file mode 100644 index 00000000000..01c0a5b7d78 --- /dev/null +++ b/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 '%s' is not allowed", millis)); + } +} diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/OptionParser.java b/besu/src/main/java/org/hyperledger/besu/cli/options/OptionParser.java index 0cab4ce8a8d..003ad8846e8 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/OptionParser.java +++ b/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()); diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptions.java index 0e1791825ec..aa3621ee959 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptions.java +++ b/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 { 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,7 +129,8 @@ 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}, @@ -143,7 +138,7 @@ static class Layered { 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.allOptionsOfClass(Legacy.class)); CommandLineUtils.failIfOptionDoesntMeetRequirement( commandLine, "Could not use layered transaction pool options with legacy implementation", !txPoolImplementation.equals(LEGACY), - allOptionsOfGroup(Layered.class)); - } - - private List 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.allOptionsOfClass(Layered.class)); } @Override @@ -273,39 +258,6 @@ public TransactionPoolConfiguration toDomainObject() { @Override public List getCLIOptions() { - return getCLIOptions(getClass(), this, new TransactionPoolOptions()); - } - - private List getCLIOptions( - final Class startClass, final Object currOptions, final Object defaults) { - List 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(getClass(), this, new TransactionPoolOptions()); } } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/TransactionPoolOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/TransactionPoolOptions.java index f26048f78f4..97bdb054ab1 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/TransactionPoolOptions.java +++ b/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 { 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