From: Valentine Wallace <vwallace@protonmail.com>
Date: Fri, 3 Feb 2023 17:53:01 +0000 (-0500)
Subject: Abandon payment on behalf of the user on payment path failure
X-Git-Tag: v0.0.114-beta~17^2~3
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=82e0880442a4c7e2fc9a49f1e3fb20740957ae2b;p=rust-lightning

Abandon payment on behalf of the user on payment path failure

Removed retry_single_path_payment, it's replaced by automatic_retries with
AutoRetry::Success
---

diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs
index a088f64f..39887e83 100644
--- a/fuzz/src/chanmon_consistency.rs
+++ b/fuzz/src/chanmon_consistency.rs
@@ -914,6 +914,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 						events::Event::PaymentClaimed { .. } => {},
 						events::Event::PaymentPathSuccessful { .. } => {},
 						events::Event::PaymentPathFailed { .. } => {},
+						events::Event::PaymentFailed { .. } => {},
 						events::Event::ProbeSuccessful { .. } | events::Event::ProbeFailed { .. } => {
 							// Even though we don't explicitly send probes, because probes are
 							// detected based on hashing the payment hash+preimage, its rather
diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs
index e1ea256a..fdb66cc4 100644
--- a/lightning/src/ln/chanmon_update_fail_tests.rs
+++ b/lightning/src/ln/chanmon_update_fail_tests.rs
@@ -1752,12 +1752,18 @@ fn test_monitor_update_on_pending_forwards() {
 	commitment_signed_dance!(nodes[0], nodes[1], bs_updates.commitment_signed, false, true);
 
 	let events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 2);
+	assert_eq!(events.len(), 3);
 	if let Event::PaymentPathFailed { payment_hash, payment_failed_permanently, .. } = events[0] {
 		assert_eq!(payment_hash, payment_hash_1);
 		assert!(payment_failed_permanently);
 	} else { panic!("Unexpected event!"); }
 	match events[1] {
+		Event::PaymentFailed { payment_hash, .. } => {
+			assert_eq!(payment_hash, payment_hash_1);
+		},
+		_ => panic!("Unexpected event"),
+	}
+	match events[2] {
 		Event::PendingHTLCsForwardable { .. } => { },
 		_ => panic!("Unexpected event"),
 	};
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 9f888848..f9a2424e 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1800,17 +1800,18 @@ macro_rules! expect_payment_failed {
 }
 
 pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
-	node: &'a Node<'b, 'c, 'd>, payment_failed_event: Event, expected_payment_hash: PaymentHash,
+	payment_failed_events: Vec<Event>, expected_payment_hash: PaymentHash,
 	expected_payment_failed_permanently: bool, conditions: PaymentFailedConditions<'e>
 ) {
-	let expected_payment_id = match payment_failed_event {
+	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, network_update, short_channel_id,
 			#[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_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");
@@ -1839,7 +1840,7 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
 					},
 					Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }) if chan_closed => {
 						if let Some(scid) = conditions.expected_blamed_scid {
-							assert_eq!(short_channel_id, scid);
+							assert_eq!(*short_channel_id, scid);
 						}
 						assert!(is_permanent);
 					},
