Merge pull request #879 from synonymdev/fix/msat-invoice-precision

fix: avoid msat truncation when paying invoices with built-in amounts

Author: ovitrif

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Fixed
+- Avoid msat truncation when paying invoices and LNURL callbacks #879
 - Fix ANR on RGS server settings screen caused by catastrophic regex backtracking #880
 - Fix crash when returning app to foreground on Receive screen #875
 - Show loading state on Spending tab when node is not running #875

app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt

@@ -97,7 +97,7 @@ class NotifyPaymentReceivedHandler @Inject constructor(
             is NotifyPaymentReceived.Command.Onchain -> command.event.txid
         },
         sats = when (command) {
-            is NotifyPaymentReceived.Command.Lightning -> (command.event.amountMsat / 1000u).toLong()
+            is NotifyPaymentReceived.Command.Lightning -> ((command.event.amountMsat + 999u) / 1000u).toLong()
             is NotifyPaymentReceived.Command.Onchain -> command.event.details.amountSats
         },
     )

app/src/main/java/to/bitkit/ext/Lnurl.kt

@@ -24,5 +24,36 @@ fun LnurlPayData.commentAllowed(): Boolean = commentAllowed?.let { it > 0u } ==
 fun LnurlPayData.maxSendableSat(): ULong = maxSendable / MSATS_PER_SAT
 fun LnurlPayData.minSendableSat(): ULong = msatsToSatsCeil(minSendable)
 
+/**
+ * True when the LNURL-pay endpoint specifies a single exact amount.
+ *
+ * This also covers the sub-sat edge case where `minSendable` and `maxSendable` differ
+ * in their sub-sat fraction but map to the same (or inverted) sat range after rounding,
+ * e.g. `minSendable = 500500, maxSendable = 500500` → `minSendableSat() = 501, maxSendableSat() = 500`.
+ */
+fun LnurlPayData.isFixedAmount(): Boolean =
+    minSendable == maxSendable || (minSendable > 0u && minSendableSat() > maxSendableSat())
+
+/**
+ * Returns the amount in millisatoshis to send in the LNURL-pay callback.
+ *
+ * For fixed-amount requests (including sub-sat ranges) the original msat value
+ * from the server is returned verbatim, avoiding precision loss from the
+ * msat→sat→msat round-trip.
+ *
+ * For variable-amount requests the user-selected sat amount is converted to msats.
+ */
+fun LnurlPayData.callbackAmountMsats(userSats: ULong? = null): ULong =
+    if (isFixedAmount()) minSendable else (userSats ?: minSendableSat()) * MSATS_PER_SAT
+
 fun LnurlWithdrawData.minWithdrawableSat(): ULong = msatsToSatsCeil(minWithdrawable ?: 0u)
 fun LnurlWithdrawData.maxWithdrawableSat(): ULong = maxWithdrawable / MSATS_PER_SAT
+
+/**
+ * True when the LNURL-withdraw endpoint specifies a single exact amount,
+ * including the sub-sat edge case where rounding causes `min > max` in whole sats.
+ */
+fun LnurlWithdrawData.isFixedAmount(): Boolean {
+    val min = minWithdrawable ?: 0u
+    return min == maxWithdrawable || (min > 0u && minWithdrawableSat() > maxWithdrawableSat())
+}

app/src/main/java/to/bitkit/ext/PaymentDetails.kt

@@ -2,4 +2,5 @@ package to.bitkit.ext
 
 import org.lightningdevkit.ldknode.PaymentDetails
 
-val PaymentDetails.amountSats: ULong? get() = amountMsat?.let { it / 1000u }
+val PaymentDetails.amountSats: ULong?
+    get() = amountMsat?.let { (it + 999u) / 1000u }

app/src/main/java/to/bitkit/fcm/WakeNodeWorker.kt

