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 <johncantrell97@gmail.com>
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 465256d21ea99bfa136062c8272c4dc34f65316b..b3eed7db4d2fb5ef776eb340fa93dc4e6cb12d93 100644 (file)
--- 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<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                                () => {
                                                        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<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                                        let mut fail_htlc_msgs = Vec::new();
                                                        for forward_info in pending_forwards.drain(..) {
                                                                match forward_info {
-                                                                       HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
-                                                                                       routing: PendingHTLCRouting::Forward {
-                                                                                               onion_packet, ..
-                                                                                       }, 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 {
+                                                                                       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<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                } else {
                                        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, .. },
-                                                                       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, ..
+                                                               }
+                                                       }) => {
                                                                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<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                                        PendingHTLCRouting::ReceiveKeysend { .. } => 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 14def2f6b7a3d9a133681da6bc67087a10700400..fb630abb9a88e71b9551f8e51a98707d77a9e9fb 100644 (file)
--- 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"),