Remove PaymentPathFailed::retry
authorValentine Wallace <vwallace@protonmail.com>
Wed, 1 Mar 2023 16:46:15 +0000 (11:46 -0500)
committerValentine Wallace <vwallace@protonmail.com>
Mon, 13 Mar 2023 15:59:03 +0000 (11:59 -0400)
We now support automatic retries in ChannelManager and no longer support manual
retries, so the field is useless.

lightning-background-processor/src/lib.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/outbound_payment.rs
lightning/src/util/events.rs
pending_changelog/2063.txt [new file with mode: 0644]

index f64f1e7651effd4f0ddf229c23b89ee40ea3bb7c..e407c4fdb82b27ff5d74f24812e35884ba21c014 100644 (file)
@@ -1369,7 +1369,6 @@ mod tests {
                        failure: PathFailure::OnPath { network_update: None },
                        path: path.clone(),
                        short_channel_id: Some(scored_scid),
-                       retry: None,
                });
                let event = receiver
                        .recv_timeout(Duration::from_secs(EVENT_DEADLINE))
@@ -1389,7 +1388,6 @@ mod tests {
                        failure: PathFailure::OnPath { network_update: None },
                        path: path.clone(),
                        short_channel_id: None,
-                       retry: None,
                });
                let event = receiver
                        .recv_timeout(Duration::from_secs(EVENT_DEADLINE))
