Feature/remove experimental merge flag (#3875)

* deprecate Xmerge-support and engine-rpc-enabled cli params Signed-off-by: garyschulte <garyschulte@gmail.com>
hyperledger · May 20, 2022 · 20daaf4 · 20daaf4
1 parent 81c404b
commit 20daaf4
Show file tree

Hide file tree

Showing 18 changed files with 81 additions and 130 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### Bug Fixes
 - Stop backward sync if genesis block has been reached [#3869](https://github.com/hyperledger/besu/pull/3869)
+- Deprecate experimental merge flag and engine-rpc-enabled flag [#3875](https://github.com/hyperledger/besu/pull/3875)
 
 ## 22.4.1
 

diff --git a/...l/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java b/...l/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java
@@ -191,10 +191,6 @@ public void startNode(final BesuNode node) {
     }
 
     if (node.isEngineRpcEnabled()) {
-      params.add("--Xmerge-support");
-      params.add("true");
-
-      params.add("--engine-rpc-enabled");
       params.add("--engine-rpc-port");
       params.add(node.jsonEngineListenPort().get().toString());
     }

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -23,7 +23,6 @@
 import static org.hyperledger.besu.cli.util.CommandLineUtils.DEPENDENCY_WARNING_MSG;
 import static org.hyperledger.besu.cli.util.CommandLineUtils.DEPRECATED_AND_USELESS_WARNING_MSG;
 import static org.hyperledger.besu.cli.util.CommandLineUtils.DEPRECATION_WARNING_MSG;
-import static org.hyperledger.besu.config.experimental.MergeConfigOptions.isMergeEnabled;
 import static org.hyperledger.besu.controller.BesuController.DATABASE_PATH;
 import static org.hyperledger.besu.ethereum.api.graphql.GraphQLConfiguration.DEFAULT_GRAPHQL_HTTP_PORT;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration.DEFAULT_ENGINE_JSON_RPC_PORT;
@@ -61,7 +60,6 @@
 import org.hyperledger.besu.cli.options.unstable.EvmOptions;
 import org.hyperledger.besu.cli.options.unstable.IpcOptions;
 import org.hyperledger.besu.cli.options.unstable.LauncherOptions;
-import org.hyperledger.besu.cli.options.unstable.MergeOptions;
 import org.hyperledger.besu.cli.options.unstable.MetricsCLIOptions;
 import org.hyperledger.besu.cli.options.unstable.MiningOptions;
 import org.hyperledger.besu.cli.options.unstable.NatOptions;
@@ -88,6 +86,7 @@
 import org.hyperledger.besu.config.GenesisConfigFile;
 import org.hyperledger.besu.config.GenesisConfigOptions;
 import org.hyperledger.besu.config.GoQuorumOptions;
+import org.hyperledger.besu.config.MergeConfigOptions;
 import org.hyperledger.besu.consensus.qbft.pki.PkiBlockCreationConfiguration;
 import org.hyperledger.besu.consensus.qbft.pki.PkiBlockCreationConfigurationProvider;
 import org.hyperledger.besu.controller.BesuController;
@@ -282,7 +281,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   private final NatOptions unstableNatOptions = NatOptions.create();
   private final NativeLibraryOptions unstableNativeLibraryOptions = NativeLibraryOptions.create();
   private final RPCOptions unstableRPCOptions = RPCOptions.create();
-  private final MergeOptions mergeOptions = MergeOptions.create();
   final LauncherOptions unstableLauncherOptions = LauncherOptions.create();
   private final PrivacyPluginOptions unstablePrivacyPluginOptions = PrivacyPluginOptions.create();
   private final EvmOptions unstableEvmOptions = EvmOptions.create();
@@ -569,8 +567,9 @@ static class GraphQlOptionGroup {
   static class EngineRPCOptionGroup {
     @Option(
         names = {"--engine-rpc-enabled"},
-        description = "Set to start the Engine JSON-RPC service (default: ${DEFAULT-VALUE})")
-    private final Boolean isEngineRpcEnabled = false;
+        description = "deprectaed parameter, do not use.",
+        hidden = true)
+    private final Boolean deprecatedIsEngineRpcEnabled = false;
 
     @Option(
         names = {"--engine-rpc-port"},
@@ -1389,15 +1388,18 @@ public void run() {
 
     try {
       configureLogging(true);
-
       // Set the goquorum compatibility mode based on the genesis file
       if (genesisFile != null) {
         genesisConfigOptions = readGenesisConfigOptions();
+
         if (genesisConfigOptions.isQuorum()) {
           enableGoQuorumCompatibilityMode();
         }
       }
 
+      // set merge config on the basis of genesis config
+      setMergeConfigOptions();
+
       instantiateSignatureAlgorithmFactory();
       configureNativeLibs();
       logger.info("Starting Besu version: {}", BesuInfo.nodeName(identityString));
@@ -1501,7 +1503,6 @@ private void handleUnstableOptions() {
             .put("Mining", unstableMiningOptions)
             .put("Native Library", unstableNativeLibraryOptions)
             .put("Launcher", unstableLauncherOptions)
-            .put("Merge", mergeOptions)
             .put("EVM Options", unstableEvmOptions)
             .put("IPC Options", unstableIpcOptions)
             .build();
@@ -1812,7 +1813,7 @@ public void validateRpcOptionsParams() {
   }
 
   private GenesisConfigOptions readGenesisConfigOptions() {
-    final GenesisConfigOptions genesisConfigOptions;
+
     try {
       final GenesisConfigFile genesisConfigFile = GenesisConfigFile.fromConfig(genesisConfig());
       genesisConfigOptions = genesisConfigFile.getConfigOptions(genesisConfigOverrides);
@@ -2108,7 +2109,12 @@ private JsonRpcConfiguration createEngineJsonRpcConfiguration(
       final Integer listenPort, final List<String> allowCallsFrom) {
     JsonRpcConfiguration engineConfig =
         jsonRpcConfiguration(listenPort, Arrays.asList("ENGINE", "ETH"), allowCallsFrom);
-    engineConfig.setEnabled(engineRPCOptionGroup.isEngineRpcEnabled);
+    if (engineRPCOptionGroup.deprecatedIsEngineRpcEnabled) {
+      logger.warn(
+          "--engine-api-enabled parameter has been deprecated and will be removed in a future release.  "
+              + "Merge support is implicitly enabled by the presence of terminalTotalDifficulty in the genesis config.");
+    }
+    engineConfig.setEnabled(isMergeEnabled());
     if (engineRPCOptionGroup.isEngineAuthEnabled) {
       engineConfig.setAuthenticationEnabled(true);
       engineConfig.setAuthenticationAlgorithm(JwtAlgorithm.HS256);
@@ -2933,7 +2939,16 @@ private String genesisConfig() {
       return Resources.toString(genesisFile.toURI().toURL(), UTF_8);
     } catch (final IOException e) {
       throw new ParameterException(
-          this.commandLine, String.format("Unable to load genesis file %s.", genesisFile), e);
+          this.commandLine, String.format("Unable to load genesis URL %s.", genesisFile), e);
+    }
+  }
+
+  private static String genesisConfig(final NetworkName networkName) {
+    try (final InputStream genesisFileInputStream =
+        EthNetworkConfig.class.getResourceAsStream(networkName.getGenesisFile())) {
+      return new String(genesisFileInputStream.readAllBytes(), UTF_8);
+    } catch (IOException | NullPointerException e) {
+      throw new IllegalStateException(e);
     }
   }
 
@@ -3063,10 +3078,7 @@ private List<Integer> getEffectivePorts() {
         effectivePorts,
         jsonRPCWebsocketOptionGroup.rpcWsPort,
         jsonRPCWebsocketOptionGroup.isRpcWsEnabled);
-    addPortIfEnabled(
-        effectivePorts,
-        engineRPCOptionGroup.engineRpcPort,
-        engineRPCOptionGroup.isEngineRpcEnabled);
+    addPortIfEnabled(effectivePorts, engineRPCOptionGroup.engineRpcPort, isMergeEnabled());
     addPortIfEnabled(
         effectivePorts, metricsOptionGroup.metricsPort, metricsOptionGroup.isMetricsEnabled);
     addPortIfEnabled(
@@ -3174,6 +3186,22 @@ protected void enableGoQuorumCompatibilityMode() {
     isGoQuorumCompatibilityMode = true;
   }
 
+  private void setMergeConfigOptions() {
+    MergeConfigOptions.setMergeEnabled(
+        Optional.ofNullable(genesisConfigOptions)
+            .orElseGet(
+                () ->
+                    GenesisConfigFile.fromConfig(
+                            genesisConfig(Optional.ofNullable(network).orElse(MAINNET)))
+                        .getConfigOptions(genesisConfigOverrides))
+            .getTerminalTotalDifficulty()
+            .isPresent());
+  }
+
+  private boolean isMergeEnabled() {
+    return MergeConfigOptions.isMergeEnabled();
+  }
+
   public static List<String> getJDKEnabledCypherSuites() {
     try {
       SSLContext context = SSLContext.getInstance("TLS");

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/MergeOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/MergeOptions.java
@@ -14,35 +14,26 @@
  */
 package org.hyperledger.besu.cli.options.unstable;
 
-import static org.hyperledger.besu.config.experimental.MergeConfigOptions.setMergeEnabled;
-
 import java.util.Stack;
 
 import net.consensys.quorum.mainnet.launcher.options.Options;
+import org.slf4j.LoggerFactory;
 import picocli.CommandLine;
 import picocli.CommandLine.Option;
 
-/** Unstable support for eth1/2 merge */
+/** DEPRECATED in favor of genesis config. */
 public class MergeOptions implements Options {
   // To make it easier for tests to reset the value to default
-  public static final boolean MERGE_ENABLED_DEFAULT_VALUE = false;
+  private static final org.slf4j.Logger LOG = LoggerFactory.getLogger(MergeOptions.class);
 
   @Option(
       hidden = true,
       names = {"--Xmerge-support"},
-      description = "Enable experimental support for eth1/eth2 merge (default: ${DEFAULT-VALUE})",
+      description = "Deprecated config parameter, do not use",
       arity = "1",
       parameterConsumer = MergeConfigConsumer.class)
-  @SuppressWarnings({"FieldCanBeFinal"})
-  private static boolean mergeEnabled = MERGE_ENABLED_DEFAULT_VALUE;
-
-  public static MergeOptions create() {
-    return new MergeOptions();
-  }
-
-  static Boolean isMergeEnabled() {
-    return mergeEnabled;
-  }
+  @SuppressWarnings({"FieldCanBeFinal", "UnusedVariable"})
+  private static boolean deprecatedMergeEnabled = false;
 
   @SuppressWarnings({"JdkObsolete"})
   static class MergeConfigConsumer implements CommandLine.IParameterConsumer {
@@ -51,7 +42,9 @@ public void consumeParameters(
         final Stack<String> args,
         final CommandLine.Model.ArgSpec argSpec,
         final CommandLine.Model.CommandSpec commandSpec) {
-      setMergeEnabled(Boolean.parseBoolean(args.pop()));
+      LOG.warn(
+          "--Xmerge-support parameter has been deprecated and will be removed in a future release.  "
+              + "Merge support is implicitly enabled by the presence of terminalTotalDifficulty in the genesis config.");
     }
   }
 }
diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuController.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuController.java
@@ -19,7 +19,6 @@
 import org.hyperledger.besu.config.GenesisConfigOptions;
 import org.hyperledger.besu.config.PowAlgorithm;
 import org.hyperledger.besu.config.QbftConfigOptions;
-import org.hyperledger.besu.config.experimental.MergeConfigOptions;
 import org.hyperledger.besu.crypto.NodeKey;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod;
@@ -212,8 +211,8 @@ BesuControllerBuilder fromGenesisConfig(
         throw new IllegalArgumentException("Unknown consensus mechanism defined");
       }
 
-      // use merge config if experimental merge flag is enabled:
-      if (MergeConfigOptions.isMergeEnabled()) {
+      // wrap with TransitionBesuControllerBuilder if we have a terminal total difficulty:
+      if (configOptions.getTerminalTotalDifficulty().isPresent()) {
         // TODO this should be changed to vanilla MergeBesuControllerBuilder and the Transition*
         // series of classes removed after we successfully transition to PoS
         // https://github.com/hyperledger/besu/issues/2897

diff --git a/besu/src/test/java/org/hyperledger/besu/RunnerBuilderTest.java b/besu/src/test/java/org/hyperledger/besu/RunnerBuilderTest.java
@@ -25,7 +25,7 @@
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.cli.config.EthNetworkConfig;
-import org.hyperledger.besu.config.experimental.MergeConfigOptions;
+import org.hyperledger.besu.config.MergeConfigOptions;
 import org.hyperledger.besu.consensus.common.bft.BftEventQueue;
 import org.hyperledger.besu.consensus.common.bft.network.PeerConnectionTracker;
 import org.hyperledger.besu.consensus.common.bft.protocol.BftProtocolManager;

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
@@ -55,7 +55,7 @@
 import org.hyperledger.besu.BesuInfo;
 import org.hyperledger.besu.cli.config.EthNetworkConfig;
 import org.hyperledger.besu.config.GenesisConfigFile;
-import org.hyperledger.besu.config.experimental.MergeConfigOptions;
+import org.hyperledger.besu.config.MergeConfigOptions;
 import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.Hash;
@@ -1993,12 +1993,7 @@ public void rpcApiNoAuthMethodsIgnoresDuplicatesAndMustBeUsed() {
   @Test
   public void engineApiAuthOptions() {
     parseCommand(
-        "--rpc-http-enabled",
-        "--Xmerge-support",
-        "true",
-        "--engine-jwt-enabled",
-        "--engine-jwt-secret",
-        "/tmp/fakeKey.hex");
+        "--rpc-http-enabled", "--engine-jwt-enabled", "--engine-jwt-secret", "/tmp/fakeKey.hex");
     verify(mockRunnerBuilder).engineJsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture());
     assertThat(jsonRpcConfigArgumentCaptor.getValue().isAuthenticationEnabled()).isTrue();
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
@@ -3403,9 +3398,11 @@ public void blockProducingOptionsWarnsMinerShouldBeEnabled() {
   public void blockProducingOptionsDoNotWarnWhenMergeEnabled() {
 
     final Address requestedCoinbase = Address.fromHexString("0000011111222223333344444");
+    // TODO: once we have mainnet TTD, we can remove the TTD override parameter here
+    // https://github.com/hyperledger/besu/issues/3874
     parseCommand(
-        "--Xmerge-support",
-        "true",
+        "--override-genesis-config",
+        "terminalTotalDifficulty=1337",
         "--miner-coinbase",
         requestedCoinbase.toString(),
         "--min-gas-price",
@@ -4768,14 +4765,16 @@ public void assertThatCheckPortClashRejectsAsExpected() throws Exception {
   public void assertThatCheckPortClashRejectsAsExpectedForEngineApi() throws Exception {
     // use WS port for HTTP
     final int port = 8545;
+    // TODO: once we have mainnet TTD, we can remove the TTD override parameter here
+    // https://github.com/hyperledger/besu/issues/3874
     parseCommand(
-        "--Xmerge-support",
-        "true",
+        "--override-genesis-config",
+        "terminalTotalDifficulty=1337",
         "--rpc-http-enabled",
-        "--engine-rpc-enabled",
-        "--engine-rpc-port",
+        "--rpc-http-port",
         String.valueOf(port),
-        "--rpc-ws-enabled");
+        "--engine-rpc-port",
+        String.valueOf(port));
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8))
         .contains(

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/options/unstable/MergeConfigOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/options/unstable/MergeConfigOptionsTest.java
diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml
@@ -61,7 +61,6 @@ rpc-http-enabled=false
 rpc-http-host="5.6.7.8"
 rpc-http-port=5678
 engine-rpc-port=5679
-engine-rpc-enabled=true
 rpc-http-max-active-connections=100
 rpc-http-api=["DEBUG","ETH"]
 rpc-http-apis=["DEBUG","ETH"]

diff --git a/...nfig/experimental/MergeConfigOptions.java → ...edger/besu/config/MergeConfigOptions.java b/...nfig/experimental/MergeConfigOptions.java → ...edger/besu/config/MergeConfigOptions.java
@@ -12,11 +12,10 @@
  *
  * SPDX-License-Identifier: Apache-2.0
  */
-package org.hyperledger.besu.config.experimental;
+package org.hyperledger.besu.config;
 
 import java.util.concurrent.atomic.AtomicBoolean;
 
-/** For now there is a static config that is driven by a command line option. */
 public class MergeConfigOptions {
   private static final AtomicBoolean mergeEnabled = new AtomicBoolean(false);
 
@@ -27,10 +26,4 @@ public static void setMergeEnabled(final boolean bool) {
   public static boolean isMergeEnabled() {
     return mergeEnabled.get();
   }
-
-  public static void doIfMergeEnabled(final Runnable mergeDo) {
-    if (isMergeEnabled()) {
-      mergeDo.run();
-    }
-  }
 }