Merge pull request #2617 from wpaulino/no-persist-same-channel-update
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Fri, 29 Sep 2023 17:39:49 +0000 (17:39 +0000)
committerGitHub <noreply@github.com>
Fri, 29 Sep 2023 17:39:49 +0000 (17:39 +0000)
Avoid persisting on same counterparty's ChannelUpdate

lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs

index 55e56be2a79377079377591f14ad1fef33311f17..f618a461858113ed7e45b8052225f92b61cffe7b 100644 (file)
@@ -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")]
index 00becff19d73c5e4a013ec2472addd4c2bdd8cc2..cfa17295a53f37bd8fabd63b54ea4053a27f27eb 100644 (file)
@@ -6756,7 +6756,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(