search

lightningdevkit / ldk-node

A ready-to-go node implementation built using LDK.
Other
140 stars 72 forks source link

Add `SendingParameters` struct for customizable payments #336

Closed slanesuke closed 2 weeks ago

slanesuke commented 1 month ago

Based on #166 and #328

This PR introduces a PaymentParameters struct to allow users to override fields like max_total_cltv_expiry_delta, max_total_routing_fee_msat, and expiry_time. I'd appreciate suggestions on other useful fields to include as well.

I've added the optional PaymentParameters param to the send methods for both BOLT11 and Spontaneous payments. But, I need advice on whether we should be including it in the send methods for BOLT12 payments when paying for an offer. Specifically, I'm uncertain if values like max_total_cltv_expiry_delta or expiry_time can be overridden in this context.

joschisan commented 1 month ago

For fedimint just having max_total_routing_fee_msat as an absolute value would be the most user friendly.

tnull commented 1 month ago

For fedimint just having max_total_routing_fee_msat as an absolute value would be the most user friendly.

Thanks, good to know!

tnull commented 1 month ago

Ah, seems this needs a rebase to make CI pass (which we just fixed, excuse the inconvenience!)

slanesuke commented 1 month ago

Ah, seems this needs a rebase to make CI pass (which we just fixed, excuse the inconvenience!)

done!

slanesuke commented 1 month ago

rebased!

tnull commented 1 month ago

You'll also need to account for the new send argument in all the test cases (CLN, bindings tests, etc)l

slanesuke commented 3 weeks ago

@tnull I could use some advice on how to go about overriding routing params in the bolt12 methods. In bolt11/spontaneous send methods I could access the route parameters so it was a bit easier. But when paying offers I can't find where/how to override the existing routing params. Any thoughts?

slanesuke commented 3 weeks ago

rebased

tnull commented 3 weeks ago

@tnull I could use some advice on how to go about overriding routing params in the bolt12 methods. In bolt11/spontaneous send methods I could access the route parameters so it was a bit easier. But when paying offers I can't find where/how to override the existing routing params. Any thoughts?

I think unfortunately LDK currently doesn't provide a way to override all of the fields, now opened the issue here: https://github.com/lightningdevkit/rust-lightning/issues/3262

Let's keep this PR BOLT11/Spontaneous only and I'll open a tracking issue to make sure we won't forget to add SendingParameters to Bolt12Payment::send once the API becomes available.

tnull commented 3 weeks ago

I think CI fails as you haven't updated the Kotlin tests in bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt yet.

Could you also try to clean up the commit history a bit? Seems there are a few overlapping / back-and-forth commits going on.

slanesuke commented 3 weeks ago

@tnull I could use some advice on how to go about overriding routing params in the bolt12 methods. In bolt11/spontaneous send methods I could access the route parameters so it was a bit easier. But when paying offers I can't find where/how to override the existing routing params. Any thoughts?

I think unfortunately LDK currently doesn't provide a way to override all of the fields, now opened the issue here: lightningdevkit/rust-lightning#3262

Let's keep this PR BOLT11/Spontaneous only and I'll open a tracking issue to make sure we won't forget to add SendingParameters to Bolt12Payment::send once the API becomes available.

Okay, sounds good.