Don't apply NetworkUpdate from failed paths to NetworkGraph

If updates returned from failed paths are permanently applied to our
graph in a manner observable to the rest of the network, malicious
intermediaries may be able to fail HTLCs and infer the payment origin.

Here, we therefore avoid permanent application of network updates and
will rather switch to consider them temporarily for payment retries in
the next step.

Author: tnull

lightning-background-processor/src/lib.rs

@@ -26,7 +26,7 @@ use lightning::chain;
 use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use lightning::chain::chainmonitor::{ChainMonitor, Persist};
 use lightning::sign::{EntropySource, NodeSigner, SignerProvider};
-use lightning::events::{Event, PathFailure};
+use lightning::events::Event;
 #[cfg(feature = "std")]
 use lightning::events::{EventHandler, EventsProvider};
 use lightning::ln::channelmanager::ChannelManager;
@@ -226,16 +226,6 @@ where
 	}
 }
 
-fn handle_network_graph_update<L: Deref>(
-	network_graph: &NetworkGraph<L>, event: &Event
-) where L::Target: Logger {
-	if let Event::PaymentPathFailed {
-		failure: PathFailure::OnPath { network_update: Some(ref upd) }, .. } = event
-	{
-		network_graph.handle_network_update(upd);
-	}
-}
-
 /// Updates scorer based on event and returns whether an update occurred so we can decide whether
 /// to persist.
 fn update_scorer<'a, S: 'static + Deref<Target = SC> + Send + Sync, SC: 'a + WriteableScore<'a>>(
@@ -636,9 +626,6 @@ where
 		let logger = &logger;
 		let persister = &persister;
 		async move {
-			if let Some(network_graph) = network_graph {
-				handle_network_graph_update(network_graph, &event)
-			}
 			if let Some(ref scorer) = scorer {
 				if update_scorer(scorer, &event) {
 					log_trace!(logger, "Persisting scorer after update");
@@ -768,10 +755,6 @@ impl BackgroundProcessor {
 		let stop_thread_clone = stop_thread.clone();
 		let handle = thread::spawn(move || -> Result<(), std::io::Error> {
 			let event_handler = |event| {
-				let network_graph = gossip_sync.network_graph();
-				if let Some(network_graph) = network_graph {
-					handle_network_graph_update(network_graph, &event)
-				}
 				if let Some(ref scorer) = scorer {
 					if update_scorer(scorer, &event) {
 						log_trace!(logger, "Persisting scorer after update");

lightning/src/events/mod.rs

@@ -125,8 +125,12 @@ pub enum PathFailure {
 	},
 	/// A hop on the path failed to forward our payment.
 	OnPath {
-		/// If present, this [`NetworkUpdate`] should be applied to the [`NetworkGraph`] so that routing
-		/// decisions can take into account the update.
+		/// If present, this [`NetworkUpdate`] can be applied to a [`NetworkGraph`] so that
+		/// routing decisions can permanently take the update into account.
+		///
+		/// However, note that if applying the update is obversable to the rest of the network
+		/// (e.g., through public gossip), it can introduce a privacy leak as malicious
+		/// intermediaries may fail HTLCs and try to infer the payment's origin.
 		///
 		/// [`NetworkUpdate`]: crate::routing::gossip::NetworkUpdate
 		/// [`NetworkGraph`]: crate::routing::gossip::NetworkGraph
@@ -624,8 +628,8 @@ pub enum Event {
 		/// the payment has failed, not just the route in question. If this is not set, the payment may
 		/// be retried via a different route.
 		payment_failed_permanently: bool,
-		/// Extra error details based on the failure type. May contain an update that needs to be
-		/// applied to the [`NetworkGraph`].
+		/// Extra error details based on the failure type. May contain an update that can be
+		/// applied to a [`NetworkGraph`].
 		///
 		/// [`NetworkGraph`]: crate::routing::gossip::NetworkGraph
 		failure: PathFailure,