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 55e56be2a79377079377591f14ad1fef33311f17..f618a461858113ed7e45b8052225f92b61cffe7b 100644 (file)
--- 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 392f3e1cb052cf196b50b2b7e3734b62ab167e4c..6b4f494efe2851186afb97bb10dd053505c6b40f 100644 (file)
--- 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(