From d53ab1990c37ed4b639bb86afbcd3af0dadacc87 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 22 Jul 2025 15:12:15 +0200 Subject: [PATCH 1/3] Rustfmt some channel.rs --- lightning/src/ln/channel.rs | 127 +++++++++++++++++++++++++----------- 1 file changed, 88 insertions(+), 39 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 967ad41bc1b..c420a7f60de 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10582,22 +10582,32 @@ where /// Queues up an outbound HTLC to send by placing it in the holding cell. You should call /// [`Self::maybe_free_holding_cell_htlcs`] in order to actually generate and send the /// commitment update. - #[rustfmt::skip] pub fn queue_add_htlc( - &mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, - onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option, - blinding_point: Option, fee_estimator: &LowerBoundedFeeEstimator, logger: &L + &mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, + source: HTLCSource, onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option, + blinding_point: Option, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, ) -> Result<(), (LocalHTLCFailureReason, String)> - where F::Target: FeeEstimator, L::Target: Logger + where + F::Target: FeeEstimator, + L::Target: Logger, { - self - .send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, true, - skimmed_fee_msat, blinding_point, fee_estimator, logger) - .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?")) - .map_err(|err| { - debug_assert!(err.0.is_temporary(), "Queuing HTLC should return temporary error"); - err - }) + self.send_htlc( + amount_msat, + payment_hash, + cltv_expiry, + source, + onion_routing_packet, + true, + skimmed_fee_msat, + blinding_point, + fee_estimator, + logger, + ) + .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?")) + .map_err(|err| { + debug_assert!(err.0.is_temporary(), "Queuing HTLC should return temporary error"); + err + }) } /// Adds a pending outbound HTLC to this channel, note that you probably want @@ -10616,18 +10626,19 @@ where /// on this [`FundedChannel`] if `force_holding_cell` is false. /// /// `Err`'s will always be temporary channel failures. - #[rustfmt::skip] fn send_htlc( - &mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, - onion_routing_packet: msgs::OnionPacket, mut force_holding_cell: bool, + &mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, + source: HTLCSource, onion_routing_packet: msgs::OnionPacket, mut force_holding_cell: bool, skimmed_fee_msat: Option, blinding_point: Option, - fee_estimator: &LowerBoundedFeeEstimator, logger: &L + fee_estimator: &LowerBoundedFeeEstimator, logger: &L, ) -> Result, (LocalHTLCFailureReason, String)> - where F::Target: FeeEstimator, L::Target: Logger + where + F::Target: FeeEstimator, + L::Target: Logger, { - if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) || - self.context.channel_state.is_local_shutdown_sent() || - self.context.channel_state.is_remote_shutdown_sent() + if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) + || self.context.channel_state.is_local_shutdown_sent() + || self.context.channel_state.is_remote_shutdown_sent() { return Err((LocalHTLCFailureReason::ChannelNotReady, "Cannot send HTLC until channel is fully established and we haven't started shutting down".to_owned())); @@ -10639,13 +10650,23 @@ where let available_balances = self.get_available_balances(fee_estimator); if amount_msat < available_balances.next_outbound_htlc_minimum_msat { - return Err((LocalHTLCFailureReason::HTLCMinimum, format!("Cannot send less than our next-HTLC minimum - {} msat", - available_balances.next_outbound_htlc_minimum_msat))); + return Err(( + LocalHTLCFailureReason::HTLCMinimum, + format!( + "Cannot send less than our next-HTLC minimum - {} msat", + available_balances.next_outbound_htlc_minimum_msat + ), + )); } if amount_msat > available_balances.next_outbound_htlc_limit_msat { - return Err((LocalHTLCFailureReason::HTLCMaximum, format!("Cannot send more than our next-HTLC maximum - {} msat", - available_balances.next_outbound_htlc_limit_msat))); + return Err(( + LocalHTLCFailureReason::HTLCMaximum, + format!( + "Cannot send more than our next-HTLC maximum - {} msat", + available_balances.next_outbound_htlc_limit_msat + ), + )); } if self.context.channel_state.is_peer_disconnected() { @@ -10655,16 +10676,26 @@ where // disconnected during the time the previous hop was doing the commitment dance we may // end up getting here after the forwarding delay. In any case, returning an // IgnoreError will get ChannelManager to do the right thing and fail backwards now. - return Err((LocalHTLCFailureReason::PeerOffline, - "Cannot send an HTLC while disconnected from channel counterparty".to_owned())); + return Err(( + LocalHTLCFailureReason::PeerOffline, + "Cannot send an HTLC while disconnected from channel counterparty".to_owned(), + )); } let need_holding_cell = !self.context.channel_state.can_generate_new_commitment(); - log_debug!(logger, "Pushing new outbound HTLC with hash {} for {} msat {}", - payment_hash, amount_msat, - if force_holding_cell { "into holding cell" } - else if need_holding_cell { "into holding cell as we're awaiting an RAA or monitor" } - else { "to peer" }); + log_debug!( + logger, + "Pushing new outbound HTLC with hash {} for {} msat {}", + payment_hash, + amount_msat, + if force_holding_cell { + "into holding cell" + } else if need_holding_cell { + "into holding cell as we're awaiting an RAA or monitor" + } else { + "to peer" + } + ); if need_holding_cell { force_holding_cell = true; @@ -10952,24 +10983,42 @@ where /// /// Shorthand for calling [`Self::send_htlc`] followed by a commitment update, see docs on /// [`Self::send_htlc`] and [`Self::build_commitment_no_state_update`] for more info. - #[rustfmt::skip] pub fn send_htlc_and_commit( &mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option, - fee_estimator: &LowerBoundedFeeEstimator, logger: &L + fee_estimator: &LowerBoundedFeeEstimator, logger: &L, ) -> Result, ChannelError> - where F::Target: FeeEstimator, L::Target: Logger + where + F::Target: FeeEstimator, + L::Target: Logger, { - let send_res = self.send_htlc(amount_msat, payment_hash, cltv_expiry, source, - onion_routing_packet, false, skimmed_fee_msat, None, fee_estimator, logger); + let send_res = self.send_htlc( + amount_msat, + payment_hash, + cltv_expiry, + source, + onion_routing_packet, + false, + skimmed_fee_msat, + None, + fee_estimator, + logger, + ); // All [`LocalHTLCFailureReason`] errors are temporary, so they are [`ChannelError::Ignore`]. match send_res.map_err(|(_, msg)| ChannelError::Ignore(msg))? { Some(_) => { let monitor_update = self.build_commitment_no_status_check(logger); - self.monitor_updating_paused(false, true, false, Vec::new(), Vec::new(), Vec::new()); + self.monitor_updating_paused( + false, + true, + false, + Vec::new(), + Vec::new(), + Vec::new(), + ); Ok(self.push_ret_blockable_mon_update(monitor_update)) }, - None => Ok(None) + None => Ok(None), } } From 29aab5d846d136f75343e48f98fdf8090757bff9 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 23 Jul 2025 08:38:10 +0200 Subject: [PATCH 2/3] Remove spurious comment in channel.rs --- lightning/src/ln/channel.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c420a7f60de..02e302444fc 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10577,8 +10577,6 @@ where Ok((None, None)) } - // Send stuff to our remote peers: - /// Queues up an outbound HTLC to send by placing it in the holding cell. You should call /// [`Self::maybe_free_holding_cell_htlcs`] in order to actually generate and send the /// commitment update. From 5e3af3bc4f59a22c76f02e9303174debced937df Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 22 Jul 2025 15:09:13 +0200 Subject: [PATCH 3/3] Simplify send_htlc return value --- lightning/src/ln/channel.rs | 53 +++++++++++++------------------------ 1 file changed, 18 insertions(+), 35 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 02e302444fc..89c29708b75 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7238,12 +7238,12 @@ where fee_estimator, logger, ) { - Ok(update_add_msg_opt) => { - // `send_htlc` only returns `Ok(None)`, when an update goes into + Ok(can_add_htlc) => { + // `send_htlc` only returns `Ok(false)`, when an update goes into // the holding cell, but since we're currently freeing it, we should - // always expect to see the `update_add` go out. + // always expect to see the htlc added. debug_assert!( - update_add_msg_opt.is_some(), + can_add_htlc, "Must generate new update if we're freeing the holding cell" ); update_add_count += 1; @@ -10601,7 +10601,7 @@ where fee_estimator, logger, ) - .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?")) + .map(|can_add_htlc| assert!(!can_add_htlc, "We forced holding cell?")) .map_err(|err| { debug_assert!(err.0.is_temporary(), "Queuing HTLC should return temporary error"); err @@ -10611,8 +10611,9 @@ where /// Adds a pending outbound HTLC to this channel, note that you probably want /// [`Self::send_htlc_and_commit`] instead cause you'll want both messages at once. /// - /// This returns an optional UpdateAddHTLC as we may be in a state where we cannot add HTLCs on - /// the wire: + /// This returns a boolean indicating whether we are in a state where we can add HTLCs on the wire. + /// Reasons we may not be able to add HTLCs on the wire include: + /// /// * In cases where we're waiting on the remote peer to send us a revoke_and_ack, we /// wouldn't be able to determine what they actually ACK'ed if we have two sets of updates /// awaiting ACK. @@ -10629,7 +10630,7 @@ where source: HTLCSource, onion_routing_packet: msgs::OnionPacket, mut force_holding_cell: bool, skimmed_fee_msat: Option, blinding_point: Option, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, - ) -> Result, (LocalHTLCFailureReason, String)> + ) -> Result where F::Target: FeeEstimator, L::Target: Logger, @@ -10710,7 +10711,7 @@ where skimmed_fee_msat, blinding_point, }); - return Ok(None); + return Ok(false); } // Record the approximate time when the HTLC is sent to the peer. This timestamp is later used to calculate the @@ -10731,20 +10732,9 @@ where skimmed_fee_msat, send_timestamp, }); - - let res = msgs::UpdateAddHTLC { - channel_id: self.context.channel_id, - htlc_id: self.context.next_holder_htlc_id, - amount_msat, - payment_hash, - cltv_expiry, - onion_routing_packet, - skimmed_fee_msat, - blinding_point, - }; self.context.next_holder_htlc_id += 1; - Ok(Some(res)) + Ok(true) } #[rustfmt::skip] @@ -11003,20 +10993,13 @@ where logger, ); // All [`LocalHTLCFailureReason`] errors are temporary, so they are [`ChannelError::Ignore`]. - match send_res.map_err(|(_, msg)| ChannelError::Ignore(msg))? { - Some(_) => { - let monitor_update = self.build_commitment_no_status_check(logger); - self.monitor_updating_paused( - false, - true, - false, - Vec::new(), - Vec::new(), - Vec::new(), - ); - Ok(self.push_ret_blockable_mon_update(monitor_update)) - }, - None => Ok(None), + let can_add_htlc = send_res.map_err(|(_, msg)| ChannelError::Ignore(msg))?; + if can_add_htlc { + let monitor_update = self.build_commitment_no_status_check(logger); + self.monitor_updating_paused(false, true, false, Vec::new(), Vec::new(), Vec::new()); + Ok(self.push_ret_blockable_mon_update(monitor_update)) + } else { + Ok(None) } }