@@ -1853,10 +1854,7 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
 		_ => panic!("Unexpected event"),
 	};
 	if !conditions.expected_mpp_parts_remain {
-		node.node.abandon_payment(expected_payment_id);
-		let events = node.node.get_and_clear_pending_events();
-		assert_eq!(events.len(), 1);
-		match events[0] {
+		match &payment_failed_events[1] {
 			Event::PaymentFailed { ref payment_hash, ref payment_id } => {
 				assert_eq!(*payment_hash, expected_payment_hash, "unexpected second payment_hash");
 				assert_eq!(*payment_id, expected_payment_id);
@@ -1870,9 +1868,8 @@ pub fn expect_payment_failed_conditions<'a, 'b, 'c, 'd, 'e>(
 	node: &'a Node<'b, 'c, 'd>, expected_payment_hash: PaymentHash, expected_payment_failed_permanently: bool,
 	conditions: PaymentFailedConditions<'e>
 ) {
-	let mut events = node.node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	expect_payment_failed_conditions_event(node, events.pop().unwrap(), expected_payment_hash, expected_payment_failed_permanently, conditions);
+	let events = node.node.get_and_clear_pending_events();
+	expect_payment_failed_conditions_event(events, expected_payment_hash, expected_payment_failed_permanently, conditions);
 }
 
 pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_paths: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) -> PaymentId {
@@ -2157,22 +2154,6 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
 }
 
 pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths_slice: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_hash: PaymentHash) {
-	let expected_payment_id = pass_failed_payment_back_no_abandon(origin_node, expected_paths_slice, skip_last, our_payment_hash);
-	if !skip_last {
-		origin_node.node.abandon_payment(expected_payment_id.unwrap());
-		let events = origin_node.node.get_and_clear_pending_events();
-		assert_eq!(events.len(), 1);
-		match events[0] {
-			Event::PaymentFailed { ref payment_hash, ref payment_id } => {
-				assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
-				assert_eq!(*payment_id, expected_payment_id.unwrap());
-			}
-			_ => panic!("Unexpected second event"),
-		}
-	}
-}
-
-pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths_slice: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_hash: PaymentHash) -> Option<PaymentId> {
 	let mut expected_paths: Vec<_> = expected_paths_slice.iter().collect();
 	check_added_monitors!(expected_paths[0].last().unwrap(), expected_paths.len());
 
@@ -2196,8 +2177,6 @@ pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b
 	per_path_msgs.sort_unstable_by(|(_, node_id_a), (_, node_id_b)| node_id_a.cmp(node_id_b));
 	expected_paths.sort_unstable_by(|path_a, path_b| path_a[path_a.len() - 2].node.get_our_node_id().cmp(&path_b[path_b.len() - 2].node.get_our_node_id()));
 
-	let mut expected_payment_id = None;
-
 	for (i, (expected_route, (path_msgs, next_hop))) in expected_paths.iter().zip(per_path_msgs.drain(..)).enumerate() {
 		let mut next_msgs = Some(path_msgs);
 		let mut expected_next_node = next_hop;
@@ -2245,8 +2224,9 @@ pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b
 			assert!(origin_node.node.get_and_clear_pending_msg_events().is_empty());
 			commitment_signed_dance!(origin_node, prev_node, next_msgs.as_ref().unwrap().1, false);
 			let events = origin_node.node.get_and_clear_pending_events();
-			assert_eq!(events.len(), 1);
-			expected_payment_id = Some(match events[0] {
+			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, .. } => {
 					assert_eq!(payment_hash, our_payment_hash);
 					assert!(payment_failed_permanently);
@@ -2257,7 +2237,16 @@ pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b
 					payment_id.unwrap()
 				},
 				_ => panic!("Unexpected event"),
-			});
+			};
+			if i == expected_paths.len() - 1 {
+				match events[1] {
+					Event::PaymentFailed { ref payment_hash, ref payment_id } => {
+						assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
+						assert_eq!(*payment_id, expected_payment_id);
+					}
+					_ => panic!("Unexpected second event"),
+				}
+			}
 		}
 	}
 
@@ -2266,8 +2255,6 @@ pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b
 	assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_events().is_empty());
 	assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_msg_events().is_empty());
 	check_added_monitors!(expected_paths[0].last().unwrap(), 0);
-
-	expected_payment_id
 }
 
 pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], our_payment_hash: PaymentHash)  {
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 9055d4e7..3da49ca7 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -3170,7 +3170,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
 	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 
 	let events = nodes[1].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), if deliver_bs_raa { 2 + nodes.len() - 1 } else { 3 + nodes.len() });
+	assert_eq!(events.len(), if deliver_bs_raa { 3 + nodes.len() - 1 } else { 4 + nodes.len() });
 	match events[0] {
 		Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } => { },
 		_ => panic!("Unexepected event"),
@@ -3181,20 +3181,11 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
 		},
 		_ => panic!("Unexpected event"),
 	}
-	if !deliver_bs_raa {
-		match events[2] {
-			Event::PendingHTLCsForwardable { .. } => { },
-			_ => panic!("Unexpected event"),
-		};
-		nodes[1].node.abandon_payment(PaymentId(fourth_payment_hash.0));
-		let payment_failed_events = nodes[1].node.get_and_clear_pending_events();
-		assert_eq!(payment_failed_events.len(), 1);
-		match payment_failed_events[0] {
-			Event::PaymentFailed { ref payment_hash, .. } => {
-				assert_eq!(*payment_hash, fourth_payment_hash);
-			},
-			_ => panic!("Unexpected event"),
-		}
+	match events[2] {
+		Event::PaymentFailed { ref payment_hash, .. } => {
+			assert_eq!(*payment_hash, fourth_payment_hash);
+		},
+		_ => panic!("Unexpected event"),
 	}
 
 	nodes[1].node.process_pending_htlc_forwards();
@@ -3242,7 +3233,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
 			commitment_signed_dance!(nodes[0], nodes[1], commitment_signed, false, true);
 
 			let events = nodes[0].node.get_and_clear_pending_events();
-			assert_eq!(events.len(), 3);
+			assert_eq!(events.len(), 6);
 			match events[0] {
 				Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => {
 					assert!(failed_htlcs.insert(payment_hash.0));
@@ -3255,19 +3246,37 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
 				_ => panic!("Unexpected event"),
 			}
 			match events[1] {
+				Event::PaymentFailed { ref payment_hash, .. } => {
+					assert_eq!(*payment_hash, first_payment_hash);
+				},
+				_ => panic!("Unexpected event"),
+			}
+			match events[2] {
 				Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => {
 					assert!(failed_htlcs.insert(payment_hash.0));
 					assert!(network_update.is_some());
 				},
 				_ => panic!("Unexpected event"),
 			}
-			match events[2] {
+			match events[3] {
+				Event::PaymentFailed { ref payment_hash, .. } => {
+					assert_eq!(*payment_hash, second_payment_hash);
+				},
+				_ => panic!("Unexpected event"),
+			}
+			match events[4] {
 				Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => {
 					assert!(failed_htlcs.insert(payment_hash.0));
 					assert!(network_update.is_some());
 				},
 				_ => panic!("Unexpected event"),
 			}
+			match events[5] {
+				Event::PaymentFailed { ref payment_hash, .. } => {
+					assert_eq!(*payment_hash, third_payment_hash);
+				},
+				_ => panic!("Unexpected event"),
+			}
 		},
 		_ => panic!("Unexpected event"),
 	}
@@ -3354,7 +3363,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() {
 		nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &update_add_htlc);
 	}
 	let events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 2);
