diff --git a/CHANGELOG.md b/CHANGELOG.md index 0310e32a0c9..4afd0ccffe8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## 23.4.5 + +### Breaking Changes + +### Additions and Improvements + +### Bug Fixes + - Remove miner-related option warnings if the change isn't using Ethash consensus algorithm [#5669](https://github.com/hyperledger/besu/pull/5669) + +### Download Links + ## 23.4.4 ### Breaking Changes diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index f707a3a5171..9fedca30c04 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -2080,7 +2080,7 @@ private void issueOptionWarnings() { "--remote-connections-max-percentage")); // Check that block producer options work - if (!isMergeEnabled()) { + if (!isMergeEnabled() && getActualGenesisConfigOptions().isEthHash()) { CommandLineUtils.checkOptionDependencies( logger, commandLine, @@ -2091,17 +2091,18 @@ private void issueOptionWarnings() { "--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, diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index 997af020a57..934d871f53f 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -170,6 +170,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", @@ -3702,7 +3716,7 @@ public void stratumMiningIsEnabledWhenSpecified() throws Exception { @Test public void stratumMiningOptionsRequiresServiceToBeEnabled() { - parseCommand("--miner-stratum-enabled"); + parseCommand("--network", "dev", "--miner-stratum-enabled"); verifyOptionsConstraintLoggerCall("--miner-enabled", "--miner-stratum-enabled"); @@ -3716,7 +3730,7 @@ public void stratumMiningOptionsRequiresServiceToBeEnabled() { 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"); @@ -3771,6 +3785,46 @@ public void blockProducingOptionsWarnsMinerShouldBeEnabledToml() throws IOExcept 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() { @@ -3819,6 +3873,33 @@ public void minGasPriceRequiresMainOptionToml() throws IOException { 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"); diff --git a/config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java index d5acee99fb4..921d179993a 100644 --- a/config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java +++ b/config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java @@ -64,6 +64,13 @@ public interface GenesisConfigOptions { */ boolean isClique(); + /** + * Is a Proof of Authority network. + * + * @return the boolean + */ + boolean isPoa(); + /** * Is consensus migration boolean. * diff --git a/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java index 7db62667776..6f7c76c65b5 100644 --- a/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java +++ b/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java @@ -144,6 +144,11 @@ public boolean isQbft() { 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; diff --git a/config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java index 0288392545f..6fad16904cf 100644 --- a/config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java +++ b/config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java @@ -115,6 +115,11 @@ public boolean isQbft() { return false; } + @Override + public boolean isPoa() { + return false; + } + @Override public CheckpointConfigOptions getCheckpointOptions() { return CheckpointConfigOptions.DEFAULT; diff --git a/config/src/test/java/org/hyperledger/besu/config/GenesisConfigOptionsTest.java b/config/src/test/java/org/hyperledger/besu/config/GenesisConfigOptionsTest.java index 9e60a40db80..0cffcf5d247 100644 --- a/config/src/test/java/org/hyperledger/besu/config/GenesisConfigOptionsTest.java +++ b/config/src/test/java/org/hyperledger/besu/config/GenesisConfigOptionsTest.java @@ -46,13 +46,22 @@ public void shouldNotUseEthHashIfEthHashNotPresent() { 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"); } @@ -61,6 +70,7 @@ public void shouldUseCliqueWhenCliqueInConfig() { public void shouldNotUseCliqueIfCliqueNotPresent() { final GenesisConfigOptions config = fromConfigOptions(emptyMap()); assertThat(config.isClique()).isFalse(); + assertThat(config.isPoa()).isFalse(); assertThat(config.getCliqueConfigOptions()).isSameAs(CliqueConfigOptions.DEFAULT); } @@ -228,6 +238,7 @@ public void shouldSupportEmptyGenesisConfig() { final GenesisConfigOptions config = GenesisConfigFile.fromConfig("{}").getConfigOptions(); assertThat(config.isEthHash()).isFalse(); assertThat(config.isClique()).isFalse(); + assertThat(config.isPoa()).isFalse(); assertThat(config.getHomesteadBlockNumber()).isEmpty(); }