Deprecate '--custom-network' and merge it's functionality to '--network' (#3567)

* Deprecate '--custom-network' and merge it's functionality to '--network'

We cannot remove `--custom-network` yet, there are still two users
depends on this: hive tests, and kurtosis interop test.

We an remove/disable `--custom-network` after both of hive tests and kurtosis interop
test accept the PR to fix them.

Now, the `--custom-network` only deprecated and not printed on CLI
anymore.

* Remove --custom-network from nrpc and merge it's functionality to --network

Author: jangko

execution_chain/common/chain_config.nim

@@ -664,10 +664,16 @@ func genesisBlockForNetwork*(id: NetworkId): Genesis
   else:
     Genesis()
 
-func networkParams*(id: NetworkId): NetworkParams
-    {.gcsafe, raises: [ValueError, RlpError].} =
-  result.genesis = genesisBlockForNetwork(id)
-  result.config  = chainConfigForNetwork(id)
+func networkParams*(id: NetworkId): NetworkParams =
+  try:
+    NetworkParams(
+      genesis: genesisBlockForNetwork(id),
+      config : chainConfigForNetwork(id)
+    )
+  except ValueError as exc:
+    raiseAssert exc.msg
+  except RlpError as exc:
+    raiseAssert exc.msg
 
 func `==`*(a, b: Genesis): bool =
   if a.isNil and b.isNil: return true

execution_chain/config.nim

@@ -21,7 +21,8 @@ import
     chronicles,
     confutils,
     confutils/defs,
-    confutils/std/net
+    confutils/std/net,
+    results
   ],
   eth/[common, net/nat],
   ./networking/[bootnodes, eth1_enr as enr],
@@ -163,13 +164,16 @@ type
         "- sepolia/11155111: Test network (proof-of-work)\n" &
         "- holesky/17000   : The holesovice post-merge testnet\n" &
         "- hoodi/560048    : The second long-standing, merged-from-genesis, public Ethereum testnet\n" &
-        "- other           : Custom"
-      defaultValue: "" # the default value is set in makeConfig
+        "- path            : /path/to/genesis-or-network-configuration.json\n" &
+        "Both --network: name/path --network:id can be set at the same time to override network id number"
+      defaultValue: @[] # the default value is set in makeConfig
       defaultValueDesc: "mainnet(1)"
       abbr: "i"
-      name: "network" }: string
+      name: "network" }: seq[string]
 
