From 24a8540ad978967c66abb57eeff795dc9f0b654d Mon Sep 17 00:00:00 2001 From: davotoula Date: Tue, 25 Aug 2026 18:45:51 +0200 Subject: [PATCH] fix(nwc): never let a NIP-47 refusal or timeout reach the user as silence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Field report (BrollyZapper, 2026-08-25): a QUOTA_EXCEEDED on pay_invoice and a RESTRICTED on list_transactions both reached the phone and showed nothing at all — no toast, no dialog, no error state. The action simply looked like it had not happened. Three separate defects produce that symptom. 1. The zap path had no user-visible timeout. NwcSignerState's 60s safety net only dropped the relay subscription: it never cleaned the tracker entry and never told anyone. A response lost in transit (the same trip measured relay.damus.io refusing 40% of websocket upgrades) was therefore permanent silence. The timeout now retires the request and fires an onTimeout callback that every interactive caller renders. NwcPaymentTracker.cleanup returns whether it was the one to remove the entry, so a timeout racing a real response stays quiet rather than overwriting the wallet's own answer. 2. WalletTransactionsScreen never read walletViewModel.error. The ViewModel set it correctly on both the refusal and the timeout paths; the view branched on isLoading/isEmpty only and rendered "No transactions yet" over the top of it. 3. Consumers matched on PayInvoiceErrorResponse, which the deserializer only produces when result_type == "pay_invoice". NIP-47 does not require a wallet to echo result_type on an error, and an error for any other method takes the generic NwcErrorResponse branch — so those refusals were dropped without a word, and the DVM screen went as far as thanking the user for a payment that had just been refused. All of them now match IErrorResponseLike, and the remaining else branches report an unreadable response instead of nothing. Also: errorMessage() falls back to the code name when a wallet sends `code` without `message` (message is optional in NIP-47), and stale wallet errors are cleared when a transaction fetch or page load succeeds. --- .../amethyst/model/AccountZapActions.kt | 15 +++- .../nip47WalletConnect/NwcSignerState.kt | 48 +++++++++-- .../amethyst/service/V4VPaymentHandler.kt | 52 +++++++++--- .../amethyst/service/ZapPaymentHandler.kt | 33 +++++++- .../invoice/InvoicePaymentDispatcher.kt | 28 +++++-- .../ui/screen/loggedIn/AccountViewModel.kt | 3 +- .../dvms/DvmContentDiscoveryScreen.kt | 36 ++++++--- .../profile/payment/SendPaymentScreen.kt | 28 ++++--- .../wallet/WalletTransactionsScreen.kt | 46 +++++++++++ .../screen/loggedIn/wallet/WalletViewModel.kt | 2 + amethyst/src/main/res/values/strings.xml | 3 + .../commons/service/nwc/NwcPaymentTracker.kt | 9 ++- .../service/nwc/NwcPaymentTrackerTest.kt | 80 +++++++++++++++++++ .../amethyst/desktop/nwc/NwcPaymentHandler.kt | 9 ++- .../quartz/nip47WalletConnect/rpc/Response.kt | 8 +- .../quartz/nip47WalletConnect/ResponseTest.kt | 52 ++++++++++++ 16 files changed, 394 insertions(+), 58 deletions(-) create mode 100644 commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/service/nwc/NwcPaymentTrackerTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountZapActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountZapActions.kt index ea88acf512..9a78284513 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountZapActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountZapActions.kt @@ -99,18 +99,20 @@ class AccountZapActions( suspend fun sendNwcRequest( request: Request, + onTimeout: (() -> Unit)? = null, onResponse: (Response?) -> Unit, ) { - val (event, relay) = account.nip47SignerState.sendNwcRequest(request, onResponse) + val (event, relay) = account.nip47SignerState.sendNwcRequest(request, onTimeout, onResponse) account.client.publish(event, setOf(relay)) } suspend fun sendNwcRequestToWallet( walletUri: Nip47WalletConnect.Nip47URINorm, request: Request, + onTimeout: (() -> Unit)? = null, onResponse: (Response?) -> Unit, ): HexKey { - val (event, relay) = account.nip47SignerState.sendNwcRequestToWallet(walletUri, request, onResponse) + val (event, relay) = account.nip47SignerState.sendNwcRequestToWallet(walletUri, request, onTimeout, onResponse) account.client.publish(event, setOf(relay)) return event.id } @@ -127,12 +129,19 @@ class AccountZapActions( */ fun cleanupNwcRequest(requestId: HexKey) = LocalCache.paymentTracker.cleanup(requestId) + /** + * @param onTimeout invoked when no kind-23195 reply arrives before + * [NwcSignerState.NWC_RESPONSE_TIMEOUT_MS]. Pass one on any path with a user + * watching: without it a response lost in transit is indistinguishable from + * the action never having happened. + */ suspend fun sendZapPaymentRequestFor( bolt11: String, zappedNote: Note?, + onTimeout: (() -> Unit)? = null, onResponse: (Response?) -> Unit, ) { - val (event, relay) = account.nip47SignerState.sendZapPaymentRequestFor(bolt11, zappedNote, onResponse) + val (event, relay) = account.nip47SignerState.sendZapPaymentRequestFor(bolt11, zappedNote, onTimeout, onResponse) account.client.publish(event, setOf(relay)) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip47WalletConnect/NwcSignerState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip47WalletConnect/NwcSignerState.kt index 5ce3a777eb..c8f983b628 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip47WalletConnect/NwcSignerState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip47WalletConnect/NwcSignerState.kt @@ -42,6 +42,7 @@ import com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcTransaction import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PaymentReceivedNotification import com.vitorpamplona.quartz.nip47WalletConnect.rpc.Request import com.vitorpamplona.quartz.nip47WalletConnect.rpc.Response +import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -203,8 +204,9 @@ class NwcSignerState( */ suspend fun sendNwcRequest( request: Request, + onTimeout: (() -> Unit)? = null, onResponse: (Response?) -> Unit, - ): Pair = sendNwcRequestToWallet(defaultWalletUri.value, request, onResponse) + ): Pair = sendNwcRequestToWallet(defaultWalletUri.value, request, onTimeout, onResponse) /** * Sends a generic NIP-47 request to a specific wallet. @@ -212,6 +214,7 @@ class NwcSignerState( suspend fun sendNwcRequestToWallet( walletUri: Nip47WalletConnect.Nip47URINorm?, request: Request, + onTimeout: (() -> Unit)? = null, onResponse: (Response?) -> Unit, ): Pair { val walletService = walletUri ?: throw IllegalArgumentException("No NIP47 setup") @@ -234,13 +237,14 @@ class NwcSignerState( // be missed. assembler.subscribeAndFlush(filter) - // Safety net: drop the filter after 60s if the wallet never replies. + // Safety net: drop the filter after the timeout if the wallet never replies. // The happy path (response arrives) cancels this job and unsubscribes // through assembler.unsubscribeSoon, which debounces. val timeoutJob = scope.launch(Dispatchers.IO) { - delay(60000) + delay(NWC_RESPONSE_TIMEOUT_MS) assembler.unsubscribe(filter) + giveUpWaiting(event.id, onTimeout) } val responseCache = NostrWalletConnectResponseCache(walletSigner) @@ -259,6 +263,7 @@ class NwcSignerState( suspend fun sendZapPaymentRequestFor( bolt11: String, zappedNote: Note?, + onTimeout: (() -> Unit)? = null, onResponse: (Response?) -> Unit, ): Pair { val walletService = defaultWalletUri.value ?: throw IllegalArgumentException("No NIP47 setup") @@ -278,13 +283,14 @@ class NwcSignerState( // See sendNwcRequestToWallet above for the rationale. assembler.subscribeAndFlush(filter) - // Safety net: drop the filter after 60s if the wallet never replies. + // Safety net: drop the filter after the timeout if the wallet never replies. // The happy path (response arrives) cancels this job and instead // hands off to assembler.unsubscribeSoon, which debounces. val timeoutJob = scope.launch(Dispatchers.IO) { - delay(60000) // waits 1 minute to complete payment. + delay(NWC_RESPONSE_TIMEOUT_MS) assembler.unsubscribe(filter) + giveUpWaiting(event.id, onTimeout) } cache.consume(event, zappedNote, true, walletService.relayUri) { @@ -295,4 +301,36 @@ class NwcSignerState( return Pair(event, walletService.relayUri) } + + /** + * Retires a request whose response never arrived: removes the tracker entry so it + * does not leak, and tells the caller so the user hears about it. A silent give-up + * is the worst outcome for a payment UI — the action just appears not to have + * happened, which is indistinguishable from a refusal the wallet did send. + * + * A `cleanup` that returns false means a response beat us to the tracker entry, + * so the response path is already reporting and this must stay quiet. + */ + private fun giveUpWaiting( + requestId: HexKey, + onTimeout: (() -> Unit)?, + ) { + val wasStillPending = cache.paymentTracker.cleanup(requestId) + if (wasStillPending) { + Log.w("NwcSignerState") { + "No NIP-47 response for request $requestId after ${NWC_RESPONSE_TIMEOUT_MS}ms; giving up and dropping the subscription." + } + onTimeout?.invoke() + } + } + + companion object { + /** + * How long a NIP-47 request waits for its kind-23195 reply before the client + * gives up. Exposed so the UI can name the number it shows the user. + */ + const val NWC_RESPONSE_TIMEOUT_MS = 60000L + + val NWC_RESPONSE_TIMEOUT_SECONDS = (NWC_RESPONSE_TIMEOUT_MS / 1000).toInt() + } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/V4VPaymentHandler.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/V4VPaymentHandler.kt index 3f7657ca83..4b6e625aa0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/V4VPaymentHandler.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/V4VPaymentHandler.kt @@ -25,6 +25,7 @@ import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.model.payments.PaymentSource import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.Note +import com.vitorpamplona.amethyst.model.nip47WalletConnect.NwcSignerState import com.vitorpamplona.amethyst.service.lnurl.LightningAddressResolver import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.quartz.nip01Core.core.toHexKey @@ -159,15 +160,28 @@ class V4VPaymentHandler( tlvRecords = tlvRecords, ) - account.zaps.sendNwcRequest(request) { response: Response? -> - if (response is IErrorResponseLike) { + account.zaps.sendNwcRequest( + request = request, + onResponse = { response: Response? -> + if (response is IErrorResponseLike) { + onError( + stringRes(context, R.string.error_dialog_pay_invoice_error), + response.errorMessage() + ?: stringRes(context, R.string.error_parsing_error_message), + ) + } + }, + onTimeout = { onError( stringRes(context, R.string.error_dialog_pay_invoice_error), - response.errorMessage() - ?: stringRes(context, R.string.error_parsing_error_message), + stringRes( + context, + R.string.wallet_connect_no_response_error, + NwcSignerState.NWC_RESPONSE_TIMEOUT_SECONDS, + ), ) - } - } + }, + ) } } @@ -250,15 +264,29 @@ class V4VPaymentHandler( is PaymentSource.Nwc -> { var done = 0 payables.forEach { payable -> - account.zaps.sendZapPaymentRequestFor(payable.invoice, zappedNote) { response -> - if (response is IErrorResponseLike) { + account.zaps.sendZapPaymentRequestFor( + bolt11 = payable.invoice, + zappedNote = zappedNote, + onResponse = { response -> + if (response is IErrorResponseLike) { + onError( + stringRes(context, R.string.error_dialog_pay_invoice_error), + response.errorMessage() + ?: stringRes(context, R.string.error_parsing_error_message), + ) + } + }, + onTimeout = { onError( stringRes(context, R.string.error_dialog_pay_invoice_error), - response.errorMessage() - ?: stringRes(context, R.string.error_parsing_error_message), + stringRes( + context, + R.string.wallet_connect_no_response_error, + NwcSignerState.NWC_RESPONSE_TIMEOUT_SECONDS, + ), ) - } - } + }, + ) done++ onProgress(done.toFloat() / payables.size) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt index 0d5ad13cfa..e7bd337125 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt @@ -28,11 +28,13 @@ import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.Note import com.vitorpamplona.amethyst.model.User +import com.vitorpamplona.amethyst.model.nip47WalletConnect.NwcSignerState import com.vitorpamplona.amethyst.service.lnurl.LightningAddressResolver import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.quartz.experimental.clink.pointers.NDebit import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl -import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceErrorResponse +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.IErrorResponseLike +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceSuccessResponse import com.vitorpamplona.quartz.nip53LiveActivities.streaming.LiveActivitiesEvent import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent import com.vitorpamplona.quartz.nip57Zaps.LnZapRequestEvent @@ -419,19 +421,42 @@ class ZapPaymentHandler( zappedNote = note, onResponse = { response -> progress.step() - if (response is PayInvoiceErrorResponse) { + // Matched on IErrorResponseLike rather than PayInvoiceErrorResponse: + // a wallet that omits `result_type` on an error (NIP-47 does not + // require it) deserializes to the generic NwcErrorResponse, and the + // narrower check dropped those refusals without a word. + if (response is IErrorResponseLike) { onError( stringRes(context, R.string.error_dialog_pay_invoice_error), stringRes( context, R.string.wallet_connect_pay_invoice_error_error, - response.error?.message - ?: response.error?.code?.toString() ?: "Error parsing error message", + response.errorMessage() + ?: stringRes(context, R.string.error_parsing_error_message), ), payable.info.user, ) + } else if (response !is PayInvoiceSuccessResponse) { + // Undecryptable (null) or a shape we cannot read. Say so rather + // than leaving the zap looking like it silently worked. + onError( + stringRes(context, R.string.error_dialog_pay_invoice_error), + stringRes(context, R.string.wallet_connect_unreadable_response_error), + payable.info.user, + ) } }, + onTimeout = { + onError( + stringRes(context, R.string.error_dialog_pay_invoice_error), + stringRes( + context, + R.string.wallet_connect_no_response_error, + NwcSignerState.NWC_RESPONSE_TIMEOUT_SECONDS, + ), + payable.info.user, + ) + }, ) progress.step() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/invoice/InvoicePaymentDispatcher.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/invoice/InvoicePaymentDispatcher.kt index 3487f7fd44..be94452fc2 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/invoice/InvoicePaymentDispatcher.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/invoice/InvoicePaymentDispatcher.kt @@ -30,11 +30,12 @@ import androidx.compose.runtime.remember import androidx.compose.ui.platform.LocalContext import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.model.payments.PaymentSource +import com.vitorpamplona.amethyst.model.nip47WalletConnect.NwcSignerState import com.vitorpamplona.amethyst.ui.note.payViaIntent import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.quartz.lightning.LnInvoiceUtil -import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceErrorResponse +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.IErrorResponseLike import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceSuccessResponse /** @@ -85,16 +86,31 @@ fun InvoicePaymentDispatcher( onConfirm = { when (source) { is PaymentSource.Nwc -> - accountViewModel.sendZapPaymentRequestFor(bolt11, null) { response -> + accountViewModel.sendZapPaymentRequestFor( + bolt11 = bolt11, + zappedNote = null, + onTimeout = { + onError( + stringRes( + context, + R.string.wallet_connect_no_response_error, + NwcSignerState.NWC_RESPONSE_TIMEOUT_SECONDS, + ), + ) + }, + ) { response -> when (response) { is PayInvoiceSuccessResponse -> onSuccess() - is PayInvoiceErrorResponse -> + // IErrorResponseLike, not PayInvoiceErrorResponse: a wallet that + // omits `result_type` on an error still deserializes to something + // renderable, and the narrower check dropped it silently. + is IErrorResponseLike -> onError( - response.error?.message - ?: response.error?.code?.toString() + response.errorMessage() ?: stringRes(context, R.string.error_parsing_error_message), ) - else -> {} + else -> + onError(stringRes(context, R.string.wallet_connect_unreadable_response_error)) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 3ac12e21da..4c2361d48b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -2788,9 +2788,10 @@ class AccountViewModel( bolt11: String, zappedNote: Note?, onSent: () -> Unit = {}, + onTimeout: (() -> Unit)? = null, onResponse: (Response?) -> Unit, ) = launchSigner { - account.zaps.sendZapPaymentRequestFor(bolt11, zappedNote, onResponse) + account.zaps.sendZapPaymentRequestFor(bolt11, zappedNote, onTimeout, onResponse) onSent() } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/dvms/DvmContentDiscoveryScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/dvms/DvmContentDiscoveryScreen.kt index bf971317fa..94a2e7a201 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/dvms/DvmContentDiscoveryScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/dvms/DvmContentDiscoveryScreen.kt @@ -52,6 +52,7 @@ import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.model.payments.PaymentSource import com.vitorpamplona.amethyst.model.Note import com.vitorpamplona.amethyst.model.User +import com.vitorpamplona.amethyst.model.nip47WalletConnect.NwcSignerState import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.EventFinderFilterAssemblerSubscription import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNoteAndMap import com.vitorpamplona.amethyst.ui.components.LoadNote @@ -76,7 +77,8 @@ import com.vitorpamplona.amethyst.ui.theme.SimpleImage75Modifier import com.vitorpamplona.amethyst.ui.theme.Size35dp import com.vitorpamplona.quartz.lightning.LnInvoiceUtil import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter -import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceErrorResponse +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.IErrorResponseLike +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceSuccessResponse import com.vitorpamplona.quartz.nip89AppHandlers.definition.AppDefinitionEvent import com.vitorpamplona.quartz.nip89AppHandlers.definition.AppMetadata import com.vitorpamplona.quartz.nip90Dvms.contentDiscoveryResponse.NIP90ContentDiscoveryResponseEvent @@ -402,17 +404,31 @@ fun DvmPaymentActions( onSent = { onStatusUpdate(nwcPaymentRequest) }, + onTimeout = { + onStatusUpdate( + stringRes( + context, + R.string.wallet_connect_no_response_error, + NwcSignerState.NWC_RESPONSE_TIMEOUT_SECONDS, + ), + ) + }, onResponse = { response -> onStatusUpdate( - if (response is PayInvoiceErrorResponse) { - stringRes( - context, - R.string.wallet_connect_pay_invoice_error_error, - response.error?.message - ?: response.error?.code?.toString() ?: "Error parsing error message", - ) - } else { - thankYou + when (response) { + is PayInvoiceSuccessResponse -> thankYou + // IErrorResponseLike, not PayInvoiceErrorResponse: a wallet + // that omits `result_type` on an error still deserializes to + // something renderable, and the narrower check thanked the + // user for a payment that had just been refused. + is IErrorResponseLike -> + stringRes( + context, + R.string.wallet_connect_pay_invoice_error_error, + response.errorMessage() + ?: stringRes(context, R.string.error_parsing_error_message), + ) + else -> stringRes(context, R.string.wallet_connect_unreadable_response_error) }, ) }, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/payment/SendPaymentScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/payment/SendPaymentScreen.kt index 629f595cb9..60561e545d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/payment/SendPaymentScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/payment/SendPaymentScreen.kt @@ -52,6 +52,7 @@ import com.vitorpamplona.amethyst.model.DEFAULT_ONCHAIN_ZAP_SATS import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.MIN_ONCHAIN_ZAP_SATS import com.vitorpamplona.amethyst.model.User +import com.vitorpamplona.amethyst.model.nip47WalletConnect.NwcSignerState import com.vitorpamplona.amethyst.service.ClinkOfferPayer import com.vitorpamplona.amethyst.service.relayClient.reqCommand.user.UserFinderFilterAssemblerSubscription import com.vitorpamplona.amethyst.service.relayClient.reqCommand.user.observeUserInfo @@ -74,7 +75,7 @@ import com.vitorpamplona.quartz.experimental.clink.offers.OfferErrorCode import com.vitorpamplona.quartz.experimental.clink.pointers.ClinkPointerParser import com.vitorpamplona.quartz.experimental.clink.pointers.NOffer import com.vitorpamplona.quartz.experimental.clink.pointers.OfferPriceType -import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceErrorResponse +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.IErrorResponseLike import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceSuccessResponse import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent import com.vitorpamplona.quartz.nipBCOnchainZaps.chain.FeeEstimates @@ -355,6 +356,12 @@ private fun SendPaymentLoaded( val clinkNoResponseLabel = stringRes(R.string.clink_debit_no_response) val invoiceErrorLabel = stringRes(R.string.error_dialog_pay_invoice_error) val parsingErrorLabel = stringRes(R.string.error_parsing_error_message) + val unreadableResponseLabel = stringRes(R.string.wallet_connect_unreadable_response_error) + val noResponseLabel = + stringRes( + R.string.wallet_connect_no_response_error, + NwcSignerState.NWC_RESPONSE_TIMEOUT_SECONDS, + ) // Payment callbacks arrive on IO/relay threads. Snapshot state writes are // thread-safe, but every other payment flow in the app marshals UI state @@ -382,18 +389,21 @@ private fun SendPaymentLoaded( when (val source = pickedSource) { is PaymentSource.Nwc -> { postStage(PaymentFlowStage.InProgress(stringRes(context, R.string.send_payment_paying_via, source.name))) - accountViewModel.sendZapPaymentRequestFor(invoice, null) { response -> + accountViewModel.sendZapPaymentRequestFor( + bolt11 = invoice, + zappedNote = null, + onTimeout = { postStage(PaymentFlowStage.Failure(noResponseLabel)) }, + ) { response -> when (response) { is PayInvoiceSuccessResponse -> postStage(PaymentFlowStage.Success(successTitle)) - is PayInvoiceErrorResponse -> + // IErrorResponseLike, not PayInvoiceErrorResponse: a wallet that + // omits `result_type` on an error still deserializes to something + // renderable, and the narrower check dropped it silently. + is IErrorResponseLike -> postStage( - PaymentFlowStage.Failure( - response.error?.message - ?: response.error?.code?.toString() - ?: parsingErrorLabel, - ), + PaymentFlowStage.Failure(response.errorMessage() ?: parsingErrorLabel), ) - else -> {} + else -> postStage(PaymentFlowStage.Failure(unreadableResponseLabel)) } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/WalletTransactionsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/WalletTransactionsScreen.kt index 4e0b8be67d..9771c7ef36 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/WalletTransactionsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/WalletTransactionsScreen.kt @@ -41,6 +41,7 @@ import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect @@ -89,6 +90,7 @@ fun WalletTransactionsScreen( val isLoadingMore by walletViewModel.isLoadingMore.collectAsState() val hasMore by walletViewModel.hasMoreTransactions.collectAsState() val currentFilter by walletViewModel.transactionFilter.collectAsState() + val error by walletViewModel.error.collectAsState() val listState = rememberLazyListState() @@ -132,6 +134,7 @@ fun WalletTransactionsScreen( ) }, ) { padding -> + val currentError = error if (isLoading && transactions.isEmpty()) { Column( modifier = @@ -148,6 +151,34 @@ fun WalletTransactionsScreen( style = MaterialTheme.typography.bodyLarge, ) } + } else if (currentError != null && transactions.isEmpty()) { + // A wallet refusal (e.g. RESTRICTED) leaves the list empty. Without this + // branch the screen would claim "no transactions yet" and hide the reason. + Column( + modifier = + Modifier + .padding(padding) + .fillMaxSize() + .padding(24.dp), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center, + ) { + Text( + text = stringRes(R.string.wallet_transactions_load_failed), + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.error, + ) + Spacer(modifier = Modifier.height(8.dp)) + Text( + text = currentError, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + Spacer(modifier = Modifier.height(16.dp)) + TextButton(onClick = { walletViewModel.fetchTransactions() }) { + Text(stringRes(R.string.wallet_refresh)) + } + } } else if (transactions.isEmpty()) { Column( modifier = @@ -168,6 +199,21 @@ fun WalletTransactionsScreen( modifier = Modifier.padding(padding), state = listState, ) { + if (currentError != null) { + // Already-loaded transactions stay visible; the banner explains why + // the latest refresh or page load did not land. + item { + Text( + text = currentError, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + modifier = + Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp, vertical = 8.dp), + ) + } + } item { TransactionFilterRow(currentFilter) { walletViewModel.setTransactionFilter(it) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/WalletViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/WalletViewModel.kt index 15e1150248..ccbce839f5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/WalletViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/WalletViewModel.kt @@ -533,6 +533,7 @@ class WalletViewModel : ViewModel() { val walletUri = getWalletUri(walletId) ?: return viewModelScope.launch(Dispatchers.IO) { _isLoading.value = true + _error.value = null _hasMoreTransactions.value = true var requestId: HexKey? = null val timeoutJob = launchTimeout({ requestId }) { _isLoading.value = false } @@ -611,6 +612,7 @@ class WalletViewModel : ViewModel() { } else { newTxs.size >= pageSize } + _error.value = null } is NwcErrorResponse -> { diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 88f8bd5ea1..63747b2dac 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -3055,6 +3055,8 @@ Unable to fetch invoice from receiver\'s servers Your wallet connect provider returned the following error: %1$s + Your wallet did not respond within %1$d seconds. The payment may or may not have gone through — check your wallet before retrying. + Your wallet connect provider sent a reply Amethyst could not read. Tor isn\'t connecting Amethyst couldn\'t finish bootstrapping Tor. Use a regular (non-Tor) connection so the feed can load? We\'ll remember this choice for the next hour and re-try Tor after that. @@ -3210,6 +3212,7 @@ Received Sent Refresh + Could not load transactions All Zaps Non-Zaps diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/nwc/NwcPaymentTracker.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/nwc/NwcPaymentTracker.kt index ab6b09448d..c4ec34a70d 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/nwc/NwcPaymentTracker.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/nwc/NwcPaymentTracker.kt @@ -142,10 +142,13 @@ class NwcPaymentTracker { /** * Manually removes a pending request (e.g., on timeout). + * + * Returns true when this call is the one that removed it. Callers racing a + * response use that to decide who reports the outcome: the response path + * consumes the entry through [onResponseReceived], so a timeout that gets + * false must stay quiet rather than overwrite a real wallet answer. */ - fun cleanup(requestId: HexKey) { - awaitingRequests.remove(requestId) - } + fun cleanup(requestId: HexKey): Boolean = awaitingRequests.remove(requestId) != null /** * Returns count of pending requests (for debugging/monitoring). diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/service/nwc/NwcPaymentTrackerTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/service/nwc/NwcPaymentTrackerTest.kt new file mode 100644 index 0000000000..5937a908b5 --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/service/nwc/NwcPaymentTrackerTest.kt @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.service.nwc + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertIs +import kotlin.test.assertTrue + +/** + * [NwcPaymentTracker.cleanup] is what decides who gets to tell the user how a NIP-47 + * request ended. The give-up path and the response path race for the same entry, and + * exactly one of them must speak — otherwise the user either hears nothing (the bug + * that motivated this) or hears "timed out" over a refusal the wallet did send. + */ +class NwcPaymentTrackerTest { + private val requestId = "a".repeat(64) + private val walletPubkey = "b".repeat(64) + private val attackerPubkey = "c".repeat(64) + + private fun trackerWithPendingRequest(): NwcPaymentTracker = + NwcPaymentTracker().apply { + registerRequest(requestId, walletPubkey, null) { } + } + + @Test + fun cleanupReportsWhoRemovedTheEntry() { + val tracker = trackerWithPendingRequest() + + assertTrue(tracker.cleanup(requestId), "the first give-up owns the entry and must report") + assertFalse(tracker.cleanup(requestId), "a second give-up must stay quiet") + assertEquals(0, tracker.pendingCount()) + } + + @Test + fun cleanupStaysQuietAfterAResponseConsumedTheRequest() { + val tracker = trackerWithPendingRequest() + + assertIs(tracker.onResponseReceived(requestId, walletPubkey)) + assertFalse( + tracker.cleanup(requestId), + "the response already reported; a timeout firing afterwards must not overwrite it", + ) + } + + @Test + fun cleanupStillReportsWhenOnlySpoofedRepliesArrived() { + val tracker = trackerWithPendingRequest() + + // A wrong-author reply is dropped and deliberately leaves the request pending, + // so the give-up path is the only thing that can tell the user anything. + assertIs(tracker.onResponseReceived(requestId, attackerPubkey)) + assertEquals(1, tracker.spoofAttemptsFor(requestId)) + assertTrue(tracker.cleanup(requestId)) + } + + @Test + fun cleanupOfAnUnknownRequestReportsNothing() { + assertFalse(NwcPaymentTracker().cleanup(requestId)) + } +} diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nwc/NwcPaymentHandler.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nwc/NwcPaymentHandler.kt index f21c341df8..4fcd463061 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nwc/NwcPaymentHandler.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nwc/NwcPaymentHandler.kt @@ -32,9 +32,9 @@ import com.vitorpamplona.quartz.nip47WalletConnect.Nip47WalletConnect import com.vitorpamplona.quartz.nip47WalletConnect.events.LnZapPaymentRequestEvent import com.vitorpamplona.quartz.nip47WalletConnect.events.LnZapPaymentResponseEvent import com.vitorpamplona.quartz.nip47WalletConnect.rpc.GetBalanceSuccessResponse +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.IErrorResponseLike import com.vitorpamplona.quartz.nip47WalletConnect.rpc.MakeInvoiceSuccessResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.NwcErrorResponse -import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceErrorResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceSuccessResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.Response import kotlinx.coroutines.launch @@ -181,8 +181,11 @@ class NwcPaymentHandler( PaymentResult.Success(response.result?.preimage) } - is PayInvoiceErrorResponse -> { - PaymentResult.Error(response.error?.message ?: "Unknown error") + // IErrorResponseLike, not PayInvoiceErrorResponse: a wallet that omits + // `result_type` on an error deserializes to the generic shape, and the + // narrower check reported "unexpected response type" instead of the reason. + is IErrorResponseLike -> { + PaymentResult.Error(response.errorMessage() ?: "Unknown error") } else -> { diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/rpc/Response.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/rpc/Response.kt index 363c65b47b..b109074f68 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/rpc/Response.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/rpc/Response.kt @@ -31,6 +31,10 @@ abstract class Response( * NIP-47 responses that carry a human-readable error message. Lets UI * collapse generic NwcErrorResponse and method-specific *ErrorResponse * handling into one branch. + * + * [errorMessage] falls back to the machine-readable code name when the wallet + * omits `message`, because NIP-47 only requires `code`. Returning null there + * would leave the UI with nothing to show for a perfectly conforming refusal. */ interface IErrorResponseLike { fun errorMessage(): String? @@ -42,7 +46,7 @@ class NwcErrorResponse( val error: NwcError? = null, ) : Response(resultType), IErrorResponseLike { - override fun errorMessage() = error?.message + override fun errorMessage() = error?.message ?: error?.code?.name } // pay_invoice success response @@ -65,7 +69,7 @@ class PayInvoiceErrorResponse( val message: String? = null, ) - override fun errorMessage() = error?.message + override fun errorMessage() = error?.message ?: error?.code?.name } // pay success response (nostr-wallet-connect/nwc#2). For a settled BOLT12 payment diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/ResponseTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/ResponseTest.kt index 49951ead95..be2824b4c9 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/ResponseTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/ResponseTest.kt @@ -26,6 +26,7 @@ import com.vitorpamplona.quartz.nip47WalletConnect.rpc.CreateConnectionSuccessRe import com.vitorpamplona.quartz.nip47WalletConnect.rpc.GetBalanceSuccessResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.GetBudgetSuccessResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.GetInfoSuccessResponse +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.IErrorResponseLike import com.vitorpamplona.quartz.nip47WalletConnect.rpc.ListTransactionsSuccessResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.LookupInvoiceSuccessResponse import com.vitorpamplona.quartz.nip47WalletConnect.rpc.MakeHoldInvoiceSuccessResponse @@ -444,6 +445,57 @@ class ResponseTest { assertEquals(NwcErrorCode.EXPIRED, response.error?.code) } + // --- Every refusal must be renderable (IErrorResponseLike) --- + // + // Field report, BrollyZapper 2026-08-25: a QUOTA_EXCEEDED on pay_invoice and a + // RESTRICTED on list_transactions both reached the phone and showed nothing. + // These pin the two payloads to a branch the UI can render. + + @Test + fun testQuotaExceededPayInvoiceIsRenderable() { + val json = + """{"result_type":"pay_invoice","error":{"code":"QUOTA_EXCEEDED",""" + + """"message":"this payment would exceed the connection's budget for this period"}}""" + val response = OptimizedJsonMapper.fromJsonTo(json) + assertIs(response) + assertIs(response) + assertEquals(NwcErrorCode.QUOTA_EXCEEDED, response.error?.code) + assertEquals( + "this payment would exceed the connection's budget for this period", + (response as IErrorResponseLike).errorMessage(), + ) + } + + @Test + fun testRestrictedListTransactionsIsRenderable() { + val json = """{"result_type":"list_transactions","error":{"code":"RESTRICTED","message":"sending is disabled on this node"}}""" + val response = OptimizedJsonMapper.fromJsonTo(json) + // Not a pay_invoice, so the deserializer yields the generic shape. It must + // still be renderable, or the transactions screen shows "no transactions yet". + assertIs(response) + assertIs(response) + assertEquals("sending is disabled on this node", (response as IErrorResponseLike).errorMessage()) + } + + @Test + fun testErrorWithoutResultTypeIsRenderable() { + // NIP-47 does not require a wallet to echo result_type on an error. + val json = """{"error":{"code":"RESTRICTED","message":"sending is disabled on this node"}}""" + val response = OptimizedJsonMapper.fromJsonTo(json) + assertIs(response) + assertEquals("sending is disabled on this node", (response as IErrorResponseLike).errorMessage()) + } + + @Test + fun testErrorMessageFallsBackToCodeNameWhenMessageMissing() { + // `message` is optional in NIP-47; `code` alone must still produce text. + val payInvoice = OptimizedJsonMapper.fromJsonTo("""{"result_type":"pay_invoice","error":{"code":"QUOTA_EXCEEDED"}}""") + assertEquals("QUOTA_EXCEEDED", (payInvoice as IErrorResponseLike).errorMessage()) + + val generic = OptimizedJsonMapper.fromJsonTo("""{"result_type":"list_transactions","error":{"code":"RESTRICTED"}}""") + assertEquals("RESTRICTED", (generic as IErrorResponseLike).errorMessage()) + } + // --- Null/missing result --- @Test