LDK-Review-Club-1643

LDK-Review-Club-1643.md

LDK Review Club 5th Session

Aug 16, 2022, 6 PM UTC, #review-club on Discord

lightningdevkit/rust-lightning#1643

Host: dunxen PR author: jurvis

The PR branch HEAD was 1762fa9 at the time of this review club meeting.

Notes

• LDK currently does not track in-flight HTLCs across outstanding payments when it comes to routing. This can be problematic in cases such as retrying payments where some parts failed: We won't consider the in-flight HTLC to occupy available liquidity across a path and we'd end up reusing such a path on retry even if it does not have enough liquidity for both HTLCs, which is clearly not helpful.
• InvoicePayer (aliased to InvoicePayerUsingTime) is a utility for paying invoices and sending spontaneous payments which can retry failed payment paths. It wraps a Router, a Payer (which ChannelManager implements), and other things needed for making payments.
• This PR introduces the ability to track in-flight HTLCs accross outstanding payments when routing to solve issues such as the one descirbed above.

Questions

1. Did you review the (draft) PR? Concept ACK/nACK?
2. This is the first time we're digging deeper into LDK's payment logic and the lightning-invoice crate, so firstly, what is the point of the lightning-invoice crate and what functionality does it provide?
3. In the payment module, what is the purpose of the InvoicePayer struct and how are Payer and Router involved when making payments.
4. How and where does this PR propose tracking the amounts for currently in-flight HTLCs? How are these amounts considered when making payments?
5. On what conditions/events will we stop tracking the amounts for these HTLCs? What does "stop tracking" mean here?
6. How might we improve this tracking in the future with CLTV deltas or other payment-context fields as well?

All good, the description is super clear and accurate! If you're looking for an opener, there could be follow-ups on how we use more payment-context related fields to improve scoring (e.g CLTV delta). Restart, and getting deep in runtime implications on in-flight HTLC tracking is fine too.

Thanks, @ariard! I went with the payment-context-related fields for the last one.