+    # TODO: disable --custom-network if both hive and kurtosis not using this anymore.
     customNetwork {.
+      hidden
       desc: "Use custom genesis block for private Ethereum Network (as /path/to/genesis.json)"
       defaultValueDesc: ""
       abbr: "c"
@@ -669,22 +673,86 @@ proc loadBootstrapFile(fileName: string, output: var seq[ENode]) =
 proc loadStaticPeersFile(fileName: string, output: var seq[ENode]) =
   fileName.loadEnodeFile(output, "static peers")
 
-proc getNetworkId(conf: NimbusConf): Option[NetworkId] =
-  if conf.network.len == 0:
-    return none NetworkId
+func decOrHex(s: string): bool =
+  const allowedDigits = Digits + HexDigits + {'x', 'X'}
+  for c in s:
+    if c notin allowedDigits:
+      return false
+  true
 
-  let network = toLowerAscii(conf.network)
-  case network
-  of "mainnet": return some MainNet
-  of "sepolia": return some SepoliaNet
-  of "holesky": return some HoleskyNet
-  of "hoodi": return some HoodiNet
+proc parseNetworkId(network: string): NetworkId =
+  try:
+    return parseHexOrDec256(network)
+  except CatchableError:
+    error "Failed to parse network id", id=network
+    quit QuitFailure
+
+proc parseNetworkParams(network: string): (NetworkParams, bool) =
+  case toLowerAscii(network)
+  of "mainnet": (networkParams(MainNet), false)
+  of "sepolia": (networkParams(SepoliaNet), false)
+  of "holesky": (networkParams(HoleskyNet), false)
+  of "hoodi"  : (networkParams(HoodiNet), false)
   else:
-    try:
-      some parseHexOrDec256(network)
-    except CatchableError:
-      error "Failed to parse network name or id", network
+    var params: NetworkParams
+    if not loadNetworkParams(network, params):
+      # `loadNetworkParams` have it's own error log
       quit QuitFailure
+    (params, true)
+
+proc processNetworkParamsAndNetworkId(conf: var NimbusConf) =
+  if conf.network.len == 0 and conf.customNetwork.isNone:
+    # Default value if none is set
+    conf.networkId = MainNet
+    conf.networkParams = networkParams(MainNet)
+    return
+
+  var
+    params: Opt[NetworkParams]
+    id: Opt[NetworkId]
+    simulatedCustomNetwork = false
+
+  for network in conf.network:
+    if decOrHex(network):
+      if id.isSome:
+        warn "Network ID already set, ignore new value", id=network
+        continue
+      id = Opt.some parseNetworkId(network)
+    else:
+      if params.isSome:
+        warn "Network configuration already set, ignore new value", network
+        continue
+      let (parsedParams, custom) = parseNetworkParams(network)
+      params = Opt.some parsedParams
+      # Simulate --custom-network while it is still not disabled.
+      if custom:
+        conf.customNetwork = some parsedParams
+        simulatedCustomNetwork = true
+
+  if conf.customNetwork.isSome:
+    if params.isNone:
+      warn "`--custom-network` is deprecated, please use `--network`"
+    elif not simulatedCustomNetwork:
+      warn "Network configuration already set by `--network`, `--custom-network` override it"
+    params = if conf.customNetwork.isSome: Opt.some conf.customNetwork.get
+             else: Opt.none(NetworkParams)
+    if id.isNone:
+      # WARNING: networkId and chainId are two distinct things
+      # their usage should not be mixed in other places.
+      # We only set networkId to chainId if networkId not set in cli and
+      # --custom-network is set.
+      # If chainId is not defined in config file, it's ok because
+      # zero means CustomNet
+      id = Opt.some NetworkId(params.value.config.chainId)
+
+  if id.isNone and params.isSome:
+    id = Opt.some NetworkId(params.value.config.chainId)
+
+  if conf.customNetwork.isNone and params.isNone:
+    params = Opt.some networkParams(id.value)
+
+  conf.networkParams = params.expect("Network params exists")
+  conf.networkId = id.expect("Network ID exists")
 
 proc getRpcFlags(api: openArray[string]): set[RpcFlag] =
   if api.len == 0:
@@ -830,27 +898,7 @@ proc makeConfig*(cmdLine = commandLineParams()): NimbusConf
 
   setupLogging(result.logLevel, result.logStdout, none(OutFile))
 
-  var networkId = result.getNetworkId()
-
-  if result.customNetwork.isSome:
-    result.networkParams = result.customNetwork.get()
-    if networkId.isNone:
-      # WARNING: networkId and chainId are two distinct things
-      # they usage should not be mixed in other places.
-      # We only set networkId to chainId if networkId not set in cli and
-      # --custom-network is set.
-      # If chainId is not defined in config file, it's ok because
-      # zero means CustomNet
-      networkId = some(NetworkId(result.networkParams.config.chainId))
-
-  if networkId.isNone:
-    # bootnodes is set via getBootNodes
-    networkId = some MainNet
-
-  result.networkId = networkId.get()
-
-  if result.customNetwork.isNone:
-    result.networkParams = networkParams(result.networkId)
+  processNetworkParamsAndNetworkId(result)
 
   if result.cmd == noCommand:
     if result.udpPort == Port(0):

hive_integration/nodocker/engine/engine_env.nim

@@ -66,7 +66,7 @@ proc makeCom*(conf: NimbusConf): CommonRef =
 
 proc envConfig*(): NimbusConf =
   makeConfig(@[
-    "--custom-network:" & genesisFile,
+    "--network:" & genesisFile,
     "--listen-address: 127.0.0.1",
   ])
 

hive_integration/nodocker/rpc/test_env.nim

@@ -67,7 +67,7 @@ proc setupEnv*(taskPool: Taskpool): TestEnv =
     # "--nat:extip:0.0.0.0",
     "--network:7",
     "--import-key:" & initPath / "private-key",
-    "--custom-network:" & initPath / "genesis.json",
+    "--network:" & initPath / "genesis.json",
     "--rpc",
     "--rpc-api:eth,debug",
     # "--http-address:0.0.0.0",

nrpc/config.nim

@@ -60,28 +60,22 @@ type
       defaultValue: ""
       name: "beacon-api" .}: string
 
