feat(ln-client): make LN gateway known upfront #3882
Conversation
| /// Pays a LN invoice with our available funds using the supplied `gateway`. | ||
| /// | ||
| /// The `gateway` can be acquired by calling | ||
| /// [`LightningClientModule::select_active_gateway`]. | ||
| pub async fn pay_bolt11_invoice<M: Serialize + MaybeSend + MaybeSync>( | ||
| &self, | ||
| gateway: LightningGateway, | ||
| invoice: Bolt11Invoice, | ||
| extra_meta: M, | ||
| ) -> anyhow::Result<OutgoingLightningPayment> { |
There was a problem hiding this comment.
This is the important part, everything else is just fixing up tests.
elsirion
force-pushed
the
2023-12-selectable-gw
branch
4 times, most recently
from
64d06d8
to
72f89f5
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3882 +/- ##
==========================================
- Coverage 57.09% 57.04% -0.06%
==========================================
Files 193 193
Lines 42649 42929 +280
==========================================
+ Hits 24351 24488 +137
- Misses 18298 18441 +143 ☔ View full report in Codecov by Sentry. |
elsirion
force-pushed
the
2023-12-selectable-gw
branch
from
72f89f5
to
1b8b635
Compare
|
Turns out we have tests for being able to pay internal invoices even if no LN GW is present (which is probably good). I'm trying a new, minimal approach, but longer term we need to think about how to best pull that pay fn apart so that users can be better informed. |
| let OutgoingLightningPayment { | ||
| payment_type, | ||
| contract_id, | ||
| fee: _, | ||
| } = user_lightning_module | ||
| .pay_bolt11_invoice(invoice.clone(), ()) | ||
| .pay_bolt11_invoice(gateway, invoice.clone(), ()) |
There was a problem hiding this comment.
why not call pay_invoice here?
There was a problem hiding this comment.
I think I got tired of repeating the same code after a while, introduced the fn but forgot that one.
fedimint-cli/src/client.rs
Outdated
| @@ -254,11 +254,14 @@ pub async fn handle_command( | |||
| let lightning_module = client.get_first_module::<LightningClientModule>(); | |||
| lightning_module.select_active_gateway().await?; | |||
|
|
|||
| let gateway = lightning_module.select_active_gateway().await.ok(); | |||
There was a problem hiding this comment.
I don't like that you're ignoring the error everywhere. Perhaps create a opt_select_active_gateway that returns an Option but also logs as a warning the error returned by select_active_gateway?
There was a problem hiding this comment.
I was a bit unsure about this, the reason why it's an option in the first place is that we currently have support for internal payments without a gateway being present. So it wasn't entirely clear to me if thats something we'd want to warn about till it's an actual problem. But you are right that it makes debugging the root cause harder, so will add it.
elsirion
force-pushed
the
2023-12-selectable-gw
branch
from
1b8b635
to
d585926
Compare
Otherwise, fees that will be paid cannot be shown to users upfront.
elsirion
force-pushed
the
2023-12-selectable-gw
branch
from
d585926
to
12d572f
Compare
Otherwise, fees that will be paid cannot be shown to users upfront.