Skip to content

Simplify send htlc #3946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 23, 2025
Merged

Simplify send htlc #3946

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d53ab19
Rustfmt some channel.rs
joostjager Jul 22, 2025
29aab5d
Remove spurious comment in channel.rs
joostjager Jul 23, 2025
5e3af3b
Simplify send_htlc return value
joostjager Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 96 additions & 66 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -10577,34 +10577,43 @@ 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.
#[rustfmt::skip]
pub fn queue_add_htlc<F: Deref, L: Deref>(
&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource,
onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option<u64>,
blinding_point: Option<PublicKey>, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L
&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32,
source: HTLCSource, onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option<u64>,
blinding_point: Option<PublicKey>, fee_estimator: &LowerBoundedFeeEstimator<F>, 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(|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
})
}

/// 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.
Expand All @@ -10616,18 +10625,19 @@ where
/// on this [`FundedChannel`] if `force_holding_cell` is false.
///
/// `Err`'s will always be temporary channel failures.
#[rustfmt::skip]
fn send_htlc<F: Deref, L: Deref>(
&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<u64>, blinding_point: Option<PublicKey>,
fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L
) -> Result<Option<msgs::UpdateAddHTLC>, (LocalHTLCFailureReason, String)>
where F::Target: FeeEstimator, L::Target: Logger
fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
) -> Result<bool, (LocalHTLCFailureReason, String)>
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()));
Expand All @@ -10639,13 +10649,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() {
Expand All @@ -10655,16 +10675,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;
Expand All @@ -10681,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
Expand All @@ -10702,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]
Expand Down Expand Up @@ -10952,24 +10971,35 @@ 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<F: Deref, L: Deref>(
&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32,
source: HTLCSource, onion_routing_packet: msgs::OnionPacket, skimmed_fee_msat: Option<u64>,
fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L
fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
) -> Result<Option<ChannelMonitorUpdate>, 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());
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)
}
}

Expand Down
Loading