+	assert_eq!(events.len(), 3);
 	// Check that Alice fails backward the pending HTLC from the second payment.
 	match events[0] {
 		Event::PaymentPathFailed { payment_hash, .. } => {
@@ -3363,6 +3372,12 @@ fn fail_backward_pending_htlc_upon_channel_failure() {
 		_ => panic!("Unexpected event"),
 	}
 	match events[1] {
+		Event::PaymentFailed { payment_hash, .. } => {
+			assert_eq!(payment_hash, failed_payment_hash);
+		},
+		_ => panic!("Unexpected event"),
+	}
+	match events[2] {
 		Event::ChannelClosed { reason: ClosureReason::ProcessingError { ref err }, .. } => {
 			assert_eq!(err, "Remote side tried to send a 0-msat HTLC");
 		},
@@ -3594,7 +3609,7 @@ fn test_simple_peer_disconnect() {
 	reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (1, 0), (1, 0), (false, false));
 	{
 		let events = nodes[0].node.get_and_clear_pending_events();
-		assert_eq!(events.len(), 3);
+		assert_eq!(events.len(), 4);
 		match events[0] {
 			Event::PaymentSent { payment_preimage, payment_hash, .. } => {
 				assert_eq!(payment_preimage, payment_preimage_3);
@@ -3610,6 +3625,12 @@ fn test_simple_peer_disconnect() {
 			_ => panic!("Unexpected event"),
 		}
 		match events[2] {
+			Event::PaymentFailed { payment_hash, .. } => {
+				assert_eq!(payment_hash, payment_hash_5);
+			},
+			_ => panic!("Unexpected event"),
+		}
+		match events[3] {
 			Event::PaymentPathSuccessful { .. } => {},
 			_ => panic!("Unexpected event"),
 		}
@@ -5123,7 +5144,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
 	}
 
 	let as_events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(as_events.len(), if announce_latest { 5 } else { 3 });
+	assert_eq!(as_events.len(), if announce_latest { 10 } else { 6 });
 	let mut as_failds = HashSet::new();
 	let mut as_updates = 0;
 	for event in as_events.iter() {
@@ -5137,6 +5158,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
 			if network_update.is_some() {
 				as_updates += 1;
 			}
+		} else if let &Event::PaymentFailed { .. } = event {
 		} else { panic!("Unexpected event"); }
 	}
 	assert!(as_failds.contains(&payment_hash_1));
@@ -5148,7 +5170,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
 	assert!(as_failds.contains(&payment_hash_6));
 
 	let bs_events = nodes[1].node.get_and_clear_pending_events();
-	assert_eq!(bs_events.len(), if announce_latest { 4 } else { 3 });
+	assert_eq!(bs_events.len(), if announce_latest { 8 } else { 6 });
 	let mut bs_failds = HashSet::new();
 	let mut bs_updates = 0;
 	for event in bs_events.iter() {
@@ -5162,6 +5184,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
 			if network_update.is_some() {
 				bs_updates += 1;
 			}
+		} else if let &Event::PaymentFailed { .. } = event {
 		} else { panic!("Unexpected event"); }
 	}
 	assert!(bs_failds.contains(&payment_hash_1));
@@ -5670,7 +5693,7 @@ fn test_fail_holding_cell_htlc_upon_free() {
 
 	// Check that the payment failed to be sent out.
 	let events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
+	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, .. } => {
 			assert_eq!(PaymentId(our_payment_hash.0), *payment_id.as_ref().unwrap());
@@ -5682,6 +5705,12 @@ fn test_fail_holding_cell_htlc_upon_free() {
 		},
 		_ => panic!("Unexpected event"),
 	}
+	match &events[1] {
+		&Event::PaymentFailed { ref payment_hash, .. } => {
+			assert_eq!(our_payment_hash.clone(), *payment_hash);
+		},
+		_ => panic!("Unexpected event"),
+	}
 }
 
 // Test that if multiple HTLCs are released from the holding cell and one is
@@ -5755,7 +5784,7 @@ fn test_free_and_fail_holding_cell_htlcs() {
 
 	// Check that the second payment failed to be sent out.
 	let events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
+	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, .. } => {
 			assert_eq!(payment_id_2, *payment_id.as_ref().unwrap());
@@ -5767,6 +5796,12 @@ fn test_free_and_fail_holding_cell_htlcs() {
 		},
 		_ => panic!("Unexpected event"),
 	}
+	match &events[1] {
+		&Event::PaymentFailed { ref payment_hash, .. } => {
+			assert_eq!(payment_hash_2.clone(), *payment_hash);
+		},
+		_ => panic!("Unexpected event"),
+	}
 
 	// Complete the first payment and the RAA from the fee update.
 	let (payment_event, send_raa_event) = {
@@ -6649,7 +6684,7 @@ fn test_channel_failed_after_message_with_badonion_node_perm_bits_set() {
 	}
 
 	let events_5 = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events_5.len(), 1);
+	assert_eq!(events_5.len(), 2);
 
 	// Expect a PaymentPathFailed event with a ChannelFailure network update for the channel between
 	// the node originating the error to its next hop.
@@ -6663,6 +6698,12 @@ fn test_channel_failed_after_message_with_badonion_node_perm_bits_set() {
 		},
 		_ => panic!("Unexpected event"),
 	}
+	match events_5[1] {
+		Event::PaymentFailed { payment_hash, .. } => {
+			assert_eq!(payment_hash, our_payment_hash);
+		},
+		_ => panic!("Unexpected event"),
+	}
 
 	// TODO: Test actual removal of channel from NetworkGraph when it's implemented.
 }
