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..894bc5c32c 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,24 @@ 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"), + OutgoingRequest("outgoing_request", shouldLogIncomingRejection = false), + UnsupportedLocalRole("unsupported_local_role"), + 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 +108,63 @@ 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 +} + +@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, + ) + } + + 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 = + PubkyPublicKeyFormat.normalized(this)?.let(PubkyPublicKeyFormat::redacted) ?: "" + +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 } @@ -133,6 +209,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 { @@ -383,7 +460,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) { + diagnostics.logParseRejection(record.counterparty, result.reason) + } + null + } + } + } val history = records.mapNotNull { it.toPaykitPaymentRequestHistory(now) } .sortedByDescending { it.createdAt } if ( @@ -613,65 +700,113 @@ 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, +) + @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( + 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) + } + 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..dd36b61e9f 100644 --- a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt @@ -57,6 +57,35 @@ 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"), + RequestExpired("request_expired"), + ResolutionFailed("resolution_failed"), + ; + + val category: String + get() = when (this) { + NoSupportedEndpoint, EndpointNotPayable, PaymentDetailsPending, ResolutionFailed -> "resolution" + InvalidPaymentTarget, PaymentTargetNotRoutable, RequestExpired -> "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..c120d8261c 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 @@ -149,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 @@ -170,6 +172,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 @@ -241,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, @@ -345,6 +349,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 +638,7 @@ class AppViewModel @Inject constructor( invalidatePaymentRequestPresentation(dismissActiveRequest = paymentRequestIdentity != null) clearPaymentRequestPresentationRetries() paymentRequestIdentity = null - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() paymentRequestSheetTransitionJob?.cancel() paymentRequestSheetTransitionJob = null try { @@ -649,7 +654,7 @@ class AppViewModel @Inject constructor( if (identityChanged) { invalidatePaymentRequestPresentation(dismissActiveRequest = paymentRequestIdentity != null) clearPaymentRequestPresentationRetries() - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() paymentRequestSheetTransitionJob?.cancel() paymentRequestSheetTransitionJob = null } @@ -818,7 +823,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 +864,7 @@ class AppViewModel @Inject constructor( val request = paykitPaymentRequestRepo.pendingRequest(requestedId) if (request != null) return listOf(request) invalidatePaymentRequestPresentation() - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() return null } @@ -873,19 +878,30 @@ 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 } + 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() - requestedPaymentRequestId = null + clearRequestedPaymentRequestPresentation() } return false } if (result !is PublicPaykitPaymentResult.Opened) { - deferPaymentRequestPresentation(request) + deferPaymentRequestPresentation( + request = request, + reason = result?.incomingPaymentRequestFailureReason + ?: IncomingPaykitPaymentRequestFailureReason.ResolutionFailed, + ) return false } @@ -904,17 +920,29 @@ 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, + ) { + 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) { 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) } @@ -939,6 +967,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) @@ -947,10 +998,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,14 +2526,13 @@ class AppViewModel @Inject constructor( // TODO Workaround for https://github.com/synonymdev/bitkit-core/issues/63 if (Bip21Utils.isDuplicatedBip21(input)) { - hideSheet() + if (clearIncomingPaymentRequestTarget()) return@withContext toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), description = context.getString(R.string.other__scan__error__generic), testTag = "DuplicatedBip21Toast", ) - clearActiveContactPaymentContext() return@withContext } @@ -2492,7 +2547,9 @@ class AppViewModel @Inject constructor( } if (input.startsWith("$PUBKYAUTH_SCHEME://", ignoreCase = true)) { - clearActiveContactPaymentContext() + clearActiveContactPaymentContext( + failureReason = IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ) if (!fromMainScanner) { hideSheet() toast( @@ -2522,7 +2579,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 +2612,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,23 +2628,25 @@ class AppViewModel @Inject constructor( is Scanner.NodeId -> handleNonPaymentScan { onScanNodeId(scan) } is Scanner.Gift -> handleNonPaymentScan { onScanGift(scan.code, scan.amount) } else -> { - 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), 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 +2663,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 +2680,33 @@ 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 clearIncomingPaymentRequestTarget( + failureReason: IncomingPaykitPaymentRequestFailureReason = + IncomingPaykitPaymentRequestFailureReason.InvalidPaymentTarget, + ): Boolean { + val hasIncomingPaymentRequest = activeIncomingPaymentRequest() != null + val shouldHideSheet = !hasIncomingPaymentRequest || currentSheet.value is Sheet.Send + clearActiveContactPaymentContext(failureReason = failureReason) + if (shouldHideSheet) hideSheet() + return hasIncomingPaymentRequest + } + + private fun clearActiveContactPaymentContext( + failureReason: IncomingPaykitPaymentRequestFailureReason, + retryIncomingRequest: Boolean = true, + ) { val interruptedRequest = synchronized(contactPaymentContextLock) { val request = activeContactPaymentContext?.incomingPaymentRequest activeContactPaymentContext = null @@ -2631,7 +2718,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 +2729,7 @@ class AppViewModel @Inject constructor( requestedPaymentRequestId == interruptedRequest.id || paykitPaymentRequestRepo.automaticPendingRequests().any { it.id == interruptedRequest.id } ) { - deferPaymentRequestPresentation(interruptedRequest) + deferPaymentRequestPresentation(interruptedRequest, failureReason) } isSubmittingPaymentRequest = false } @@ -2687,26 +2774,24 @@ class AppViewModel @Inject constructor( ) { val validatedAddress = runCatching { coreService.validateBitcoinAddress(invoice.address) } .getOrElse { - hideSheet() + if (clearIncomingPaymentRequestTarget()) return toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), description = context.getString(R.string.wallet__error_invalid_bitcoin_address), testTag = "InvalidAddressToast", ) - clearActiveContactPaymentContext() return } if (NetworkValidationHelper.isNetworkMismatch(validatedAddress.network.toLdkNetwork(), Env.network)) { - hideSheet() + if (clearIncomingPaymentRequestTarget()) return toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), description = context.getString(R.string.other__scan__error__generic), testTag = "InvalidAddressToast", ) - clearActiveContactPaymentContext() return } val hardwareWalletId = activeHardwareWalletId @@ -2919,20 +3004,20 @@ class AppViewModel @Inject constructor( else -> SendFundingSource.Savings } + @Suppress("ReturnCount") private suspend fun onScanLightning( invoice: LightningInvoice, scanResult: String, fromMainScanner: Boolean, ) { if (invoice.isExpired) { - hideSheet() + if (clearIncomingPaymentRequestTarget()) return toast( type = Toast.ToastType.ERROR, title = context.getString(R.string.other__scan_err_decoding), description = context.getString(R.string.other__scan__error__expired), testTag = "ExpiredLightningToast", ) - clearActiveContactPaymentContext() return } @@ -3000,7 +3085,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,7 +3687,7 @@ class AppViewModel @Inject constructor( description = context.getString(R.string.wallet__payment_request_mismatch), testTag = "PaymentFailedToast", ) - hideSheet() + clearIncomingPaymentRequestTarget() } private fun getLnurlInvoiceFetchErrorMessage(error: Throwable): String = when (error) { @@ -4440,8 +4527,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..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 @@ -1222,6 +1223,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/PaykitPaymentRequestDiagnosticsTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt new file mode 100644 index 0000000000..1647ebbf5b --- /dev/null +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestDiagnosticsTest.kt @@ -0,0 +1,69 @@ +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 to.bitkit.utils.Logger +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class PaykitPaymentRequestDiagnosticsTest { + private val sut = PaykitPaymentRequestDiagnostics() + + @Before + fun setUp() { + Logger.reset() + ShadowLog.clear() + } + + @Test + 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='pubky3r…k8yw5xg'")) + assertFalse(output.contains(counterparty)) + } + + @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")) + } + + @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/repositories/PaykitPaymentRequestRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt index a11dc7ec88..d20cbc23a7 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt @@ -31,6 +31,7 @@ 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 @@ -67,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( @@ -84,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) } @@ -105,6 +114,74 @@ 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.OutgoingRequest, + paymentRequestRecord(role = PaymentRequestLocalRole.UNKNOWN) 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 `refresh emits reason specific parse rejection diagnostic`() = test { + val record = paymentRequestRecord( + asset = "BTC", + counterparty = "secret", + ) + whenever(paykitSdkService.paymentRequests()).thenReturn(listOf(record)) + + sut.refresh().getOrThrow() + + 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( @@ -654,6 +731,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 +747,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..faed638630 100644 --- a/app/src/test/java/to/bitkit/repositories/PublicPaykitRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PublicPaykitRepoTest.kt @@ -231,6 +231,28 @@ 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, + ) + assertEquals("presentation", IncomingPaykitPaymentRequestFailureReason.RequestExpired.category) + assertEquals("request_expired", IncomingPaykitPaymentRequestFailureReason.RequestExpired.logValue) + } + @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..30fa70582d 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 @@ -94,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 @@ -102,7 +105,9 @@ 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 import to.bitkit.repositories.PaykitPaymentRequestRepo import to.bitkit.repositories.PaykitPaymentRequestTarget @@ -196,6 +201,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() @@ -400,6 +406,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { privatePaykitRepo = privatePaykitRepo, paykitPaymentRequestRepo = paykitPaymentRequestRepo, paykitPaymentProofRepo = paykitPaymentProofRepo, + paykitPaymentRequestDiagnostics = paykitPaymentRequestDiagnostics, refreshContactPaykitReceivers = refreshContactPaykitReceivers, samRockRepo = samRockRepo, appUpdateSheet = mock(), @@ -617,21 +624,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 +651,221 @@ 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 + 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 `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.last() + assertEquals("PaymentRequestUnavailableToast", terminalToast.testTag) + assertEquals("Payment Request", terminalToast.title) + 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) + 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 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) + 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) + verify(paykitPaymentRequestDiagnostics, times(15)).logPresentationRejection( + request.counterparty, + IncomingPaykitPaymentRequestFailureReason.NoSupportedEndpoint, + ) + val toastCaptor = argumentCaptor() + verify(toastManager, times(2)).enqueue(toastCaptor.capture()) + val (waitingToast, terminalToast) = toastCaptor.allValues + assertNull(waitingToast.testTag) + assertEquals("PaymentRequestUnavailableToast", terminalToast.testTag) } @Test @@ -4393,9 +4616,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()) diff --git a/changelog.d/next/1217.fixed.md b/changelog.d/next/1217.fixed.md new file mode 100644 index 0000000000..2d5de1f2aa --- /dev/null +++ b/changelog.d/next/1217.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..980bd28932 --- /dev/null +++ b/docs/payment-requests.md @@ -0,0 +1,41 @@ +# 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. +- 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`, `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`, `payment_target_not_routable`, and `request_expired`. + +`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 + +- Payment Requests screen: `PaymentRequestsScreen`. +- Incoming request row: `PaymentRequestRow-`. +- Dismiss action: `PaymentRequestDismiss-`. +- Pay action: `PaymentRequestPay-`. +- Terminal feedback: `PaymentRequestUnavailableToast`. +- Expiration feedback: `PaymentRequestExpiredToast`. 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 + +