Skip to content

Commit

Permalink
Include min_final_cltv_expiry_delta in blinded paths (#694)
Browse files Browse the repository at this point in the history 
* Include `min_final_cltv_expiry_delta` in blinded paths

If a payer uses the current block height as the expiry for the payments
they send to us, we will reject it because we need to have a few blocks
to potentially force-close and get our funds back safely.

With blinded paths, we don't need to rely on payers to add a safety
margin: we can directly encode it in the blinded path's total expiry
delta.

* Correctly set `htlc_minimum` in blinded paths

We must use the most restrictive value for `htlc_minimum`, not the
least restrictive one. When we don't have the remote channel update
we should use the value from our node params.

The `htlc_maximum` is allowed to be twice the payment amount according
to the BOLTs: we reflect that in the blinded path data.

* Reject `invoice_request`s below `htlc_minimum`

We reject invoice requests that specify an amount below our
`htlc_minimum`. It doesn't make sense to attempt that payment,
it won't be relayed by our peer.
  • Loading branch information
@t-bast
t-bast committed Aug 23, 2024
1 parent fb0ae19 commit 90cf70b
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 12 deletions.
28 changes: 21 additions & 7 deletions src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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" }
Expand All @@ -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) {
Expand All @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions src/commonMain/kotlin/fr/acinq/lightning/wire/OfferTypes.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 1 addition & 3 deletions src/commonTest/kotlin/fr/acinq/lightning/channel/states/NormalTestsCommon.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
35 changes: 34 additions & 1 deletion src/commonTest/kotlin/fr/acinq/lightning/payment/OfferManagerTestsCommon.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/commonTest/kotlin/fr/acinq/lightning/tests/TestConstants.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 90cf70b

Please sign in to comment.