@@ -6734,7 +6775,7 @@ fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) {
 	connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
 	let events = nodes[0].node.get_and_clear_pending_events();
 	// Only 2 PaymentPathFailed events should show up, over-dust HTLC has to be failed by timeout tx
-	assert_eq!(events.len(), 2);
+	assert_eq!(events.len(), 4);
 	let mut first_failed = false;
 	for event in events {
 		match event {
@@ -6745,7 +6786,8 @@ fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) {
 				} else {
 					assert_eq!(payment_hash, payment_hash_2);
 				}
-			}
+			},
+			Event::PaymentFailed { .. } => {}
 			_ => panic!("Unexpected event"),
 		}
 	}
@@ -9034,9 +9076,11 @@ fn do_test_dup_htlc_second_rejected(test_for_second_fail_panic: bool) {
 		commitment_signed_dance!(nodes[0], nodes[1], fail_updates_1.commitment_signed, false);
 
 		let failure_events = nodes[0].node.get_and_clear_pending_events();
-		assert_eq!(failure_events.len(), 2);
+		assert_eq!(failure_events.len(), 4);
 		if let Event::PaymentPathFailed { .. } = failure_events[0] {} else { panic!(); }
-		if let Event::PaymentPathFailed { .. } = failure_events[1] {} else { panic!(); }
+		if let Event::PaymentFailed { .. } = failure_events[1] {} else { panic!(); }
+		if let Event::PaymentPathFailed { .. } = failure_events[2] {} else { panic!(); }
+		if let Event::PaymentFailed { .. } = failure_events[3] {} else { panic!(); }
 	} else {
 		// Let the second HTLC fail and claim the first
 		expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCDestination::FailedPayment { payment_hash: our_payment_hash }]);
@@ -9047,7 +9091,7 @@ fn do_test_dup_htlc_second_rejected(test_for_second_fail_panic: bool) {
 		nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]);
 		commitment_signed_dance!(nodes[0], nodes[1], fail_updates_1.commitment_signed, false);
 
-		expect_payment_failed_conditions(&nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain());
+		expect_payment_failed_conditions(&nodes[0], our_payment_hash, true, PaymentFailedConditions::new());
 
 		claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage);
 	}
@@ -9169,7 +9213,27 @@ fn test_inconsistent_mpp_params() {
 	assert_eq!(events.len(), 1);
 	pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), true, None);
 
-	claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage);
+	do_claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage);
+	let events = nodes[0].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 3);
+	match events[0] {
+		Event::PaymentSent { payment_hash, .. } => { // The payment was abandoned earlier, so the fee paid will be None
+			assert_eq!(payment_hash, our_payment_hash);
+		},
+		_ => panic!("Unexpected event")
+	}
+	match events[1] {
+		Event::PaymentPathSuccessful { payment_hash, .. } => {
+			assert_eq!(payment_hash.unwrap(), our_payment_hash);
+		},
+		_ => panic!("Unexpected event")
+	}
+	match events[2] {
+		Event::PaymentPathSuccessful { payment_hash, .. } => {
+			assert_eq!(payment_hash.unwrap(), our_payment_hash);
+		},
+		_ => panic!("Unexpected event")
+	}
 }
 
 #[test]
diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index 088cf866..c4435b47 100644
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -1241,11 +1241,10 @@ fn do_test_revoked_counterparty_commitment_balances(confirm_htlc_spend_first: bo
 	test_spendable_output(&nodes[1], &as_revoked_txn[0]);
 
 	let mut payment_failed_events = nodes[1].node.get_and_clear_pending_events();
-	expect_payment_failed_conditions_event(&nodes[1], payment_failed_events.pop().unwrap(),
-		dust_payment_hash, false, PaymentFailedConditions::new());
-	expect_payment_failed_conditions_event(&nodes[1], payment_failed_events.pop().unwrap(),
+	expect_payment_failed_conditions_event(payment_failed_events[..2].to_vec(),
 		missing_htlc_payment_hash, false, PaymentFailedConditions::new());
-	assert!(payment_failed_events.is_empty());
+	expect_payment_failed_conditions_event(payment_failed_events[2..].to_vec(),
+		dust_payment_hash, false, PaymentFailedConditions::new());
 
 	connect_blocks(&nodes[1], 1);
 	test_spendable_output(&nodes[1], &claim_txn[if confirm_htlc_spend_first { 2 } else { 3 }]);
diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs
index fbc8e1c9..bb69bbb3 100644
--- a/lightning/src/ln/onion_route_tests.rs
+++ b/lightning/src/ln/onion_route_tests.rs
@@ -166,7 +166,7 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
 	commitment_signed_dance!(nodes[0], nodes[1], update_1_0.commitment_signed, false, true);
 
 	let events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
+	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] {
 		assert_eq!(*payment_failed_permanently, !expected_retryable);
 		assert_eq!(*all_paths_failed, true);
@@ -212,10 +212,7 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
 	} else {
 		panic!("Unexpected event");
 	}
-	nodes[0].node.abandon_payment(payment_id);
-	let events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	match events[0] {
+	match events[1] {
 		Event::PaymentFailed { payment_hash: ev_payment_hash, payment_id: ev_payment_id } => {
 			assert_eq!(*payment_hash, ev_payment_hash);
 			assert_eq!(payment_id, ev_payment_id);
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index dd5cd892..55719c40 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -1009,6 +1009,7 @@ impl OutboundPayments {
 		#[cfg(not(test))]
 		let (network_update, short_channel_id, payment_retryable, _, _) = onion_error.decode_onion_failure(secp_ctx, logger, &source);
 
+		let payment_is_probe = payment_is_probe(payment_hash, &payment_id, probing_cookie_secret);
 		let mut session_priv_bytes = [0; 32];
 		session_priv_bytes.copy_from_slice(&session_priv[..]);
 		let mut outbounds = self.pending_outbound_payments.lock().unwrap();
@@ -1025,7 +1026,7 @@ impl OutboundPayments {
 				log_trace!(logger, "Received failure of HTLC with payment_hash {} after payment completion", log_bytes!(payment_hash.0));
 				return
 			}
-			let is_retryable_now = payment.get().is_auto_retryable_now();
+			let mut is_retryable_now = payment.get().is_auto_retryable_now();
 			if let Some(scid) = short_channel_id {
 				payment.get_mut().insert_previously_failed_scid(scid);
 			}
@@ -1056,6 +1057,10 @@ impl OutboundPayments {
 				});
 			}
 
+			if !payment_is_probe && (!is_retryable_now || !payment_retryable || retry.is_none()) {
+				let _ = payment.get_mut().mark_abandoned(); // we'll only Err if it's a legacy payment
+				is_retryable_now = false;
+			}
 			if payment.get().remaining_parts() == 0 {
 				all_paths_failed = true;
 				if payment.get().abandoned() {
@@ -1075,7 +1080,7 @@ impl OutboundPayments {
 		log_trace!(logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0));
 
 		let path_failure = {
-			if payment_is_probe(payment_hash, &payment_id, probing_cookie_secret) {
+			if payment_is_probe {
 				if !payment_retryable {
 					events::Event::ProbeSuccessful {
 						payment_id: *payment_id,
@@ -1097,7 +1102,9 @@ impl OutboundPayments {
 				if let Some(scid) = short_channel_id {
 					retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid));
 				}
-				if payment_retryable && attempts_remaining && retry.is_some() {
+				// 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 {
 					debug_assert!(full_failure_ev.is_none());
 					pending_retry_ev = Some(events::Event::PendingHTLCsForwardable {
 						time_forwardable: Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS),
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index fcd57d8f..f75cbd43 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -43,60 +43,6 @@ use {
 	std::time::{SystemTime, Duration}
 };
 
-#[test]
-fn retry_single_path_payment() {
-	let chanmon_cfgs = create_chanmon_cfgs(3);
-	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
-	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
-	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
-
-	let _chan_0 = create_announced_chan_between_nodes(&nodes, 0, 1);
-	let chan_1 = create_announced_chan_between_nodes(&nodes, 2, 1);
-	// Rebalance to find a route
-	send_payment(&nodes[2], &vec!(&nodes[1])[..], 3_000_000);
-
-	let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], 100_000);
-
-	// Rebalance so that the first hop fails.
-	send_payment(&nodes[1], &vec!(&nodes[2])[..], 2_000_000);
-
-	// Make sure the payment fails on the first hop.
-	let payment_id = PaymentId(payment_hash.0);
-	nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret), payment_id).unwrap();
-	check_added_monitors!(nodes[0], 1);
-	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
-	assert_eq!(events.len(), 1);
-	let mut payment_event = SendEvent::from_event(events.pop().unwrap());
-	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
-	check_added_monitors!(nodes[1], 0);
-	commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
-	expect_pending_htlcs_forwardable!(nodes[1]);
-	expect_pending_htlcs_forwardable_and_htlc_handling_failed!(&nodes[1], vec![HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: chan_1.2 }]);
-	let htlc_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
-	assert!(htlc_updates.update_add_htlcs.is_empty());
-	assert_eq!(htlc_updates.update_fail_htlcs.len(), 1);
-	assert!(htlc_updates.update_fulfill_htlcs.is_empty());
-	assert!(htlc_updates.update_fail_malformed_htlcs.is_empty());
-	check_added_monitors!(nodes[1], 1);
-	nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]);
-	commitment_signed_dance!(nodes[0], nodes[1], htlc_updates.commitment_signed, false);
-	expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
-
-	// Rebalance the channel so the retry succeeds.
-	send_payment(&nodes[2], &vec!(&nodes[1])[..], 3_000_000);
-
-	// Mine two blocks (we expire retries after 3, so this will check that we don't expire early)
-	connect_blocks(&nodes[0], 2);
-
-	// Retry the payment and make sure it succeeds.
-	nodes[0].node.retry_payment(&route, payment_id).unwrap();
-	check_added_monitors!(nodes[0], 1);
-	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
-	assert_eq!(events.len(), 1);
-	pass_along_path(&nodes[0], &[&nodes[1], &nodes[2]], 100_000, payment_hash, Some(payment_secret), events.pop().unwrap(), true, None);
-	claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], false, payment_preimage);
-}
-
 #[test]
 fn mpp_failure() {
 	let chanmon_cfgs = create_chanmon_cfgs(4);
@@ -136,7 +82,8 @@ fn mpp_retry() {
 	// Rebalance
 	send_payment(&nodes[3], &vec!(&nodes[2])[..], 1_500_000);
 
-	let (mut route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[3], 1_000_000);
+	let amt_msat = 1_000_000;
+	let (mut route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[3], amt_msat);
 	let path = route.paths[0].clone();
 	route.paths.push(path);
 	route.paths[0][0].pubkey = nodes[1].node.get_our_node_id();
@@ -148,7 +95,14 @@ fn mpp_retry() {
 
 	// Initiate the MPP payment.
 	let payment_id = PaymentId(payment_hash.0);
-	nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret), payment_id).unwrap();
+	let mut route_params = RouteParameters {
+		payment_params: route.payment_params.clone().unwrap(),
+		final_value_msat: amt_msat,
+		final_cltv_expiry_delta: TEST_FINAL_CLTV,
+	};
+
+	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
+	nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), payment_id, route_params.clone(), Retry::Attempts(1)).unwrap();
 	check_added_monitors!(nodes[0], 2); // one monitor per path
 	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
 	assert_eq!(events.len(), 2);
@@ -184,25 +138,23 @@ fn mpp_retry() {
 	check_added_monitors!(nodes[2], 1);
 	nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]);
 	commitment_signed_dance!(nodes[0], nodes[2], htlc_updates.commitment_signed, false);
