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