From c8c535412dab31ca88866fc54ab472458494adb5 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 28 Sep 2023 16:02:25 -0700 Subject: [PATCH] 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. --- lightning/src/ln/channel.rs | 16 ++++++++++++---- lightning/src/ln/channelmanager.rs | 7 ++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 55e56be2..f618a461 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5537,14 +5537,20 @@ impl Channel 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 { + 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 392f3e1c..6b4f494e 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( -- 2.30.2