Properly provide PaymentPathSuccessful event for replay claims

When a payment was sent and ultimately completed through an
on-chain HTLC claim which we discover during startup, we
deliberately break the payment tracking logic to keep it around
forever, declining to send a `PaymentPathSuccessful` event but
ensuring that we don't constantly replay the claim on every
startup.

However, now that we now have logic to complete a claim by marking
it as completed in a `ChannelMonitor` and not replaying information
about the claim on every startup. Thus, we no longer need to take
the conservative stance and can correctly replay claims now,
generating `PaymentPathSuccessful` events and allowing the state to
be removed.

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -16565,21 +16565,13 @@ where
 									let mut compl_action = Some(
 										EventCompletionAction::ReleasePaymentCompleteChannelMonitorUpdate(update)
 									);
-									// Note that we set `from_onchain` to "false" here,
-									// deliberately keeping the pending payment around forever.
-									// Given it should only occur when we have a channel we're
-									// force-closing for being stale that's okay.
-									// The alternative would be to wipe the state when claiming,
-									// generating a `PaymentPathSuccessful` event but regenerating
-									// it and the `PaymentSent` on every restart until the
-									// `ChannelMonitor` is removed.
 									pending_outbounds.claim_htlc(
 										payment_id,
 										preimage,
 										bolt12_invoice,
 										session_priv,
 										path,
-										false,
+										true,
 										&mut compl_action,
 										&pending_events,
 										&&logger,

lightning/src/ln/monitor_tests.rs

@@ -3359,12 +3359,15 @@ fn do_test_lost_preimage_monitor_events(on_counterparty_tx: bool) {
 
 	check_added_monitors(&nodes[1], 0);
 	let preimage_events = nodes[1].node.get_and_clear_pending_events();
-	assert_eq!(preimage_events.len(), 2, "{preimage_events:?}");
+	assert_eq!(preimage_events.len(), 3, "{preimage_events:?}");
 	for ev in preimage_events {
 		match ev {
 			Event::PaymentSent { payment_hash, .. } => {
 				assert_eq!(payment_hash, hash_b);
 			},
+			Event::PaymentPathSuccessful { payment_hash, .. } => {
+				assert_eq!(payment_hash, Some(hash_b));
+			},
 			Event::PaymentForwarded { claim_from_onchain_tx, .. } => {
 				assert!(claim_from_onchain_tx);
 			},

lightning/src/ln/payment_tests.rs

@@ -1379,13 +1379,19 @@ fn do_test_dup_htlc_onchain_doesnt_fail_on_reload(
 		} else {
 			expect_payment_sent(&nodes[0], payment_preimage, None, true, false);
 		}
-		// After reload, the ChannelManager identified the failed payment and queued up the
-		// PaymentSent (or not, if `persist_manager_post_event` resulted in us detecting we
-		// already did that) and corresponding ChannelMonitorUpdate to mark the payment
-		// handled, but while processing the pending `MonitorEvent`s (which were not processed
-		// before the monitor was persisted) we will end up with a duplicate
-		// ChannelMonitorUpdate.
-		check_added_monitors(&nodes[0], 2);
+		if persist_manager_post_event {
+			// After reload, the ChannelManager identified the failed payment and queued up the
+			// PaymentSent (or not, if `persist_manager_post_event` resulted in us detecting we
+			// already did that) and corresponding ChannelMonitorUpdate to mark the payment
+			// handled, but while processing the pending `MonitorEvent`s (which were not processed
+			// before the monitor was persisted) we will end up with a duplicate
+			// ChannelMonitorUpdate.
+			check_added_monitors(&nodes[0], 2);
+		} else {
+			// ...unless we got the PaymentSent event, in which case we have de-duplication logic
+			// preventing a redundant monitor event.
+			check_added_monitors(&nodes[0], 1);
+		}
 	}
 
 	// Note that if we re-connect the block which exposed nodes[0] to the payment preimage (but
@@ -4130,7 +4136,7 @@ fn do_no_missing_sent_on_reload(persist_manager_with_payment: bool, at_midpoint:
 	// pending payment from being re-hydrated on the next startup.
 	let events = nodes[0].node.get_and_clear_pending_events();
 	check_added_monitors(&nodes[0], 1);
-	assert_eq!(events.len(), 2);
+	assert_eq!(events.len(), 3, "{events:?}");
 	if let Event::ChannelClosed { reason: ClosureReason::OutdatedChannelManager, .. } = events[0] {
 	} else {
 		panic!();
@@ -4140,6 +4146,10 @@ fn do_no_missing_sent_on_reload(persist_manager_with_payment: bool, at_midpoint:
 	} else {
 		panic!();
 	}
+	if let Event::PaymentPathSuccessful { .. } = events[2] {
+	} else {
+		panic!();
+	}
 	// Note that we don't get a PaymentPathSuccessful here as we leave the HTLC pending to avoid
 	// the double-claim that would otherwise appear at the end of this test.
 	nodes[0].node.timer_tick_occurred();