subscribeToInvoice() call forget invoices after nodejs restarts

[grunch]

If nodejs restarts at the time an invoice is held

invoice with hash: a76fa7f91fffca163edbb1373c0358b065dc216397f02d51761f10f588e3db19 is being held!

Node loses tracking of this event and when releasing, if the invoice is paid, but the subscribeToInvoice does not follow the normal workflow, the status is not changed to PAID_HOLD_INVOICE and as a consequence the buyer is not paid.

We need to think a way of avoid this situation, one is never restart nodejs if at least one invoice is held, but we need to have a procedure to follow if the server crashes.

[csralvall]

I've been trying to figure this out. There are some things that I couldn't understand:

• How the orders that follow the paths below achieve a SUCCESS status in database?

  1. waitPayment ----> addInvoiceMessage ----> /fiatsent ----> fiatSentMessage ----> /release
  2. waitPayment ----> subscribeInvoice ----> onGoingTakeSellMessage ----> /fiatsent ----> fiatSentMessage ----> /release

• We can make some assumptions about the status of the lightning node connected to the bot? That would discard the situation where the node goes offline and the bot server restarts.

• I'm biased by the /buy orders because I've never done a /sell order. Is the /release command executed at some point of the transaction? It is not executed? What is the flow of the program in that case?

[grunch]

This is the order workflow

order type=buy

Order published (PENDING) -> taken (WAITING_PAYMENT) -> bot ask buyer invoice (WAITING_BUYER_INVOICE) -> ACTIVE -> /fiatsent (FIAT_SENT) -> /release (PAID_HOLD_INVOICE) -> after buyer's invoice paid (SUCCESS)

order type=sell

Order published (PENDING) -> taken (WAITING_BUYER_INVOICE) -> bot ask seller to pay invoice (WAITING_PAYMENT) -> ACTIVE -> /fiatsent (FIAT_SENT) -> /release (PAID_HOLD_INVOICE) -> after buyer's invoice paid (SUCCESS)

On waitPayment() we create the hold invoice to seller and create a nodejs event to listen when that invoice status change, that way we can monitorize and do stuff when we receive the seller's payment.

The issue is that I don't know how to persist that event if nodejs crash for any reason, we can save it on database and delete it after the seller release the payment but that implies that we need to change our logic on subscribeInvoice(), that's not my first option, I am want to hear ideas before do that.

[csralvall]

I was thinking that the we-can re subscribe to the invoice when executing the /release command using the order.hash. We could even recycle the subscribeInvoice function.

On /release execution, if invoice.is_held some messages will be printed again and we should verify that the logic triggered under that condition doesn't roll back things in the database, . If invoice.is_confirmed we should check again that the logic doesn't roll back changes in database, but it will put the order in path to SUCCESS again.

WDYT?

[grunch]

That's sounds great! I think we should check that re-subscribe and tie again the process doesn't get some unexpected errors, but the idea is simple and what I was looking for, great idea 🥇

[csralvall]

Good! I will start testing by my own.

[grunch]

I thought about this, the key is re-subscribe, I think the easiest way of doing it is:

1. nodejs starts
2. nodejs connects to lnd node
3. we run getinvoices({ lnd, is_unconfirmed: true })
4. If we get results with is_held: true we query order by hash
5. Then we subscribe again subscribeInvoice(bot, hash)

Before push this to production we need to test we can test it each separately.

[csralvall]

I've reproduced the behavior detailed above in this branch. I had to create a new function for lightning service and change some things in the files related to the server main process to make it work properly.

I've tested it in the following cases:

• Order = BUY.
  • Situation: The server restarts after invoice is paid and before the command /fiatsent is executed.
  • Result:
    1. The bot re sends the message asking for an invoice to the buyer. You can send a new invoice or the same invoice used in the first part of the operation.
    2. The bot asks the buyer to re send the command /fiatsent.
    3. The bot asks the seller to execute /release.
    4. The operation is finished successfully.
