Use HODL invoices

[ekzyis]

Not working yet, see FIXME in api/resolvers/item.js:72

// FIXME:
// secret is null for HODL invoices since you have to specify your own hash with LND:
// https://api.lightning.community/api/lnd/invoices/add-hold-invoice#messages
// Therefore, LND does not know the preimage and we have to save it ourselves ...

Closes #415

TODO:

• find way to get preimage to settle hodl invoice after payment and successful action: added new column preimage to invoice table
• make sure hodl invoices are canceled to not risk force closures.
  According to this comment, HODL invoices are automatically canceled after they expire but I would like to see some official docs on this.
  -- invoices are now canceled if they expire in checkInvoice of worker
• delete code which is no longer needed like contacts: just add retry or cancel/refund button. refund will cancel the HODL invoice

---

Works now:

https://files.ekzyis.com/public/sn/hodl_invoices.mp4

---

New TODOs:

• rebase on master
• Fix invoices not used if session exists

[huumn]

Zero out your anon user's account balance and test this.

Because the invoice isn't settled before the action, anon's account has an insufficient balance.

This will probably require either

1. adding held funds to anon's account balance and reversing on cancel/expire
2. making exceptions throughout the code base for anon
3. adding held funds in another statement (within the same tx) before the action

[ekzyis]

Because the invoice isn't settled before the action, anon's account has an insufficient balance.

Good catch! Fixed that now. I picked

adding held funds in another statement (within the same tx) before the action

since it required the least code and rollback is automatic which seems like the most secure solution

https://files.ekzyis.com/public/sn/hodl_invoices_2.mp4

I'll keep this as a draft now though since I want to sleep over this and do another pass over the code. Maybe I missed something.

[huumn]

As we were philosophizing about code style on monday ...

One blanket observation about this PR that I think is worth noting is that we are introducing several more places where we are interacting with LND, ie when I imagine changing something about our interaction with lnd in the future I now have several more places to look/change.

This "interaction creep" is natural and it's not obvious to me how we'd reduce the "interaction points" in this case (as I'm not the author), but it's something we always want to keep in mind; ie how easy will it be for me to change this code in the future without introducing bugs.

[ekzyis]

As we were philosophizing about code style on monday ...

We did? 🙈 😄

One blanket observation about this PR that I think is worth noting is that we are introducing several more places where we are interacting with LND, ie when I imagine changing something about our interaction with lnd in the future I now have several more places to look/change.

You are right that it's used in more places now but afaict, the only new place is in api/resolvers/item.js to settle the hodl invoices, no?

Other than that, I only updated existing LND code in api/resolvers/wallet.js (to conditionally create HODL invoices instead of normal invoices which automatically settle) and in the worker.

This "interaction creep" is natural and it's not obvious to me how we'd reduce the "interaction points" in this case (as I'm not the author), but it's something we always want to keep in mind; ie how easy will it be for me to change this code in the future without introducing bugs.

I agree. I was thinking about if it's worth to also move the settlement code to the worker by confirming the invoice during item creation and adding a pgboss job. Then the worker would pick up the job and settle the invoice. Then all LND code for finalizing hodl invoices (settlement or cancellation) would be in the worker.

But I don't think it's worth it since it's more changes for only moving a single line to a different file imo

[huumn]

I agree. I was thinking about if it's worth to also move the settlement code to the worker by confirming the invoice during item creation and adding a pgboss job. Then the worker would pick up the job and settle the invoice. Then all LND code for finalizing hodl invoices (settlement or cancellation) would be in the worker.

But I don't think it's worth it since it's more changes for only moving a single line to a different file imo

I think you're thinking about this right. It's not important that we do it just this minute but it needs to be on The List™️.

[huumn]

Also sorry for my recent push, but bright side: it should allow you to delete a satisfying amount of duplicate code.

[ekzyis]

I'll keep this as a draft now though since I want to sleep over this and do another pass over the code. Maybe I missed something.

Yep, I did. The following current logic prevents paying per invoice for logged-in stackers:

api/resolvers/item.js:1123 @ 41600f8:

let invoice
if (me) {
  item.userId = Number(me.id)
} else {
  if (!invoiceHash) {
    throw new GraphQLError('you must be logged in or pay', { extensions: { code: 'FORBIDDEN' } })
  }
  invoice = await checkInvoice(models, invoiceHash, invoiceHmac, item.parentId ? ANON_COMMENT_FEE : ANON_POST_FEE)
  item.userId = invoice.user.id
  spamInterval = ANON_ITEM_SPAM_INTERVAL
}

[ekzyis]

I was thinking about adding #405 in this PR but I think it's better to do this in a separate PR now to not hold up this PR any longer.

It should be fairly easy though. With the current logic, any additional paid sats will already go to the user balance.
So just frontend changes required

[huumn]

If it interests you, please also consider refactoring "invoicable" stuff into a Form error handler.

[ekzyis]

Ok, this should be done now. Did a final pass on all requirements: https://files.ekzyis.com/public/sn/hodl_invoices_3.mp4

requirements:

• posting
  • discussion as anon and stacker
  • link as anon and stacker
  • poll as anon and stacker
  • bounty only as stacker
  • job only as stacker
• zapping as anon and stacker
• commenting as anon and stacker
• invoices are canceled if expired
• on error, user can cancel invoice manually

[huumn]

Love seeing all these code deletions! So satisfying!

[huumn]

Oh dang I just realized updating by rebasing re-authors commits. I'll stop doing that.

[huumn]

First of all, this is AMAZING.

One thing I've noticed so far: local storage isn't cleared on success ... better than losing it on failure though.

[ekzyis]

Oh dang I just realized updating by rebasing re-authors commits. I'll stop doing that.

Oh, I thought rebases only update "committed by" not "authored by"

First of all, this is AMAZING.

😊

One thing I've noticed so far: local storage isn't cleared on success ... better than losing it on failure though.

Oh, right, I'll try to think of something