-    network {.
+    network* {.
       desc: "Name or id number of Ethereum network"
       longDesc:
         "- mainnet/1       : Ethereum main network\n" &
         "- sepolia/11155111: Test network (proof-of-work)\n" &
         "- holesky/17000   : The holesovice post-merge testnet\n" &
         "- hoodi/560048    : The second long-standing, merged-from-genesis, public Ethereum testnet\n" &
-        "- other           : Custom"
+        "- path            : Custom config for private Ethereum Network (as /path/to/metadata)\n" &
+        "                    Path to a folder containing custom network configuration files\n" &
+        "                    such as genesis.json, config.yaml, etc.\n" &
+        "                    config.yaml is the configuration file for the CL client"
       defaultValue: "" # the default value is set in makeConfig
       defaultValueDesc: "mainnet(1)"
       abbr: "i"
       name: "network" }: string
 
-    customNetworkFolder* {.
-        desc: "Use custom config for private Ethereum Network (as /path/to/metadata)"
-        longDesc:
-          "Path to a folder containing custom network configuration files\n" &
-          "such as genesis.json, config.yaml, etc.\n" &
-          "config.yaml is the configuration file for the CL client"
-        defaultValue: ""
-        name: "custom-network" .}: string
-
     networkId* {.
       ignore # this field is not processed by confutils
       defaultValue: MainNet # the defaultValue value is set by `makeConfig`
@@ -153,22 +147,33 @@ proc parseCmdArg(T: type NetworkParams, p: string): T
 func completeCmdArg(T: type NetworkParams, val: string): seq[string] =
   return @[]
 
+func decOrHex(s: string): bool =
+  const allowedDigits = Digits + HexDigits + {'x', 'X'}
+  for c in s:
+    if c notin allowedDigits:
+      return false
+  true
+
+proc parseNetworkId(network: string): Opt[NetworkId] =
+  try:
+    Opt.some parseHexOrDec256(network)
+  except CatchableError:
+    error "Failed to parse network id", id=network
+    Opt.none NetworkId
 
 proc getNetworkId(conf: NRpcConf): Opt[NetworkId] =
   if conf.network.len == 0:
-    return Opt.none NetworkId
+    return Opt.some MainNet
 
   let network = toLowerAscii(conf.network)
   case network
   of "mainnet": return Opt.some MainNet
   of "sepolia": return Opt.some SepoliaNet
   of "holesky": return Opt.some HoleskyNet
+  of "hoodi"  : return Opt.some HoodiNet
   else:
-    try:
-      Opt.some parseHexOrDec256(network)
-    except CatchableError:
-      error "Failed to parse network name or id", network
-      quit QuitFailure
+    if decOrHex(network):
+      return parseNetworkId(network)
 
 # KLUDGE: The `load()` template does currently not work within any exception
 #         annotated environment.
@@ -188,12 +193,15 @@ proc makeConfig*(cmdLine = commandLineParams()): NRpcConf
   except CatchableError as e:
     raise e
 
-  var networkId = result.getNetworkId()
+  var
+    networkId = result.getNetworkId()
+    customNetwork = false
 
-  if result.customNetworkFolder.len > 0:
+  if result.network.len > 0 and networkId.isNone:
+    customNetwork = true
     var networkParams = NetworkParams()
-    if not loadNetworkParams(result.customNetworkFolder.joinPath("genesis.json"), networkParams):
-      error "Failed to load customNetwork", path=result.customNetworkFolder
+    if not loadNetworkParams(result.network.joinPath("genesis.json"), networkParams):
+      error "Failed to load customNetwork", path=result.network
       quit QuitFailure
     result.networkParams = networkParams
     if networkId.isNone:
@@ -205,13 +213,9 @@ proc makeConfig*(cmdLine = commandLineParams()): NRpcConf
       # zero means CustomNet
       networkId = Opt.some(NetworkId(result.networkParams.config.chainId))
 
-  if networkId.isNone:
-    # bootnodes is set via getBootNodes
-    networkId = Opt.some MainNet
-
-  result.networkId = networkId.get()
+  result.networkId = networkId.expect("Network ID exists")
 
-  if result.customNetworkFolder.len == 0:
+  if not customNetwork:
     result.networkParams = networkParams(result.networkId)
 
 

nrpc/nrpc.nim

@@ -94,10 +94,7 @@ template loadNetworkConfig(conf: NRpcConf): (RuntimeConfig, uint64, uint64) =
     (getMetadataForNetwork("hoodi").cfg, 0'u64, 0'u64)
   else:
     notice "Loading custom network, assuming post-merge"
-    if conf.customNetworkFolder.len == 0:
-      error "Custom network file not provided"
-      quit(QuitFailure)
-    let (cfg, unloaded) = readRuntimeConfig(conf.customNetworkFolder.joinPath("config.yaml"))
+    let (cfg, unloaded) = readRuntimeConfig(conf.network.joinPath("config.yaml"))
     debug "Fields unknown", unloaded = unloaded
     (cfg, 0'u64, 0'u64)
 
@@ -210,7 +207,7 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} =
     progressTrackedHead = currentBlockNumber
 
   template estimateProgressForSync() =
-    let 
+    let
       blocks = int(currentBlockNumber - progressTrackedHead)
       curTime = Moment.now()
       diff = curTime - time

tests/networking/p2p_test_helper.nim

@@ -40,7 +40,7 @@ proc makeCom(conf: NimbusConf): CommonRef =
 
 proc envConfig(): NimbusConf =
   makeConfig(@[
-    "--custom-network:" & genesisFile,
+    "--network:" & genesisFile,
     "--listen-address: 127.0.0.1",
   ])
 

tests/test_configuration.nim

@@ -47,36 +47,36 @@ proc configurationMain*() =
       check bb.cmd == NimbusCmd.`import-rlp`
       check bb.blocksFile[0].string == genesisFile
 
-    test "custom-network loading config file with no genesis data":
+    test "network loading config file with no genesis data":
       # no genesis will fallback to geth compatibility mode
-      let conf = makeConfig(@["--custom-network:" & noGenesis])
+      let conf = makeConfig(@["--network:" & noGenesis])
       check conf.networkParams.genesis.isNil.not
 
-    test "custom-network loading config file with no 'config'":
+    test "network loading config file with no 'config'":
       # no config will result in empty config, CommonRef keep working
-      let conf = makeConfig(@["--custom-network:" & noConfig])
+      let conf = makeConfig(@["--network:" & noConfig])
       check conf.networkParams.config.isNil == false
 
     test "network-id":
       let aa = makeTestConfig()
       check aa.networkId == MainNet
       check aa.networkParams != NetworkParams()
 
-      let conf = makeConfig(@["--custom-network:" & genesisFile, "--network:345"])
+      let conf = makeConfig(@["--network:" & genesisFile, "--network:345"])
       check conf.networkId == 345.u256
 
-    test "network-id first, custom-network next":
-      let conf = makeConfig(@["--network:678", "--custom-network:" & genesisFile])
+    test "network-id first, network next":
+      let conf = makeConfig(@["--network:678", "--network:" & genesisFile])
       check conf.networkId == 678.u256
 
-    test "network-id set, no custom-network":
+    test "network-id set, no network":
       let conf = makeConfig(@["--network:678"])
       check conf.networkId == 678.u256
       check conf.networkParams.genesis == Genesis()
       check conf.networkParams.config == ChainConfig()
 
     test "network-id not set, copy from chainId of custom network":
-      let conf = makeConfig(@["--custom-network:" & genesisFile])
+      let conf = makeConfig(@["--network:" & genesisFile])
       check conf.networkId == 123.u256
 
     test "network-id not set, sepolia set":
@@ -165,11 +165,11 @@ proc configurationMain*() =
       let cc = makeConfig(@["--static-peers:" & bootNode, "--static-peers:" & bootNode])
       check cc.getStaticPeers().len == 2
 
-    test "chainId of custom-network is oneof std network":
+    test "chainId of network is oneof std network":
       const
         chainid1 = "tests" / "customgenesis" / "chainid1.json"
 
-      let conf = makeConfig(@["--custom-network:" & chainid1])
+      let conf = makeConfig(@["--network:" & chainid1])
       check conf.networkId == 1.u256
       check conf.networkParams.config.londonBlock.get() == 1337
       check conf.getBootNodes().len == 0