fix: update success status (#3244)

Co-authored-by: Vlad Stan <stan.v.vlad@gmail.com>

lnbits/app.py

@@ -27,6 +27,7 @@ from lnbits.core.helpers import migrate_extension_database
 from lnbits.core.models.notifications import NotificationType
 from lnbits.core.services.extensions import deactivate_extension, get_valid_extensions
 from lnbits.core.services.notifications import enqueue_notification
+from lnbits.core.services.payments import check_pending_payments
 from lnbits.core.tasks import (
     audit_queue,
     collect_exchange_rates_data,
@@ -57,7 +58,6 @@ from .core import init_core_routers
 from .core.db import core_app_extra
 from .core.models.extensions import Extension, ExtensionMeta, InstallableExtension
 from .core.services import check_admin_settings, check_webpush_settings
-from .core.services.payments import check_pending_payments
 from .middleware import (
     AuditMiddleware,
     ExtensionsRedirectMiddleware,
@@ -66,11 +66,7 @@ from .middleware import (
     add_ip_block_middleware,
     add_ratelimit_middleware,
 )
-from .tasks import (
-    internal_invoice_listener,
-    invoice_listener,
-    run_interval,
-)
+from .tasks import internal_invoice_listener, invoice_listener, run_interval
 
 
 async def startup(app: FastAPI):

lnbits/core/services/payments.py

@@ -323,17 +323,14 @@ async def update_pending_payments(wallet_id: str):
         await update_pending_payment(payment)
 
 
-async def update_pending_payment(payment: Payment) -> bool:
+async def update_pending_payment(payment: Payment) -> Payment:
     status = await payment.check_status()
     if status.failed:
         payment.status = PaymentState.FAILED
         await update_payment(payment)
-        return True
-    if status.success:
-        payment.status = PaymentState.SUCCESS
-        await update_payment(payment)
-        return True
-    return False
+    elif status.success:
+        payment = await update_payment_success_status(payment, status)
+    return payment
 
 
 async def check_pending_payments():
@@ -357,20 +354,9 @@ async def check_pending_payments():
     if count > 0:
         logger.info(f"Task: checking {count} pending payments of last 15 days...")
         for i, payment in enumerate(pending_payments):
-            status = await payment.check_status()
+            payment = await update_pending_payment(payment)
             prefix = f"payment ({i+1} / {count})"
-            if status.failed:
-                payment.status = PaymentState.FAILED
-                await update_payment(payment)
-                logger.debug(f"{prefix} failed {payment.checking_id}")
-            elif status.success:
-                payment.fee = status.fee_msat or 0
-                payment.preimage = status.preimage
-                payment.status = PaymentState.SUCCESS
-                await update_payment(payment)
-                logger.debug(f"{prefix} success {payment.checking_id}")
-            else:
-                logger.debug(f"{prefix} pending {payment.checking_id}")
+            logger.debug(f"{prefix} {payment.status} {payment.checking_id}")
             await asyncio.sleep(0.01)  # to avoid complete blocking
         logger.info(
             f"Task: pending check finished for {count} payments"
@@ -765,19 +751,20 @@ async def _pay_external_invoice(
     )
 
     fee_reserve_msat = fee_reserve(amount_msat, internal=False)
-    service_fee_msat = service_fee(amount_msat, internal=False)
 
     task = create_task(
         _fundingsource_pay_invoice(checking_id, payment.bolt11, fee_reserve_msat)
     )
 
     # make sure a hold invoice or deferred payment is not blocking the server
+    wait_time = max(1, settings.lnbits_funding_source_pay_invoice_wait_seconds)
     try:
-        wait_time = max(1, settings.lnbits_funding_source_pay_invoice_wait_seconds)
-        payment_response = await asyncio.wait_for(task, wait_time)
+        payment_response = await asyncio.wait_for(task, timeout=wait_time)
     except asyncio.TimeoutError:
         # return pending payment on timeout
-        logger.debug(f"payment timeout, {checking_id} is still pending")
+        logger.debug(
+            f"payment timeout after {wait_time}s, {checking_id} is still pending"
+        )
         return payment
 
     # payment failed
@@ -791,18 +778,28 @@ async def _pay_external_invoice(
         message = payment_response.error_message or "without an error message."
         raise PaymentError(f"Payment failed: {message}", status="failed")
 
-    # payment.ok can be True (paid) or None (pending)!
-    payment.status = (
-        PaymentState.SUCCESS if payment_response.ok is True else PaymentState.PENDING
-    )
-    payment.fee = -(abs(payment_response.fee_msat or 0) + abs(service_fee_msat))
-    payment.preimage = payment_response.preimage
-    await update_payment(payment, payment_response.checking_id, conn=conn)
-    payment.checking_id = payment_response.checking_id
-    if payment.success:
+    if payment_response.success:
+        payment = await update_payment_success_status(
+            payment, payment_response, conn=conn
+        )
         await send_payment_notification(wallet, payment)
         logger.success(f"payment successful {payment_response.checking_id}")
 
+    payment.checking_id = payment_response.checking_id
+    return payment
+
+
+async def update_payment_success_status(
+    payment: Payment,
+    status: PaymentStatus,
+    conn: Optional[Connection] = None,
+) -> Payment:
+    if status.success:
+        service_fee_msat = service_fee(payment.amount, internal=False)
+        payment.status = PaymentState.SUCCESS
+        payment.fee = -(abs(status.fee_msat or 0) + abs(service_fee_msat))
+        payment.preimage = payment.preimage or status.preimage
+        await update_payment(payment, conn=conn)
     return payment
 
 
@@ -836,8 +833,7 @@ async def _verify_external_payment(
 
     if status.success:
         # payment was successful on the fundingsource
-        payment.status = PaymentState.SUCCESS
-        await update_payment(payment, conn=conn)
+        await update_payment_success_status(payment, status, conn=conn)
         raise PaymentError(
             "Failed payment was already paid on the fundingsource.",
             status="success",

lnbits/wallets/breez_liquid.py

@@ -196,7 +196,7 @@ else:
             fees = req.fees_sat * 1000 if req.fees_sat and req.fees_sat > 0 else 0
 
             if payment.status != breez_sdk.PaymentState.COMPLETE:
-                return await self._wait_for_outgoing_payment(checking_id, fees, 5)
+                return await self._wait_for_outgoing_payment(checking_id, fees, 10)
 
             if not isinstance(payment.details, breez_sdk.PaymentDetails.LIGHTNING):
                 return PaymentResponse(
@@ -221,9 +221,13 @@ else:
                     return PaymentPendingStatus()
                 if payment.status == breez_sdk.PaymentState.FAILED:
                     return PaymentFailedStatus()
-                if payment.status == breez_sdk.PaymentState.COMPLETE:
+                if payment.status == breez_sdk.PaymentState.COMPLETE and isinstance(
+                    payment.details, breez_sdk.PaymentDetails.LIGHTNING
+                ):
                     return PaymentSuccessStatus(
-                        paid=True, fee_msat=int(payment.fees_sat * 1000)
+                        paid=True,
+                        fee_msat=int(payment.fees_sat * 1000),
+                        preimage=payment.details.preimage,
                     )
                 return PaymentPendingStatus()
             except Exception as exc:
@@ -272,6 +276,7 @@ else:
         async def _wait_for_outgoing_payment(
             self, checking_id: str, fees: int, timeout: int
         ) -> PaymentResponse:
+            logger.debug(f"waiting for outgoing payment {checking_id} to complete")
             try:
                 breez_outgoing_queue[checking_id] = Queue()
                 payment_details = await asyncio.wait_for(

tests/conftest.py

@@ -307,6 +307,7 @@ def _settings_cleanup(settings: Settings):
     settings.lnbits_reserve_fee_percent = 1
     settings.lnbits_reserve_fee_min = 2000
     settings.lnbits_service_fee = 0
+    settings.lnbits_reserve_fee_percent = 0
     settings.lnbits_wallet_limit_daily_max_withdraw = 0
     settings.lnbits_admin_extensions = []
     settings.lnbits_admin_users = []

tests/unit/test_pay_invoice.py

@@ -359,16 +359,19 @@ async def test_retry_failed_invoice(
 
 @pytest.mark.anyio
 async def test_pay_external_invoice_pending(
-    from_wallet: Wallet, mocker: MockerFixture, external_funding_source: FakeWallet
+    from_wallet: Wallet,
+    mocker: MockerFixture,
+    external_funding_source: FakeWallet,
+    settings: Settings,
 ):
+    settings.lnbits_reserve_fee_min = 1000  # msats
     invoice_amount = 2103
     external_invoice = await external_funding_source.create_invoice(invoice_amount)
     assert external_invoice.payment_request
     assert external_invoice.checking_id
 
-    preimage = "0000000000000000000000000000000000000000000000000000000000002103"
     payment_reponse_pending = PaymentResponse(
-        ok=None, checking_id=external_invoice.checking_id, preimage=preimage
+        ok=None, checking_id=external_invoice.checking_id
     )
     mocker.patch(
         "lnbits.wallets.FakeWallet.pay_invoice",
@@ -392,12 +395,12 @@ async def test_pay_external_invoice_pending(
     assert _payment.checking_id == payment.payment_hash
     assert _payment.amount == -2103_000
     assert _payment.bolt11 == external_invoice.payment_request
-    assert _payment.preimage == preimage
 
     wallet = await get_wallet(from_wallet.id)
     assert wallet
+    reserve_fee_sat = int(abs(settings.lnbits_reserve_fee_min // 1000))
     assert (
-        balance_before - invoice_amount == wallet.balance
+        balance_before - invoice_amount - reserve_fee_sat == wallet.balance
     ), "Pending payment is subtracted."
 
     assert ws_notification.call_count == 0, "Websocket notification not sent."
@@ -405,8 +408,12 @@ async def test_pay_external_invoice_pending(
 
 @pytest.mark.anyio
 async def test_retry_pay_external_invoice_pending(
-    from_wallet: Wallet, mocker: MockerFixture, external_funding_source: FakeWallet
+    from_wallet: Wallet,
+    mocker: MockerFixture,
+    external_funding_source: FakeWallet,
+    settings: Settings,
 ):
+    settings.lnbits_reserve_fee_min = 2000  # msats
     invoice_amount = 2106
     external_invoice = await external_funding_source.create_invoice(invoice_amount)
     assert external_invoice.payment_request
@@ -440,9 +447,10 @@ async def test_retry_pay_external_invoice_pending(
 
     wallet = await get_wallet(from_wallet.id)
     assert wallet
-    # TODO: is this correct?
+    reserve_fee_sat = int(abs(settings.lnbits_reserve_fee_min // 1000))
+
     assert (
-        balance_before - invoice_amount == wallet.balance
+        balance_before - invoice_amount - reserve_fee_sat == wallet.balance
     ), "Failed payment is subtracted."
 
     assert ws_notification.call_count == 0, "Websocket notification not sent."