lightningdevkit / ldk-node

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

Retry pending payments on restart #171

Open tnull opened 11 months ago

tnull commented 11 months ago

When restarting, we currently don't retry any pending payments, but we should.

See ChannelManager docs

jbesraa commented 11 months ago

Happy to pick this up

jbesraa commented 11 months ago

So I was looking around to understand how we get the different params to send_payment_with_route and came up with this code. I fetch recent_payments from list_recent_payments that is mentioned in the docs and it looks like the function we want

does this make sense? is this the correct approach?

 16     /// Retry a pending payment after going offline.
|   15     pub fn retry_payment(&self) {
|   14         let cm = Arc::clone(&self.channel_manager);
|   13         let recent_payments = cm.list_recent_payments();
|   12         for payment in recent_payments {
|   11             match payment {
|   10                 RecentPaymentDetails::Pending { payment_hash, total_msat } => {
|    9                     let payment_in_store: PaymentDetails = self.payment_store.get(&payment_hash).unwrap();
|    8                     let invoice = Bolt11Invoice::from_str(std::str::from_utf8(&payment_in_store.hash.0).unwrap()).unwrap();
|    7                     let payer = self.channel_manager.get_our_node_id();
|    6                     let usable_channels = self.channel_manager.list_usable_channels();
|    5                     let first_hops = usable_channels.iter().collect::<Vec<_>>();
|    4                     let inflight_htlcs = self.channel_manager.compute_inflight_htlcs();
|    3                     let payment_params = PaymentParameters::from_node_id(
|    2                         invoice.recover_payee_pub_key(),
|    1                         invoice.min_final_cltv_expiry_delta() as u32,
| 730                      );
|    1                     let route_params = RouteParameters { payment_params, final_value_msat: total_msat };
|    2                     let route = self
|    3                         .router
|    4                         .find_route(&payer, &route_params, Some(&first_hops), inflight_htlcs)
|    5                         .map_err(|e| {
|    6                             log_error!(self.logger, "Failed to find path for payment probe: {:?}", e);
|    7                             // Error::ProbeSendingFailed
|    8                         }).unwrap();
|    9                     let payment_id = PaymentId(invoice.payment_hash().into_inner());
|   10                     let payment_secret = Some(*invoice.payment_secret());
|   11                     let recipient_onion = RecipientOnionFields { payment_secret, payment_metadata: None };
|   12                     let _ = cm.send_payment_with_route(
|   13                         &route,
|   14                         payment_hash,
|   15                         recipient_onion,
|   16                         payment_id,
|   17                     );
|   18                 }
|   19                 _ => {}
|   20             }
|   21         }
|   22     }
|   23
jbesraa commented 11 months ago

@tnull would love your input here (:

tnull commented 10 months ago

@tnull would love your input here (:

Excuse the delay getting back to this.

So I was looking around to understand how we get the different params to send_payment_with_route and came up with this code. I fetch recent_payments from list_recent_payments that is mentioned in the docs and it looks like the function we want

does this make sense? is this the correct approach?

Unfortunately I suspect that we currently don't store sufficient data to retry the payment as is (i.e., the payment hash is not the serialized invoice, so Bolt11Invoice::from_str as you suggest above wont work.

I'll assign myself here and will look into what steps we need to take, e.g., if we first have to implement invoice storage to this properly.