search

lndk-org / lndk

MIT License
76 stars 19 forks source link

Feature: faster feedback on failure receiving an invoice #120

Open mauricepoirrier opened 1 month ago

mauricepoirrier commented 1 month ago

Feature Description

I've been playing with bolt12-playground and every now and then I mess up the network and I can't pay from lndk to another node (e.g., lndk-> eclair or lndk -> lndk). The node fails to find a path for sending the payment, and the cmd hangs for ~2 minutes. Logs indicate a failure to find a path and a timeout when waiting for an invoice.

2024-06-16 22:30:44 2024-06-17T02:30:44.281671463+00:00 TRACE lndk::onion_messenger - Failed to find path when sending OffersMessage
2024-06-16 22:32:24 2024-06-17T02:32:24.189796217+00:00 ERROR lndk - Did not receive invoice in 100 seconds.

Problems identified

  1. Path Finding: If lndk fails to find a path, it should not remain idle. An immediate error or a retry mechanism could be more effective.
  2. Waiting for Invoice Timeout: The fixed 100-second timeout for receiving an invoice could be made configurable to enhance UX in production and provide faster feedback during development / experimental phase.

Design

Double checking the code, when node sends the invoice request message through if something happen while connecting to peer it doesn't handle any error (or timeout if building the route in the future)

Proposed solutions

  1. Path finding: Raise a new error if lndk can't find a path. It allows faster feedback and it could be controlled for retries at user level or lndk level in the future.
  2. Waiting for invoice Timeout: Allow timeout setting in a parameter to the request or in the config file of the daemon. I don't really know which one could be a better approach.
orbitalturtle commented 1 month ago

Yes @mauricepoirrier I completely agree with this! It's been on my list of things to do. 😅 I think making the timeout configurable would be very nice. We could probably lower the default timeout to like 30 seconds as well.

If you're interested in working on this, go for it! (And if you do, please feel free to let me know if you have any questions about the LNDK code etc.)

mauricepoirrier commented 1 month ago

Thank you @orbitalturtle, I'll work on this!

Double check on the solution, making the timeout configurable means changing from the config file or the paramenters on the request, both or either 😅?

orbitalturtle commented 1 month ago

Good point, I like the idea of adding a parameter to the request, in case the user wants to try different timeouts without having to restart LNDK.