-	expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
+	let mut events = nodes[0].node.get_and_clear_pending_events();
+	match events[1] {
+		Event::PendingHTLCsForwardable { .. } => {},
+		_ => panic!("Unexpected event")
+	}
+	events.remove(1);
+	expect_payment_failed_conditions_event(events, payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
 
 	// Rebalance the channel so the second half of the payment can succeed.
 	send_payment(&nodes[3], &vec!(&nodes[2])[..], 1_500_000);
 
-	// Make sure it errors as expected given a too-large amount.
-	if let Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { err })) = nodes[0].node.retry_payment(&route, payment_id) {
-		assert!(err.contains("over total_payment_amt_msat"));
-	} else { panic!("Unexpected error"); }
-
-	// Make sure it errors as expected given the wrong payment_id.
-	if let Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { err })) = nodes[0].node.retry_payment(&route, PaymentId([0; 32])) {
-		assert!(err.contains("not found"));
-	} else { panic!("Unexpected error"); }
-
 	// Retry the second half of the payment and make sure it succeeds.
-	let mut path = route.clone();
-	path.paths.remove(0);
-	nodes[0].node.retry_payment(&path, payment_id).unwrap();
+	route.paths.remove(0);
+	route_params.final_value_msat = 1_000_000;
+	route_params.payment_params.previously_failed_channels.push(chan_4_update.contents.short_channel_id);
+	nodes[0].router.expect_find_route(route_params, Ok(route));
+	nodes[0].node.process_pending_htlc_forwards();
 	check_added_monitors!(nodes[0], 1);
 	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
 	assert_eq!(events.len(), 1);
