diff --git a/lightning-liquidity/tests/lsps2_integration_tests.rs b/lightning-liquidity/tests/lsps2_integration_tests.rs index e4ace27b715..c2e5f00ac19 100644 --- a/lightning-liquidity/tests/lsps2_integration_tests.rs +++ b/lightning-liquidity/tests/lsps2_integration_tests.rs @@ -38,6 +38,7 @@ use lightning::ln::peer_handler::CustomMessageHandler; use lightning::log_error; use lightning::routing::router::{RouteHint, RouteHintHop}; use lightning::sign::NodeSigner; +use lightning::util::config::HTLCInterceptionFlags; use lightning::util::errors::APIError; use lightning::util::logger::Logger; use lightning::util::test_utils::{TestBroadcaster, TestStore}; @@ -1157,7 +1158,7 @@ fn client_trusts_lsp_end_to_end_test() { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut service_node_config = test_default_channel_config(); - service_node_config.accept_intercept_htlcs = true; + service_node_config.htlc_interception_flags = HTLCInterceptionFlags::ToInterceptSCIDs as u8; let mut client_node_config = test_default_channel_config(); client_node_config.manually_accept_inbound_channels = true; @@ -1630,7 +1631,7 @@ fn late_payment_forwarded_and_safe_after_force_close_does_not_broadcast() { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut service_node_config = test_default_channel_config(); - service_node_config.accept_intercept_htlcs = true; + service_node_config.htlc_interception_flags = HTLCInterceptionFlags::ToInterceptSCIDs as u8; let mut client_node_config = test_default_channel_config(); client_node_config.manually_accept_inbound_channels = true; @@ -1821,7 +1822,7 @@ fn htlc_timeout_before_client_claim_results_in_handling_failed() { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut service_node_config = test_default_channel_config(); - service_node_config.accept_intercept_htlcs = true; + service_node_config.htlc_interception_flags = HTLCInterceptionFlags::ToInterceptSCIDs as u8; let mut client_node_config = test_default_channel_config(); client_node_config.manually_accept_inbound_channels = true; @@ -2157,7 +2158,7 @@ fn client_trusts_lsp_partial_fee_does_not_trigger_broadcast() { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut service_node_config = test_default_channel_config(); - service_node_config.accept_intercept_htlcs = true; + service_node_config.htlc_interception_flags = HTLCInterceptionFlags::ToInterceptSCIDs as u8; let mut client_node_config = test_default_channel_config(); client_node_config.manually_accept_inbound_channels = true; diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index d97ae6097b6..277ce612494 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1250,28 +1250,29 @@ pub enum Event { short_channel_id: Option, }, /// 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`] and have set [`UserConfig::accept_intercept_htlcs`]. + /// you've set some flags on [`UserConfig::htlc_interception_flags`]. /// /// [`ChannelManager::forward_intercepted_htlc`] or - /// [`ChannelManager::fail_intercepted_htlc`] MUST be called in response to this event. See - /// their docs for more information. + /// [`ChannelManager::fail_intercepted_htlc`] MUST be called in response to this event in a + /// timely manner (i.e. within some number of seconds, not minutes). See their docs for more + /// information. /// /// # Failure Behavior and Persistence /// This event will eventually be replayed after failures-to-handle (i.e., the event handler /// returning `Err(ReplayEvent ())`) and will be persisted across restarts. /// - /// [`ChannelManager::get_intercept_scid`]: crate::ln::channelmanager::ChannelManager::get_intercept_scid - /// [`UserConfig::accept_intercept_htlcs`]: crate::util::config::UserConfig::accept_intercept_htlcs + /// [`UserConfig::htlc_interception_flags`]: crate::util::config::UserConfig::htlc_interception_flags /// [`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, - /// The fake scid that was programmed as the next hop's scid, generated using - /// [`ChannelManager::get_intercept_scid`]. + /// The SCID which was selected by the sender as the next hop. It may point to one of our + /// channels, an intercept SCID generated with [`ChannelManager::get_intercept_scid`], or + /// an unknown SCID if [`HTLCInterceptionFlags::ToUnknownSCIDs`] was selected. /// /// [`ChannelManager::get_intercept_scid`]: crate::ln::channelmanager::ChannelManager::get_intercept_scid + /// [`HTLCInterceptionFlags::ToUnknownSCIDs`]: crate::util::config::HTLCInterceptionFlags::ToUnknownSCIDs requested_next_hop_scid: u64, /// The payment hash used for this HTLC. payment_hash: PaymentHash, @@ -1282,7 +1283,8 @@ pub enum Event { /// Forwarding less than this amount may break compatibility with LDK versions prior to 0.0.116. /// /// 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, + /// check that whatever fee you want has been included here (by comparing with + /// [`Self::HTLCIntercepted::inbound_amount_msat`]) or subtract it as required. Further, /// LDK will not stop you from forwarding more than you received. expected_outbound_amount_msat: u64, }, diff --git a/lightning/src/ln/async_payments_tests.rs b/lightning/src/ln/async_payments_tests.rs index 1f1bb70714d..14430ce1c43 100644 --- a/lightning/src/ln/async_payments_tests.rs +++ b/lightning/src/ln/async_payments_tests.rs @@ -60,7 +60,7 @@ use crate::sign::NodeSigner; use crate::sync::Mutex; use crate::types::features::Bolt12InvoiceFeatures; use crate::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; -use crate::util::config::UserConfig; +use crate::util::config::{HTLCInterceptionFlags, UserConfig}; use crate::util::ser::Writeable; use bitcoin::constants::ChainHash; use bitcoin::network::Network; @@ -3042,7 +3042,7 @@ fn intercepted_hold_htlc() { recipient_cfg.channel_handshake_limits.force_announced_channel_preference = false; let mut lsp_cfg = test_default_channel_config(); - lsp_cfg.accept_intercept_htlcs = true; + lsp_cfg.htlc_interception_flags = HTLCInterceptionFlags::ToInterceptSCIDs as u8; lsp_cfg.accept_forwards_to_priv_channels = true; lsp_cfg.enable_htlc_hold = true; diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 7941a81f61e..47bb093f95e 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -32,7 +32,7 @@ use crate::routing::router::{ use crate::sign::{NodeSigner, PeerStorageKey, ReceiveAuthKey, Recipient}; use crate::types::features::{BlindedHopFeatures, ChannelFeatures, NodeFeatures}; use crate::types::payment::{PaymentHash, PaymentSecret}; -use crate::util::config::UserConfig; +use crate::util::config::{HTLCInterceptionFlags, UserConfig}; use crate::util::ser::{WithoutLength, Writeable}; use crate::util::test_utils::{self, bytes_from_hex, pubkey_from_hex, secret_from_hex}; use bitcoin::hex::DisplayHex; @@ -703,7 +703,8 @@ fn do_blinded_intercept_payment(intercept_node_fails: bool) { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut intercept_forwards_config = test_default_channel_config(); - intercept_forwards_config.accept_intercept_htlcs = true; + intercept_forwards_config.htlc_interception_flags = + HTLCInterceptionFlags::ToInterceptSCIDs as u8; let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(intercept_forwards_config), None]); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f2e8fa70e4f..f13e7cb75af 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -122,7 +122,9 @@ use crate::types::features::{ }; use crate::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; use crate::types::string::UntrustedString; -use crate::util::config::{ChannelConfig, ChannelConfigOverrides, ChannelConfigUpdate, UserConfig}; +use crate::util::config::{ + ChannelConfig, ChannelConfigOverrides, ChannelConfigUpdate, HTLCInterceptionFlags, UserConfig, +}; use crate::util::errors::APIError; use crate::util::logger::{Level, Logger, WithContext}; use crate::util::scid_utils::fake_scid; @@ -634,14 +636,6 @@ impl Readable for PaymentId { pub struct InterceptId(pub [u8; 32]); impl InterceptId { - /// This intercept id corresponds to an HTLC that will be forwarded on - /// [`ChannelManager::forward_intercepted_htlc`]. - fn from_incoming_shared_secret(ss: &[u8; 32]) -> Self { - Self(Sha256::hash(ss).to_byte_array()) - } - - /// This intercept id corresponds to an HTLC that will be forwarded on receipt of a - /// [`ReleaseHeldHtlc`] onion message. fn from_htlc_id_and_chan_id( htlc_id: u64, channel_id: &ChannelId, counterparty_node_id: &PublicKey, ) -> Self { @@ -2607,46 +2601,6 @@ where /// [`update_channel`]: chain::Watch::update_channel /// [`ChannelUpdate`]: msgs::ChannelUpdate /// [`read`]: ReadableArgs::read -// -// Lock order: -// The tree structure below illustrates the lock order requirements for the different locks of the -// `ChannelManager`. Locks can be held at the same time if they are on the same branch in the tree, -// and should then be taken in the order of the lowest to the highest level in the tree. -// Note that locks on different branches shall not be taken at the same time, as doing so will -// create a new lock order for those specific locks in the order they were taken. -// -// Lock order tree: -// -// `pending_offers_messages` -// -// `pending_async_payments_messages` -// -// `total_consistency_lock` -// | -// |__`forward_htlcs` -// | -// |__`pending_intercepted_htlcs` -// | -// |__`decode_update_add_htlcs` -// | -// |__`per_peer_state` -// | -// |__`claimable_payments` -// | -// |__`pending_outbound_payments` // This field's struct contains a map of pending outbounds -// | -// |__`peer_state` -// | -// |__`short_to_chan_info` -// | -// |__`outbound_scid_aliases` -// | -// |__`best_block` -// | -// |__`pending_events` -// | -// |__`pending_background_events` -// pub struct ChannelManager< M: Deref, T: Deref, @@ -5051,10 +5005,55 @@ where } } + fn forward_needs_intercept( + &self, outbound_chan: Option<&FundedChannel>, outgoing_scid: u64, + ) -> bool { + let intercept_flags = self.config.read().unwrap().htlc_interception_flags; + if let Some(chan) = outbound_chan { + if !chan.context.should_announce() { + if chan.context.is_connected() { + if intercept_flags & (HTLCInterceptionFlags::ToOnlinePrivateChannels as u8) != 0 + { + return true; + } + } else { + if intercept_flags & (HTLCInterceptionFlags::ToOfflinePrivateChannels as u8) + != 0 + { + return true; + } + } + } else { + if intercept_flags & (HTLCInterceptionFlags::ToPublicChannels as u8) != 0 { + return true; + } + } + } else { + if fake_scid::is_valid_intercept( + &self.fake_scid_rand_bytes, + outgoing_scid, + &self.chain_hash, + ) { + if intercept_flags & (HTLCInterceptionFlags::ToInterceptSCIDs as u8) != 0 { + return true; + } + } else if fake_scid::is_valid_phantom( + &self.fake_scid_rand_bytes, + outgoing_scid, + &self.chain_hash, + ) { + // Handled as a normal forward + } else if intercept_flags & (HTLCInterceptionFlags::ToUnknownSCIDs as u8) != 0 { + return true; + } + } + false + } + #[rustfmt::skip] fn can_forward_htlc_to_outgoing_channel( &self, chan: &mut FundedChannel, msg: &msgs::UpdateAddHTLC, next_packet: &NextPacketDetails - ) -> Result<(), LocalHTLCFailureReason> { + ) -> Result { if !chan.context.should_announce() && !self.config.read().unwrap().accept_forwards_to_priv_channels { @@ -5063,6 +5062,7 @@ where // we don't allow forwards outbound over them. return Err(LocalHTLCFailureReason::PrivateChannelForward); } + let intercepted; if let HopConnector::ShortChannelId(outgoing_scid) = next_packet.outgoing_connector { if chan.funding.get_channel_type().supports_scid_privacy() && outgoing_scid != chan.context.outbound_scid_alias() { // `option_scid_alias` (referred to in LDK as `scid_privacy`) means @@ -5070,6 +5070,7 @@ where // we don't have the channel here. return Err(LocalHTLCFailureReason::RealSCIDForward); } + intercepted = self.forward_needs_intercept(Some(chan), outgoing_scid); } else { return Err(LocalHTLCFailureReason::InvalidTrampolineForward); } @@ -5079,9 +5080,11 @@ where // around to doing the actual forward, but better to fail early if we can and // hopefully an attacker trying to path-trace payments cannot make this occur // on a small/per-node/per-channel scale. - if !chan.context.is_live() { + if !intercepted && !chan.context.is_live() { if !chan.context.is_enabled() { return Err(LocalHTLCFailureReason::ChannelDisabled); + } else if !chan.context.is_connected() { + return Err(LocalHTLCFailureReason::PeerOffline); } else { return Err(LocalHTLCFailureReason::ChannelNotReady); } @@ -5091,13 +5094,13 @@ where } chan.htlc_satisfies_config(msg, next_packet.outgoing_amt_msat, next_packet.outgoing_cltv_value)?; - Ok(()) + Ok(intercepted) } /// Executes a callback `C` that returns some value `X` on the channel found with the given /// `scid`. `None` is returned when the channel is not found. - fn do_funded_channel_callback) -> X>( - &self, scid: u64, callback: C, + fn do_funded_channel_callback) -> X>( + &self, scid: u64, mut callback: C, ) -> Option { let (counterparty_node_id, channel_id) = match self.short_to_chan_info.read().unwrap().get(&scid).cloned() { @@ -5117,37 +5120,58 @@ where } } - #[rustfmt::skip] - fn can_forward_htlc( - &self, msg: &msgs::UpdateAddHTLC, next_packet_details: &NextPacketDetails - ) -> Result<(), LocalHTLCFailureReason> { + fn can_forward_htlc_intercepted( + &self, msg: &msgs::UpdateAddHTLC, next_packet_details: &NextPacketDetails, + ) -> Result { let outgoing_scid = match next_packet_details.outgoing_connector { HopConnector::ShortChannelId(scid) => scid, HopConnector::Trampoline(_) => { return Err(LocalHTLCFailureReason::InvalidTrampolineForward); - } + }, }; - match self.do_funded_channel_callback(outgoing_scid, |chan: &mut FundedChannel| { - self.can_forward_htlc_to_outgoing_channel(chan, msg, next_packet_details) - }) { - Some(Ok(())) => {}, - Some(Err(e)) => return Err(e), - None => { - // If we couldn't find the channel info for the scid, it may be a phantom or - // intercept forward. - if (self.config.read().unwrap().accept_intercept_htlcs && - fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, outgoing_scid, &self.chain_hash)) || - fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, outgoing_scid, &self.chain_hash) - {} else { - return Err(LocalHTLCFailureReason::UnknownNextPeer); - } - } - } + // TODO: We do the fake SCID namespace check a bunch of times here (and indirectly via + // `forward_needs_intercept`, including as called in + // `can_forward_htlc_to_outgoing_channel`), we should find a way to reduce the number of + // times we do it. + let intercept = + match self.do_funded_channel_callback(outgoing_scid, |chan: &mut FundedChannel| { + self.can_forward_htlc_to_outgoing_channel(chan, msg, next_packet_details) + }) { + Some(Ok(intercept)) => intercept, + Some(Err(e)) => return Err(e), + None => { + // Perform basic sanity checks on the amounts and CLTV being forwarded + if next_packet_details.outgoing_amt_msat > msg.amount_msat { + return Err(LocalHTLCFailureReason::FeeInsufficient); + } + let cltv_delta = + msg.cltv_expiry.saturating_sub(next_packet_details.outgoing_cltv_value); + if cltv_delta < MIN_CLTV_EXPIRY_DELTA.into() { + return Err(LocalHTLCFailureReason::IncorrectCLTVExpiry); + } + + if fake_scid::is_valid_phantom( + &self.fake_scid_rand_bytes, + outgoing_scid, + &self.chain_hash, + ) { + false + } else if self.forward_needs_intercept(None, outgoing_scid) { + true + } else { + return Err(LocalHTLCFailureReason::UnknownNextPeer); + } + }, + }; let cur_height = self.best_block.read().unwrap().height + 1; - check_incoming_htlc_cltv(cur_height, next_packet_details.outgoing_cltv_value, msg.cltv_expiry)?; + check_incoming_htlc_cltv( + cur_height, + next_packet_details.outgoing_cltv_value, + msg.cltv_expiry, + )?; - Ok(()) + Ok(intercept) } #[rustfmt::skip] @@ -6873,11 +6897,8 @@ where /// Intercepted HTLCs can be useful for Lightning Service Providers (LSPs) to open a just-in-time /// channel to a receiving node if the node lacks sufficient inbound liquidity. /// - /// To make use of intercepted HTLCs, set [`UserConfig::accept_intercept_htlcs`] and use - /// [`ChannelManager::get_intercept_scid`] to generate short channel id(s) to put in the - /// receiver's invoice route hints. These route hints will signal to LDK to generate an - /// [`HTLCIntercepted`] event when it receives the forwarded HTLC, and this method or - /// [`ChannelManager::fail_intercepted_htlc`] MUST be called in response to the event. + /// To make use of intercepted HTLCs, set [`UserConfig::htlc_interception_flags`] must have a + /// non-0 value. /// /// Note that LDK does not enforce fee requirements in `amt_to_forward_msat`, and will not stop /// you from forwarding more than you received. See @@ -6887,7 +6908,7 @@ where /// Errors if the event was not handled in time, in which case the HTLC was automatically failed /// backwards. /// - /// [`UserConfig::accept_intercept_htlcs`]: crate::util::config::UserConfig::accept_intercept_htlcs + /// [`UserConfig::htlc_interception_flags`]: crate::util::config::UserConfig::htlc_interception_flags /// [`HTLCIntercepted`]: events::Event::HTLCIntercepted /// [`HTLCIntercepted::expected_outbound_amount_msat`]: events::Event::HTLCIntercepted::expected_outbound_amount_msat // TODO: when we move to deciding the best outbound channel at forward time, only take @@ -7114,6 +7135,22 @@ where }); let shared_secret = next_hop.shared_secret().secret_bytes(); + macro_rules! fail_htlc { + ($reason: expr) => {{ + let htlc_fail = self.htlc_failure_from_update_add_err( + &update_add_htlc, + &incoming_counterparty_node_id, + $reason, + is_intro_node_blinded_forward, + &shared_secret, + ); + let failure_type = + get_htlc_failure_type(outgoing_scid_opt, update_add_htlc.payment_hash); + htlc_fails.push((htlc_fail, failure_type, $reason.into())); + continue; + }}; + } + // Nodes shouldn't expect us to hold HTLCs for them if we don't advertise htlc_hold feature // support. // @@ -7126,18 +7163,7 @@ where if update_add_htlc.hold_htlc.is_some() && !BaseMessageHandler::provided_node_features(self).supports_htlc_hold() { - let reason = LocalHTLCFailureReason::TemporaryNodeFailure; - let htlc_fail = self.htlc_failure_from_update_add_err( - &update_add_htlc, - &incoming_counterparty_node_id, - reason, - is_intro_node_blinded_forward, - &shared_secret, - ); - let failure_type = - get_htlc_failure_type(outgoing_scid_opt, update_add_htlc.payment_hash); - htlc_fails.push((htlc_fail, failure_type, reason.into())); - continue; + fail_htlc!(LocalHTLCFailureReason::TemporaryNodeFailure); } // Process the HTLC on the incoming channel. @@ -7154,38 +7180,20 @@ where ) { Some(Ok(_)) => {}, Some(Err(reason)) => { - let htlc_fail = self.htlc_failure_from_update_add_err( - &update_add_htlc, - &incoming_counterparty_node_id, - reason, - is_intro_node_blinded_forward, - &shared_secret, - ); - let failure_type = - get_htlc_failure_type(outgoing_scid_opt, update_add_htlc.payment_hash); - htlc_fails.push((htlc_fail, failure_type, reason.into())); - continue; + fail_htlc!(reason); }, // The incoming channel no longer exists, HTLCs should be resolved onchain instead. None => continue 'outer_loop, } // Now process the HTLC on the outgoing channel if it's a forward. + let mut intercept_forward = false; if let Some(next_packet_details) = next_packet_details_opt.as_ref() { - if let Err(reason) = - self.can_forward_htlc(&update_add_htlc, next_packet_details) - { - let htlc_fail = self.htlc_failure_from_update_add_err( - &update_add_htlc, - &incoming_counterparty_node_id, - reason, - is_intro_node_blinded_forward, - &shared_secret, - ); - let failure_type = - get_htlc_failure_type(outgoing_scid_opt, update_add_htlc.payment_hash); - htlc_fails.push((htlc_fail, failure_type, reason.into())); - continue; + match self.can_forward_htlc_intercepted(&update_add_htlc, next_packet_details) { + Err(reason) => { + fail_htlc!(reason); + }, + Ok(intercept) => intercept_forward = intercept, } } @@ -7196,7 +7204,86 @@ where incoming_accept_underpaying_htlcs, next_packet_details_opt.map(|d| d.next_packet_pubkey), ) { - Ok(info) => htlc_forwards.push((info, update_add_htlc.htlc_id)), + Ok(info) => { + let to_pending_add = |info| PendingAddHTLCInfo { + prev_outbound_scid_alias: incoming_scid_alias, + prev_counterparty_node_id: incoming_counterparty_node_id, + prev_funding_outpoint: incoming_funding_txo, + prev_channel_id: incoming_channel_id, + prev_htlc_id: update_add_htlc.htlc_id, + prev_user_channel_id: incoming_user_channel_id, + forward_info: info, + }; + let intercept_id = || { + InterceptId::from_htlc_id_and_chan_id( + update_add_htlc.htlc_id, + &incoming_channel_id, + &incoming_counterparty_node_id, + ) + }; + let logger = WithContext::from( + &self.logger, + None, + Some(incoming_channel_id), + Some(update_add_htlc.payment_hash), + ); + if info.routing.should_hold_htlc() { + let mut held_htlcs = self.pending_intercepted_htlcs.lock().unwrap(); + let intercept_id = intercept_id(); + match held_htlcs.entry(intercept_id) { + hash_map::Entry::Vacant(entry) => { + log_debug!( + logger, + "Intercepted held HTLC with id {intercept_id}, holding until the recipient is online" + ); + let pending_add = to_pending_add(info); + entry.insert(pending_add); + }, + hash_map::Entry::Occupied(_) => { + debug_assert!(false, "Should never have two HTLCs with the same channel id and htlc id"); + log_error!(logger, "Duplicate intercept id for HTLC"); + fail_htlc!(LocalHTLCFailureReason::TemporaryNodeFailure); + }, + } + } else if intercept_forward { + let intercept_id = intercept_id(); + let mut pending_intercepts = + self.pending_intercepted_htlcs.lock().unwrap(); + match pending_intercepts.entry(intercept_id) { + hash_map::Entry::Vacant(entry) => { + let pending_add = to_pending_add(info); + if let Ok(intercept_ev) = + create_htlc_intercepted_event(intercept_id, &pending_add) + { + log_debug!( + logger, + "Intercepted HTLC, generating intercept event with ID {intercept_id}" + ); + let ev_entry = (intercept_ev, None); + self.pending_events.lock().unwrap().push_back(ev_entry); + entry.insert(pending_add); + } else { + debug_assert!(false); + log_error!( + logger, + "Failed to generate an intercept event for HTLC" + ); + fail_htlc!(LocalHTLCFailureReason::TemporaryNodeFailure); + } + }, + hash_map::Entry::Occupied(_) => { + log_error!( + logger, + "Failed to forward incoming HTLC: detected duplicate intercepted payment", + ); + debug_assert!(false, "Should never have two HTLCs with the same channel id and htlc id"); + fail_htlc!(LocalHTLCFailureReason::TemporaryNodeFailure); + }, + } + } else { + htlc_forwards.push((info, update_add_htlc.htlc_id)) + } + }, Err(inbound_err) => { let failure_type = get_htlc_failure_type(outgoing_scid_opt, update_add_htlc.payment_hash); @@ -7220,7 +7307,7 @@ where incoming_funding_txo, incoming_channel_id, incoming_user_channel_id, - htlc_forwards.drain(..).collect(), + htlc_forwards, ); self.forward_htlcs(&mut [pending_forwards]); for (htlc_fail, failure_type, failure_reason) in htlc_fails.drain(..) { @@ -11625,26 +11712,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ ref mut pending_forwards, ) in per_source_pending_forwards { - let mut new_intercept_events = VecDeque::new(); - let mut failed_intercept_forwards = Vec::new(); if !pending_forwards.is_empty() { for (forward_info, prev_htlc_id) in pending_forwards.drain(..) { let scid = match forward_info.routing { PendingHTLCRouting::Forward { short_channel_id, .. } => short_channel_id, - PendingHTLCRouting::TrampolineForward { .. } => 0, - PendingHTLCRouting::Receive { .. } => 0, - PendingHTLCRouting::ReceiveKeysend { .. } => 0, + PendingHTLCRouting::TrampolineForward { .. } + | PendingHTLCRouting::Receive { .. } + | 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 payment_hash = forward_info.payment_hash; - let logger = WithContext::from( - &self.logger, - None, - Some(prev_channel_id), - Some(payment_hash), - ); let pending_add = PendingAddHTLCInfo { prev_outbound_scid_alias, prev_counterparty_node_id, @@ -11654,112 +11730,17 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ prev_user_channel_id, forward_info, }; - let mut fail_intercepted_htlc = |pending_add: PendingAddHTLCInfo| { - let htlc_source = - HTLCSource::PreviousHopData(pending_add.htlc_previous_hop_data()); - let reason = HTLCFailReason::from_failure_code( - LocalHTLCFailureReason::UnknownNextPeer, - ); - let failure_type = HTLCHandlingFailureType::InvalidForward { - requested_forward_scid: scid, - }; - failed_intercept_forwards.push(( - htlc_source, - payment_hash, - reason, - failure_type, - )); - }; - // In the case that we have an HTLC that we're supposed to hold onto until the - // recipient comes online *and* the outbound scid is encoded as - // `fake_scid::is_valid_intercept`, we should first wait for the recipient to come - // online before generating an `HTLCIntercepted` event, since the event cannot be - // acted on until the recipient is online to cooperatively open the JIT channel. Once - // we receive the `ReleaseHeldHtlc` message from the recipient, we will circle back - // here and resume generating the event below. - if pending_add.forward_info.routing.should_hold_htlc() { - let intercept_id = InterceptId::from_htlc_id_and_chan_id( - prev_htlc_id, - &prev_channel_id, - &prev_counterparty_node_id, - ); - let mut held_htlcs = self.pending_intercepted_htlcs.lock().unwrap(); - match held_htlcs.entry(intercept_id) { - hash_map::Entry::Vacant(entry) => { - log_trace!( - logger, - "Intercepted held HTLC with id {}, holding until the recipient is online", - intercept_id - ); - entry.insert(pending_add); - }, - hash_map::Entry::Occupied(_) => { - debug_assert!(false, "Should never have two HTLCs with the same channel id and htlc id"); - fail_intercepted_htlc(pending_add); - }, - } - } else if !is_our_scid - && pending_add.forward_info.incoming_amt_msat.is_some() - && fake_scid::is_valid_intercept( - &self.fake_scid_rand_bytes, - scid, - &self.chain_hash, - ) { - let intercept_id = InterceptId::from_incoming_shared_secret( - &pending_add.forward_info.incoming_shared_secret, - ); - let mut pending_intercepts = self.pending_intercepted_htlcs.lock().unwrap(); - match pending_intercepts.entry(intercept_id) { - hash_map::Entry::Vacant(entry) => { - if let Ok(intercept_ev) = - create_htlc_intercepted_event(intercept_id, &pending_add) - { - new_intercept_events.push_back((intercept_ev, None)); - entry.insert(pending_add); - } else { - debug_assert!(false); - fail_intercepted_htlc(pending_add); - } - }, - hash_map::Entry::Occupied(_) => { - log_info!( - logger, - "Failed to forward incoming HTLC: detected duplicate intercepted payment over short channel id {}", - scid - ); - fail_intercepted_htlc(pending_add); - }, - } - } else { - match self.forward_htlcs.lock().unwrap().entry(scid) { - hash_map::Entry::Occupied(mut entry) => { - entry.get_mut().push(HTLCForwardInfo::AddHTLC(pending_add)); - }, - hash_map::Entry::Vacant(entry) => { - entry.insert(vec![HTLCForwardInfo::AddHTLC(pending_add)]); - }, - } + match self.forward_htlcs.lock().unwrap().entry(scid) { + hash_map::Entry::Occupied(mut entry) => { + entry.get_mut().push(HTLCForwardInfo::AddHTLC(pending_add)); + }, + hash_map::Entry::Vacant(entry) => { + entry.insert(vec![HTLCForwardInfo::AddHTLC(pending_add)]); + }, } } } - - 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, - None, - ); - } - - if !new_intercept_events.is_empty() { - let mut events = self.pending_events.lock().unwrap(); - events.append(&mut new_intercept_events); - } } } @@ -15951,6 +15932,7 @@ where prev_outbound_scid_alias, htlc_id, } => { + let _serialize_guard = PersistenceNotifierGuard::notify_on_drop(self); // It's possible the release_held_htlc message raced ahead of us transitioning the pending // update_add to `Self::pending_intercept_htlcs`. If that's the case, update the pending // update_add to indicate that the HTLC should be released immediately. @@ -15989,16 +15971,18 @@ where }, } }; - match htlc.forward_info.routing { - PendingHTLCRouting::Forward { ref mut hold_htlc, .. } => { + let next_hop_scid = match htlc.forward_info.routing { + PendingHTLCRouting::Forward { ref mut hold_htlc, short_channel_id, .. } => { debug_assert!(hold_htlc.is_some()); *hold_htlc = None; + short_channel_id }, _ => { debug_assert!(false, "HTLC intercepts can only be forwards"); + // Let the HTLC be auto-failed before it expires. return; }, - } + }; let logger = WithContext::from( &self.logger, @@ -16008,16 +15992,56 @@ where ); log_trace!(logger, "Releasing held htlc with intercept_id {}", intercept_id); - let mut per_source_pending_forward = [( - htlc.prev_outbound_scid_alias, - htlc.prev_counterparty_node_id, - htlc.prev_funding_outpoint, - htlc.prev_channel_id, - htlc.prev_user_channel_id, - vec![(htlc.forward_info, htlc.prev_htlc_id)], - )]; - self.forward_htlcs(&mut per_source_pending_forward); - PersistenceNotifierGuard::notify_on_drop(self); + let should_intercept = self + .do_funded_channel_callback(next_hop_scid, |chan| { + self.forward_needs_intercept(Some(chan), next_hop_scid) + }) + .unwrap_or_else(|| self.forward_needs_intercept(None, next_hop_scid)); + + if should_intercept { + let intercept_id = InterceptId::from_htlc_id_and_chan_id( + htlc.prev_htlc_id, + &htlc.prev_channel_id, + &htlc.prev_counterparty_node_id, + ); + let mut pending_intercepts = self.pending_intercepted_htlcs.lock().unwrap(); + match pending_intercepts.entry(intercept_id) { + hash_map::Entry::Vacant(entry) => { + if let Ok(intercept_ev) = + create_htlc_intercepted_event(intercept_id, &htlc) + { + self.pending_events.lock().unwrap().push_back((intercept_ev, None)); + entry.insert(htlc); + } else { + debug_assert!(false); + // Let the HTLC be auto-failed before it expires. + return; + } + }, + hash_map::Entry::Occupied(_) => { + log_error!( + logger, + "Failed to forward incoming HTLC: detected duplicate intercepted payment", + ); + debug_assert!( + false, + "Should never have two HTLCs with the same channel id and htlc id", + ); + // Let the HTLC be auto-failed before it expires. + return; + }, + } + } else { + let mut per_source_pending_forward = [( + htlc.prev_outbound_scid_alias, + htlc.prev_counterparty_node_id, + htlc.prev_funding_outpoint, + htlc.prev_channel_id, + htlc.prev_user_channel_id, + vec![(htlc.forward_info, htlc.prev_htlc_id)], + )]; + self.forward_htlcs(&mut per_source_pending_forward); + } }, _ => return, } @@ -16745,6 +16769,7 @@ where } } + let our_pending_intercepts = self.pending_intercepted_htlcs.lock().unwrap(); // Since some FundingNegotiation variants are not persisted, any splice in such state must // be failed upon reload. However, as the necessary information for the SpliceFailed event @@ -16842,7 +16867,6 @@ where } 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); } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index e072deb6a97..717933f2ee0 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2905,7 +2905,7 @@ pub fn check_payment_claimable( _ => {}, } }, - _ => panic!("Unexpected event"), + _ => panic!("Unexpected event {event:?}"), } } diff --git a/lightning/src/ln/interception_tests.rs b/lightning/src/ln/interception_tests.rs new file mode 100644 index 00000000000..a5487e79e5c --- /dev/null +++ b/lightning/src/ln/interception_tests.rs @@ -0,0 +1,278 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +//! Tests that test standing up a network of ChannelManagers, creating channels, sending +//! payments/messages between them, and often checking the resulting ChannelMonitors are able to +//! claim outputs on-chain. + +use crate::events::{Event, HTLCHandlingFailureReason, HTLCHandlingFailureType}; +use crate::ln::channelmanager::{PaymentId, RecipientOnionFields}; +use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler}; +use crate::ln::onion_utils::LocalHTLCFailureReason; +use crate::routing::router::PaymentParameters; +use crate::util::config::HTLCInterceptionFlags; + +use crate::prelude::*; + +use crate::ln::functional_test_utils::*; + +#[derive(Clone, Copy, PartialEq, Eq)] +enum ForwardingMods { + FeeTooLow, + CLTVBelowConfig, + CLTVBelowMin, + None, +} + +fn do_test_htlc_interception_flags(flags_bitmask: u8, flag_bit: u8, modification: ForwardingMods) { + // Tests that the `htlc_interception_flags` bitmask given by `flags_bitmask` correctly + // intercepts (or doesn't intercept) an HTLC which is of type `flag_bit` + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + + let mut intercept_config = test_default_channel_config(); + intercept_config.htlc_interception_flags = flags_bitmask; + intercept_config.channel_config.forwarding_fee_base_msat = 1000; + intercept_config.channel_config.cltv_expiry_delta = 6 * 24; + intercept_config.accept_forwards_to_priv_channels = true; + + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(intercept_config), None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes(&nodes, 0, 1); + + let node_0_id = nodes[0].node.get_our_node_id(); + let node_1_id = nodes[1].node.get_our_node_id(); + let node_2_id = nodes[2].node.get_our_node_id(); + + // First open the right type of channel (and get it in the right state) for the bit we're + // testing. + let (target_scid, target_chan_id) = match flag_bit { + 1 | 2 => { + create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 0); + let chan_id = nodes[2].node.list_channels()[0].channel_id; + let scid = nodes[2].node.list_channels()[0].short_channel_id.unwrap(); + if (1 << flag_bit) == HTLCInterceptionFlags::ToOfflinePrivateChannels as u8 { + nodes[1].node.peer_disconnected(node_2_id); + nodes[2].node.peer_disconnected(node_1_id); + } else { + assert_eq!(1 << flag_bit, HTLCInterceptionFlags::ToOnlinePrivateChannels as u8); + } + (scid, chan_id) + }, + 0 | 3 | 4 => { + let (chan_upd, _, chan_id, _) = create_announced_chan_between_nodes(&nodes, 1, 2); + if (1 << flag_bit) == HTLCInterceptionFlags::ToInterceptSCIDs as u8 { + (nodes[1].node.get_intercept_scid(), chan_id) + } else if (1 << flag_bit) == HTLCInterceptionFlags::ToPublicChannels as u8 { + (chan_upd.contents.short_channel_id, chan_id) + } else if (1 << flag_bit) == HTLCInterceptionFlags::ToUnknownSCIDs as u8 { + (42424242, chan_id) + } else { + panic!(); + } + }, + _ => panic!("Invalid flag_bit: {}", flag_bit), + }; + + // Start every node on the same block height to ensure we don't hit spurious CLTV issues + connect_blocks(&nodes[0], 2 * CHAN_CONFIRM_DEPTH + 1 - nodes[0].best_block_info().1); + connect_blocks(&nodes[1], 2 * CHAN_CONFIRM_DEPTH + 1 - nodes[1].best_block_info().1); + connect_blocks(&nodes[2], 2 * CHAN_CONFIRM_DEPTH + 1 - nodes[2].best_block_info().1); + + // Send the HTLC from nodes[0] to nodes[1] and process it to generate the interception (if + // we're set to intercept it). + let amt_msat = 100_000; + let bolt11 = nodes[2].node.create_bolt11_invoice(Default::default()).unwrap(); + let pay_params = PaymentParameters::from_bolt11_invoice(&bolt11); + let (mut route, payment_hash, payment_preimage, payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[2], pay_params, amt_msat); + route.paths[0].hops[1].short_channel_id = target_scid; + + let interception_bit_match = (flags_bitmask & (1 << flag_bit)) != 0; + match modification { + ForwardingMods::FeeTooLow => { + assert!( + interception_bit_match, + "No reason to test failing if we aren't trying to intercept", + ); + route.paths[0].hops[0].fee_msat = 500; + }, + ForwardingMods::CLTVBelowConfig => { + route.paths[0].hops[0].cltv_expiry_delta = 6 * 12; + assert!( + interception_bit_match, + "No reason to test failing if we aren't trying to intercept", + ); + }, + ForwardingMods::CLTVBelowMin => { + route.paths[0].hops[0].cltv_expiry_delta = 6; + }, + ForwardingMods::None => {}, + } + + let onion = RecipientOnionFields::secret_only(payment_secret); + let payment_id = PaymentId(payment_hash.0); + nodes[0].node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap(); + check_added_monitors(&nodes[0], 1); + + let payment_event = SendEvent::from_node(&nodes[0]); + nodes[1].node.handle_update_add_htlc(node_0_id, &payment_event.msgs[0]); + do_commitment_signed_dance(&nodes[1], &nodes[0], &payment_event.commitment_msg, false, true); + expect_and_process_pending_htlcs(&nodes[1], false); + + if interception_bit_match && modification == ForwardingMods::None { + // If we were set to intercept, check that we got an interception event then + // forward the HTLC on to nodes[2] and claim the payment. + let intercept_id; + let events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1, "{events:?}"); + if let Event::HTLCIntercepted { intercept_id: id, requested_next_hop_scid, .. } = &events[0] + { + assert_eq!(*requested_next_hop_scid, target_scid, + "Bitmask {flags_bitmask:#x}: Expected interception for bit {flag_bit} to target SCID {target_scid}"); + intercept_id = *id; + } else { + panic!("{events:?}"); + } + + if (1 << flag_bit) == HTLCInterceptionFlags::ToOfflinePrivateChannels as u8 { + let mut reconnect_args = ReconnectArgs::new(&nodes[1], &nodes[2]); + reconnect_args.send_channel_ready = (true, true); + reconnect_nodes(reconnect_args); + } + + nodes[1] + .node + .forward_intercepted_htlc(intercept_id, &target_chan_id, node_2_id, amt_msat) + .unwrap(); + expect_and_process_pending_htlcs(&nodes[1], false); + check_added_monitors(&nodes[1], 1); + + let forward_ev = SendEvent::from_node(&nodes[1]); + nodes[2].node.handle_update_add_htlc(node_1_id, &forward_ev.msgs[0]); + do_commitment_signed_dance(&nodes[2], &nodes[1], &forward_ev.commitment_msg, false, true); + + nodes[2].node.process_pending_htlc_forwards(); + expect_payment_claimable!(nodes[2], payment_hash, payment_secret, amt_msat); + claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage); + } else { + // If we were not set to intercept, check that the HTLC either failed or was + // automatically forwarded as appropriate. + match (modification, flag_bit) { + (ForwardingMods::None, 2 | 3) => { + check_added_monitors(&nodes[1], 1); + + let forward_ev = SendEvent::from_node(&nodes[1]); + assert_eq!(forward_ev.node_id, node_2_id); + nodes[2].node.handle_update_add_htlc(node_1_id, &forward_ev.msgs[0]); + let commitment = &forward_ev.commitment_msg; + do_commitment_signed_dance(&nodes[2], &nodes[1], commitment, false, true); + + nodes[2].node.process_pending_htlc_forwards(); + expect_payment_claimable!(nodes[2], payment_hash, payment_secret, amt_msat); + claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage); + }, + _ => { + let events = nodes[1].node.get_and_clear_pending_events(); + let reason_from_mod = match modification { + ForwardingMods::FeeTooLow => Some(LocalHTLCFailureReason::FeeInsufficient), + ForwardingMods::CLTVBelowConfig => { + Some(LocalHTLCFailureReason::IncorrectCLTVExpiry) + }, + ForwardingMods::CLTVBelowMin => { + Some(LocalHTLCFailureReason::IncorrectCLTVExpiry) + }, + ForwardingMods::None => None, + }; + let (expected_failure_type, reason); + if (1 << flag_bit) == HTLCInterceptionFlags::ToOfflinePrivateChannels as u8 { + expected_failure_type = HTLCHandlingFailureType::Forward { + node_id: Some(node_2_id), + channel_id: target_chan_id, + }; + reason = reason_from_mod.unwrap_or(LocalHTLCFailureReason::PeerOffline); + } else if (1 << flag_bit) == HTLCInterceptionFlags::ToInterceptSCIDs as u8 { + expected_failure_type = HTLCHandlingFailureType::InvalidForward { + requested_forward_scid: target_scid, + }; + reason = reason_from_mod.unwrap_or(LocalHTLCFailureReason::UnknownNextPeer); + } else if (1 << flag_bit) == HTLCInterceptionFlags::ToUnknownSCIDs as u8 { + expected_failure_type = HTLCHandlingFailureType::InvalidForward { + requested_forward_scid: target_scid, + }; + reason = reason_from_mod.unwrap_or(LocalHTLCFailureReason::UnknownNextPeer); + } else { + expected_failure_type = HTLCHandlingFailureType::Forward { + node_id: Some(node_2_id), + channel_id: target_chan_id, + }; + reason = reason_from_mod + .expect("We should only fail because of a mod or unknown next-hop"); + } + if let Event::HTLCHandlingFailed { failure_reason, failure_type, .. } = &events[0] { + assert_eq!(*failure_reason, Some(HTLCHandlingFailureReason::Local { reason })); + assert_eq!(*failure_type, expected_failure_type); + } else { + panic!("{events:?}"); + } + + check_added_monitors(&nodes[1], 1); + let fail_msgs = get_htlc_update_msgs(&nodes[1], &node_0_id); + nodes[0].node.handle_update_fail_htlc(node_1_id, &fail_msgs.update_fail_htlcs[0]); + let commitment = fail_msgs.commitment_signed; + do_commitment_signed_dance(&nodes[0], &nodes[1], &commitment, true, true); + expect_payment_failed!(nodes[0], payment_hash, false); + }, + } + } +} + +const MAX_BITMASK: u8 = HTLCInterceptionFlags::AllValidHTLCs as u8; +const MAX_FLAG: u8 = 4; + +#[test] +fn test_htlc_interception_flags() { + // Test all 2^5 = 32 combinations of the HTLCInterceptionFlags bitmask + // For each combination, test 5 different HTLC forwards and verify correct interception behavior + assert_eq!((1 << MAX_FLAG + 1) - 1, MAX_BITMASK); + + for flags_bitmask in 0..=MAX_BITMASK { + for flag_bit in 0..=MAX_FLAG { + do_test_htlc_interception_flags(flags_bitmask, flag_bit, ForwardingMods::None); + } + } +} + +#[test] +fn test_htlc_bad_for_chan_config() { + // Test that interception won't be done if an HTLC fails to meet the target channel's channel + // config. + let have_chan_flags = [ + HTLCInterceptionFlags::ToOfflinePrivateChannels, + HTLCInterceptionFlags::ToOnlinePrivateChannels, + HTLCInterceptionFlags::ToPublicChannels, + ]; + for flag in have_chan_flags { + assert_eq!((flag as u8).count_ones(), 1); + let bit = (flag as u8).trailing_zeros() as u8; + do_test_htlc_interception_flags(flag as u8, bit, ForwardingMods::FeeTooLow); + do_test_htlc_interception_flags(flag as u8, bit, ForwardingMods::CLTVBelowConfig); + } +} + + +#[test] +fn test_htlc_bad_no_chan() { + // Test that setting the CLTV below the hard-coded minimum fails whether we're intercepting for + // a channel or not. + for flag_bit in 0..=MAX_FLAG { + do_test_htlc_interception_flags(1 << flag_bit, flag_bit, ForwardingMods::CLTVBelowMin); + } +} diff --git a/lightning/src/ln/mod.rs b/lightning/src/ln/mod.rs index 04aa8181b92..c289eeb8f4c 100644 --- a/lightning/src/ln/mod.rs +++ b/lightning/src/ln/mod.rs @@ -82,6 +82,9 @@ pub mod functional_tests; #[cfg(any(test, feature = "_externalize_tests"))] #[allow(unused_mut)] pub mod htlc_reserve_unit_tests; +#[cfg(any(test, feature = "_externalize_tests"))] +#[allow(unused_mut)] +pub mod interception_tests; #[cfg(test)] #[allow(unused_mut)] mod max_payment_path_len_tests; diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index dbc2ebc9d48..944acb6eea1 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -1572,7 +1572,8 @@ pub enum LocalHTLCFailureReason { /// /// The forwarding node has tampered with this value, or has a bug in its implementation. FinalIncorrectHTLCAmount, - /// The channel has been marked as disabled because the channel peer is offline. + /// The HTLC couldn't be forwarded because the channel counterparty has been offline for some + /// time. ChannelDisabled, /// The HTLC expires too far in the future, so it is rejected to avoid the worst-case outcome /// of funds being held for extended periods of time. diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index f9894fa8819..80a4f41545b 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -45,6 +45,7 @@ use crate::sign::EntropySource; use crate::types::features::{Bolt11InvoiceFeatures, ChannelTypeFeatures}; use crate::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; use crate::types::string::UntrustedString; +use crate::util::config::HTLCInterceptionFlags; use crate::util::errors::APIError; use crate::util::ser::Writeable; use crate::util::test_utils; @@ -2210,7 +2211,8 @@ fn do_test_intercepted_payment(test: InterceptTest) { let mut zero_conf_chan_config = test_default_channel_config(); zero_conf_chan_config.manually_accept_inbound_channels = true; let mut intercept_forwards_config = test_default_channel_config(); - intercept_forwards_config.accept_intercept_htlcs = true; + intercept_forwards_config.htlc_interception_flags = + HTLCInterceptionFlags::ToInterceptSCIDs as u8; let configs = [None, Some(intercept_forwards_config), Some(zero_conf_chan_config)]; let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &configs); @@ -2435,7 +2437,8 @@ fn do_accept_underpaying_htlcs_config(num_mpp_parts: usize) { let max_in_flight_percent = 10; let mut intercept_forwards_config = test_default_channel_config(); - intercept_forwards_config.accept_intercept_htlcs = true; + intercept_forwards_config.htlc_interception_flags = + HTLCInterceptionFlags::ToInterceptSCIDs as u8; intercept_forwards_config .channel_handshake_config .max_inbound_htlc_value_in_flight_percent_of_channel = max_in_flight_percent; diff --git a/lightning/src/ln/reload_tests.rs b/lightning/src/ln/reload_tests.rs index d143082821d..aff2a5831a4 100644 --- a/lightning/src/ln/reload_tests.rs +++ b/lightning/src/ln/reload_tests.rs @@ -26,7 +26,7 @@ use crate::util::test_channel_signer::TestChannelSigner; use crate::util::test_utils; use crate::util::errors::APIError; use crate::util::ser::{Writeable, ReadableArgs}; -use crate::util::config::UserConfig; +use crate::util::config::{HTLCInterceptionFlags, UserConfig}; use bitcoin::hashes::Hash; use bitcoin::hash_types::BlockHash; @@ -931,7 +931,8 @@ fn do_forwarded_payment_no_manager_persistence(use_cs_commitment: bool, claim_ht let new_chain_monitor; let mut intercept_forwards_config = test_default_channel_config(); - intercept_forwards_config.accept_intercept_htlcs = true; + intercept_forwards_config.htlc_interception_flags = + HTLCInterceptionFlags::ToInterceptSCIDs as u8; let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(intercept_forwards_config), None]); let nodes_1_deserialized; @@ -1189,7 +1190,8 @@ fn do_manager_persisted_pre_outbound_edge_forward(intercept_htlc: bool) { let persister; let new_chain_monitor; let mut intercept_forwards_config = test_default_channel_config(); - intercept_forwards_config.accept_intercept_htlcs = true; + intercept_forwards_config.htlc_interception_flags = + HTLCInterceptionFlags::ToInterceptSCIDs as u8; let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(intercept_forwards_config), None]); let nodes_1_deserialized; let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index dd1aaa40424..b89a591efa9 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -855,6 +855,111 @@ impl crate::util::ser::Readable for LegacyChannelConfig { } } +/// Flags which can be set on [`UserConfig::htlc_interception_flags`]. Each flag selects some set +/// of HTLCs which are forwarded across this node to be intercepted instead, generating an +/// [`Event::HTLCIntercepted`] instead of automatically forwarding the HTLC and allowing it to be +/// forwarded or rejected manually. +/// +/// [`Event::HTLCIntercepted`]: crate::events::Event::HTLCIntercepted +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum HTLCInterceptionFlags { + /// If this flag is set, LDK will intercept HTLCs that are attempting to be forwarded over fake + /// short channel ids generated via [`ChannelManager::get_intercept_scid`]. This allows you to + /// only intercept HTLCs which are specifically marked for interception by the invoice being + /// paid. + /// + /// Note that because LDK is not aware of which channel the HTLC will be forwarded over at the + /// time of interception, only basic checks to ensure the fee the HTLC intends to pay is not + /// negative and a minimum CLTV delta between the incoming and outgoing HTLC edge are performed + /// before the [`Event::HTLCIntercepted`] is generated. You must validate the fee and CLTV + /// delta meets your requirements before forwarding the HTLC. + /// + /// [`ChannelManager::get_intercept_scid`]: crate::ln::channelmanager::ChannelManager::get_intercept_scid + /// [`Event::HTLCIntercepted`]: crate::events::Event::HTLCIntercepted + ToInterceptSCIDs = 1 << 0, + /// If this flag is set, any attempts to forward a payment to a private channel while the + /// channel counterparty is offline will instead generate an [`Event::HTLCIntercepted`] which + /// must be handled the same as any other intercepted HTLC. + /// + /// This is useful for LSPs that may need to wake the recipient node (e.g. via a mobile push + /// notification). Note that in this case you must ensure that you set a quick timeout to fail + /// the HTLC if the recipient node fails to come online (e.g. within 10 seconds). + /// + /// Before interception, the HTLC is validated against the forwarding config of the outbound + /// channel to ensure it pays sufficient fee and meets the + /// [`ChannelConfig::cltv_expiry_delta`]. + /// + /// [`Event::HTLCIntercepted`]: crate::events::Event::HTLCIntercepted + ToOfflinePrivateChannels = 1 << 1, + /// If this flag is set, any attempts to forward a payment to a private channel while the + /// channel counterparty is online will instead generate an [`Event::HTLCIntercepted`] which + /// must be handled the same as any other intercepted HTLC. + /// + /// This is the complement to [`Self::ToOfflinePrivateChannels`] and, together, they allow + /// intercepting all HTLCs destined for private channels. This may be useful for LSPs that wish + /// to take an additional fee paid by the recipient on all forwards to clients. + /// + /// Before interception, the HTLC is validated against the forwarding config of the outbound + /// channel to ensure it pays sufficient fee and meets the + /// [`ChannelConfig::cltv_expiry_delta`]. + /// + /// [`Event::HTLCIntercepted`]: crate::events::Event::HTLCIntercepted + ToOnlinePrivateChannels = 1 << 2, + /// If this flag is set, any attempts to forward a payment to a publicly announced channel will + /// instead generate an [`Event::HTLCIntercepted`] which must be handled the same as any other + /// intercepted HTLC. + /// + /// Before interception, the HTLC is validated against the forwarding config of the outbound + /// channel to ensure it pays sufficient fee and meets the + /// [`ChannelConfig::cltv_expiry_delta`]. + /// + /// [`Event::HTLCIntercepted`]: crate::events::Event::HTLCIntercepted + ToPublicChannels = 1 << 3, + /// If these flags are set, any attempts to forward a payment to a channel of ours or an fake + /// short channel id generated via [`ChannelManager::get_intercept_scid`] will instead generate + /// an [`Event::HTLCIntercepted`] which must be handled the same as any other intercepted HTLC. + /// + /// In the case of intercept SCIDs, only basic checks to ensure the fee the HTLC intends to pay + /// is not negative and a minimum CLTV delta between the incoming and outgoing HTLC edge are + /// performed before the [`Event::HTLCIntercepted`] is generated. You must validate the fee and + /// CLTV delta meets your requirements before forwarding the HTLC. + /// + /// [`ChannelManager::get_intercept_scid`]: crate::ln::channelmanager::ChannelManager::get_intercept_scid + /// [`Event::HTLCIntercepted`]: crate::events::Event::HTLCIntercepted + ToAllKnownSCIDs = Self::ToInterceptSCIDs as isize + | Self::ToOfflinePrivateChannels as isize + | Self::ToOnlinePrivateChannels as isize + | Self::ToPublicChannels as isize, + /// If this flag is set, any attempts to forward a payment to an unknown short channel id will + /// instead generate an [`Event::HTLCIntercepted`] which must be handled the same as any other + /// intercepted HTLC. + /// + /// Note that because LDK is not aware of which channel the HTLC will be forwarded over at the + /// time of interception, only basic checks to ensure the fee the HTLC intends to pay is not + /// negative and a minimum CLTV delta between the incoming and outgoing HTLC edge are performed + /// before the [`Event::HTLCIntercepted`] is generated. You must validate the fee and CLTV + /// delta meets your requirements before forwarding the HTLC. + /// + /// [`Event::HTLCIntercepted`]: crate::events::Event::HTLCIntercepted + ToUnknownSCIDs = 1 << 4, + /// If these flags are set, all HTLCs being forwarded over this node will instead generate an + /// [`Event::HTLCIntercepted`] which must be handled the same as any other intercepted HTLC. + /// + /// In the case of intercept or unknown SCIDs, only basic checks to ensure the fee the HTLC + /// intends to pay is not negative and a minimum CLTV delta between the incoming and outgoing + /// HTLC edge are performed before the [`Event::HTLCIntercepted`] is generated. You must + /// validate the fee and CLTV delta meets your requirements before forwarding the HTLC. + /// + /// [`Event::HTLCIntercepted`]: crate::events::Event::HTLCIntercepted + AllValidHTLCs = Self::ToAllKnownSCIDs as isize | Self::ToUnknownSCIDs as isize, +} + +impl Into for HTLCInterceptionFlags { + fn into(self) -> u8 { + self as u8 + } +} + /// Top-level config which holds ChannelHandshakeLimits and ChannelConfig. /// /// `Default::default()` provides sane defaults for most configurations @@ -907,17 +1012,21 @@ pub struct UserConfig { /// [`msgs::OpenChannel`]: crate::ln::msgs::OpenChannel /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel pub manually_accept_inbound_channels: bool, - /// If this is set to `true`, LDK will intercept HTLCs that are attempting to be forwarded over - /// fake short channel ids generated via [`ChannelManager::get_intercept_scid`]. Upon HTLC - /// intercept, LDK will generate an [`Event::HTLCIntercepted`] which MUST be handled by the user. + /// Flags consisting of OR'd values from [`HTLCInterceptionFlags`] which describe HTLCs + /// forwarded over this node to intercept. Any HTLCs which are intercepted will generate an + /// [`Event::HTLCIntercepted`] event which must be handled to forward or fail the HTLC. /// - /// Setting this to `true` may break backwards compatibility with LDK versions < 0.0.113. + /// Do NOT hold on to intercepted HTLCs for more than a few seconds, they must always be + /// forwarded or failed nearly immediately to avoid performing accidental denial of service + /// attacks against other lightning nodes and being punished appropriately by other nodes. /// - /// Default value: `false` + /// To ensure efficiency and reliable HTLC latency you should ensure you only intercept types + /// of HTLCs which you need to manually forward or reject. + /// + /// Defalut value: `0` (indicating no HTLCs will be intercepted). /// - /// [`ChannelManager::get_intercept_scid`]: crate::ln::channelmanager::ChannelManager::get_intercept_scid /// [`Event::HTLCIntercepted`]: crate::events::Event::HTLCIntercepted - pub accept_intercept_htlcs: bool, + pub htlc_interception_flags: u8, /// If this is set to `true`, the user needs to manually pay [`Bolt12Invoice`]s when received. /// /// When set to `true`, [`Event::InvoiceReceived`] will be generated for each received @@ -984,7 +1093,7 @@ impl Default for UserConfig { accept_forwards_to_priv_channels: false, accept_inbound_channels: true, manually_accept_inbound_channels: false, - accept_intercept_htlcs: false, + htlc_interception_flags: 0, manually_handle_bolt12_invoices: false, enable_dual_funded_channels: false, enable_htlc_hold: false, @@ -1007,7 +1116,7 @@ impl Readable for UserConfig { accept_forwards_to_priv_channels: Readable::read(reader)?, accept_inbound_channels: Readable::read(reader)?, manually_accept_inbound_channels: Readable::read(reader)?, - accept_intercept_htlcs: Readable::read(reader)?, + htlc_interception_flags: Readable::read(reader)?, manually_handle_bolt12_invoices: Readable::read(reader)?, enable_dual_funded_channels: Readable::read(reader)?, hold_outbound_htlcs_at_next_hop: Readable::read(reader)?, diff --git a/pending_changelog/matt-full-interception.txt b/pending_changelog/matt-full-interception.txt new file mode 100644 index 00000000000..2cc51a56305 --- /dev/null +++ b/pending_changelog/matt-full-interception.txt @@ -0,0 +1,4 @@ +# Backwards Compatibility + * HTLCs which were first received on an LDK version prior to LDK 0.2 will no + longer be intercepted. Instead, they will be handled as if they were not + intercepted and be forwarded/failed automatically.