From: Wilmer Paulino <wilmer@wilmerpaulino.com>
Date: Thu, 28 Sep 2023 23:02:25 +0000 (-0700)
Subject: Avoid persisting on same counterparty's ChannelUpdate
X-Git-Tag: v0.0.117-rc1~10^2
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=c8c535412dab31ca88866fc54ab472458494adb5;p=rust-lightning

Avoid persisting on same counterparty's ChannelUpdate

Some nodes may rebroadcast their `ChannelUpdate` to their counterparty
on every connection establishment, which leads to us doing an additional
persist most of the time when nothing has changed. Now, we'll only
persist if we receive an update that changes anything.
---

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 55e56be2a..f618a4618 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -5537,14 +5537,20 @@ impl<SP: Deref> Channel<SP> where
 		}
 	}
 
-	pub fn channel_update(&mut self, msg: &msgs::ChannelUpdate) -> Result<(), ChannelError> {
-		self.context.counterparty_forwarding_info = Some(CounterpartyForwardingInfo {
+	/// Applies the `ChannelUpdate` and returns a boolean indicating whether a change actually
+	/// happened.
+	pub fn channel_update(&mut self, msg: &msgs::ChannelUpdate) -> Result<bool, ChannelError> {
+		let new_forwarding_info = Some(CounterpartyForwardingInfo {
 			fee_base_msat: msg.contents.fee_base_msat,
 			fee_proportional_millionths: msg.contents.fee_proportional_millionths,
 			cltv_expiry_delta: msg.contents.cltv_expiry_delta
 		});
+		let did_change = self.context.counterparty_forwarding_info != new_forwarding_info;
+		if did_change {
+			self.context.counterparty_forwarding_info = new_forwarding_info;
+		}
 
-		Ok(())
+		Ok(did_change)
 	}
 
 	/// Begins the shutdown process, getting a message for the remote peer and returning all
@@ -8140,7 +8146,7 @@ mod tests {
 			},
 			signature: Signature::from(unsafe { FFISignature::new() })
 		};
-		node_a_chan.channel_update(&update).unwrap();
+		assert!(node_a_chan.channel_update(&update).unwrap());
 
 		// The counterparty can send an update with a higher minimum HTLC, but that shouldn't
 		// change our official htlc_minimum_msat.
@@ -8153,6 +8159,8 @@ mod tests {
 			},
 			None => panic!("expected counterparty forwarding info to be Some")
 		}
+
+		assert!(!node_a_chan.channel_update(&update).unwrap());
 	}
 
 	#[cfg(feature = "_test_vectors")]
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 392f3e1cb..6b4f494ef 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -6757,7 +6757,12 @@ where
 						return Ok(NotifyOption::SkipPersistNoEvents);
 					} else {
 						log_debug!(self.logger, "Received channel_update {:?} for channel {}.", msg, chan_id);
-						try_chan_phase_entry!(self, chan.channel_update(&msg), chan_phase_entry);
+						let did_change = try_chan_phase_entry!(self, chan.channel_update(&msg), chan_phase_entry);
+						// If nothing changed after applying their update, we don't need to bother
+						// persisting.
+						if !did_change {
+							return Ok(NotifyOption::SkipPersistNoEvents);
+						}
 					}
 				} else {
 					return try_chan_phase_entry!(self, Err(ChannelError::Close(