PaymentPathFailed: add initial send error details

lightning-background-processor/src/lib.rs

@@ -1366,7 +1366,6 @@ mod tests {
 			payment_hash: PaymentHash([42; 32]),
 			payment_failed_permanently: false,
 			network_update: None,
-			all_paths_failed: true,
 			path: path.clone(),
 			short_channel_id: Some(scored_scid),
 			retry: None,
@@ -1387,7 +1386,6 @@ mod tests {
 			payment_hash: PaymentHash([42; 32]),
 			payment_failed_permanently: true,
 			network_update: None,
-			all_paths_failed: true,
 			path: path.clone(),
 			short_channel_id: None,
 			retry: None,

lightning/src/ln/functional_test_utils.rs

@@ -2240,10 +2240,9 @@ pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
 			if i == expected_paths.len() - 1 { assert_eq!(events.len(), 2); } else { assert_eq!(events.len(), 1); }
 
 			let expected_payment_id = match events[0] {
-				Event::PaymentPathFailed { payment_hash, payment_failed_permanently, all_paths_failed, ref path, ref payment_id, .. } => {
+				Event::PaymentPathFailed { payment_hash, payment_failed_permanently, ref path, ref payment_id, .. } => {
 					assert_eq!(payment_hash, our_payment_hash);
 					assert!(payment_failed_permanently);
-					assert_eq!(all_paths_failed, i == expected_paths.len() - 1);
 					for (idx, hop) in expected_route.iter().enumerate() {
 						assert_eq!(hop.node.get_our_node_id(), path[idx].pubkey);
 					}

lightning/src/ln/functional_tests.rs

@@ -5695,11 +5695,10 @@ fn test_fail_holding_cell_htlc_upon_free() {
 	let events = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 2);
 	match &events[0] {
-		&Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, .. } => {
+		&Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref short_channel_id, .. } => {
 			assert_eq!(PaymentId(our_payment_hash.0), *payment_id.as_ref().unwrap());
 			assert_eq!(our_payment_hash.clone(), *payment_hash);
 			assert_eq!(*payment_failed_permanently, false);
-			assert_eq!(*all_paths_failed, true);
 			assert_eq!(*network_update, None);
 			assert_eq!(*short_channel_id, Some(route.paths[0][0].short_channel_id));
 		},
@@ -5786,11 +5785,10 @@ fn test_free_and_fail_holding_cell_htlcs() {
 	let events = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 2);
 	match &events[0] {
-		&Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, .. } => {
+		&Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref short_channel_id, .. } => {
 			assert_eq!(payment_id_2, *payment_id.as_ref().unwrap());
 			assert_eq!(payment_hash_2.clone(), *payment_hash);
 			assert_eq!(*payment_failed_permanently, false);
-			assert_eq!(*all_paths_failed, true);
 			assert_eq!(*network_update, None);
 			assert_eq!(*short_channel_id, Some(route_2.paths[0][0].short_channel_id));
 		},

lightning/src/ln/onion_route_tests.rs

@@ -167,9 +167,8 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
 
 	let events = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 2);
-	if let &Event::PaymentPathFailed { ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, .. } = &events[0] {
+	if let &Event::PaymentPathFailed { ref payment_failed_permanently, ref network_update, ref short_channel_id, ref error_code, .. } = &events[0] {
 		assert_eq!(*payment_failed_permanently, !expected_retryable);
-		assert_eq!(*all_paths_failed, true);
 		assert_eq!(*error_code, expected_error_code);
 		if expected_channel_update.is_some() {
 			match network_update {

lightning/src/ln/outbound_payment.rs

@@ -17,7 +17,6 @@ use crate::chain::keysinterface::{EntropySource, NodeSigner, Recipient};
 use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
 use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId};
 use crate::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA as LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA;
-use crate::ln::msgs::DecodeError;
 use crate::ln::onion_utils::HTLCFailReason;
 use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router};
 use crate::util::errors::APIError;
@@ -795,7 +794,6 @@ impl OutboundPayments {
 					payment_hash,
 					payment_failed_permanently: false,
 					network_update: None,
-					all_paths_failed: false,
 					path,
 					short_channel_id: failed_scid,
 					retry: None,
@@ -1158,7 +1156,6 @@ impl OutboundPayments {
 			awaiting_retry
 		});
 
-		let mut all_paths_failed = false;
 		let mut full_failure_ev = None;
 		let mut pending_retry_ev = false;
 		let mut retry = None;
@@ -1207,7 +1204,6 @@ impl OutboundPayments {
 				is_retryable_now = false;
 			}
 			if payment.get().remaining_parts() == 0 {
-				all_paths_failed = true;
 				if payment.get().abandoned() {
 					if !payment_is_probe {
 						full_failure_ev = Some(events::Event::PaymentFailed {
@@ -1260,7 +1256,6 @@ impl OutboundPayments {
 					payment_hash: payment_hash.clone(),
 					payment_failed_permanently: !payment_retryable,
 					network_update,
-					all_paths_failed,
 					path: path.clone(),
 					short_channel_id,
 					retry,

lightning/src/ln/payment_tests.rs

@@ -2139,7 +2139,7 @@ fn retry_multi_path_single_failed_payment() {
 	assert_eq!(events.len(), 1);
 	match events[0] {
 		Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently: false,
-			network_update: None, all_paths_failed: false, short_channel_id: Some(expected_scid), .. } => {
+			network_update: None, short_channel_id: Some(expected_scid), .. } => {
 			assert_eq!(payment_hash, ev_payment_hash);
 			assert_eq!(expected_scid, route.paths[1][0].short_channel_id);
 		},
@@ -2212,7 +2212,7 @@ fn immediate_retry_on_failure() {
 	assert_eq!(events.len(), 1);
 	match events[0] {
 		Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently: false,
-		network_update: None, all_paths_failed: false, short_channel_id: Some(expected_scid), .. } => {
+		network_update: None, short_channel_id: Some(expected_scid), .. } => {
 			assert_eq!(payment_hash, ev_payment_hash);
 			assert_eq!(expected_scid, route.paths[1][0].short_channel_id);
 		},
@@ -2226,7 +2226,8 @@ fn immediate_retry_on_failure() {
 #[test]
 fn no_extra_retries_on_back_to_back_fail() {
 	// In a previous release, we had a race where we may exceed the payment retry count if we
-	// get two failures in a row with the second having `all_paths_failed` set.
+	// get two failures in a row with the second indicating that all paths had failed (this field,
+	// `all_paths_failed`, has since been removed).
 	// Generally, when we give up trying to retry a payment, we don't know for sure what the
 	// current state of the ChannelManager event queue is. Specifically, we cannot be sure that
 	// there are not multiple additional `PaymentPathFailed` or even `PaymentSent` events

lightning/src/util/events.rs

@@ -21,7 +21,6 @@ use crate::ln::channelmanager::{InterceptId, PaymentId};
 use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS;
 use crate::ln::features::ChannelTypeFeatures;
 use crate::ln::msgs;
-use crate::ln::msgs::DecodeError;
 use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
 use crate::routing::gossip::NetworkUpdate;
 use crate::util::ser::{BigSize, FixedLengthReader, Writeable, Writer, MaybeReadable, Readable, WithoutLength, OptionDeserWrapper};
@@ -631,13 +630,12 @@ pub enum Event {
 	/// handle the HTLC.
 	///
 	/// Note that this does *not* indicate that all paths for an MPP payment have failed, see
-	/// [`Event::PaymentFailed`] and [`all_paths_failed`].
+	/// [`Event::PaymentFailed`].
 	///
 	/// See [`ChannelManager::abandon_payment`] for giving up on this payment before its retries have
 	/// been exhausted.
 	///
 	/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
-	/// [`all_paths_failed`]: Self::PaymentPathFailed::all_paths_failed
 	PaymentPathFailed {
 		/// The id returned by [`ChannelManager::send_payment`] and used with
 		/// [`ChannelManager::abandon_payment`].
@@ -661,10 +659,6 @@ pub enum Event {
 		///
 		/// [`NetworkGraph`]: crate::routing::gossip::NetworkGraph
 		network_update: Option<NetworkUpdate>,
-		/// For both single-path and multi-path payments, this is set if all paths of the payment have
-		/// failed. This will be set to false if (1) this is an MPP payment and (2) other parts of the
-		/// larger MPP payment were still in flight when this event was generated.
-		all_paths_failed: bool,
 		/// The payment path that failed.
 		path: Vec<RouteHop>,
 		/// The channel responsible for the failed payment path.
@@ -967,7 +961,7 @@ impl Writeable for Event {
 			},
 			&Event::PaymentPathFailed {
 				ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update,
-				ref all_paths_failed, ref path, ref short_channel_id, ref retry,
+				ref path, ref short_channel_id, ref retry,
 				#[cfg(test)]
 				ref error_code,
 				#[cfg(test)]
@@ -982,7 +976,7 @@ impl Writeable for Event {
 					(0, payment_hash, required),
 					(1, network_update, option),
 					(2, payment_failed_permanently, required),
-					(3, all_paths_failed, required),
+					(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),
@@ -1199,7 +1193,6 @@ impl MaybeReadable for Event {
 					let mut payment_hash = PaymentHash([0; 32]);
 					let mut payment_failed_permanently = false;
 					let mut network_update = None;
-					let mut all_paths_failed = Some(true);
 					let mut path: Option<Vec<RouteHop>> = Some(vec![]);
 					let mut short_channel_id = None;
 					let mut retry = None;
@@ -1208,7 +1201,6 @@ impl MaybeReadable for Event {
 						(0, payment_hash, required),
 						(1, network_update, ignorable),
 						(2, payment_failed_permanently, required),
-						(3, all_paths_failed, option),
 						(5, path, vec_type),
 						(7, short_channel_id, option),
 						(9, retry, option),
@@ -1219,7 +1211,6 @@ impl MaybeReadable for Event {
 						payment_hash,
 						payment_failed_permanently,
 						network_update,
-						all_paths_failed: all_paths_failed.unwrap(),
 						path: path.unwrap(),
 						short_channel_id,
 						retry,

lightning/src/util/ser_macros.rs

@@ -949,7 +949,7 @@ macro_rules! impl_writeable_tlv_based_enum_upgradable {
 						Ok(Some($st::$tuple_variant_name(Readable::read(reader)?)))
 					}),*)*
 					_ if id % 2 == 1 => Ok(None),
-					_ => Err(DecodeError::UnknownRequiredFeature),
+					_ => Err($crate::ln::msgs::DecodeError::UnknownRequiredFeature),
 				}
 			}
 		}

pending_changelog/2043.txt

@@ -0,0 +1,11 @@
+## API Updates
+- `Event::PaymentPathFailed::all_paths_failed` has been removed, as we've dropped support for manual
+	payment retries.
+
+## Backwards Compatibility
+- If downgrading from 0.0.114 to a previous version, `Event::PaymentPathFailed::all_paths_failed`
+	will always be set to `false`. Users who wish to support downgrading and currently rely on the
+	field should should first migrate to always calling `ChannelManager::abandon_payment` and awaiting
+	`PaymentFailed` events before retrying (see the field docs for more info on this approach:
+	<https://docs.rs/lightning/0.0.113/lightning/util/events/enum.Event.html#variant.PaymentPathFailed.field.all_paths_failed>),
+	and then migrate to 0.0.114.