Prevent repeating payments on same payment hash (#652)

The check must include pending payments, not only successfully sent payments. --------- Co-authored-by: Robbie Hanson <[email protected]>
ACINQ · Nov 19, 2024 · a222785 · a222785
1 parent a4269e0
commit a222785
Show file tree

Hide file tree

Showing 18 changed files with 83 additions and 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
@@ -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
@@ -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
@@ -162,6 +162,7 @@
     <!-- scanning errors -->
 
     <string name="send_error_invoice_expired">Esta factura está vencida.</string>
+    <string name="send_error_payment_pending">Este pago ya se está procesando. Por favor, espere a que se complete.</string>
     <string name="send_error_already_paid">Este pago ya se realizó.</string>
     <string name="send_error_invalid_chain">Este pago no usa la misma cadena de bloques de la billetera.</string>
     <string name="send_error_lnurl_invalid">Error al procesar este enlace LNURL. Comprueba que sea válido.</string>

diff --git 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 @@
     <!-- scanning errors -->
 
     <string name="send_error_invoice_expired">Platnost této faktury vypršela.</string>
+    <string name="send_error_payment_pending">Tato platba se již zpracovává. Počkejte prosím na její dokončení.</string>
     <string name="send_error_already_paid">Tato platba již byla uhrazena.</string>
     <string name="send_error_invalid_chain">Tato platba nepoužívá stejný blockchain jako vaše peněženka.</string>
     <string name="send_error_lnurl_invalid">Tento LNURL odkaz se nepodařilo zpracovat. Ujistěte se, že je platný.</string>

diff --git 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 @@
     <!-- scanning errors -->
 
     <string name="send_error_invoice_expired">Die Rechnung ist abgelaufen.</string>
+    <string name="send_error_payment_pending">Diese Zahlung wird bereits bearbeitet. Bitte warten Sie, bis sie abgeschlossen ist.</string>
     <string name="send_error_already_paid">Diese Zahlung wurde bereits getätigt.</string>
     <string name="send_error_invalid_chain">Diese Zahlung verwendet nicht die gleiche Blockchain wie Ihre Wallet.</string>
     <string name="send_error_lnurl_invalid">Dieser LNURL-Link konnte nicht verarbeitet werden. Stellen Sie sicher, dass er gültig ist.</string>

diff --git 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 @@
     <!-- scanning errors -->
 
     <string name="send_error_invoice_expired">Esta factura ha caducado.</string>
+    <string name="send_error_payment_pending">Este pago ya se está procesando. Por favor, espere a que se complete.</string>
     <string name="send_error_already_paid">Este pago ya ha sido abonado.</string>
     <string name="send_error_invalid_chain">Este pago no utiliza la misma blockchain que su cartera.</string>
     <string name="send_error_lnurl_invalid">No se ha podido procesar este enlace LNURL. Asegúrese de que es válido.</string>

diff --git 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 @@
     <!-- scanning errors -->
 
     <string name="send_error_invoice_expired">Cette requête a expiré.</string>
+    <string name="send_error_payment_pending">Ce paiement est déjà en cours de traitement. Veuillez attendre qu\'il soit terminé.</string>
     <string name="send_error_already_paid">Ce paiement a déjà été réglé.</string>
     <string name="send_error_invalid_chain">Ce paiement n\'utilise pas la même blockchain que votre wallet.</string>
     <string name="send_error_lnurl_invalid">Ce lien LNURL n\'a pas pu être traité. Assurez-vous qu\'il soit valide.</string>

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
@@ -163,6 +163,7 @@
     <!-- scanning errors -->
 
     <string name="send_error_invoice_expired">Esta fatura está expirada.</string>
+    <string name="send_error_payment_pending">Este pagamento já está sendo processado. Aguarde a conclusão.</string>
     <string name="send_error_already_paid">Este pagamento já foi pago.</string>
     <string name="send_error_invalid_chain">Este pagamento não usa o mesmo blockchain que sua carteira.</string>
     <string name="send_error_lnurl_invalid">Falha ao processar esse link LNURL. Verifique se ele é válido.</string>

diff --git 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 @@
     <!-- scanning errors -->
 
     <string name="send_error_invoice_expired">Platnosť tejto faktúry vypršala.</string>
+    <string name="send_error_payment_pending">Táto platba sa už spracováva. Počkajte, prosím, na jej dokončenie.</string>
     <string name="send_error_already_paid">Táto platba už bola uhradená.</string>
     <string name="send_error_invalid_chain">Táto platba nepoužíva ten istý blockchain ako vaša peňaženka.</string>
     <string name="send_error_lnurl_invalid">Tento LNURL odkaz sa nepodarilo spracovať. Uistite sa, že je platný.</string>

diff --git 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 @@
     <!-- scanning errors -->
 
     <string name="send_error_invoice_expired">Ankara hii imeisha muda wake.</string>
+    <string name="send_error_payment_pending">Malipo haya tayari yanachakatwa. Tafadhali subiri ikamilike.</string>
     <string name="send_error_already_paid">Malipo haya tayari yamelipwa.</string>
     <string name="send_error_invalid_chain">Malipo haya hayatumii mnyororo sawa na pochi yako.</string>
     <string name="send_error_lnurl_invalid">Imeshindwa kuchakata kiungo hiki cha LNURL. Hakikisha ni halali.</string>

diff --git 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 @@
     <!-- scanning errors -->
 
     <string name="send_error_invoice_expired">Hoá đơn này đã hết hạn.</string>
+    <string name="send_error_payment_pending">Thanh toán này đang được xử lý. Vui lòng đợi để hoàn tất.</string>
     <string name="send_error_already_paid">Khoản thanh toán này đã được trả.</string>
     <string name="send_error_invalid_chain">Khoản thanh toán này không sử dụng cùng blockchain với ví của bạn.</string>
     <string name="send_error_lnurl_invalid">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.</string>

diff --git a/phoenix-android/src/main/res/values/important_strings.xml b/phoenix-android/src/main/res/values/important_strings.xml
@@ -167,6 +167,7 @@
     <!-- send errors -->
 
     <string name="send_error_invoice_expired">This invoice is expired.</string>
+    <string name="send_error_payment_pending">This payment is already being processed. Please wait for it to complete.</string>
     <string name="send_error_already_paid">This payment has already been paid.</string>
     <string name="send_error_invalid_chain">This payment does not use the same blockchain as your wallet.</string>
     <string name="send_error_lnurl_invalid">Failed to process this LNURL link. Make sure it is valid.</string>

diff --git a/phoenix-android/src/main/res/values/strings.xml b/phoenix-android/src/main/res/values/strings.xml
@@ -620,6 +620,7 @@
 
     <string name="lnurl_pay_error_header">Payment has failed.</string>
     <string name="lnurl_pay_error_invalid_chain">The invoice returned by <b>%1$s</b> does not use the same chain as your wallet.</string>
+    <string name="lnurl_pay_error_payment_pending">The invoice returned by <b>%1$s</b> is already in progress.</string>
     <string name="lnurl_pay_error_already_paid">The invoice returned by <b>%1$s</b> has already been paid.</string>
     <string name="lnurl_pay_error_invalid_amount">The invoice returned by <b>%1$s</b> has an incorrect amount.</string>
     <string name="lnurl_pay_error_invalid_malformed">The invoice returned by <b>%1$s</b> is malformed.</string>

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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(