Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No new notification is received via the /v1/invoices/subscribe API for duplicated invoice payments #6791

Closed
AzulPretoBranco opened this issue Aug 3, 2022 · 5 comments

Comments

@AzulPretoBranco
Copy link

Background

I'm working on a project that integrates with the LND REST API to, among other things, monitor payed invoices.
The flow is something like:

  1. Create a new invoice using the REST API
  2. Share that invoice with a 3rd party
  3. The 3rd party eventually pays the invoice
  4. I receive a notification via the REST API (/v1/invoices/subscribe) that an invoice was settled and I proceed to do things on my system based on that.

This simple flow works fine, the problem happens when the same invoice is payed twice.
For example, that 3rd party shares the invoice with a friend and both try to pay the same invoice.

If that happens we will only receive a notification via the /v1/invoices/subscribe API that an invoice was settled for the first payment.
The second payment will be silently accepted by the LND and, as far I as know, there is no way for me to know that the second payment happened.

Your environment

Steps to reproduce

I'm able to replicate this use case relatively easy, the steps are:

  1. I create a new invoice in LND
  2. Pay that invoice using the Zap Wallet
  3. The payment happens with success and I receive a notification via /v1/invoices/subscribe
LND LOGS:
2022-08-03 17:38:31.611 [DBG] INVC: Invoice(pay_hash=ce5ec24501b9c279631e7164687186fe0dfc71abe635b8ce2abceb983f4bbd90, pay_addr=cb152bfcb34415faf10c2e8bda9a6103ba90077fe1cc69616311a3154d8f8355): settle resolution result outcome: settled, at accept height: 2314701, amt=10000 mSAT, expiry=2314744, circuit=(Chan ID=2227420:44:1, HTLC ID=159), mpp=total=10000 mSAT, addr=cb152bfcb34415faf10c2e8bda9a6103ba90077fe1cc69616311a3154d8f8355, amp=<nil>, metadata=<nil>
2022-08-03 17:38:31.611 [DBG] HSWC: ChannelLink(0d7e25860065eb0d642d5a98b1d568609ae3626a81462394f4ce8da54c86cd66:1): received settle resolution for (Chan ID=2227420:44:1, HTLC ID=159) with outcome: settled
2022-08-03 17:38:31.611 [INF] HSWC: ChannelLink(0d7e25860065eb0d642d5a98b1d568609ae3626a81462394f4ce8da54c86cd66:1): settling htlc ce5ec24501b9c279631e7164687186fe0dfc71abe635b8ce2abceb983f4bbd90 as exit hop
  1. Next I try to pay the same invoice again using the Lightning: Testnet Wallet
  2. The invoice is payed again with success but no notification is triggered
LND LOGS:
2022-08-03 17:40:20.529 [DBG] INVC: Invoice(pay_hash=ce5ec24501b9c279631e7164687186fe0dfc71abe635b8ce2abceb983f4bbd90): settle resolution result outcome: accepting duplicate payment to settled invoice, at accept height: 2314701, amt=10000 mSAT, expiry=2314744, circuit=(Chan ID=2227420:44:1, HTLC ID=160), mpp=<nil>, amp=<nil>, metadata=<nil>
2022-08-03 17:40:20.529 [DBG] HSWC: ChannelLink(0d7e25860065eb0d642d5a98b1d568609ae3626a81462394f4ce8da54c86cd66:1): received settle resolution for (Chan ID=2227420:44:1, HTLC ID=160) with outcome: accepting duplicate payment to settled invoice
2022-08-03 17:40:20.529 [INF] HSWC: ChannelLink(0d7e25860065eb0d642d5a98b1d568609ae3626a81462394f4ce8da54c86cd66:1): settling htlc ce5ec24501b9c279631e7164687186fe0dfc71abe635b8ce2abceb983f4bbd90 as exit hop

This is also a problem for other wallets like the Blue Wallet + LND Hub.
If we do the same steps:

  1. Generate a new invoice using the Blue Wallet
  2. Pay the first time using Zap Wallet
  3. Pay the same invoice a second time using the Lightning: Testnet Wallet

Both payments succeed but inside the Blue Wallet the balance doesn't reflect accordingly, the second payment is "lost" (stays in LND only) and doesn't reach the Blue Wallet (I believe the reason could be connected to this problem, most likely Blue Wallet is not notified of the second payment)

Expected behaviour

I was expected to be notified of all payments, even duplicated ones, but that doesn't seem to happen.
Without some kind of notification it is not possible to realistically and accurately process invoice payments.

If you think adding a notification for the second payment doesn't make sense what about adding a configuration to LND to instead of silently accepting the second payment reject it instead?

Actual behaviour

No new notification is received via the /v1/invoices/subscribe API for duplicated invoice payments.

Any questions please ask.

@positiveblue
Copy link
Contributor

positiveblue commented Aug 3, 2022

Hello @AzulPretoBranco I think that by default if you try to pay the same invoice twice you will get an error FAILURE_REASON_INCORRECT_PAYMENT_DETAILS (your node will reject the second payment).

But that's your node playing nice. Bolt11 Invoices should not be payed more than once. If the user shares it with 100 people, and 3 of them pay it, you have no way to know who those three are.

Paying the same invoice twice is not recommended. If I am one of the hops in the middle, I already saw the preimage for that invoice, which means that if the second payment goes through me I can take the full payment without having to forward it (i would be able to claim the htlc that the previous hop opened to me without having to create an htlc to the next hop)

There are some invoices designed to be paid multiple times (AMP for example) but for what you specify this is not the case. Are you using your own node with lndhub on top of it or you are using the one from the bluewallet team?

