From 129e1f6be24485497990ad8c119f2b20f22861e8 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 4 Nov 2022 13:01:25 -0400 Subject: [PATCH 1/9] Persist pending intercepted htlcs in ChannelManager No htlcs are intercepted yet, that will be added in upcoming commit(s) Co-authored-by: John Cantrell Co-authored-by: Valentine Wallace --- lightning/src/ln/channelmanager.rs | 34 ++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d788356d63c..58db00e1407 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -207,6 +207,24 @@ impl Readable for PaymentId { Ok(PaymentId(buf)) } } + +/// An identifier used to uniquely identify an intercepted HTLC to LDK. +/// (C-not exported) as we just use [u8; 32] directly +#[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)] +pub struct InterceptId(pub [u8; 32]); + +impl Writeable for InterceptId { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + self.0.write(w) + } +} + +impl Readable for InterceptId { + fn read(r: &mut R) -> Result { + let buf: [u8; 32] = Readable::read(r)?; + Ok(InterceptId(buf)) + } +} /// Tracks the inbound corresponding to an outbound HTLC #[allow(clippy::derive_hash_xor_eq)] // Our Hash is faithful to the data, we just don't have SecretKey::hash #[derive(Clone, PartialEq, Eq)] @@ -751,6 +769,11 @@ pub struct ChannelManager pub(super) forward_htlcs: Mutex>>, #[cfg(not(test))] forward_htlcs: Mutex>>, + /// Storage for HTLCs that have been intercepted and bubbled up to the user. We hold them here + /// until the user tells us what we should do with them. + /// + /// See `ChannelManager` struct-level documentation for lock order requirements. + pending_intercepted_htlcs: Mutex>, /// Map from payment hash to the payment data and any HTLCs which are to us and can be /// failed/claimed by the user. @@ -1566,6 +1589,7 @@ impl ChannelManager Writeable for ChannelMana _ => {}, } } + + let mut pending_intercepted_htlcs = None; + let our_pending_intercepts = self.pending_intercepted_htlcs.lock().unwrap(); + if our_pending_intercepts.len() != 0 { + pending_intercepted_htlcs = Some(our_pending_intercepts); + } write_tlv_fields!(writer, { (1, pending_outbound_payments_no_retry, required), + (2, pending_intercepted_htlcs, option), (3, pending_outbound_payments, required), (5, self.our_network_pubkey, required), (7, self.fake_scid_rand_bytes, required), @@ -7306,12 +7337,14 @@ impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> // pending_outbound_payments_no_retry is for compatibility with 0.0.101 clients. let mut pending_outbound_payments_no_retry: Option>> = None; let mut pending_outbound_payments = None; + let mut pending_intercepted_htlcs: Option> = Some(HashMap::new()); let mut received_network_pubkey: Option = None; let mut fake_scid_rand_bytes: Option<[u8; 32]> = None; let mut probing_cookie_secret: Option<[u8; 32]> = None; let mut claimable_htlc_purposes = None; read_tlv_fields!(reader, { (1, pending_outbound_payments_no_retry, option), + (2, pending_intercepted_htlcs, option), (3, pending_outbound_payments, option), (5, received_network_pubkey, option), (7, fake_scid_rand_bytes, option), @@ -7534,6 +7567,7 @@ impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> inbound_payment_key: expanded_inbound_key, pending_inbound_payments: Mutex::new(pending_inbound_payments), pending_outbound_payments: Mutex::new(pending_outbound_payments.unwrap()), + pending_intercepted_htlcs: Mutex::new(pending_intercepted_htlcs.unwrap()), forward_htlcs: Mutex::new(forward_htlcs), claimable_htlcs: Mutex::new(claimable_htlcs), From 3a1268e177feaaea01a38a5d3f2475f5f770ae9d Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 4 Nov 2022 11:54:57 -0400 Subject: [PATCH 2/9] Add fake scid namespace for intercepted HTLCs This is useful for LSPs who wish to create a just-in-time channel for end users receiving a lightning payment. These fake scids will be encoded into route hints in end user invoices, and signal to LDK to create an event triggering the JIT channel, after which the payment will be received. Co-authored-by: John Cantrell Co-authored-by: Valentine Wallace --- lightning/src/util/scid_utils.rs | 33 ++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/lightning/src/util/scid_utils.rs b/lightning/src/util/scid_utils.rs index 651b36ef32c..1d951b8f936 100644 --- a/lightning/src/util/scid_utils.rs +++ b/lightning/src/util/scid_utils.rs @@ -63,6 +63,8 @@ pub fn scid_from_parts(block: u64, tx_index: u64, vout_index: u64) -> Result bool { let block_height = scid_utils::block_from_scid(&scid); let tx_index = scid_utils::tx_index_from_scid(&scid); @@ -160,11 +163,21 @@ pub(crate) mod fake_scid { && valid_vout == scid_utils::vout_from_scid(&scid) as u8 } + /// Returns whether the given fake scid falls into the intercept namespace. + pub fn is_valid_intercept(fake_scid_rand_bytes: &[u8; 32], scid: u64, genesis_hash: &BlockHash) -> bool { + let block_height = scid_utils::block_from_scid(&scid); + let tx_index = scid_utils::tx_index_from_scid(&scid); + let namespace = Namespace::Intercept; + let valid_vout = namespace.get_encrypted_vout(block_height, tx_index, fake_scid_rand_bytes); + block_height >= segwit_activation_height(genesis_hash) + && valid_vout == scid_utils::vout_from_scid(&scid) as u8 + } + #[cfg(test)] mod tests { use bitcoin::blockdata::constants::genesis_block; use bitcoin::network::constants::Network; - use crate::util::scid_utils::fake_scid::{is_valid_phantom, MAINNET_SEGWIT_ACTIVATION_HEIGHT, MAX_TX_INDEX, MAX_NAMESPACES, Namespace, NAMESPACE_ID_BITMASK, segwit_activation_height, TEST_SEGWIT_ACTIVATION_HEIGHT}; + use crate::util::scid_utils::fake_scid::{is_valid_intercept, is_valid_phantom, MAINNET_SEGWIT_ACTIVATION_HEIGHT, MAX_TX_INDEX, MAX_NAMESPACES, Namespace, NAMESPACE_ID_BITMASK, segwit_activation_height, TEST_SEGWIT_ACTIVATION_HEIGHT}; use crate::util::scid_utils; use crate::util::test_utils; use crate::sync::Arc; @@ -174,6 +187,10 @@ pub(crate) mod fake_scid { let phantom_namespace = Namespace::Phantom; assert!((phantom_namespace as u8) < MAX_NAMESPACES); assert!((phantom_namespace as u8) <= NAMESPACE_ID_BITMASK); + + let intercept_namespace = Namespace::Intercept; + assert!((intercept_namespace as u8) < MAX_NAMESPACES); + assert!((intercept_namespace as u8) <= NAMESPACE_ID_BITMASK); } #[test] @@ -203,6 +220,18 @@ pub(crate) mod fake_scid { assert!(!is_valid_phantom(&fake_scid_rand_bytes, invalid_fake_scid, &testnet_genesis)); } + #[test] + fn test_is_valid_intercept() { + let namespace = Namespace::Intercept; + let fake_scid_rand_bytes = [0; 32]; + let testnet_genesis = genesis_block(Network::Testnet).header.block_hash(); + let valid_encrypted_vout = namespace.get_encrypted_vout(0, 0, &fake_scid_rand_bytes); + let valid_fake_scid = scid_utils::scid_from_parts(1, 0, valid_encrypted_vout as u64).unwrap(); + assert!(is_valid_intercept(&fake_scid_rand_bytes, valid_fake_scid, &testnet_genesis)); + let invalid_fake_scid = scid_utils::scid_from_parts(1, 0, 12).unwrap(); + assert!(!is_valid_intercept(&fake_scid_rand_bytes, invalid_fake_scid, &testnet_genesis)); + } + #[test] fn test_get_fake_scid() { let mainnet_genesis = genesis_block(Network::Bitcoin).header.block_hash(); From 5efc1976cdd551fa8378108570aa2e40281bd5fb Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 4 Nov 2022 14:25:41 -0400 Subject: [PATCH 3/9] Add HTLCIntercepted event Used in upcoming commit(s) so users can intercept forwarded HTLCs Co-authored-by: John Cantrell Co-authored-by: Valentine Wallace --- lightning/src/util/events.rs | 71 +++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 6181340001b..04a3b1004bd 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -17,7 +17,7 @@ use crate::chain::keysinterface::SpendableOutputDescriptor; #[cfg(anchors)] use crate::ln::chan_utils::HTLCOutputInCommitment; -use crate::ln::channelmanager::PaymentId; +use crate::ln::channelmanager::{InterceptId, PaymentId}; use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS; use crate::ln::features::ChannelTypeFeatures; use crate::ln::msgs; @@ -288,6 +288,22 @@ pub enum BumpTransactionEvent { }, } +/// Will be used in [`Event::HTLCIntercepted`] to identify the next hop in the HTLC's path. +/// Currently only used in serialization for the sake of maintaining compatibility. More variants +/// will be added for general-purpose HTLC forward intercepts as well as trampoline forward +/// intercepts in upcoming work. +enum InterceptNextHop { + FakeScid { + requested_next_hop_scid: u64, + }, +} + +impl_writeable_tlv_based_enum!(InterceptNextHop, + (0, FakeScid) => { + (0, requested_next_hop_scid, required), + }; +); + /// An Event which you should probably take some action in response to. /// /// Note that while Writeable and Readable are implemented for Event, you probably shouldn't use @@ -585,6 +601,24 @@ pub enum Event { /// now + 5*time_forwardable). time_forwardable: Duration, }, + /// Used to indicate that we've intercepted an HTLC forward. + HTLCIntercepted { + /// An id to help LDK identify which HTLC is being forwarded or failed. + intercept_id: InterceptId, + /// The fake scid that was programmed as the next hop's scid. + requested_next_hop_scid: u64, + /// The payment hash used for this HTLC. + payment_hash: PaymentHash, + /// How many msats were received on the inbound edge of this HTLC. + inbound_amount_msat: u64, + /// How many msats the payer intended to route to the next node. Depending on the reason you are + /// intercepting this payment, you might take a fee by forwarding less than this amount. + /// + /// Note that LDK will NOT check that expected fees were factored into this value. You MUST + /// check that whatever fee you want has been included here or subtract it as required. Further, + /// LDK will not stop you from forwarding more than you received. + expected_outbound_amount_msat: u64, + }, /// Used to indicate that an output which you should know how to spend was confirmed on chain /// and is now spendable. /// Such an output will *not* ever be spent by rust-lightning, and are not at risk of your @@ -825,6 +859,17 @@ impl Writeable for Event { (0, WithoutLength(outputs), required), }); }, + &Event::HTLCIntercepted { requested_next_hop_scid, payment_hash, inbound_amount_msat, expected_outbound_amount_msat, intercept_id } => { + 6u8.write(writer)?; + let intercept_scid = InterceptNextHop::FakeScid { requested_next_hop_scid }; + write_tlv_fields!(writer, { + (0, intercept_id, required), + (2, intercept_scid, required), + (4, payment_hash, required), + (6, inbound_amount_msat, required), + (8, expected_outbound_amount_msat, required), + }); + } &Event::PaymentForwarded { fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id } => { 7u8.write(writer)?; write_tlv_fields!(writer, { @@ -1054,6 +1099,30 @@ impl MaybeReadable for Event { }; f() }, + 6u8 => { + let mut payment_hash = PaymentHash([0; 32]); + let mut intercept_id = InterceptId([0; 32]); + let mut requested_next_hop_scid = InterceptNextHop::FakeScid { requested_next_hop_scid: 0 }; + let mut inbound_amount_msat = 0; + let mut expected_outbound_amount_msat = 0; + read_tlv_fields!(reader, { + (0, intercept_id, required), + (2, requested_next_hop_scid, required), + (4, payment_hash, required), + (6, inbound_amount_msat, required), + (8, expected_outbound_amount_msat, required), + }); + let next_scid = match requested_next_hop_scid { + InterceptNextHop::FakeScid { requested_next_hop_scid: scid } => scid + }; + Ok(Some(Event::HTLCIntercepted { + payment_hash, + requested_next_hop_scid: next_scid, + inbound_amount_msat, + expected_outbound_amount_msat, + intercept_id, + })) + }, 7u8 => { let f = || { let mut fee_earned_msat = None; From 8fe7cbe9216975bfdb783af2b11203400e0a2d21 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 4 Nov 2022 16:23:47 -0400 Subject: [PATCH 4/9] Generate HTLCIntercepted event upon interceptable forward And store the pending intercepted HTLC in pending_intercepted_htlcs Co-authored-by: John Cantrell Co-authored-by: Valentine Wallace --- lightning/src/ln/channelmanager.rs | 88 +++++++++++++++++++++++++----- lightning/src/util/events.rs | 11 +++- 2 files changed, 83 insertions(+), 16 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 58db00e1407..d802318975e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -92,8 +92,8 @@ use core::ops::Deref; pub(super) enum PendingHTLCRouting { Forward { onion_packet: msgs::OnionPacket, - /// The SCID from the onion that we should forward to. This could be a "real" SCID, an - /// outbound SCID alias, or a phantom node SCID. + /// The SCID from the onion that we should forward to. This could be a real SCID or a fake one + /// generated using `get_fake_scid` from the scid_utils::fake_scid module. short_channel_id: u64, // This should be NonZero eventually when we bump MSRV }, Receive { @@ -684,6 +684,8 @@ pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, M, T, F, L> = ChannelManage // `total_consistency_lock` // | // |__`forward_htlcs` +// | | +// | |__`pending_intercepted_htlcs` // | // |__`pending_inbound_payments` // | | @@ -2230,8 +2232,10 @@ impl ChannelManager { // unknown_next_peer // Note that this is likely a timing oracle for detecting whether an scid is a - // phantom. - if fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, *short_channel_id, &self.genesis_hash) { + // phantom or an intercept. + if fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, *short_channel_id, &self.genesis_hash) || + fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, *short_channel_id, &self.genesis_hash) + { None } else { break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None)); @@ -5091,28 +5095,82 @@ impl ChannelManager)]) { for &mut (prev_short_channel_id, prev_funding_outpoint, prev_user_channel_id, ref mut pending_forwards) in per_source_pending_forwards { let mut forward_event = None; + let mut new_intercept_events = Vec::new(); + let mut failed_intercept_forwards = Vec::new(); if !pending_forwards.is_empty() { - let mut forward_htlcs = self.forward_htlcs.lock().unwrap(); - if forward_htlcs.is_empty() { - forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS)) - } for (forward_info, prev_htlc_id) in pending_forwards.drain(..) { - match forward_htlcs.entry(match forward_info.routing { - PendingHTLCRouting::Forward { short_channel_id, .. } => short_channel_id, - PendingHTLCRouting::Receive { .. } => 0, - PendingHTLCRouting::ReceiveKeysend { .. } => 0, - }) { + let scid = match forward_info.routing { + PendingHTLCRouting::Forward { short_channel_id, .. } => short_channel_id, + PendingHTLCRouting::Receive { .. } => 0, + PendingHTLCRouting::ReceiveKeysend { .. } => 0, + }; + // Pull this now to avoid introducing a lock order with `forward_htlcs`. + let is_our_scid = self.short_to_chan_info.read().unwrap().contains_key(&scid); + + let mut forward_htlcs = self.forward_htlcs.lock().unwrap(); + let forward_htlcs_empty = forward_htlcs.is_empty(); + match forward_htlcs.entry(scid) { hash_map::Entry::Occupied(mut entry) => { entry.get_mut().push(HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { prev_short_channel_id, prev_funding_outpoint, prev_htlc_id, prev_user_channel_id, forward_info })); }, hash_map::Entry::Vacant(entry) => { - entry.insert(vec!(HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { - prev_short_channel_id, prev_funding_outpoint, prev_htlc_id, prev_user_channel_id, forward_info }))); + if !is_our_scid && forward_info.incoming_amt_msat.is_some() && + fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, scid, &self.genesis_hash) + { + let intercept_id = InterceptId(Sha256::hash(&forward_info.incoming_shared_secret).into_inner()); + let mut pending_intercepts = self.pending_intercepted_htlcs.lock().unwrap(); + match pending_intercepts.entry(intercept_id) { + hash_map::Entry::Vacant(entry) => { + new_intercept_events.push(events::Event::HTLCIntercepted { + requested_next_hop_scid: scid, + payment_hash: forward_info.payment_hash, + inbound_amount_msat: forward_info.incoming_amt_msat.unwrap(), + expected_outbound_amount_msat: forward_info.outgoing_amt_msat, + intercept_id + }); + entry.insert(PendingAddHTLCInfo { + prev_short_channel_id, prev_funding_outpoint, prev_htlc_id, prev_user_channel_id, forward_info }); + }, + hash_map::Entry::Occupied(_) => { + log_info!(self.logger, "Failed to forward incoming HTLC: detected duplicate intercepted payment over short channel id {}", scid); + let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData { + short_channel_id: prev_short_channel_id, + outpoint: prev_funding_outpoint, + htlc_id: prev_htlc_id, + incoming_packet_shared_secret: forward_info.incoming_shared_secret, + phantom_shared_secret: None, + }); + + failed_intercept_forwards.push((htlc_source, forward_info.payment_hash, + HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() }, + HTLCDestination::InvalidForward { requested_forward_scid: scid }, + )); + } + } + } else { + // We don't want to generate a PendingHTLCsForwardable event if only intercepted + // payments are being processed. + if forward_htlcs_empty { + forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS)); + } + entry.insert(vec!(HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { + prev_short_channel_id, prev_funding_outpoint, prev_htlc_id, prev_user_channel_id, forward_info }))); + } } } } } + + for (htlc_source, payment_hash, failure_reason, destination) in failed_intercept_forwards.drain(..) { + self.fail_htlc_backwards_internal(htlc_source, &payment_hash, failure_reason, destination); + } + + if !new_intercept_events.is_empty() { + let mut events = self.pending_events.lock().unwrap(); + events.append(&mut new_intercept_events); + } + match forward_event { Some(time) => { let mut pending_events = self.pending_events.lock().unwrap(); diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 04a3b1004bd..268f9c9088c 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -182,6 +182,12 @@ pub enum HTLCDestination { /// Short channel id we are requesting to forward an HTLC to. requested_forward_scid: u64, }, + /// We couldn't forward to the outgoing scid. An example would be attempting to send a duplicate + /// intercept HTLC. + InvalidForward { + /// Short channel id we are requesting to forward an HTLC to. + requested_forward_scid: u64 + }, /// Failure scenario where an HTLC may have been forwarded to be intended for us, /// but is invalid for some reason, so we reject it. /// @@ -200,12 +206,15 @@ impl_writeable_tlv_based_enum_upgradable!(HTLCDestination, (0, node_id, required), (2, channel_id, required), }, + (1, InvalidForward) => { + (0, requested_forward_scid, required), + }, (2, UnknownNextHop) => { (0, requested_forward_scid, required), }, (4, FailedPayment) => { (0, payment_hash, required), - } + }, ); #[cfg(anchors)] From c1f1b78ea6fcfb72686fd4be34ddb09931b706af Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 6 Nov 2022 16:06:44 -0500 Subject: [PATCH 5/9] Utils for forwarding intercepted htlcs + getting intercept scids See ChannelManager::forward_intercepted_htlc and ChannelManager::get_intercept_scid for details Co-authored-by: John Cantrell Co-authored-by: Valentine Wallace --- lightning/src/ln/channelmanager.rs | 68 +++++++++++++++ lightning/src/ln/payment_tests.rs | 128 ++++++++++++++++++++++++++++- lightning/src/util/events.rs | 11 ++- 3 files changed, 203 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d802318975e..de86a8e703c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3051,6 +3051,57 @@ impl ChannelManager Result<(), APIError> { + let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); + + let next_hop_scid = match self.channel_state.lock().unwrap().by_id.get(next_hop_channel_id) { + Some(chan) => chan.get_short_channel_id().unwrap_or(chan.outbound_scid_alias()), + None => return Err(APIError::APIMisuseError { + err: format!("Channel with id {:?} not found", next_hop_channel_id) + }) + }; + + let payment = self.pending_intercepted_htlcs.lock().unwrap().remove(&intercept_id) + .ok_or_else(|| APIError::APIMisuseError { + err: format!("Payment with intercept id {:?} not found", intercept_id.0) + })?; + + let routing = match payment.forward_info.routing { + PendingHTLCRouting::Forward { onion_packet, .. } => { + PendingHTLCRouting::Forward { onion_packet, short_channel_id: next_hop_scid } + }, + _ => unreachable!() // Only `PendingHTLCRouting::Forward`s are intercepted + }; + let pending_htlc_info = PendingHTLCInfo { + outgoing_amt_msat: amt_to_forward_msat, routing, ..payment.forward_info + }; + + let mut per_source_pending_forward = [( + payment.prev_short_channel_id, + payment.prev_funding_outpoint, + payment.prev_user_channel_id, + vec![(pending_htlc_info, payment.prev_htlc_id)] + )]; + self.forward_htlcs(&mut per_source_pending_forward); + Ok(()) + } + /// Processes HTLCs which are pending waiting on random forward delay. /// /// Should only really ever be called in response to a PendingHTLCsForwardable event. @@ -5772,6 +5823,23 @@ impl ChannelManager u64 { + let best_block_height = self.best_block.read().unwrap().height(); + let short_to_chan_info = self.short_to_chan_info.read().unwrap(); + loop { + let scid_candidate = fake_scid::Namespace::Intercept.get_fake_scid(best_block_height, &self.genesis_hash, &self.fake_scid_rand_bytes, &self.keys_manager); + // Ensure the generated scid doesn't conflict with a real channel. + if short_to_chan_info.contains_key(&scid_candidate) { continue } + return scid_candidate + } + } + /// Gets inflight HTLC information by processing pending outbound payments that are in /// our channels. May be used during pathfinding to account for in-use channel liquidity. pub fn compute_inflight_htlcs(&self) -> InFlightHtlcs { diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index baaa60a7a6a..4b2ed1f0d8e 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -16,10 +16,11 @@ use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS use crate::chain::transaction::OutPoint; use crate::chain::keysinterface::KeysInterface; use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS; -use crate::ln::channelmanager::{self, BREAKDOWN_TIMEOUT, ChannelManager, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS}; +use crate::ln::channelmanager::{self, BREAKDOWN_TIMEOUT, ChannelManager, InterceptId, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS}; use crate::ln::msgs; use crate::ln::msgs::ChannelMessageHandler; -use crate::routing::router::{PaymentParameters, get_route}; +use crate::routing::gossip::RoutingFees; +use crate::routing::router::{find_route, get_route, PaymentParameters, RouteHint, RouteHintHop, RouteParameters}; use crate::util::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider}; use crate::util::test_utils; use crate::util::errors::APIError; @@ -1371,3 +1372,126 @@ fn test_holding_cell_inflight_htlcs() { // Clear pending events so test doesn't throw a "Had excess message on node..." error nodes[0].node.get_and_clear_pending_msg_events(); } + +#[test] +fn forward_intercepted_payment() { + // Test that detecting an intercept scid on payment forward will signal LDK to generate an + // intercept event, which the LSP can then use to open a JIT channel to forward the payment. + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let mut chan_config = test_default_channel_config(); + chan_config.manually_accept_inbound_channels = true; + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, Some(chan_config)]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + let scorer = test_utils::TestScorer::with_penalty(0); + let random_seed_bytes = chanmon_cfgs[0].keys_manager.get_secure_random_bytes(); + + let _ = create_announced_chan_between_nodes(&nodes, 0, 1, channelmanager::provided_init_features(), channelmanager::provided_init_features()).2; + + let amt_msat = 100_000; + let intercept_scid = nodes[1].node.get_intercept_scid(); + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + .with_route_hints(vec![ + RouteHint(vec![RouteHintHop { + src_node_id: nodes[1].node.get_our_node_id(), + short_channel_id: intercept_scid, + fees: RoutingFees { + base_msat: 1000, + proportional_millionths: 0, + }, + cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA, + htlc_minimum_msat: None, + htlc_maximum_msat: None, + }]) + ]) + .with_features(channelmanager::provided_invoice_features()); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + let route = find_route( + &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph, None, nodes[0].logger, + &scorer, &random_seed_bytes + ).unwrap(); + + let (payment_hash, payment_secret) = nodes[2].node.create_inbound_payment(Some(amt_msat), 60 * 60).unwrap(); + nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)).unwrap(); + let payment_event = { + { + let mut added_monitors = nodes[0].chain_monitor.added_monitors.lock().unwrap(); + assert_eq!(added_monitors.len(), 1); + added_monitors.clear(); + } + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + SendEvent::from_event(events.remove(0)) + }; + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]); + commitment_signed_dance!(nodes[1], nodes[0], &payment_event.commitment_msg, false, true); + + // Check that we generate the PaymentIntercepted event when an intercept forward is detected. + let events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + let (intercept_id, expected_outbound_amount_msat) = match events[0] { + crate::util::events::Event::HTLCIntercepted { + intercept_id, expected_outbound_amount_msat, payment_hash: pmt_hash, inbound_amount_msat, requested_next_hop_scid: short_channel_id + } => { + assert_eq!(pmt_hash, payment_hash); + assert_eq!(inbound_amount_msat, route.get_total_amount() + route.get_total_fees()); + assert_eq!(short_channel_id, intercept_scid); + (intercept_id, expected_outbound_amount_msat) + }, + _ => panic!() + }; + + // Check for unknown channel id error. + let unknown_chan_id_err = nodes[1].node.forward_intercepted_htlc(intercept_id, &[42; 32], nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err(); + assert_eq!(unknown_chan_id_err , APIError::APIMisuseError { err: format!("Channel with id {:?} not found", [42; 32]) }); + + // Open the just-in-time channel so the payment can then be forwarded. + let (_, channel_id) = open_zero_conf_channel(&nodes[1], &nodes[2], None); + + // Check for unknown intercept id error. + let unknown_intercept_id = InterceptId([42; 32]); + let unknown_intercept_id_err = nodes[1].node.forward_intercepted_htlc(unknown_intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err(); + assert_eq!(unknown_intercept_id_err , APIError::APIMisuseError { err: format!("Payment with intercept id {:?} not found", unknown_intercept_id.0) }); + + // Finally, forward the intercepted payment through and claim it. + nodes[1].node.forward_intercepted_htlc(intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap(); + expect_pending_htlcs_forwardable!(nodes[1]); + + let payment_event = { + { + let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); + assert_eq!(added_monitors.len(), 1); + added_monitors.clear(); + } + let mut events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + SendEvent::from_event(events.remove(0)) + }; + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]); + commitment_signed_dance!(nodes[2], nodes[1], &payment_event.commitment_msg, false, true); + expect_pending_htlcs_forwardable!(nodes[2]); + + let payment_preimage = nodes[2].node.get_payment_preimage(payment_hash, payment_secret).unwrap(); + expect_payment_received!(&nodes[2], payment_hash, payment_secret, amt_msat, Some(payment_preimage), nodes[2].node.get_our_node_id()); + do_claim_payment_along_route(&nodes[0], &vec!(&vec!(&nodes[1], &nodes[2])[..]), false, payment_preimage); + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2); + match events[0] { + Event::PaymentSent { payment_preimage: ref ev_preimage, payment_hash: ref ev_hash, ref fee_paid_msat, .. } => { + assert_eq!(payment_preimage, *ev_preimage); + assert_eq!(payment_hash, *ev_hash); + assert_eq!(fee_paid_msat, &Some(1000)); + }, + _ => panic!("Unexpected event") + } + match events[1] { + Event::PaymentPathSuccessful { payment_hash: hash, .. } => { + assert_eq!(hash, Some(payment_hash)); + }, + _ => panic!("Unexpected event") + } +} diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 268f9c9088c..d7e7bb1e37b 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -610,11 +610,18 @@ pub enum Event { /// now + 5*time_forwardable). time_forwardable: Duration, }, - /// Used to indicate that we've intercepted an HTLC forward. + /// Used to indicate that we've intercepted an HTLC forward. This event will only be generated if + /// you've encoded an intercept scid in the receiver's invoice route hints using + /// [`ChannelManager::get_intercept_scid`]. + /// + /// [`ChannelManager::get_intercept_scid`]: crate::ln::channelmanager::ChannelManager::get_intercept_scid HTLCIntercepted { /// An id to help LDK identify which HTLC is being forwarded or failed. intercept_id: InterceptId, - /// The fake scid that was programmed as the next hop's scid. + /// The fake scid that was programmed as the next hop's scid, generated using + /// [`ChannelManager::get_intercept_scid`]. + /// + /// [`ChannelManager::get_intercept_scid`]: crate::ln::channelmanager::ChannelManager::get_intercept_scid requested_next_hop_scid: u64, /// The payment hash used for this HTLC. payment_hash: PaymentHash, From f79ad2efb161d8ea9ccda9d67cf0ee2d63586cbe Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 7 Nov 2022 11:16:49 -0500 Subject: [PATCH 6/9] Allow failing back intercepted HTLCs Co-authored-by: John Cantrell Co-authored-by: Valentine Wallace --- lightning/src/ln/channelmanager.rs | 32 +++++++- lightning/src/ln/payment_tests.rs | 116 ++++++++++++++++++----------- lightning/src/util/events.rs | 6 ++ 3 files changed, 108 insertions(+), 46 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index de86a8e703c..c4d90480eec 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3059,7 +3059,8 @@ impl ChannelManager ChannelManager Result<(), APIError> { + let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); + + let payment = self.pending_intercepted_htlcs.lock().unwrap().remove(&intercept_id) + .ok_or_else(|| APIError::APIMisuseError { + err: format!("Payment with InterceptId {:?} not found", intercept_id) + })?; + + if let PendingHTLCRouting::Forward { short_channel_id, .. } = payment.forward_info.routing { + let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData { + short_channel_id: payment.prev_short_channel_id, + outpoint: payment.prev_funding_outpoint, + htlc_id: payment.prev_htlc_id, + incoming_packet_shared_secret: payment.forward_info.incoming_shared_secret, + phantom_shared_secret: None, + }); + + let failure_reason = HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() }; + let destination = HTLCDestination::UnknownNextHop { requested_forward_scid: short_channel_id }; + self.fail_htlc_backwards_internal(htlc_source, &payment.forward_info.payment_hash, failure_reason, destination); + } else { unreachable!() } // Only `PendingHTLCRouting::Forward`s are intercepted + + Ok(()) + } + /// Processes HTLCs which are pending waiting on random forward delay. /// /// Should only really ever be called in response to a PendingHTLCsForwardable event. diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 4b2ed1f0d8e..59f6909b837 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -1374,9 +1374,15 @@ fn test_holding_cell_inflight_htlcs() { } #[test] -fn forward_intercepted_payment() { +fn intercepted_payment() { // Test that detecting an intercept scid on payment forward will signal LDK to generate an - // intercept event, which the LSP can then use to open a JIT channel to forward the payment. + // intercept event, which the LSP can then use to either (a) open a JIT channel to forward the + // payment or (b) fail the payment. + do_test_intercepted_payment(false); + do_test_intercepted_payment(true); +} + +fn do_test_intercepted_payment(fail_intercept: bool) { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut chan_config = test_default_channel_config(); @@ -1449,49 +1455,69 @@ fn forward_intercepted_payment() { let unknown_chan_id_err = nodes[1].node.forward_intercepted_htlc(intercept_id, &[42; 32], nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err(); assert_eq!(unknown_chan_id_err , APIError::APIMisuseError { err: format!("Channel with id {:?} not found", [42; 32]) }); - // Open the just-in-time channel so the payment can then be forwarded. - let (_, channel_id) = open_zero_conf_channel(&nodes[1], &nodes[2], None); - - // Check for unknown intercept id error. - let unknown_intercept_id = InterceptId([42; 32]); - let unknown_intercept_id_err = nodes[1].node.forward_intercepted_htlc(unknown_intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err(); - assert_eq!(unknown_intercept_id_err , APIError::APIMisuseError { err: format!("Payment with intercept id {:?} not found", unknown_intercept_id.0) }); - - // Finally, forward the intercepted payment through and claim it. - nodes[1].node.forward_intercepted_htlc(intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap(); - expect_pending_htlcs_forwardable!(nodes[1]); - - let payment_event = { - { - let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); - assert_eq!(added_monitors.len(), 1); - added_monitors.clear(); + if fail_intercept { + // Ensure we can fail the intercepted payment back. + nodes[1].node.fail_intercepted_htlc(intercept_id).unwrap(); + expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCDestination::UnknownNextHop { requested_forward_scid: intercept_scid }]); + nodes[1].node.process_pending_htlc_forwards(); + let update_fail = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + check_added_monitors!(&nodes[1], 1); + assert!(update_fail.update_fail_htlcs.len() == 1); + let fail_msg = update_fail.update_fail_htlcs[0].clone(); + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_msg); + commitment_signed_dance!(nodes[0], nodes[1], update_fail.commitment_signed, false); + + // Ensure the payment fails with the expected error. + let fail_conditions = PaymentFailedConditions::new() + .blamed_scid(intercept_scid) + .blamed_chan_closed(true) + .expected_htlc_error_data(0x4000 | 10, &[]); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions); + } else { + // Open the just-in-time channel so the payment can then be forwarded. + let (_, channel_id) = open_zero_conf_channel(&nodes[1], &nodes[2], None); + + // Check for unknown intercept id error. + let unknown_intercept_id = InterceptId([42; 32]); + let unknown_intercept_id_err = nodes[1].node.forward_intercepted_htlc(unknown_intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err(); + assert_eq!(unknown_intercept_id_err , APIError::APIMisuseError { err: format!("Payment with intercept id {:?} not found", unknown_intercept_id.0) }); + + // Finally, forward the intercepted payment through and claim it. + nodes[1].node.forward_intercepted_htlc(intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap(); + expect_pending_htlcs_forwardable!(nodes[1]); + + let payment_event = { + { + let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); + assert_eq!(added_monitors.len(), 1); + added_monitors.clear(); + } + let mut events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + SendEvent::from_event(events.remove(0)) + }; + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]); + commitment_signed_dance!(nodes[2], nodes[1], &payment_event.commitment_msg, false, true); + expect_pending_htlcs_forwardable!(nodes[2]); + + let payment_preimage = nodes[2].node.get_payment_preimage(payment_hash, payment_secret).unwrap(); + expect_payment_received!(&nodes[2], payment_hash, payment_secret, amt_msat, Some(payment_preimage), nodes[2].node.get_our_node_id()); + do_claim_payment_along_route(&nodes[0], &vec!(&vec!(&nodes[1], &nodes[2])[..]), false, payment_preimage); + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2); + match events[0] { + Event::PaymentSent { payment_preimage: ref ev_preimage, payment_hash: ref ev_hash, ref fee_paid_msat, .. } => { + assert_eq!(payment_preimage, *ev_preimage); + assert_eq!(payment_hash, *ev_hash); + assert_eq!(fee_paid_msat, &Some(1000)); + }, + _ => panic!("Unexpected event") + } + match events[1] { + Event::PaymentPathSuccessful { payment_hash: hash, .. } => { + assert_eq!(hash, Some(payment_hash)); + }, + _ => panic!("Unexpected event") } - let mut events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - SendEvent::from_event(events.remove(0)) - }; - nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]); - commitment_signed_dance!(nodes[2], nodes[1], &payment_event.commitment_msg, false, true); - expect_pending_htlcs_forwardable!(nodes[2]); - - let payment_preimage = nodes[2].node.get_payment_preimage(payment_hash, payment_secret).unwrap(); - expect_payment_received!(&nodes[2], payment_hash, payment_secret, amt_msat, Some(payment_preimage), nodes[2].node.get_our_node_id()); - do_claim_payment_along_route(&nodes[0], &vec!(&vec!(&nodes[1], &nodes[2])[..]), false, payment_preimage); - let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2); - match events[0] { - Event::PaymentSent { payment_preimage: ref ev_preimage, payment_hash: ref ev_hash, ref fee_paid_msat, .. } => { - assert_eq!(payment_preimage, *ev_preimage); - assert_eq!(payment_hash, *ev_hash); - assert_eq!(fee_paid_msat, &Some(1000)); - }, - _ => panic!("Unexpected event") - } - match events[1] { - Event::PaymentPathSuccessful { payment_hash: hash, .. } => { - assert_eq!(hash, Some(payment_hash)); - }, - _ => panic!("Unexpected event") } } diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index d7e7bb1e37b..e3e458cb900 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -614,7 +614,13 @@ pub enum Event { /// you've encoded an intercept scid in the receiver's invoice route hints using /// [`ChannelManager::get_intercept_scid`]. /// + /// [`ChannelManager::forward_intercepted_htlc`] or + /// [`ChannelManager::fail_intercepted_htlc`] MUST be called in response to this event. See + /// their docs for more information. + /// /// [`ChannelManager::get_intercept_scid`]: crate::ln::channelmanager::ChannelManager::get_intercept_scid + /// [`ChannelManager::forward_intercepted_htlc`]: crate::ln::channelmanager::ChannelManager::forward_intercepted_htlc + /// [`ChannelManager::fail_intercepted_htlc`]: crate::ln::channelmanager::ChannelManager::fail_intercepted_htlc HTLCIntercepted { /// An id to help LDK identify which HTLC is being forwarded or failed. intercept_id: InterceptId, From ddcd9b0463a923c16768e8a4c51ace83b0524ad3 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 14 Nov 2022 13:36:52 -0500 Subject: [PATCH 7/9] Add config knob for forwarding intercept payments --- lightning/src/ln/channelmanager.rs | 15 +++++++++------ lightning/src/ln/payment_tests.rs | 10 +++++++--- lightning/src/util/config.rs | 12 ++++++++++++ lightning/src/util/events.rs | 3 ++- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c4d90480eec..3b1bf7432fb 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2233,8 +2233,9 @@ impl ChannelManager { // unknown_next_peer // Note that this is likely a timing oracle for detecting whether an scid is a // phantom or an intercept. - if fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, *short_channel_id, &self.genesis_hash) || - fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, *short_channel_id, &self.genesis_hash) + if (self.default_configuration.accept_intercept_htlcs && + fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, *short_channel_id, &self.genesis_hash)) || + fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, *short_channel_id, &self.genesis_hash) { None } else { @@ -3057,14 +3058,16 @@ impl ChannelManager Date: Mon, 14 Nov 2022 15:05:37 -0500 Subject: [PATCH 8/9] Automatically fail intercepts back on timeout --- lightning/src/ln/channelmanager.rs | 30 +++++++++++++++- lightning/src/ln/payment_tests.rs | 55 ++++++++++++++++++++++++------ 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 3b1bf7432fb..f27ca53bcf0 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3067,6 +3067,9 @@ impl ChannelManager ChannelManager Result<(), APIError> { let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); @@ -6256,7 +6262,6 @@ where if height >= htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER { let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec(); htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(height)); - timed_out_htlcs.push((HTLCSource::PreviousHopData(htlc.prev_hop.clone()), payment_hash.clone(), HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data @@ -6266,6 +6271,29 @@ where }); !htlcs.is_empty() // Only retain this entry if htlcs has at least one entry. }); + + let mut intercepted_htlcs = self.pending_intercepted_htlcs.lock().unwrap(); + intercepted_htlcs.retain(|_, htlc| { + if height >= htlc.forward_info.outgoing_cltv_value - HTLC_FAIL_BACK_BUFFER { + let prev_hop_data = HTLCSource::PreviousHopData(HTLCPreviousHopData { + short_channel_id: htlc.prev_short_channel_id, + htlc_id: htlc.prev_htlc_id, + incoming_packet_shared_secret: htlc.forward_info.incoming_shared_secret, + phantom_shared_secret: None, + outpoint: htlc.prev_funding_outpoint, + }); + + let requested_forward_scid /* intercept scid */ = match htlc.forward_info.routing { + PendingHTLCRouting::Forward { short_channel_id, .. } => short_channel_id, + _ => unreachable!(), + }; + timed_out_htlcs.push((prev_hop_data, htlc.forward_info.payment_hash, + HTLCFailReason::Reason { failure_code: 0x2000 | 2, data: Vec::new() }, + HTLCDestination::InvalidForward { requested_forward_scid })); + log_trace!(self.logger, "Timing out intercepted HTLC with requested forward scid {}", requested_forward_scid); + false + } else { true } + }); } self.handle_init_event_channel_failures(failed_channels); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index a876b73b88a..ead5bcbd469 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -16,7 +16,7 @@ use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS use crate::chain::transaction::OutPoint; use crate::chain::keysinterface::KeysInterface; use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS; -use crate::ln::channelmanager::{self, BREAKDOWN_TIMEOUT, ChannelManager, InterceptId, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS}; +use crate::ln::channelmanager::{self, BREAKDOWN_TIMEOUT, ChannelManager, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS}; use crate::ln::msgs; use crate::ln::msgs::ChannelMessageHandler; use crate::routing::gossip::RoutingFees; @@ -1243,6 +1243,13 @@ fn abandoned_send_payment_idempotent() { claim_payment(&nodes[0], &[&nodes[1]], second_payment_preimage); } +#[derive(PartialEq)] +enum InterceptTest { + Forward, + Fail, + Timeout, +} + #[test] fn test_trivial_inflight_htlc_tracking(){ // In this test, we test three scenarios: @@ -1378,11 +1385,13 @@ fn intercepted_payment() { // Test that detecting an intercept scid on payment forward will signal LDK to generate an // intercept event, which the LSP can then use to either (a) open a JIT channel to forward the // payment or (b) fail the payment. - do_test_intercepted_payment(false); - do_test_intercepted_payment(true); + do_test_intercepted_payment(InterceptTest::Forward); + do_test_intercepted_payment(InterceptTest::Fail); + // Make sure that intercepted payments will be automatically failed back if too many blocks pass. + do_test_intercepted_payment(InterceptTest::Timeout); } -fn do_test_intercepted_payment(fail_intercept: bool) { +fn do_test_intercepted_payment(test: InterceptTest) { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); @@ -1459,7 +1468,7 @@ fn do_test_intercepted_payment(fail_intercept: bool) { let unknown_chan_id_err = nodes[1].node.forward_intercepted_htlc(intercept_id, &[42; 32], nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err(); assert_eq!(unknown_chan_id_err , APIError::APIMisuseError { err: format!("Channel with id {:?} not found", [42; 32]) }); - if fail_intercept { + if test == InterceptTest::Fail { // Ensure we can fail the intercepted payment back. nodes[1].node.fail_intercepted_htlc(intercept_id).unwrap(); expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCDestination::UnknownNextHop { requested_forward_scid: intercept_scid }]); @@ -1477,15 +1486,10 @@ fn do_test_intercepted_payment(fail_intercept: bool) { .blamed_chan_closed(true) .expected_htlc_error_data(0x4000 | 10, &[]); expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions); - } else { + } else if test == InterceptTest::Forward { // Open the just-in-time channel so the payment can then be forwarded. let (_, channel_id) = open_zero_conf_channel(&nodes[1], &nodes[2], None); - // Check for unknown intercept id error. - let unknown_intercept_id = InterceptId([42; 32]); - let unknown_intercept_id_err = nodes[1].node.forward_intercepted_htlc(unknown_intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err(); - assert_eq!(unknown_intercept_id_err , APIError::APIMisuseError { err: format!("Payment with intercept id {:?} not found", unknown_intercept_id.0) }); - // Finally, forward the intercepted payment through and claim it. nodes[1].node.forward_intercepted_htlc(intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap(); expect_pending_htlcs_forwardable!(nodes[1]); @@ -1523,5 +1527,34 @@ fn do_test_intercepted_payment(fail_intercept: bool) { }, _ => panic!("Unexpected event") } + } else if test == InterceptTest::Timeout { + let mut block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; + connect_block(&nodes[0], &block); + connect_block(&nodes[1], &block); + let block_count = 183; // find_route adds a random CLTV offset, so hardcode rather than summing consts + for _ in 0..block_count { + block.header.prev_blockhash = block.block_hash(); + connect_block(&nodes[0], &block); + connect_block(&nodes[1], &block); + } + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], vec![HTLCDestination::InvalidForward { requested_forward_scid: intercept_scid }]); + check_added_monitors!(nodes[1], 1); + let htlc_timeout_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + assert!(htlc_timeout_updates.update_add_htlcs.is_empty()); + assert_eq!(htlc_timeout_updates.update_fail_htlcs.len(), 1); + assert!(htlc_timeout_updates.update_fail_malformed_htlcs.is_empty()); + assert!(htlc_timeout_updates.update_fee.is_none()); + + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_timeout_updates.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[0], nodes[1], htlc_timeout_updates.commitment_signed, false); + expect_payment_failed!(nodes[0], payment_hash, false, 0x2000 | 2, []); + + // Check for unknown intercept id error. + let (_, channel_id) = open_zero_conf_channel(&nodes[1], &nodes[2], None); + let unknown_intercept_id_err = nodes[1].node.forward_intercepted_htlc(intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err(); + assert_eq!(unknown_intercept_id_err , APIError::APIMisuseError { err: format!("Payment with intercept id {:?} not found", intercept_id.0) }); } } From acff8f6353184211791e4049ce588fa935308467 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 22 Nov 2022 19:15:56 -0500 Subject: [PATCH 9/9] Don't forward HTLC intercepts over unestablished channels --- lightning/src/ln/channelmanager.rs | 9 ++++++++- lightning/src/ln/payment_tests.rs | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f27ca53bcf0..20014f799ee 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3078,7 +3078,14 @@ impl ChannelManager chan.get_short_channel_id().unwrap_or(chan.outbound_scid_alias()), + Some(chan) => { + if !chan.is_usable() { + return Err(APIError::APIMisuseError { + err: format!("Channel with id {:?} not fully established", next_hop_channel_id) + }) + } + chan.get_short_channel_id().unwrap_or(chan.outbound_scid_alias()) + }, None => return Err(APIError::APIMisuseError { err: format!("Channel with id {:?} not found", next_hop_channel_id) }) diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index ead5bcbd469..2da80ae3f1c 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -1487,6 +1487,12 @@ fn do_test_intercepted_payment(test: InterceptTest) { .expected_htlc_error_data(0x4000 | 10, &[]); expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions); } else if test == InterceptTest::Forward { + // Check that we'll fail as expected when sending to a channel that isn't in `ChannelReady` yet. + let temp_chan_id = nodes[1].node.create_channel(nodes[2].node.get_our_node_id(), 100_000, 0, 42, None).unwrap(); + let unusable_chan_err = nodes[1].node.forward_intercepted_htlc(intercept_id, &temp_chan_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err(); + assert_eq!(unusable_chan_err , APIError::APIMisuseError { err: format!("Channel with id {:?} not fully established", temp_chan_id) }); + assert_eq!(nodes[1].node.get_and_clear_pending_msg_events().len(), 1); + // Open the just-in-time channel so the payment can then be forwarded. let (_, channel_id) = open_zero_conf_channel(&nodes[1], &nodes[2], None);