Fix/cancel orders expired holdinvoice continued

jobs/cancel_orders.ts

@@ -1,6 +1,7 @@
 import { HasTelegram } from '../bot/start';
 import { User, Order } from '../models';
 import { cancelShowHoldInvoice, cancelAddInvoice } from '../bot/commands';
+import { cancelHoldInvoice } from '../ln';
 import * as messages from '../bot/messages';
 import {
   getUserI18nContext,
@@ -116,10 +117,75 @@ const cancelOrders = async (bot: HasTelegram) => {
       ],
     });
     for (const order of expiredOrders) {
-      order.status = 'EXPIRED';
-      await order.save();
-      OrderEvents.orderUpdated(order);
-      logger.info(`Order Id ${order.id} expired!`);
+      await PerOrderIdMutex.instance.runExclusive(
+        String(order._id),
+        async () => {
+          const updatedOrder = await Order.findById(order._id);
+          if (!updatedOrder) return;
+          // Don't stomp an order that advanced after the find above (e.g. a
+          // release/payout currently in flight under the same mutex).
+          if (
+            updatedOrder.status !== 'ACTIVE' &&
+            updatedOrder.status !== 'FIAT_SENT'
+          )
+            return;
+
+          // For ACTIVE orders the buyer never signalled fiat-sent, so it is
+          // safe to cancel the hold invoice and refund the seller instead of
+          // orphaning the hash. check_hold_invoice_expired ignores EXPIRED
+          // orders, so without this the seller's funds would stay locked until
+          // the on-chain CLTV timeout. FIAT_SENT orders are NOT auto-refunded
+          // here (the buyer claims to have paid) and are left for the
+          // dispute/admin flow.
+          if (updatedOrder.status === 'ACTIVE' && updatedOrder.hash) {
+            try {
+              await cancelHoldInvoice({ hash: updatedOrder.hash });
+            } catch (error) {
+              logger.error(
+                `Order Id ${updatedOrder.id}: failed to cancel hold invoice, will retry on next run: ${error}`,
+              );
+              return; // leave the order ACTIVE so the next run retries it
+            }
+
+            // Only reached if the cancellation succeeded.
+            // The seller's sats are no longer in escrow: warn both parties so
+            // the buyer doesn't send fiat off-platform expecting the hold
+            // invoice to still be backing the trade.
+            const buyerUser = await User.findOne({
+              _id: updatedOrder.buyer_id,
+            });
+            const sellerUser = await User.findOne({
+              _id: updatedOrder.seller_id,
+            });
+            if (buyerUser !== null && sellerUser !== null) {
+              const i18nCtxBuyer = await getUserI18nContext(buyerUser);
+              const i18nCtxSeller = await getUserI18nContext(sellerUser);
+              await messages.toBuyerHoldInvoiceExpiredMessage(
+                bot,
+                buyerUser,
+                updatedOrder,
+                i18nCtxBuyer,
+              );
+              await messages.toSellerHoldInvoiceExpiredMessage(
+                bot,
+                sellerUser,
+                updatedOrder,
+                i18nCtxSeller,
+              );
+            }
+          } else if (updatedOrder.status === 'FIAT_SENT' && updatedOrder.hash) {
+            logger.warning(
+              `Order Id ${updatedOrder.id} expired in FIAT_SENT with an open ` +
+                `hold invoice; leaving it for dispute/admin handling`,
+            );
+          }
+
+          updatedOrder.status = 'EXPIRED';
+          await updatedOrder.save();
+          OrderEvents.orderUpdated(updatedOrder);
+          logger.info(`Order Id ${updatedOrder.id} expired!`);
+        },
+      );
     }
   } catch (error) {
     logger.error(error);

ln/hold_invoice.ts

@@ -56,6 +56,9 @@ const cancelHoldInvoice = async ({ hash }: { hash: string }) => {
     await lightning.cancelHodlInvoice({ lnd, id: hash });
   } catch (error) {
     logger.error(error);
+    // Callers must not mark an order as canceled/expired if the
+    // invoice cancellation did not happen.
+    throw error;
   }
 };
 

