CipherLabs
/
besu
mirror of https://github.com/hyperledger/besu
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix `poa-block-txs-selection-max-time` option that was inadvertently reset to its default after being configured (#6444)

Browse Source 
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
pull/6466/head
Fabio Di Fabio 10 months ago committed by GitHub
parent 6ac419bf0b
commit 16016ece0a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 119 additions and 52 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 1
      CHANGELOG.md
  2. 31
      besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
  3. 60
      besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java
  4. 4
      besu/src/main/java/org/hyperledger/besu/cli/util/TomlConfigurationDefaultProvider.java
  5. 27
      besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java
  6. 17
      besu/src/test/java/org/hyperledger/besu/cli/options/AbstractCLIOptionsTest.java
  7. 27
      besu/src/test/java/org/hyperledger/besu/cli/options/MiningOptionsTest.java
  8. 4
      besu/src/test/java/org/hyperledger/besu/cli/options/SynchronizerOptionsTest.java

1
CHANGELOG.md
Unescape View File

@ -14,6 +14,7 @@
### Bug fixes
- Fix the way an advertised host configured with `--p2p-host` is treated when communicating with the originator of a PING packet [#6225](https://github.com/hyperledger/besu/pull/6225)
- Fix `poa-block-txs-selection-max-time` option that was inadvertently reset to its default after being configured [#6444](https://github.com/hyperledger/besu/pull/6444)
### Download Links

31
besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Unescape View File

@ -124,7 +124,6 @@ import org.hyperledger.besu.ethereum.api.tls.FileBasedPasswordProvider;
import org.hyperledger.besu.ethereum.api.tls.TlsClientAuthConfiguration;
import org.hyperledger.besu.ethereum.api.tls.TlsConfiguration;
import org.hyperledger.besu.ethereum.chain.Blockchain;
import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters;
import org.hyperledger.besu.ethereum.core.MiningParameters;
import org.hyperledger.besu.ethereum.core.PrivacyParameters;
import org.hyperledger.besu.ethereum.eth.sync.SyncMode;
@ -225,6 +224,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalInt;
import java.util.Set;
import java.util.TreeMap;
import java.util.function.Function;
@ -2758,32 +2758,28 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
private MiningParameters getMiningParameters() {
if (miningParameters == null) {
final var miningParametersBuilder =
ImmutableMiningParameters.builder().from(miningOptions.toDomainObject());
final var actualGenesisOptions = getActualGenesisConfigOptions();
if (actualGenesisOptions.isPoa()) {
miningParametersBuilder.genesisBlockPeriodSeconds(
getGenesisBlockPeriodSeconds(actualGenesisOptions));
}
miningParameters = miningParametersBuilder.build();
miningOptions.setGenesisBlockPeriodSeconds(
getGenesisBlockPeriodSeconds(getActualGenesisConfigOptions()));
miningParameters = miningOptions.toDomainObject();
}
return miningParameters;
}
private int getGenesisBlockPeriodSeconds(final GenesisConfigOptions genesisConfigOptions) {
private OptionalInt getGenesisBlockPeriodSeconds(
final GenesisConfigOptions genesisConfigOptions) {
if (genesisConfigOptions.isClique()) {
return genesisConfigOptions.getCliqueConfigOptions().getBlockPeriodSeconds();
return OptionalInt.of(genesisConfigOptions.getCliqueConfigOptions().getBlockPeriodSeconds());
}
if (genesisConfigOptions.isIbft2()) {
return genesisConfigOptions.getBftConfigOptions().getBlockPeriodSeconds();
return OptionalInt.of(genesisConfigOptions.getBftConfigOptions().getBlockPeriodSeconds());
}
if (genesisConfigOptions.isQbft()) {
return genesisConfigOptions.getQbftConfigOptions().getBlockPeriodSeconds();
return OptionalInt.of(genesisConfigOptions.getQbftConfigOptions().getBlockPeriodSeconds());
}
throw new IllegalArgumentException("Should only be called for a PoA network");
return OptionalInt.empty();
}
private boolean isPruningEnabled() {
@ -3246,7 +3242,12 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
return genesisConfigOptions.getEcCurve();
}
private GenesisConfigOptions getActualGenesisConfigOptions() {
/**
* Return the genesis config options after applying any specified config overrides
*
* @return the genesis config options after applying any specified config overrides
*/
protected GenesisConfigOptions getActualGenesisConfigOptions() {
return Optional.ofNullable(genesisConfigOptions)
.orElseGet(
() ->

60
besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java
Unescape View File

@ -40,6 +40,7 @@ import org.hyperledger.besu.ethereum.core.MiningParameters;
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;
@ -188,6 +189,8 @@ public class MiningOptions implements CLIOptions<MiningParameters> {
DEFAULT_POS_BLOCK_CREATION_REPETITION_MIN_DURATION;
}
private OptionalInt maybeGenesisBlockPeriodSeconds;
private MiningOptions() {}
/**
@ -199,6 +202,16 @@ public class MiningOptions implements CLIOptions<MiningParameters> {
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;
}
/**
* Validate that there are no inconsistencies in the specified options. For example that the
* options are valid for the selected implementation.
@ -285,6 +298,7 @@ public class MiningOptions implements CLIOptions<MiningParameters> {
static MiningOptions fromConfig(final MiningParameters miningParameters) {
final MiningOptions miningOptions = MiningOptions.create();
miningOptions.setGenesisBlockPeriodSeconds(miningParameters.getGenesisBlockPeriodSeconds());
miningOptions.isMiningEnabled = miningParameters.isMiningEnabled();
miningOptions.iStratumMiningEnabled = miningParameters.isStratumMiningEnabled();
miningOptions.stratumNetworkInterface = miningParameters.getStratumNetworkInterface();
@ -319,6 +333,11 @@ public class MiningOptions implements CLIOptions<MiningParameters> {
@Override
public MiningParameters toDomainObject() {
if (maybeGenesisBlockPeriodSeconds == null) {
throw new IllegalStateException(
"genesisBlockPeriodSeconds must be set before using this object");
}
final var updatableInitValuesBuilder =
MutableInitValues.builder()
.isMiningEnabled(isMiningEnabled)
@ -334,27 +353,26 @@ public class MiningOptions implements CLIOptions<MiningParameters> {
updatableInitValuesBuilder.coinbase(coinbase);
}
final var miningParametersBuilder =
ImmutableMiningParameters.builder()
.mutableInitValues(updatableInitValuesBuilder.build())
.isStratumMiningEnabled(iStratumMiningEnabled)
.stratumNetworkInterface(stratumNetworkInterface)
.stratumPort(stratumPort)
.nonPoaBlockTxsSelectionMaxTime(nonPoaBlockTxsSelectionMaxTime)
.poaBlockTxsSelectionMaxTime(poaBlockTxsSelectionMaxTime)
.unstable(
ImmutableMiningParameters.Unstable.builder()
.remoteSealersLimit(unstableOptions.remoteSealersLimit)
.remoteSealersTimeToLive(unstableOptions.remoteSealersTimeToLive)
.powJobTimeToLive(unstableOptions.powJobTimeToLive)
.maxOmmerDepth(unstableOptions.maxOmmersDepth)
.stratumExtranonce(unstableOptions.stratumExtranonce)
.posBlockCreationMaxTime(unstableOptions.posBlockCreationMaxTime)
.posBlockCreationRepetitionMinDuration(
unstableOptions.posBlockCreationRepetitionMinDuration)
.build());
return miningParametersBuilder.build();
return ImmutableMiningParameters.builder()
.genesisBlockPeriodSeconds(maybeGenesisBlockPeriodSeconds)
.mutableInitValues(updatableInitValuesBuilder.build())
.isStratumMiningEnabled(iStratumMiningEnabled)
.stratumNetworkInterface(stratumNetworkInterface)
.stratumPort(stratumPort)
.nonPoaBlockTxsSelectionMaxTime(nonPoaBlockTxsSelectionMaxTime)
.poaBlockTxsSelectionMaxTime(poaBlockTxsSelectionMaxTime)
.unstable(
ImmutableMiningParameters.Unstable.builder()
.remoteSealersLimit(unstableOptions.remoteSealersLimit)
.remoteSealersTimeToLive(unstableOptions.remoteSealersTimeToLive)
.powJobTimeToLive(unstableOptions.powJobTimeToLive)
.maxOmmerDepth(unstableOptions.maxOmmersDepth)
.stratumExtranonce(unstableOptions.stratumExtranonce)
.posBlockCreationMaxTime(unstableOptions.posBlockCreationMaxTime)
.posBlockCreationRepetitionMinDuration(
unstableOptions.posBlockCreationRepetitionMinDuration)
.build())
.build();
}
@Override

4
besu/src/main/java/org/hyperledger/besu/cli/util/TomlConfigurationDefaultProvider.java
Unescape View File

@ -17,6 +17,7 @@ package org.hyperledger.besu.cli.util;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.util.number.Fraction;
import org.hyperledger.besu.util.number.Percentage;
import org.hyperledger.besu.util.number.PositiveNumber;
import java.io.File;
import java.io.FileInputStream;
@ -129,7 +130,8 @@ public class TomlConfigurationDefaultProvider implements IDefaultValueProvider {
|| type.equals(Float.class)
|| type.equals(float.class)
|| type.equals(Percentage.class)
|| type.equals(Fraction.class);
|| type.equals(Fraction.class)
|| type.equals(PositiveNumber.class);
}
private String getEntryAsString(final OptionSpec spec) {

27
besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java
Unescape View File

@ -44,6 +44,7 @@ import org.hyperledger.besu.cli.options.unstable.MetricsCLIOptions;
import org.hyperledger.besu.cli.options.unstable.NetworkingOptions;
import org.hyperledger.besu.cli.options.unstable.SynchronizerOptions;
import org.hyperledger.besu.components.BesuComponent;
import org.hyperledger.besu.config.GenesisConfigOptions;
import org.hyperledger.besu.consensus.qbft.pki.PkiBlockCreationConfiguration;
import org.hyperledger.besu.consensus.qbft.pki.PkiBlockCreationConfigurationProvider;
import org.hyperledger.besu.controller.BesuController;
@ -133,20 +134,37 @@ import picocli.CommandLine.RunLast;
@ExtendWith(MockitoExtension.class)
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 JsonObject VALID_GENESIS_QBFT_POST_LONDON =
(new JsonObject())
.put(
"config",
new JsonObject()
.put("londonBlock", 0)
.put("qbft", new JsonObject().put("blockperiodseconds", 5)));
.put(
"qbft",
new JsonObject().put("blockperiodseconds", POA_BLOCK_PERIOD_SECONDS)));
protected static final JsonObject VALID_GENESIS_IBFT2_POST_LONDON =
(new JsonObject())
.put(
"config",
new JsonObject()
.put("londonBlock", 0)
.put("ibft2", new JsonObject().put("blockperiodseconds", 5)));
.put(
"ibft2",
new JsonObject().put("blockperiodseconds", POA_BLOCK_PERIOD_SECONDS)));
protected static final JsonObject VALID_GENESIS_CLIQUE_POST_LONDON =
(new JsonObject())
.put(
"config",
new JsonObject()
.put("londonBlock", 0)
.put(
"clique",
new JsonObject().put("blockperiodseconds", POA_BLOCK_PERIOD_SECONDS)));
protected final PrintStream originalOut = System.out;
protected final PrintStream originalErr = System.err;
protected final ByteArrayOutputStream commandOutput = new ByteArrayOutputStream();
@ -561,6 +579,11 @@ public abstract class CommandTestAbstract {
return vertx;
}
@Override
public GenesisConfigOptions getActualGenesisConfigOptions() {
return super.getActualGenesisConfigOptions();
}
public CommandSpec getSpec() {
return spec;
}

17
besu/src/test/java/org/hyperledger/besu/cli/options/AbstractCLIOptionsTest.java
Unescape View File

@ -67,7 +67,10 @@ public abstract class AbstractCLIOptionsTest<D, T extends CLIOptions<D>>
final TestBesuCommand cmd = parseCommand(cliOptions);
final T optionsFromCommand = getOptionsFromBesuCommand(cmd);
assertThat(optionsFromCommand).usingRecursiveComparison().isEqualTo(options);
assertThat(optionsFromCommand)
.usingRecursiveComparison()
.ignoringFields(getNonOptionFields())
.isEqualTo(options);
assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
@ -82,10 +85,10 @@ public abstract class AbstractCLIOptionsTest<D, T extends CLIOptions<D>>
final T optionsFromCommand = getOptionsFromBesuCommand(cmd);
// Check default values supplied by CLI match expected default values
final String[] fieldsToIgnore = getFieldsWithComputedDefaults().toArray(new String[0]);
assertThat(optionsFromCommand)
.usingRecursiveComparison()
.ignoringFields(fieldsToIgnore)
.ignoringFields(getFieldsWithComputedDefaults())
.ignoringFields(getNonOptionFields())
.isEqualTo(defaultOptions);
}
@ -93,8 +96,12 @@ public abstract class AbstractCLIOptionsTest<D, T extends CLIOptions<D>>
protected abstract D createCustomizedDomainObject();
protected List<String> getFieldsWithComputedDefaults() {
return Collections.emptyList();
protected String[] getFieldsWithComputedDefaults() {
return new String[0];
}
protected String[] getNonOptionFields() {
return new String[0];
}
protected List<String> getFieldsToIgnore() {

27
besu/src/test/java/org/hyperledger/besu/cli/options/MiningOptionsTest.java
Unescape View File

@ -32,7 +32,9 @@ import org.hyperledger.besu.util.number.PositiveNumber;
import java.io.IOException;
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;
@ -361,13 +363,16 @@ public class MiningOptionsTest extends AbstractCLIOptionsTest<MiningParameters,
@Test
public void poaBlockTxsSelectionMaxTimeOptionOver100Percent() throws IOException {
final Path genesisFileIBFT2 = createFakeGenesisFile(VALID_GENESIS_IBFT2_POST_LONDON);
final Path genesisFileClique = createFakeGenesisFile(VALID_GENESIS_CLIQUE_POST_LONDON);
internalTestSuccess(
miningParams ->
assertThat(miningParams.getPoaBlockTxsSelectionMaxTime())
.isEqualTo(PositiveNumber.fromInt(200)),
miningParams -> {
assertThat(miningParams.getPoaBlockTxsSelectionMaxTime())
.isEqualTo(PositiveNumber.fromInt(200));
assertThat(miningParams.getBlockTxsSelectionMaxTime())
.isEqualTo(Duration.ofSeconds(POA_BLOCK_PERIOD_SECONDS * 2).toMillis());
},
"--genesis-file",
genesisFileIBFT2.toString(),
genesisFileClique.toString(),
"--poa-block-txs-selection-max-time",
"200");
}
@ -407,6 +412,16 @@ public class MiningOptionsTest extends AbstractCLIOptionsTest<MiningParameters,
@Override
protected MiningOptions getOptionsFromBesuCommand(final TestBesuCommand besuCommand) {
return besuCommand.getMiningOptions();
final var miningOptions = besuCommand.getMiningOptions();
miningOptions.setGenesisBlockPeriodSeconds(
besuCommand.getActualGenesisConfigOptions().isPoa()
? OptionalInt.of(POA_BLOCK_PERIOD_SECONDS)
: OptionalInt.empty());
return miningOptions;
}
@Override
protected String[] getNonOptionFields() {
return new String[] {"maybeGenesisBlockPeriodSeconds"};
}
}

4
besu/src/test/java/org/hyperledger/besu/cli/options/SynchronizerOptionsTest.java
Unescape View File

@ -82,8 +82,8 @@ public class SynchronizerOptionsTest
}
@Override
protected List<String> getFieldsWithComputedDefaults() {
return Arrays.asList("maxTrailingPeers", "computationParallelism");
protected String[] getFieldsWithComputedDefaults() {
return new String[] {"maxTrailingPeers", "computationParallelism"};
}
@Override

Write Preview
Loading…
Cancel
Save