Merge pull request #1826 from RoboSats/fix-invoice-cleanup

Fix invoice cleanup

api/logics.py

@@ -219,15 +219,21 @@ class Logics:
     def is_buyer(order, user):
         is_maker = order.maker == user
         is_taker = order.taker == user
+        is_pretaker = TakeOrder.objects.filter(
+            taker=user, order=order, expires_at__gt=timezone.now()
+        ).exists()
         return (is_maker and order.type == Order.Types.BUY) or (
-            is_taker and order.type == Order.Types.SELL
+            (is_pretaker or is_taker) and order.type == Order.Types.SELL
         )
 
     def is_seller(order, user):
         is_maker = order.maker == user
         is_taker = order.taker == user
+        is_pretaker = TakeOrder.objects.filter(
+            taker=user, order=order, expires_at__gt=timezone.now()
+        ).exists()
         return (is_maker and order.type == Order.Types.SELL) or (
-            is_taker and order.type == Order.Types.BUY
+            (is_pretaker or is_taker) and order.type == Order.Types.BUY
         )
 
     def calc_sats(amount, exchange_rate, premium):
@@ -273,11 +279,10 @@ class Logics:
 
     @classmethod
     def take_order_expires(cls, take_order):
-        if take_order.expires_at < timezone.now():
+        if take_order.expires_at > timezone.now():
-            cls.cancel_bond(take_order.taker_bond)
+            take_order.expires_at = timezone.now()
-            return True
+            take_order.save(update_fields=["expires_at"])
-        else:
+        cls.cancel_bond(take_order.taker_bond)
-            return False
 
     @classmethod
     def order_expires(cls, order):
@@ -322,7 +327,7 @@ class Logics:
 
             take_orders_queryset = TakeOrder.objects.filter(order=order)
             for idx, take_order in enumerate(take_orders_queryset):
-                take_order.cancel(cls)
+                cls.take_order_expires(take_order)
 
             order.save(update_fields=["expiry_reason"])
 
@@ -446,7 +451,7 @@ class Logics:
     @classmethod
     def kick_taker(cls, take_order):
         """The taker did not lock the taker_bond. Now he has to go"""
-        take_order.cancel(cls)
+        cls.take_order_expires(take_order)
         # Add a time out to the taker
         if take_order.taker:
             robot = take_order.taker.robot
@@ -1083,7 +1088,7 @@ class Logics:
                     take_orders_queryset = TakeOrder.objects.filter(order=order)
                     for idx, take_order in enumerate(take_orders_queryset):
                         order.log("Pretaker bond was <b>unlocked</b>")
-                        take_order.cancel(cls)
+                        cls.take_order_expires(take_order)
 
                     send_notification.delay(
                         order_id=order.id, message="public_order_cancelled"

api/management/commands/clean_orders.py

@@ -49,8 +49,6 @@ class Command(BaseCommand):
                         order=order, expires_at__gt=timezone.now()
                     )
                     for idx, take_order in enumerate(take_orders_queryset):
-                        take_order.expires_at = order.expires_at
-                        take_order.save()
                         Logics.take_order_expires(take_order)
 
             # It should not happen, but if it cannot locate the hold invoice
@@ -77,10 +75,9 @@ class Command(BaseCommand):
         for idx, take_order in enumerate(take_orders_queryset):
             context = str(take_order) + " was expired"
             try:
-                if Logics.take_order_expires(
+                Logics.take_order_expires(take_order)
-                    take_order
+                take_order.delete()
-                ):  # Take order send to expire here
+                debug["expired_take_orders"].append({idx: context})
-                    debug["expired_take_orders"].append({idx: context})
 
             # It should not happen, but if it cannot locate the hold invoice
             # it probably was cancelled by another thread, make it expire anyway.

api/management/commands/follow_invoices.py

@@ -302,8 +302,8 @@ class Command(BaseCommand):
                 return
 
             elif hasattr(lnpayment, "order_taken"):
-                self.stderr.write("Expiting take order with cancelled escrow")
+                self.stderr.write("Expiting order with cancelled escrow")
-                Logics.take_order_expires(lnpayment.order_escrow)
+                Logics.order_expires(lnpayment.order_taken)
                 return
 
         # TODO If a lnpayment goes from LOCKED to INVGEN. Totally weird

api/models/take_order.py

@@ -2,7 +2,6 @@ from django.core.validators import MaxValueValidator, MinValueValidator
 from django.contrib.auth.models import User
 from django.db import models
 from django.conf import settings
-from django.utils import timezone
 
 
 class TakeOrder(models.Model):
@@ -41,11 +40,5 @@ class TakeOrder(models.Model):
     # timestamp of last_satoshis
     last_satoshis_time = models.DateTimeField(null=True, default=None, blank=True)
 
-    def cancel(self, cls):
-        if self.expires_at > timezone.now():
-            self.expires_at = timezone.now()
-            self.save(update_fields=["expires_at"])
-        cls.cancel_bond(self.taker_bond)
-
     def __str__(self):
         return f"Order {self.order.id} taken by Robot({self.taker.robot.id},{self.taker.username}) for {self.amount} fiat units"

tests/test_trade_pipeline.py

@@ -437,7 +437,7 @@ class TradeTest(BaseAPITestCase):
         self.assertAlmostEqual(float(data["amount"]), 80)
         self.assertFalse(data["is_maker"])
         self.assertFalse(data["is_buyer"])
-        self.assertFalse(data["is_seller"])
+        self.assertTrue(data["is_seller"])
         self.assertTrue(data["is_taker"])
         self.assertTrue(data["is_participant"])
         self.assertTrue(data["maker_locked"])
@@ -506,7 +506,7 @@ class TradeTest(BaseAPITestCase):
         self.assertEqual(data["maker_status"], "Active")
         self.assertFalse(data["is_maker"])
         self.assertFalse(data["is_buyer"])
-        self.assertFalse(data["is_seller"])
+        self.assertTrue(data["is_seller"])
         self.assertTrue(data["is_taker"])
         self.assertTrue(data["is_participant"])
         self.assertTrue(data["maker_locked"])
@@ -544,7 +544,7 @@ class TradeTest(BaseAPITestCase):
         self.assertEqual(data["maker_status"], "Active")
         self.assertFalse(data["is_maker"])
         self.assertFalse(data["is_buyer"])
-        self.assertFalse(data["is_seller"])
+        self.assertTrue(data["is_seller"])
         self.assertTrue(data["is_taker"])
         self.assertTrue(data["is_participant"])
         self.assertTrue(data["maker_locked"])