Fix interpretation of option_onion_messages (#2670)

According to the spec, option_onion_messages signals that the node can forward onion messages which is different from being able to send or receive them (there is no feature bit for that).
We now allow nodes with this feature disabled to still receive messages and remove the NoRelay relay policy as it is redundant.

eclair-core/src/main/resources/reference.conf

@@ -466,10 +466,11 @@ eclair {
 
   onion-messages {
     # Valid values are
-    # - no-relay: Allow sending and receiving onion messages but never relay them
     # - channels-only: Only relay messages from peers with which we have a channel to peers with which we have a channel.
     # - relay-all: Relay everything and create new connections if necessary
     relay-policy = "channels-only"
+    # If you want to never relay onion messages (but still be able to send and receive them), you need to set
+    # features.option_onion_messages = disabled
 
     # Transient connections opened to relay messages will be closed after this delay of inactivity
     kill-transient-connection-after = 30 seconds

eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala

@@ -27,7 +27,7 @@ import fr.acinq.eclair.channel.fsm.Channel.{ChannelConf, UnhandledExceptionStrat
 import fr.acinq.eclair.crypto.Noise.KeyPair
 import fr.acinq.eclair.crypto.keymanager.{ChannelKeyManager, NodeKeyManager}
 import fr.acinq.eclair.db._
-import fr.acinq.eclair.io.MessageRelay.{NoRelay, RelayAll, RelayChannelsOnly, RelayPolicy}
+import fr.acinq.eclair.io.MessageRelay.{RelayAll, RelayChannelsOnly, RelayPolicy}
 import fr.acinq.eclair.io.PeerConnection
 import fr.acinq.eclair.message.OnionMessages.OnionMessageConfig
 import fr.acinq.eclair.payment.relay.Relayer.{AsyncPaymentsParams, RelayFees, RelayParams}
@@ -427,7 +427,6 @@ object NodeParams extends Logging {
     }
 
     val onionMessageRelayPolicy: RelayPolicy = config.getString("onion-messages.relay-policy") match {
-      case "no-relay" => NoRelay
       case "channels-only" => RelayChannelsOnly
       case "relay-all" => RelayAll
     }

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

@@ -49,7 +49,6 @@ object MessageRelay {
   }
 
   sealed trait RelayPolicy
-  case object NoRelay extends RelayPolicy
   case object RelayChannelsOnly extends RelayPolicy
   case object RelayAll extends RelayPolicy
   // @formatter:on
@@ -58,9 +57,6 @@ object MessageRelay {
     Behaviors.receivePartial {
       case (context, RelayMessage(messageId, switchboard, prevNodeId, nextNodeId, msg, policy, replyTo_opt)) =>
         policy match {
-          case NoRelay =>
-            replyTo_opt.foreach(_ ! AgainstPolicy(messageId, policy))
-            Behaviors.stopped
           case RelayChannelsOnly =>
             switchboard ! GetPeerInfo(context.messageAdapter(WrappedPeerInfo), prevNodeId)
             waitForPreviousPeer(messageId, switchboard, nextNodeId, msg, replyTo_opt)

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

@@ -275,16 +275,16 @@ class Peer(val nodeParams: NodeParams, remoteNodeId: PublicKey, wallet: OnchainP
         gotoConnected(connectionReady, d.channels)
 
       case Event(msg: OnionMessage, _: ConnectedData) =>
-        if (nodeParams.features.hasFeature(Features.OnionMessages)) {
-          OnionMessages.process(nodeParams.privateKey, msg) match {
-            case OnionMessages.DropMessage(reason) =>
-              log.debug(s"dropping message from ${remoteNodeId.value.toHex}: ${reason.toString}")
-            case OnionMessages.SendMessage(nextNodeId, message) =>
-              switchboard ! RelayMessage(randomBytes32(), Some(remoteNodeId), nextNodeId, message, nodeParams.onionMessageConfig.relayPolicy, None)
-            case received: OnionMessages.ReceiveMessage =>
-              log.info(s"received message from ${remoteNodeId.value.toHex}: $received")
-              context.system.eventStream.publish(received)
-          }
+        OnionMessages.process(nodeParams.privateKey, msg) match {
+          case OnionMessages.DropMessage(reason) =>
+            log.debug("dropping message from {}: {}", remoteNodeId.value.toHex, reason.toString)
+          case OnionMessages.SendMessage(nextNodeId, message) if nodeParams.features.hasFeature(Features.OnionMessages) =>
+            switchboard ! RelayMessage(randomBytes32(), Some(remoteNodeId), nextNodeId, message, nodeParams.onionMessageConfig.relayPolicy, None)
+          case OnionMessages.SendMessage(_, _) =>
+            log.debug("dropping message from {}: relaying onion messages is disabled", remoteNodeId.value.toHex)
+          case received: OnionMessages.ReceiveMessage =>
+            log.info("received message from {}: {}", remoteNodeId.value.toHex, received)
+            context.system.eventStream.publish(received)
         }
         stay()
 

eclair-core/src/test/scala/fr/acinq/eclair/integration/MessageIntegrationSpec.scala

@@ -52,7 +52,7 @@ class MessageIntegrationSpec extends IntegrationSpec {
     instantiateEclairNode("C", ConfigFactory.parseMap(Map("eclair.node-alias" -> "C", "eclair.server.port" -> 30702, "eclair.api.port" -> 30782, s"eclair.features.${Features.OnionMessages.rfcName}" -> "optional", "eclair.onion-messages.relay-policy" -> "relay-all").asJava).withFallback(commonConfig))
     instantiateEclairNode("D", ConfigFactory.parseMap(Map("eclair.node-alias" -> "D", "eclair.server.port" -> 30703, "eclair.api.port" -> 30783).asJava).withFallback(commonConfig))
     instantiateEclairNode("E", ConfigFactory.parseMap(Map("eclair.node-alias" -> "E", "eclair.server.port" -> 30704, "eclair.api.port" -> 30784, s"eclair.features.${Features.OnionMessages.rfcName}" -> "optional", "eclair.onion-messages.relay-policy" -> "channels-only").asJava).withFallback(commonConfig))
-    instantiateEclairNode("F", ConfigFactory.parseMap(Map("eclair.node-alias" -> "F", "eclair.server.port" -> 30705, "eclair.api.port" -> 30785, s"eclair.features.${Features.OnionMessages.rfcName}" -> "optional", "eclair.onion-messages.relay-policy" -> "no-relay").asJava).withFallback(commonConfig))
+    instantiateEclairNode("F", ConfigFactory.parseMap(Map("eclair.node-alias" -> "F", "eclair.server.port" -> 30705, "eclair.api.port" -> 30785, s"eclair.features.${Features.OnionMessages.rfcName}" -> "disabled").asJava).withFallback(commonConfig))
   }
 
   test("try to reach unknown node") {
@@ -134,7 +134,7 @@ class MessageIntegrationSpec extends IntegrationSpec {
     eventListener.expectMsgType[OnionMessages.ReceiveMessage](max = 60 seconds)
   }
 
-  test("send to connected node with no-relay") {
+  test("send from node with option_onion_messages disabled") {
     val fabrice = new EclairImpl(nodes("F"))
     connect(nodes("F"), nodes("A"))
 
@@ -146,6 +146,17 @@ class MessageIntegrationSpec extends IntegrationSpec {
     eventListener.expectMsgType[OnionMessages.ReceiveMessage](max = 60 seconds)
   }
 
+  test("send to node with option_onion_messages disabled") {
+    val alice = new EclairImpl(nodes("A"))
+
+    val probe = TestProbe()
+    val eventListener = TestProbe()
+    nodes("F").system.eventStream.subscribe(eventListener.ref, classOf[OnionMessages.ReceiveMessage])
+    alice.sendOnionMessage(Nil, Left(nodes("F").nodeParams.nodeId), None, ByteVector.empty).pipeTo(probe.ref)
+    assert(probe.expectMsgType[SendOnionMessageResponse].sent)
+    eventListener.expectMsgType[OnionMessages.ReceiveMessage](max = 60 seconds)
+  }
+
   test("send with hop") {
     val alice = new EclairImpl(nodes("A"))
     connect(nodes("B"), nodes("C"))

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

@@ -148,16 +148,4 @@ class MessageRelaySpec extends ScalaTestWithActorTestKit(ConfigFactory.load("app
 
     assert(peer.expectMessageType[Peer.RelayOnionMessage].msg == message)
   }
-
-  test("no relay") { f =>
-    import f._
-
-    val Right((_, message)) = OnionMessages.buildMessage(randomKey(), randomKey(), randomKey(), Seq(IntermediateNode(aliceId)), Recipient(bobId, None), TlvStream.empty)
-    val messageId = randomBytes32()
-    val previousNodeId = randomKey().publicKey
-    relay ! RelayMessage(messageId, switchboard.ref, previousNodeId, bobId, message, NoRelay, Some(probe.ref))
-
-    switchboard.expectNoMessage(100 millis)
-    probe.expectMessage(AgainstPolicy(messageId, NoRelay))
-  }
 }