Fix reconnection to clearnet addresses via Tor

Author: rorp

eclair-core/src/main/scala/fr/acinq/eclair/io/ReconnectionTask.scala

@@ -187,17 +187,14 @@ object ReconnectionTask {
   // @formatter:on
 
   def selectNodeAddress(nodeParams: NodeParams, nodeAddresses: Seq[NodeAddress]): Option[NodeAddress] = {
-    // it doesn't make sense to mix tor and clearnet addresses, so we separate them and decide whether we use one or the other
+    // it makes perfect sense to mix tor and clearnet addresses over tor, but, we separate them when tor is not configured.
     val torAddresses = nodeAddresses.collect { case o: OnionAddress => o }
     val clearnetAddresses = nodeAddresses diff torAddresses
     val selectedAddresses = nodeParams.socksProxy_opt match {
-      case Some(params) if clearnetAddresses.nonEmpty && params.useForTor && (!params.useForIPv4 || !params.useForIPv6) =>
-        // Remote has clearnet (and possibly tor addresses), and we support tor, but we have configured it to only use
-        // tor when strictly necessary. In this case we will only connect over clearnet.
-        clearnetAddresses
-      case Some(params) if torAddresses.nonEmpty && params.useForTor =>
-        // In all other cases, if they have a tor address and we support tor, we use tor.
-        torAddresses
+      case Some(params) if params.useForTor =>
+        // The SOCKS5 proxy is enabled, and specifically configured to handle tor addresses.
+        // This is the only case when we can connect to both tor and clearnet.
+        nodeAddresses
       case _ =>
         // Otherwise, if we don't support tor or they don't have a tor address, we use clearnet.
         clearnetAddresses

eclair-core/src/test/scala/fr/acinq/eclair/io/ReconnectionTaskSpec.scala

@@ -247,18 +247,29 @@ class ReconnectionTaskSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike
       nodeParams.socksProxy_opt returns Some(socksParams)
       assert(ReconnectionTask.selectNodeAddress(nodeParams, List(clearnet)).contains(clearnet))
       assert(ReconnectionTask.selectNodeAddress(nodeParams, List(tor)).contains(tor))
-      assert(ReconnectionTask.selectNodeAddress(nodeParams, List(clearnet, tor)).contains(clearnet))
+      assert(ReconnectionTask.selectNodeAddress(nodeParams, List(clearnet, tor)).exists(Set(clearnet, tor)(_)))
     }
     {
-      // tor supported and enabled for clearnet addresses: return tor addresses when available
+      // tor supported and enabled for clearnet addresses: return both tor and clearnet addresses when available
       val socksParams = mock[Socks5ProxyParams]
       socksParams.useForTor returns true
       socksParams.useForIPv4 returns true
       socksParams.useForIPv6 returns true
       nodeParams.socksProxy_opt returns Some(socksParams)
       assert(ReconnectionTask.selectNodeAddress(nodeParams, List(clearnet)).contains(clearnet))
       assert(ReconnectionTask.selectNodeAddress(nodeParams, List(tor)).contains(tor))
-      assert(ReconnectionTask.selectNodeAddress(nodeParams, List(clearnet, tor)).contains(tor))
+      assert(ReconnectionTask.selectNodeAddress(nodeParams, List(clearnet, tor)).exists(Set(clearnet, tor)(_)))
+    }
+    {
+      // tor supported and enabled for clearnet addresses, but disabled for tor: return clearnet addresses when available
+      val socksParams = mock[Socks5ProxyParams]
+      socksParams.useForTor returns false
+      socksParams.useForIPv4 returns true
+      socksParams.useForIPv6 returns true
+      nodeParams.socksProxy_opt returns Some(socksParams)
+      assert(ReconnectionTask.selectNodeAddress(nodeParams, List(clearnet)).contains(clearnet))
+      assert(ReconnectionTask.selectNodeAddress(nodeParams, List(tor)).isEmpty)
+      assert(ReconnectionTask.selectNodeAddress(nodeParams, List(clearnet, tor)).contains(clearnet))
     }
   }