From 52d7d3cae44c7971394e1390da989af5b56ecd81 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Wed, 2 Sep 2026 02:17:47 +0200 Subject: [PATCH 1/8] fix: expose payment request failures --- .../PaymentRequestsScreenTest.kt | 8 +- .../repositories/PaykitPaymentRequestRepo.kt | 173 +++++++++++++----- .../bitkit/repositories/PrivatePaykitRepo.kt | 2 - .../bitkit/repositories/PublicPaykitRepo.kt | 28 +++ .../paymentrequests/PaymentRequestsScreen.kt | 10 +- .../java/to/bitkit/viewmodels/AppViewModel.kt | 123 ++++++++++--- app/src/main/res/values/strings.xml | 1 + .../PaykitPaymentRequestRepoTest.kt | 53 +++++- .../repositories/PublicPaykitRepoTest.kt | 20 ++ .../viewmodels/AppViewModelSendFlowTest.kt | 57 +++++- changelog.d/next/1209.fixed.md | 1 + docs/payment-requests.md | 36 ++++ .../requested-resolution-failure.xml | 21 +++ 13 files changed, 446 insertions(+), 87 deletions(-) create mode 100644 changelog.d/next/1209.fixed.md create mode 100644 docs/payment-requests.md create mode 100644 journeys/payment-requests/requested-resolution-failure.xml diff --git a/app/src/androidTest/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreenTest.kt b/app/src/androidTest/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreenTest.kt index 35fd856b3d..fb872e7f47 100644 --- a/app/src/androidTest/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreenTest.kt +++ b/app/src/androidTest/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreenTest.kt @@ -47,7 +47,9 @@ class PaymentRequestsScreenTest { } } - composeTestRule.onNodeWithTag("PaymentRequestRowincoming").assertIsDisplayed() + composeTestRule.onNodeWithTag("PaymentRequestRow-incoming").assertIsDisplayed() + composeTestRule.onNodeWithTag("PaymentRequestDismiss-incoming").assertIsDisplayed() + composeTestRule.onNodeWithTag("PaymentRequestPay-incoming").assertIsDisplayed() composeTestRule.onNodeWithTag("MoneyPrimary").assertIsDisplayed() composeTestRule.onNodeWithTag("MoneySecondary").assertIsDisplayed() composeTestRule.onNodeWithTag("PaymentRequestsSeeAll").assertIsDisplayed() @@ -105,8 +107,8 @@ class PaymentRequestsScreenTest { } } - composeTestRule.onNodeWithTag("PaymentRequestRowaccepted").assertIsDisplayed() - composeTestRule.onNodeWithTag("PaymentRequestRowoutgoing").assertIsDisplayed() + composeTestRule.onNodeWithTag("PaymentRequestRow-accepted").assertIsDisplayed() + composeTestRule.onNodeWithTag("PaymentRequestRow-outgoing").assertIsDisplayed() composeTestRule.onNodeWithText("Waiting for", substring = true).assertIsDisplayed() composeTestRule.onNodeWithText("PAYMENT REQUESTS").assertIsDisplayed() composeTestRule.onNodeWithText("TODAY").assertIsDisplayed() diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt index 4159a4d5d4..d8b89f7d6c 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt @@ -8,6 +8,7 @@ import com.synonym.paykit.OutboundPrivateMessageStatus import com.synonym.paykit.PaymentRequestLifecycleState import com.synonym.paykit.PaymentRequestLocalRole import com.synonym.paykit.PaymentRequestRecord +import com.synonym.paykit.PaymentRequestTerms import com.synonym.paykit.PrivateJsonObject import com.synonym.paykit.PrivateStreamCounterpartyIntakeReport import kotlinx.coroutines.CoroutineDispatcher @@ -75,6 +76,23 @@ data class PaykitPaymentRequest( val direction: PaykitPaymentRequestDirection = PaykitPaymentRequestDirection.Incoming, val lifecycleState: PaymentRequestLifecycleState = PaymentRequestLifecycleState.PROPOSED, ) { + enum class ParseFailure( + val logValue: String, + val shouldLogIncomingRejection: Boolean = true, + ) { + MissingLocalRole("missing_local_role"), + UnsupportedLocalRole("unsupported_local_role", shouldLogIncomingRejection = false), + NonActionableState("non_actionable_state", shouldLogIncomingRejection = false), + MissingTerms("missing_terms"), + RecurringRequest("recurring_request"), + UnsupportedAsset("unsupported_asset"), + InvalidAmount("invalid_amount"), + AmountOutOfRange("amount_out_of_range"), + NoSupportedEndpoint("no_supported_endpoint"), + InvalidExpiration("invalid_expiration"), + Expired("expired"), + } + val id: PaykitPaymentRequestId get() = PaykitPaymentRequestId(paymentRequestId, counterparty, counterpartyReceiverPath) @@ -89,6 +107,18 @@ data class PaykitPaymentRequest( fun acceptsPaymentAmount(amountSats: ULong): Boolean = amountSats == this.amountSats } +internal sealed interface PaykitPaymentRequestParseResult { + data class Parsed(val request: PaykitPaymentRequest) : PaykitPaymentRequestParseResult + data class Rejected(val reason: PaykitPaymentRequest.ParseFailure) : PaykitPaymentRequestParseResult +} + +private data class ParsedPaykitPaymentRequestTerms( + val terms: PaymentRequestTerms, + val amountSats: ULong, + val endpoints: List, + val expiresAt: Instant?, +) + enum class PaykitPaymentRequestDeliveryStatus { Queued, Sent } enum class PaykitPaymentRequestDirection { Incoming, Outgoing } @@ -383,7 +413,17 @@ class PaykitPaymentRequestRepo @Inject constructor( paykitSdkService.receivePrivateMessagesFromLinkedPeers().also(::logIntakeFailures) val now = clock.now() val records = paykitSdkService.paymentRequests() - val incoming = records.mapNotNull { it.toPaykitPaymentRequest(PaymentRequestLocalRole.PAYER, now) } + val incoming = records.mapNotNull { record -> + when (val result = record.parseIncomingPaykitPaymentRequest(now)) { + is PaykitPaymentRequestParseResult.Parsed -> result.request + is PaykitPaymentRequestParseResult.Rejected -> { + if (result.reason.shouldLogIncomingRejection) { + Logger.warn(record.incomingPaymentRequestRejectionLog(result.reason), context = TAG) + } + null + } + } + } val history = records.mapNotNull { it.toPaykitPaymentRequestHistory(now) } .sortedByDescending { it.createdAt } if ( @@ -613,65 +653,112 @@ private fun List.withExpiredLifecycle(now: Instant): List< private val bitcoinAmountPattern = Regex("(?:[0-9]+(?:\\.[0-9]*)?|\\.[0-9]+)") +internal fun PaymentRequestRecord.parseIncomingPaykitPaymentRequest( + now: Instant, +): PaykitPaymentRequestParseResult = parsePaykitPaymentRequest( + expectedRole = PaymentRequestLocalRole.PAYER, + now = now, + requiresActionableRequest = true, +) + +internal fun PaymentRequestRecord.incomingPaymentRequestRejectionLog( + reason: PaykitPaymentRequest.ParseFailure, +): String = "Rejected incoming Paykit payment request: category='parse' reason='${reason.logValue}' " + + "counterparty='${PubkyPublicKeyFormat.redacted(counterparty)}'" + @Suppress("CyclomaticComplexMethod", "ReturnCount") -private fun PaymentRequestRecord.toPaykitPaymentRequest( +private fun PaymentRequestRecord.parsePaykitPaymentRequest( expectedRole: PaymentRequestLocalRole, now: Instant, requiresActionableRequest: Boolean = true, -): PaykitPaymentRequest? { - if (localRole != expectedRole || state == PaymentRequestLifecycleState.ACTIVE_RECURRING) return null - if (requiresActionableRequest && state != PaymentRequestLifecycleState.PROPOSED) return null - val requestTerms = terms ?: return null - if (requestTerms.recurrence != null || requestTerms.amount.asset != "btc") return null +): PaykitPaymentRequestParseResult { + val role = localRole + ?: return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.MissingLocalRole) + if (role != expectedRole) { + return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.UnsupportedLocalRole) + } + if (requiresActionableRequest && state != PaymentRequestLifecycleState.PROPOSED) { + return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.NonActionableState) + } + if (state == PaymentRequestLifecycleState.ACTIVE_RECURRING) { + return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.RecurringRequest) + } + val requestTerms = terms + ?: return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.MissingTerms) + if (requestTerms.recurrence != null) { + return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.RecurringRequest) + } + if (requestTerms.amount.asset != "btc") { + return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.UnsupportedAsset) + } val amountSats = requestTerms.amount.value.toSats() - ?.takeIf { it <= ULong.MAX_VALUE / 1000uL } - ?: return null + ?: return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.InvalidAmount) + if (amountSats > ULong.MAX_VALUE / 1000uL) { + return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.AmountOutOfRange) + } val endpoints = requestTerms.acceptedPaymentEndpointIdentifiers .filter { MethodId.fromRawValue(it) != null } .distinct() - if (requiresActionableRequest && endpoints.isEmpty()) return null + if (requiresActionableRequest && endpoints.isEmpty()) { + return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.NoSupportedEndpoint) + } val expiresAt = requestTerms.proposalExpiresAt?.let { - runCatching { Instant.parse(it) }.getOrNull() ?: return null + runCatching { Instant.parse(it) }.getOrNull() + ?: return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.InvalidExpiration) + } + if (requiresActionableRequest && expiresAt != null && expiresAt <= now) { + return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.Expired) } - if (requiresActionableRequest && expiresAt != null && expiresAt <= now) return null - return PaykitPaymentRequest( - paymentRequestId = paymentRequestId, - counterparty = counterparty, - counterpartyReceiverPath = counterpartyReceiverPath, - amountValue = requestTerms.amount.value, - amountSats = amountSats, - note = requestTerms.metadata.note(), - createdAt = lastEventAt?.let { runCatching { Instant.parse(it) }.getOrNull() }, - expiresAt = expiresAt, - acceptedPaymentEndpointIdentifiers = endpoints, - deliveryStatus = if (expectedRole == PaymentRequestLocalRole.PAYEE) { - if (proposalOutboundStatus == OutboundPrivateMessageStatus.SENT) { - PaykitPaymentRequestDeliveryStatus.Sent - } else { - PaykitPaymentRequestDeliveryStatus.Queued - } - } else { - null - }, - direction = if (expectedRole == PaymentRequestLocalRole.PAYER) { - PaykitPaymentRequestDirection.Incoming - } else { - PaykitPaymentRequestDirection.Outgoing - }, - lifecycleState = if (state == PaymentRequestLifecycleState.PROPOSED && expiresAt?.let { it <= now } == true) { - PaymentRequestLifecycleState.PROPOSAL_EXPIRED - } else { - state - }, - ) + val parsedTerms = ParsedPaykitPaymentRequestTerms(requestTerms, amountSats, endpoints, expiresAt) + return PaykitPaymentRequestParseResult.Parsed(toPaykitPaymentRequest(expectedRole, parsedTerms, now)) } +private fun PaymentRequestRecord.toPaykitPaymentRequest( + expectedRole: PaymentRequestLocalRole, + parsedTerms: ParsedPaykitPaymentRequestTerms, + now: Instant, +) = PaykitPaymentRequest( + paymentRequestId = paymentRequestId, + counterparty = counterparty, + counterpartyReceiverPath = counterpartyReceiverPath, + amountValue = parsedTerms.terms.amount.value, + amountSats = parsedTerms.amountSats, + note = parsedTerms.terms.metadata.note(), + createdAt = lastEventAt?.let { runCatching { Instant.parse(it) }.getOrNull() }, + expiresAt = parsedTerms.expiresAt, + acceptedPaymentEndpointIdentifiers = parsedTerms.endpoints, + deliveryStatus = if (expectedRole == PaymentRequestLocalRole.PAYEE) { + if (proposalOutboundStatus == OutboundPrivateMessageStatus.SENT) { + PaykitPaymentRequestDeliveryStatus.Sent + } else { + PaykitPaymentRequestDeliveryStatus.Queued + } + } else { + null + }, + direction = if (expectedRole == PaymentRequestLocalRole.PAYER) { + PaykitPaymentRequestDirection.Incoming + } else { + PaykitPaymentRequestDirection.Outgoing + }, + lifecycleState = if ( + state == PaymentRequestLifecycleState.PROPOSED && parsedTerms.expiresAt?.let { it <= now } == true + ) { + PaymentRequestLifecycleState.PROPOSAL_EXPIRED + } else { + state + }, +) + private fun PaymentRequestRecord.toPaykitPaymentRequestHistory(now: Instant): PaykitPaymentRequest? { val role = localRole ?: return null if (role == PaymentRequestLocalRole.UNKNOWN) return null - return toPaykitPaymentRequest(role, now, requiresActionableRequest = false) + return when (val result = parsePaykitPaymentRequest(role, now, requiresActionableRequest = false)) { + is PaykitPaymentRequestParseResult.Parsed -> result.request + is PaykitPaymentRequestParseResult.Rejected -> null + } } private fun PaymentRequestRecord.toCreatedPaykitPaymentRequest( diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt index 10b24fac99..30edda3bdb 100644 --- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt @@ -371,8 +371,6 @@ class PrivatePaykitRepo @Inject constructor( val publicKey = normalizedPublicKey(request.counterparty) ?: throw PrivatePaykitError.InvalidPublicKey beginContactPayment(publicKey, request).getOrThrow() } - }.onFailure { - Logger.warn("Failed to present incoming Paykit payment request", it, context = TAG) } suspend fun consumePrivatePaymentList( diff --git a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt index 91e277bb9c..18e9a63d12 100644 --- a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt @@ -57,6 +57,34 @@ sealed interface PublicPaykitPaymentResult { data object WaitingForUpdatedPaymentList : PublicPaykitPaymentResult } +internal enum class IncomingPaykitPaymentRequestFailureReason( + val logValue: String, +) { + NoSupportedEndpoint("no_supported_endpoint"), + EndpointNotPayable("endpoint_not_payable"), + PaymentDetailsPending("payment_details_pending"), + InvalidPaymentTarget("invalid_payment_target"), + PaymentTargetNotRoutable("payment_target_not_routable"), + ResolutionFailed("resolution_failed"), + ; + + val category: String + get() = when (this) { + NoSupportedEndpoint, EndpointNotPayable, PaymentDetailsPending, ResolutionFailed -> "resolution" + InvalidPaymentTarget, PaymentTargetNotRoutable -> "presentation" + } +} + +internal val PublicPaykitPaymentResult.incomingPaymentRequestFailureReason: + IncomingPaykitPaymentRequestFailureReason? + get() = when (this) { + is PublicPaykitPaymentResult.Opened -> null + PublicPaykitPaymentResult.NoEndpoint -> IncomingPaykitPaymentRequestFailureReason.NoSupportedEndpoint + PublicPaykitPaymentResult.NotOpened -> IncomingPaykitPaymentRequestFailureReason.EndpointNotPayable + PublicPaykitPaymentResult.WaitingForUpdatedPaymentList -> + IncomingPaykitPaymentRequestFailureReason.PaymentDetailsPending + } + data class PrivatePaykitPaymentContext( val receiverPath: String, val paymentListVersion: ULong, diff --git a/app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt b/app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt index abdab879f5..a561a2d3cf 100644 --- a/app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt +++ b/app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt @@ -490,7 +490,7 @@ internal fun PaymentRequestCard( Modifier } ) - .testTag("PaymentRequestRow${request.paymentRequestId}"), + .testTag("PaymentRequestRow-${request.paymentRequestId}") ) { Row( verticalAlignment = Alignment.CenterVertically, @@ -539,7 +539,9 @@ internal fun PaymentRequestCard( ) }, size = ButtonSize.Small, - modifier = Modifier.weight(1f), + modifier = Modifier + .weight(1f) + .testTag("PaymentRequestDismiss-${request.paymentRequestId}") ) PrimaryButton( text = stringResource(R.string.wallet__payment_request_pay), @@ -553,7 +555,9 @@ internal fun PaymentRequestCard( ) }, size = ButtonSize.Small, - modifier = Modifier.weight(1f), + modifier = Modifier + .weight(1f) + .testTag("PaymentRequestPay-${request.paymentRequestId}") ) } } diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index dae74658dc..72f374c70f 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -141,6 +141,7 @@ import to.bitkit.repositories.ConnectivityState import to.bitkit.repositories.CurrencyRepo import to.bitkit.repositories.HealthRepo import to.bitkit.repositories.HwWalletRepo +import to.bitkit.repositories.IncomingPaykitPaymentRequestFailureReason import to.bitkit.repositories.LightningRepo import to.bitkit.repositories.LnurlPayInvoiceMismatchError import to.bitkit.repositories.MethodId @@ -170,6 +171,7 @@ import to.bitkit.repositories.SamRockRepo import to.bitkit.repositories.TransferRepo import to.bitkit.repositories.WalletRepo import to.bitkit.repositories.WidgetsRepo +import to.bitkit.repositories.incomingPaymentRequestFailureReason import to.bitkit.services.AppUpdaterService import to.bitkit.services.CoreService import to.bitkit.services.MigrationService @@ -345,6 +347,7 @@ class AppViewModel @Inject constructor( private var activeContactPaymentContext: ContactPaymentContext? = null private val pendingContactPaymentContexts = mutableMapOf() private var requestedPaymentRequestId: PaykitPaymentRequestId? = null + private var shouldRestorePaymentRequestSheet = false private var preparedContactPaymentContext: ContactPaymentContext? = null private var isPresentingPaymentRequest = false private var paymentRequestPresentationGeneration = 0L @@ -633,7 +636,7 @@ class AppViewModel @Inject constructor( invalidatePaymentRequestPresentation(dismissActiveRequest = paymentRequestIdentity != null) clearPaymentRequestPresentationRetries() paymentRequestIdentity = null - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() paymentRequestSheetTransitionJob?.cancel() paymentRequestSheetTransitionJob = null try { @@ -649,7 +652,7 @@ class AppViewModel @Inject constructor( if (identityChanged) { invalidatePaymentRequestPresentation(dismissActiveRequest = paymentRequestIdentity != null) clearPaymentRequestPresentationRetries() - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() paymentRequestSheetTransitionJob?.cancel() paymentRequestSheetTransitionJob = null } @@ -818,7 +821,7 @@ class AppViewModel @Inject constructor( if (currentSheet.value !is Sheet.Send || activeIncomingPaymentRequest()?.id != request.id) return@launch if (paykitPaymentRequestRepo.markPresented(request)) { paymentRequestPresentationGeneration++ - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() clearPaymentRequestPresentationRetry(request.id) } } @@ -859,7 +862,7 @@ class AppViewModel @Inject constructor( val request = paykitPaymentRequestRepo.pendingRequest(requestedId) if (request != null) return listOf(request) invalidatePaymentRequestPresentation() - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() return null } @@ -873,19 +876,24 @@ class AppViewModel @Inject constructor( request: PaykitPaymentRequest, generation: Long, ): Boolean { - val result = privatePaykitRepo.beginPaymentRequest(request).getOrNull() + val presentationResult = privatePaykitRepo.beginPaymentRequest(request) + val result = presentationResult.getOrNull() if (!isCurrentPaymentRequestPresentation(request, generation) || isPaymentRequestPresentationBlocked()) { return true } if (!paykitPaymentRequestRepo.isPending(request)) { if (requestedPaymentRequestId == request.id) { invalidatePaymentRequestPresentation() - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() } return false } if (result !is PublicPaykitPaymentResult.Opened) { - deferPaymentRequestPresentation(request) + deferPaymentRequestPresentation( + request = request, + reason = result?.incomingPaymentRequestFailureReason + ?: IncomingPaykitPaymentRequestFailureReason.ResolutionFailed, + ) return false } @@ -904,17 +912,34 @@ class AppViewModel @Inject constructor( !paykitPaymentRequestRepo.isProcessing(request) && (requestedPaymentRequestId?.let { it == request.id } ?: true) - private fun deferPaymentRequestPresentation(request: PaykitPaymentRequest) { + private fun deferPaymentRequestPresentation( + request: PaykitPaymentRequest, + reason: IncomingPaykitPaymentRequestFailureReason, + ) { + Logger.warn( + "Rejected incoming Paykit payment request presentation: category='${reason.category}' " + + "reason='${reason.logValue}' " + + "counterparty='${PubkyPublicKeyFormat.redacted(request.counterparty)}'", + context = TAG, + ) val attempt = paymentRequestPresentationRetryAttempts[request.id] ?: 0 val retryDelay = PAYKIT_PAYMENT_REQUEST_PRESENTATION_RETRY_DELAYS.getOrNull(attempt) ?: if (requestedPaymentRequestId == request.id) { Logger.warn( - "Giving up requested payment request presentation after '${attempt + 1}' attempts", + "Stopped retrying requested incoming Paykit payment request after " + + "'${attempt + 1}' presentation attempts", context = TAG, ) + val restorePaymentRequestSheet = shouldRestorePaymentRequestSheet paymentRequestPresentationGeneration++ - requestedPaymentRequestId = null - showSheet(Sheet.PaymentRequests) + clearRequestedPaymentRequestPresentation() + toast( + type = Toast.ToastType.ERROR, + title = context.getString(R.string.wallet__payment_request), + description = context.getString(R.string.wallet__payment_request_unavailable), + testTag = "PaymentRequestUnavailableToast", + ) + if (restorePaymentRequestSheet) showSheet(Sheet.PaymentRequests) viewModelScope.launch { paykitPaymentRequestRepo.markPresented(request) } @@ -947,10 +972,15 @@ class AppViewModel @Inject constructor( } if (requestedPaymentRequestId?.let { it !in requestIds } == true) { invalidatePaymentRequestPresentation() - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() } } + private fun clearRequestedPaymentRequestPresentation() { + requestedPaymentRequestId = null + shouldRestorePaymentRequestSheet = false + } + private fun clearPaymentRequestPresentationRetry(requestId: PaykitPaymentRequestId) { paymentRequestPresentationRetryAttempts.remove(requestId) paymentRequestPresentationRetryJobs.remove(requestId)?.cancel() @@ -2470,6 +2500,9 @@ class AppViewModel @Inject constructor( // TODO Workaround for https://github.com/synonymdev/bitkit-core/issues/63 if (Bip21Utils.isDuplicatedBip21(input)) { + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) hideSheet() toast( type = Toast.ToastType.ERROR, @@ -2477,7 +2510,6 @@ class AppViewModel @Inject constructor( description = context.getString(R.string.other__scan__error__generic), testTag = "DuplicatedBip21Toast", ) - clearActiveContactPaymentContext() return@withContext } @@ -2492,7 +2524,9 @@ class AppViewModel @Inject constructor( } if (input.startsWith("$PUBKYAUTH_SCHEME://", ignoreCase = true)) { - clearActiveContactPaymentContext() + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) if (!fromMainScanner) { hideSheet() toast( @@ -2522,7 +2556,9 @@ class AppViewModel @Inject constructor( ) if (route != null) { - clearActiveContactPaymentContext() + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) if (currentSheet.value is Sheet.Send) hideSheet() mainScreenEffect(MainScreenEffect.Navigate(route)) if (route is Routes.ContactDetail) { @@ -2553,7 +2589,9 @@ class AppViewModel @Inject constructor( title = context.getString(R.string.hardware__send_onchain_only_title), description = context.getString(R.string.hardware__send_onchain_only_text), ) - clearActiveContactPaymentContext() + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.PaymentTargetNotRoutable, + ) return } @@ -2567,6 +2605,9 @@ class AppViewModel @Inject constructor( is Scanner.NodeId -> handleNonPaymentScan { onScanNodeId(scan) } is Scanner.Gift -> handleNonPaymentScan { onScanGift(scan.code, scan.amount) } else -> { + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) hideSheet() Logger.warn( if (scan == null) "Failed to decode scan data" else "Received unhandled scan data '$scan'", @@ -2577,13 +2618,14 @@ class AppViewModel @Inject constructor( title = context.getString(R.string.other__qr_error_header), description = context.getString(R.string.other__qr_error_text), ) - clearActiveContactPaymentContext() } } } private suspend fun handleSamRockSetup(setup: SamRockSetupRequest) { - clearActiveContactPaymentContext() + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) if (!setup.requestsBitcoinOnchain) { hideSheet() @@ -2600,7 +2642,9 @@ class AppViewModel @Inject constructor( } private suspend fun handleInvalidSamRockSetup(input: String) { - clearActiveContactPaymentContext() + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) hideSheet() val descriptionRes = when { SamRockSetupRequest.isPublicHttpProtocolUrl(input) -> R.string.btcpay__unsupported_http_text @@ -2615,11 +2659,22 @@ class AppViewModel @Inject constructor( } private suspend fun handleNonPaymentScan(action: suspend () -> Unit) { - clearActiveContactPaymentContext() + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) action() } - fun clearActiveContactPaymentContext(retryIncomingRequest: Boolean = true) { + fun clearActiveContactPaymentContext(retryIncomingRequest: Boolean = true) = + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.ResolutionFailed, + retryIncomingRequest = retryIncomingRequest, + ) + + private fun clearActiveContactPaymentContext( + failureReason: IncomingPaykitPaymentRequestFailureReason, + retryIncomingRequest: Boolean = true, + ) { val interruptedRequest = synchronized(contactPaymentContextLock) { val request = activeContactPaymentContext?.incomingPaymentRequest activeContactPaymentContext = null @@ -2631,7 +2686,7 @@ class AppViewModel @Inject constructor( if (!retryIncomingRequest) { paymentRequestPresentationGeneration++ if (requestedPaymentRequestId == interruptedRequest.id) { - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() } clearPaymentRequestPresentationRetry(interruptedRequest.id) viewModelScope.launch { paykitPaymentRequestRepo.markPresented(interruptedRequest) } @@ -2642,7 +2697,7 @@ class AppViewModel @Inject constructor( requestedPaymentRequestId == interruptedRequest.id || paykitPaymentRequestRepo.automaticPendingRequests().any { it.id == interruptedRequest.id } ) { - deferPaymentRequestPresentation(interruptedRequest) + deferPaymentRequestPresentation(interruptedRequest, failureReason) } isSubmittingPaymentRequest = false } @@ -2687,6 +2742,9 @@ class AppViewModel @Inject constructor( ) { val validatedAddress = runCatching { coreService.validateBitcoinAddress(invoice.address) } .getOrElse { + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) hideSheet() toast( type = Toast.ToastType.ERROR, @@ -2694,11 +2752,13 @@ class AppViewModel @Inject constructor( description = context.getString(R.string.wallet__error_invalid_bitcoin_address), testTag = "InvalidAddressToast", ) - clearActiveContactPaymentContext() return } if (NetworkValidationHelper.isNetworkMismatch(validatedAddress.network.toLdkNetwork(), Env.network)) { + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) hideSheet() toast( type = Toast.ToastType.ERROR, @@ -2706,7 +2766,6 @@ class AppViewModel @Inject constructor( description = context.getString(R.string.other__scan__error__generic), testTag = "InvalidAddressToast", ) - clearActiveContactPaymentContext() return } val hardwareWalletId = activeHardwareWalletId @@ -2925,6 +2984,9 @@ class AppViewModel @Inject constructor( fromMainScanner: Boolean, ) { if (invoice.isExpired) { + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) hideSheet() toast( type = Toast.ToastType.ERROR, @@ -2932,7 +2994,6 @@ class AppViewModel @Inject constructor( description = context.getString(R.string.other__scan__error__expired), testTag = "ExpiredLightningToast", ) - clearActiveContactPaymentContext() return } @@ -3000,7 +3061,9 @@ class AppViewModel @Inject constructor( title = context.getString(R.string.other__lnurl_pay_error), description = context.getString(R.string.other__scan__error__generic), ) - clearActiveContactPaymentContext() + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) return } val paymentAmount = incomingAmount ?: displaySats @@ -3600,6 +3663,9 @@ class AppViewModel @Inject constructor( description = context.getString(R.string.wallet__payment_request_mismatch), testTag = "PaymentFailedToast", ) + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) hideSheet() } @@ -4440,8 +4506,9 @@ class AppViewModel @Inject constructor( invalidatePaymentRequestPresentation() clearPaymentRequestPresentationRetry(id) requestedPaymentRequestId = id + shouldRestorePaymentRequestSheet = _currentSheet.value is Sheet.PaymentRequests - if (_currentSheet.value is Sheet.PaymentRequests) { + if (shouldRestorePaymentRequestSheet) { hideSheet(shouldFlushDeferredScan = false) paymentRequestSheetTransitionJob?.cancel() val job = viewModelScope.launch { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 30c8a6ee5c..0e92c573c0 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1222,6 +1222,7 @@ Rejected Unavailable %1$s at %2$s + The payment request is no longer available. Waiting for payment Waiting for updated private payment details. Bitkit will retry automatically. Waiting for %1$s to pay diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt index a11dc7ec88..050d5bdcd7 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt @@ -35,6 +35,7 @@ import org.mockito.kotlin.verifyBlocking import org.mockito.kotlin.whenever import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore +import to.bitkit.models.PubkyPublicKeyFormat import to.bitkit.services.PaykitPaymentRequestProposalTerms import to.bitkit.services.PaykitReceiverPaths import to.bitkit.services.PaykitSdkService @@ -105,6 +106,55 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { assertEquals(listOf(MethodId.Bolt11.rawValue), request.acceptedPaymentEndpointIdentifiers) } + @Test + fun `incoming parse failures are reason specific`() { + val cases = listOf( + paymentRequestRecord(role = null) to PaykitPaymentRequest.ParseFailure.MissingLocalRole, + paymentRequestRecord(role = PaymentRequestLocalRole.PAYEE) to + PaykitPaymentRequest.ParseFailure.UnsupportedLocalRole, + paymentRequestRecord(state = PaymentRequestLifecycleState.ACCEPTED) to + PaykitPaymentRequest.ParseFailure.NonActionableState, + paymentRequestRecord().copy(terms = null) to PaykitPaymentRequest.ParseFailure.MissingTerms, + paymentRequestRecord(asset = "BTC") to PaykitPaymentRequest.ParseFailure.UnsupportedAsset, + paymentRequestRecord(amount = "not-bitcoin") to PaykitPaymentRequest.ParseFailure.InvalidAmount, + paymentRequestRecord(amount = "184467440737.09551615") to + PaykitPaymentRequest.ParseFailure.AmountOutOfRange, + paymentRequestRecord(endpoints = listOf("btc-unsupported-method")) to + PaykitPaymentRequest.ParseFailure.NoSupportedEndpoint, + paymentRequestRecord(expiresAt = "not-a-timestamp") to + PaykitPaymentRequest.ParseFailure.InvalidExpiration, + paymentRequestRecord(expiresAt = clock.now().toString()) to PaykitPaymentRequest.ParseFailure.Expired, + ) + + cases.forEach { (record, expectedReason) -> + val result = record.parseIncomingPaykitPaymentRequest(clock.now()) + as PaykitPaymentRequestParseResult.Rejected + + assertEquals(expectedReason, result.reason) + } + } + + @Test + fun `incoming parse rejection log excludes request data`() { + val requestId = "do-not-log-this-id" + val record = paymentRequestRecord( + id = requestId, + asset = "BTC", + counterparty = COUNTERPARTY, + ) + val result = record.parseIncomingPaykitPaymentRequest(clock.now()) + as PaykitPaymentRequestParseResult.Rejected + + val output = record.incomingPaymentRequestRejectionLog(result.reason) + + assertTrue(output.contains("category='parse' reason='unsupported_asset'")) + assertTrue(output.contains("counterparty='${PubkyPublicKeyFormat.redacted(COUNTERPARTY)}'")) + assertFalse(output.contains(COUNTERPARTY)) + assertFalse(output.contains(requestId)) + assertFalse(output.contains(record.terms?.amount?.value.orEmpty())) + assertFalse(output.contains(record.terms?.acceptedPaymentEndpointIdentifiers?.single().orEmpty())) + } + @Test fun `refresh rejects amounts outside the app payment range`() = test { whenever(paykitSdkService.paymentRequests()).thenReturn( @@ -654,6 +704,7 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { role: PaymentRequestLocalRole? = PaymentRequestLocalRole.PAYER, state: PaymentRequestLifecycleState = PaymentRequestLifecycleState.PROPOSED, amount: String = "0.001", + asset: String = "btc", expiresAt: String? = null, endpoints: List = listOf(MethodId.Bolt11.rawValue), counterparty: String = COUNTERPARTY, @@ -669,7 +720,7 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { proposalOutboundStatus = null, proposalEventId = "proposal-event", terms = PaymentRequestTerms( - amount = PaymentRequestAmount(value = amount, asset = "btc"), + amount = PaymentRequestAmount(value = amount, asset = asset), paymentReference = PAYMENT_REFERENCE, proposalExpiresAt = expiresAt, recurrence = null, diff --git a/app/src/test/java/to/bitkit/repositories/PublicPaykitRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PublicPaykitRepoTest.kt index 612a5d8205..c5703c6123 100644 --- a/app/src/test/java/to/bitkit/repositories/PublicPaykitRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PublicPaykitRepoTest.kt @@ -231,6 +231,26 @@ class PublicPaykitRepoTest : BaseUnitTest() { assertEquals(PublicPaykitPaymentResult.Opened(PUBLIC_BOLT11), result) } + @Test + fun `payment launch results have reason specific incoming request failures`() { + assertEquals( + null, + PublicPaykitPaymentResult.Opened(PUBLIC_BOLT11).incomingPaymentRequestFailureReason, + ) + assertEquals( + IncomingPaykitPaymentRequestFailureReason.NoSupportedEndpoint, + PublicPaykitPaymentResult.NoEndpoint.incomingPaymentRequestFailureReason, + ) + assertEquals( + IncomingPaykitPaymentRequestFailureReason.EndpointNotPayable, + PublicPaykitPaymentResult.NotOpened.incomingPaymentRequestFailureReason, + ) + assertEquals( + IncomingPaykitPaymentRequestFailureReason.PaymentDetailsPending, + PublicPaykitPaymentResult.WaitingForUpdatedPaymentList.incomingPaymentRequestFailureReason, + ) + } + @Suppress("LongParameterList") private fun createRepo( pubkyRepo: PubkyRepo = this.pubkyRepo, diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 76c59f2e27..381e23af65 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -46,6 +46,7 @@ import org.lightningdevkit.ldknode.SpendableUtxo import org.lightningdevkit.ldknode.TransactionDetails import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.atLeast import org.mockito.kotlin.check import org.mockito.kotlin.clearInvocations @@ -82,6 +83,7 @@ import to.bitkit.models.PubkyProfile import to.bitkit.models.SamRockPaymentMethod import to.bitkit.models.SamRockSetupRequest import to.bitkit.models.SendFailureDetails +import to.bitkit.models.Toast import to.bitkit.models.TransactionSpeed import to.bitkit.models.TransportType import to.bitkit.models.USD @@ -617,21 +619,22 @@ class AppViewModelSendFlowTest : BaseUnitTest() { } @Test - fun `failed manual request presentation returns to the request queue`() = test { + fun `failed manual request resolution returns to the request sheet with terminal feedback`() = test { sut.setIsAuthenticated(true) val request = paymentRequest() + whenever(context.getString(R.string.wallet__payment_request)).thenReturn("Payment Request") + whenever(context.getString(R.string.wallet__payment_request_waiting_for_details)).thenReturn("Waiting") + whenever(context.getString(R.string.wallet__payment_request_unavailable)).thenReturn( + "The payment request is no longer available." + ) whenever(privatePaykitRepo.beginPaymentRequest(request)).thenReturn( - Result.success( - PublicPaykitPaymentResult.Opened( - paymentRequest = "bitcoin:first?lightning=bitcoin:second", - privatePaymentContext = PrivatePaykitPaymentContext("bitkit/server", 8uL), - ), - ) + Result.success(PublicPaykitPaymentResult.WaitingForUpdatedPaymentList) ) pendingPaykitPaymentRequests.value = listOf(request) enablePaykitUi() pubkyPublicKey.value = testPublicKey runCurrent() + clearInvocations(toastManager) sut.showPaymentRequests() sut.openIncomingPaymentRequest(request.id) @@ -643,6 +646,46 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertEquals(Sheet.PaymentRequests, sut.currentSheet.value) verify(paykitPaymentRequestRepo).markPresented(request) verify(privatePaykitRepo, times(15)).beginPaymentRequest(request) + val toastCaptor = argumentCaptor() + verify(toastManager, times(2)).enqueue(toastCaptor.capture()) + val (waitingToast, terminalToast) = toastCaptor.allValues + assertEquals("Payment Request", waitingToast.title) + assertEquals("Waiting", waitingToast.description) + assertEquals("PaymentRequestUnavailableToast", terminalToast.testTag) + assertEquals("Payment Request", terminalToast.title) + assertEquals("The payment request is no longer available.", terminalToast.description) + } + + @Test + fun `failed request opened from the full screen does not replace it with the request sheet`() = test { + sut.setIsAuthenticated(true) + val request = paymentRequest() + whenever(context.getString(R.string.wallet__payment_request)).thenReturn("Payment Request") + whenever(context.getString(R.string.wallet__payment_request_waiting_for_details)).thenReturn("Waiting") + whenever(context.getString(R.string.wallet__payment_request_unavailable)).thenReturn( + "The payment request is no longer available." + ) + whenever(privatePaykitRepo.beginPaymentRequest(request)).thenReturn( + Result.success(PublicPaykitPaymentResult.NoEndpoint) + ) + pendingPaykitPaymentRequests.value = listOf(request) + enablePaykitUi() + pubkyPublicKey.value = testPublicKey + runCurrent() + clearInvocations(toastManager) + + sut.openIncomingPaymentRequest(request.id) + advanceTimeBy(30.seconds.inWholeMilliseconds) + runCurrent() + + assertNull(sut.currentSheet.value) + verify(paykitPaymentRequestRepo).markPresented(request) + verify(privatePaykitRepo, times(15)).beginPaymentRequest(request) + val toastCaptor = argumentCaptor() + verify(toastManager, times(2)).enqueue(toastCaptor.capture()) + val (waitingToast, terminalToast) = toastCaptor.allValues + assertNull(waitingToast.testTag) + assertEquals("PaymentRequestUnavailableToast", terminalToast.testTag) } @Test diff --git a/changelog.d/next/1209.fixed.md b/changelog.d/next/1209.fixed.md new file mode 100644 index 0000000000..2d5de1f2aa --- /dev/null +++ b/changelog.d/next/1209.fixed.md @@ -0,0 +1 @@ +Incoming Payment Requests now report safe failure reasons and show an error when an opened request cannot be resolved. diff --git a/docs/payment-requests.md b/docs/payment-requests.md new file mode 100644 index 0000000000..d6ba84a391 --- /dev/null +++ b/docs/payment-requests.md @@ -0,0 +1,36 @@ +# Incoming Payment Request failures + +Bitkit distinguishes Payment Requests rejected while reading a Paykit record from requests that +parse successfully but cannot be opened. + +## Failure contract + +- Parse-time rejection emits a warning with category `parse`, a stable reason, and only the + redacted counterparty. It excludes the request id, amount, note, endpoint identifier, and + endpoint payload. +- Open-time rejection emits a warning with category `resolution` or `presentation`, a stable + reason, and only the redacted counterparty. +- An explicit Pay action tries immediately and fourteen more times at two-second intervals. After + the fifteenth failure, Bitkit shows a localized error and leaves the request available for + another attempt. +- Automatic presentation uses the same initial retries, then continues every 120 seconds without + showing terminal feedback. + +The parse reasons are `missing_local_role`, `missing_terms`, `recurring_request`, +`unsupported_asset`, `invalid_amount`, `amount_out_of_range`, `no_supported_endpoint`, +`invalid_expiration`, and `expired`. + +The resolution reasons are `no_supported_endpoint`, `endpoint_not_payable`, +`payment_details_pending`, and `resolution_failed`. The presentation reasons are +`invalid_payment_target` and `payment_target_not_routable`. + +`unsupported_local_role` and `non_actionable_state` are expected filtering of outgoing or +completed records, so they do not emit incoming-rejection warnings. + +## Accessibility identifiers + +- Payment Requests screen: `PaymentRequestsScreen`. +- Incoming request row: `PaymentRequestRow-`. +- Dismiss action: `PaymentRequestDismiss-`. +- Pay action: `PaymentRequestPay-`. +- Terminal feedback: `PaymentRequestUnavailableToast`. diff --git a/journeys/payment-requests/requested-resolution-failure.xml b/journeys/payment-requests/requested-resolution-failure.xml new file mode 100644 index 0000000000..4f42853c2e --- /dev/null +++ b/journeys/payment-requests/requested-resolution-failure.xml @@ -0,0 +1,21 @@ + + + Verifies a user explicitly opening an incoming Payment Request receives localized terminal + feedback after resolution retries exhaust, while the request remains available for another + attempt. + + Precondition: onboarded dev wallet with Paykit UI enabled, a profile, and one linked saved + contact. Seed exactly one proposed incoming Payment Request from that contact with a known + payment-request id and a supported accepted endpoint identifier, while the controlled Paykit + peer returns no matching endpoint for at least 35 seconds. Start on Payment Requests (testTag + "PaymentRequestsScreen"). + + + Verify the incoming request row (testTag "PaymentRequestRow-<payment-request-id>") is visible + Tap Pay (testTag "PaymentRequestPay-<payment-request-id>") + Wait up to 35 seconds for the terminal error toast (testTag "PaymentRequestUnavailableToast") + Verify the toast title is "Payment Request" and its description is "The payment request is no longer available." + Verify Payment Requests (testTag "PaymentRequestsScreen") remains visible + Verify the incoming request row (testTag "PaymentRequestRow-<payment-request-id>") remains visible for a later retry + + From 9aa6a2706010dea14baa2953e9ccf8ce3a1c77e9 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Wed, 2 Sep 2026 02:31:40 +0200 Subject: [PATCH 2/8] fix: redact invalid request counterparties --- .../repositories/PaykitPaymentRequestRepo.kt | 41 ++++++++++++--- .../java/to/bitkit/viewmodels/AppViewModel.kt | 9 ++-- .../PaykitPaymentRequestDiagnosticsTest.kt | 50 +++++++++++++++++++ .../PaykitPaymentRequestRepoTest.kt | 30 +++++------ .../viewmodels/AppViewModelSendFlowTest.kt | 12 +++++ 5 files changed, 115 insertions(+), 27 deletions(-) create mode 100644 app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt index d8b89f7d6c..05e3e687be 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt @@ -112,6 +112,39 @@ internal sealed interface PaykitPaymentRequestParseResult { data class Rejected(val reason: PaykitPaymentRequest.ParseFailure) : PaykitPaymentRequestParseResult } +@Singleton +class PaykitPaymentRequestDiagnostics @Inject constructor() { + companion object { + private const val TAG = "PaykitPaymentRequestDiagnostics" + } + + internal fun logParseRejection( + counterparty: String, + reason: PaykitPaymentRequest.ParseFailure, + ) { + Logger.warn( + "Rejected incoming Paykit payment request: category='parse' reason='${reason.logValue}' " + + "counterparty='${counterparty.redactedForPaymentRequestDiagnostics()}'", + context = TAG, + ) + } + + internal fun logPresentationRejection( + counterparty: String, + reason: IncomingPaykitPaymentRequestFailureReason, + ) { + Logger.warn( + "Rejected incoming Paykit payment request presentation: category='${reason.category}' " + + "reason='${reason.logValue}' " + + "counterparty='${counterparty.redactedForPaymentRequestDiagnostics()}'", + context = TAG, + ) + } +} + +private fun String.redactedForPaymentRequestDiagnostics(): String = + PubkyPublicKeyFormat.normalized(this)?.let(PubkyPublicKeyFormat::redacted) ?: "" + private data class ParsedPaykitPaymentRequestTerms( val terms: PaymentRequestTerms, val amountSats: ULong, @@ -163,6 +196,7 @@ class PaykitPaymentRequestRepo @Inject constructor( private val paykitSdkService: PaykitSdkService, private val settingsStore: SettingsStore, private val presentationStore: PaykitPaymentRequestPresentationStore, + private val diagnostics: PaykitPaymentRequestDiagnostics, private val clock: Clock, ) { companion object { @@ -418,7 +452,7 @@ class PaykitPaymentRequestRepo @Inject constructor( is PaykitPaymentRequestParseResult.Parsed -> result.request is PaykitPaymentRequestParseResult.Rejected -> { if (result.reason.shouldLogIncomingRejection) { - Logger.warn(record.incomingPaymentRequestRejectionLog(result.reason), context = TAG) + diagnostics.logParseRejection(record.counterparty, result.reason) } null } @@ -661,11 +695,6 @@ internal fun PaymentRequestRecord.parseIncomingPaykitPaymentRequest( requiresActionableRequest = true, ) -internal fun PaymentRequestRecord.incomingPaymentRequestRejectionLog( - reason: PaykitPaymentRequest.ParseFailure, -): String = "Rejected incoming Paykit payment request: category='parse' reason='${reason.logValue}' " + - "counterparty='${PubkyPublicKeyFormat.redacted(counterparty)}'" - @Suppress("CyclomaticComplexMethod", "ReturnCount") private fun PaymentRequestRecord.parsePaykitPaymentRequest( expectedRole: PaymentRequestLocalRole, diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 72f374c70f..7e8c4c6187 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -150,6 +150,7 @@ import to.bitkit.repositories.PaykitPaymentProofKind import to.bitkit.repositories.PaykitPaymentProofRepo import to.bitkit.repositories.PaykitPaymentRequest import to.bitkit.repositories.PaykitPaymentRequestCreation +import to.bitkit.repositories.PaykitPaymentRequestDiagnostics import to.bitkit.repositories.PaykitPaymentRequestDraft import to.bitkit.repositories.PaykitPaymentRequestError import to.bitkit.repositories.PaykitPaymentRequestId @@ -243,6 +244,7 @@ class AppViewModel @Inject constructor( private val privatePaykitRepo: PrivatePaykitRepo, private val paykitPaymentRequestRepo: PaykitPaymentRequestRepo, private val paykitPaymentProofRepo: PaykitPaymentProofRepo, + private val paykitPaymentRequestDiagnostics: PaykitPaymentRequestDiagnostics, private val refreshContactPaykitReceivers: RefreshContactPaykitReceiversUseCase, private val samRockRepo: SamRockRepo, private val appUpdateSheet: AppUpdateTimedSheet, @@ -916,12 +918,7 @@ class AppViewModel @Inject constructor( request: PaykitPaymentRequest, reason: IncomingPaykitPaymentRequestFailureReason, ) { - Logger.warn( - "Rejected incoming Paykit payment request presentation: category='${reason.category}' " + - "reason='${reason.logValue}' " + - "counterparty='${PubkyPublicKeyFormat.redacted(request.counterparty)}'", - context = TAG, - ) + paykitPaymentRequestDiagnostics.logPresentationRejection(request.counterparty, reason) val attempt = paymentRequestPresentationRetryAttempts[request.id] ?: 0 val retryDelay = PAYKIT_PAYMENT_REQUEST_PRESENTATION_RETRY_DELAYS.getOrNull(attempt) ?: if (requestedPaymentRequestId == request.id) { diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt new file mode 100644 index 0000000000..1810ec925c --- /dev/null +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt @@ -0,0 +1,50 @@ +package to.bitkit.repositories + +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowLog +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class PaykitPaymentRequestDiagnosticsTest { + private val sut = PaykitPaymentRequestDiagnostics() + + @Before + fun setUp() { + ShadowLog.clear() + } + + @Test + fun `parse rejection logs a safe reason and invalid counterparty placeholder`() { + sut.logParseRejection("secret", PaykitPaymentRequest.ParseFailure.UnsupportedAsset) + + val output = paymentRequestDiagnostic() + + assertTrue(output.contains("category='parse' reason='unsupported_asset'")) + assertTrue(output.contains("counterparty=''")) + assertFalse(output.contains("secret")) + } + + @Test + fun `presentation rejection logs a safe reason and invalid counterparty placeholder`() { + sut.logPresentationRejection( + "secret", + IncomingPaykitPaymentRequestFailureReason.NoSupportedEndpoint, + ) + + val output = paymentRequestDiagnostic() + + assertTrue(output.contains("category='resolution' reason='no_supported_endpoint'")) + assertTrue(output.contains("counterparty=''")) + assertFalse(output.contains("secret")) + } + + private fun paymentRequestDiagnostic(): String = ShadowLog.getLogsForTag("APP") + .single { it.msg.contains("Rejected incoming Paykit payment request") } + .msg +} diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt index 050d5bdcd7..47a60d059e 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt @@ -31,11 +31,11 @@ import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.times +import org.mockito.kotlin.verify import org.mockito.kotlin.verifyBlocking import org.mockito.kotlin.whenever import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore -import to.bitkit.models.PubkyPublicKeyFormat import to.bitkit.services.PaykitPaymentRequestProposalTerms import to.bitkit.services.PaykitReceiverPaths import to.bitkit.services.PaykitSdkService @@ -68,6 +68,7 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { private val paykitSdkService = mock() private val settingsStore = mock() private val presentationStore = mock() + private val diagnostics = mock() private var schedulerOriginMillis = 0L private val clock = object : Clock { override fun now(): Instant = START_TIME.plus( @@ -85,7 +86,14 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { whenever(settingsStore.isPaykitEnabled).thenReturn(flowOf(true)) whenever(settingsStore.data).thenReturn(flowOf(SettingsData(sharesPrivatePaykitEndpoints = true))) whenever(presentationStore.load(LOCAL_IDENTITY)).thenReturn(emptySet()) - sut = PaykitPaymentRequestRepo(testDispatcher, paykitSdkService, settingsStore, presentationStore, clock) + sut = PaykitPaymentRequestRepo( + testDispatcher, + paykitSdkService, + settingsStore, + presentationStore, + diagnostics, + clock, + ) sut.activate(LOCAL_IDENTITY) } @@ -135,24 +143,16 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { } @Test - fun `incoming parse rejection log excludes request data`() { - val requestId = "do-not-log-this-id" + fun `refresh emits reason specific parse rejection diagnostic`() = test { val record = paymentRequestRecord( - id = requestId, asset = "BTC", - counterparty = COUNTERPARTY, + counterparty = "secret", ) - val result = record.parseIncomingPaykitPaymentRequest(clock.now()) - as PaykitPaymentRequestParseResult.Rejected + whenever(paykitSdkService.paymentRequests()).thenReturn(listOf(record)) - val output = record.incomingPaymentRequestRejectionLog(result.reason) + sut.refresh().getOrThrow() - assertTrue(output.contains("category='parse' reason='unsupported_asset'")) - assertTrue(output.contains("counterparty='${PubkyPublicKeyFormat.redacted(COUNTERPARTY)}'")) - assertFalse(output.contains(COUNTERPARTY)) - assertFalse(output.contains(requestId)) - assertFalse(output.contains(record.terms?.amount?.value.orEmpty())) - assertFalse(output.contains(record.terms?.acceptedPaymentEndpointIdentifiers?.single().orEmpty())) + verify(diagnostics).logParseRejection("secret", PaykitPaymentRequest.ParseFailure.UnsupportedAsset) } @Test diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 381e23af65..bef9dc0379 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -96,6 +96,7 @@ import to.bitkit.repositories.ConnectivityState import to.bitkit.repositories.CurrencyRepo import to.bitkit.repositories.HealthRepo import to.bitkit.repositories.HwWalletRepo +import to.bitkit.repositories.IncomingPaykitPaymentRequestFailureReason import to.bitkit.repositories.LightningRepo import to.bitkit.repositories.LightningState import to.bitkit.repositories.MethodId @@ -104,6 +105,7 @@ import to.bitkit.repositories.PaykitPaymentProofKind import to.bitkit.repositories.PaykitPaymentProofRepo import to.bitkit.repositories.PaykitPaymentRequest import to.bitkit.repositories.PaykitPaymentRequestCreation +import to.bitkit.repositories.PaykitPaymentRequestDiagnostics import to.bitkit.repositories.PaykitPaymentRequestDraft import to.bitkit.repositories.PaykitPaymentRequestId import to.bitkit.repositories.PaykitPaymentRequestRepo @@ -198,6 +200,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { private val privatePaykitRepo = mock() private val paykitPaymentRequestRepo = mock() private val paykitPaymentProofRepo = mock() + private val paykitPaymentRequestDiagnostics = mock() private val samRockRepo = mock() private val widgetsRepo = mock() private val formatMoneyValue = mock() @@ -402,6 +405,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { privatePaykitRepo = privatePaykitRepo, paykitPaymentRequestRepo = paykitPaymentRequestRepo, paykitPaymentProofRepo = paykitPaymentProofRepo, + paykitPaymentRequestDiagnostics = paykitPaymentRequestDiagnostics, refreshContactPaykitReceivers = refreshContactPaykitReceivers, samRockRepo = samRockRepo, appUpdateSheet = mock(), @@ -646,6 +650,10 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertEquals(Sheet.PaymentRequests, sut.currentSheet.value) verify(paykitPaymentRequestRepo).markPresented(request) verify(privatePaykitRepo, times(15)).beginPaymentRequest(request) + verify(paykitPaymentRequestDiagnostics, times(15)).logPresentationRejection( + request.counterparty, + IncomingPaykitPaymentRequestFailureReason.PaymentDetailsPending, + ) val toastCaptor = argumentCaptor() verify(toastManager, times(2)).enqueue(toastCaptor.capture()) val (waitingToast, terminalToast) = toastCaptor.allValues @@ -681,6 +689,10 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertNull(sut.currentSheet.value) verify(paykitPaymentRequestRepo).markPresented(request) verify(privatePaykitRepo, times(15)).beginPaymentRequest(request) + verify(paykitPaymentRequestDiagnostics, times(15)).logPresentationRejection( + request.counterparty, + IncomingPaykitPaymentRequestFailureReason.NoSupportedEndpoint, + ) val toastCaptor = argumentCaptor() verify(toastManager, times(2)).enqueue(toastCaptor.capture()) val (waitingToast, terminalToast) = toastCaptor.allValues From 84bc24ae2e7ddf098ff983a2ad40f74b5d03ddbf Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Wed, 2 Sep 2026 02:54:42 +0200 Subject: [PATCH 3/8] chore: rename changelog fragment --- changelog.d/next/{1209.fixed.md => 1217.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{1209.fixed.md => 1217.fixed.md} (100%) diff --git a/changelog.d/next/1209.fixed.md b/changelog.d/next/1217.fixed.md similarity index 100% rename from changelog.d/next/1209.fixed.md rename to changelog.d/next/1217.fixed.md From a1feb3e063012e6e8146b510976c9d9caa2cd5f5 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Wed, 2 Sep 2026 13:01:46 +0200 Subject: [PATCH 4/8] test: cover valid pubky redaction --- .../repositories/PaykitPaymentRequestDiagnosticsTest.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt index 1810ec925c..30fc7f5d6a 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt @@ -20,14 +20,15 @@ class PaykitPaymentRequestDiagnosticsTest { } @Test - fun `parse rejection logs a safe reason and invalid counterparty placeholder`() { - sut.logParseRejection("secret", PaykitPaymentRequest.ParseFailure.UnsupportedAsset) + fun `parse rejection logs a safe reason and redacted counterparty`() { + val counterparty = "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xg" + sut.logParseRejection(counterparty, PaykitPaymentRequest.ParseFailure.UnsupportedAsset) val output = paymentRequestDiagnostic() assertTrue(output.contains("category='parse' reason='unsupported_asset'")) - assertTrue(output.contains("counterparty=''")) - assertFalse(output.contains("secret")) + assertTrue(output.contains("counterparty='pubky3r…k8yw5xg'")) + assertFalse(output.contains(counterparty)) } @Test From 4015578d99aac8b46cdd2a406040bf9ce1e0e7ae Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Thu, 3 Sep 2026 23:53:53 +0200 Subject: [PATCH 5/8] fix: keep request sheet after invalid target --- .../java/to/bitkit/viewmodels/AppViewModel.kt | 3 +- .../viewmodels/AppViewModelSendFlowTest.kt | 39 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 7e8c4c6187..3bbaf2543f 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -2602,10 +2602,11 @@ class AppViewModel @Inject constructor( is Scanner.NodeId -> handleNonPaymentScan { onScanNodeId(scan) } is Scanner.Gift -> handleNonPaymentScan { onScanGift(scan.code, scan.amount) } else -> { + val hasIncomingPaymentRequest = activeIncomingPaymentRequest() != null clearActiveContactPaymentContext( failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, ) - hideSheet() + if (!hasIncomingPaymentRequest) hideSheet() Logger.warn( if (scan == null) "Failed to decode scan data" else "Received unhandled scan data '$scan'", context = TAG, diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index bef9dc0379..6a5580eab7 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -664,6 +664,45 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertEquals("The payment request is no longer available.", terminalToast.description) } + @Test + fun `opened request with an invalid target returns to the request sheet`() = test { + sut.setIsAuthenticated(true) + val request = paymentRequest() + val invalidTarget = "not-a-payment-invoice" + whenever(context.getString(R.string.wallet__payment_request)).thenReturn("Payment Request") + whenever(context.getString(R.string.wallet__payment_request_waiting_for_details)).thenReturn("Waiting") + whenever(context.getString(R.string.wallet__payment_request_unavailable)).thenReturn( + "The payment request is no longer available." + ) + stubOpenedPaymentRequest(request, invalidTarget) + whenever(coreService.decode(invalidTarget)).thenThrow(IllegalStateException("invalid")) + pendingPaykitPaymentRequests.value = listOf(request) + enablePaykitUi() + pubkyPublicKey.value = testPublicKey + runCurrent() + clearInvocations(toastManager) + + sut.showPaymentRequests() + sut.openIncomingPaymentRequest(request.id) + advanceTimeBy(TRANSITION_SCREEN_MS) + runCurrent() + advanceTimeBy(30.seconds.inWholeMilliseconds) + runCurrent() + + assertEquals(Sheet.PaymentRequests, sut.currentSheet.value) + verify(paykitPaymentRequestRepo).markPresented(request) + verify(privatePaykitRepo, times(15)).beginPaymentRequest(request) + verify(paykitPaymentRequestDiagnostics, times(15)).logPresentationRejection( + request.counterparty, + IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) + val toastCaptor = argumentCaptor() + verify(toastManager, atLeast(2)).enqueue(toastCaptor.capture()) + val terminalToast = toastCaptor.allValues.single { it.testTag == "PaymentRequestUnavailableToast" } + assertEquals("Payment Request", terminalToast.title) + assertEquals("The payment request is no longer available.", terminalToast.description) + } + @Test fun `failed request opened from the full screen does not replace it with the request sheet`() = test { sut.setIsAuthenticated(true) From 2faeecf5d3a35e01c10e3723a3d14e4777917430 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Fri, 4 Sep 2026 00:51:48 +0200 Subject: [PATCH 6/8] fix: keep request sheet restore paths --- .../repositories/PaykitPaymentRequestRepo.kt | 11 ++- .../bitkit/repositories/PublicPaykitRepo.kt | 3 +- .../java/to/bitkit/viewmodels/AppViewModel.kt | 69 +++++++++++------- app/src/main/res/values/strings.xml | 1 + .../PaykitPaymentRequestRepoTest.kt | 27 +++++++ .../repositories/PublicPaykitRepoTest.kt | 2 + .../viewmodels/AppViewModelSendFlowTest.kt | 73 ++++++++++++++++++- docs/payment-requests.md | 17 +++-- 8 files changed, 168 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt index 05e3e687be..7dc65ee319 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt @@ -81,7 +81,8 @@ data class PaykitPaymentRequest( val shouldLogIncomingRejection: Boolean = true, ) { MissingLocalRole("missing_local_role"), - UnsupportedLocalRole("unsupported_local_role", shouldLogIncomingRejection = false), + OutgoingRequest("outgoing_request", shouldLogIncomingRejection = false), + UnsupportedLocalRole("unsupported_local_role"), NonActionableState("non_actionable_state", shouldLogIncomingRejection = false), MissingTerms("missing_terms"), RecurringRequest("recurring_request"), @@ -704,7 +705,13 @@ private fun PaymentRequestRecord.parsePaykitPaymentRequest( val role = localRole ?: return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.MissingLocalRole) if (role != expectedRole) { - return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.UnsupportedLocalRole) + return PaykitPaymentRequestParseResult.Rejected( + if (expectedRole == PaymentRequestLocalRole.PAYER && role == PaymentRequestLocalRole.PAYEE) { + PaykitPaymentRequest.ParseFailure.OutgoingRequest + } else { + PaykitPaymentRequest.ParseFailure.UnsupportedLocalRole + }, + ) } if (requiresActionableRequest && state != PaymentRequestLifecycleState.PROPOSED) { return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.NonActionableState) diff --git a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt index 18e9a63d12..dd36b61e9f 100644 --- a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt @@ -65,13 +65,14 @@ internal enum class IncomingPaykitPaymentRequestFailureReason( PaymentDetailsPending("payment_details_pending"), InvalidPaymentTarget("invalid_payment_target"), PaymentTargetNotRoutable("payment_target_not_routable"), + RequestExpired("request_expired"), ResolutionFailed("resolution_failed"), ; val category: String get() = when (this) { NoSupportedEndpoint, EndpointNotPayable, PaymentDetailsPending, ResolutionFailed -> "resolution" - InvalidPaymentTarget, PaymentTargetNotRoutable -> "presentation" + InvalidPaymentTarget, PaymentTargetNotRoutable, RequestExpired -> "presentation" } } diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 3bbaf2543f..d84d5ca522 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -883,6 +883,10 @@ class AppViewModel @Inject constructor( if (!isCurrentPaymentRequestPresentation(request, generation) || isPaymentRequestPresentationBlocked()) { return true } + if (presentationResult.exceptionOrNull() is PaykitPaymentRequestError.RequestExpired) { + finishExpiredPaymentRequestPresentation(request) + return false + } if (!paykitPaymentRequestRepo.isPending(request)) { if (requestedPaymentRequestId == request.id) { invalidatePaymentRequestPresentation() @@ -961,6 +965,29 @@ class AppViewModel @Inject constructor( } } + private fun finishExpiredPaymentRequestPresentation(request: PaykitPaymentRequest) { + paykitPaymentRequestDiagnostics.logPresentationRejection( + request.counterparty, + IncomingPaykitPaymentRequestFailureReason.RequestExpired, + ) + val restorePaymentRequestSheet = + requestedPaymentRequestId == request.id && shouldRestorePaymentRequestSheet + val showExpiredToast = requestedPaymentRequestId == request.id + paymentRequestPresentationGeneration++ + if (requestedPaymentRequestId == request.id) { + clearRequestedPaymentRequestPresentation() + } + clearPaymentRequestPresentationRetry(request.id) + if (!showExpiredToast) return + toast( + type = Toast.ToastType.ERROR, + title = context.getString(R.string.wallet__payment_request), + description = context.getString(R.string.wallet__payment_request_expired), + testTag = "PaymentRequestExpiredToast", + ) + if (restorePaymentRequestSheet) showSheet(Sheet.PaymentRequests) + } + private fun retainPaymentRequestPresentationState(requests: List) { val requestIds = requests.mapTo(mutableSetOf()) { it.id } paymentRequestPresentationRetryAttempts.keys.retainAll(requestIds) @@ -2497,10 +2524,7 @@ class AppViewModel @Inject constructor( // TODO Workaround for https://github.com/synonymdev/bitkit-core/issues/63 if (Bip21Utils.isDuplicatedBip21(input)) { - clearActiveContactPaymentContext( - failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, - ) - hideSheet() + clearIncomingPaymentRequestTarget() toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), @@ -2602,15 +2626,12 @@ class AppViewModel @Inject constructor( is Scanner.NodeId -> handleNonPaymentScan { onScanNodeId(scan) } is Scanner.Gift -> handleNonPaymentScan { onScanGift(scan.code, scan.amount) } else -> { - val hasIncomingPaymentRequest = activeIncomingPaymentRequest() != null - clearActiveContactPaymentContext( - failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, - ) - if (!hasIncomingPaymentRequest) hideSheet() + val hasIncomingPaymentRequest = clearIncomingPaymentRequestTarget() Logger.warn( if (scan == null) "Failed to decode scan data" else "Received unhandled scan data '$scan'", context = TAG, ) + if (hasIncomingPaymentRequest) return toast( type = Toast.ToastType.WARNING, title = context.getString(R.string.other__qr_error_header), @@ -2669,6 +2690,16 @@ class AppViewModel @Inject constructor( retryIncomingRequest = retryIncomingRequest, ) + private fun clearIncomingPaymentRequestTarget( + failureReason: IncomingPaykitPaymentRequestFailureReason = + IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ): Boolean { + val hasIncomingPaymentRequest = activeIncomingPaymentRequest() != null + clearActiveContactPaymentContext(failureReason = failureReason) + if (!hasIncomingPaymentRequest) hideSheet() + return hasIncomingPaymentRequest + } + private fun clearActiveContactPaymentContext( failureReason: IncomingPaykitPaymentRequestFailureReason, retryIncomingRequest: Boolean = true, @@ -2740,10 +2771,7 @@ class AppViewModel @Inject constructor( ) { val validatedAddress = runCatching { coreService.validateBitcoinAddress(invoice.address) } .getOrElse { - clearActiveContactPaymentContext( - failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, - ) - hideSheet() + clearIncomingPaymentRequestTarget() toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), @@ -2754,10 +2782,7 @@ class AppViewModel @Inject constructor( } if (NetworkValidationHelper.isNetworkMismatch(validatedAddress.network.toLdkNetwork(), Env.network)) { - clearActiveContactPaymentContext( - failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, - ) - hideSheet() + clearIncomingPaymentRequestTarget() toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), @@ -2982,10 +3007,7 @@ class AppViewModel @Inject constructor( fromMainScanner: Boolean, ) { if (invoice.isExpired) { - clearActiveContactPaymentContext( - failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, - ) - hideSheet() + clearIncomingPaymentRequestTarget() toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), @@ -3661,10 +3683,7 @@ class AppViewModel @Inject constructor( description = context.getString(R.string.wallet__payment_request_mismatch), testTag = "PaymentFailedToast", ) - clearActiveContactPaymentContext( - failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, - ) - hideSheet() + clearIncomingPaymentRequestTarget() } private fun getLnurlInvoiceFetchErrorMessage(error: Throwable): String = when (error) { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 0e92c573c0..b5a000f8a9 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1193,6 +1193,7 @@ Dismiss Edit expiration Enter pubky + The payment request has expired. Expires in 1 day 1 hour diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt index 47a60d059e..d20cbc23a7 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt @@ -119,6 +119,8 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { val cases = listOf( paymentRequestRecord(role = null) to PaykitPaymentRequest.ParseFailure.MissingLocalRole, paymentRequestRecord(role = PaymentRequestLocalRole.PAYEE) to + PaykitPaymentRequest.ParseFailure.OutgoingRequest, + paymentRequestRecord(role = PaymentRequestLocalRole.UNKNOWN) to PaykitPaymentRequest.ParseFailure.UnsupportedLocalRole, paymentRequestRecord(state = PaymentRequestLifecycleState.ACCEPTED) to PaykitPaymentRequest.ParseFailure.NonActionableState, @@ -155,6 +157,31 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { verify(diagnostics).logParseRejection("secret", PaykitPaymentRequest.ParseFailure.UnsupportedAsset) } + @Test + fun `refresh logs unknown local role as unsupported_local_role`() = test { + val record = paymentRequestRecord( + role = PaymentRequestLocalRole.UNKNOWN, + counterparty = "secret", + ) + whenever(paykitSdkService.paymentRequests()).thenReturn(listOf(record)) + + sut.refresh().getOrThrow() + + verify(diagnostics).logParseRejection("secret", PaykitPaymentRequest.ParseFailure.UnsupportedLocalRole) + assertTrue(sut.pendingRequests.value.isEmpty()) + } + + @Test + fun `refresh does not log outgoing payee requests`() = test { + whenever(paykitSdkService.paymentRequests()).thenReturn( + listOf(paymentRequestRecord(role = PaymentRequestLocalRole.PAYEE, counterparty = "secret")), + ) + + sut.refresh().getOrThrow() + + verify(diagnostics, never()).logParseRejection(any(), any()) + } + @Test fun `refresh rejects amounts outside the app payment range`() = test { whenever(paykitSdkService.paymentRequests()).thenReturn( diff --git a/app/src/test/java/to/bitkit/repositories/PublicPaykitRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PublicPaykitRepoTest.kt index c5703c6123..faed638630 100644 --- a/app/src/test/java/to/bitkit/repositories/PublicPaykitRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PublicPaykitRepoTest.kt @@ -249,6 +249,8 @@ class PublicPaykitRepoTest : BaseUnitTest() { IncomingPaykitPaymentRequestFailureReason.PaymentDetailsPending, PublicPaykitPaymentResult.WaitingForUpdatedPaymentList.incomingPaymentRequestFailureReason, ) + assertEquals("presentation", IncomingPaykitPaymentRequestFailureReason.RequestExpired.category) + assertEquals("request_expired", IncomingPaykitPaymentRequestFailureReason.RequestExpired.logValue) } @Suppress("LongParameterList") diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 6a5580eab7..23035a38ac 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -107,6 +107,7 @@ import to.bitkit.repositories.PaykitPaymentRequest import to.bitkit.repositories.PaykitPaymentRequestCreation import to.bitkit.repositories.PaykitPaymentRequestDiagnostics import to.bitkit.repositories.PaykitPaymentRequestDraft +import to.bitkit.repositories.PaykitPaymentRequestError import to.bitkit.repositories.PaykitPaymentRequestId import to.bitkit.repositories.PaykitPaymentRequestRepo import to.bitkit.repositories.PaykitPaymentRequestTarget @@ -698,11 +699,81 @@ class AppViewModelSendFlowTest : BaseUnitTest() { ) val toastCaptor = argumentCaptor() verify(toastManager, atLeast(2)).enqueue(toastCaptor.capture()) - val terminalToast = toastCaptor.allValues.single { it.testTag == "PaymentRequestUnavailableToast" } + val terminalToast = toastCaptor.allValues.last() + assertEquals("PaymentRequestUnavailableToast", terminalToast.testTag) assertEquals("Payment Request", terminalToast.title) assertEquals("The payment request is no longer available.", terminalToast.description) } + @Test + fun `mismatched bolt11 from a request sheet returns to the request sheet`() = test { + sut.setIsAuthenticated(true) + val request = paymentRequest() + val bolt11 = "lnbcrt1mismatchedrequest" + whenever(context.getString(R.string.wallet__payment_request)).thenReturn("Payment Request") + whenever(context.getString(R.string.wallet__payment_request_waiting_for_details)).thenReturn("Waiting") + whenever(context.getString(R.string.wallet__payment_request_unavailable)).thenReturn( + "The payment request is no longer available." + ) + stubOpenedPaymentRequest(request, bolt11) + stubLightningScan(bolt11 = bolt11, amountSats = 1_000u) + pendingPaykitPaymentRequests.value = listOf(request) + enablePaykitUi() + pubkyPublicKey.value = testPublicKey + runCurrent() + clearInvocations(toastManager) + + sut.showPaymentRequests() + sut.openIncomingPaymentRequest(request.id) + advanceTimeBy(TRANSITION_SCREEN_MS) + runCurrent() + advanceTimeBy(30.seconds.inWholeMilliseconds) + runCurrent() + + assertEquals(Sheet.PaymentRequests, sut.currentSheet.value) + verify(paykitPaymentRequestRepo).markPresented(request) + verify(privatePaykitRepo, times(15)).beginPaymentRequest(request) + verify(paykitPaymentRequestDiagnostics, times(15)).logPresentationRejection( + request.counterparty, + IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) + } + + @Test + fun `expired explicit request shows the expired toast once`() = test { + sut.setIsAuthenticated(true) + val request = paymentRequest() + whenever(context.getString(R.string.wallet__payment_request)).thenReturn("Payment Request") + whenever(context.getString(R.string.wallet__payment_request_expired)).thenReturn( + "The payment request has expired." + ) + whenever(privatePaykitRepo.beginPaymentRequest(request)).thenReturn( + Result.failure(PaykitPaymentRequestError.RequestExpired) + ) + pendingPaykitPaymentRequests.value = listOf(request) + enablePaykitUi() + pubkyPublicKey.value = testPublicKey + runCurrent() + clearInvocations(toastManager) + + sut.showPaymentRequests() + sut.openIncomingPaymentRequest(request.id) + advanceTimeBy(TRANSITION_SCREEN_MS) + runCurrent() + + assertEquals(Sheet.PaymentRequests, sut.currentSheet.value) + verify(privatePaykitRepo).beginPaymentRequest(request) + verify(paykitPaymentRequestDiagnostics).logPresentationRejection( + request.counterparty, + IncomingPaykitPaymentRequestFailureReason.RequestExpired, + ) + val toastCaptor = argumentCaptor() + verify(toastManager).enqueue(toastCaptor.capture()) + assertEquals("PaymentRequestExpiredToast", toastCaptor.lastValue.testTag) + assertEquals("Payment Request", toastCaptor.lastValue.title) + assertEquals("The payment request has expired.", toastCaptor.lastValue.description) + } + @Test fun `failed request opened from the full screen does not replace it with the request sheet`() = test { sut.setIsAuthenticated(true) diff --git a/docs/payment-requests.md b/docs/payment-requests.md index d6ba84a391..980bd28932 100644 --- a/docs/payment-requests.md +++ b/docs/payment-requests.md @@ -13,19 +13,23 @@ parse successfully but cannot be opened. - An explicit Pay action tries immediately and fourteen more times at two-second intervals. After the fifteenth failure, Bitkit shows a localized error and leaves the request available for another attempt. +- If the request expires during an explicit presentation attempt, Bitkit logs + `category=presentation reason=request_expired` and shows `PaymentRequestExpiredToast` with the + localized `wallet__payment_request_expired` message exactly once. - Automatic presentation uses the same initial retries, then continues every 120 seconds without showing terminal feedback. -The parse reasons are `missing_local_role`, `missing_terms`, `recurring_request`, -`unsupported_asset`, `invalid_amount`, `amount_out_of_range`, `no_supported_endpoint`, -`invalid_expiration`, and `expired`. +The parse reasons are `missing_local_role`, `outgoing_request`, `unsupported_local_role`, +`missing_terms`, `recurring_request`, `unsupported_asset`, `invalid_amount`, `amount_out_of_range`, +`no_supported_endpoint`, `invalid_expiration`, and `expired`. The resolution reasons are `no_supported_endpoint`, `endpoint_not_payable`, `payment_details_pending`, and `resolution_failed`. The presentation reasons are -`invalid_payment_target` and `payment_target_not_routable`. +`invalid_payment_target`, `payment_target_not_routable`, and `request_expired`. -`unsupported_local_role` and `non_actionable_state` are expected filtering of outgoing or -completed records, so they do not emit incoming-rejection warnings. +`outgoing_request` and `non_actionable_state` are expected filtering of outgoing or completed +records, so they do not emit incoming-rejection warnings. `unsupported_local_role` identifies an +unknown role and emits a privacy-safe warning with only the redacted counterparty. ## Accessibility identifiers @@ -34,3 +38,4 @@ completed records, so they do not emit incoming-rejection warnings. - Dismiss action: `PaymentRequestDismiss-`. - Pay action: `PaymentRequestPay-`. - Terminal feedback: `PaymentRequestUnavailableToast`. +- Expiration feedback: `PaymentRequestExpiredToast`. From 156a4c44fd8085c2d5e61f50af02dcfbf2c56e99 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Fri, 4 Sep 2026 18:54:10 +0200 Subject: [PATCH 7/8] fix: preserve payment request failure state --- .../java/to/bitkit/viewmodels/AppViewModel.kt | 12 +++--- .../viewmodels/AppViewModelSendFlowTest.kt | 40 +++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index d84d5ca522..845f0d1deb 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -2524,7 +2524,7 @@ class AppViewModel @Inject constructor( // TODO Workaround for https://github.com/synonymdev/bitkit-core/issues/63 if (Bip21Utils.isDuplicatedBip21(input)) { - clearIncomingPaymentRequestTarget() + if (clearIncomingPaymentRequestTarget()) return@withContext toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), @@ -2695,8 +2695,9 @@ class AppViewModel @Inject constructor( IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, ): Boolean { val hasIncomingPaymentRequest = activeIncomingPaymentRequest() != null + val shouldHideSheet = !hasIncomingPaymentRequest || currentSheet.value is Sheet.Send clearActiveContactPaymentContext(failureReason = failureReason) - if (!hasIncomingPaymentRequest) hideSheet() + if (shouldHideSheet) hideSheet() return hasIncomingPaymentRequest } @@ -2771,7 +2772,7 @@ class AppViewModel @Inject constructor( ) { val validatedAddress = runCatching { coreService.validateBitcoinAddress(invoice.address) } .getOrElse { - clearIncomingPaymentRequestTarget() + if (clearIncomingPaymentRequestTarget()) return toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), @@ -2782,7 +2783,7 @@ class AppViewModel @Inject constructor( } if (NetworkValidationHelper.isNetworkMismatch(validatedAddress.network.toLdkNetwork(), Env.network)) { - clearIncomingPaymentRequestTarget() + if (clearIncomingPaymentRequestTarget()) return toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), @@ -3001,13 +3002,14 @@ class AppViewModel @Inject constructor( else -> SendFundingSource.Savings } + @Suppress("ReturnCount") private suspend fun onScanLightning( invoice: LightningInvoice, scanResult: String, fromMainScanner: Boolean, ) { if (invoice.isExpired) { - clearIncomingPaymentRequestTarget() + if (clearIncomingPaymentRequestTarget()) return toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 23035a38ac..4ce9eff2ad 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -705,6 +705,43 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertEquals("The payment request is no longer available.", terminalToast.description) } + @Test + fun `duplicated bip21 request target leaves terminal feedback visible`() = test { + sut.setIsAuthenticated(true) + val request = paymentRequest() + val first = "bitcoin:bcrt1qfirst?amount=0.00000001" + val duplicatedBip21 = first + "bitcoin:bcrt1qsecond?amount=0.00000001" + whenever(context.getString(R.string.wallet__payment_request)).thenReturn("Payment Request") + whenever(context.getString(R.string.wallet__payment_request_waiting_for_details)).thenReturn("Waiting") + whenever(context.getString(R.string.wallet__payment_request_unavailable)).thenReturn( + "The payment request is no longer available." + ) + stubOpenedPaymentRequest(request, duplicatedBip21) + pendingPaykitPaymentRequests.value = listOf(request) + enablePaykitUi() + pubkyPublicKey.value = testPublicKey + runCurrent() + clearInvocations(toastManager) + + sut.showPaymentRequests() + sut.openIncomingPaymentRequest(request.id) + advanceTimeBy(TRANSITION_SCREEN_MS) + runCurrent() + advanceTimeBy(30.seconds.inWholeMilliseconds) + runCurrent() + + assertEquals(Sheet.PaymentRequests, sut.currentSheet.value) + verify(paykitPaymentRequestRepo).markPresented(request) + verify(privatePaykitRepo, times(15)).beginPaymentRequest(request) + verify(paykitPaymentRequestDiagnostics, times(15)).logPresentationRejection( + request.counterparty, + IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) + val toastCaptor = argumentCaptor() + verify(toastManager, times(2)).enqueue(toastCaptor.capture()) + assertEquals("PaymentRequestUnavailableToast", toastCaptor.lastValue.testTag) + } + @Test fun `mismatched bolt11 from a request sheet returns to the request sheet`() = test { sut.setIsAuthenticated(true) @@ -4558,9 +4595,12 @@ class AppViewModelSendFlowTest : BaseUnitTest() { isPaymentRequest = true, ), ) + sut.showSheet(Sheet.Send(SendRoute.Confirm)) + advanceUntilIdle() confirmCurrentPayment() + assertNull(sut.currentSheet.value) verify(paykitPaymentRequestRepo, never()).accept(any()) verify(privatePaykitRepo, never()).consumePrivatePaymentList(any(), any()) verify(lightningRepo, never()).payInvoice(any(), anyOrNull()) From b79c43d667c959e4fdafb1f9bec148c11a6a9ee1 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Sat, 5 Sep 2026 03:57:28 +0200 Subject: [PATCH 8/8] fix: log safe paykit resolution errors --- .../repositories/PaykitPaymentRequestRepo.kt | 12 ++++++++++ .../java/to/bitkit/viewmodels/AppViewModel.kt | 4 +++- .../PaykitPaymentRequestDiagnosticsTest.kt | 22 +++++++++++++++++-- .../viewmodels/AppViewModelSendFlowTest.kt | 21 ++++++++++++++++++ 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt index 7dc65ee319..894bc5c32c 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt @@ -141,6 +141,18 @@ class PaykitPaymentRequestDiagnostics @Inject constructor() { context = TAG, ) } + + internal fun logPresentationFailure( + counterparty: String, + error: Throwable, + ) { + Logger.warn( + "Failed to resolve incoming Paykit payment request: " + + "category='resolution' errorType='${error::class.simpleName ?: "Unknown"}' " + + "counterparty='${counterparty.redactedForPaymentRequestDiagnostics()}'", + context = TAG, + ) + } } private fun String.redactedForPaymentRequestDiagnostics(): String = diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 845f0d1deb..c120d8261c 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -883,10 +883,12 @@ class AppViewModel @Inject constructor( if (!isCurrentPaymentRequestPresentation(request, generation) || isPaymentRequestPresentationBlocked()) { return true } - if (presentationResult.exceptionOrNull() is PaykitPaymentRequestError.RequestExpired) { + val error = presentationResult.exceptionOrNull() + if (error is PaykitPaymentRequestError.RequestExpired) { finishExpiredPaymentRequestPresentation(request) return false } + if (error != null) paykitPaymentRequestDiagnostics.logPresentationFailure(request.counterparty, error) if (!paykitPaymentRequestRepo.isPending(request)) { if (requestedPaymentRequestId == request.id) { invalidatePaymentRequestPresentation() diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt index 30fc7f5d6a..1647ebbf5b 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt @@ -6,6 +6,7 @@ import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config import org.robolectric.shadows.ShadowLog +import to.bitkit.utils.Logger import kotlin.test.assertFalse import kotlin.test.assertTrue @@ -16,6 +17,7 @@ class PaykitPaymentRequestDiagnosticsTest { @Before fun setUp() { + Logger.reset() ShadowLog.clear() } @@ -45,7 +47,23 @@ class PaykitPaymentRequestDiagnosticsTest { assertFalse(output.contains("secret")) } - private fun paymentRequestDiagnostic(): String = ShadowLog.getLogsForTag("APP") - .single { it.msg.contains("Rejected incoming Paykit payment request") } + @Test + fun `presentation failure logs error type without throwable message`() { + val counterparty = "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xg" + val secret = "private payment payload" + sut.logPresentationFailure(counterparty, IllegalStateException(secret)) + + val output = paymentRequestDiagnostic("Failed to resolve incoming Paykit payment request") + + assertTrue(output.contains("category='resolution' errorType='IllegalStateException'")) + assertTrue(output.contains("counterparty='pubky3r…k8yw5xg'")) + assertFalse(output.contains(secret)) + assertFalse(output.contains(counterparty)) + } + + private fun paymentRequestDiagnostic( + message: String = "Rejected incoming Paykit payment request", + ): String = ShadowLog.getLogsForTag("APP") + .single { it.msg.contains(message) } .msg } diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 4ce9eff2ad..30fa70582d 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -811,6 +811,27 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertEquals("The payment request has expired.", toastCaptor.lastValue.description) } + @Test + fun `failed explicit request logs a redacted resolution error`() = test { + sut.setIsAuthenticated(true) + val request = paymentRequest() + val failure = IllegalStateException("private payment payload") + whenever(privatePaykitRepo.beginPaymentRequest(request)).thenReturn(Result.failure(failure)) + pendingPaykitPaymentRequests.value = listOf(request) + enablePaykitUi() + pubkyPublicKey.value = testPublicKey + runCurrent() + + sut.openIncomingPaymentRequest(request.id) + runCurrent() + + verify(paykitPaymentRequestDiagnostics).logPresentationFailure(request.counterparty, failure) + verify(paykitPaymentRequestDiagnostics).logPresentationRejection( + request.counterparty, + IncomingPaykitPaymentRequestFailureReason.ResolutionFailed, + ) + } + @Test fun `failed request opened from the full screen does not replace it with the request sheet`() = test { sut.setIsAuthenticated(true)