From c8a290f8c1b657f82cd901d21619270257c55eac Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 15 Oct 2024 05:10:58 +0200 Subject: [PATCH 1/7] Increase default invoice expiry We increase the default expiry of Bolt 11 and Bolt 12 invoices: they are now valid for 1 day. This ensures that if a payment takes a while to reach us (most likely because we've been offline for a few hours), the invoice is still valid when we come back online to receive the payment. We also increase our `min_final_expiry_delta` to 144 blocks and our `fulfill_safety_before_timeout` to 12 blocks. This gives us more time to publish our commitment transaction and HTLC-success transaction in case our peer doesn't acknowledge a preimage we reveal, so that we can claim the payment on-chain. --- .../kotlin/fr/acinq/lightning/NodeParams.kt | 14 ++++++++------ .../fr/acinq/lightning/payment/Bolt11Invoice.kt | 4 ++-- .../payment/OutgoingPaymentHandlerTestsCommon.kt | 2 +- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt b/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt index 2778d28f5..2697c7da5 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt @@ -7,12 +7,10 @@ import fr.acinq.lightning.blockchain.fee.FeerateTolerance import fr.acinq.lightning.blockchain.fee.OnChainFeeConf import fr.acinq.lightning.crypto.KeyManager import fr.acinq.lightning.logging.LoggerFactory -import fr.acinq.lightning.payment.Bolt11Invoice import fr.acinq.lightning.payment.LiquidityPolicy import fr.acinq.lightning.utils.msat import fr.acinq.lightning.utils.sat import fr.acinq.lightning.utils.toMilliSatoshi -import fr.acinq.lightning.wire.LiquidityAds import fr.acinq.lightning.wire.OfferTypes import io.ktor.utils.io.charsets.* import io.ktor.utils.io.core.* @@ -21,6 +19,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.asSharedFlow import kotlin.time.Duration +import kotlin.time.Duration.Companion.hours import kotlin.time.Duration.Companion.minutes import kotlin.time.Duration.Companion.seconds @@ -217,7 +216,7 @@ data class NodeParams( maxHtlcValueInFlightMsat = 20_000_000_000L, maxAcceptedHtlcs = 6, expiryDeltaBlocks = CltvExpiryDelta(144), - fulfillSafetyBeforeTimeoutBlocks = CltvExpiryDelta(6), + fulfillSafetyBeforeTimeoutBlocks = CltvExpiryDelta(12), checkHtlcTimeoutAfterStartupDelay = 30.seconds, checkHtlcTimeoutInterval = 10.seconds, htlcMinimum = 1000.msat, @@ -232,7 +231,7 @@ data class NodeParams( mppAggregationWindow = 60.seconds, maxPaymentAttempts = 5, zeroConfPeers = emptySet(), - paymentRecipientExpiryParams = RecipientCltvExpiryParams(CltvExpiryDelta(75), CltvExpiryDelta(200)), + paymentRecipientExpiryParams = RecipientCltvExpiryParams(CltvExpiryDelta(72), CltvExpiryDelta(144)), liquidityPolicy = MutableStateFlow( LiquidityPolicy.Auto( inboundLiquidityTarget = null, @@ -243,9 +242,12 @@ data class NodeParams( maxAllowedFeeCredit = 0.msat ) ), - minFinalCltvExpiryDelta = Bolt11Invoice.DEFAULT_MIN_FINAL_EXPIRY_DELTA, + // We use a long expiry delta here for a few reasons: + // - we want to ensure we're able to get HTLC-success txs confirmed if our peer ignores our preimage + // - we may be offline for a while, so we want our peer to be able to hold HTLCs and forward them when we come back online + minFinalCltvExpiryDelta = CltvExpiryDelta(144), maxFinalCltvExpiryDelta = CltvExpiryDelta(360), - bolt12invoiceExpiry = 60.seconds, + bolt12invoiceExpiry = 24.hours, ) /** diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/Bolt11Invoice.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/Bolt11Invoice.kt index b0cebd6ee..d24e49d34 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/payment/Bolt11Invoice.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/payment/Bolt11Invoice.kt @@ -105,8 +105,8 @@ data class Bolt11Invoice( } companion object { - const val DEFAULT_EXPIRY_SECONDS = 3600 - val DEFAULT_MIN_FINAL_EXPIRY_DELTA = CltvExpiryDelta(18) + const val DEFAULT_EXPIRY_SECONDS = 24 * 3600 // 1 day + val DEFAULT_MIN_FINAL_EXPIRY_DELTA = CltvExpiryDelta(18) // default value from Bolt 11 private val prefixes = mapOf( Chain.Regtest to "lnbcrt", diff --git a/src/commonTest/kotlin/fr/acinq/lightning/payment/OutgoingPaymentHandlerTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/payment/OutgoingPaymentHandlerTestsCommon.kt index 1d3bbba2f..01cb3c0ed 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/payment/OutgoingPaymentHandlerTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/payment/OutgoingPaymentHandlerTestsCommon.kt @@ -470,7 +470,7 @@ class OutgoingPaymentHandlerTestsCommon : LightningTestSuite() { assertIs(payloadB) assertEquals(add.amount, payloadB.amount) assertEquals(300_000.msat, payloadB.totalAmount) - assertEquals(CltvExpiry(TestConstants.defaultBlockHeight.toLong()) + Bolt11Invoice.DEFAULT_MIN_FINAL_EXPIRY_DELTA, payloadB.expiry) + assertEquals(CltvExpiry(TestConstants.defaultBlockHeight.toLong()) + TestConstants.Alice.nodeParams.minFinalCltvExpiryDelta, payloadB.expiry) assertEquals(invoice.paymentSecret, payloadB.paymentSecret) } From da123a8215ff4e7c11ec3b1196fbb1f994e3dca8 Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 15 Oct 2024 05:52:58 +0200 Subject: [PATCH 2/7] Refactor minFinalCltvExpiry This commit is purely a refactoring (without any behavior change) of the minimum cltv expiry we require on incoming payments. --- .../payment/IncomingPaymentHandler.kt | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt index ccc4f050f..5b8c713a4 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt @@ -159,7 +159,14 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { return process(Either.Left(htlc), remoteFeatures, currentBlockHeight, currentFeerate, remoteFundingRates, currentFeeCredit) } - private suspend fun process(htlc: Either, remoteFeatures: Features, currentBlockHeight: Int, currentFeerate: FeeratePerKw, remoteFundingRates: LiquidityAds.WillFundRates?, currentFeeCredit: MilliSatoshi): ProcessAddResult { + private suspend fun process( + htlc: Either, + remoteFeatures: Features, + currentBlockHeight: Int, + currentFeerate: FeeratePerKw, + remoteFundingRates: LiquidityAds.WillFundRates?, + currentFeeCredit: MilliSatoshi + ): ProcessAddResult { // There are several checks we could perform *before* decrypting the onion. // But we need to carefully handle which error message is returned to prevent information leakage, so we always peel the onion first. return when (val res = toPaymentPart(privateKey, htlc)) { @@ -169,7 +176,14 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { } /** Main payment processing, that handles payment parts. */ - private suspend fun processPaymentPart(paymentPart: PaymentPart, remoteFeatures: Features, currentBlockHeight: Int, currentFeerate: FeeratePerKw, remoteFundingRates: LiquidityAds.WillFundRates?, currentFeeCredit: MilliSatoshi): ProcessAddResult { + private suspend fun processPaymentPart( + paymentPart: PaymentPart, + remoteFeatures: Features, + currentBlockHeight: Int, + currentFeerate: FeeratePerKw, + remoteFundingRates: LiquidityAds.WillFundRates?, + currentFeeCredit: MilliSatoshi + ): ProcessAddResult { val logger = MDCLogger(logger.logger, staticMdc = paymentPart.mdc() + ("feeCredit" to currentFeeCredit)) when (paymentPart) { is HtlcPart -> logger.info { "processing htlc part expiry=${paymentPart.htlc.cltvExpiry}" } @@ -475,8 +489,8 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { logger.warning { "invalid amount (overpayment): ${paymentPart.totalAmount}, expected: ${incomingPayment.origin.paymentRequest.amount}" } Either.Left(rejectPaymentPart(privateKey, paymentPart, incomingPayment, currentBlockHeight)) } - paymentPart is HtlcPart && paymentPart.htlc.cltvExpiry < minFinalCltvExpiry(incomingPayment.origin.paymentRequest, currentBlockHeight) -> { - logger.warning { "payment with expiry too small: ${paymentPart.htlc.cltvExpiry}, min is ${minFinalCltvExpiry(incomingPayment.origin.paymentRequest, currentBlockHeight)}" } + paymentPart is HtlcPart && paymentPart.htlc.cltvExpiry < minFinalCltvExpiry(nodeParams, incomingPayment.origin, currentBlockHeight) -> { + logger.warning { "payment with expiry too small: ${paymentPart.htlc.cltvExpiry}, min is ${minFinalCltvExpiry(nodeParams, incomingPayment.origin, currentBlockHeight)}" } Either.Left(rejectPaymentPart(privateKey, paymentPart, incomingPayment, currentBlockHeight)) } else -> Either.Right(incomingPayment) @@ -504,8 +518,8 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { logger.warning { "invalid amount (underpayment): ${paymentPart.totalAmount}, expected: ${metadata.amount}" } Either.Left(rejectPaymentPart(privateKey, paymentPart, incomingPayment, currentBlockHeight)) } - paymentPart is HtlcPart && paymentPart.htlc.cltvExpiry < nodeParams.minFinalCltvExpiryDelta.toCltvExpiry(currentBlockHeight.toLong()) -> { - logger.warning { "payment with expiry too small: ${paymentPart.htlc.cltvExpiry}, min is ${nodeParams.minFinalCltvExpiryDelta.toCltvExpiry(currentBlockHeight.toLong())}" } + paymentPart is HtlcPart && paymentPart.htlc.cltvExpiry < minFinalCltvExpiry(nodeParams, incomingPayment.origin, currentBlockHeight) -> { + logger.warning { "payment with expiry too small: ${paymentPart.htlc.cltvExpiry}, min is ${minFinalCltvExpiry(nodeParams, incomingPayment.origin, currentBlockHeight)}" } Either.Left(rejectPaymentPart(privateKey, paymentPart, incomingPayment, currentBlockHeight)) } metadata.createdAtMillis + nodeParams.bolt12invoiceExpiry.inWholeMilliseconds < currentTimestampMillis() && incomingPayment.received == null -> { @@ -612,9 +626,12 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { return SendOnTheFlyFundingMessage(msg) } - private fun minFinalCltvExpiry(paymentRequest: Bolt11Invoice, currentBlockHeight: Int): CltvExpiry { - val minFinalCltvExpiryDelta = paymentRequest.minFinalExpiryDelta ?: Bolt11Invoice.DEFAULT_MIN_FINAL_EXPIRY_DELTA - return minFinalCltvExpiryDelta.toCltvExpiry(currentBlockHeight.toLong()) + private fun minFinalCltvExpiry(nodeParams: NodeParams, origin: IncomingPayment.Origin, currentBlockHeight: Int): CltvExpiry { + val minFinalExpiryDelta = when (origin) { + is IncomingPayment.Origin.Invoice -> origin.paymentRequest.minFinalExpiryDelta ?: Bolt11Invoice.DEFAULT_MIN_FINAL_EXPIRY_DELTA + else -> nodeParams.minFinalCltvExpiryDelta + } + return minFinalExpiryDelta.toCltvExpiry(currentBlockHeight.toLong()) } } From ccfdc162de685be1a2f8d0794f32d6ef266f4f4f Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 15 Oct 2024 05:55:19 +0200 Subject: [PATCH 3/7] Add failing test for delayed HTLC relay When using on-the-fly funding, we have to create and publish an on-chain transaction after accepting `will_add_htlc`, before we can relay the matching HTLCs. This can create issues when there isn't enough margin in the `min_final_expiry_delta` parameter where the HTLC is rejected. We add failing tests that demonstrate this behavior. --- .../IncomingPaymentHandlerTestsCommon.kt | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt index dd2152f6b..9d0a93807 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt @@ -923,7 +923,10 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { val fundingFee = purchase.fundingFee.copy(amount = 0.msat) val htlc = makeUpdateAddHtlc(7, channelId, paymentHandler, incomingPayment.paymentHash, makeMppPayload(amount, amount, paymentSecret), fundingFee = fundingFee) assertEquals(htlc.amountMsat, amount) - val result = paymentHandler.process(htlc, Features.empty, TestConstants.defaultBlockHeight, TestConstants.feeratePerKw, TestConstants.fundingRates) + // Before relaying the payment, we have created an on-chain transaction, which took some time. + // We are now closer to the min_final_expiry_delta, but we've committed to accepting this HTLC already. + val currentBlockHeight = TestConstants.defaultBlockHeight + 24 + val result = paymentHandler.process(htlc, Features.empty, currentBlockHeight, TestConstants.feeratePerKw, TestConstants.fundingRates) assertIs(result) val (expectedActions, expectedReceivedWith) = setOf( // @formatter:off @@ -1661,18 +1664,31 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { val payment = InboundLiquidityOutgoingPayment(UUID.randomUUID(), channelId, TxId(randomBytes32()), 500.sat, purchase, 0, null, null) paymentHandler.db.addOutgoingPayment(payment) - val cltvExpiry = TestConstants.Bob.nodeParams.minFinalCltvExpiryDelta.toCltvExpiry(TestConstants.defaultBlockHeight.toLong()) - val (finalPayload, route) = makeBlindedPayload(TestConstants.Bob.nodeParams.nodeId, defaultAmount, defaultAmount, cltvExpiry, preimage = preimage) - val add = makeUpdateAddHtlc(0, randomBytes32(), paymentHandler, paymentHash, finalPayload, route.blindingKey, payment.fundingFee) - val result = paymentHandler.process(add, Features.empty, TestConstants.defaultBlockHeight, TestConstants.feeratePerKw, TestConstants.fundingRates) - assertIs(result) - val fulfill = ChannelCommand.Htlc.Settlement.Fulfill(add.id, preimage, commit = true) - assertEquals(setOf(WrappedChannelCommand(add.channelId, fulfill)), result.actions.toSet()) - assertEquals(result.incomingPayment.received, result.received) - assertEquals(defaultAmount - payment.fundingFee.amount, result.received.amount) - val receivedWith = IncomingPayment.ReceivedWith.LightningPayment(defaultAmount - payment.fundingFee.amount, add.channelId, 0, payment.fundingFee) - assertEquals(listOf(receivedWith), result.received.receivedWith) - checkDbPayment(result.incomingPayment, paymentHandler.db) + // Before relaying the payment, we must create an on-chain transaction, which may take some time. + // We may thus be closer to the min_final_expiry_delta, but we've committed to accepting this HTLC already. + // As long as the expiry is greater than twice our fulfill_safety_before_timeout, we will accept it. + run { + val cltvExpiry = TestConstants.Bob.nodeParams.fulfillSafetyBeforeTimeoutBlocks.toCltvExpiry(TestConstants.defaultBlockHeight.toLong()) + val (finalPayload, route) = makeBlindedPayload(TestConstants.Bob.nodeParams.nodeId, defaultAmount, defaultAmount, cltvExpiry, preimage = preimage) + val add = makeUpdateAddHtlc(0, randomBytes32(), paymentHandler, paymentHash, finalPayload, route.blindingKey, payment.fundingFee) + val result = paymentHandler.process(add, Features.empty, TestConstants.defaultBlockHeight, TestConstants.feeratePerKw, TestConstants.fundingRates) + assertIs(result) + } + run { + assertTrue((TestConstants.Bob.nodeParams.fulfillSafetyBeforeTimeoutBlocks * 2) < TestConstants.Bob.nodeParams.minFinalCltvExpiryDelta) + val cltvExpiry = (TestConstants.Bob.nodeParams.fulfillSafetyBeforeTimeoutBlocks * 2).toCltvExpiry(TestConstants.defaultBlockHeight.toLong()) + val (finalPayload, route) = makeBlindedPayload(TestConstants.Bob.nodeParams.nodeId, defaultAmount, defaultAmount, cltvExpiry, preimage = preimage) + val add = makeUpdateAddHtlc(0, randomBytes32(), paymentHandler, paymentHash, finalPayload, route.blindingKey, payment.fundingFee) + val result = paymentHandler.process(add, Features.empty, TestConstants.defaultBlockHeight, TestConstants.feeratePerKw, TestConstants.fundingRates) + assertIs(result) + val fulfill = ChannelCommand.Htlc.Settlement.Fulfill(add.id, preimage, commit = true) + assertEquals(setOf(WrappedChannelCommand(add.channelId, fulfill)), result.actions.toSet()) + assertEquals(result.incomingPayment.received, result.received) + assertEquals(defaultAmount - payment.fundingFee.amount, result.received.amount) + val receivedWith = IncomingPayment.ReceivedWith.LightningPayment(defaultAmount - payment.fundingFee.amount, add.channelId, 0, payment.fundingFee) + assertEquals(listOf(receivedWith), result.received.receivedWith) + checkDbPayment(result.incomingPayment, paymentHandler.db) + } } @Test From 2461be31171bacef25f82436ebdb76ca4d9f5db0 Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 15 Oct 2024 06:14:25 +0200 Subject: [PATCH 4/7] Fix `min_final_expiry_delta` failures for on-the-fly funding We fix the failing tests added in the previous commit, by allowing on-the-fly HTLCs that are below `min_final_expiry_delta` as long as their expiry is greater than twice our `fulfill_safety_before_timeout` parameter. This is what we want to guarantee: that we have enough time to potentially force-close and claim HTLC-success transactions. --- .../kotlin/fr/acinq/lightning/CltvExpiry.kt | 2 ++ .../kotlin/fr/acinq/lightning/NodeParams.kt | 3 +++ .../payment/IncomingPaymentHandler.kt | 23 ++++++++++++++----- .../acinq/lightning/wire/LightningMessages.kt | 1 + 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/CltvExpiry.kt b/src/commonMain/kotlin/fr/acinq/lightning/CltvExpiry.kt index ff13d95f9..bd1492707 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/CltvExpiry.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/CltvExpiry.kt @@ -40,7 +40,9 @@ data class CltvExpiryDelta(private val underlying: Int) : Comparable get() = _nodeEvents.asSharedFlow() init { + // Verify required features are set and obsolete features aren't set. require(features.hasFeature(Feature.VariableLengthOnion, FeatureSupport.Mandatory)) { "${Feature.VariableLengthOnion.rfcName} should be mandatory" } require(features.hasFeature(Feature.PaymentSecret, FeatureSupport.Mandatory)) { "${Feature.PaymentSecret.rfcName} should be mandatory" } require(features.hasFeature(Feature.ChannelType, FeatureSupport.Mandatory)) { "${Feature.ChannelType.rfcName} should be mandatory" } @@ -175,6 +176,8 @@ data class NodeParams( require(!features.hasFeature(Feature.PayToOpenClient)) { "${Feature.PayToOpenClient.rfcName} has been deprecated" } require(!features.hasFeature(Feature.PayToOpenProvider)) { "${Feature.PayToOpenProvider.rfcName} has been deprecated" } Features.validateFeatureGraph(features) + // Verify expiry parameters are consistent with each other. + require((fulfillSafetyBeforeTimeoutBlocks * 2) < minFinalCltvExpiryDelta) { "min_final_expiry_delta must be at least twice as long as fulfill_safety_before_timeout_blocks" } } /** diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt index 5b8c713a4..d5ac9b3fb 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt @@ -489,8 +489,8 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { logger.warning { "invalid amount (overpayment): ${paymentPart.totalAmount}, expected: ${incomingPayment.origin.paymentRequest.amount}" } Either.Left(rejectPaymentPart(privateKey, paymentPart, incomingPayment, currentBlockHeight)) } - paymentPart is HtlcPart && paymentPart.htlc.cltvExpiry < minFinalCltvExpiry(nodeParams, incomingPayment.origin, currentBlockHeight) -> { - logger.warning { "payment with expiry too small: ${paymentPart.htlc.cltvExpiry}, min is ${minFinalCltvExpiry(nodeParams, incomingPayment.origin, currentBlockHeight)}" } + paymentPart is HtlcPart && paymentPart.htlc.cltvExpiry < minFinalCltvExpiry(nodeParams, paymentPart, incomingPayment.origin, currentBlockHeight) -> { + logger.warning { "payment with expiry too small: ${paymentPart.htlc.cltvExpiry}, min is ${minFinalCltvExpiry(nodeParams, paymentPart, incomingPayment.origin, currentBlockHeight)}" } Either.Left(rejectPaymentPart(privateKey, paymentPart, incomingPayment, currentBlockHeight)) } else -> Either.Right(incomingPayment) @@ -518,8 +518,8 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { logger.warning { "invalid amount (underpayment): ${paymentPart.totalAmount}, expected: ${metadata.amount}" } Either.Left(rejectPaymentPart(privateKey, paymentPart, incomingPayment, currentBlockHeight)) } - paymentPart is HtlcPart && paymentPart.htlc.cltvExpiry < minFinalCltvExpiry(nodeParams, incomingPayment.origin, currentBlockHeight) -> { - logger.warning { "payment with expiry too small: ${paymentPart.htlc.cltvExpiry}, min is ${minFinalCltvExpiry(nodeParams, incomingPayment.origin, currentBlockHeight)}" } + paymentPart is HtlcPart && paymentPart.htlc.cltvExpiry < minFinalCltvExpiry(nodeParams, paymentPart, incomingPayment.origin, currentBlockHeight) -> { + logger.warning { "payment with expiry too small: ${paymentPart.htlc.cltvExpiry}, min is ${minFinalCltvExpiry(nodeParams, paymentPart, incomingPayment.origin, currentBlockHeight)}" } Either.Left(rejectPaymentPart(privateKey, paymentPart, incomingPayment, currentBlockHeight)) } metadata.createdAtMillis + nodeParams.bolt12invoiceExpiry.inWholeMilliseconds < currentTimestampMillis() && incomingPayment.received == null -> { @@ -626,12 +626,23 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { return SendOnTheFlyFundingMessage(msg) } - private fun minFinalCltvExpiry(nodeParams: NodeParams, origin: IncomingPayment.Origin, currentBlockHeight: Int): CltvExpiry { + private fun minFinalCltvExpiry(nodeParams: NodeParams, paymentPart: PaymentPart, origin: IncomingPayment.Origin, currentBlockHeight: Int): CltvExpiry { val minFinalExpiryDelta = when (origin) { is IncomingPayment.Origin.Invoice -> origin.paymentRequest.minFinalExpiryDelta ?: Bolt11Invoice.DEFAULT_MIN_FINAL_EXPIRY_DELTA else -> nodeParams.minFinalCltvExpiryDelta } - return minFinalExpiryDelta.toCltvExpiry(currentBlockHeight.toLong()) + return when { + paymentPart is HtlcPart && paymentPart.htlc.usesOnTheFlyFunding -> { + // This HTLC is using on-the-fly funding, so it may have been forwarded with a delay + // because an on-chain transaction was required. + // If the expiry is now below our min_final_expiry_delta, we sill want to accept this + // HTLC if we can do so safely, because we need to pay funding fees by fulfilling it. + // As long as we have time to publish a force-close, it is safe to accept the HTLC. + val overrideFinalExpiryDelta = minFinalExpiryDelta.min(nodeParams.fulfillSafetyBeforeTimeoutBlocks * 2) + overrideFinalExpiryDelta.toCltvExpiry(currentBlockHeight.toLong()) + } + else -> minFinalExpiryDelta.toCltvExpiry(currentBlockHeight.toLong()) + } } } diff --git a/src/commonMain/kotlin/fr/acinq/lightning/wire/LightningMessages.kt b/src/commonMain/kotlin/fr/acinq/lightning/wire/LightningMessages.kt index 91f7bb76c..cceec2df5 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/wire/LightningMessages.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/wire/LightningMessages.kt @@ -1095,6 +1095,7 @@ data class UpdateAddHtlc( val blinding: PublicKey? = tlvStream.get()?.publicKey val fundingFee: LiquidityAds.FundingFee? = tlvStream.get()?.fee + val usesOnTheFlyFunding: Boolean = fundingFee != null override fun write(out: Output) { LightningCodecs.writeBytes(channelId, out) From 7882f8e185710ad61c4e3e133e8d05fb2cb31c5f Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 15 Oct 2024 15:53:37 +0200 Subject: [PATCH 5/7] Remove `max_final_expiry_delta` parameter This field was not at all doing what was documented: it doesn't make sense to reject a payment because its `min_final_expiry_delta` is too large, we only care about the total path's expiry delta which cannot exceed 2016 blocks. We were using this for the blinded path expiry, but the spec says we must rather use the invoice expiry, roughly converted to a block height. We also reorder `minFinalExpiryDelta` to put it closer to other related parameters. --- .../kotlin/fr/acinq/lightning/NodeParams.kt | 19 ++++++++----------- .../acinq/lightning/payment/OfferManager.kt | 4 +++- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt b/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt index a74c28281..0a2720f1f 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt @@ -104,9 +104,10 @@ data class RecipientCltvExpiryParams(val min: CltvExpiryDelta, val max: CltvExpi * @param maxHtlcValueInFlightMsat cap on the total value of pending HTLCs in a channel: this lets us limit our exposure to HTLCs risk. * @param maxAcceptedHtlcs cap on the number of pending HTLCs in a channel: this lets us limit our exposure to HTLCs risk. * @param expiryDeltaBlocks cltv-expiry-delta used in our channel_update: since our channels are private and we don't relay payments, this will be basically ignored. + * @param minFinalCltvExpiryDelta cltv-expiry-delta that we require when receiving a payment. * @param fulfillSafetyBeforeTimeoutBlocks number of blocks necessary to react to a malicious peer that doesn't acknowledge and sign our HTLC preimages. * @param checkHtlcTimeoutAfterStartupDelay delay before we check for timed out HTLCs in our channels after a wallet restart. - * @param htlcMinimum minimum accepted htlc value. + * @param htlcMinimum minimum accepted HTLC value. * @param toRemoteDelayBlocks number of blocks our peer will have to wait before they get their main output back in case they force-close a channel. * @param maxToLocalDelayBlocks maximum number of blocks we will have to wait before we get our main output back in case we force-close a channel. * @param minDepthBlocks minimum depth of a transaction before we consider it safely confirmed. @@ -115,13 +116,11 @@ data class RecipientCltvExpiryParams(val min: CltvExpiryDelta, val max: CltvExpi * @param pingInterval delay between ping messages. * @param initialRandomReconnectDelay delay before which we reconnect to our peers (will be randomized based on this value). * @param maxReconnectInterval maximum delay between reconnection attempts. - * @param mppAggregationWindow amount of time we will wait to receive all parts of a multi-part payment. + * @param mppAggregationWindow amount of time we will wait to receive all parts of a multipart payment. * @param maxPaymentAttempts maximum number of retries when attempting an outgoing payment. * @param paymentRecipientExpiryParams configure the expiry delta used for the final node when sending payments. * @param zeroConfPeers list of peers with whom we use zero-conf (note that this is a strong trust assumption). * @param liquidityPolicy fee policy for liquidity events, can be modified at any time. - * @param minFinalCltvExpiryDelta cltv-expiry-delta that we require when receiving a payment. - * @param maxFinalCltvExpiryDelta maximum cltv-expiry-delta that we accept when receiving a payment. * @param bolt12invoiceExpiry duration for which bolt12 invoices that we create are valid. */ data class NodeParams( @@ -135,6 +134,7 @@ data class NodeParams( val maxHtlcValueInFlightMsat: Long, val maxAcceptedHtlcs: Int, val expiryDeltaBlocks: CltvExpiryDelta, + val minFinalCltvExpiryDelta: CltvExpiryDelta, val fulfillSafetyBeforeTimeoutBlocks: CltvExpiryDelta, val checkHtlcTimeoutAfterStartupDelay: Duration, val checkHtlcTimeoutInterval: Duration, @@ -152,8 +152,6 @@ data class NodeParams( val paymentRecipientExpiryParams: RecipientCltvExpiryParams, val zeroConfPeers: Set, val liquidityPolicy: MutableStateFlow, - val minFinalCltvExpiryDelta: CltvExpiryDelta, - val maxFinalCltvExpiryDelta: CltvExpiryDelta, val bolt12invoiceExpiry: Duration, ) { val nodePrivateKey get() = keyManager.nodeKeys.nodeKey.privateKey @@ -219,6 +217,10 @@ data class NodeParams( maxHtlcValueInFlightMsat = 20_000_000_000L, maxAcceptedHtlcs = 6, expiryDeltaBlocks = CltvExpiryDelta(144), + // We use a long expiry delta here for a few reasons: + // - we want to ensure we're able to get HTLC-success txs confirmed if our peer ignores our preimage + // - we may be offline for a while, so we want our peer to be able to hold HTLCs and forward them when we come back online + minFinalCltvExpiryDelta = CltvExpiryDelta(144), fulfillSafetyBeforeTimeoutBlocks = CltvExpiryDelta(12), checkHtlcTimeoutAfterStartupDelay = 30.seconds, checkHtlcTimeoutInterval = 10.seconds, @@ -245,11 +247,6 @@ data class NodeParams( maxAllowedFeeCredit = 0.msat ) ), - // We use a long expiry delta here for a few reasons: - // - we want to ensure we're able to get HTLC-success txs confirmed if our peer ignores our preimage - // - we may be offline for a while, so we want our peer to be able to hold HTLCs and forward them when we come back online - minFinalCltvExpiryDelta = CltvExpiryDelta(144), - maxFinalCltvExpiryDelta = CltvExpiryDelta(360), bolt12invoiceExpiry = 24.hours, ) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt index 45af0851c..843bbbf91 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt @@ -181,11 +181,13 @@ class OfferManager(val nodeParams: NodeParams, val walletParams: WalletParams, v maxHtlc = amount * 2, allowedFeatures = Features.empty ) + // We assume 10 minutes between each block to convert the invoice expiry to a CLTV expiry for the blinded path. + val pathExpiry = (paymentInfo.cltvExpiryDelta + (nodeParams.bolt12invoiceExpiry.inWholeMinutes.toInt() / 10)).toCltvExpiry(currentBlockHeight.toLong()) val remoteNodePayload = RouteBlindingEncryptedData( TlvStream( RouteBlindingEncryptedDataTlv.OutgoingNodeId(EncodedNodeId.WithPublicKey.Wallet(nodeParams.nodeId)), RouteBlindingEncryptedDataTlv.PaymentRelay(cltvExpiryDelta, paymentInfo.feeProportionalMillionths, paymentInfo.feeBase), - RouteBlindingEncryptedDataTlv.PaymentConstraints((paymentInfo.cltvExpiryDelta + nodeParams.maxFinalCltvExpiryDelta).toCltvExpiry(currentBlockHeight.toLong()), paymentInfo.minHtlc) + RouteBlindingEncryptedDataTlv.PaymentConstraints(pathExpiry, paymentInfo.minHtlc) ) ).write().toByteVector() val blindedRoute = RouteBlinding.create(randomKey(), listOf(remoteNodeId, nodeParams.nodeId), listOf(remoteNodePayload, recipientPayload)).route From 646b38eeaac10057716dad7370b85c8134320cbb Mon Sep 17 00:00:00 2001 From: pm47 Date: Tue, 15 Oct 2024 14:20:51 +0200 Subject: [PATCH 6/7] typed bolt 11/12 expiries and made them consistent Also, revert the change to `Bolt11Invoice.DEFAULT_EXPIRY_SECONDS`. This value is from the spec and should be kept. --- .../kotlin/fr/acinq/lightning/NodeParams.kt | 9 ++++++--- .../kotlin/fr/acinq/lightning/io/Peer.kt | 4 ++-- .../fr/acinq/lightning/payment/Bolt11Invoice.kt | 2 +- .../lightning/payment/IncomingPaymentHandler.kt | 7 ++++--- .../fr/acinq/lightning/payment/OfferManager.kt | 4 ++-- .../kotlin/fr/acinq/lightning/io/peer/PeerTest.kt | 3 ++- .../payment/IncomingPaymentHandlerTestsCommon.kt | 14 ++++++++------ 7 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt b/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt index 0a2720f1f..598833bb5 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt @@ -107,6 +107,8 @@ data class RecipientCltvExpiryParams(val min: CltvExpiryDelta, val max: CltvExpi * @param minFinalCltvExpiryDelta cltv-expiry-delta that we require when receiving a payment. * @param fulfillSafetyBeforeTimeoutBlocks number of blocks necessary to react to a malicious peer that doesn't acknowledge and sign our HTLC preimages. * @param checkHtlcTimeoutAfterStartupDelay delay before we check for timed out HTLCs in our channels after a wallet restart. + * @param bolt11InvoiceExpiry duration for which bolt11 invoices that we create are valid. + * @param bolt12InvoiceExpiry duration for which bolt12 invoices that we create are valid. * @param htlcMinimum minimum accepted HTLC value. * @param toRemoteDelayBlocks number of blocks our peer will have to wait before they get their main output back in case they force-close a channel. * @param maxToLocalDelayBlocks maximum number of blocks we will have to wait before we get our main output back in case we force-close a channel. @@ -121,7 +123,6 @@ data class RecipientCltvExpiryParams(val min: CltvExpiryDelta, val max: CltvExpi * @param paymentRecipientExpiryParams configure the expiry delta used for the final node when sending payments. * @param zeroConfPeers list of peers with whom we use zero-conf (note that this is a strong trust assumption). * @param liquidityPolicy fee policy for liquidity events, can be modified at any time. - * @param bolt12invoiceExpiry duration for which bolt12 invoices that we create are valid. */ data class NodeParams( val loggerFactory: LoggerFactory, @@ -138,6 +139,8 @@ data class NodeParams( val fulfillSafetyBeforeTimeoutBlocks: CltvExpiryDelta, val checkHtlcTimeoutAfterStartupDelay: Duration, val checkHtlcTimeoutInterval: Duration, + val bolt11InvoiceExpiry: Duration, + val bolt12InvoiceExpiry: Duration, val htlcMinimum: MilliSatoshi, val toRemoteDelayBlocks: CltvExpiryDelta, val maxToLocalDelayBlocks: CltvExpiryDelta, @@ -152,7 +155,6 @@ data class NodeParams( val paymentRecipientExpiryParams: RecipientCltvExpiryParams, val zeroConfPeers: Set, val liquidityPolicy: MutableStateFlow, - val bolt12invoiceExpiry: Duration, ) { val nodePrivateKey get() = keyManager.nodeKeys.nodeKey.privateKey val nodeId get() = keyManager.nodeKeys.nodeKey.publicKey @@ -224,6 +226,8 @@ data class NodeParams( fulfillSafetyBeforeTimeoutBlocks = CltvExpiryDelta(12), checkHtlcTimeoutAfterStartupDelay = 30.seconds, checkHtlcTimeoutInterval = 10.seconds, + bolt11InvoiceExpiry = 24.hours, + bolt12InvoiceExpiry = 24.hours, htlcMinimum = 1000.msat, minDepthBlocks = 3, toRemoteDelayBlocks = CltvExpiryDelta(2016), @@ -247,7 +251,6 @@ data class NodeParams( maxAllowedFeeCredit = 0.msat ) ), - bolt12invoiceExpiry = 24.hours, ) /** diff --git a/src/commonMain/kotlin/fr/acinq/lightning/io/Peer.kt b/src/commonMain/kotlin/fr/acinq/lightning/io/Peer.kt index a1afd9c6d..9d5aaca5f 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/io/Peer.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/io/Peer.kt @@ -719,7 +719,7 @@ class Peer( return res.await() } - suspend fun createInvoice(paymentPreimage: ByteVector32, amount: MilliSatoshi?, description: Either, expirySeconds: Long? = null): Bolt11Invoice { + suspend fun createInvoice(paymentPreimage: ByteVector32, amount: MilliSatoshi?, description: Either, expiry: Duration? = null): Bolt11Invoice { // we add one extra hop which uses a virtual channel with a "peer id", using the highest remote fees and expiry across all // channels to maximize the likelihood of success on the first payment attempt val remoteChannelUpdates = _channels.values.mapNotNull { channelState -> @@ -741,7 +741,7 @@ class Peer( ) ) ) - return incomingPaymentHandler.createInvoice(paymentPreimage, amount, description, extraHops, expirySeconds) + return incomingPaymentHandler.createInvoice(paymentPreimage, amount, description, extraHops, expiry ?: nodeParams.bolt11InvoiceExpiry) } // The (node_id, fcm_token) tuple only needs to be registered once. diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/Bolt11Invoice.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/Bolt11Invoice.kt index d24e49d34..ea4b27717 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/payment/Bolt11Invoice.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/payment/Bolt11Invoice.kt @@ -105,7 +105,7 @@ data class Bolt11Invoice( } companion object { - const val DEFAULT_EXPIRY_SECONDS = 24 * 3600 // 1 day + const val DEFAULT_EXPIRY_SECONDS = 3600 // default value from Bolt 11 val DEFAULT_MIN_FINAL_EXPIRY_DELTA = CltvExpiryDelta(18) // default value from Bolt 11 private val prefixes = mapOf( diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt index d5ac9b3fb..a440c2c18 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt @@ -17,6 +17,7 @@ import fr.acinq.lightning.logging.MDCLogger import fr.acinq.lightning.logging.mdc import fr.acinq.lightning.utils.* import fr.acinq.lightning.wire.* +import kotlin.time.Duration sealed class PaymentPart { abstract val amount: MilliSatoshi @@ -79,7 +80,7 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { amount: MilliSatoshi?, description: Either, extraHops: List>, - expirySeconds: Long? = null, + expiry: Duration? = null, timestampSeconds: Long = currentTimestampSeconds() ): Bolt11Invoice { val paymentHash = Crypto.sha256(paymentPreimage).toByteVector32() @@ -95,7 +96,7 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { randomBytes32(), // We always include a payment metadata in our invoices, which lets us test whether senders support it ByteVector("2a"), - expirySeconds, + expiry?.inWholeSeconds, extraHops, timestampSeconds ) @@ -522,7 +523,7 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val db: PaymentsDb) { logger.warning { "payment with expiry too small: ${paymentPart.htlc.cltvExpiry}, min is ${minFinalCltvExpiry(nodeParams, paymentPart, incomingPayment.origin, currentBlockHeight)}" } Either.Left(rejectPaymentPart(privateKey, paymentPart, incomingPayment, currentBlockHeight)) } - metadata.createdAtMillis + nodeParams.bolt12invoiceExpiry.inWholeMilliseconds < currentTimestampMillis() && incomingPayment.received == null -> { + metadata.createdAtMillis + nodeParams.bolt12InvoiceExpiry.inWholeMilliseconds < currentTimestampMillis() && incomingPayment.received == null -> { logger.warning { "the invoice is expired" } Either.Left(rejectPaymentPart(privateKey, paymentPart, incomingPayment, currentBlockHeight)) } diff --git a/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt b/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt index 843bbbf91..44fc5f894 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/payment/OfferManager.kt @@ -182,7 +182,7 @@ class OfferManager(val nodeParams: NodeParams, val walletParams: WalletParams, v allowedFeatures = Features.empty ) // We assume 10 minutes between each block to convert the invoice expiry to a CLTV expiry for the blinded path. - val pathExpiry = (paymentInfo.cltvExpiryDelta + (nodeParams.bolt12invoiceExpiry.inWholeMinutes.toInt() / 10)).toCltvExpiry(currentBlockHeight.toLong()) + val pathExpiry = (paymentInfo.cltvExpiryDelta + (nodeParams.bolt12InvoiceExpiry.inWholeMinutes.toInt() / 10)).toCltvExpiry(currentBlockHeight.toLong()) val remoteNodePayload = RouteBlindingEncryptedData( TlvStream( RouteBlindingEncryptedDataTlv.OutgoingNodeId(EncodedNodeId.WithPublicKey.Wallet(nodeParams.nodeId)), @@ -192,7 +192,7 @@ class OfferManager(val nodeParams: NodeParams, val walletParams: WalletParams, v ).write().toByteVector() val blindedRoute = RouteBlinding.create(randomKey(), listOf(remoteNodeId, nodeParams.nodeId), listOf(remoteNodePayload, recipientPayload)).route val path = Bolt12Invoice.Companion.PaymentBlindedContactInfo(OfferTypes.ContactInfo.BlindedPath(blindedRoute), paymentInfo) - val invoice = Bolt12Invoice(request, preimage, decrypted.blindedPrivateKey, nodeParams.bolt12invoiceExpiry.inWholeSeconds, nodeParams.features.bolt12Features(), listOf(path)) + val invoice = Bolt12Invoice(request, preimage, decrypted.blindedPrivateKey, nodeParams.bolt12InvoiceExpiry.inWholeSeconds, nodeParams.features.bolt12Features(), listOf(path)) val destination = Destination.BlindedPath(decrypted.content.replyPath) when (val invoiceMessage = buildMessage(randomKey(), randomKey(), intermediateNodes(destination), destination, TlvStream(OnionMessagePayloadTlv.Invoice(invoice.records)))) { is Left -> { diff --git a/src/commonTest/kotlin/fr/acinq/lightning/io/peer/PeerTest.kt b/src/commonTest/kotlin/fr/acinq/lightning/io/peer/PeerTest.kt index ffcfed201..9e92686f3 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/io/peer/PeerTest.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/io/peer/PeerTest.kt @@ -33,6 +33,7 @@ import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import kotlin.test.* +import kotlin.time.Duration.Companion.hours import kotlin.time.Duration.Companion.seconds class PeerTest : LightningTestSuite() { @@ -396,7 +397,7 @@ class PeerTest : LightningTestSuite() { val bob = newPeer(nodeParams, walletParams) run { - val invoice = bob.createInvoice(randomBytes32(), 1.msat, Either.Left("A description: \uD83D\uDE2C"), 3600L * 3) + val invoice = bob.createInvoice(randomBytes32(), 1.msat, Either.Left("A description: \uD83D\uDE2C"), 3.hours) assertEquals(1.msat, invoice.amount) assertEquals(3600L * 3, invoice.expirySeconds) assertEquals("A description: \uD83D\uDE2C", invoice.description) diff --git a/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt index 9d0a93807..83b6a422a 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandlerTestsCommon.kt @@ -27,6 +27,8 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first import kotlin.test.* +import kotlin.time.Duration +import kotlin.time.Duration.Companion.hours import kotlin.time.Duration.Companion.milliseconds class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { @@ -1189,7 +1191,7 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { payee = paymentHandler, amount = defaultAmount, timestamp = currentTimestampSeconds() - 3600 - 60, // over one hour ago - expirySeconds = 3600 // one hour expiration + expiry = 1.hours ) val add = makeUpdateAddHtlc(0, randomBytes32(), paymentHandler, incomingPayment.paymentHash, makeMppPayload(10_000.msat, defaultAmount, paymentSecret)) val result = paymentHandler.process(add, Features.empty, TestConstants.defaultBlockHeight, TestConstants.feeratePerKw, remoteFundingRates = null) @@ -1521,14 +1523,14 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { // create incoming payment that has expired and not been paid val expiredInvoice = paymentHandler.createInvoice( - randomBytes32(), defaultAmount, Either.Left("expired"), listOf(), expirySeconds = 3600, + randomBytes32(), defaultAmount, Either.Left("expired"), listOf(), expiry = 1.hours, timestampSeconds = 1 ) // create incoming payment that has expired and been paid delay(100.milliseconds) val paidInvoice = paymentHandler.createInvoice( - defaultPreimage, defaultAmount, Either.Left("paid"), listOf(), expirySeconds = 3600, + defaultPreimage, defaultAmount, Either.Left("paid"), listOf(), expiry = 1.hours, timestampSeconds = 100 ) paymentHandler.db.receivePayment( @@ -1549,7 +1551,7 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { // create unexpired payment delay(100.milliseconds) - val unexpiredInvoice = paymentHandler.createInvoice(randomBytes32(), defaultAmount, Either.Left("unexpired"), listOf(), expirySeconds = 3600) + val unexpiredInvoice = paymentHandler.createInvoice(randomBytes32(), defaultAmount, Either.Left("unexpired"), listOf(), expiry = 1.hours) val unexpiredPayment = paymentHandler.db.getIncomingPayment(unexpiredInvoice.paymentHash)!! val paidPayment = paymentHandler.db.getIncomingPayment(paidInvoice.paymentHash)!! @@ -1883,8 +1885,8 @@ class IncomingPaymentHandlerTestsCommon : LightningTestSuite() { return Pair(payload, route) } - private suspend fun makeIncomingPayment(payee: IncomingPaymentHandler, amount: MilliSatoshi?, expirySeconds: Long? = null, timestamp: Long = currentTimestampSeconds()): Pair { - val paymentRequest = payee.createInvoice(defaultPreimage, amount, Either.Left("unit test"), listOf(), expirySeconds, timestamp) + private suspend fun makeIncomingPayment(payee: IncomingPaymentHandler, amount: MilliSatoshi?, expiry: Duration? = null, timestamp: Long = currentTimestampSeconds()): Pair { + val paymentRequest = payee.createInvoice(defaultPreimage, amount, Either.Left("unit test"), listOf(), expiry, timestamp) assertNotNull(paymentRequest.paymentMetadata) return Pair(payee.db.getIncomingPayment(paymentRequest.paymentHash)!!, paymentRequest.paymentSecret) } From 6c35867fc94f33119cecbd9314312dc993d94ce7 Mon Sep 17 00:00:00 2001 From: pm47 Date: Tue, 15 Oct 2024 15:39:51 +0200 Subject: [PATCH 7/7] clarify comment on RecipientCltvExpiryParams --- src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt b/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt index 598833bb5..94f519965 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt @@ -80,8 +80,11 @@ data class WalletParams( /** * When sending a payment, if the expiry used for the last node is very close to the current block height, * it lets intermediate nodes figure out their position in the route. To protect against this, a random - * delta between min and max will be added to the current block height, which makes it look like there - * are more hops after the final node. + * number of blocks between min and max will be added to the current block height, before the `minFinalExpiryDelta` + * requested by the recipient, which makes it look like there are more hops after the final node. + * + * The overall cltv expiry delta for an outgoing trampoline payment will thus be: + * `cltvExpiryDelta` = `random(min, max)` + `minFinalExpiryDelta` + `TrampolineFees.cltvExpiryDelta` */ data class RecipientCltvExpiryParams(val min: CltvExpiryDelta, val max: CltvExpiryDelta) { fun computeFinalExpiry(currentBlockHeight: Int, minFinalExpiryDelta: CltvExpiryDelta): CltvExpiry {