Skip to content

Commit

Permalink
Allow HTLC over-payment and over-expiry (#393)
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 authored Feb 8, 2024
1 parent 85395a3 commit 307c4bd
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 12 deletions.
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
Original file line number Diff line number Diff line change
Expand Up @@ -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<IncomingPaymentHandler.ProcessAddResult.Pending>(result)
assertTrue(result.actions.isEmpty())
Expand All @@ -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<IncomingPaymentHandler.ProcessAddResult.Accepted>(result)
val expected = setOf(
WrappedChannelCommand(channelId, ChannelCommand.Htlc.Settlement.Fulfill(3, incomingPayment.preimage, commit = true)),
Expand All @@ -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<IncomingPaymentHandler.ProcessAddResult.Accepted>(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<IncomingPaymentHandler.ProcessAddResult.Accepted>(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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 307c4bd

Please sign in to comment.