ln/refactor: use LocalHTLCFailureCodes in onion error processing

Author: carlaKC

lightning/src/ln/onion_utils.rs

@@ -20,7 +20,7 @@ use crate::routing::router::{BlindedTail, Path, RouteHop, RouteParameters, Tramp
 use crate::sign::{NodeSigner, Recipient};
 use crate::types::features::{ChannelFeatures, NodeFeatures};
 use crate::types::payment::{PaymentHash, PaymentPreimage};
-use crate::util::errors::{self, APIError};
+use crate::util::errors::APIError;
 use crate::util::logger::Logger;
 use crate::util::ser::{
 	LengthCalculatingWriter, Readable, ReadableArgs, VecWriter, Writeable, Writer,
@@ -975,7 +975,7 @@ mod fuzzy_onion_utils {
 		#[allow(dead_code)]
 		pub(crate) hold_times: Vec<u32>,
 		#[cfg(any(test, feature = "_test_utils"))]
-		pub(crate) onion_error_code: Option<u16>,
+		pub(crate) onion_error_code: Option<LocalHTLCFailureReason>,
 		#[cfg(any(test, feature = "_test_utils"))]
 		pub(crate) onion_error_data: Option<Vec<u8>>,
 	}
@@ -1126,7 +1126,7 @@ where
 			Some(hop) => hop,
 			None => {
 				// Got an error from within a blinded route.
-				_error_code_ret = Some(BADONION | PERM | 24); // invalid_onion_blinding
+				_error_code_ret = Some(LocalHTLCFailureReason::InvalidOnionBlinding);
 				_error_packet_ret = Some(vec![0; 32]);
 				res = Some(FailureLearnings {
 					network_update: None,
@@ -1154,7 +1154,7 @@ where
 					// The failing hop is within a multi-hop blinded path.
 					#[cfg(not(test))]
 					{
-						_error_code_ret = Some(BADONION | PERM | 24); // invalid_onion_blinding
+						_error_code_ret = Some(LocalHTLCFailureReason::InvalidOnionBlinding);
 						_error_packet_ret = Some(vec![0; 32]);
 					}
 					#[cfg(test)]
@@ -1166,9 +1166,12 @@ where
 							&encrypted_packet.data,
 						))
 						.unwrap();
-						_error_code_ret = Some(u16::from_be_bytes(
-							err_packet.failuremsg.get(0..2).unwrap().try_into().unwrap(),
-						));
+						_error_code_ret = Some(
+							u16::from_be_bytes(
+								err_packet.failuremsg.get(0..2).unwrap().try_into().unwrap(),
+							)
+							.into(),
+						);
 						_error_packet_ret = Some(err_packet.failuremsg[2..].to_vec());
 					}
 
@@ -1279,22 +1282,19 @@ where
 			},
 		};
 
-		let error_code = u16::from_be_bytes(error_code_slice.try_into().expect("len is 2"));
+		let error_code = u16::from_be_bytes(error_code_slice.try_into().expect("len is 2")).into();
 		_error_code_ret = Some(error_code);
 		_error_packet_ret = Some(err_packet.failuremsg[2..].to_vec());
 
-		let (debug_field, debug_field_size) = errors::get_onion_debug_field(error_code);
+		let (debug_field, debug_field_size) = error_code.get_onion_debug_field();
 
 		// indicate that payment parameter has failed and no need to update Route object
-		let payment_failed = match error_code & 0xff {
-			15 | 18 | 19 | 23 => true,
-			_ => false,
-		} && is_from_final_non_blinded_node; // PERM bit observed below even if this error is from the intermediate nodes
+		let payment_failed = error_code.is_recipient_failure() && is_from_final_non_blinded_node;
 
 		let mut network_update = None;
 		let mut short_channel_id = None;
 
-		if error_code & BADONION == BADONION {
+		if error_code.is_badonion() {
 			// If the error code has the BADONION bit set, always blame the channel from the node
 			// "originating" the error to its next hop. The "originator" is ultimately actually claiming
 			// that its counterparty is the one who is failing the HTLC.
@@ -1308,12 +1308,13 @@ where
 					is_permanent: true,
 				});
 			}
