Fix GHSA-hp64-3gp5-v8fr

api/errors.py

@@ -72,6 +72,7 @@
     3003: "You have not earned rewards",
     3004: "Give me a new invoice",
     3005: "Invoice payment failure: {failure_reason}",
+    3006: "Payment status is uncertain, your rewards will be restored once it resolves. Check back later.",
     # 4000 - Bad address
     4000: "You submitted an empty address",
     4001: "The mining fee is too low. Must be higher than {min_mining_fee_rate} Sat/vbyte",

api/logics.py

@@ -1694,7 +1694,8 @@ def complete_order(cls, order):
         and computes the coordinator revenue.
         """
         if not order.transition_status(
-            Order.Status.SUC, from_statuses=[Order.Status.FSE, Order.Status.PAY, Order.Status.FAI]
+            Order.Status.SUC,
+            from_statuses=[Order.Status.FSE, Order.Status.PAY, Order.Status.FAI],
         ):
             return
 
@@ -1916,12 +1917,25 @@ def withdraw_rewards(cls, user, invoice, routing_budget_ppm):
             user.robot.save(update_fields=["earned_rewards", "claimed_rewards"])
             return True, None
 
-        # If fails, adds the rewards again.
-        else:
+        # Re-fetch lnpayment to get the status written by pay_invoice.
+        lnpayment.refresh_from_db(fields=["status"])
+
+        # Only restore rewards when the node has confirmed the payment definitively
+        # failed (FAILRO). Any other outcome — stream ended without a final status
+        # (VALIDI, unchanged) or still in-flight (FLIGHT on CLN PENDING) — means the
+        # HTLC may still settle. Restoring rewards in that window would allow a second
+        # withdrawal against the same funds. The LNPayment row with its payment_hash
+        # stays in the DB; a retry with the same invoice is blocked by the duplicate-
+        # hash guard above (error 3004) until the hash is resolved or expires.
+        if lnpayment.status == LNPayment.Status.FAILRO:
             user.robot.earned_rewards = num_satoshis
             user.robot.save(update_fields=["earned_rewards"])
             return False, new_error(3005, {"failure_reason": failure_reason})
 
+        # Payment outcome is ambiguous (in-flight or stream ended without final status).
+        # Rewards remain at 0 until the payment resolves. The user should retry later.
+        return False, new_error(3006, {"failure_reason": failure_reason})
+
     @classmethod
     def compute_proceeds(cls, order):
         """

api/tests/test_withdraw_rewards.py