tests/jobs/cancel_orders.spec.ts

@@ -0,0 +1,195 @@
+const { expect } = require('chai');
+const sinon = require('sinon');
+const proxyquire = require('proxyquire');
+
+proxyquire.noCallThru();
+
+describe('Job: cancel_orders (ACTIVE hold invoice cancellation branch)', () => {
+  let sandbox: any;
+  let cancelOrders: any;
+
+  let orderFindStub: any;
+  let orderFindByIdStub: any;
+  let userFindOneStub: any;
+  let cancelHoldInvoiceStub: any;
+  let toBuyerHoldInvoiceExpiredMessageStub: any;
+  let toSellerHoldInvoiceExpiredMessageStub: any;
+  let orderUpdatedStub: any;
+  let loggerWarningStub: any;
+  let loggerErrorStub: any;
+  let getUserI18nContextStub: any;
+
+  let expiredOrders: any[];
+  let updatedOrder: any;
+
+  const buyerUser = { _id: 'buyer1' };
+  const sellerUser = { _id: 'seller1' };
+  const i18nCtx = { t: (key: string) => key };
+
+  beforeEach(() => {
+    sandbox = sinon.createSandbox();
+
+    orderFindByIdStub = sandbox.stub().callsFake(async () => updatedOrder);
+
+    // The job runs three separate Order.find queries; only the "expired
+    // orders" query (no `$and`, no `admin_warned`) is relevant here.
+    orderFindStub = sandbox.stub().callsFake(async (query: any) => {
+      if (query && ('$and' in query || 'admin_warned' in query)) return [];
+      return expiredOrders;
+    });
+
+    userFindOneStub = sandbox.stub().callsFake(async ({ _id }: any) => {
+      if (_id === 'buyer1') return buyerUser;
+      if (_id === 'seller1') return sellerUser;
+      return null;
+    });
+
+    cancelHoldInvoiceStub = sandbox.stub().resolves();
+    toBuyerHoldInvoiceExpiredMessageStub = sandbox.stub().resolves();
+    toSellerHoldInvoiceExpiredMessageStub = sandbox.stub().resolves();
+    orderUpdatedStub = sandbox.stub();
+    loggerWarningStub = sandbox.stub();
+    loggerErrorStub = sandbox.stub();
+    getUserI18nContextStub = sandbox.stub().resolves(i18nCtx);
+
+    const jobModule = proxyquire('../../jobs/cancel_orders', {
+      '../models': {
+        User: { findOne: userFindOneStub },
+        Order: { find: orderFindStub, findById: orderFindByIdStub },
+      },
+      '../bot/commands': {
+        cancelShowHoldInvoice: sandbox.stub().resolves(),
+        cancelAddInvoice: sandbox.stub().resolves(),
+      },
+      '../ln': { cancelHoldInvoice: cancelHoldInvoiceStub },
+      '../bot/messages': {
+        expiredOrderMessage: sandbox.stub().resolves(),
+        toBuyerExpiredOrderMessage: sandbox.stub().resolves(),
+        toSellerExpiredOrderMessage: sandbox.stub().resolves(),
+        toBuyerHoldInvoiceExpiredMessage: toBuyerHoldInvoiceExpiredMessageStub,
+        toSellerHoldInvoiceExpiredMessage: toSellerHoldInvoiceExpiredMessageStub,
+      },
+      '../util': {
+        getUserI18nContext: getUserI18nContextStub,
+        holdInvoiceExpirationInSecs: () => ({
+          expirationTimeInSecs: 0,
+          safetyWindowInSecs: 0,
+        }),
+        PerOrderIdMutex: {
+          instance: {
+            runExclusive: async (_id: string, cb: () => Promise<any>) => cb(),
+          },
+        },
+      },
+      '../logger': {
+        logger: {
+          error: loggerErrorStub,
+          warning: loggerWarningStub,
+          info: sandbox.stub(),
+        },
+      },
+      '../bot/modules/events/orders': { orderUpdated: orderUpdatedStub },
+    });
+
+    cancelOrders = jobModule.default;
+  });
+
+  afterEach(() => {
+    sandbox.restore();
+  });
+
+  it('marks an ACTIVE order with a hash as EXPIRED when cancelHoldInvoice succeeds', async () => {
+    updatedOrder = {
+      _id: 'order1',
+      id: 'order1',
+      status: 'ACTIVE',
+      hash: 'hash1',
+      buyer_id: 'buyer1',
+      seller_id: 'seller1',
+      save: sandbox.stub().resolves(),
+    };
+    expiredOrders = [{ _id: 'order1' }];
+
+    await cancelOrders({} as any);
+
+    expect(cancelHoldInvoiceStub.calledOnceWith({ hash: 'hash1' })).to.equal(
+      true,
+    );
+    expect(updatedOrder.status).to.equal('EXPIRED');
+    expect(updatedOrder.save.calledOnce).to.equal(true);
+    expect(toBuyerHoldInvoiceExpiredMessageStub.calledOnce).to.equal(true);
+    expect(toSellerHoldInvoiceExpiredMessageStub.calledOnce).to.equal(true);
+    expect(orderUpdatedStub.calledOnceWith(updatedOrder)).to.equal(true);
+  });
+
+  it('leaves the order ACTIVE and sends no messages when cancelHoldInvoice throws', async () => {
+    updatedOrder = {
+      _id: 'order1',
+      id: 'order1',
+      status: 'ACTIVE',
+      hash: 'hash1',
+      buyer_id: 'buyer1',
+      seller_id: 'seller1',
+      save: sandbox.stub().resolves(),
+    };
+    expiredOrders = [{ _id: 'order1' }];
+    cancelHoldInvoiceStub.rejects(new Error('LND error: unable to cancel'));
+
+    await cancelOrders({} as any);
+
+    expect(cancelHoldInvoiceStub.calledOnceWith({ hash: 'hash1' })).to.equal(
+      true,
+    );
+    expect(updatedOrder.status).to.equal('ACTIVE');
+    expect(updatedOrder.save.called).to.equal(false);
+    expect(toBuyerHoldInvoiceExpiredMessageStub.called).to.equal(false);
+    expect(toSellerHoldInvoiceExpiredMessageStub.called).to.equal(false);
+    expect(orderUpdatedStub.called).to.equal(false);
+  });
+
+  it('marks an ACTIVE order without a hash as EXPIRED without calling cancelHoldInvoice', async () => {
+    updatedOrder = {
+      _id: 'order1',
+      id: 'order1',
+      status: 'ACTIVE',
+      hash: null,
+      buyer_id: 'buyer1',
+      seller_id: 'seller1',
+      save: sandbox.stub().resolves(),
+    };
+    expiredOrders = [{ _id: 'order1' }];
+
+    await cancelOrders({} as any);
+
+    expect(cancelHoldInvoiceStub.called).to.equal(false);
+    expect(updatedOrder.status).to.equal('EXPIRED');
+    expect(updatedOrder.save.calledOnce).to.equal(true);
+    expect(orderUpdatedStub.calledOnceWith(updatedOrder)).to.equal(true);
+  });
+
+  it('marks a FIAT_SENT order as EXPIRED without calling cancelHoldInvoice, warning instead', async () => {
+    updatedOrder = {
+      _id: 'order1',
+      id: 'order1',
+      status: 'FIAT_SENT',
+      hash: 'hash1',
+      buyer_id: 'buyer1',
+      seller_id: 'seller1',
+      save: sandbox.stub().resolves(),
+    };
+    expiredOrders = [{ _id: 'order1' }];
+
+    await cancelOrders({} as any);
+
+    expect(cancelHoldInvoiceStub.called).to.equal(false);
+    expect(loggerWarningStub.calledOnce).to.equal(true);
+    expect(loggerWarningStub.firstCall.args[0]).to.match(
+      /dispute\/admin handling/,
+    );
+    expect(updatedOrder.status).to.equal('EXPIRED');
+    expect(updatedOrder.save.calledOnce).to.equal(true);
+    expect(orderUpdatedStub.calledOnceWith(updatedOrder)).to.equal(true);
+  });
+});
+
+export {};