index 6e0cd1709809d5d15c03f3cc036904c1003c956c..478e52da471f5c572b119d5970b7574d8c53a70d 100644 (file)
@@ -3834,9 +3834,9 @@ where
                // from block_connected which may run during initialization prior to the chain_monitor
                // being fully configured. See the docs for `ChannelManagerReadArgs` for more.
                match source {
-                       HTLCSource::OutboundRoute { ref path, ref session_priv, ref payment_id, ref payment_params, .. } => {
+                       HTLCSource::OutboundRoute { ref path, ref session_priv, ref payment_id, .. } => {
                                if self.pending_outbound_payments.fail_htlc(source, payment_hash, onion_error, path,
-                                       session_priv, payment_id, payment_params, self.probing_cookie_secret, &self.secp_ctx,
+                                       session_priv, payment_id, self.probing_cookie_secret, &self.secp_ctx,
                                        &self.pending_events, &self.logger)
                                { self.push_pending_forwards_ev(); }
                        },
index 1e99a475fe9f1501dd5042c8b4d344e55a7aa5f8..c8d3beb3cba8e3d899e96fc2717c8d4e7893660d 100644 (file)
@@ -1868,20 +1868,13 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
 ) {
        if conditions.expected_mpp_parts_remain { assert_eq!(payment_failed_events.len(), 1); } else { assert_eq!(payment_failed_events.len(), 2); }
        let expected_payment_id = match &payment_failed_events[0] {
-               Event::PaymentPathFailed { payment_hash, payment_failed_permanently, path, retry, payment_id, failure, short_channel_id,
+               Event::PaymentPathFailed { payment_hash, payment_failed_permanently, payment_id, failure,
                        #[cfg(test)]
                        error_code,
                        #[cfg(test)]
                        error_data, .. } => {
                        assert_eq!(*payment_hash, expected_payment_hash, "unexpected payment_hash");
                        assert_eq!(*payment_failed_permanently, expected_payment_failed_permanently, "unexpected payment_failed_permanently value");
-                       assert!(retry.is_some(), "expected retry.is_some()");
-                       assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path");
-                       assert_eq!(retry.as_ref().unwrap().payment_params.payee_pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
-                       if let Some(scid) = short_channel_id {
-                               assert!(retry.as_ref().unwrap().payment_params.previously_failed_channels.contains(&scid));
-                       }
-
                        #[cfg(test)]
                        {
                                assert!(error_code.is_some(), "expected error_code.is_some() = true");
index fdf9cdd0e252b5af46c69dc2908e4bd0ecc9b20d..544d3e6c993d11252702b9f7b1a85903772066b5 100644 (file)
@@ -97,14 +97,6 @@ impl PendingOutboundPayment {
                        _ => false,
                }
        }
-       fn payment_parameters(&mut self) -> Option<&mut PaymentParameters> {
-               match self {
-                       PendingOutboundPayment::Retryable { payment_params: Some(ref mut params), .. } => {
-                               Some(params)
-                       },
-                       _ => None,
-               }
-       }
        pub fn insert_previously_failed_scid(&mut self, scid: u64) {
                if let PendingOutboundPayment::Retryable { payment_params: Some(params), .. } = self {
                        params.previously_failed_channels.push(scid);
@@ -798,7 +790,6 @@ impl OutboundPayments {
                                        failure: events::PathFailure::InitialSend { err: e },
                                        path,
                                        short_channel_id: failed_scid,
-                                       retry: None,
                                        #[cfg(test)]
                                        error_code: None,
                                        #[cfg(test)]
@@ -1128,8 +1119,8 @@ impl OutboundPayments {
        pub(super) fn fail_htlc<L: Deref>(
                &self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason,
                path: &Vec<RouteHop>, session_priv: &SecretKey, payment_id: &PaymentId,
-               payment_params: &Option<PaymentParameters>, probing_cookie_secret: [u8; 32],
-               secp_ctx: &Secp256k1<secp256k1::All>, pending_events: &Mutex<Vec<events::Event>>, logger: &L
+               probing_cookie_secret: [u8; 32], secp_ctx: &Secp256k1<secp256k1::All>,
+               pending_events: &Mutex<Vec<events::Event>>, logger: &L
        ) -> bool where L::Target: Logger {
                #[cfg(test)]
                let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_error.decode_onion_failure(secp_ctx, logger, &source);
@@ -1157,7 +1148,6 @@ impl OutboundPayments {
 
                let mut full_failure_ev = None;
                let mut pending_retry_ev = false;
-               let mut retry = None;
                let attempts_remaining = if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) {
                        if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
                                log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@@ -1169,27 +1159,13 @@ impl OutboundPayments {
                        }
                        let mut is_retryable_now = payment.get().is_auto_retryable_now();
                        if let Some(scid) = short_channel_id {
+                               // TODO: If we decided to blame ourselves (or one of our channels) in
+                               // process_onion_failure we should close that channel as it implies our
+                               // next-hop is needlessly blaming us!
                                payment.get_mut().insert_previously_failed_scid(scid);
                        }
 
-                       // We want to move towards only using the `PaymentParameters` in the outbound payments
-                       // map. However, for backwards-compatibility, we still need to support passing the
-                       // `PaymentParameters` data that was shoved in the HTLC (and given to us via
-                       // `payment_params`) back to the user.
-                       let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
-                       if let Some(params) = payment.get_mut().payment_parameters() {
-                               retry = Some(RouteParameters {
-                                       payment_params: params.clone(),
-                                       final_value_msat: path_last_hop.fee_msat,
-                               });
-                       } else if let Some(params) = payment_params {
-                               retry = Some(RouteParameters {
-                                       payment_params: params.clone(),
-                                       final_value_msat: path_last_hop.fee_msat,
-                               });
-                       }
-
-                       if payment_is_probe || !is_retryable_now || !payment_retryable || retry.is_none() {
+                       if payment_is_probe || !is_retryable_now || !payment_retryable {
                                let _ = payment.get_mut().mark_abandoned(); // we'll only Err if it's a legacy payment
                                is_retryable_now = false;
                        }
@@ -1229,12 +1205,6 @@ impl OutboundPayments {
                                        }
                                }
                        } else {
-                               // TODO: If we decided to blame ourselves (or one of our channels) in
-                               // process_onion_failure we should close that channel as it implies our
-                               // next-hop is needlessly blaming us!
-                               if let Some(scid) = short_channel_id {
-                                       retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid));
-                               }
                                // If we miss abandoning the payment above, we *must* generate an event here or else the
                                // payment will sit in our outbounds forever.
                                if attempts_remaining && !already_awaiting_retry {
@@ -1248,7 +1218,6 @@ impl OutboundPayments {
                                        failure: events::PathFailure::OnPath { network_update },
                                        path: path.clone(),
                                        short_channel_id,
-                                       retry,
                                        #[cfg(test)]
                                        error_code: onion_error_code,
                                        #[cfg(test)]
index 5bc8a5084f2924706bf812aad46f4c750f851bc0..8c981fd1c482db6b9c75323b639735e87d4bcac6 100644 (file)
@@ -701,10 +701,6 @@ pub enum Event {
                /// If this is `Some`, then the corresponding channel should be avoided when the payment is
                /// retried. May be `None` for older [`Event`] serializations.
                short_channel_id: Option<u64>,
-               /// Parameters used by LDK to compute a new [`Route`] when retrying the failed payment path.
-               ///
-               /// [`Route`]: crate::routing::router::Route
-               retry: Option<RouteParameters>,
 #[cfg(test)]
                error_code: Option<u16>,
 #[cfg(test)]
@@ -992,7 +988,7 @@ impl Writeable for Event {
                        },
                        &Event::PaymentPathFailed {
                                ref payment_id, ref payment_hash, ref payment_failed_permanently, ref failure,
-                               ref path, ref short_channel_id, ref retry,
+                               ref path, ref short_channel_id,
                                #[cfg(test)]
                                ref error_code,
                                #[cfg(test)]
@@ -1010,7 +1006,7 @@ impl Writeable for Event {
                                        (3, false, required), // all_paths_failed in LDK versions prior to 0.0.114
                                        (5, *path, vec_type),
                                        (7, short_channel_id, option),
-                                       (9, retry, option),
+                                       (9, None::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
                                        (11, payment_id, option),
                                        (13, failure, required),
                                });
@@ -1227,7 +1223,6 @@ impl MaybeReadable for Event {
                                        let mut network_update = None;
                                        let mut path: Option<Vec<RouteHop>> = Some(vec![]);
                                        let mut short_channel_id = None;
-                                       let mut retry = None;
                                        let mut payment_id = None;
                                        let mut failure_opt = None;
                                        read_tlv_fields!(reader, {
@@ -1236,7 +1231,6 @@ impl MaybeReadable for Event {
                                                (2, payment_failed_permanently, required),
                                                (5, path, vec_type),
                                                (7, short_channel_id, option),
-                                               (9, retry, option),
                                                (11, payment_id, option),
                                                (13, failure_opt, upgradable_option),
                                        });
@@ -1248,7 +1242,6 @@ impl MaybeReadable for Event {
                                                failure,
                                                path: path.unwrap(),
                                                short_channel_id,
-                                               retry,
                                                #[cfg(test)]
                                                error_code,
                                                #[cfg(test)]
diff --git a/pending_changelog/2063.txt b/pending_changelog/2063.txt
new file mode 100644 (file)
index 0000000..8ac52a4
--- /dev/null
@@ -0,0 +1,4 @@
+## API Updates
+
+- `Event::PaymentPathFailed::retry` will always be `None` if we initiate a payment on 0.0.115
+       then downgrade to an earlier version (#2063)