@@ -0,0 +1,180 @@
+"""
+Unit tests for Logics.withdraw_rewards (M-02 fix).
+
+Verifies that earned_rewards is only restored when the node has confirmed
+the payment definitively failed (LNPayment.Status.FAILRO).  Any ambiguous
+outcome — stream ended without a final status (status stays VALIDI) or
+still in-flight (status FLIGHT, e.g. CLN PENDING) — must NOT restore
+rewards, to prevent a double-spend via a second invoice submission.
+
+These tests mock LNNode.pay_invoice and the LNPayment model to avoid
+requiring a real Lightning node.
+"""
+
+from unittest.mock import MagicMock, patch
+
+from django.contrib.auth.models import User
+from django.test import TestCase
+
+from api.errors import ERRORS
+from api.models import LNPayment, Robot
+
+
+def _make_user(earned_rewards: int = 500):
+    """Create a minimal in-memory user/robot double for testing."""
+    user = MagicMock(spec=User)
+    robot = MagicMock(spec=Robot)
+    robot.pk = 1
+    robot.earned_rewards = earned_rewards
+    robot.claimed_rewards = 0
+    user.robot = robot
+    return user
+
+
+def _make_lnpayment(status=LNPayment.Status.VALIDI, num_satoshis=490):
+    """Return a minimal LNPayment-like object."""
+    lnp = MagicMock(spec=LNPayment)
+    lnp.status = status
+    lnp.num_satoshis = num_satoshis
+    return lnp
+
+
+class WithdrawRewardsTest(TestCase):
+    """
+    Tests for Logics.withdraw_rewards covering the three pay_invoice outcomes.
+
+    The atomic block and DB interactions are mocked so these run without a
+    full database / Lightning node.
+    """
+
+    def setUp(self):
+        self.invoice = "lnbc1..."
+        self.routing_budget_ppm = 1000
+        self.num_satoshis = 490  # earned_rewards(500) minus routing budget
+
+    def _run_withdraw(self, user, pay_invoice_return, lnpayment_status_after):
+        """
+        Helper: patches all external calls and runs withdraw_rewards.
+
+        pay_invoice_return     – (paid: bool, failure_reason: str|None)
+        lnpayment_status_after – the LNPayment.status value after pay_invoice runs
+                                  (simulates what pay_invoice writes to the DB row)
+        """
+        lnp = _make_lnpayment(
+            status=LNPayment.Status.VALIDI,  # initial status
+            num_satoshis=self.num_satoshis,
+        )
+
+        def fake_refresh_from_db(fields=None):
+            # Simulate pay_invoice having updated the status in the DB.
+            lnp.status = lnpayment_status_after
+
+        lnp.refresh_from_db = fake_refresh_from_db
+
+        validate_result = {
+            "valid": True,
+            "description": "test",
+            "payment_hash": "abc123",
+            "created_at": "2026-01-01T00:00:00Z",
+            "expires_at": "2026-01-02T00:00:00Z",
+        }
+
+        with (
+            patch("api.logics.transaction") as mock_tx,
+            patch("api.logics.Robot.objects") as mock_robot_qs,
+            patch("api.logics.LNPayment.objects") as mock_lnp_qs,
+            patch("api.logics.User.objects") as mock_user_qs,
+            patch(
+                "api.logics.LNNode.validate_ln_invoice", return_value=validate_result
+            ),
+            patch("api.logics.LNNode.pay_invoice", return_value=pay_invoice_return),
+        ):
+            # transaction.atomic() as context manager
+            mock_tx.atomic.return_value.__enter__ = MagicMock(return_value=None)
+            mock_tx.atomic.return_value.__exit__ = MagicMock(return_value=False)
+
+            # select_for_update().get() returns the robot
+            mock_robot_qs.select_for_update.return_value.get.return_value = user.robot
+
+            # LNPayment.objects.create() returns our fake lnpayment
+            mock_lnp_qs.create.return_value = lnp
+
+            # User.objects.get() for ESCROW_USERNAME
+            mock_user_qs.get.return_value = MagicMock()
+
+            from api.logics import Logics
+
+            return Logics.withdraw_rewards(user, self.invoice, self.routing_budget_ppm)
+
+    # ------------------------------------------------------------------
+    # Case 1: payment SUCCEEDED — rewards stay zeroed, claimed_rewards bumped
+    # ------------------------------------------------------------------
+    def test_succeeded_zeros_rewards_and_credits_claimed(self):
+        user = _make_user(earned_rewards=500)
+        paid, error = self._run_withdraw(
+            user,
+            pay_invoice_return=(True, None),
+            lnpayment_status_after=LNPayment.Status.SUCCED,
+        )
+
+        self.assertTrue(paid)
+        self.assertIsNone(error)
+        self.assertEqual(user.robot.earned_rewards, 0)
+        self.assertEqual(user.robot.claimed_rewards, self.num_satoshis)
+
+    # ------------------------------------------------------------------
+    # Case 2: payment definitively FAILED (FAILRO) — rewards restored
+    # ------------------------------------------------------------------
+    def test_failro_restores_rewards(self):
+        user = _make_user(earned_rewards=500)
+        paid, error = self._run_withdraw(
+            user,
+            pay_invoice_return=(False, "no route"),
+            lnpayment_status_after=LNPayment.Status.FAILRO,
+        )
+
+        self.assertFalse(paid)
+        self.assertIn("bad_invoice", error)
+        self.assertEqual(error["error_code"], 3005)
+        # Rewards must be restored to the routing-budget-trimmed invoice amount.
+        self.assertEqual(user.robot.earned_rewards, self.num_satoshis)
+
+    # ------------------------------------------------------------------
+    # Case 3: ambiguous — stream ended without final status (status stays
+    # VALIDI).  Rewards must NOT be restored (double-spend prevention).
+    # ------------------------------------------------------------------
+    def test_ambiguous_stream_end_does_not_restore_rewards(self):
+        user = _make_user(earned_rewards=500)
+        paid, error = self._run_withdraw(
+            user,
+            pay_invoice_return=(False, "Payment stream ended without a final status"),
+            lnpayment_status_after=LNPayment.Status.VALIDI,  # unchanged
+        )
+
+        self.assertFalse(paid)
+        self.assertIn("bad_invoice", error)
+        self.assertEqual(error["error_code"], 3006)
+        # The critical assertion: rewards must remain 0.
+        self.assertEqual(user.robot.earned_rewards, 0)
+        self.assertIn(
+            ERRORS[3006],
+            error["bad_invoice"],
+        )
+
+    # ------------------------------------------------------------------
+    # Case 4: CLN PENDING — status is FLIGHT.
+    # Rewards must NOT be restored.
+    # ------------------------------------------------------------------
+    def test_cln_pending_flight_does_not_restore_rewards(self):
+        user = _make_user(earned_rewards=500)
+        paid, error = self._run_withdraw(
+            user,
+            pay_invoice_return=(False, "Payment isn't failed (yet)"),
+            lnpayment_status_after=LNPayment.Status.FLIGHT,
+        )
+
+        self.assertFalse(paid)
+        self.assertIn("bad_invoice", error)
+        self.assertEqual(error["error_code"], 3006)
+        # Rewards must remain at 0 — HTLC is still in-flight.
+        self.assertEqual(user.robot.earned_rewards, 0)