@@ -388,9 +340,15 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
 
 	// Send two payments - one which will get to nodes[2] and will be claimed, one which we'll time
 	// out and retry.
-	let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], 1_000_000);
+	let amt_msat = 1_000_000;
+	let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], amt_msat);
 	let (payment_preimage_1, payment_hash_1, _, payment_id_1) = send_along_route(&nodes[0], route.clone(), &[&nodes[1], &nodes[2]], 1_000_000);
-	nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)).unwrap();
+	let route_params = RouteParameters {
+		payment_params: route.payment_params.clone().unwrap(),
+		final_value_msat: amt_msat,
+		final_cltv_expiry_delta: TEST_FINAL_CLTV,
+	};
+	nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
 	check_added_monitors!(nodes[0], 1);
 
 	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -499,7 +457,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
 		confirm_transaction(&nodes[0], &first_htlc_timeout_tx);
 	}
 	nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clear();
-	expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
+	expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new());
 
 	// Finally, retry the payment (which was reloaded from the ChannelMonitor when nodes[0] was
 	// reloaded) via a route over the new channel, which work without issue and eventually be
@@ -525,8 +483,8 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
 		nodes[1].node.timer_tick_occurred();
 	}
 
-	assert!(nodes[0].node.retry_payment(&new_route, payment_id_1).is_err()); // Shouldn't be allowed to retry a fulfilled payment
-	nodes[0].node.retry_payment(&new_route, PaymentId(payment_hash.0)).unwrap();
+	assert!(nodes[0].node.send_payment(&new_route, payment_hash, &Some(payment_secret), payment_id_1).is_err()); // Shouldn't be allowed to retry a fulfilled payment
+	nodes[0].node.send_payment(&new_route, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)).unwrap();
 	check_added_monitors!(nodes[0], 1);
 	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
 	assert_eq!(events.len(), 1);
@@ -668,14 +626,14 @@ fn do_test_completed_payment_not_retryable_on_reload(use_dust: bool) {
 	// (which should also still work).
 	connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
 	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
-	// We set mpp_parts_remain to avoid having abandon_payment called
-	expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
+	expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new());
 
 	let chan_0_monitor_serialized = get_monitor!(nodes[0], chan_id).encode();
 	let chan_1_monitor_serialized = get_monitor!(nodes[0], chan_id_3).encode();
 	nodes_0_serialized = nodes[0].node.encode();
 
-	assert!(nodes[0].node.retry_payment(&new_route, payment_id).is_ok());
+	// After the payment failed, we're free to send it again.
+	assert!(nodes[0].node.send_payment(&new_route, payment_hash, &Some(payment_secret), payment_id).is_ok());
 	assert!(!nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 
 	reload_node!(nodes[0], test_default_channel_config(), nodes_0_serialized, &[&chan_0_monitor_serialized, &chan_1_monitor_serialized], second_persister, second_new_chain_monitor, second_nodes_0_deserialized);
@@ -685,19 +643,20 @@ fn do_test_completed_payment_not_retryable_on_reload(use_dust: bool) {
 
 	// Now resend the payment, delivering the HTLC and actually claiming it this time. This ensures
 	// the payment is not (spuriously) listed as still pending.
-	assert!(nodes[0].node.retry_payment(&new_route, payment_id).is_ok());
+	assert!(nodes[0].node.send_payment(&new_route, payment_hash, &Some(payment_secret), payment_id).is_ok());
 	check_added_monitors!(nodes[0], 1);
 	pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], if use_dust { 1_000 } else { 1_000_000 }, payment_hash, payment_secret);
 	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage);
 
-	assert!(nodes[0].node.retry_payment(&new_route, payment_id).is_err());
+	assert!(nodes[0].node.send_payment(&new_route, payment_hash, &Some(payment_secret), payment_id).is_err());
 	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 
 	let chan_0_monitor_serialized = get_monitor!(nodes[0], chan_id).encode();
 	let chan_1_monitor_serialized = get_monitor!(nodes[0], chan_id_3).encode();
 	nodes_0_serialized = nodes[0].node.encode();
 
-	// Ensure that after reload we cannot retry the payment.
+	// Check that after reload we can send the payment again (though we shouldn't, since it was
+	// claimed previously).
 	reload_node!(nodes[0], test_default_channel_config(), nodes_0_serialized, &[&chan_0_monitor_serialized, &chan_1_monitor_serialized], third_persister, third_new_chain_monitor, third_nodes_0_deserialized);
 	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
 
