Only validate --miner-enabled option for ethash networks (#5669)

* Modify the min-gas-price option validation
* Check for whether ethash is in use, either from genesis or network config, and use that for miner checks
* Add genesis configuration isPoa() convenience function

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matt Whitehead <matthew1001@hotmail.com>
Co-authored-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by:  Simon Dudley <simon.dudley@consensys.net>
This commit is contained in:
Matt Whitehead
2023-07-11 08:59:14 +01:00
committed by GitHub
parent 6ac03af19b
commit 3f2a0160a8
7 changed files with 124 additions and 13 deletions

View File

@@ -14,6 +14,7 @@
- Use the node's configuration to determine if DNS enode URLs are allowed in calls to `admin_addPeer` and `admin_removePeer` [#5584](https://github.com/hyperledger/besu/pull/5584)
- Align the implementation of Eth/68 `NewPooledTransactionHashes` to other clients, using unsigned int for encoding size. [#5640](https://github.com/hyperledger/besu/pull/5640)
- Failure at startup when enabling layered txpool before initial sync done [#5636](https://github.com/hyperledger/besu/issues/5636)
- Remove miner-related option warnings if the change isn't using Ethash consensus algorithm [#5669](https://github.com/hyperledger/besu/pull/5669)
### Download Links

View File

@@ -2133,7 +2133,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
"--remote-connections-max-percentage"));
// Check that block producer options work
if (!isMergeEnabled()) {
if (!isMergeEnabled() && getActualGenesisConfigOptions().isEthHash()) {
CommandLineUtils.checkOptionDependencies(
logger,
commandLine,
@@ -2144,17 +2144,18 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
"--min-gas-price",
"--min-block-occupancy-ratio",
"--miner-extra-data"));
// Check that mining options are able to work
CommandLineUtils.checkOptionDependencies(
logger,
commandLine,
"--miner-enabled",
!minerOptionGroup.isMiningEnabled,
asList(
"--miner-stratum-enabled",
"--Xminer-remote-sealers-limit",
"--Xminer-remote-sealers-hashrate-ttl"));
}
// Check that mining options are able to work
CommandLineUtils.checkOptionDependencies(
logger,
commandLine,
"--miner-enabled",
!minerOptionGroup.isMiningEnabled,
asList(
"--miner-stratum-enabled",
"--Xminer-remote-sealers-limit",
"--Xminer-remote-sealers-hashrate-ttl"));
CommandLineUtils.failIfOptionDoesntMeetRequirement(
commandLine,

View File

@@ -168,6 +168,20 @@ public class BesuCommandTest extends CommandTestAbstract {
(new JsonObject()).put("config", new JsonObject().put("ecCurve", "secp256k1"));
private static final String ENCLAVE_PUBLIC_KEY_PATH =
BesuCommand.class.getResource("/orion_publickey.pub").getPath();
private static final JsonObject VALID_GENESIS_QBFT_POST_LONDON =
(new JsonObject())
.put(
"config",
new JsonObject()
.put("londonBlock", 0)
.put("qbft", new JsonObject().put("blockperiodseconds", 5)));
private static final JsonObject VALID_GENESIS_IBFT2_POST_LONDON =
(new JsonObject())
.put(
"config",
new JsonObject()
.put("londonBlock", 0)
.put("ibft2", new JsonObject().put("blockperiodseconds", 5)));
private static final String[] VALID_ENODE_STRINGS = {
"enode://" + VALID_NODE_ID + "@192.168.0.1:4567",
@@ -3684,7 +3698,7 @@ public class BesuCommandTest extends CommandTestAbstract {
@Test
public void stratumMiningOptionsRequiresServiceToBeEnabled() {
parseCommand("--miner-stratum-enabled");
parseCommand("--network", "dev", "--miner-stratum-enabled");
verifyOptionsConstraintLoggerCall("--miner-enabled", "--miner-stratum-enabled");
@@ -3698,7 +3712,7 @@ public class BesuCommandTest extends CommandTestAbstract {
public void stratumMiningOptionsRequiresServiceToBeEnabledToml() throws IOException {
final Path toml = createTempFile("toml", "miner-stratum-enabled=true\n");
parseCommand("--config-file", toml.toString());
parseCommand("--network", "dev", "--config-file", toml.toString());
verifyOptionsConstraintLoggerCall("--miner-enabled", "--miner-stratum-enabled");
@@ -3753,6 +3767,46 @@ public class BesuCommandTest extends CommandTestAbstract {
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}
@Test
public void blockProducingOptionsDoNotWarnWhenPoA() throws IOException {
final Path genesisFileQBFT = createFakeGenesisFile(VALID_GENESIS_QBFT_POST_LONDON);
parseCommand(
"--genesis-file",
genesisFileQBFT.toString(),
"--min-gas-price",
"42",
"--miner-extra-data",
"0x1122334455667788990011223344556677889900112233445566778899001122");
verify(mockLogger, atMost(0))
.warn(
stringArgumentCaptor.capture(),
stringArgumentCaptor.capture(),
stringArgumentCaptor.capture());
assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
final Path genesisFileIBFT2 = createFakeGenesisFile(VALID_GENESIS_IBFT2_POST_LONDON);
parseCommand(
"--genesis-file",
genesisFileIBFT2.toString(),
"--min-gas-price",
"42",
"--miner-extra-data",
"0x1122334455667788990011223344556677889900112233445566778899001122");
verify(mockLogger, atMost(0))
.warn(
stringArgumentCaptor.capture(),
stringArgumentCaptor.capture(),
stringArgumentCaptor.capture());
assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}
@Test
public void blockProducingOptionsDoNotWarnWhenMergeEnabled() {
@@ -3801,6 +3855,33 @@ public class BesuCommandTest extends CommandTestAbstract {
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}
@Test
public void minGasPriceDoesNotRequireMainOptionWhenPoA() throws IOException {
final Path genesisFileQBFT = createFakeGenesisFile(VALID_GENESIS_QBFT_POST_LONDON);
parseCommand("--genesis-file", genesisFileQBFT.toString(), "--min-gas-price", "0");
verify(mockLogger, atMost(0))
.warn(
stringArgumentCaptor.capture(),
stringArgumentCaptor.capture(),
stringArgumentCaptor.capture());
assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
final Path genesisFileIBFT2 = createFakeGenesisFile(VALID_GENESIS_IBFT2_POST_LONDON);
parseCommand("--genesis-file", genesisFileIBFT2.toString(), "--min-gas-price", "0");
verify(mockLogger, atMost(0))
.warn(
stringArgumentCaptor.capture(),
stringArgumentCaptor.capture(),
stringArgumentCaptor.capture());
assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}
@Test
public void miningParametersAreCaptured() {
final Address requestedCoinbase = Address.fromHexString("0000011111222223333344444");

View File

@@ -65,6 +65,13 @@ public interface GenesisConfigOptions {
*/
boolean isClique();
/**
* Is a Proof of Authority network.
*
* @return the boolean
*/
boolean isPoa();
/**
* Is consensus migration boolean.
*

View File

@@ -146,6 +146,11 @@ public class JsonGenesisConfigOptions implements GenesisConfigOptions {
return configRoot.has(QBFT_CONFIG_KEY);
}
@Override
public boolean isPoa() {
return isQbft() || isClique() || isIbft2() || isIbftLegacy();
}
@Override
public BftConfigOptions getBftConfigOptions() {
final String fieldKey = isIbft2() ? IBFT2_CONFIG_KEY : QBFT_CONFIG_KEY;

View File

@@ -116,6 +116,11 @@ public class StubGenesisConfigOptions implements GenesisConfigOptions, Cloneable
return false;
}
@Override
public boolean isPoa() {
return false;
}
@Override
public CheckpointConfigOptions getCheckpointOptions() {
return CheckpointConfigOptions.DEFAULT;

View File

@@ -48,13 +48,22 @@ public class GenesisConfigOptionsTest {
public void shouldUseIbft2WhenIbft2InConfig() {
final GenesisConfigOptions config = fromConfigOptions(singletonMap("ibft2", emptyMap()));
assertThat(config.isIbft2()).isTrue();
assertThat(config.isPoa()).isTrue();
assertThat(config.getConsensusEngine()).isEqualTo("ibft2");
}
public void shouldUseQbftWhenQbftInConfig() {
final GenesisConfigOptions config = fromConfigOptions(singletonMap("qbft", emptyMap()));
assertThat(config.isQbft()).isTrue();
assertThat(config.isPoa()).isTrue();
assertThat(config.getConsensusEngine()).isEqualTo("qbft");
}
@Test
public void shouldUseCliqueWhenCliqueInConfig() {
final GenesisConfigOptions config = fromConfigOptions(singletonMap("clique", emptyMap()));
assertThat(config.isClique()).isTrue();
assertThat(config.isPoa()).isTrue();
assertThat(config.getCliqueConfigOptions()).isNotSameAs(CliqueConfigOptions.DEFAULT);
assertThat(config.getConsensusEngine()).isEqualTo("clique");
}
@@ -63,6 +72,7 @@ public class GenesisConfigOptionsTest {
public void shouldNotUseCliqueIfCliqueNotPresent() {
final GenesisConfigOptions config = fromConfigOptions(emptyMap());
assertThat(config.isClique()).isFalse();
assertThat(config.isPoa()).isFalse();
assertThat(config.getCliqueConfigOptions()).isSameAs(CliqueConfigOptions.DEFAULT);
}
@@ -230,6 +240,7 @@ public class GenesisConfigOptionsTest {
final GenesisConfigOptions config = GenesisConfigFile.fromConfig("{}").getConfigOptions();
assertThat(config.isEthHash()).isFalse();
assertThat(config.isClique()).isFalse();
assertThat(config.isPoa()).isFalse();
assertThat(config.getHomesteadBlockNumber()).isEmpty();
}