From 7b317125572781a2fd04502a87f914a444c91821 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 20 Jul 2023 13:59:23 -0400 Subject: [PATCH] Struct-ify decoded onion failures To avoid several long hard-to-read tuple return values. --- lightning/src/ln/onion_utils.rs | 47 ++++++++++++++++++++++------ lightning/src/ln/outbound_payment.rs | 9 ++++-- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index 64a1e3fa3..d55077770 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -396,15 +396,22 @@ pub(super) fn build_first_hop_failure_packet(shared_secret: &[u8], failure_type: encrypt_failure_packet(shared_secret, &failure_packet.encode()[..]) } +pub(crate) struct DecodedOnionFailure { + pub(crate) network_update: Option, + pub(crate) short_channel_id: Option, + pub(crate) payment_retryable: bool, + #[cfg(test)] + pub(crate) onion_error_code: Option, + #[cfg(test)] + pub(crate) onion_error_data: Option>, +} + /// Process failure we got back from upstream on a payment we sent (implying htlc_source is an /// OutboundRoute). -/// Returns update, a boolean indicating that the payment itself failed, the short channel id of -/// the responsible channel, and the error code. #[inline] pub(super) fn process_onion_failure( secp_ctx: &Secp256k1, logger: &L, htlc_source: &HTLCSource, mut packet_decrypted: Vec -) -> (Option, Option, bool, Option, Option>) -where L::Target: Logger { +) -> DecodedOnionFailure where L::Target: Logger { let (path, session_priv, first_hop_htlc_msat) = if let &HTLCSource::OutboundRoute { ref path, ref session_priv, ref first_hop_htlc_msat, .. } = htlc_source { @@ -634,12 +641,24 @@ where L::Target: Logger { log_info!(logger, "Onion Error[from {}: {}({:#x})] {}", route_hop.pubkey, title, error_code, description); } }).expect("Route that we sent via spontaneously grew invalid keys in the middle of it?"); - if let Some((channel_update, short_channel_id, payment_retryable)) = res { - (channel_update, short_channel_id, payment_retryable, error_code_ret, error_packet_ret) + if let Some((network_update, short_channel_id, payment_retryable)) = res { + DecodedOnionFailure { + network_update, short_channel_id, payment_retryable, + #[cfg(test)] + onion_error_code: error_code_ret, + #[cfg(test)] + onion_error_data: error_packet_ret + } } else { // only not set either packet unparseable or hmac does not match with any // payment not retryable only when garbage is from the final node - (None, None, !is_from_final_node, None, None) + DecodedOnionFailure { + network_update: None, short_channel_id: None, payment_retryable: !is_from_final_node, + #[cfg(test)] + onion_error_code: None, + #[cfg(test)] + onion_error_data: None + } } } @@ -763,12 +782,12 @@ impl HTLCFailReason { pub(super) fn decode_onion_failure( &self, secp_ctx: &Secp256k1, logger: &L, htlc_source: &HTLCSource - ) -> (Option, Option, bool, Option, Option>) - where L::Target: Logger { + ) -> DecodedOnionFailure where L::Target: Logger { match self.0 { HTLCFailReasonRepr::LightningError { ref err } => { process_onion_failure(secp_ctx, logger, &htlc_source, err.data.clone()) }, + #[allow(unused)] HTLCFailReasonRepr::Reason { ref failure_code, ref data, .. } => { // we get a fail_malformed_htlc from the first hop // TODO: We'd like to generate a NetworkUpdate for temporary @@ -776,7 +795,15 @@ impl HTLCFailReason { // generally ignores its view of our own channels as we provide them via // ChannelDetails. if let &HTLCSource::OutboundRoute { ref path, .. } = htlc_source { - (None, Some(path.hops[0].short_channel_id), true, Some(*failure_code), Some(data.clone())) + DecodedOnionFailure { + network_update: None, + payment_retryable: true, + short_channel_id: Some(path.hops[0].short_channel_id), + #[cfg(test)] + onion_error_code: Some(*failure_code), + #[cfg(test)] + onion_error_data: Some(data.clone()), + } } else { unreachable!(); } } } diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 9c8d66458..d0851373b 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -17,7 +17,7 @@ use crate::sign::{EntropySource, NodeSigner, Recipient}; use crate::events::{self, PaymentFailureReason}; use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; use crate::ln::channelmanager::{ChannelDetails, EventCompletionAction, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId}; -use crate::ln::onion_utils::HTLCFailReason; +use crate::ln::onion_utils::{DecodedOnionFailure, HTLCFailReason}; use crate::routing::router::{InFlightHtlcs, Path, PaymentParameters, Route, RouteParameters, Router}; use crate::util::errors::APIError; use crate::util::logger::Logger; @@ -1293,9 +1293,12 @@ impl OutboundPayments { pending_events: &Mutex)>>, logger: &L, ) -> bool where L::Target: Logger { #[cfg(test)] - let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_error.decode_onion_failure(secp_ctx, logger, &source); + let DecodedOnionFailure { + network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data + } = onion_error.decode_onion_failure(secp_ctx, logger, &source); #[cfg(not(test))] - let (network_update, short_channel_id, payment_retryable, _, _) = onion_error.decode_onion_failure(secp_ctx, logger, &source); + let DecodedOnionFailure { network_update, short_channel_id, payment_retryable } = + onion_error.decode_onion_failure(secp_ctx, logger, &source); let payment_is_probe = payment_is_probe(payment_hash, &payment_id, probing_cookie_secret); let mut session_priv_bytes = [0; 32]; -- 2.39.5