@@ -1233,20 +1192,17 @@ fn abandoned_send_payment_idempotent() {
 	nodes[1].node.fail_htlc_backwards(&first_payment_hash);
 	expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], [HTLCDestination::FailedPayment { payment_hash: first_payment_hash }]);
 
-	pass_failed_payment_back_no_abandon(&nodes[0], &[&[&nodes[1]]], false, first_payment_hash);
-	check_send_rejected!();
-
-	// Until we abandon the payment, no matter how many timer ticks pass, we still cannot reuse the
+	// Until we abandon the payment upon path failure, no matter how many timer ticks pass, we still cannot reuse the
 	// PaymentId.
 	for _ in 0..=IDEMPOTENCY_TIMEOUT_TICKS {
 		nodes[0].node.timer_tick_occurred();
 	}
 	check_send_rejected!();
 
-	nodes[0].node.abandon_payment(payment_id);
-	get_event!(nodes[0], Event::PaymentFailed);
+	pass_failed_payment_back(&nodes[0], &[&[&nodes[1]]], false, first_payment_hash);
 
-	// However, we can reuse the PaymentId immediately after we `abandon_payment`.
+	// However, we can reuse the PaymentId immediately after we `abandon_payment` upon passing the
+	// failed payment back.
 	nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id).unwrap();
 	check_added_monitors!(nodes[0], 1);
 	pass_along_route(&nodes[0], &[&[&nodes[1]]], 100_000, second_payment_hash, second_payment_secret);
@@ -1682,12 +1638,12 @@ fn do_automatic_retries(test: AutoRetry) {
 			check_added_monitors!(&nodes[1], 1);
 			assert!(update_1.update_fail_htlcs.len() == 1);
 			let fail_msg = update_1.update_fail_htlcs[0].clone();
-
 			nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_msg);
 			commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false);
 
 			// Ensure the attempt fails and a new PendingHTLCsForwardable event is generated for the retry
 			let mut events = nodes[0].node.get_and_clear_pending_events();
+			assert_eq!(events.len(), 2);
 			match events[0] {
 				Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, ..  } => {
 					assert_eq!(payment_hash, ev_payment_hash);
@@ -1696,12 +1652,18 @@ fn do_automatic_retries(test: AutoRetry) {
 				_ => panic!("Unexpected event"),
 			}
 			if $expect_pending_htlcs_forwardable {
-				assert_eq!(events.len(), 2);
 				match events[1] {
 					Event::PendingHTLCsForwardable { .. } => {},
 					_ => panic!("Unexpected event"),
 				}
-			} else { assert_eq!(events.len(), 1) }
+			} else {
+				match events[1] {
+					Event::PaymentFailed { payment_hash: ev_payment_hash, .. } => {
+						assert_eq!(payment_hash, ev_payment_hash);
+					},
+					_ => panic!("Unexpected event"),
+				}
+			}
 		}
 	}
 
@@ -1753,17 +1715,6 @@ fn do_automatic_retries(test: AutoRetry) {
 		nodes[0].node.process_pending_htlc_forwards();
 		let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events();
 		assert_eq!(msg_events.len(), 0);
-
-		nodes[0].node.abandon_payment(PaymentId(payment_hash.0));
-		let events = nodes[0].node.get_and_clear_pending_events();
-		assert_eq!(events.len(), 1);
-		match events[0] {
-			Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id } => {
-				assert_eq!(payment_hash, *ev_payment_hash);
-				assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);
-			},
-			_ => panic!("Unexpected event"),
-		}
 	} else if test == AutoRetry::FailTimeout {
 		#[cfg(not(feature = "no-std"))] {
 			// Ensure ChannelManager will not retry a payment if it times out due to Retry::Timeout.
@@ -2434,7 +2385,7 @@ fn no_extra_retries_on_back_to_back_fail() {
 	// by adding the `PaymentFailed` event.
 	//
 	// Because we now retry payments as a batch, we simply return a single-path route in the
-	// second, batched, request, have that fail, then complete the payment via `abandon_payment`.
+	// second, batched, request, have that fail, ensure the payment was abandoned.
 	let mut events = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 4);
 	match events[0] {
@@ -2471,7 +2422,7 @@ fn no_extra_retries_on_back_to_back_fail() {
 	commitment_signed_dance!(nodes[0], nodes[1], &bs_fail_update.commitment_signed, false, true);
 
 	let mut events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
+	assert_eq!(events.len(), 2);
 	match events[0] {
 		Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, ..  } => {
 			assert_eq!(payment_hash, ev_payment_hash);
@@ -2479,10 +2430,7 @@ fn no_extra_retries_on_back_to_back_fail() {
 		},
 		_ => panic!("Unexpected event"),
 	}
-	nodes[0].node.abandon_payment(PaymentId(payment_hash.0));
-	events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	match events[0] {
+	match events[1] {
 		Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id } => {
 			assert_eq!(payment_hash, *ev_payment_hash);
 			assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);