Send normal failures for Bolt12 payments (#773)

Since we are always using 1-hop blinded paths from our LSP to ourselves,
there are no public intermediate nodes that could be probed inside that
blinded path. The requirement to always return malformed failure is thus
unnecessary (and harmful for the payer who doesn't know why the payment
failed). We now return normal failures in that case, which should be
forwarded by our LSP to the upstream nodes.

This also fixes an issue where non-trampoline blinded payments were not
correctly failed because the extraction of the onion shared secret did
not uses the `path_key` to derive the blinded onion decryption key.

Author: t-bast

modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/Commitments.kt

@@ -702,7 +702,7 @@ data class Commitments(
             // we have already sent a fail/fulfill for this htlc
             CommitmentChanges.alreadyProposed(changes.localChanges.proposed, htlc.id) -> Either.Left(UnknownHtlcId(channelId, cmd.id))
             else -> {
-                when (val result = OutgoingPaymentPacket.buildHtlcFailure(nodeSecret, htlc.paymentHash, htlc.onionRoutingPacket, cmd.reason)) {
+                when (val result = OutgoingPaymentPacket.buildHtlcFailure(nodeSecret, htlc.paymentHash, htlc.onionRoutingPacket, htlc.pathKey, cmd.reason)) {
                     is Either.Right -> {
                         val fail = UpdateFailHtlc(channelId, cmd.id, result.value)
                         Either.Right(Pair(copy(changes = changes.addLocalProposal(fail)), fail))

modules/core/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt

@@ -575,10 +575,11 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) {
         }
 
         private fun rejectPaymentPart(privateKey: PrivateKey, paymentPart: PaymentPart, incomingPayment: LightningIncomingPayment?, currentBlockHeight: Int): ProcessAddResult.Rejected {
-            val failureMsg = when (paymentPart.finalPayload) {
-                is PaymentOnion.FinalPayload.Blinded -> InvalidOnionBlinding(Sphinx.hash(paymentPart.onionPacket))
-                is PaymentOnion.FinalPayload.Standard -> IncorrectOrUnknownPaymentDetails(paymentPart.totalAmount, currentBlockHeight.toLong())
-            }
+            // Note that for blinded payment, the BOLTs say we should always return InvalidOnionBlinding.
+            // However, this is in order to protect against malicious nodes probing the blinded path.
+            // But in our case, the blinded path is simply a private channel between our LSP and us, so there is nothing to probe!
+            // We can thus return normal failures, which are more helpful for the payer than InvalidOnionBlinding.
+            val failureMsg = IncorrectOrUnknownPaymentDetails(paymentPart.totalAmount, currentBlockHeight.toLong())
             val rejectedAction = when (paymentPart) {
                 is HtlcPart -> actionForFailureMessage(failureMsg, paymentPart.htlc)
                 is WillAddHtlcPart -> actionForWillAddHtlcFailure(privateKey, failureMsg, paymentPart.htlc)

modules/core/src/commonMain/kotlin/fr/acinq/lightning/payment/OutgoingPaymentPacket.kt

@@ -10,6 +10,7 @@ import fr.acinq.lightning.Feature
 import fr.acinq.lightning.Lightning
 import fr.acinq.lightning.MilliSatoshi
 import fr.acinq.lightning.channel.ChannelCommand
+import fr.acinq.lightning.crypto.RouteBlinding
 import fr.acinq.lightning.crypto.sphinx.FailurePacket
 import fr.acinq.lightning.crypto.sphinx.PacketAndSecrets
 import fr.acinq.lightning.crypto.sphinx.Sphinx
@@ -144,9 +145,10 @@ object OutgoingPaymentPacket {
         return Triple(trampolineAmount, trampolineExpiry, paymentOnion)
     }
 
-    fun buildHtlcFailure(nodeSecret: PrivateKey, paymentHash: ByteVector32, onion: OnionRoutingPacket, reason: ChannelCommand.Htlc.Settlement.Fail.Reason): Either<FailureMessage, ByteVector> {
-        // we need to decrypt the payment onion to obtain the shared secret to build the error packet
-        return when (val result = Sphinx.peel(nodeSecret, paymentHash, onion)) {
+    fun buildHtlcFailure(nodeSecret: PrivateKey, paymentHash: ByteVector32, onion: OnionRoutingPacket, pathKey: PublicKey?, reason: ChannelCommand.Htlc.Settlement.Fail.Reason): Either<FailureMessage, ByteVector> {
+        // We need to decrypt the payment onion to obtain the shared secret to build the error packet.
+        val onionDecryptionKey = pathKey?.let { RouteBlinding.derivePrivateKey(nodeSecret, it) } ?: nodeSecret
+        return when (val result = Sphinx.peel(onionDecryptionKey, paymentHash, onion)) {
             is Either.Right -> {
                 val encryptedReason = when (reason) {
                     is ChannelCommand.Htlc.Settlement.Fail.Reason.Bytes -> FailurePacket.wrap(reason.bytes.toByteArray(), result.value.sharedSecret)
@@ -160,7 +162,7 @@ object OutgoingPaymentPacket {
 
     fun buildWillAddHtlcFailure(nodeSecret: PrivateKey, willAddHtlc: WillAddHtlc, failure: FailureMessage): OnTheFlyFundingMessage {
         val reason = ChannelCommand.Htlc.Settlement.Fail.Reason.Failure(failure)
-        return when (val f = buildHtlcFailure(nodeSecret, willAddHtlc.paymentHash, willAddHtlc.finalPacket, reason)) {
+        return when (val f = buildHtlcFailure(nodeSecret, willAddHtlc.paymentHash, willAddHtlc.finalPacket, willAddHtlc.pathKey, reason)) {
             is Either.Right -> WillFailHtlc(willAddHtlc.id, willAddHtlc.paymentHash, f.value)
             is Either.Left -> WillFailMalformedHtlc(willAddHtlc.id, willAddHtlc.paymentHash, Sphinx.hash(willAddHtlc.finalPacket), f.value.code)
         }

modules/core/src/commonMain/kotlin/fr/acinq/lightning/wire/PaymentOnion.kt

@@ -401,7 +401,6 @@ object PaymentOnion {
     data class ChannelRelayPayload(val records: TlvStream<OnionPaymentPayloadTlv>) : PerHopPayload() {
         val amountToForward = records.get<OnionPaymentPayloadTlv.AmountToForward>()!!.amount
         val outgoingCltv = records.get<OnionPaymentPayloadTlv.OutgoingCltv>()!!.cltv
-        val outgoingChannelId = records.get<OnionPaymentPayloadTlv.OutgoingChannelId>()!!.shortChannelId
 
         override fun write(out: Output) = tlvSerializer.write(records, out)
 
@@ -423,6 +422,16 @@ object PaymentOnion {
         }
     }
 
+    data class BlindedChannelRelayPayload(val records: TlvStream<OnionPaymentPayloadTlv>) : PerHopPayload() {
+        override fun write(out: Output) = tlvSerializer.write(records, out)
+
+        companion object : PerHopPayloadReader<BlindedChannelRelayPayload> {
+            override fun read(input: Input): Either<InvalidOnionPayload, BlindedChannelRelayPayload> {
+                return PerHopPayload.read(input).map { BlindedChannelRelayPayload(it) }
+            }
+        }
+    }
+
     data class NodeRelayPayload(val records: TlvStream<OnionPaymentPayloadTlv>) : PerHopPayload() {
         val amountToForward = records.get<OnionPaymentPayloadTlv.AmountToForward>()!!.amount
         val outgoingCltv = records.get<OnionPaymentPayloadTlv.OutgoingCltv>()!!.cltv

modules/core/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt

@@ -1768,8 +1768,8 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
         val result = paymentHandler.process(add, Features.empty, TestConstants.defaultBlockHeight, TestConstants.feeratePerKw, remoteFundingRates = null)
 
         assertIs<IncomingPaymentHandler.ProcessAddResult.Rejected>(result)
-        val expectedFailure = InvalidOnionBlinding(hash(add.onionRoutingPacket))
-        val expected = ChannelCommand.Htlc.Settlement.FailMalformed(add.id, expectedFailure.onionHash, expectedFailure.code, commit = true)
+        val expectedFailure = IncorrectOrUnknownPaymentDetails(defaultAmount, TestConstants.defaultBlockHeight.toLong())
+        val expected = ChannelCommand.Htlc.Settlement.Fail(add.id, ChannelCommand.Htlc.Settlement.Fail.Reason.Failure(expectedFailure), commit = true)
         assertEquals(setOf(WrappedChannelCommand(add.channelId, expected)), result.actions.toSet())
     }
 
@@ -1819,8 +1819,8 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
         val result = paymentHandler.process(add, Features.empty, TestConstants.defaultBlockHeight, TestConstants.feeratePerKw, remoteFundingRates = null)
 
         assertIs<IncomingPaymentHandler.ProcessAddResult.Rejected>(result)
-        val expectedFailure = InvalidOnionBlinding(hash(add.onionRoutingPacket))
-        val expected = ChannelCommand.Htlc.Settlement.FailMalformed(add.id, expectedFailure.onionHash, expectedFailure.code, commit = true)
+        val expectedFailure = IncorrectOrUnknownPaymentDetails(amountTooLow, TestConstants.defaultBlockHeight.toLong())
+        val expected = ChannelCommand.Htlc.Settlement.Fail(add.id, ChannelCommand.Htlc.Settlement.Fail.Reason.Failure(expectedFailure), commit = true)
         assertEquals(setOf(WrappedChannelCommand(add.channelId, expected)), result.actions.toSet())
     }
 
@@ -1835,8 +1835,8 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
         val result = paymentHandler.process(add, Features.empty, TestConstants.defaultBlockHeight, TestConstants.feeratePerKw, remoteFundingRates = null)
 
         assertIs<IncomingPaymentHandler.ProcessAddResult.Rejected>(result)
-        val expectedFailure = InvalidOnionBlinding(hash(add.onionRoutingPacket))
-        val expected = ChannelCommand.Htlc.Settlement.FailMalformed(add.id, expectedFailure.onionHash, expectedFailure.code, commit = true)
+        val expectedFailure = IncorrectOrUnknownPaymentDetails(metadata.amount, TestConstants.defaultBlockHeight.toLong())
+        val expected = ChannelCommand.Htlc.Settlement.Fail(add.id, ChannelCommand.Htlc.Settlement.Fail.Reason.Failure(expectedFailure), commit = true)
         assertEquals(setOf(WrappedChannelCommand(add.channelId, expected)), result.actions.toSet())
     }
 

modules/core/src/commonTest/kotlin/fr/acinq/lightning/payment/PaymentPacketTestsCommon.kt

@@ -8,8 +8,10 @@ import fr.acinq.lightning.Lightning.randomBytes
 import fr.acinq.lightning.Lightning.randomBytes32
 import fr.acinq.lightning.Lightning.randomBytes64
 import fr.acinq.lightning.Lightning.randomKey
+import fr.acinq.lightning.channel.ChannelCommand
 import fr.acinq.lightning.channel.states.Channel
 import fr.acinq.lightning.crypto.RouteBlinding
+import fr.acinq.lightning.crypto.sphinx.FailurePacket
 import fr.acinq.lightning.crypto.sphinx.PacketAndSecrets
 import fr.acinq.lightning.crypto.sphinx.Sphinx
 import fr.acinq.lightning.crypto.sphinx.Sphinx.hash
@@ -123,6 +125,19 @@ class PaymentPacketTestsCommon : LightningTestSuite() {
             return Pair(outerPayload, innerPayload)
         }
 
+        // Wallets don't need to decrypt onions where they're the introduction node of a blinded path, but it's useful to test that encryption works correctly.
+        fun decryptBlindedChannelRelay(add: UpdateAddHtlc, privateKey: PrivateKey): Pair<OnionRoutingPacket, PublicKey> {
+            val decrypted = Sphinx.peel(privateKey, add.paymentHash, add.onionRoutingPacket).right!!
+            assertFalse(decrypted.isLastPacket)
+            val payload = PaymentOnion.PerHopPayload.read(decrypted.payload.toByteArray()).right!!
+            val pathKey = payload.get<OnionPaymentPayloadTlv.PathKey>()?.publicKey ?: add.pathKey
+            assertNotNull(pathKey)
+            val encryptedData = payload.get<OnionPaymentPayloadTlv.EncryptedRecipientData>()!!.data
+            val nextPathKey = RouteBlinding.decryptPayload(privateKey, pathKey, encryptedData).map { it.second }.right
+            assertNotNull(nextPathKey)
+            return Pair(decrypted.nextPacket, nextPathKey)
+        }
+
         // Wallets don't need to decrypt onions for intermediate nodes, but it's useful to test that encryption works correctly.
         fun decryptRelayToBlinded(add: UpdateAddHtlc, privateKey: PrivateKey): Pair<PaymentOnion.FinalPayload, PaymentOnion.RelayToBlindedPayload> {
             val decrypted = Sphinx.peel(privateKey, add.paymentHash, add.onionRoutingPacket).right!!
@@ -504,6 +519,110 @@ class PaymentPacketTestsCommon : LightningTestSuite() {
         assertEquals(InvalidOnionBlinding(hash(addD.onionRoutingPacket)), failure)
     }
 
+    @Test
+    fun `build htlc failure onion`() {
+        // B sends a payment B -> C -> D.
+        val (amountC, expiryC, onionC) = run {
+            val payloadD = PaymentOnion.FinalPayload.Standard.createMultiPartPayload(finalAmount, finalAmount, finalExpiry, paymentSecret, paymentMetadata = null)
+            encryptChannelRelay(paymentHash, listOf(ChannelHop(b, c, channelUpdateBC), ChannelHop(c, d, channelUpdateCD)), payloadD)
+        }
+        assertEquals(amountBC, amountC)
+        assertEquals(expiryBC, expiryC)
+
+        // C relays the payment to D.
+        val addC = UpdateAddHtlc(randomBytes32(), 2, amountC, paymentHash, expiryC, onionC.packet)
+        val (payloadC, packetD) = decryptChannelRelay(addC, privC)
+        val addD = UpdateAddHtlc(randomBytes32(), 3, payloadC.amountToForward, paymentHash, payloadC.outgoingCltv, packetD)
+        assertNotNull(IncomingPaymentPacket.decrypt(addD, privD).right)
+        val willAddD = WillAddHtlc(Block.RegtestGenesisBlock.hash, randomBytes32(), payloadC.amountToForward, paymentHash, payloadC.outgoingCltv, packetD)
+        assertNotNull(IncomingPaymentPacket.decrypt(willAddD, privD))
+
+        // D returns a failure.
+        val failure = IncorrectOrUnknownPaymentDetails(finalAmount, currentBlockCount)
+        val encryptedFailuresD = run {
+            val encryptedFailureD = OutgoingPaymentPacket.buildHtlcFailure(privD, paymentHash, addD.onionRoutingPacket, addD.pathKey, ChannelCommand.Htlc.Settlement.Fail.Reason.Failure(failure)).right
+            assertNotNull(encryptedFailureD)
+            val willFailD = OutgoingPaymentPacket.buildWillAddHtlcFailure(privD, willAddD, failure)
+            assertIs<WillFailHtlc>(willFailD)
+            listOf(encryptedFailureD, willFailD.reason)
+        }
+        encryptedFailuresD.forEach { encryptedFailureD ->
+            // C cannot decrypt the failure: it re-encrypts and relays that failure to B.
+            val encryptedFailureC = OutgoingPaymentPacket.buildHtlcFailure(privC, paymentHash, addC.onionRoutingPacket, addC.pathKey, ChannelCommand.Htlc.Settlement.Fail.Reason.Bytes(encryptedFailureD)).right
+            assertNotNull(encryptedFailureC)
+            // B decrypts the failure.
+            val decrypted = FailurePacket.decrypt(encryptedFailureC.toByteArray(), onionC.sharedSecrets)
+            assertTrue(decrypted.isSuccess)
+            assertEquals(d, decrypted.get().originNode)
+            assertEquals(failure, decrypted.get().failureMessage)
+        }
+    }
+
+    @Test
+    fun `build htlc failure onion -- blinded payment`() {
+        // D creates a blinded path C -> D.
+        val offer = OfferTypes.Offer.createNonBlindedOffer(finalAmount, "test offer", d, Features(Feature.BasicMultiPartPayment to FeatureSupport.Optional), Block.RegtestGenesisBlock.hash)
+        val pathId = OfferPaymentMetadata.V1(offer.offerId, finalAmount, paymentPreimage, randomKey().publicKey(), "hello", 1, currentTimestampMillis()).toPathId(privD)
+        val blindedPayloadD = RouteBlindingEncryptedData(TlvStream(RouteBlindingEncryptedDataTlv.PathId(pathId)))
+        val blindedPayloadC = RouteBlindingEncryptedData(
+            TlvStream(
+                RouteBlindingEncryptedDataTlv.OutgoingChannelId(channelUpdateCD.shortChannelId),
+                RouteBlindingEncryptedDataTlv.PaymentRelay(channelUpdateCD.cltvExpiryDelta, channelUpdateCD.feeProportionalMillionths, channelUpdateCD.feeBaseMsat),
+                RouteBlindingEncryptedDataTlv.PaymentConstraints(finalExpiry, 1.msat),
+            )
+        )
+        val blindedRoute = RouteBlinding.create(randomKey(), listOf(c, d), listOf(blindedPayloadC, blindedPayloadD).map { it.write().byteVector() }).route
+
+        // B sends a blinded payment B -> C -> D using this blinded path.
+        val onionC = run {
+            val payloadD = PaymentOnion.FinalPayload.Blinded(
+                TlvStream(
+                    OnionPaymentPayloadTlv.AmountToForward(finalAmount),
+                    OnionPaymentPayloadTlv.TotalAmount(finalAmount),
+                    OnionPaymentPayloadTlv.OutgoingCltv(finalExpiry),
+                    OnionPaymentPayloadTlv.EncryptedRecipientData(blindedRoute.encryptedPayloads.last()),
+                ),
+                blindedPayloadD
+            )
+            val payloadC = PaymentOnion.BlindedChannelRelayPayload(
+                TlvStream(
+                    OnionPaymentPayloadTlv.EncryptedRecipientData(blindedRoute.encryptedPayloads.first()),
+                    OnionPaymentPayloadTlv.PathKey(blindedRoute.firstPathKey),
+                )
+            )
+            OutgoingPaymentPacket.buildOnion(listOf(c, blindedRoute.blindedNodeIds.last()), listOf(payloadC, payloadD), paymentHash, OnionRoutingPacket.PaymentPacketLength)
+        }
+
+        // C relays the payment to D.
+        val addC = UpdateAddHtlc(randomBytes32(), 2, amountBC, paymentHash, expiryBC, onionC.packet)
+        val (packetD, pathKeyD) = decryptBlindedChannelRelay(addC, privC)
+        val addD = UpdateAddHtlc(randomBytes32(), 3, amountCD, paymentHash, expiryCD, packetD, pathKeyD, fundingFee = null)
+        assertNotNull(IncomingPaymentPacket.decrypt(addD, privD).right)
+        val willAddD = WillAddHtlc(Block.RegtestGenesisBlock.hash, randomBytes32(), amountCD, paymentHash, expiryCD, packetD, pathKeyD)
+        assertNotNull(IncomingPaymentPacket.decrypt(willAddD, privD).right)
+
+        // D returns a failure: note that it is not a blinded failure, since there is no need to protect the blinded path against probing.
+        // This ensures that payers get a meaningful error from wallet recipients.
+        val failure = IncorrectOrUnknownPaymentDetails(finalAmount, currentBlockCount)
+        val encryptedFailuresD = run {
+            val encryptedFailureD = OutgoingPaymentPacket.buildHtlcFailure(privD, paymentHash, addD.onionRoutingPacket, addD.pathKey, ChannelCommand.Htlc.Settlement.Fail.Reason.Failure(failure)).right
+            assertNotNull(encryptedFailureD)
+            val willFailD = OutgoingPaymentPacket.buildWillAddHtlcFailure(privD, willAddD, failure)
+            assertIs<WillFailHtlc>(willFailD)
+            listOf(encryptedFailureD, willFailD.reason)
+        }
+        encryptedFailuresD.forEach { encryptedFailureD ->
+            // C cannot decrypt the failure: it re-encrypts and relays that failure to B.
+            val encryptedFailureC = OutgoingPaymentPacket.buildHtlcFailure(privC, paymentHash, addC.onionRoutingPacket, addC.pathKey, ChannelCommand.Htlc.Settlement.Fail.Reason.Bytes(encryptedFailureD)).right
+            assertNotNull(encryptedFailureC)
+            // B decrypts the failure.
+            val decrypted = FailurePacket.decrypt(encryptedFailureC.toByteArray(), onionC.sharedSecrets)
+            assertTrue(decrypted.isSuccess)
+            assertEquals(blindedRoute.blindedNodeIds.last(), decrypted.get().originNode)
+            assertEquals(failure, decrypted.get().failureMessage)
+        }
+    }
+
     @Test
     fun `prune outgoing blinded paths`() {
         // We create an invoice with a large number of blinded paths.