diff --git a/CHANGELOG.md b/CHANGELOG.md index a7b6d4bbe..02ccde42a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/app/src/main/java/to/bitkit/ext/Lnurl.kt b/app/src/main/java/to/bitkit/ext/Lnurl.kt index 834aec505..757cf6f31 100644 --- a/app/src/main/java/to/bitkit/ext/Lnurl.kt +++ b/app/src/main/java/to/bitkit/ext/Lnurl.kt @@ -24,5 +24,43 @@ 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()) +} + +/** + * The amount in whole sats to use when creating a withdraw invoice for a fixed-amount request. + * + * Uses floor division so the invoice amount never exceeds `maxWithdrawable` in msats. + */ +fun LnurlWithdrawData.fixedWithdrawAmountSat(): ULong = maxWithdrawable / MSATS_PER_SAT diff --git a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt index f5d2182b5..9575864cd 100644 --- a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt +++ b/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 = executeWhenNodeRunning("createInvoiceMsats") { + updateGeoBlockState() + runCatching { lightningService.receiveMsats(amountMsats, description, expirySeconds) } + } + @Suppress("ForbiddenComment") suspend fun fetchLnurlInvoice( callbackUrl: String, - amountSats: ULong, + amountMsats: ULong, comment: String? = null, ): Result { 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, ) diff --git a/app/src/main/java/to/bitkit/services/LightningService.kt b/app/src/main/java/to/bitkit/services/LightningService.kt index e57505d64..86ddd0bc4 100644 --- a/app/src/main/java/to/bitkit/services/LightningService.kt +++ b/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, ) diff --git a/app/src/main/java/to/bitkit/services/LnurlService.kt b/app/src/main/java/to/bitkit/services/LnurlService.kt index b5bc799ce..f917b5ce4 100644 --- a/app/src/main/java/to/bitkit/services/LnurlService.kt +++ b/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 = 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 } diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 55ac8e0a1..60d3f73b6 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -77,6 +77,9 @@ 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.fixedWithdrawAmountSat +import to.bitkit.ext.isFixedAmount import to.bitkit.ext.maxSendableSat import to.bitkit.ext.maxWithdrawableSat import to.bitkit.ext.minSendableSat @@ -1219,7 +1222,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 +1468,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 = if (isFixed) data.maxSendableSat() else 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 +1480,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 +1490,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 +1515,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 +1528,17 @@ class AppViewModel @Inject constructor( return } + val displayAmount = if (isFixed) data.fixedWithdrawAmountSat() else 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 +1647,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 +1775,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 +1826,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 +1855,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 +1887,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 +2644,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 diff --git a/app/src/main/java/to/bitkit/viewmodels/ProbingToolViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/ProbingToolViewModel.kt index f8eb3c204..ffe6cfb85 100644 --- a/app/src/main/java/to/bitkit/viewmodels/ProbingToolViewModel.kt +++ b/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 diff --git a/app/src/main/java/to/bitkit/viewmodels/QuickPayViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/QuickPayViewModel.kt index feda0ed8e..24f39567e 100644 --- a/app/src/main/java/to/bitkit/viewmodels/QuickPayViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/QuickPayViewModel.kt @@ -39,22 +39,25 @@ class QuickPayViewModel @Inject constructor( fun pay(data: QuickPayData) { viewModelScope.launch { - val (bolt11, amount) = when (data) { + val (bolt11, amount, displaySats) = when (data) { is QuickPayData.Bolt11 -> { Logger.info("QuickPay: processing bolt11 invoice") - data.bolt11 to data.sats + Triple(data.bolt11, null, data.sats) } is QuickPayData.LnurlPay -> { Logger.info("QuickPay: fetching LNURL Pay invoice from callback") - val invoice = lightningRepo.fetchLnurlInvoice(callbackUrl = data.callback, amountSats = data.sats) + val invoice = lightningRepo.fetchLnurlInvoice( + callbackUrl = data.callback, + amountMsats = data.amountMsats, + ) .getOrElse { error -> _uiState.update { it.copy(result = QuickPayResult.Error(error.message.orEmpty())) } return@launch } - invoice.bolt11 to data.sats + Triple(invoice.bolt11, null, data.sats) } } @@ -65,7 +68,7 @@ class QuickPayViewModel @Inject constructor( it.copy( result = QuickPayResult.Success( paymentHash = paymentHash, - amountWithFee = amount.toLong() // TODO GET FEE WHEN AVAILABLE + amountWithFee = displaySats.toLong() // TODO GET FEE WHEN AVAILABLE ) ) } @@ -77,7 +80,7 @@ class QuickPayViewModel @Inject constructor( it.copy( result = QuickPayResult.Pending( paymentHash = error.paymentHash, - amount = amount.toLong(), + amount = displaySats.toLong(), ) ) } diff --git a/app/src/test/java/to/bitkit/ext/LnurlExtTest.kt b/app/src/test/java/to/bitkit/ext/LnurlExtTest.kt index 16ab55c08..6606b7963 100644 --- a/app/src/test/java/to/bitkit/ext/LnurlExtTest.kt +++ b/app/src/test/java/to/bitkit/ext/LnurlExtTest.kt @@ -74,4 +74,81 @@ class LnurlExtTest : BaseUnitTest() { val nonRoundMin = nullMin.copy(minWithdrawable = 1_500u) assertEquals(2u, nonRoundMin.minWithdrawableSat()) } + + @Test + fun `isFixedAmount returns true when min equals max`() { + val data = lnurlPayData(minSendable = 5_000u, maxSendable = 5_000u) + assertEquals(true, data.isFixedAmount()) + } + + @Test + fun `isFixedAmount returns true for sub-sat fixed amount`() { + val data = lnurlPayData(minSendable = 500_500u, maxSendable = 500_500u) + assertEquals(501u, data.minSendableSat()) + assertEquals(500u, data.maxSendableSat()) + assertEquals(true, data.isFixedAmount()) + } + + @Test + fun `isFixedAmount returns false for variable range`() { + val data = lnurlPayData(minSendable = 1_000u, maxSendable = 100_000u) + assertEquals(false, data.isFixedAmount()) + } + + @Test + fun `callbackAmountMsats returns original msats for fixed amount`() { + val data = lnurlPayData(minSendable = 500_500u, maxSendable = 500_500u) + assertEquals(500_500u, data.callbackAmountMsats(500u)) + } + + @Test + fun `callbackAmountMsats converts user sats for variable amount`() { + val data = lnurlPayData(minSendable = 1_000u, maxSendable = 100_000u) + assertEquals(50_000u, data.callbackAmountMsats(50u)) + } + + @Test + fun `withdraw isFixedAmount returns true for sub-sat fixed amount`() { + val data = withdrawData(minWithdrawable = 500_500u, maxWithdrawable = 500_500u) + assertEquals(true, data.isFixedAmount()) + } + + @Test + fun `withdraw isFixedAmount returns false for variable range`() { + val data = withdrawData(minWithdrawable = 1_000u, maxWithdrawable = 100_000u) + assertEquals(false, data.isFixedAmount()) + } + + @Test + fun `fixedWithdrawAmountSat floors to avoid exceeding max`() { + val data = withdrawData(minWithdrawable = 500_500u, maxWithdrawable = 500_500u) + assertEquals(500u, data.fixedWithdrawAmountSat()) + } + + private fun lnurlPayData( + minSendable: ULong = 1_000u, + maxSendable: ULong = 100_000u, + ) = LnurlPayData( + uri = "lnurl", + callback = "callback", + minSendable = minSendable, + maxSendable = maxSendable, + metadataStr = "[]", + commentAllowed = null, + allowsNostr = false, + nostrPubkey = null, + ) + + private fun withdrawData( + minWithdrawable: ULong? = null, + maxWithdrawable: ULong = 1_000u, + ) = LnurlWithdrawData( + uri = "lnurl", + callback = "callback", + k1 = "k1", + defaultDescription = "desc", + minWithdrawable = minWithdrawable, + maxWithdrawable = maxWithdrawable, + tag = "withdraw", + ) } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index ea7508a9a..8f19e76a6 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -19,7 +19,7 @@ activity-compose = { module = "androidx.activity:activity-compose", version = "1 appcompat = { module = "androidx.appcompat:appcompat", version = "1.7.1" } barcode-scanning = { module = "com.google.mlkit:barcode-scanning", version = "17.3.0" } biometric = { module = "androidx.biometric:biometric", version = "1.4.0-alpha05" } -bitkit-core = { module = "com.synonym:bitkit-core-android", version = "0.1.38" } +bitkit-core = { module = "com.synonym:bitkit-core-android", version = "0.1.56" } bouncycastle-provider-jdk = { module = "org.bouncycastle:bcprov-jdk18on", version = "1.83" } camera-camera2 = { module = "androidx.camera:camera-camera2", version.ref = "camera" } camera-lifecycle = { module = "androidx.camera:camera-lifecycle", version.ref = "camera" }