• Order = BUY.
  • Situation: The server restarts after invoice is paid and before the command /release is executed.
  • Result:
    1. The bot re sends the message asking for an invoice to the buyer. You can send a new invoice or the same invoice used in the first part of the operation.
    2. The bot asks the buyer to re send the command /fiatsent.
    3. The bot asks the seller to execute /release.
    4. The operation is finished successfully.

Info: In both cases the path followed by the code is: resubscribeInvoices => subscribeInvoice => onGoingTakeBuyMessage.

Drawback: The messages re sent didn't give any clue to the user about what was happening or if something gone wrong with their orders. A message explaining the situation could be helpful.

Caveat: The parameter is_unconfirmed in the function getInvoices seems to be not working, because the use of different values with that parameter return the same responses. I have tested it with the following code:

const unconfirmedInvoices = (await getInvoices({ lnd, is_unconfirmed: true })).invoices;
const confirmedInvoices = (await getInvoices({ lnd, is_unconfirmed: false })).invoices;
console.log(JSON.stringify(confirmedInvoices)==JSON.stringify(unconfirmedInvoices));

[grunch]

Great job!! I just tested and made some small fixes, here is the patch, apply it and try again

diff --git a/ln/resubscribe_invoices.js b/ln/resubscribe_invoices.js
index 52d774a..e0179c7 100644
--- a/ln/resubscribe_invoices.js
+++ b/ln/resubscribe_invoices.js
@@ -14,16 +14,17 @@ const resubscribeInvoices = async (bot) => {
     if (Array.isArray(unconfirmedInvoices) && unconfirmedInvoices.length > 0) {
         const heldInvoices = unconfirmedInvoices.filter(isHeld);
         for (const invoice of heldInvoices) {
-            const orderInDB = Order.findById(invoice.id);
+          const orderInDB = await Order.findOne({ hash: invoice.id });
             if(!!orderInDB) {
-                await subscribeInvoice(bot, invoice.id);
-                invoicesReSubscribed++;
+              console.log(`Re-subscribing: invoice with hash: ${invoice.id} is being held!`);
+              await subscribeInvoice(bot, invoice.id, true);
+              invoicesReSubscribed++;
             } 
         };
     }
     console.log(`Invoices resubscribed: ${invoicesReSubscribed}`);
   } catch (error) {
-    console.log(`resuscribeInvoice catch: ${error}`);
+    console.log(`ResuscribeInvoice catch: ${error}`);
     return false;
   }
 };
diff --git a/ln/subscribe_invoice.js b/ln/subscribe_invoice.js
index 8173436..29f93f5 100644
--- a/ln/subscribe_invoice.js
+++ b/ln/subscribe_invoice.js
@@ -4,11 +4,11 @@ const { payToBuyer } = require('./pay_request');
 const lnd = require('./connect');
 const messages = require('../bot/messages');
 
-const subscribeInvoice = async (bot, id) => {
+const subscribeInvoice = async (bot, id, resub) => {
   try {
     const sub = subscribeToInvoice({ id, lnd });
     sub.on('invoice_updated', async (invoice) => {
-      if (invoice.is_held) {
+      if (invoice.is_held && !resub) {
         console.log(`invoice with hash: ${id} is being held!`);
         const order = await Order.findOne({ hash: invoice.id });
         const buyerUser = await User.findOne({ _id: order.buyer_id });

[csralvall]

I've just tested it. It works perfectly. For the sake of completeness I updated my test cases results:

• Order = BUY.

  • Situation: The server restarts after invoice is paid and before the command /fiatsent is executed.
  • Result:
    1. The bot keeps waiting for the /fiatsent command.
    2. /fiatsent is executed.
    3. /release is executed.
    4. The operation is finished successfully.

• Order = BUY.

  • Situation: The server restarts after invoice is paid and before the command /release is executed.
  • Result:
    1. The bot keeps waiting for the /release command.
    2. /release is executed.
    3. The operation is finished successfully.

I will open a PR with the changes.