-		} else if error_code & NODE == NODE {
-			let is_permanent = error_code & PERM == PERM;
-			network_update =
-				Some(NetworkUpdate::NodeFailure { node_id: *route_hop.pubkey(), is_permanent });
+		} else if error_code.is_node() {
+			network_update = Some(NetworkUpdate::NodeFailure {
+				node_id: *route_hop.pubkey(),
+				is_permanent: error_code.is_permanent(),
+			});
 			short_channel_id = route_hop.short_channel_id();
-		} else if error_code & PERM == PERM {
+		} else if error_code.is_permanent() {
 			if !payment_failed {
 				if let ErrorHop::RouteHop(failing_route_hop) = failing_route_hop {
 					network_update = Some(NetworkUpdate::ChannelFailure {
@@ -1323,7 +1324,7 @@ where
 				}
 				short_channel_id = failing_route_hop.short_channel_id();
 			}
-		} else if error_code & UPDATE == UPDATE {
+		} else if error_code.is_temporary() {
 			if let Some(update_len_slice) =
 				err_packet.failuremsg.get(debug_field_size + 2..debug_field_size + 4)
 			{
@@ -1357,8 +1358,9 @@ where
 		} else if payment_failed {
 			// Only blame the hop when a value in the HTLC doesn't match the corresponding value in the
 			// onion.
-			short_channel_id = match error_code & 0xff {
-				18 | 19 => route_hop.short_channel_id(),
+			short_channel_id = match error_code {
+				LocalHTLCFailureReason::FinalIncorrectCLTVExpiry
+				| LocalHTLCFailureReason::FinalIncorrectHTLCAmount => route_hop.short_channel_id(),
 				_ => None,
 			};
 		} else {
@@ -1374,30 +1376,27 @@ where
 		res = Some(FailureLearnings {
 			network_update,
 			short_channel_id,
-			payment_failed_permanently: error_code & PERM == PERM && is_from_final_non_blinded_node,
+			payment_failed_permanently: error_code.is_permanent() && is_from_final_non_blinded_node,
 			failed_within_blinded_path: false,
 		});
 
-		let (description, title) = errors::get_onion_error_description(error_code);
 		if debug_field_size > 0 && err_packet.failuremsg.len() >= 4 + debug_field_size {
 			log_info!(
 				logger,
-				"Onion Error[from {}: {}({:#x}) {}({})] {}",
+				"Onion Error[from {}: {:?}({:#x}) {}({})]",
 				route_hop.pubkey(),
-				title,
 				error_code,
+				error_code.failure_code(),
 				debug_field,
 				log_bytes!(&err_packet.failuremsg[4..4 + debug_field_size]),
-				description
 			);
 		} else {
 			log_info!(
 				logger,
-				"Onion Error[from {}: {}({:#x})] {}",
+				"Onion Error[from {}: {:?}({:#x})]",
 				route_hop.pubkey(),
-				title,
 				error_code,
-				description
+				error_code.failure_code(),
 			);
 		}
 
@@ -1651,14 +1650,45 @@ impl LocalHTLCFailureReason {
 		}
 	}
 
+	/// Returns the name of an error's data field and its expected length.
+	fn get_onion_debug_field(&self) -> (&'static str, usize) {
+		match self {
+			Self::InvalidOnionVersion | Self::InvalidOnionHMAC | Self::InvalidOnionKey => {
+				("sha256_of_onion", 32)
+			},
+			Self::AmountBelowMinimum | Self::FeeInsufficient => ("htlc_msat", 8),
+			Self::IncorrectCLTVExpiry | Self::FinalIncorrectCLTVExpiry => ("cltv_expiry", 4),
+			Self::FinalIncorrectHTLCAmount => ("incoming_htlc_msat", 8),
+			Self::ChannelDisabled => ("flags", 2),
+			_ => ("", 0),
+		}
+	}
+
 	pub(super) fn is_temporary(&self) -> bool {
 		self.failure_code() & UPDATE == UPDATE
 	}
 
-	#[cfg(test)]
 	pub(super) fn is_permanent(&self) -> bool {
 		self.failure_code() & PERM == PERM
 	}
+
+	fn is_badonion(&self) -> bool {
+		self.failure_code() & BADONION == BADONION
+	}
+
+	fn is_node(&self) -> bool {
+		self.failure_code() & NODE == NODE
+	}
+
+	/// Returns true if the failure is only sent by the final recipient. Note that this function
+	/// only checks [`LocalHTLCFailureReason`] variants that represent bolt 04 errors directly,
+	/// as it's intended to analyze errors we've received as a sender.
+	fn is_recipient_failure(&self) -> bool {
+		self.failure_code() == LocalHTLCFailureReason::IncorrectPaymentDetails.failure_code()
+			|| *self == LocalHTLCFailureReason::FinalIncorrectCLTVExpiry
+			|| *self == LocalHTLCFailureReason::FinalIncorrectHTLCAmount
+			|| *self == LocalHTLCFailureReason::MPPTimeout
+	}
 }
 
 impl From<u16> for LocalHTLCFailureReason {
@@ -2006,7 +2036,7 @@ impl HTLCFailReason {
 						failed_within_blinded_path: false,
 						hold_times: Vec::new(),
 						#[cfg(any(test, feature = "_test_utils"))]
-						onion_error_code: Some(failure_reason.failure_code()),
+						onion_error_code: Some(*failure_reason),
 						#[cfg(any(test, feature = "_test_utils"))]
 						onion_error_data: Some(data.clone()),
 					}
@@ -3093,7 +3123,7 @@ mod tests {
 					test_attributable_failure_packet_onion_with_mutation(Some(mutation));
 
 				if decrypted_failure.onion_error_code
-					== Some(LocalHTLCFailureReason::IncorrectPaymentDetails.failure_code())
+					== Some(LocalHTLCFailureReason::IncorrectPaymentDetails)
 				{
 					continue;
 				}
@@ -3109,7 +3139,7 @@ mod tests {
 		let decrypted_failure = test_attributable_failure_packet_onion_with_mutation(None);
 		assert_eq!(
 			decrypted_failure.onion_error_code,
-			Some(LocalHTLCFailureReason::IncorrectPaymentDetails.failure_code())
+			Some(LocalHTLCFailureReason::IncorrectPaymentDetails)
 		);
 		assert_eq!(decrypted_failure.hold_times, [5, 4, 3, 2, 1]);
 	}
@@ -3334,7 +3364,7 @@ mod tests {
 			);
 			assert_eq!(
 				decrypted_failure.onion_error_code,
-				Some(LocalHTLCFailureReason::IncorrectPaymentDetails.failure_code())
+				Some(LocalHTLCFailureReason::IncorrectPaymentDetails),
 			);
 		}
 
@@ -3380,7 +3410,7 @@ mod tests {
 
 				let decrypted_failure =
 					process_onion_failure(&secp_ctx, &logger, &htlc_source, first_hop_error_packet);
-				assert_eq!(decrypted_failure.onion_error_code, Some(error_code.failure_code()));
+				assert_eq!(decrypted_failure.onion_error_code, Some(error_code));
 			};
 
 			{
@@ -3411,7 +3441,7 @@ mod tests {
 					&htlc_source,
 					trampoline_outer_hop_error_packet,
 				);
-				assert_eq!(decrypted_failure.onion_error_code, Some(error_code.failure_code()));
+				assert_eq!(decrypted_failure.onion_error_code, Some(error_code));
 			};
 
 			{
@@ -3447,7 +3477,7 @@ mod tests {
 					&htlc_source,
 					trampoline_inner_hop_error_packet,
 				);
-				assert_eq!(decrypted_failure.onion_error_code, Some(error_code.failure_code()));
+				assert_eq!(decrypted_failure.onion_error_code, Some(error_code));
 			}
 
 			{
@@ -3488,7 +3518,7 @@ mod tests {
 					&htlc_source,
 					trampoline_second_hop_error_packet,
 				);
-				assert_eq!(decrypted_failure.onion_error_code, Some(error_code.failure_code()));
+				assert_eq!(decrypted_failure.onion_error_code, Some(error_code));
 			}
 		}
 	}

lightning/src/ln/outbound_payment.rs

@@ -2288,7 +2288,7 @@ impl OutboundPayments {
 					path: path.clone(),
 					short_channel_id,
 					#[cfg(any(test, feature = "_test_utils"))]
-					error_code: onion_error_code,
+					error_code: onion_error_code.map(|f| f.failure_code()),
 					#[cfg(any(test, feature = "_test_utils"))]
 					error_data: onion_error_data
 				}

lightning/src/util/errors.rs

@@ -101,50 +101,3 @@ impl_writeable_tlv_based_enum_upgradable!(APIError,
 	(8, MonitorUpdateInProgress) => {},
 	(10, IncompatibleShutdownScript) => { (0, script, required), },
 );
-
-#[inline]
-pub(crate) fn get_onion_debug_field(error_code: u16) -> (&'static str, usize) {
-	match error_code & 0xff {
-		4 | 5 | 6 => ("sha256_of_onion", 32),
-		11 | 12 => ("htlc_msat", 8),
-		13 | 18 => ("cltv_expiry", 4),
-		19 => ("incoming_htlc_msat", 8),
-		20 => ("flags", 2),
-		_ => ("", 0),
-	}
-}
-
-#[inline]
-pub(crate) fn get_onion_error_description(error_code: u16) -> (&'static str, &'static str) {
-	const BADONION: u16 = 0x8000;
-	const PERM: u16 = 0x4000;
-	const NODE: u16 = 0x2000;
-	const UPDATE: u16 = 0x1000;
-	match error_code {
-		_c if _c == PERM|1 => ("The realm byte was not understood by the processing node", "invalid_realm"),
-		_c if _c == NODE|2 => ("Node indicated temporary node failure", "temporary_node_failure"),
-		_c if _c == PERM|NODE|2 => ("Node indicated permanent node failure", "permanent_node_failure"),
-		_c if _c == PERM|NODE|3 => ("Node indicated the required node feature is missing in the onion", "required_node_feature_missing"),
-		_c if _c == BADONION|PERM|4 => ("Node indicated the version by is not understood", "invalid_onion_version"),
-		_c if _c == BADONION|PERM|5  => ("Node indicated the HMAC of the onion is incorrect", "invalid_onion_hmac"),
-		_c if _c == BADONION|PERM|6 => ("Node indicated the ephemeral public keys is not parseable", "invalid_onion_key"),
-		_c if _c == UPDATE|7 => ("Node indicated the outgoing channel is unable to handle the HTLC temporarily", "temporary_channel_failure"),
-		_c if _c == PERM|8 => ("Node indicated the outgoing channel is unable to handle the HTLC peramanently", "permanent_channel_failure"),
-		_c if _c == PERM|9 => ("Node indicated the required feature for the outgoing channel is not satisfied", "required_channel_feature_missing"),
-		_c if _c == PERM|10 => ("Node indicated the outbound channel is not found for the specified short_channel_id in the onion packet", "unknown_next_peer"),
-		_c if _c == UPDATE|11 => ("Node indicated the HTLC amount was below the required minmum for the outbound channel", "amount_below_minimum"),
-		_c if _c == UPDATE|12 => ("Node indicated the fee amount does not meet the required level", "fee_insufficient"),
-		_c if _c == UPDATE|13 => ("Node indicated the cltv_expiry does not comply with the cltv_expiry_delta required by the outgoing channel", "incorrect_cltv_expiry"),
-		_c if _c == UPDATE|14 => ("Node indicated the CLTV expiry too close to the current block height for safe handling", "expiry_too_soon"),
-		_c if _c == PERM|15 => ("The final node indicated the payment hash is unknown or amount is incorrect", "incorrect_or_unknown_payment_details"),
-		_c if _c == PERM|16 => ("The final node indicated the payment amount is incorrect", "incorrect_payment_amount"),
-		_c if _c == 17 => ("The final node indicated the CLTV expiry is too close to the current block height for safe handling", "final_expiry_too_soon"),
-		_c if _c == 18 => ("The final node indicated the CLTV expiry in the HTLC does not match the value in the onion", "final_incorrect_cltv_expiry"),
-		_c if _c == 19 => ("The final node indicated the amount in the HTLC does not match the value in the onion", "final_incorrect_htlc_amount"),
-		_c if _c == UPDATE|20 => ("Node indicated the outbound channel has been disabled", "channel_disabled"),
-		_c if _c == 21 => ("Node indicated the CLTV expiry in the HTLC is too far in the future", "expiry_too_far"),
-		_c if _c == PERM|22 => ("Node indicated that the decrypted onion per-hop payload was not understood by it or is incomplete", "invalid_onion_payload"),
-		_c if _c == 23 => ("The final node indicated the complete amount of the multi-part payment was not received within a reasonable time", "mpp_timeout"),
-		_ => ("Unknown", ""),
-	}
-}