Select single optimal channel instead of iterating

Author: wvanlint

fuzz/src/full_stack.rs

@@ -1366,8 +1366,8 @@ mod tests {
 
 		// process the now-pending HTLC forward
 		ext_from_hex("07", &mut test);
-		// Two feerate requests to check dust exposure
-		ext_from_hex("00fd00fd", &mut test);
+		// Three feerate requests to check dust exposure
+		ext_from_hex("00fd00fd00fd", &mut test);
 		// client now sends id 1 update_add_htlc and commitment_signed (CHECK 7: UpdateHTLCs event for node 03020000 with 1 HTLCs for channel 3f000000)
 
 		// we respond with commitment_signed then revoke_and_ack (a weird, but valid, order)
@@ -1478,8 +1478,8 @@ mod tests {
 		// process the now-pending HTLC forward
 		ext_from_hex("07", &mut test);
 
-		// Two feerate requests to check dust exposure
-		ext_from_hex("00fd00fd", &mut test);
+		// Three feerate requests to check dust exposure
+		ext_from_hex("00fd00fd00fd", &mut test);
 
 		// client now sends id 1 update_add_htlc and commitment_signed (CHECK 7 duplicate)
 		// we respond with revoke_and_ack, then commitment_signed, then update_fail_htlc
@@ -1602,8 +1602,8 @@ mod tests {
 
 		// process the now-pending HTLC forward
 		ext_from_hex("07", &mut test);
-		// Two feerate requests to check dust exposure
-		ext_from_hex("00fd00fd", &mut test);
+		// Three feerate requests to check dust exposure
+		ext_from_hex("00fd00fd00fd", &mut test);
 		// client now sends id 1 update_add_htlc and commitment_signed (CHECK 7 duplicate)
 
 		// connect a block with one transaction of len 125

lightning/src/ln/channelmanager.rs

@@ -5055,34 +5055,45 @@ where
 									// applying non-strict forwarding.
 									// The channel with the least amount of outbound liquidity will be used to maximize the
 									// probability of being able to successfully forward a subsequent HTLC.
-									let mut channels_with_peer = peer_state.channel_by_id.values_mut().filter_map(|phase| match phase {
-										ChannelPhase::Funded(chan) => Some(chan),
+									let maybe_optimal_channel = peer_state.channel_by_id.values_mut().filter_map(|phase| match phase {
+										ChannelPhase::Funded(chan) => {
+											let balances = chan.context.get_available_balances(&self.fee_estimator);
+											if outgoing_amt_msat <= balances.next_outbound_htlc_limit_msat &&
+												outgoing_amt_msat >= balances.next_outbound_htlc_minimum_msat {
+												Some((chan, balances))
+											} else {
+												None
+											}
+										},
 										_ => None,
-									}).collect::<Vec<&mut Channel<_>>>();
-									channels_with_peer.sort_by_key(|chan| chan.context.get_available_balances(&self.fee_estimator).outbound_capacity_msat);
-									let successfully_added = channels_with_peer.iter_mut().any(|chan| {
-										let logger = WithChannelContext::from(&self.logger, &chan.context, Some(payment_hash));
-										let add_result = chan.queue_add_htlc(outgoing_amt_msat, payment_hash,
-											outgoing_cltv_value, htlc_source.clone(), onion_packet.clone(),
-											skimmed_fee_msat, next_blinding_point, &self.fee_estimator, &&logger);
-										match add_result {
-											Ok(_) => {
-												log_trace!(logger, "Forwarding HTLC from SCID {} with payment_hash {} and specified next hop SCID {} over channel {} with corresponding peer {}",
-													prev_short_channel_id, &payment_hash, short_chan_id, chan.context.channel_id(), &counterparty_node_id);
-											},
-											Err(ChannelError::Ignore(ref msg)) => {
-												log_trace!(logger, "Not forwarding HTLC with payment_hash {} over channel {} with peer {}: {}. Will attempt other channels with the same peer if possible.",
-													&payment_hash, chan.context.channel_id(), &counterparty_node_id, msg);
-											},
-											Err(_) => {
-												panic!("Stated return value requirements in send_htlc() were not met");
-											},
+									}).min_by_key(|(_, balances)| balances.next_outbound_htlc_limit_msat).map(|(c, _)| c);
+									let optimal_channel = match maybe_optimal_channel {
+										Some(chan) => chan,
+										None => {
+											// Fall back to the specified channel to return an appropriate error.
+											if let Some(ChannelPhase::Funded(ref mut chan)) = peer_state.channel_by_id.get_mut(&forward_chan_id) {
+												chan
+											} else {
+												forwarding_channel_not_found!(core::iter::once(forward_info).chain(draining_pending_forwards));
+												break;
+											}
+										}
+									};
+
+									let logger = WithChannelContext::from(&self.logger, &optimal_channel.context, Some(payment_hash));
+									log_trace!(logger, "Forwarding HTLC from SCID {} with payment_hash {} and specified next hop SCID {} over optimal channel {} with corresponding peer {}",
+										prev_short_channel_id, &payment_hash, short_chan_id, optimal_channel.context.channel_id(), &counterparty_node_id);
+									if let Err(e) = optimal_channel.queue_add_htlc(outgoing_amt_msat,
+											payment_hash, outgoing_cltv_value, htlc_source.clone(),
+											onion_packet.clone(), skimmed_fee_msat, next_blinding_point, &self.fee_estimator,
+											&&logger)
+									{
+										if let ChannelError::Ignore(msg) = e {
+											log_trace!(logger, "Failed to forward HTLC with payment_hash {} to peer {}: {}", &payment_hash, &counterparty_node_id, msg);
+										} else {
+											panic!("Stated return value requirements in send_htlc() were not met");
 										}
-										add_result.is_ok()
-									});
 
-									if !successfully_added {
-										log_trace!(self.logger, "Failed to forward HTLC with payment_hash {} to peer {}", &payment_hash, &counterparty_node_id);
 										if let Some(ChannelPhase::Funded(ref mut chan)) = peer_state.channel_by_id.get_mut(&forward_chan_id) {
 											let (failure_code, data) = self.get_htlc_temp_fail_err_and_data(0x1000|7, short_chan_id, chan);
 											failed_forwards.push((htlc_source, payment_hash,