@@ -192,7 +192,7 @@ class WakeNodeWorker @AssistedInject constructor(
         showDetails: Boolean,
         hiddenBody: String,
     ) {
-        val sats = event.amountMsat / 1000u
+        val sats = (event.amountMsat + 999u) / 1000u
         // Save for UI to pick up
         cacheStore.setBackgroundReceive(
             NewTransactionSheetDetails(

app/src/main/java/to/bitkit/repositories/LightningRepo.kt

@@ -897,20 +897,29 @@ class LightningRepo @Inject constructor(
         runCatching { lightningService.receive(amountSats, description, expirySeconds) }
     }
 
+    suspend fun createInvoiceMsats(
+        amountMsats: ULong,
+        description: String,
+        expirySeconds: UInt = 86_400u,
+    ): Result<String> = executeWhenNodeRunning("createInvoiceMsats") {
+        updateGeoBlockState()
+        runCatching { lightningService.receiveMsats(amountMsats, description, expirySeconds) }
+    }
+
     @Suppress("ForbiddenComment")
     suspend fun fetchLnurlInvoice(
         callbackUrl: String,
-        amountSats: ULong,
+        amountMsats: ULong,
         comment: String? = null,
     ): Result<LightningInvoice> {
         return runCatching {
             // TODO use bitkit-core getLnurlInvoice if it works with callbackUrl
-            val bolt11 = lnurlService.fetchLnurlInvoice(callbackUrl, amountSats, comment).getOrThrow().pr
+            val bolt11 = lnurlService.fetchLnurlInvoice(callbackUrl, amountMsats, comment).getOrThrow().pr
             val decoded = (coreService.decode(bolt11) as Scanner.Lightning).invoice
             return@runCatching decoded
         }.onFailure {
             Logger.error(
-                "fetchLnurlInvoice error, url: $callbackUrl, amount: $amountSats, comment: $comment",
+                "Failed to fetch LNURL invoice, url: '$callbackUrl', amountMsats: '$amountMsats', comment: '$comment'",
                 it,
                 context = TAG,
             )

app/src/main/java/to/bitkit/services/LightningService.kt

@@ -590,15 +590,19 @@ class LightningService @Inject constructor(
     }
 
     suspend fun receive(sat: ULong? = null, description: String, expirySecs: UInt = 3600u): String {
+        return receiveMsats(amountMsat = sat?.let { it * 1000u }, description = description, expirySecs = expirySecs)
+    }
+
+    suspend fun receiveMsats(amountMsat: ULong? = null, description: String, expirySecs: UInt = 3600u): String {
         val node = this.node ?: throw ServiceError.NodeNotSetup()
 
         val message = description
 
         return ServiceQueue.LDK.background {
-            val bolt11Invoice: Bolt11Invoice = if (sat != null) {
+            val bolt11Invoice: Bolt11Invoice = if (amountMsat != null) {
                 node.bolt11Payment()
                     .receive(
-                        amountMsat = sat * 1000u,
+                        amountMsat = amountMsat,
                         description = Bolt11InvoiceDescription.Direct(description = message),
                         expirySecs = expirySecs,
                     )

app/src/main/java/to/bitkit/services/LnurlService.kt

@@ -41,14 +41,14 @@ class LnurlService @Inject constructor(
 
     suspend fun fetchLnurlInvoice(
         callbackUrl: String,
-        amountSats: ULong,
+        amountMsats: ULong,
         comment: String? = null,
     ): Result<LnurlPayResponse> = runCatching {
         Logger.debug("Fetching LNURL pay invoice from: $callbackUrl", context = TAG)
 
         val response = client.get(callbackUrl) {
             url {
-                parameters["amount"] = "${amountSats * 1000u}" // convert to msat
+                parameters["amount"] = "$amountMsats"
                 comment?.takeIf { it.isNotBlank() }?.let {
                     parameters["comment"] = it
                 }

app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt

@@ -77,6 +77,8 @@ import to.bitkit.ext.channelId
 import to.bitkit.ext.claimableAtHeight
 import to.bitkit.ext.getClipboardText
 import to.bitkit.ext.getSatsPerVByteFor
+import to.bitkit.ext.callbackAmountMsats
+import to.bitkit.ext.isFixedAmount
 import to.bitkit.ext.maxSendableSat
 import to.bitkit.ext.maxWithdrawableSat
 import to.bitkit.ext.minSendableSat
@@ -1219,7 +1221,7 @@ class AppViewModel @Inject constructor(
                 val maxSendable = maxSendableLightningSats()
                 when (val lnurl = _sendUiState.value.lnurl) {
                     null -> amount <= maxSendable && lightningRepo.canSend(amount)
-                    is LnurlParams.LnurlWithdraw -> amount < lnurl.data.maxWithdrawableSat()
+                    is LnurlParams.LnurlWithdraw -> amount <= lnurl.data.maxWithdrawableSat()
                     is LnurlParams.LnurlPay -> {
                         val maxSat = lnurl.data.maxSendableSat()
                         amount <= maxSat && amount <= maxSendable && lightningRepo.canSend(amount)
@@ -1465,10 +1467,10 @@ class AppViewModel @Inject constructor(
     private suspend fun onScanLnurlPay(data: LnurlPayData) {
         Logger.debug("LNURL: $data", context = TAG)
 
-        val minSendable = data.minSendableSat()
-        val maxSendable = data.maxSendableSat()
+        val isFixed = data.isFixedAmount()
+        val displaySats = data.minSendableSat()
 
-        if (!lightningRepo.canSend(minSendable)) {
+        if (!lightningRepo.canSend(displaySats.coerceAtLeast(1u))) {
             toast(
                 type = Toast.ToastType.WARNING,
                 title = context.getString(R.string.other__lnurl_pay_error),
@@ -1477,8 +1479,7 @@ class AppViewModel @Inject constructor(
             return
         }
 
-        val hasAmount = minSendable == maxSendable && minSendable > 0u
-        val initialAmount = if (hasAmount) minSendable else 0u
+        val initialAmount = if (isFixed) displaySats else 0u
 
         _sendUiState.update {
             it.copy(
@@ -1488,10 +1489,10 @@ class AppViewModel @Inject constructor(
             )
         }
 
-        if (hasAmount) {
-            Logger.info("Found amount $$minSendable in lnurlPay, proceeding with payment", context = TAG)
+        if (isFixed) {
+            Logger.info("Found fixed amount '$displaySats' sats in lnurlPay, proceeding with payment", context = TAG)
 
-            val quickPayHandled = handleQuickPayIfApplicable(amountSats = minSendable, lnurlPay = data)
+            val quickPayHandled = handleQuickPayIfApplicable(amountSats = displaySats, lnurlPay = data)
             if (quickPayHandled) return
 
             if (isMainScanner) {
@@ -1513,10 +1514,11 @@ class AppViewModel @Inject constructor(
     private suspend fun onScanLnurlWithdraw(data: LnurlWithdrawData) {
         Logger.debug("LNURL: $data", context = TAG)
 
+        val isFixed = data.isFixedAmount()
         val minWithdrawable = data.minWithdrawableSat()
         val maxWithdrawable = data.maxWithdrawableSat()
 
-        if (minWithdrawable > maxWithdrawable) {
+        if (!isFixed && minWithdrawable > maxWithdrawable) {
             toast(
                 type = Toast.ToastType.WARNING,
                 title = context.getString(R.string.other__lnurl_withdr_error),
@@ -1525,15 +1527,17 @@ class AppViewModel @Inject constructor(
             return
         }
 
+        val displayAmount = minWithdrawable
+
         _sendUiState.update {
             it.copy(
                 payMethod = SendMethod.LIGHTNING,
-                amount = minWithdrawable,
+                amount = displayAmount,
                 lnurl = LnurlParams.LnurlWithdraw(data = data)
             )
         }
 
-        if (minWithdrawable == maxWithdrawable) {
+        if (isFixed || minWithdrawable == maxWithdrawable) {
             delay(TRANSITION_SCREEN_MS)
             if (isMainScanner) {
                 showSheet(Sheet.Send(SendRoute.WithdrawConfirm))
@@ -1642,7 +1646,11 @@ class AppViewModel @Inject constructor(
 
             val quickPayData: QuickPayData = when {
                 lnurlPay != null -> {
-                    QuickPayData.LnurlPay(sats = amountSats, callback = lnurlPay.callback)
+                    QuickPayData.LnurlPay(
+                        sats = amountSats,
+                        callback = lnurlPay.callback,
+                        amountMsats = lnurlPay.callbackAmountMsats(amountSats),
+                    )
                 }
 
                 else -> {
@@ -1766,9 +1774,10 @@ class AppViewModel @Inject constructor(
         val isLnurlPay = lnurl is LnurlParams.LnurlPay
 
         if (isLnurlPay) {
+            val amountMsats = lnurl.data.callbackAmountMsats(amount)
             lightningRepo.fetchLnurlInvoice(
                 callbackUrl = lnurl.data.callback,
-                amountSats = amount,
+                amountMsats = amountMsats,
                 comment = _sendUiState.value.comment.takeIf { it.isNotEmpty() },
             ).onSuccess { invoice ->
                 _sendUiState.update {
@@ -1816,8 +1825,8 @@ class AppViewModel @Inject constructor(
                 val decodedInvoice = requireNotNull(_sendUiState.value.decodedInvoice)
                 val bolt11 = decodedInvoice.bolt11
 
-                // Determine if we should override amount
-                val paymentAmount = decodedInvoice.amountSatoshis.takeIf { it > 0uL } ?: amount
+                val paymentAmount = if (decodedInvoice.amountSatoshis > 0uL) null else amount
+                val displayAmountSats = decodedInvoice.amountSatoshis.takeIf { it > 0uL } ?: amount ?: 0uL
 
                 val tags = _sendUiState.value.selectedTags
                 var createdMetadataPaymentId: String? = null
@@ -1845,14 +1854,14 @@ class AppViewModel @Inject constructor(
                             type = NewTransactionSheetType.LIGHTNING,
                             direction = NewTransactionSheetDirection.SENT,
                             paymentHashOrTxId = actualPaymentHash,
-                            sats = paymentAmount.toLong(), // TODO Add fee when available
+                            sats = displayAmountSats.toLong(), // TODO Add fee when available
                         ),
                     )
                 }.onFailure {
                     if (it is PaymentPendingException) {
                         Logger.info("Lightning payment pending", context = TAG)
                         pendingPaymentRepo.track(it.paymentHash)
-                        setSendEffect(SendEffect.NavigateToPending(it.paymentHash, paymentAmount.toLong()))
+                        setSendEffect(SendEffect.NavigateToPending(it.paymentHash, displayAmountSats.toLong()))
                         return@onFailure
                     }
                     // Delete pre-activity metadata on failure
@@ -1877,19 +1886,23 @@ class AppViewModel @Inject constructor(
                 return@launch
             }
 
-            _sendUiState.update {
-                it.copy(
-                    amount = it.amount.coerceAtLeast(
-                        (lnurl.data.minWithdrawable ?: 0u) / 1000u
-                    )
+            val invoice = if (lnurl.data.isFixedAmount()) {
+                lightningRepo.createInvoiceMsats(
+                    amountMsats = lnurl.data.maxWithdrawable,
+                    description = lnurl.data.defaultDescription,
+                    expirySeconds = 3600u,
                 )
-            }
-
-            val invoice = lightningRepo.createInvoice(
-                amountSats = _sendUiState.value.amount,
-                description = lnurl.data.defaultDescription,
-                expirySeconds = 3600u,
-            ).getOrNull()
+            } else {
+                val withdrawAmountSats = _sendUiState.value.amount.coerceAtLeast(
+                    (lnurl.data.minWithdrawable ?: 0u) / 1000u
+                )
+                _sendUiState.update { it.copy(amount = withdrawAmountSats) }
+                lightningRepo.createInvoice(
+                    amountSats = withdrawAmountSats,
+                    description = lnurl.data.defaultDescription,
+                    expirySeconds = 3600u,
+                )
+            }.getOrNull()
 
             if (invoice == null) {
                 setSendEffect(SendEffect.NavigateToWithdrawError)
@@ -2630,6 +2643,6 @@ sealed interface QuickPayData {
     data class Bolt11(override val sats: ULong, val bolt11: String) : QuickPayData
 
     @Stable
-    data class LnurlPay(override val sats: ULong, val callback: String) : QuickPayData
+    data class LnurlPay(override val sats: ULong, val callback: String, val amountMsats: ULong) : QuickPayData
 }
 // endregion

app/src/main/java/to/bitkit/viewmodels/ProbingToolViewModel.kt

@@ -179,7 +179,7 @@ class ProbingToolViewModel @Inject constructor(
 
             is Scanner.LnurlPay -> {
                 val amount = amountSats ?: return@runCatching null
-                lightningRepo.fetchLnurlInvoice(decoded.data.callback, amount).getOrThrow().bolt11
+                lightningRepo.fetchLnurlInvoice(decoded.data.callback, amount * 1000u).getOrThrow().bolt11
             }
 
             else -> null