Skip to content

Commit

Permalink
Allow HTLC over-payment and over-expiry
Browse files Browse the repository at this point in the history 
Recipients must allow HTLC values greater than what the onion payload
contains, otherwise they can be trivially identified as being the
final recipient.

See lightning/bolts#1032 for more details.
  • Loading branch information
@t-bast
t-bast committed Oct 25, 2022
1 parent 3e1f291 commit ce405b7
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 24 deletions.
8 changes: 4 additions & 4 deletions src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentPacket.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,16 @@ object IncomingPaymentPacket {

private fun validate(add: UpdateAddHtlc, payload: PaymentOnion.FinalPayload): Either<FailureMessage, PaymentOnion.FinalPayload> {
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<FailureMessage, PaymentOnion.FinalPayload> {
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
Expand Down
88 changes: 68 additions & 20 deletions src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,12 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
assertEquals(expectedFees, result2.received.fees)

val (id1, id2) = result2.received.receivedWith.map { (it as IncomingPayment.ReceivedWith.NewChannel).id }
assertEquals(setOf(
makeReceivedWithNewChannel(payToOpenRequest1, uuid = id1),
makeReceivedWithNewChannel(payToOpenRequest2, uuid = id2)
), result2.received.receivedWith)
assertEquals(
setOf(
makeReceivedWithNewChannel(payToOpenRequest1, uuid = id1),
makeReceivedWithNewChannel(payToOpenRequest2, uuid = id2)
), result2.received.receivedWith
)

checkDbPayment(result2.incomingPayment, paymentHandler.db)
}
Expand All @@ -218,10 +220,12 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
assertEquals(expectedFees, result2.received.fees)

val (id1, id2) = result2.received.receivedWith.map { (it as IncomingPayment.ReceivedWith.NewChannel).id }
assertEquals(setOf(
makeReceivedWithNewChannel(payToOpenRequest1, uuid = id1),
makeReceivedWithNewChannel(payToOpenRequest2, uuid = id2)
), result2.received.receivedWith)
assertEquals(
setOf(
makeReceivedWithNewChannel(payToOpenRequest1, uuid = id1),
makeReceivedWithNewChannel(payToOpenRequest2, uuid = id2)
), result2.received.receivedWith
)

checkDbPayment(result2.incomingPayment, paymentHandler.db)
}
Expand Down Expand Up @@ -779,16 +783,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)
assertTrue { result is IncomingPaymentHandler.ProcessAddResult.Pending }
assertTrue { result.actions.isEmpty() }
Expand All @@ -798,8 +808,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)
assertTrue { result is IncomingPaymentHandler.ProcessAddResult.Accepted }
val expected = setOf(
WrappedChannelCommand(channelId, ChannelCommand.ExecuteCommand(CMD_FULFILL_HTLC(3, incomingPayment.preimage, commit = true))),
Expand All @@ -810,6 +819,29 @@ 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)

assertTrue { result is IncomingPaymentHandler.ProcessAddResult.Accepted }
val expected = WrappedChannelCommand(add.channelId, ChannelCommand.ExecuteCommand(CMD_FULFILL_HTLC(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)

assertTrue { result is IncomingPaymentHandler.ProcessAddResult.Accepted }
val expected = WrappedChannelCommand(add.channelId, ChannelCommand.ExecuteCommand(CMD_FULFILL_HTLC(add.id, incomingPayment.preimage, commit = true)))
assertEquals(setOf(expected), result.actions.toSet())
}

@Test
fun `reprocess duplicate htlcs`() = runSuspendTest {
val (paymentHandler, incomingPayment, paymentSecret) = createFixture(defaultAmount)
Expand Down Expand Up @@ -1219,15 +1251,21 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() {
val paymentHandler = IncomingPaymentHandler(TestConstants.Bob.nodeParams, TestConstants.Bob.walletParams, InMemoryPaymentsDb())

// create incoming payment that has expired and not been paid
val expiredInvoice = paymentHandler.createInvoice(randomBytes32(), defaultAmount, "expired", listOf(), expirySeconds = 3600,
timestampSeconds = 1)
val expiredInvoice = paymentHandler.createInvoice(
randomBytes32(), defaultAmount, "expired", listOf(), expirySeconds = 3600,
timestampSeconds = 1
)

// create incoming payment that has expired and been paid
delay(100)
val paidInvoice = paymentHandler.createInvoice(defaultPreimage, defaultAmount, "paid", listOf(), expirySeconds = 3600,
timestampSeconds = 100)
paymentHandler.db.receivePayment(paidInvoice.paymentHash, receivedWith = setOf(IncomingPayment.ReceivedWith.NewChannel(id = UUID.randomUUID(), amount = 15_000_000.msat, fees = 1_000_000.msat, null)),
receivedAt = 101) // simulate incoming payment being paid before it expired
val paidInvoice = paymentHandler.createInvoice(
defaultPreimage, defaultAmount, "paid", listOf(), expirySeconds = 3600,
timestampSeconds = 100
)
paymentHandler.db.receivePayment(
paidInvoice.paymentHash, receivedWith = setOf(IncomingPayment.ReceivedWith.NewChannel(id = UUID.randomUUID(), amount = 15_000_000.msat, fees = 1_000_000.msat, null)),
receivedAt = 101 // simulate incoming payment being paid before it expired
)

// create unexpired payment
delay(100)
Expand Down Expand Up @@ -1277,6 +1315,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,
Expand Down

0 comments on commit ce405b7

Please sign in to comment.