Include min_final_cltv_expiry_delta in blinded paths

src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt

@@ -4,9 +4,12 @@ import fr.acinq.bitcoin.ByteVector32
 import fr.acinq.bitcoin.PublicKey
 import fr.acinq.bitcoin.utils.Either.Left
 import fr.acinq.bitcoin.utils.Either.Right
-import fr.acinq.lightning.*
+import fr.acinq.lightning.EncodedNodeId
+import fr.acinq.lightning.Features
 import fr.acinq.lightning.Lightning.randomBytes32
 import fr.acinq.lightning.Lightning.randomKey
+import fr.acinq.lightning.NodeParams
+import fr.acinq.lightning.WalletParams
 import fr.acinq.lightning.crypto.RouteBlinding
 import fr.acinq.lightning.io.OfferInvoiceReceived
 import fr.acinq.lightning.io.OfferNotPaid
@@ -18,7 +21,6 @@ import fr.acinq.lightning.message.OnionMessages.Destination
 import fr.acinq.lightning.message.OnionMessages.IntermediateNode
 import fr.acinq.lightning.message.OnionMessages.buildMessage
 import fr.acinq.lightning.utils.currentTimestampMillis
-import fr.acinq.lightning.utils.msat
 import fr.acinq.lightning.utils.toByteVector
 import fr.acinq.lightning.wire.*
 import kotlinx.coroutines.flow.MutableSharedFlow
