From 582b827a4d3651aeb4002d4c357cdd8156e97c89 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 4 Nov 2022 12:28:36 -0400 Subject: [PATCH] Refactor HTLCForwardInfo::AddHTLC for intercept forwards In upcoming commit(s), we'll want to store intercepted HTLC forwards in ChannelManager before the user signals that they should be forwarded. It wouldn't make sense to store a HTLCForwardInfo as-is because the FailHTLC variant doesn't make sense, so we refactor out the ::AddHTLC contents into its own struct for storage. Co-authored-by: John Cantrell Co-authored-by: Valentine Wallace --- lightning/src/ln/channelmanager.rs | 85 +++++++++++++++------------ lightning/src/ln/onion_route_tests.rs | 18 +++--- 2 files changed, 58 insertions(+), 45 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 465256d2..b3eed7db 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -129,20 +129,22 @@ pub(super) enum PendingHTLCStatus { Fail(HTLCFailureMsg), } -pub(super) enum HTLCForwardInfo { - AddHTLC { - forward_info: PendingHTLCInfo, +pub(super) struct PendingAddHTLCInfo { + pub(super) forward_info: PendingHTLCInfo, + + // These fields are produced in `forward_htlcs()` and consumed in + // `process_pending_htlc_forwards()` for constructing the + // `HTLCSource::PreviousHopData` for failed and forwarded + // HTLCs. + // + // Note that this may be an outbound SCID alias for the associated channel. + prev_short_channel_id: u64, + prev_htlc_id: u64, + prev_funding_outpoint: OutPoint, +} - // These fields are produced in `forward_htlcs()` and consumed in - // `process_pending_htlc_forwards()` for constructing the - // `HTLCSource::PreviousHopData` for failed and forwarded - // HTLCs. - // - // Note that this may be an outbound SCID alias for the associated channel. - prev_short_channel_id: u64, - prev_htlc_id: u64, - prev_funding_outpoint: OutPoint, - }, +pub(super) enum HTLCForwardInfo { + AddHTLC(PendingAddHTLCInfo), FailHTLC { htlc_id: u64, err_packet: msgs::OnionErrorPacket, @@ -3149,9 +3151,13 @@ impl ChannelManager { for forward_info in pending_forwards.drain(..) { match forward_info { - HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo { - routing, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value }, - prev_funding_outpoint } => { + HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { + prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, + forward_info: PendingHTLCInfo { + routing, incoming_shared_secret, payment_hash, amt_to_forward, + outgoing_cltv_value + } + }) => { macro_rules! failure_handler { ($msg: expr, $err_code: expr, $err_data: expr, $phantom_ss: expr, $next_hop_unknown: expr) => { log_info!(self.logger, "Failed to accept/forward incoming HTLC: {}", $msg); @@ -3252,11 +3258,13 @@ impl ChannelManager { + HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { + prev_short_channel_id, prev_htlc_id, prev_funding_outpoint , + forward_info: PendingHTLCInfo { + incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value, + routing: PendingHTLCRouting::Forward { onion_packet, .. }, + }, + }) => { log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, log_bytes!(payment_hash.0), short_chan_id); let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id: prev_short_channel_id, @@ -3377,9 +3385,12 @@ impl ChannelManager { + HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { + prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, + forward_info: PendingHTLCInfo { + routing, incoming_shared_secret, payment_hash, amt_to_forward, .. + } + }) => { let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret) = match routing { PendingHTLCRouting::Receive { payment_data, incoming_cltv_expiry, phantom_shared_secret } => { let _legacy_hop_data = Some(payment_data.clone()); @@ -5089,12 +5100,12 @@ impl ChannelManager 0, }) { hash_map::Entry::Occupied(mut entry) => { - entry.get_mut().push(HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_funding_outpoint, - prev_htlc_id, forward_info }); + entry.get_mut().push(HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { + prev_short_channel_id, prev_funding_outpoint, prev_htlc_id, forward_info })); }, hash_map::Entry::Vacant(entry) => { - entry.insert(vec!(HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_funding_outpoint, - prev_htlc_id, forward_info })); + entry.insert(vec!(HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { + prev_short_channel_id, prev_funding_outpoint, prev_htlc_id, forward_info }))); } } } @@ -6681,18 +6692,20 @@ impl_writeable_tlv_based_enum!(HTLCFailReason, }, ;); +impl_writeable_tlv_based!(PendingAddHTLCInfo, { + (0, forward_info, required), + (2, prev_short_channel_id, required), + (4, prev_htlc_id, required), + (6, prev_funding_outpoint, required), +}); + impl_writeable_tlv_based_enum!(HTLCForwardInfo, - (0, AddHTLC) => { - (0, forward_info, required), - (2, prev_short_channel_id, required), - (4, prev_htlc_id, required), - (6, prev_funding_outpoint, required), - }, (1, FailHTLC) => { (0, htlc_id, required), (2, err_packet, required), - }, -;); + }; + (0, AddHTLC) +); impl_writeable_tlv_based!(PendingInboundPayment, { (0, payment_secret, required), diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 14def2f6..fb630abb 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -15,7 +15,7 @@ use crate::chain::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GR use crate::chain::keysinterface::{KeysInterface, Recipient}; use crate::ln::{PaymentHash, PaymentSecret}; use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS; -use crate::ln::channelmanager::{self, ChannelManager, ChannelManagerReadArgs, HTLCForwardInfo, CLTV_FAR_FAR_AWAY, MIN_CLTV_EXPIRY_DELTA, PendingHTLCInfo, PendingHTLCRouting, PaymentId}; +use crate::ln::channelmanager::{self, ChannelManager, ChannelManagerReadArgs, HTLCForwardInfo, CLTV_FAR_FAR_AWAY, MIN_CLTV_EXPIRY_DELTA, PendingAddHTLCInfo, PendingHTLCInfo, PendingHTLCRouting, PaymentId}; use crate::ln::onion_utils; use crate::routing::gossip::{NetworkUpdate, RoutingFees, NodeId}; use crate::routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop}; @@ -554,7 +554,7 @@ fn test_onion_failure() { for (_, pending_forwards) in nodes[1].node.forward_htlcs.lock().unwrap().iter_mut() { for f in pending_forwards.iter_mut() { match f { - &mut HTLCForwardInfo::AddHTLC { ref mut forward_info, .. } => + &mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { ref mut forward_info, .. }) => forward_info.outgoing_cltv_value += 1, _ => {}, } @@ -567,7 +567,7 @@ fn test_onion_failure() { for (_, pending_forwards) in nodes[1].node.forward_htlcs.lock().unwrap().iter_mut() { for f in pending_forwards.iter_mut() { match f { - &mut HTLCForwardInfo::AddHTLC { ref mut forward_info, .. } => + &mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { ref mut forward_info, .. }) => forward_info.amt_to_forward -= 1, _ => {}, } @@ -1035,12 +1035,12 @@ fn test_phantom_onion_hmac_failure() { let mut forward_htlcs = nodes[1].node.forward_htlcs.lock().unwrap(); let mut pending_forward = forward_htlcs.get_mut(&phantom_scid).unwrap(); match pending_forward[0] { - HTLCForwardInfo::AddHTLC { + HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { forward_info: PendingHTLCInfo { routing: PendingHTLCRouting::Forward { ref mut onion_packet, .. }, .. }, .. - } => { + }) => { onion_packet.hmac[onion_packet.hmac.len() - 1] ^= 1; Sha256::hash(&onion_packet.hop_data).into_inner().to_vec() }, @@ -1095,12 +1095,12 @@ fn test_phantom_invalid_onion_payload() { for (_, pending_forwards) in nodes[1].node.forward_htlcs.lock().unwrap().iter_mut() { for f in pending_forwards.iter_mut() { match f { - &mut HTLCForwardInfo::AddHTLC { + &mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { forward_info: PendingHTLCInfo { routing: PendingHTLCRouting::Forward { ref mut onion_packet, .. }, .. }, .. - } => { + }) => { // Construct the onion payloads for the entire route and an invalid amount. let height = nodes[0].best_block_info().1; let session_priv = SecretKey::from_slice(&session_priv).unwrap(); @@ -1166,9 +1166,9 @@ fn test_phantom_final_incorrect_cltv_expiry() { for (_, pending_forwards) in nodes[1].node.forward_htlcs.lock().unwrap().iter_mut() { for f in pending_forwards.iter_mut() { match f { - &mut HTLCForwardInfo::AddHTLC { + &mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { forward_info: PendingHTLCInfo { ref mut outgoing_cltv_value, .. }, .. - } => { + }) => { *outgoing_cltv_value += 1; }, _ => panic!("Unexpected forward"), -- 2.30.2