Relay non-blinded failure from wallet nodes (#3050)

When using Bolt12 with a wallet node that is directly connected to us,
we don't always need to return an unreadable failure upstream. If the
wallet node sent us a normal failure, we relay it upstream instead of
overriding it: they chose to reveal that failure to the payer so we
should respect that.

We make sure we don't leak a `channel_update` (since the channel was
unannounced), and the wallet should encrypt the failure with their
blinded node_id, so this shouldn't reveal information that wasn't
already available in the invoice.

Author: t-bast

eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala

@@ -256,16 +256,30 @@ class ChannelRelay private(nodeParams: NodeParams,
       case _: CMD_FULFILL_HTLC => cmd
       case _: CMD_FAIL_HTLC | _: CMD_FAIL_MALFORMED_HTLC => r.payload match {
         case payload: IntermediatePayload.ChannelRelay.Blinded =>
-          // We are inside a blinded route, so we must carefully choose the error we return to avoid leaking information.
-          val failure = InvalidOnionBlinding(Sphinx.hash(r.add.onionRoutingPacket))
-          payload.records.get[OnionPaymentPayloadTlv.PathKey] match {
+          walletNodeId_opt match {
             case Some(_) =>
-              // We are the introduction node: we add a delay to make it look like it could come from further downstream.
-              val delay = Some(Random.nextLong(1000).millis)
-              CMD_FAIL_HTLC(cmd.id, FailureReason.LocalFailure(failure), delay, commit = true)
+              // When the next node is a wallet node directly connected to us, we forward their failure downstream
+              // because we don't need to protect the blinded path against probing since it only contains our node.
+              // Their node isn't announced so they don't reveal anything by sending back a failure message (which
+              // will be encrypted using their blinded node_id).
+              cmd match {
+                // However, when the failure comes from us, we don't want to leak the unannounced channel by revealing
+                // its channel_update: in that case, we always return a temporary node failure instead.
+                case cmd@CMD_FAIL_HTLC(_, FailureReason.LocalFailure(_: Update), _, _, _) => cmd.copy(reason = FailureReason.LocalFailure(TemporaryNodeFailure()))
+                case _ => cmd
+              }
             case None =>
-              // We are not the introduction node.
-              CMD_FAIL_MALFORMED_HTLC(cmd.id, failure.onionHash, failure.code, commit = true)
+              // We are inside a blinded route, so we must carefully choose the error we return to avoid leaking information.
+              val failure = InvalidOnionBlinding(Sphinx.hash(r.add.onionRoutingPacket))
+              payload.records.get[OnionPaymentPayloadTlv.PathKey] match {
+                case Some(_) =>
+                  // We are the introduction node: we add a delay to make it look like it could come from further downstream.
+                  val delay = Some(Random.nextLong(1000).millis)
+                  CMD_FAIL_HTLC(cmd.id, FailureReason.LocalFailure(failure), delay, commit = true)
+                case None =>
+                  // We are not the introduction node.
+                  CMD_FAIL_MALFORMED_HTLC(cmd.id, failure.onionHash, failure.code, commit = true)
+              }
           }
         case _: IntermediatePayload.ChannelRelay.Standard => cmd
       }

eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/OnTheFlyFunding.scala

@@ -96,10 +96,10 @@ object OnTheFlyFunding {
     def createFailureCommands(failure_opt: Option[FailureReason])(implicit log: LoggingAdapter): Seq[(ByteVector32, CMD_FAIL_HTLC)] = upstream match {
       case _: Upstream.Local => Nil
       case u: Upstream.Hot.Channel =>
-        val failure = htlc.pathKey_opt match {
-          case Some(_) => FailureReason.LocalFailure(InvalidOnionBlinding(Sphinx.hash(u.add.onionRoutingPacket)))
-          case None => failure_opt.getOrElse(FailureReason.LocalFailure(UnknownNextPeer()))
-        }
+        // Note that even in the Bolt12 case, we relay the downstream failure instead of sending back invalid_onion_blinding.
+        // That's because we are directly connected to the wallet: the blinded path doesn't contain any other public nodes,
+        // so we don't need to protect against probing. This allows us to return a more meaningful failure to the payer.
+        val failure = failure_opt.getOrElse(FailureReason.LocalFailure(UnknownNextPeer()))
         Seq(u.add.channelId -> CMD_FAIL_HTLC(u.add.id, failure, commit = true))
       case u: Upstream.Hot.Trampoline =>
         val failure = failure_opt match {

eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/ChannelRelayerSpec.scala

@@ -240,7 +240,7 @@ class ChannelRelayerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a
     cleanUpWakeUpActors(peerReadyManager, switchboard)
 
     // We fail without attempting on-the-fly funding.
-    expectFwdFail(register, r.add.channelId, CMD_FAIL_MALFORMED_HTLC(r.add.id, Sphinx.hash(r.add.onionRoutingPacket), InvalidOnionBlinding(Sphinx.hash(r.add.onionRoutingPacket)).code, commit = true))
+    expectFwdFail(register, r.add.channelId, CMD_FAIL_HTLC(r.add.id, FailureReason.LocalFailure(UnknownNextPeer()), commit = true))
   }
 
   test("relay blinded payment (on-the-fly funding failed)", Tag(wakeUpEnabled), Tag(onTheFlyFunding)) { f =>
@@ -265,7 +265,7 @@ class ChannelRelayerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a
     val fwdNodeId = register.expectMessageType[ForwardNodeId[Peer.ProposeOnTheFlyFunding]]
     assert(fwdNodeId.nodeId == outgoingNodeId)
     fwdNodeId.replyTo ! Register.ForwardNodeIdFailure(fwdNodeId)
-    expectFwdFail(register, r.add.channelId, CMD_FAIL_MALFORMED_HTLC(r.add.id, Sphinx.hash(r.add.onionRoutingPacket), InvalidOnionBlinding(Sphinx.hash(r.add.onionRoutingPacket)).code, commit = true))
+    expectFwdFail(register, r.add.channelId, CMD_FAIL_HTLC(r.add.id, FailureReason.LocalFailure(UnknownNextPeer()), commit = true))
   }
 
   test("relay blinded payment (on-the-fly funding not attempted)", Tag(wakeUpEnabled), Tag(onTheFlyFunding)) { f =>
@@ -292,7 +292,7 @@ class ChannelRelayerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a
     fwd.message.replyTo ! RES_ADD_FAILED(fwd.message, TooManyAcceptedHtlcs(channelIds(realScid1), 10), Some(u.channelUpdate))
 
     // We fail without attempting on-the-fly funding.
-    expectFwdFail(register, r.add.channelId, CMD_FAIL_MALFORMED_HTLC(r.add.id, Sphinx.hash(r.add.onionRoutingPacket), InvalidOnionBlinding(Sphinx.hash(r.add.onionRoutingPacket)).code, commit = true))
+    expectFwdFail(register, r.add.channelId, CMD_FAIL_HTLC(r.add.id, FailureReason.LocalFailure(TemporaryNodeFailure()), commit = true))
   }
 
   test("relay with retries") { f =>
@@ -438,7 +438,7 @@ class ChannelRelayerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a
     peerReadyManager.expectMessageType[PeerReadyManager.Register].replyTo ! PeerReadyManager.Registered(outgoingNodeId, otherAttempts = 0)
     assert(switchboard.expectMessageType[Switchboard.GetPeerInfo].remoteNodeId == outgoingNodeId)
     val fail = register.expectMessageType[Register.Forward[CMD_FAIL_HTLC]]
-    assert(fail.message.reason == FailureReason.LocalFailure(InvalidOnionBlinding(Sphinx.hash(r.add.onionRoutingPacket))))
+    assert(fail.message.reason == FailureReason.LocalFailure(UnknownNextPeer()))
 
     cleanUpWakeUpActors(peerReadyManager, switchboard)
   }

eclair-core/src/test/scala/fr/acinq/eclair/payment/relay/OnTheFlyFundingSpec.scala

@@ -246,7 +246,7 @@ class OnTheFlyFundingSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike {
     val fwd2 = register.expectMsgType[Register.Forward[CMD_FAIL_HTLC]]
     assert(fwd2.channelId == upstream2.add.channelId)
     assert(fwd2.message.id == upstream2.add.id)
-    assert(fwd2.message.reason == FailureReason.LocalFailure(InvalidOnionBlinding(Sphinx.hash(upstream2.add.onionRoutingPacket))))
+    assert(fwd2.message.reason == FailureReason.EncryptedDownstreamFailure(fail2.reason))
 
     val fail3 = WillFailMalformedHtlc(willAdd3.id, paymentHash, randomBytes32(), InvalidOnionHmac(randomBytes32()).code)
     peerConnection.send(peer, fail3)
@@ -330,7 +330,7 @@ class OnTheFlyFundingSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike {
       val fwd = register.expectMsgType[Register.Forward[CMD_FAIL_HTLC]]
       assert(fwd.channelId == u.add.channelId)
       assert(fwd.message.id == u.add.id)
-      assert(fwd.message.reason == FailureReason.LocalFailure(InvalidOnionBlinding(Sphinx.hash(u.add.onionRoutingPacket))))
+      assert(fwd.message.reason == FailureReason.LocalFailure(UnknownNextPeer()))
       assert(fwd.message.commit)
     })
     peerConnection.expectMsgType[Warning]