@AzulPretoBranco
Copy link
Author

Hi @positiveblue, thanks for the answer,

Paying the same invoice twice is not recommended. If I am one of the hops in the middle, I already saw the preimage for that invoice, which means that if the second payment goes through me I can take the full payment without having to forward it (i would be able to claim the htlc that the previous hop opened to me without having to create an htlc to the next hop)

Yes, I understand and agree with this, nevertheless, it is still technically possible, hence may question about whether or not we should receive a notification for the second payment.

There are some invoices designed to be paid multiple times (AMP for example) but for what you specify this is not the case. Are you using your own node with lndhub on top of it or you are using the one from the bluewallet team?

Correct, I'm not using AMP invoices, I'm using the regular "normal" invoices, created with lncli --network testnet addinvoice --amt 10

For now, lets forget about, Blue Wallet and LNDHub for the sake of simplicity.

Here is my setup:

  • I have my LND node (setup done by me) with a channel open with 1ML.com node ALPHA testnet node
  • I have "Zap Wallet" with a channel open with 1ML.com node ALPHA testnet node
  • And I also have another wallet, "Lightning: Testnet Wallet" also connected to 1ML.com node ALPHA testnet node
  • Honestly I don't know if 1ML.com node ALPHA testnet node is running LND or not
  1. I do an HTTP GET to /v1/invoices/subscribe using CURL via the cmd directly to my LND node
  2. On my LND node I run: lncli --network testnet addinvoice --amt 10
  3. I first pay the generated invoice using "Zap Wallet", the payment completes with success and I receive a notification via /v1/invoices/subscribe. After this I can see that the invoice state is "SETTLED" on my LND.
  4. After that I pay the same invoice via the "Lightning: Testnet Wallet", the payment also completes with success (my LND node did not rejected the payment with FAILURE_REASON_INCORRECT_PAYMENT_DETAILS) and I don't receive a notification via /v1/invoices/subscribe as explained in the ticket description

Looking at the code I can see that LND rejects duplicated payments in some flows but not others, for example, looking at invoices\update.go:

  • I believe (correct me if I'm wrong) func updateMpp will reject duplicated payments for "SETTLED" invoices, but func updateLegacy will accept duplicated payments. From the logs, the first payment via "Zap Wallet" invokes func updateMpp but the second one via "Lightning: Testnet Wallet" invokes func updateLegacy.

With that said I have two questions that maybe you can answer:

  1. In case an invoice is payed more than once, like in the use case I'm describing, shouldn't we receive a notification via /v1/invoices/subscribe for the second payment? Or the current LND implementation is working as expected?

  2. This question is more of a conceptual one with regards to "Bolt11 Invoices should not be payed more than once." and "Paying the same invoice twice is not recommended.".

    On a protocol level I am not aware of any mechanics to prevent paying the same invoice twice, and as you said, that can lead to all sorts of problems.

    With that in mind, my question is, the "should" and "not recommended" are offloaded to each wallet implementation to prevent this use case the best they can and at the same time User's knowledge of the lightning network to avoid paying the same invoice twice?

@positiveblue
Copy link
Contributor

You are correct.

First of all I had no idea about Lightning: Testnet Wallet. The app was using a really old version of the protocol, even before having tlvs, so it should be avoided. We have already removed it from the app store. No well-intentioned user should hit this because all the wallets are already updated and if someone hacks a tool to "exploit" it using the old format s/he is basically giving you money for free so it is not critical.

We need to rewrite some of this logic (after 6288) so it gets easier to reason about but this is basically a bug (for an edge case but a bug). If you tried to paid them in the reversed order: first with the Testnet wallet and then with blue wallet you would see that your node refuses to take the payment.

  1. In case an invoice is payed more than once, like in the use case I'm describing, shouldn't we receive a notification via /v1/invoices/subscribe for the second payment? Or the current LND implementation is working as expected?

Yes, it should notify you. This is something that shouldn't happen in production so it's low priority. I will create another issue to track it.

  1. With that in mind, my question is, the "should" and "not recommended" are offloaded to each wallet implementation to prevent this use case the best they can and at the same time User's knowledge of the lightning network to avoid paying the same invoice twice?

Yes, it has to be the sender the one that takes care of this because the sender cannot trust that the node s/he is paying to or any of the other nodes in the network will behave nicely.

@AzulPretoBranco
Copy link
Author

If you tried to paid them in the reversed order: first with the Testnet wallet and then with blue wallet you would see that your node refuses to take the payment.

Yes, I noticed that.

Yes, it should notify you. This is something that shouldn't happen in production so it's low priority. I will create another issue to track it.

Thanks, if possible please share the issue, I would like to keep an eye on it.

@positiveblue ok, I feel like I understand this flow a little bit better, once again thanks for the explanation.

@AzulPretoBranco
Copy link
Author

Hi @positiveblue,

I have a follow up question that maybe you can help with.

Still regarding this flow, is there any way I can configure my LND node to always go through the func updateMpp and always avoid the func updateLegacy?

My understanding is that only func updateLegacy accepts duplicated payments.
If I could configure LND in a way that requires the wallets to always send the fields that will make the flow enter the func updateMpp as a workaround I would be happy.

For example, I was taking a look at lnwire\features.go and noticed:

  • PaymentAddrRequired FeatureBit = 14
  • MPPRequired FeatureBit = 16

For my use case, does it make sense if I set both of those flags to required instead of optional?
Or they are unrelated with this flow and I'm confusing things?

Thanks!

@lucasdcf lucasdcf converted this issue into discussion #6797 Aug 4, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants