From 307c4bd5c173b980573cd74e689e966633f50b3c Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Thu, 8 Feb 2024 12:17:04 +0100 Subject: [PATCH] Allow HTLC over-payment and over-expiry (#393) Recipients must allow HTLC values greater than what the onion payload contains, otherwise they can be trivially identified as being the final recipient. See https://github.com/lightning/bolts/pull/1032 for more details. --- .../payment/IncomingPaymentPacket.kt | 8 +-- .../IncomingPaymentHandlerTestsCommon.kt | 52 ++++++++++++++++--- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentPacket.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentPacket.kt index 840931f4d..28d8c9972 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentPacket.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentPacket.kt @@ -55,16 +55,16 @@ object IncomingPaymentPacket { private fun validate(add: UpdateAddHtlc, payload: PaymentOnion.FinalPayload): Either { return when { - add.amountMsat != payload.amount -> Either.Left(FinalIncorrectHtlcAmount(add.amountMsat)) - add.cltvExpiry != payload.expiry -> Either.Left(FinalIncorrectCltvExpiry(add.cltvExpiry)) + add.amountMsat < payload.amount -> Either.Left(FinalIncorrectHtlcAmount(add.amountMsat)) + add.cltvExpiry < payload.expiry -> Either.Left(FinalIncorrectCltvExpiry(add.cltvExpiry)) else -> Either.Right(payload) } } private fun validate(add: UpdateAddHtlc, outerPayload: PaymentOnion.FinalPayload, innerPayload: PaymentOnion.FinalPayload): Either { return when { - add.amountMsat != outerPayload.amount -> Either.Left(FinalIncorrectHtlcAmount(add.amountMsat)) - add.cltvExpiry != outerPayload.expiry -> Either.Left(FinalIncorrectCltvExpiry(add.cltvExpiry)) + add.amountMsat < outerPayload.amount -> Either.Left(FinalIncorrectHtlcAmount(add.amountMsat)) + add.cltvExpiry < outerPayload.expiry -> Either.Left(FinalIncorrectCltvExpiry(add.cltvExpiry)) // previous trampoline didn't forward the right expiry outerPayload.expiry != innerPayload.expiry -> Either.Left(FinalIncorrectCltvExpiry(add.cltvExpiry)) // previous trampoline didn't forward the right amount diff --git a/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt index edd097c20..16dcc18fe 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt @@ -655,16 +655,22 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { @Test fun `receive multipart payment with amount greater than total amount`() = runSuspendTest { val channelId = randomBytes32() - val (amount1, amount2, amount3) = listOf(100_000.msat, 60_000.msat, 40_000.msat) val requestedAmount = 180_000.msat - val totalAmount = amount1 + amount2 + amount3 val (paymentHandler, incomingPayment, paymentSecret) = createFixture(requestedAmount) + // The sender overpays at many different layers: + // - the invoice requests a payment of 180 000 msat + // - the sender announces a total amount of 190 000 msat + // - the sum of individual HTLC's onion amounts is 200 000 msat + // - the sum of individual HTLC's amounts is 205 000 msat + val totalAmount = 190_000.msat + val add1 = makeUpdateAddHtlc(3, channelId, paymentHandler, incomingPayment.paymentHash, makeMppPayload(100_000.msat, totalAmount, paymentSecret)) + val add2 = makeUpdateAddHtlc(5, channelId, paymentHandler, incomingPayment.paymentHash, makeMppPayload(60_000.msat, totalAmount, paymentSecret)).copy(amountMsat = 65_000.msat) + val add3 = makeUpdateAddHtlc(6, channelId, paymentHandler, incomingPayment.paymentHash, makeMppPayload(40_000.msat, totalAmount, paymentSecret)) // Step 1 of 2: // - Alice sends first 2 multipart htlcs to Bob. // - Bob doesn't accept the MPP set yet - listOf(Pair(3L, amount1), Pair(5L, amount2)).forEach { (id, amount) -> - val add = makeUpdateAddHtlc(id, channelId, paymentHandler, incomingPayment.paymentHash, makeMppPayload(amount, totalAmount, paymentSecret)) + listOf(add1, add2).forEach { add -> val result = paymentHandler.process(add, TestConstants.defaultBlockHeight) assertIs(result) assertTrue(result.actions.isEmpty()) @@ -674,8 +680,7 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { // - Alice sends third multipart htlc to Bob // - Bob now accepts the MPP set run { - val add = makeUpdateAddHtlc(6L, channelId, paymentHandler, incomingPayment.paymentHash, makeMppPayload(amount3, totalAmount, paymentSecret)) - val result = paymentHandler.process(add, TestConstants.defaultBlockHeight) + val result = paymentHandler.process(add3, TestConstants.defaultBlockHeight) assertIs(result) val expected = setOf( WrappedChannelCommand(channelId, ChannelCommand.Htlc.Settlement.Fulfill(3, incomingPayment.preimage, commit = true)), @@ -686,6 +691,27 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { } } + @Test + fun `receive normal single HTLC over-payment`() = runSuspendTest { + val (paymentHandler, incomingPayment, paymentSecret) = createFixture(150_000.msat) + val add = makeUpdateAddHtlc(0, randomBytes32(), paymentHandler, incomingPayment.paymentHash, makeSinglePartPayload(170_000.msat, paymentSecret)).copy(amountMsat = 175_000.msat) + val result = paymentHandler.process(add, TestConstants.defaultBlockHeight) + assertIs(result) + val expected = WrappedChannelCommand(add.channelId, ChannelCommand.Htlc.Settlement.Fulfill(add.id, incomingPayment.preimage, commit = true)) + assertEquals(setOf(expected), result.actions.toSet()) + } + + @Test + fun `receive normal single HTLC with greater expiry`() = runSuspendTest { + val (paymentHandler, incomingPayment, paymentSecret) = createFixture(defaultAmount) + val add = makeUpdateAddHtlc(0, randomBytes32(), paymentHandler, incomingPayment.paymentHash, makeSinglePartPayload(defaultAmount, paymentSecret)) + val addGreaterExpiry = add.copy(cltvExpiry = add.cltvExpiry + CltvExpiryDelta(6)) + val result = paymentHandler.process(addGreaterExpiry, TestConstants.defaultBlockHeight) + assertIs(result) + val expected = WrappedChannelCommand(add.channelId, ChannelCommand.Htlc.Settlement.Fulfill(add.id, incomingPayment.preimage, commit = true)) + assertEquals(setOf(expected), result.actions.toSet()) + } + @Test fun `reprocess duplicate htlcs`() = runSuspendTest { val (paymentHandler, incomingPayment, paymentSecret) = createFixture(defaultAmount) @@ -1102,8 +1128,8 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { paymentHandler.db.receivePayment( paidInvoice.paymentHash, receivedWith = listOf(IncomingPayment.ReceivedWith.NewChannel(amount = 15_000_000.msat, serviceFee = 1_000_000.msat, miningFee = 0.sat, channelId = randomBytes32(), txId = TxId(randomBytes32()), confirmedAt = null, lockedAt = null)), - receivedAt = 101 - ) // simulate incoming payment being paid before it expired + receivedAt = 101 // simulate incoming payment being paid before it expired + ) // create unexpired payment delay(100.milliseconds) @@ -1155,6 +1181,16 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { return UpdateAddHtlc(channelId, id, finalPayload.amount, paymentHash, finalPayload.expiry, packetAndSecrets.packet) } + private fun makeSinglePartPayload( + amount: MilliSatoshi, + paymentSecret: ByteVector32, + cltvExpiryDelta: CltvExpiryDelta = CltvExpiryDelta(144), + currentBlockHeight: Int = TestConstants.defaultBlockHeight + ): PaymentOnion.FinalPayload { + val expiry = cltvExpiryDelta.toCltvExpiry(currentBlockHeight.toLong()) + return PaymentOnion.FinalPayload.createSinglePartPayload(amount, expiry, paymentSecret, null) + } + private fun makeMppPayload( amount: MilliSatoshi, totalAmount: MilliSatoshi,