diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt index 2b04369ff..45af0851c 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt @@ -132,6 +132,8 @@ class OfferManager(val nodeParams: NodeParams, val walletParams: WalletParams, v private fun receiveInvoiceRequest(decrypted: OnionMessages.DecryptedMessage, remoteChannelUpdates: List, currentBlockHeight: Int): OnionMessageAction.SendMessage? { val offer = localOffers[decrypted.pathId]!! val request = decrypted.content.records.get()?.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" } @@ -149,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) { @@ -170,8 +176,7 @@ class OfferManager(val nodeParams: NodeParams, val walletParams: WalletParams, v // 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, - // We must use the most restrictive minimum HTLC value between local and remote. - minHtlc = (listOf(nodeParams.htlcMinimum) + remoteChannelUpdates.map { it.htlcMinimumMsat }).max(), + minHtlc = minHtlc, // Payments are allowed to overpay at most two times the invoice amount. maxHtlc = amount * 2, allowedFeatures = Features.empty diff --git a/src/commonMain/kotlin/fr/acinq/lightning/wire/OfferTypes.kt b/src/commonMain/kotlin/fr/acinq/lightning/wire/OfferTypes.kt index ad5fdfdc1..e361b8e17 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/wire/OfferTypes.kt +++ b/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, diff --git a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/NormalTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/NormalTestsCommon.kt index f80517101..4b5335731 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/NormalTestsCommon.kt +++ b/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() - 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) } diff --git a/src/commonTest/kotlin/fr/acinq/lightning/payment/OfferManagerTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/payment/OfferManagerTestsCommon.kt index 851eedddf..5e2db8d8b 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/payment/OfferManagerTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/payment/OfferManagerTestsCommon.kt @@ -206,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) @@ -226,6 +226,27 @@ class OfferManagerTestsCommon : LightningTestSuite() { assertIs(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(invoiceResponse) + val (messageForBob, _) = trampolineRelay(invoiceResponse.message, aliceTrampolineKey) + assertNull(bobOfferManager.receiveMessage(messageForBob, listOf(), 0)) + val event = bobOfferManager.eventsFlow.first() + assertIs(event) + assertIs(event.reason) + } + @Test fun `pay offer with payer note`() = runSuspendTest { // Alice and Bob use the same trampoline node. diff --git a/src/commonTest/kotlin/fr/acinq/lightning/tests/TestConstants.kt b/src/commonTest/kotlin/fr/acinq/lightning/tests/TestConstants.kt index 5263c8ba4..f15e9d22f 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/tests/TestConstants.kt +++ b/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,