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.

Author: wpaulino

lightning/src/ln/channel.rs

@@ -5537,14 +5537,25 @@ impl<SP: Deref> Channel<SP> where
 		}
 	}
 
-	pub fn channel_update(&mut self, msg: &msgs::ChannelUpdate) -> Result<(), ChannelError> {
-		self.context.counterparty_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
-		});
+	/// 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 did_change = self.context.counterparty_forwarding_info.as_ref()
+			.map(|fwd_info|
+				 fwd_info.fee_base_msat != msg.contents.fee_base_msat ||
+					 fwd_info.fee_proportional_millionths != msg.contents.fee_proportional_millionths ||
+						 fwd_info.cltv_expiry_delta != msg.contents.cltv_expiry_delta
+			 ).unwrap_or(true);
+
+		if did_change {
+			self.context.counterparty_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
+			});
+		}
 
-		Ok(())
+		Ok(did_change)
 	}
 
 	/// Begins the shutdown process, getting a message for the remote peer and returning all
@@ -8140,7 +8151,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 +8164,8 @@ mod tests {
 			},
 			None => panic!("expected counterparty forwarding info to be Some")
 		}
+
+		assert!(!node_a_chan.channel_update(&update).unwrap());
 	}
 
 	#[cfg(feature = "_test_vectors")]

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(