Refactor outgoing channel lookup out from decode_update_add_htlc_onion

In the future, we plan to complete remove `decode_update_add_htlc_onion`
and replace it with a batched variant. This refactor, while improving
readability in its current form, does not feature any functional changes
and allows us to reuse most of the logic in the batched variant.

Author: wpaulino

lightning/src/ln/channelmanager.rs

@@ -3133,6 +3133,66 @@ where
 		Ok(chan_update_opt)
 	}
 
+	fn can_forward_htlc(
+		&self, msg: &msgs::UpdateAddHTLC, next_packet_details: &NextPacketDetails
+	) -> Result<Option<msgs::ChannelUpdate>, (&'static str, u16, Option<msgs::ChannelUpdate>)> {
+		let id_option = self.short_to_chan_info.read().unwrap().get(&next_packet_details.outgoing_scid).cloned();
+		let forwarding_chan_info_opt = match id_option {
+			None => { // unknown_next_peer
+				// Note that this is likely a timing oracle for detecting whether an scid is a
+				// phantom or an intercept.
+				if (self.default_configuration.accept_intercept_htlcs &&
+					fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, next_packet_details.outgoing_scid, &self.chain_hash)) ||
+					fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, next_packet_details.outgoing_scid, &self.chain_hash)
+				{
+					None
+				} else {
+					return Err(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
+				}
+			},
+			Some((cp_id, id)) => Some((cp_id.clone(), id.clone())),
+		};
+
+		let chan_update_opt = if let Some((counterparty_node_id, forwarding_id)) = forwarding_chan_info_opt {
+			let per_peer_state = self.per_peer_state.read().unwrap();
+			let peer_state_mutex_opt = per_peer_state.get(&counterparty_node_id);
+			if peer_state_mutex_opt.is_none() {
+				return Err(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
+			}
+			let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+			let peer_state = &mut *peer_state_lock;
+			let chan = match peer_state.channel_by_id.get_mut(&forwarding_id).map(
+				|chan_phase| if let ChannelPhase::Funded(chan) = chan_phase { Some(chan) } else { None }
+			).flatten() {
+				None => {
+					// Channel was removed. The short_to_chan_info and channel_by_id maps
+					// have no consistency guarantees.
+					return Err(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
+				},
+				Some(chan) => chan,
+			};
+			self.can_forward_htlc_to_outgoing_channel(chan, msg, next_packet_details)?
+		} else {
+			None
+		};
+
+		let cur_height = self.best_block.read().unwrap().height + 1;
+		if let Err((err_msg, err_code)) = check_incoming_htlc_cltv(
+			cur_height, next_packet_details.outgoing_cltv_value, msg.cltv_expiry
+		) {
+			if err_code & 0x1000 != 0 && chan_update_opt.is_none() {
+				// We really should set `incorrect_cltv_expiry` here but as we're not
+				// forwarding over a real channel we can't generate a channel_update
+				// for it. Instead we just return a generic temporary_node_failure.
+				return Err((err_msg, 0x2000 | 2, None));
+			}
+			let chan_update_opt = if err_code & 0x1000 != 0 { chan_update_opt } else { None };
+			return Err((err_msg, err_code, chan_update_opt));
+		}
+
+		Ok(chan_update_opt)
+	}
+
 	fn htlc_failure_from_update_add_err(
 		&self, msg: &msgs::UpdateAddHTLC, counterparty_node_id: &PublicKey, err_msg: &'static str,
 		mut err_code: u16, chan_update: Option<msgs::ChannelUpdate>, is_intro_node_forward: bool,
@@ -3205,71 +3265,14 @@ where
 
 		// Perform outbound checks here instead of in [`Self::construct_pending_htlc_info`] because we
 		// can't hold the outbound peer state lock at the same time as the inbound peer state lock.
-		if let Some((err, code, chan_update)) = loop {
-			let id_option = self.short_to_chan_info.read().unwrap().get(&next_packet_details.outgoing_scid).cloned();
-			let forwarding_chan_info_opt = match id_option {
-				None => { // unknown_next_peer
-					// Note that this is likely a timing oracle for detecting whether an scid is a
-					// phantom or an intercept.
-					if (self.default_configuration.accept_intercept_htlcs &&
-						fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, next_packet_details.outgoing_scid, &self.chain_hash)) ||
-						fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, next_packet_details.outgoing_scid, &self.chain_hash)
-					{
-						None
-					} else {
-						break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
-					}
-				},
-				Some((cp_id, id)) => Some((cp_id.clone(), id.clone())),
-			};
-			let chan_update_opt = if let Some((counterparty_node_id, forwarding_id)) = forwarding_chan_info_opt {
-				let per_peer_state = self.per_peer_state.read().unwrap();
-				let peer_state_mutex_opt = per_peer_state.get(&counterparty_node_id);
-				if peer_state_mutex_opt.is_none() {
-					break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
-				}
-				let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
-				let peer_state = &mut *peer_state_lock;
-				let chan = match peer_state.channel_by_id.get_mut(&forwarding_id).map(
-					|chan_phase| if let ChannelPhase::Funded(chan) = chan_phase { Some(chan) } else { None }
-				).flatten() {
-					None => {
-						// Channel was removed. The short_to_chan_info and channel_by_id maps
-						// have no consistency guarantees.
-						break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
-					},
-					Some(chan) => chan
-				};
-				match self.can_forward_htlc_to_outgoing_channel(chan, msg, &next_packet_details) {
-					Ok(chan_update_opt) => chan_update_opt,
-					Err(e) => break Some(e),
-				}
-			} else {
-				None
-			};
-
-			let cur_height = self.best_block.read().unwrap().height + 1;
-
-			if let Err((err_msg, code)) = check_incoming_htlc_cltv(
-				cur_height, next_packet_details.outgoing_cltv_value, msg.cltv_expiry
-			) {
-				if code & 0x1000 != 0 && chan_update_opt.is_none() {
-					// We really should set `incorrect_cltv_expiry` here but as we're not
-					// forwarding over a real channel we can't generate a channel_update
-					// for it. Instead we just return a generic temporary_node_failure.
-					break Some((err_msg, 0x2000 | 2, None))
-				}
-				let chan_update_opt = if code & 0x1000 != 0 { chan_update_opt } else { None };
-				break Some((err_msg, code, chan_update_opt));
-			}
+		let _ = self.can_forward_htlc(&msg, &next_packet_details).map_err(|e| {
+			let (err_msg, err_code, chan_update_opt) = e;
+			self.htlc_failure_from_update_add_err(
+				msg, counterparty_node_id, err_msg, err_code, chan_update_opt,
+				next_hop.is_intro_node_forward(), &shared_secret
+			)
+		})?;
 
-			break None;
-		}
-		{
-			return Err(self.htlc_failure_from_update_add_err(
-				msg, counterparty_node_id, err, code, chan_update, next_hop.is_intro_node_forward(), &shared_secret
-			));
-		}
 		Ok((next_hop, shared_secret, Some(next_packet_details.next_packet_pubkey)))
 	}