Refactor decode_update_add_htlc_onion to avoid doing channel lookups

This allows callers to use the method on a specific channel when the
`per_peer_state` locks have already been acquired. This will become
helpful later on when we move to decoding incoming HTLC onions once the
HTLC has been fully committed to by both sides.

Author: wpaulino

lightning/src/ln/channelmanager.rs

@@ -3062,6 +3062,49 @@ where
 		Ok(chan_update_opt)
 	}
 
+	fn find_channel_and_process_accept(
+		&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())),
+		};
+		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_accept_htlc(chan, msg, next_packet_details)
+		} else {
+			Ok(None)
+		}
+	}
+
 	fn process_failed_accept_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,
@@ -3118,14 +3161,20 @@ where
 	}
 
 	fn decode_update_add_htlc_onion(
-		&self, msg: &msgs::UpdateAddHTLC, counterparty_node_id: &PublicKey,
+		&self, msg: &msgs::UpdateAddHTLC, counterparty_node_id: &PublicKey, channel: Option<&mut Channel<SP>>,
 	) -> Result<
 		(onion_utils::Hop, [u8; 32], Option<Result<PublicKey, secp256k1::Error>>), HTLCFailureMsg
 	> {
 		let (next_hop, shared_secret, next_packet_details_opt) = decode_incoming_update_add_htlc_onion(
 			msg, &self.node_signer, &self.logger, &self.secp_ctx
 		)?;
 
+		let next_packet_details = match next_packet_details_opt {
+			Some(next_packet_details) => next_packet_details,
+			// it is a receive, so no need for outbound checks
+			None => return Ok((next_hop, shared_secret, None)),
+		};
+
 		let is_intro_node_forward = match next_hop {
 			onion_utils::Hop::Forward {
 				next_hop_data: msgs::InboundOnionPayload::BlindedForward {
@@ -3135,79 +3184,36 @@ where
 			_ => false,
 		};
 
-		let next_packet_details = match next_packet_details_opt {
-			Some(next_packet_details) => next_packet_details,
-			// it is a receive, so no need for outbound checks
-			None => return Ok((next_hop, shared_secret, None)),
+		let fail_msg_from_err = |err_msg: &'static str, err_code: u16, chan_update: Option<msgs::ChannelUpdate>| {
+			self.process_failed_accept_err(msg, counterparty_node_id, err_msg, err_code, chan_update, is_intro_node_forward, &shared_secret)
 		};
 
-		// 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_accept_htlc(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;
+		let res = if let Some(channel) = channel {
+			self.can_accept_htlc(channel, msg, &next_packet_details)
+		} else {
+			// 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.
+			self.find_channel_and_process_accept(msg, &next_packet_details)
+		};
+		let chan_update_opt = match res {
+			Ok(chan_update_opt) => chan_update_opt,
+			Err((err_msg, err_code, chan_update)) => return Err(fail_msg_from_err(err_msg, err_code, chan_update)),
+		};
 
-			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 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(fail_msg_from_err(err_msg, 0x2000 | 2, None));
 			}
-
-			break None;
-		}
-		{
-			return Err(self.process_failed_accept_err(
-				msg, counterparty_node_id, err, code, chan_update, is_intro_node_forward, &shared_secret
-			));
+			let chan_update_opt = if err_code & 0x1000 != 0 { chan_update_opt } else { None };
+			return Err(fail_msg_from_err(err_msg, err_code, chan_update_opt));
 		}
+
 		Ok((next_hop, shared_secret, Some(next_packet_details.next_packet_pubkey)))
 	}
 
@@ -6655,7 +6661,7 @@ where
 		// Note that the ChannelManager is NOT re-persisted on disk after this (unless we error
 		// closing a channel), so any changes are likely to be lost on restart!
 
-		let decoded_hop_res = self.decode_update_add_htlc_onion(msg, counterparty_node_id);
+		let decoded_hop_res = self.decode_update_add_htlc_onion(msg, counterparty_node_id, None);
 		let per_peer_state = self.per_peer_state.read().unwrap();
 		let peer_state_mutex = per_peer_state.get(counterparty_node_id)
 			.ok_or_else(|| {