From 4b186a4b285779222a33290698fff47616300156 Mon Sep 17 00:00:00 2001
From: fryorcraken <110212804+fryorcraken@users.noreply.github.com>
Date: Thu, 3 Jul 2025 11:56:43 +1000
Subject: [PATCH] fix: deprecate --dns-discovery (#3485)

* fix: deprecate `--dns-discovery`

Properly deprecates `--dns-discovery` CLI arg.
DNS Discovery is enabled if a non-empty DNS Discovery URL is passed.

* test: add test_all for factory

add and use test_all for some tests.
---
 tests/all_tests_waku.nim                          | 15 ++-------------
 tests/factory/test_all.nim                        |  1 +
 tests/wakunode_rest/test_all.nim                  | 11 ++++++-----
 .../conf_builder/dns_discovery_conf_builder.nim   | 12 ++++--------
 waku/factory/external_config.nim                  |  7 ++++---
 5 files changed, 17 insertions(+), 29 deletions(-)
 create mode 100644 tests/factory/test_all.nim

diff --git a/tests/all_tests_waku.nim b/tests/all_tests_waku.nim
index 2723fac8f..98ea0e36f 100644
--- a/tests/all_tests_waku.nim
+++ b/tests/all_tests_waku.nim
@@ -92,20 +92,9 @@ import
 import ./test_waku_keystore_keyfile, ./test_waku_keystore
 
 ## Wakunode Rest API test suite
-import
-  ./wakunode_rest/test_rest_debug,
-  ./wakunode_rest/test_rest_debug_serdes,
-  ./wakunode_rest/test_rest_relay,
-  ./wakunode_rest/test_rest_relay_serdes,
-  ./wakunode_rest/test_rest_serdes,
-  ./wakunode_rest/test_rest_filter,
-  ./wakunode_rest/test_rest_lightpush,
-  ./wakunode_rest/test_rest_lightpush_legacy,
-  ./wakunode_rest/test_rest_admin,
-  ./wakunode_rest/test_rest_cors,
-  ./wakunode_rest/test_rest_health
+import ./wakunode_rest/test_all
 
 import ./waku_rln_relay/test_all
 
 # Node Factory
-import ./factory/[test_external_config, test_node_factory, test_waku_conf]
+import ./factory/test_all
diff --git a/tests/factory/test_all.nim b/tests/factory/test_all.nim
new file mode 100644
index 000000000..b704a8ef3
--- /dev/null
+++ b/tests/factory/test_all.nim
@@ -0,0 +1 @@
+import ./test_external_config, ./test_node_factory, ./test_waku_conf
diff --git a/tests/wakunode_rest/test_all.nim b/tests/wakunode_rest/test_all.nim
index 6e34b6fdd..4071e635b 100644
--- a/tests/wakunode_rest/test_all.nim
+++ b/tests/wakunode_rest/test_all.nim
@@ -1,14 +1,15 @@
 {.used.}
 
 import
-  ./test_rest_debug_serdes,
+  ./test_rest_admin,
+  ./test_rest_cors,
   ./test_rest_debug,
+  ./test_rest_debug_serdes,
   ./test_rest_filter,
-  ./test_rest_lightpush_legacy,
   ./test_rest_health,
+  ./test_rest_lightpush,
+  ./test_rest_lightpush_legacy,
   ./test_rest_relay_serdes,
   ./test_rest_relay,
   ./test_rest_serdes,
-  ./test_rest_store,
-  ./test_rest_admin,
-  ./test_rest_cors
+  ./test_rest_store
diff --git a/waku/factory/conf_builder/dns_discovery_conf_builder.nim b/waku/factory/conf_builder/dns_discovery_conf_builder.nim
index 34337c9b1..1c577bbf8 100644
--- a/waku/factory/conf_builder/dns_discovery_conf_builder.nim
+++ b/waku/factory/conf_builder/dns_discovery_conf_builder.nim
@@ -1,4 +1,4 @@
-import chronicles, std/[net, options, sequtils], results
+import chronicles, std/[net, options, strutils], results
 import ../waku_conf
 
 logScope:
@@ -8,16 +8,12 @@ logScope:
 ## DNS Discovery Config Builder ##
 ##################################
 type DnsDiscoveryConfBuilder* = object
-  enabled*: Option[bool]
   enrTreeUrl*: Option[string]
   nameServers*: seq[IpAddress]
 
 proc init*(T: type DnsDiscoveryConfBuilder): DnsDiscoveryConfBuilder =
   DnsDiscoveryConfBuilder()
 
-proc withEnabled*(b: var DnsDiscoveryConfBuilder, enabled: bool) =
-  b.enabled = some(enabled)
-
 proc withEnrTreeUrl*(b: var DnsDiscoveryConfBuilder, enrTreeUrl: string) =
   b.enrTreeUrl = some(enrTreeUrl)
 
@@ -25,13 +21,13 @@ proc withNameServers*(b: var DnsDiscoveryConfBuilder, nameServers: seq[IpAddress
   b.nameServers = nameServers
 
 proc build*(b: DnsDiscoveryConfBuilder): Result[Option[DnsDiscoveryConf], string] =
-  if not b.enabled.get(false):
+  if b.enrTreeUrl.isNone():
     return ok(none(DnsDiscoveryConf))
 
+  if isEmptyOrWhiteSpace(b.enrTreeUrl.get()):
+    return err("dnsDiscovery.enrTreeUrl cannot be an empty string")
   if b.nameServers.len == 0:
     return err("dnsDiscovery.nameServers is not specified")
-  if b.enrTreeUrl.isNone():
-    return err("dnsDiscovery.enrTreeUrl is not specified")
 
   return ok(
     some(DnsDiscoveryConf(nameServers: b.nameServers, enrTreeUrl: b.enrTreeUrl.get()))
diff --git a/waku/factory/external_config.nim b/waku/factory/external_config.nim
index 4e71783c5..704c6d4e5 100644
--- a/waku/factory/external_config.nim
+++ b/waku/factory/external_config.nim
@@ -550,7 +550,8 @@ with the drawback of consuming some more bandwidth.""",
     .}: bool
 
     dnsDiscoveryUrl* {.
-      desc: "URL for DNS node list in format 'enrtree://<key>@<fqdn>'",
+      desc:
+        "URL for DNS node list in format 'enrtree://<key>@<fqdn>', enables DNS Discovery",
       defaultValue: "",
       name: "dns-discovery-url"
     .}: string
@@ -996,8 +997,8 @@ proc toWakuConf*(n: WakuNodeConf): ConfResult[WakuConf] =
   b.metricsServerConf.withHttpPort(n.metricsServerPort)
   b.metricsServerConf.withLogging(n.metricsLogging)
 
-  b.dnsDiscoveryConf.withEnabled(n.dnsDiscovery)
-  b.dnsDiscoveryConf.withEnrTreeUrl(n.dnsDiscoveryUrl)
+  if n.dnsDiscoveryUrl != "":
+    b.dnsDiscoveryConf.withEnrTreeUrl(n.dnsDiscoveryUrl)
   b.dnsDiscoveryConf.withNameServers(n.dnsAddrsNameServers)
 
   if n.discv5Discovery.isSome():