From a22278559d3b3a5e7fb56bbaf3040c5e47ce2c0c Mon Sep 17 00:00:00 2001 From: Dominique Padiou <5765435+dpad85@users.noreply.github.com> Date: Tue, 19 Nov 2024 10:23:06 +0100 Subject: [PATCH] Prevent repeating payments on same payment hash (#652) The check must include pending payments, not only successfully sent payments. --------- Co-authored-by: Robbie Hanson <304604+robbiehanson@users.noreply.github.com> --- .../payments/send/lnurl/LnurlPayView.kt | 1 + .../utils/extensions/ErrorExtensions.kt | 1 + .../res/values-b+es+419/important_strings.xml | 1 + .../main/res/values-cs/important_strings.xml | 1 + .../main/res/values-de/important_strings.xml | 1 + .../main/res/values-es/important_strings.xml | 1 + .../main/res/values-fr/important_strings.xml | 1 + .../res/values-pt-rBR/important_strings.xml | 1 + .../main/res/values-sk/important_strings.xml | 1 + .../main/res/values-sw/important_strings.xml | 1 + .../main/res/values-vi/important_strings.xml | 1 + .../src/main/res/values/important_strings.xml | 1 + .../src/main/res/values/strings.xml | 1 + phoenix-ios/phoenix-ios/Localizable.xcstrings | 38 +++++++++++++++++++ .../phoenix-ios/kotlin/KotlinTypes.swift | 2 + .../views/send/LnurlFlowErrorNotice.swift | 4 ++ .../views/send/ParseResultHelper.swift | 7 ++++ .../fr.acinq.phoenix/managers/SendManager.kt | 37 +++++++++--------- 18 files changed, 83 insertions(+), 18 deletions(-) diff --git a/phoenix-android/src/main/kotlin/fr/acinq/phoenix/android/payments/send/lnurl/LnurlPayView.kt b/phoenix-android/src/main/kotlin/fr/acinq/phoenix/android/payments/send/lnurl/LnurlPayView.kt index 5674ca92a..dd222566b 100644 --- a/phoenix-android/src/main/kotlin/fr/acinq/phoenix/android/payments/send/lnurl/LnurlPayView.kt +++ b/phoenix-android/src/main/kotlin/fr/acinq/phoenix/android/payments/send/lnurl/LnurlPayView.kt @@ -166,6 +166,7 @@ fun LnurlPayView( details = when (state) { is LnurlPayViewState.Error.Generic -> state.cause.localizedMessage is LnurlPayViewState.Error.PayError -> when (val error = state.error) { + is SendManager.LnurlPayError.PaymentPending -> annotatedStringResource(R.string.lnurl_pay_error_payment_pending, payIntent.callback.host) is SendManager.LnurlPayError.AlreadyPaidInvoice -> annotatedStringResource(R.string.lnurl_pay_error_already_paid, payIntent.callback.host) is SendManager.LnurlPayError.ChainMismatch -> annotatedStringResource(R.string.lnurl_pay_error_invalid_chain, payIntent.callback.host) is SendManager.LnurlPayError.BadResponseError -> when (val errorDetail = error.err) { diff --git a/phoenix-android/src/main/kotlin/fr/acinq/phoenix/android/utils/extensions/ErrorExtensions.kt b/phoenix-android/src/main/kotlin/fr/acinq/phoenix/android/utils/extensions/ErrorExtensions.kt index c27db5ee8..6bd5f763d 100644 --- a/phoenix-android/src/main/kotlin/fr/acinq/phoenix/android/utils/extensions/ErrorExtensions.kt +++ b/phoenix-android/src/main/kotlin/fr/acinq/phoenix/android/utils/extensions/ErrorExtensions.kt @@ -33,6 +33,7 @@ fun SendManager.ParseResult.BadRequest.toLocalisedMessage(): String { return when (val reason = this.reason) { is SendManager.BadRequestReason.Expired -> stringResource(R.string.send_error_invoice_expired) is SendManager.BadRequestReason.ChainMismatch -> stringResource(R.string.send_error_invalid_chain) + is SendManager.BadRequestReason.PaymentPending -> stringResource(R.string.send_error_payment_pending) is SendManager.BadRequestReason.AlreadyPaidInvoice -> stringResource(R.string.send_error_already_paid) is SendManager.BadRequestReason.ServiceError -> reason.error.toLocalisedMessage().text is SendManager.BadRequestReason.InvalidLnurl -> stringResource(R.string.send_error_lnurl_invalid) diff --git a/phoenix-android/src/main/res/values-b+es+419/important_strings.xml b/phoenix-android/src/main/res/values-b+es+419/important_strings.xml index 92d20293b..e62614a89 100644 --- a/phoenix-android/src/main/res/values-b+es+419/important_strings.xml +++ b/phoenix-android/src/main/res/values-b+es+419/important_strings.xml @@ -162,6 +162,7 @@ Esta factura está vencida. + Este pago ya se está procesando. Por favor, espere a que se complete. Este pago ya se realizó. Este pago no usa la misma cadena de bloques de la billetera. Error al procesar este enlace LNURL. Comprueba que sea válido. diff --git a/phoenix-android/src/main/res/values-cs/important_strings.xml b/phoenix-android/src/main/res/values-cs/important_strings.xml index 5e71bbb2e..651a060de 100644 --- a/phoenix-android/src/main/res/values-cs/important_strings.xml +++ b/phoenix-android/src/main/res/values-cs/important_strings.xml @@ -158,6 +158,7 @@ Platnost této faktury vypršela. + Tato platba se již zpracovává. Počkejte prosím na její dokončení. Tato platba již byla uhrazena. Tato platba nepoužívá stejný blockchain jako vaše peněženka. Tento LNURL odkaz se nepodařilo zpracovat. Ujistěte se, že je platný. diff --git a/phoenix-android/src/main/res/values-de/important_strings.xml b/phoenix-android/src/main/res/values-de/important_strings.xml index 0878933d8..af5219ea8 100644 --- a/phoenix-android/src/main/res/values-de/important_strings.xml +++ b/phoenix-android/src/main/res/values-de/important_strings.xml @@ -161,6 +161,7 @@ Die Rechnung ist abgelaufen. + Diese Zahlung wird bereits bearbeitet. Bitte warten Sie, bis sie abgeschlossen ist. Diese Zahlung wurde bereits getätigt. Diese Zahlung verwendet nicht die gleiche Blockchain wie Ihre Wallet. Dieser LNURL-Link konnte nicht verarbeitet werden. Stellen Sie sicher, dass er gültig ist. diff --git a/phoenix-android/src/main/res/values-es/important_strings.xml b/phoenix-android/src/main/res/values-es/important_strings.xml index 766f1bd64..59dc0d72d 100644 --- a/phoenix-android/src/main/res/values-es/important_strings.xml +++ b/phoenix-android/src/main/res/values-es/important_strings.xml @@ -165,6 +165,7 @@ Esta factura ha caducado. + Este pago ya se está procesando. Por favor, espere a que se complete. Este pago ya ha sido abonado. Este pago no utiliza la misma blockchain que su cartera. No se ha podido procesar este enlace LNURL. Asegúrese de que es válido. diff --git a/phoenix-android/src/main/res/values-fr/important_strings.xml b/phoenix-android/src/main/res/values-fr/important_strings.xml index 67ea17898..f8911148a 100644 --- a/phoenix-android/src/main/res/values-fr/important_strings.xml +++ b/phoenix-android/src/main/res/values-fr/important_strings.xml @@ -165,6 +165,7 @@ Cette requête a expiré. + Ce paiement est déjà en cours de traitement. Veuillez attendre qu\'il soit terminé. Ce paiement a déjà été réglé. Ce paiement n\'utilise pas la même blockchain que votre wallet. Ce lien LNURL n\'a pas pu être traité. Assurez-vous qu\'il soit valide. diff --git a/phoenix-android/src/main/res/values-pt-rBR/important_strings.xml b/phoenix-android/src/main/res/values-pt-rBR/important_strings.xml index 48c814128..9da99cc81 100644 --- a/phoenix-android/src/main/res/values-pt-rBR/important_strings.xml +++ b/phoenix-android/src/main/res/values-pt-rBR/important_strings.xml @@ -163,6 +163,7 @@ Esta fatura está expirada. + Este pagamento já está sendo processado. Aguarde a conclusão. Este pagamento já foi pago. Este pagamento não usa o mesmo blockchain que sua carteira. Falha ao processar esse link LNURL. Verifique se ele é válido. diff --git a/phoenix-android/src/main/res/values-sk/important_strings.xml b/phoenix-android/src/main/res/values-sk/important_strings.xml index bd84892b0..d5dd3cdbf 100644 --- a/phoenix-android/src/main/res/values-sk/important_strings.xml +++ b/phoenix-android/src/main/res/values-sk/important_strings.xml @@ -164,6 +164,7 @@ Platnosť tejto faktúry vypršala. + Táto platba sa už spracováva. Počkajte, prosím, na jej dokončenie. Táto platba už bola uhradená. Táto platba nepoužíva ten istý blockchain ako vaša peňaženka. Tento LNURL odkaz sa nepodarilo spracovať. Uistite sa, že je platný. diff --git a/phoenix-android/src/main/res/values-sw/important_strings.xml b/phoenix-android/src/main/res/values-sw/important_strings.xml index 8f2fea9c0..ea01ee7c1 100644 --- a/phoenix-android/src/main/res/values-sw/important_strings.xml +++ b/phoenix-android/src/main/res/values-sw/important_strings.xml @@ -167,6 +167,7 @@ Ankara hii imeisha muda wake. + Malipo haya tayari yanachakatwa. Tafadhali subiri ikamilike. Malipo haya tayari yamelipwa. Malipo haya hayatumii mnyororo sawa na pochi yako. Imeshindwa kuchakata kiungo hiki cha LNURL. Hakikisha ni halali. diff --git a/phoenix-android/src/main/res/values-vi/important_strings.xml b/phoenix-android/src/main/res/values-vi/important_strings.xml index 0c50454f7..d99de0b00 100644 --- a/phoenix-android/src/main/res/values-vi/important_strings.xml +++ b/phoenix-android/src/main/res/values-vi/important_strings.xml @@ -171,6 +171,7 @@ Hoá đơn này đã hết hạn. + Thanh toán này đang được xử lý. Vui lòng đợi để hoàn tất. Khoản thanh toán này đã được trả. Khoản thanh toán này không sử dụng cùng blockchain với ví của bạn. Không thể xử lý liên kết LNURL này. Hãy đảm bảo liên kết này có hiệu lực. diff --git a/phoenix-android/src/main/res/values/important_strings.xml b/phoenix-android/src/main/res/values/important_strings.xml index f93cf7597..d7f66466f 100644 --- a/phoenix-android/src/main/res/values/important_strings.xml +++ b/phoenix-android/src/main/res/values/important_strings.xml @@ -167,6 +167,7 @@ This invoice is expired. + This payment is already being processed. Please wait for it to complete. This payment has already been paid. This payment does not use the same blockchain as your wallet. Failed to process this LNURL link. Make sure it is valid. diff --git a/phoenix-android/src/main/res/values/strings.xml b/phoenix-android/src/main/res/values/strings.xml index 6c90e0a13..4192f5cf4 100644 --- a/phoenix-android/src/main/res/values/strings.xml +++ b/phoenix-android/src/main/res/values/strings.xml @@ -620,6 +620,7 @@ Payment has failed. The invoice returned by %1$s does not use the same chain as your wallet. + The invoice returned by %1$s is already in progress. The invoice returned by %1$s has already been paid. The invoice returned by %1$s has an incorrect amount. The invoice returned by %1$s is malformed. diff --git a/phoenix-ios/phoenix-ios/Localizable.xcstrings b/phoenix-ios/phoenix-ios/Localizable.xcstrings index 7f5bfa595..2020b5872 100644 --- a/phoenix-ios/phoenix-ios/Localizable.xcstrings +++ b/phoenix-ios/phoenix-ios/Localizable.xcstrings @@ -40476,6 +40476,9 @@ } } } + }, + "The received invoice is already in progress." : { + }, "The recipient is offline." : { "localizations" : { @@ -42088,6 +42091,41 @@ } } }, + "This payment is already being processed. Please wait for it to complete." : { + "comment" : "Error message - scanning lightning invoice", + "localizations" : { + "cs" : { + "stringUnit" : { + "state" : "translated", + "value" : "Tato platba se již zpracovává. Počkejte prosím na její dokončení." + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Diese Zahlung wird bereits bearbeitet. Bitte warten Sie, bis sie abgeschlossen ist." + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Este pago ya se está procesando. Por favor, espere a que se complete." + } + }, + "es-419" : { + "stringUnit" : { + "state" : "translated", + "value" : "Este pago ya se está procesando. Por favor, espere a que se complete." + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Ce paiement est déjà en cours de traitement. Veuillez attendre qu'il soit terminé." + } + } + } + }, "This screen is a debugging tool that can be used to manually import encrypted channels data.\n\nUse with caution." : { "localizations" : { "ar" : { diff --git a/phoenix-ios/phoenix-ios/kotlin/KotlinTypes.swift b/phoenix-ios/phoenix-ios/kotlin/KotlinTypes.swift index 353f458e9..1fcbf87ac 100644 --- a/phoenix-ios/phoenix-ios/kotlin/KotlinTypes.swift +++ b/phoenix-ios/phoenix-ios/kotlin/KotlinTypes.swift @@ -24,6 +24,7 @@ extension SendManager { typealias ParseResult_Lnurl_Auth = ParseResultLnurlAuth typealias BadRequestReason_AlreadyPaidInvoice = BadRequestReasonAlreadyPaidInvoice + typealias BadRequestReason_PaymentPending = BadRequestReasonPaymentPending typealias BadRequestReason_Bip353InvalidOffer = BadRequestReasonBip353InvalidOffer typealias BadRequestReason_Bip353InvalidUri = BadRequestReasonBip353InvalidUri typealias BadRequestReason_Bip353NameNotFound = BadRequestReasonBip353NameNotFound @@ -40,6 +41,7 @@ extension SendManager { typealias LnurlPay_Error_BadResponseError = LnurlPayErrorBadResponseError typealias LnurlPay_Error_ChainMismatch = LnurlPayErrorChainMismatch typealias LnurlPay_Error_AlreadyPaidInvoice = LnurlPayErrorAlreadyPaidInvoice + typealias LnurlPay_Error_PaymentPending = LnurlPayErrorPaymentPending typealias LnurlWithdraw_Error = LnurlWithdrawError typealias LnurlWithdraw_Error_RemoteError = LnurlWithdrawErrorRemoteError diff --git a/phoenix-ios/phoenix-ios/views/send/LnurlFlowErrorNotice.swift b/phoenix-ios/phoenix-ios/views/send/LnurlFlowErrorNotice.swift index aa85cdb5f..3766f9fe3 100644 --- a/phoenix-ios/phoenix-ios/views/send/LnurlFlowErrorNotice.swift +++ b/phoenix-ios/phoenix-ios/views/send/LnurlFlowErrorNotice.swift @@ -114,6 +114,10 @@ struct LnurlFlowErrorNotice: View { Text("You have already paid this invoice.") + } else if let _ = payError as? SendManager.LnurlPay_Error_PaymentPending { + + Text("The received invoice is already in progress.") + } else { genericErrorMessage() } diff --git a/phoenix-ios/phoenix-ios/views/send/ParseResultHelper.swift b/phoenix-ios/phoenix-ios/views/send/ParseResultHelper.swift index 4599d4027..2be4f3bc6 100644 --- a/phoenix-ios/phoenix-ios/views/send/ParseResultHelper.swift +++ b/phoenix-ios/phoenix-ios/views/send/ParseResultHelper.swift @@ -38,6 +38,13 @@ class ParseResultHelper { localized: "You've already paid this invoice. Paying it again could result in stolen funds.", comment: "Error message - scanning lightning invoice" ) + + case is SendManager.BadRequestReason_PaymentPending: + + msg = String( + localized: "This payment is already being processed. Please wait for it to complete.", + comment: "Error message - scanning lightning invoice" + ) case is SendManager.BadRequestReason_Bip353InvalidOffer: diff --git a/phoenix-shared/src/commonMain/kotlin/fr.acinq.phoenix/managers/SendManager.kt b/phoenix-shared/src/commonMain/kotlin/fr.acinq.phoenix/managers/SendManager.kt index 9fb0be423..35f3c9482 100644 --- a/phoenix-shared/src/commonMain/kotlin/fr.acinq.phoenix/managers/SendManager.kt +++ b/phoenix-shared/src/commonMain/kotlin/fr.acinq.phoenix/managers/SendManager.kt @@ -68,8 +68,9 @@ class SendManager( private val log = loggerFactory.newLogger(this::class) sealed class BadRequestReason : Exception() { - object UnknownFormat : BadRequestReason() - object AlreadyPaidInvoice : BadRequestReason() + data object UnknownFormat : BadRequestReason() + data object AlreadyPaidInvoice : BadRequestReason() + data object PaymentPending : BadRequestReason() data class Expired(val timestampSeconds: Long, val expirySeconds: Long) : BadRequestReason() data class ChainMismatch(val expected: Chain) : BadRequestReason() data class ServiceError(val url: Url, val error: LnurlError.RemoteFailure) : BadRequestReason() @@ -86,6 +87,7 @@ class SendManager( data class BadResponseError(val err: LnurlError.Pay.Invoice) : LnurlPayError() data class ChainMismatch(val expected: Chain) : LnurlPayError() data object AlreadyPaidInvoice : LnurlPayError() + data object PaymentPending : LnurlPayError() } sealed class LnurlWithdrawError { @@ -209,10 +211,16 @@ class SendManager( } val db = databaseManager.databases.filterNotNull().first() - return if (db.payments.listLightningOutgoingPayments(invoice.paymentHash).any { it.status is LightningOutgoingPayment.Status.Completed.Succeeded }) { - BadRequestReason.AlreadyPaidInvoice - } else { - null + val similarPayments = db.payments.listLightningOutgoingPayments(invoice.paymentHash) + // we MUST raise an error if this payment hash has already been paid, or is being paid. + // parallel pending payments on the same payment hash can trigger force-closes + // FIXME: this check should be done in lightning-kmp, not in Phoenix + return when { + similarPayments.any { it.status is LightningOutgoingPayment.Status.Completed.Succeeded || it.parts.any { part -> part.status is LightningOutgoingPayment.Part.Status.Succeeded } } -> + BadRequestReason.AlreadyPaidInvoice + similarPayments.any { it.status is LightningOutgoingPayment.Status.Pending || it.parts.any { part -> part.status is LightningOutgoingPayment.Part.Status.Pending } } -> + BadRequestReason.PaymentPending + else -> null } } @@ -471,22 +479,15 @@ class SendManager( return try { val invoice = task.await() when (checkForBadBolt11Invoice(invoice.invoice)) { - is BadRequestReason.ChainMismatch -> Either.Left( - LnurlPayError.ChainMismatch(expected = chain) - ) - is BadRequestReason.AlreadyPaidInvoice -> Either.Left( - LnurlPayError.AlreadyPaidInvoice - ) + is BadRequestReason.ChainMismatch -> Either.Left(LnurlPayError.ChainMismatch(expected = chain)) + is BadRequestReason.AlreadyPaidInvoice -> Either.Left(LnurlPayError.AlreadyPaidInvoice) + is BadRequestReason.PaymentPending -> Either.Left(LnurlPayError.PaymentPending) else -> Either.Right(invoice) } } catch (err: Throwable) { when (err) { - is LnurlError.RemoteFailure -> Either.Left( - LnurlPayError.RemoteError(err) - ) - is LnurlError.Pay.Invoice -> Either.Left( - LnurlPayError.BadResponseError(err) - ) + is LnurlError.RemoteFailure -> Either.Left(LnurlPayError.RemoteError(err)) + is LnurlError.Pay.Invoice -> Either.Left(LnurlPayError.BadResponseError(err)) else -> Either.Left( LnurlPayError.RemoteError( LnurlError.RemoteFailure.Unreadable(