@@ -130,6 +132,8 @@ class OfferManager(val nodeParams: NodeParams, val walletParams: WalletParams, v
     private fun receiveInvoiceRequest(decrypted: OnionMessages.DecryptedMessage, remoteChannelUpdates: List<ChannelUpdate>, currentBlockHeight: Int): OnionMessageAction.SendMessage? {
         val offer = localOffers[decrypted.pathId]!!
         val request = decrypted.content.records.get<OnionMessagePayloadTlv.InvoiceRequest>()?.let { OfferTypes.InvoiceRequest.validate(it.tlvs).right }
+        // We must use the most restrictive minimum HTLC value between local and remote.
+        val minHtlc = (listOf(nodeParams.htlcMinimum) + remoteChannelUpdates.map { it.htlcMinimumMsat }).max()
         return when {
             request == null -> {
                 logger.warning { "offerId:${offer.offerId} pathId:${decrypted.pathId} ignoring onion message: missing or invalid invoice request" }
@@ -147,8 +151,12 @@ class OfferManager(val nodeParams: NodeParams, val walletParams: WalletParams, v
                 logger.warning { "offerId:${offer.offerId} pathId:${decrypted.pathId} ignoring invalid invoice request ($request)" }
                 sendInvoiceError("ignoring invalid invoice request", decrypted.content.replyPath)
             }
+            request.requestedAmount()?.let { it < minHtlc } ?: false -> {
+                logger.warning { "offerId:${offer.offerId} pathId:${decrypted.pathId} amount too low (amount=${request.requestedAmount()} minHtlc=$minHtlc)" }
+                sendInvoiceError("amount too low, minimum amount = $minHtlc", decrypted.content.replyPath)
+            }
             else -> {
-                val amount = request.amount ?: (request.offer.amount!! * request.quantity)
+                val amount = request.requestedAmount()!!
                 val preimage = randomBytes32()
                 val truncatedPayerNote = request.payerNote?.let {
                     if (it.length <= 64) {
@@ -159,18 +167,24 @@ class OfferManager(val nodeParams: NodeParams, val walletParams: WalletParams, v
                 }
                 val pathId = OfferPaymentMetadata.V1(ByteVector32(decrypted.pathId), amount, preimage, request.payerId, truncatedPayerNote, request.quantity, currentTimestampMillis()).toPathId(nodeParams.nodePrivateKey)
                 val recipientPayload = RouteBlindingEncryptedData(TlvStream(RouteBlindingEncryptedDataTlv.PathId(pathId))).write().toByteVector()
+                val cltvExpiryDelta = remoteChannelUpdates.maxOfOrNull { it.cltvExpiryDelta } ?: walletParams.invoiceDefaultRoutingFees.cltvExpiryDelta
                 val paymentInfo = OfferTypes.PaymentInfo(
                     feeBase = remoteChannelUpdates.maxOfOrNull { it.feeBaseMsat } ?: walletParams.invoiceDefaultRoutingFees.feeBase,
                     feeProportionalMillionths = remoteChannelUpdates.maxOfOrNull { it.feeProportionalMillionths } ?: walletParams.invoiceDefaultRoutingFees.feeProportional,
-                    cltvExpiryDelta = remoteChannelUpdates.maxOfOrNull { it.cltvExpiryDelta } ?: walletParams.invoiceDefaultRoutingFees.cltvExpiryDelta,
-                    minHtlc = remoteChannelUpdates.minOfOrNull { it.htlcMinimumMsat } ?: 1.msat,
-                    maxHtlc = amount,
+                    // We include our min_final_cltv_expiry_delta in the path, but we *don't* include it in the payment_relay field
+                    // for our trampoline node (below). This ensures that we will receive payments with at least this final expiry delta.
+                    // This ensures that even when payers haven't received the latest block(s) or don't include a safety margin in the
+                    // expiry they use, we can still safely receive their payment.
+                    cltvExpiryDelta = cltvExpiryDelta + nodeParams.minFinalCltvExpiryDelta,
+                    minHtlc = minHtlc,
+                    // Payments are allowed to overpay at most two times the invoice amount.
+                    maxHtlc = amount * 2,
                     allowedFeatures = Features.empty
                 )
                 val remoteNodePayload = RouteBlindingEncryptedData(
                     TlvStream(
                         RouteBlindingEncryptedDataTlv.OutgoingNodeId(EncodedNodeId.WithPublicKey.Wallet(nodeParams.nodeId)),
-                        RouteBlindingEncryptedDataTlv.PaymentRelay(paymentInfo.cltvExpiryDelta, paymentInfo.feeProportionalMillionths, paymentInfo.feeBase),
+                        RouteBlindingEncryptedDataTlv.PaymentRelay(cltvExpiryDelta, paymentInfo.feeProportionalMillionths, paymentInfo.feeBase),
                         RouteBlindingEncryptedDataTlv.PaymentConstraints((paymentInfo.cltvExpiryDelta + nodeParams.maxFinalCltvExpiryDelta).toCltvExpiry(currentBlockHeight.toLong()), paymentInfo.minHtlc)
                     )
                 ).write().toByteVector()

src/commonMain/kotlin/fr/acinq/lightning/wire/OfferTypes.kt

@@ -860,6 +860,8 @@ object OfferTypes {
                     Features.areCompatible(offer.features, features) &&
                     checkSignature()
 
+        fun requestedAmount(): MilliSatoshi? = amount ?: offer.amount?.let { it * quantity }
+
         fun checkSignature(): Boolean =
             verifySchnorr(
                 signatureTag,

src/commonTest/kotlin/fr/acinq/lightning/channel/states/NormalTestsCommon.kt

@@ -151,12 +151,10 @@ class NormalTestsCommon : LightningTestSuite() {
     @Test
     fun `recv ChannelCommand_Htlc_Add -- 0 msat`() {
         val (alice0, bob0) = reachNormal()
-        // Alice has a minimum set to 0 msat (which should be invalid, but may mislead Bob into relaying 0-value HTLCs which is forbidden by the spec).
-        assertEquals(0.msat, alice0.commitments.params.localParams.htlcMinimum)
         val add = defaultAdd.copy(amount = 0.msat)
         val (bob1, actions) = bob0.process(add)
         val actualError = actions.findCommandError<HtlcValueTooSmall>()
-        val expectedError = HtlcValueTooSmall(bob0.channelId, 1.msat, 0.msat)
+        val expectedError = HtlcValueTooSmall(bob0.channelId, alice0.commitments.params.localParams.htlcMinimum, 0.msat)
         assertEquals(expectedError, actualError)
         assertEquals(bob0, bob1)
     }

src/commonTest/kotlin/fr/acinq/lightning/payment/OfferManagerTestsCommon.kt

@@ -85,6 +85,12 @@ class OfferManagerTestsCommon : LightningTestSuite() {
         assertIs<OnionMessageAction.PayInvoice>(payInvoice)
         assertEquals(OfferInvoiceReceived(payOffer, payInvoice.invoice), bobOfferManager.eventsFlow.first())
         assertEquals(payOffer, payInvoice.payOffer)
+        assertEquals(1, payInvoice.invoice.blindedPaths.size)
+        val path = payInvoice.invoice.blindedPaths.first()
+        assertEquals(EncodedNodeId(aliceTrampolineKey.publicKey()), path.route.route.introductionNodeId)
+        assertEquals(aliceOfferManager.nodeParams.expiryDeltaBlocks + aliceOfferManager.nodeParams.minFinalCltvExpiryDelta, path.paymentInfo.cltvExpiryDelta)
+        assertEquals(TestConstants.Alice.nodeParams.htlcMinimum, path.paymentInfo.minHtlc)
+        assertEquals(payOffer.amount * 2, path.paymentInfo.maxHtlc)
     }
 
     @Test
@@ -113,6 +119,12 @@ class OfferManagerTestsCommon : LightningTestSuite() {
         assertIs<OnionMessageAction.PayInvoice>(payInvoice)
         assertEquals(OfferInvoiceReceived(payOffer, payInvoice.invoice), bobOfferManager.eventsFlow.first())
         assertEquals(payOffer, payInvoice.payOffer)
+        assertEquals(1, payInvoice.invoice.blindedPaths.size)
+        val path = payInvoice.invoice.blindedPaths.first()
+        assertEquals(EncodedNodeId(aliceTrampolineKey.publicKey()), path.route.route.introductionNodeId)
+        assertEquals(aliceOfferManager.nodeParams.expiryDeltaBlocks + aliceOfferManager.nodeParams.minFinalCltvExpiryDelta, path.paymentInfo.cltvExpiryDelta)
+        assertEquals(TestConstants.Alice.nodeParams.htlcMinimum, path.paymentInfo.minHtlc)
+        assertEquals(payOffer.amount * 2, path.paymentInfo.maxHtlc)
     }
 
     @Test
@@ -194,7 +206,7 @@ class OfferManagerTestsCommon : LightningTestSuite() {
     }
 
     @Test
-    fun `receive invoice error`() = runSuspendTest {
+    fun `receive invoice error -- amount below offer amount`() = runSuspendTest {
         // Alice and Bob use the same trampoline node.
         val aliceOfferManager = OfferManager(TestConstants.Alice.nodeParams, aliceWalletParams, MutableSharedFlow(replay = 10), logger)
         val bobOfferManager = OfferManager(TestConstants.Bob.nodeParams, aliceWalletParams, MutableSharedFlow(replay = 10), logger)
@@ -214,6 +226,27 @@ class OfferManagerTestsCommon : LightningTestSuite() {
         assertIs<Bolt12InvoiceRequestFailure.ErrorFromRecipient>(event.reason)
     }
 
+    @Test
+    fun `receive invoice error -- amount too low`() = runSuspendTest {
+        // Alice and Bob use the same trampoline node.
+        val aliceOfferManager = OfferManager(TestConstants.Alice.nodeParams, aliceWalletParams, MutableSharedFlow(replay = 10), logger)
+        val bobOfferManager = OfferManager(TestConstants.Bob.nodeParams, aliceWalletParams, MutableSharedFlow(replay = 10), logger)
+        val offer = createOffer(aliceOfferManager, null)
+
+        // Bob sends an invoice request to Alice that pays less than the minimum htlc amount.
+        val payOffer = PayOffer(UUID.randomUUID(), randomKey(), null, 10.msat, offer, 20.seconds)
+        val (_, invoiceRequests) = bobOfferManager.requestInvoice(payOffer)
+        val (messageForAlice, _) = trampolineRelay(invoiceRequests.first(), aliceTrampolineKey)
+        // Alice sends an invoice error back to Bob.
+        val invoiceResponse = aliceOfferManager.receiveMessage(messageForAlice, listOf(), 0)
+        assertIs<OnionMessageAction.SendMessage>(invoiceResponse)
+        val (messageForBob, _) = trampolineRelay(invoiceResponse.message, aliceTrampolineKey)
+        assertNull(bobOfferManager.receiveMessage(messageForBob, listOf(), 0))
+        val event = bobOfferManager.eventsFlow.first()
+        assertIs<OfferNotPaid>(event)
+        assertIs<Bolt12InvoiceRequestFailure.ErrorFromRecipient>(event.reason)
+    }
+
     @Test
     fun `pay offer with payer note`() = runSuspendTest {
         // Alice and Bob use the same trampoline node.

src/commonTest/kotlin/fr/acinq/lightning/tests/TestConstants.kt

@@ -78,7 +78,7 @@ object TestConstants {
             ),
             maxHtlcValueInFlightMsat = 1_500_000_000L,
             maxAcceptedHtlcs = 100,
-            htlcMinimum = 0.msat,
+            htlcMinimum = 100.msat,
             toRemoteDelayBlocks = CltvExpiryDelta(144),
             maxToLocalDelayBlocks = CltvExpiryDelta(2048),
             feeBase = 100.msat,