f refactor forward-checking api

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -4993,55 +4993,53 @@ where
 		}
 	}
 
-	fn forward_needs_intercept(
-		&self, outbound_chan: Option<&FundedChannel<SP>>, outgoing_scid: u64,
-	) -> bool {
+	fn forward_needs_intercept_to_known_chan(&self, outbound_chan: &FundedChannel<SP>) -> bool {
 		let intercept_flags = self.config.read().unwrap().htlc_interception_flags;
-		if let Some(chan) = outbound_chan {
-			if !chan.context.should_announce() {
-				if chan.context.is_connected() {
-					if intercept_flags & (HTLCInterceptionFlags::ToOnlinePrivateChannels as u8) != 0
-					{
-						return true;
-					}
-				} else {
-					if intercept_flags & (HTLCInterceptionFlags::ToOfflinePrivateChannels as u8)
-						!= 0
-					{
-						return true;
-					}
+		if !outbound_chan.context.should_announce() {
+			if outbound_chan.context.is_connected() {
+				if intercept_flags & (HTLCInterceptionFlags::ToOnlinePrivateChannels as u8) != 0 {
+					return true;
 				}
 			} else {
-				if intercept_flags & (HTLCInterceptionFlags::ToPublicChannels as u8) != 0 {
+				if intercept_flags & (HTLCInterceptionFlags::ToOfflinePrivateChannels as u8) != 0 {
 					return true;
 				}
 			}
 		} else {
-			if fake_scid::is_valid_intercept(
-				&self.fake_scid_rand_bytes,
-				outgoing_scid,
-				&self.chain_hash,
-			) {
-				if intercept_flags & (HTLCInterceptionFlags::ToInterceptSCIDs as u8) != 0 {
-					return true;
-				}
-			} else if fake_scid::is_valid_phantom(
-				&self.fake_scid_rand_bytes,
-				outgoing_scid,
-				&self.chain_hash,
-			) {
-				// Handled as a normal forward
-			} else if intercept_flags & (HTLCInterceptionFlags::ToUnknownSCIDs as u8) != 0 {
+			if intercept_flags & (HTLCInterceptionFlags::ToPublicChannels as u8) != 0 {
 				return true;
 			}
 		}
 		false
 	}
 
+	fn forward_needs_intercept_to_unknown_chan(&self, outgoing_scid: u64) -> bool {
+		let intercept_flags = self.config.read().unwrap().htlc_interception_flags;
+		if fake_scid::is_valid_intercept(
+			&self.fake_scid_rand_bytes,
+			outgoing_scid,
+			&self.chain_hash,
+		) {
+			if intercept_flags & (HTLCInterceptionFlags::ToInterceptSCIDs as u8) != 0 {
+				return true;
+			}
+		} else if fake_scid::is_valid_phantom(
+			&self.fake_scid_rand_bytes,
+			outgoing_scid,
+			&self.chain_hash,
+		) {
+			// Handled as a normal forward
+		} else if intercept_flags & (HTLCInterceptionFlags::ToUnknownSCIDs as u8) != 0 {
+			return true;
+		}
+		false
+	}
+
 	#[rustfmt::skip]
 	fn can_forward_htlc_to_outgoing_channel(
-		&self, chan: &mut FundedChannel<SP>, msg: &msgs::UpdateAddHTLC, next_packet: &NextPacketDetails
-	) -> Result<bool, LocalHTLCFailureReason> {
+		&self, chan: &mut FundedChannel<SP>, msg: &msgs::UpdateAddHTLC,
+		next_packet: &NextPacketDetails, will_intercept: bool,
+	) -> Result<(), LocalHTLCFailureReason> {
 		if !chan.context.should_announce()
 			&& !self.config.read().unwrap().accept_forwards_to_priv_channels
 		{
@@ -5050,15 +5048,13 @@ where
 			// we don't allow forwards outbound over them.
 			return Err(LocalHTLCFailureReason::PrivateChannelForward);
 		}
-		let intercepted;
 		if let HopConnector::ShortChannelId(outgoing_scid) = next_packet.outgoing_connector {
 			if chan.funding.get_channel_type().supports_scid_privacy() && outgoing_scid != chan.context.outbound_scid_alias() {
 				// `option_scid_alias` (referred to in LDK as `scid_privacy`) means
 				// "refuse to forward unless the SCID alias was used", so we pretend
 				// we don't have the channel here.
 				return Err(LocalHTLCFailureReason::RealSCIDForward);
 			}
-			intercepted = self.forward_needs_intercept(Some(chan), outgoing_scid);
 		} else {
 			return Err(LocalHTLCFailureReason::InvalidTrampolineForward);
 		}
@@ -5068,7 +5064,7 @@ where
 		// around to doing the actual forward, but better to fail early if we can and
 		// hopefully an attacker trying to path-trace payments cannot make this occur
 		// on a small/per-node/per-channel scale.
-		if !intercepted && !chan.context.is_live() {
+		if !will_intercept && !chan.context.is_live() {
 			if !chan.context.is_enabled() {
 				return Err(LocalHTLCFailureReason::ChannelDisabled);
 			} else if !chan.context.is_connected() {
@@ -5080,9 +5076,7 @@ where
 		if next_packet.outgoing_amt_msat < chan.context.get_counterparty_htlc_minimum_msat() {
 			return Err(LocalHTLCFailureReason::AmountBelowMinimum);
 		}
-		chan.htlc_satisfies_config(msg, next_packet.outgoing_amt_msat, next_packet.outgoing_cltv_value)?;
-
-		Ok(intercepted)
+		chan.htlc_satisfies_config(msg, next_packet.outgoing_amt_msat, next_packet.outgoing_cltv_value)
 	}
 
 	/// Executes a callback `C` that returns some value `X` on the channel found with the given
@@ -5109,31 +5103,32 @@ where
 	}
 
 	fn can_forward_htlc_intercepted(
-		&self, msg: &msgs::UpdateAddHTLC, next_packet_details: &NextPacketDetails,
+		&self, msg: &msgs::UpdateAddHTLC, next_hop: &NextPacketDetails,
 	) -> Result<bool, LocalHTLCFailureReason> {
-		let outgoing_scid = match next_packet_details.outgoing_connector {
+		let outgoing_scid = match next_hop.outgoing_connector {
 			HopConnector::ShortChannelId(scid) => scid,
 			HopConnector::Trampoline(_) => {
 				return Err(LocalHTLCFailureReason::InvalidTrampolineForward);
 			},
 		};
 		// TODO: We do the fake SCID namespace check a bunch of times here (and indirectly via
-		// `forward_needs_intercept`, including as called in
+		// `forward_needs_intercept_*`, including as called in
 		// `can_forward_htlc_to_outgoing_channel`), we should find a way to reduce the number of
 		// times we do it.
 		let intercept =
 			match self.do_funded_channel_callback(outgoing_scid, |chan: &mut FundedChannel<SP>| {
-				self.can_forward_htlc_to_outgoing_channel(chan, msg, next_packet_details)
+				let intercept = self.forward_needs_intercept_to_known_chan(chan);
+				self.can_forward_htlc_to_outgoing_channel(chan, msg, next_hop, intercept)?;
+				Ok(intercept)
 			}) {
 				Some(Ok(intercept)) => intercept,
 				Some(Err(e)) => return Err(e),
 				None => {
 					// Perform basic sanity checks on the amounts and CLTV being forwarded
-					if next_packet_details.outgoing_amt_msat > msg.amount_msat {
+					if next_hop.outgoing_amt_msat > msg.amount_msat {
 						return Err(LocalHTLCFailureReason::FeeInsufficient);
 					}
-					let cltv_delta =
-						msg.cltv_expiry.saturating_sub(next_packet_details.outgoing_cltv_value);
+					let cltv_delta = msg.cltv_expiry.saturating_sub(next_hop.outgoing_cltv_value);
 					if cltv_delta < MIN_CLTV_EXPIRY_DELTA.into() {
 						return Err(LocalHTLCFailureReason::IncorrectCLTVExpiry);
 					}
@@ -5144,7 +5139,7 @@ where
 						&self.chain_hash,
 					) {
 						false
-					} else if self.forward_needs_intercept(None, outgoing_scid) {
+					} else if self.forward_needs_intercept_to_unknown_chan(outgoing_scid) {
 						true
 					} else {
 						return Err(LocalHTLCFailureReason::UnknownNextPeer);
@@ -5153,11 +5148,7 @@ where
 			};
 
 		let cur_height = self.best_block.read().unwrap().height + 1;
-		check_incoming_htlc_cltv(
-			cur_height,
-			next_packet_details.outgoing_cltv_value,
-			msg.cltv_expiry,
-		)?;
+		check_incoming_htlc_cltv(cur_height, next_hop.outgoing_cltv_value, msg.cltv_expiry)?;
 
 		Ok(intercept)
 	}
@@ -15982,9 +15973,9 @@ where
 
 				let should_intercept = self
 					.do_funded_channel_callback(next_hop_scid, |chan| {
-						self.forward_needs_intercept(Some(chan), next_hop_scid)
+						self.forward_needs_intercept_to_known_chan(chan)
 					})
-					.unwrap_or_else(|| self.forward_needs_intercept(None, next_hop_scid));
+					.unwrap_or_else(|| self.forward_needs_intercept_to_unknown_chan(next_hop_scid));
 
 				if should_intercept {
 					let intercept_id = InterceptId::from_htlc_id_and_chan_id(