search

stackernews / stacker.news

Internet communities that pay you Bitcoin
https://stacker.news
MIT License
439 stars 113 forks source link

Don't pay existing invoices already paid #720

Closed TonyGiorgio closed 10 months ago

TonyGiorgio commented 11 months ago

I've lost a few hundred sats due to accidentally using the autofill option instead of paste option on my phone.

I'd suggest these two things:

  1. Turn off auto fill on the withdraw page. I believe HTML form attributes make this easy.
  2. Lookup the invoice before attempting to pay it, fail the withdraw call if already paid.

I'm unsure if the payments were swiped by SN or by another node on the network (possibly by our Mutiny LSP).

ekzyis commented 11 months ago

Lookup the invoice before attempting to pay it, fail the withdraw call if already paid.

You're talking about invoices that weren't created by SN, right?

Since for invoices created by SN, we already do this by checking if the withdrawal invoice already exists in our database before paying it:

https://github.com/stackernews/stacker.news/blob/9698679d381e4706348c0c2263bb36090f03a01b/api/resolvers/wallet.js#L476-L486

If we've already seen this invoice, the database will respond with an error. See this video:

video https://github.com/stackernews/stacker.news/assets/27162016/028588df-2691-4956-9984-92348c767f43

Nonetheless, I am struggling to understand how paying the same invoice twice leads to a loss of funds. I definitely did not know that this might be a problem, so thanks for bringing this up! If you don't mind, can you elaborate on this?

Turn off auto fill on the withdraw page. I believe HTML form attributes make this easy.

But I agree, auto fill on this input doesn't make sense:

2023-12-29-234019_753x354_scrot

TonyGiorgio commented 11 months ago

Preimages are revealed to node routers when an invoice is paid. If they see the same payment being routed again with the same prrimage, they can swipe the funds.

I know for a fact that SN let me withdraw to the same invoice multiple times, so perhaps the logic for preventing that isn't working.

ekzyis commented 11 months ago

Preimages are revealed to node routers when an invoice is paid. If they see the same payment being routed again with the same prrimage, they can swipe the funds.

Ah, makes sense. Forgot that the preimages must be kept secret until the channels are closed - not until the payment is done.

I know for a fact that SN let me withdraw to the same invoice multiple times, so perhaps the logic for preventing that isn't working.

Just tried this on https://stacker.news/ with a Phoenix invoice. Same error when trying to withdraw to the same invoice again: withdrawal invoice already confirmed (to withdraw again create a new invoice)

Let's wait for @huumn on this then, maybe he knows more. He also can lookup stuff in the LN node if that helps.

huumn commented 11 months ago

I suspect tony is deleting his invoices so we can't throw this error.

We should however prevent autofilling.

ekzyis commented 11 months ago

We should however prevent autofilling.

Done in #721

TonyGiorgio commented 11 months ago

I suspect tony is deleting his invoices so we can't throw this error.

Ah, yeah I have auto delete turned on. Interesting. Is it deleted on the LND side as well? I didn't think LND would allow paying an invoice twice.

huumn commented 11 months ago

Is it deleted on the LND side as well?

It isn't but we an issue up for doing that too.

I didn't think LND would allow paying an invoice twice.

Same. I've definitely seen it throw invoice already paid messages before.