Use the `PaymentParameter` final CLTV delta over `RouteParameters`
authorMatt Corallo <git@bluematt.me>
Fri, 27 Jan 2023 20:28:20 +0000 (20:28 +0000)
committerMatt Corallo <git@bluematt.me>
Wed, 1 Feb 2023 21:16:18 +0000 (21:16 +0000)
`PaymentParams` is all about the parameters for a payment, i.e. the
parameters which are static across all the paths of a paymet.
`RouteParameters` is about the information specific to a given
`Route` (i.e. a set of paths, among multiple potential sets of
paths for a payment). The CLTV delta thus doesn't belong in
`RouterParameters` but instead in `PaymentParameters`.

Worse, because `RouteParameters` is built from the information in
the last hops of a `Route`, when we deliberately inflate the CLTV
delta in path-finding, retries of the payment will have the final
CLTV delta double-inflated as it inflates starting from the final
CLTV delta used in the last attempt.

When we calculate the `final_cltv_expiry_delta` to put in the
`RouteParameters` returned via events after a payment failure, we
should re-use the new one in the `PaymentParameters`, rather than
the one that was in the route itself.

lightning/src/ln/channelmanager.rs
lightning/src/ln/outbound_payment.rs

index 64c3ee6f671cc362561ffb44ad960161a74bd14a..8620c3cc15fa7be8e4611d9102cc59770550f277 100644 (file)
@@ -245,6 +245,10 @@ pub(crate) enum HTLCSource {
                first_hop_htlc_msat: u64,
                payment_id: PaymentId,
                payment_secret: Option<PaymentSecret>,
+               /// Note that this is now "deprecated" - we write it for forwards (and read it for
+               /// backwards) compatibility reasons, but prefer to use the data in the
+               /// [`super::outbound_payment`] module, which stores per-payment data once instead of in
+               /// each HTLC.
                payment_params: Option<PaymentParameters>,
        },
 }
index c91829393e490e988ec4a26aefdbf55b246ad4fc..a8acb9e80ba2796bca53288006cf02ecca59e76c 100644 (file)
@@ -789,7 +789,9 @@ impl OutboundPayments {
                                                Some(RouteParameters {
                                                        payment_params: payment_params.clone(),
                                                        final_value_msat: pending_amt_unsent,
-                                                       final_cltv_expiry_delta: max_unsent_cltv_delta,
+                                                       final_cltv_expiry_delta:
+                                                               if let Some(delta) = payment_params.final_cltv_expiry_delta { delta }
+                                                               else { max_unsent_cltv_delta },
                                                })
                                        } else { None }
                                } else { None },
@@ -987,7 +989,9 @@ impl OutboundPayments {
                        Some(RouteParameters {
                                payment_params: payment_params_data.clone(),
                                final_value_msat: path_last_hop.fee_msat,
-                               final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta,
+                               final_cltv_expiry_delta:
+                                       if let Some(delta) = payment_params_data.final_cltv_expiry_delta { delta }
+                                       else { path_last_hop.cltv_expiry_delta },
                        })
                } else { None };
                log_trace!(logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0));