From 790abc540d46053cacc03893e92ad57454900e94 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Fri, 24 Jun 2022 10:53:49 +0200 Subject: [PATCH 1/2] Refactor `max_mpp_path_count` to `max_path_count` Using this field just for MPP doesn't make sense when it could intuitively also be used to indicate single-path payments. We therefore rename `max_mpp_path_count` to `max_path_count` and make sure that a value of 1 ensures MPP is not even tried. --- lightning/src/routing/router.rs | 48 +++++++++++++++++---------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 420a473ff5..e5377dcbb4 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -176,10 +176,10 @@ impl_writeable_tlv_based!(RouteParameters, { /// Maximum total CTLV difference we allow for a full payment path. pub const DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA: u32 = 1008; -/// Maximum number of paths we allow an MPP payment to have. +/// Maximum number of paths we allow an (MPP) payment to have. // The default limit is currently set rather arbitrary - there aren't any real fundamental path-count // limits, but for now more than 10 paths likely carries too much one-path failure. -pub const DEFAULT_MAX_MPP_PATH_COUNT: u8 = 10; +pub const DEFAULT_MAX_PATH_COUNT: u8 = 10; // The median hop CLTV expiry delta currently seen in the network. const MEDIAN_HOP_CLTV_EXPIRY_DELTA: u32 = 40; @@ -222,16 +222,16 @@ pub struct PaymentParameters { /// Defaults to [`DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA`]. pub max_total_cltv_expiry_delta: u32, - /// The maximum number of paths that may be used by MPP payments. - /// Defaults to [`DEFAULT_MAX_MPP_PATH_COUNT`]. - pub max_mpp_path_count: u8, + /// The maximum number of paths that may be used by (MPP) payments. + /// Defaults to [`DEFAULT_MAX_PATH_COUNT`]. + pub max_path_count: u8, } impl_writeable_tlv_based!(PaymentParameters, { (0, payee_pubkey, required), (1, max_total_cltv_expiry_delta, (default_value, DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA)), (2, features, option), - (3, max_mpp_path_count, (default_value, DEFAULT_MAX_MPP_PATH_COUNT)), + (3, max_path_count, (default_value, DEFAULT_MAX_PATH_COUNT)), (4, route_hints, vec_type), (6, expiry_time, option), }); @@ -245,7 +245,7 @@ impl PaymentParameters { route_hints: vec![], expiry_time: None, max_total_cltv_expiry_delta: DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, - max_mpp_path_count: DEFAULT_MAX_MPP_PATH_COUNT, + max_path_count: DEFAULT_MAX_PATH_COUNT, } } @@ -282,11 +282,11 @@ impl PaymentParameters { Self { max_total_cltv_expiry_delta, ..self } } - /// Includes a limit for the maximum number of payment paths that may be used by MPP. + /// Includes a limit for the maximum number of payment paths that may be used. /// /// (C-not exported) since bindings don't support move semantics - pub fn with_max_mpp_path_count(self, max_mpp_path_count: u8) -> Self { - Self { max_mpp_path_count, ..self } + pub fn with_max_path_count(self, max_path_count: u8) -> Self { + Self { max_path_count, ..self } } } @@ -800,10 +800,16 @@ where L::Target: Logger { let network_channels = network_graph.channels(); let network_nodes = network_graph.nodes(); + if payment_params.max_path_count == 0 { + return Err(LightningError{err: "Can't find a route with no paths allowed.".to_owned(), action: ErrorAction::IgnoreError}); + } + // Allow MPP only if we have a features set from somewhere that indicates the payee supports // it. If the payee supports it they're supposed to include it in the invoice, so that should // work reliably. - let allow_mpp = if let Some(features) = &payment_params.features { + let allow_mpp = if payment_params.max_path_count == 1 { + false + } else if let Some(features) = &payment_params.features { features.supports_basic_mpp() } else if let Some(node) = network_nodes.get(&payee_node_id) { if let Some(node_info) = node.announcement_info.as_ref() { @@ -811,10 +817,6 @@ where L::Target: Logger { } else { false } } else { false }; - if allow_mpp && payment_params.max_mpp_path_count == 0 { - return Err(LightningError{err: "Can't find an MPP route with no paths allowed.".to_owned(), action: ErrorAction::IgnoreError}); - } - log_trace!(logger, "Searching for a route from payer {} to payee {} {} MPP and {} first hops {}overriding the network graph", our_node_pubkey, payment_params.payee_pubkey, if allow_mpp { "with" } else { "without" }, first_hops.map(|hops| hops.len()).unwrap_or(0), if first_hops.is_some() { "" } else { "not " }); @@ -872,10 +874,10 @@ where L::Target: Logger { // Taking too many smaller paths also increases the chance of payment failure. // Thus to avoid this effect, we require from our collected links to provide // at least a minimal contribution to the recommended value yet-to-be-fulfilled. - // This requirement is currently set to be 1/max_mpp_path_count of the payment + // This requirement is currently set to be 1/max_path_count of the payment // value to ensure we only ever return routes that do not violate this limit. let minimal_value_contribution_msat: u64 = if allow_mpp { - (final_value_msat + (payment_params.max_mpp_path_count as u64 - 1)) / payment_params.max_mpp_path_count as u64 + (final_value_msat + (payment_params.max_path_count as u64 - 1)) / payment_params.max_path_count as u64 } else { final_value_msat }; @@ -1694,7 +1696,7 @@ where L::Target: Logger { selected_paths.push(path); } // Make sure we would never create a route with more paths than we allow. - debug_assert!(selected_paths.len() <= payment_params.max_mpp_path_count.into()); + debug_assert!(selected_paths.len() <= payment_params.max_path_count.into()); if let Some(features) = &payment_params.features { for path in selected_paths.iter_mut() { @@ -4119,20 +4121,20 @@ mod tests { } { - // Attempt to route while setting max_mpp_path_count to 0 results in a failure. - let zero_payment_params = payment_params.clone().with_max_mpp_path_count(0); + // Attempt to route while setting max_path_count to 0 results in a failure. + let zero_payment_params = payment_params.clone().with_max_path_count(0); if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route( &our_id, &zero_payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes) { - assert_eq!(err, "Can't find an MPP route with no paths allowed."); + assert_eq!(err, "Can't find a route with no paths allowed."); } else { panic!(); } } { - // Attempt to route while setting max_mpp_path_count to 3 results in a failure. + // Attempt to route while setting max_path_count to 3 results in a failure. // This is the case because the minimal_value_contribution_msat would require each path // to account for 1/3 of the total value, which is violated by 2 out of 3 paths. - let fail_payment_params = payment_params.clone().with_max_mpp_path_count(3); + let fail_payment_params = payment_params.clone().with_max_path_count(3); if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route( &our_id, &fail_payment_params, &network_graph.read_only(), None, 250_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes) { From eb8bce0d161d5d6c135be5fd8c7ebe2699857ae0 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Fri, 24 Jun 2022 12:00:20 +0200 Subject: [PATCH 2/2] Add `send_probe` and introduce probing cookies When we send payment probes, we generate the [`PaymentHash`] based on a probing cookie secret and a random [`PaymentId`]. This allows us to discern probes from real payments, without keeping additional state. --- fuzz/src/chanmon_consistency.rs | 6 ++ fuzz/src/full_stack.rs | 2 +- lightning-invoice/src/payment.rs | 92 +++++++++++++++++--- lightning/src/ln/channelmanager.rs | 103 +++++++++++++++++++---- lightning/src/ln/mod.rs | 6 +- lightning/src/ln/payment_tests.rs | 129 +++++++++++++++++++++++++++++ lightning/src/routing/router.rs | 8 ++ lightning/src/routing/scoring.rs | 26 ++++++ lightning/src/util/events.rs | 88 ++++++++++++++++++++ 9 files changed, 428 insertions(+), 32 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 615f3aad99..9d5ccaa810 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -850,6 +850,12 @@ pub fn do_test(data: &[u8], underlying_out: Out) { events::Event::PaymentClaimed { .. } => {}, events::Event::PaymentPathSuccessful { .. } => {}, events::Event::PaymentPathFailed { .. } => {}, + events::Event::ProbeSuccessful { .. } | events::Event::ProbeFailed { .. } => { + // Even though we don't explicitly send probes, because probes are + // detected based on hashing the payment hash+preimage, its rather + // trivial for the fuzzer to build payments that accidentally end up + // looking like probes. + }, events::Event::PaymentForwarded { .. } if $node == 1 => {}, events::Event::PendingHTLCsForwardable { .. } => { nodes[$node].process_pending_htlc_forwards(); diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index b4ca316ed0..fa0211aa28 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -393,7 +393,7 @@ pub fn do_test(data: &[u8], logger: &Arc) { // Adding new calls to `KeysInterface::get_secure_random_bytes` during startup can change all the // keys subsequently generated in this test. Rather than regenerating all the messages manually, // it's easier to just increment the counter here so the keys don't change. - keys_manager.counter.fetch_sub(1, Ordering::AcqRel); + keys_manager.counter.fetch_sub(2, Ordering::AcqRel); let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret(Recipient::Node).unwrap()); let network_graph = Arc::new(NetworkGraph::new(genesis_block(network).block_hash(), Arc::clone(&logger))); let gossip_sync = Arc::new(P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger))); diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index e672b89c91..051893ea0a 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -94,6 +94,8 @@ //! # ) -> u64 { 0 } //! # fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} //! # fn payment_path_successful(&mut self, _path: &[&RouteHop]) {} +//! # fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} +//! # fn probe_successful(&mut self, _path: &[&RouteHop]) {} //! # } //! # //! # struct FakeLogger {} @@ -584,6 +586,18 @@ where .map_or(1, |attempts| attempts.count + 1); log_trace!(self.logger, "Payment {} succeeded (attempts: {})", log_bytes!(payment_hash.0), attempts); }, + Event::ProbeSuccessful { payment_hash, path, .. } => { + log_trace!(self.logger, "Probe payment {} of {}msat was successful", log_bytes!(payment_hash.0), path.last().unwrap().fee_msat); + let path = path.iter().collect::>(); + self.scorer.lock().probe_successful(&path); + }, + Event::ProbeFailed { payment_hash, path, short_channel_id, .. } => { + if let Some(short_channel_id) = short_channel_id { + log_trace!(self.logger, "Probe payment {} of {}msat failed at channel {}", log_bytes!(payment_hash.0), path.last().unwrap().fee_msat, *short_channel_id); + let path = path.iter().collect::>(); + self.scorer.lock().probe_failed(&path, *short_channel_id); + } + }, _ => {}, } @@ -1296,7 +1310,7 @@ mod tests { .expect_send(Amount::ForInvoice(final_value_msat)) .expect_send(Amount::OnRetry(final_value_msat / 2)); let router = TestRouter {}; - let scorer = RefCell::new(TestScorer::new().expect(PaymentPath::Failure { + let scorer = RefCell::new(TestScorer::new().expect(TestResult::PaymentFailure { path: path.clone(), short_channel_id: path[0].short_channel_id, })); let logger = TestLogger::new(); @@ -1332,8 +1346,8 @@ mod tests { let payer = TestPayer::new().expect_send(Amount::ForInvoice(final_value_msat)); let router = TestRouter {}; let scorer = RefCell::new(TestScorer::new() - .expect(PaymentPath::Success { path: route.paths[0].clone() }) - .expect(PaymentPath::Success { path: route.paths[1].clone() }) + .expect(TestResult::PaymentSuccess { path: route.paths[0].clone() }) + .expect(TestResult::PaymentSuccess { path: route.paths[1].clone() }) ); let logger = TestLogger::new(); let invoice_payer = @@ -1416,13 +1430,15 @@ mod tests { } struct TestScorer { - expectations: Option>, + expectations: Option>, } #[derive(Debug)] - enum PaymentPath { - Failure { path: Vec, short_channel_id: u64 }, - Success { path: Vec }, + enum TestResult { + PaymentFailure { path: Vec, short_channel_id: u64 }, + PaymentSuccess { path: Vec }, + ProbeFailure { path: Vec, short_channel_id: u64 }, + ProbeSuccess { path: Vec }, } impl TestScorer { @@ -1432,7 +1448,7 @@ mod tests { } } - fn expect(mut self, expectation: PaymentPath) -> Self { + fn expect(mut self, expectation: TestResult) -> Self { self.expectations.get_or_insert_with(|| VecDeque::new()).push_back(expectation); self } @@ -1451,13 +1467,19 @@ mod tests { fn payment_path_failed(&mut self, actual_path: &[&RouteHop], actual_short_channel_id: u64) { if let Some(expectations) = &mut self.expectations { match expectations.pop_front() { - Some(PaymentPath::Failure { path, short_channel_id }) => { + Some(TestResult::PaymentFailure { path, short_channel_id }) => { assert_eq!(actual_path, &path.iter().collect::>()[..]); assert_eq!(actual_short_channel_id, short_channel_id); }, - Some(PaymentPath::Success { path }) => { + Some(TestResult::PaymentSuccess { path }) => { panic!("Unexpected successful payment path: {:?}", path) }, + Some(TestResult::ProbeFailure { path, .. }) => { + panic!("Unexpected failed payment probe: {:?}", path) + }, + Some(TestResult::ProbeSuccess { path }) => { + panic!("Unexpected successful payment probe: {:?}", path) + }, None => panic!("Unexpected payment_path_failed call: {:?}", actual_path), } } @@ -1466,10 +1488,56 @@ mod tests { fn payment_path_successful(&mut self, actual_path: &[&RouteHop]) { if let Some(expectations) = &mut self.expectations { match expectations.pop_front() { - Some(PaymentPath::Failure { path, .. }) => { + Some(TestResult::PaymentFailure { path, .. }) => { panic!("Unexpected payment path failure: {:?}", path) }, - Some(PaymentPath::Success { path }) => { + Some(TestResult::PaymentSuccess { path }) => { + assert_eq!(actual_path, &path.iter().collect::>()[..]); + }, + Some(TestResult::ProbeFailure { path, .. }) => { + panic!("Unexpected failed payment probe: {:?}", path) + }, + Some(TestResult::ProbeSuccess { path }) => { + panic!("Unexpected successful payment probe: {:?}", path) + }, + None => panic!("Unexpected payment_path_successful call: {:?}", actual_path), + } + } + } + + fn probe_failed(&mut self, actual_path: &[&RouteHop], actual_short_channel_id: u64) { + if let Some(expectations) = &mut self.expectations { + match expectations.pop_front() { + Some(TestResult::PaymentFailure { path, .. }) => { + panic!("Unexpected failed payment path: {:?}", path) + }, + Some(TestResult::PaymentSuccess { path }) => { + panic!("Unexpected successful payment path: {:?}", path) + }, + Some(TestResult::ProbeFailure { path, short_channel_id }) => { + assert_eq!(actual_path, &path.iter().collect::>()[..]); + assert_eq!(actual_short_channel_id, short_channel_id); + }, + Some(TestResult::ProbeSuccess { path }) => { + panic!("Unexpected successful payment probe: {:?}", path) + }, + None => panic!("Unexpected payment_path_failed call: {:?}", actual_path), + } + } + } + fn probe_successful(&mut self, actual_path: &[&RouteHop]) { + if let Some(expectations) = &mut self.expectations { + match expectations.pop_front() { + Some(TestResult::PaymentFailure { path, .. }) => { + panic!("Unexpected payment path failure: {:?}", path) + }, + Some(TestResult::PaymentSuccess { path }) => { + panic!("Unexpected successful payment path: {:?}", path) + }, + Some(TestResult::ProbeFailure { path, .. }) => { + panic!("Unexpected failed payment probe: {:?}", path) + }, + Some(TestResult::ProbeSuccess { path }) => { assert_eq!(actual_path, &path.iter().collect::>()[..]); }, None => panic!("Unexpected payment_path_successful call: {:?}", actual_path), diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 871ebb92d3..29d9310ba3 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -740,6 +740,11 @@ pub struct ChannelManager ChannelMana inbound_payment_key: expanded_inbound_key, fake_scid_rand_bytes: keys_manager.get_secure_random_bytes(), + probing_cookie_secret: keys_manager.get_secure_random_bytes(), + last_node_announcement_serial: AtomicUsize::new(0), highest_seen_timestamp: AtomicUsize::new(0), @@ -2731,6 +2738,43 @@ impl ChannelMana } } + /// Send a payment that is probing the given route for liquidity. We calculate the + /// [`PaymentHash`] of probes based on a static secret and a random [`PaymentId`], which allows + /// us to easily discern them from real payments. + pub fn send_probe(&self, hops: Vec) -> Result<(PaymentHash, PaymentId), PaymentSendFailure> { + let payment_id = PaymentId(self.keys_manager.get_secure_random_bytes()); + + let payment_hash = self.probing_cookie_from_id(&payment_id); + + if hops.len() < 2 { + return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { + err: "No need probing a path with less than two hops".to_string() + })) + } + + let route = Route { paths: vec![hops], payment_params: None }; + + match self.send_payment_internal(&route, payment_hash, &None, None, Some(payment_id), None) { + Ok(payment_id) => Ok((payment_hash, payment_id)), + Err(e) => Err(e) + } + } + + /// Returns whether a payment with the given [`PaymentHash`] and [`PaymentId`] is, in fact, a + /// payment probe. + pub(crate) fn payment_is_probe(&self, payment_hash: &PaymentHash, payment_id: &PaymentId) -> bool { + let target_payment_hash = self.probing_cookie_from_id(payment_id); + target_payment_hash == *payment_hash + } + + /// Returns the 'probing cookie' for the given [`PaymentId`]. + fn probing_cookie_from_id(&self, payment_id: &PaymentId) -> PaymentHash { + let mut preimage = [0u8; 64]; + preimage[..32].copy_from_slice(&self.probing_cookie_secret); + preimage[32..].copy_from_slice(&payment_id.0); + PaymentHash(Sha256::hash(&preimage).into_inner()) + } + /// Handles the generation of a funding transaction, optionally (for tests) with a function /// which checks the correctness of the funding transaction given the associated channel. fn funding_transaction_generated_intern, &Transaction) -> Result>( @@ -3839,22 +3883,40 @@ impl ChannelMana let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone()); #[cfg(not(test))] let (network_update, short_channel_id, payment_retryable, _, _) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone()); - // TODO: If we decided to blame ourselves (or one of our channels) in - // process_onion_failure we should close that channel as it implies our - // next-hop is needlessly blaming us! - events::Event::PaymentPathFailed { - payment_id: Some(payment_id), - payment_hash: payment_hash.clone(), - rejected_by_dest: !payment_retryable, - network_update, - all_paths_failed, - path: path.clone(), - short_channel_id, - retry, -#[cfg(test)] - error_code: onion_error_code, -#[cfg(test)] - error_data: onion_error_data + + if self.payment_is_probe(payment_hash, &payment_id) { + if !payment_retryable { + events::Event::ProbeSuccessful { + payment_id, + payment_hash: payment_hash.clone(), + path: path.clone(), + } + } else { + events::Event::ProbeFailed { + payment_id: payment_id, + payment_hash: payment_hash.clone(), + path: path.clone(), + short_channel_id, + } + } + } else { + // TODO: If we decided to blame ourselves (or one of our channels) in + // process_onion_failure we should close that channel as it implies our + // next-hop is needlessly blaming us! + events::Event::PaymentPathFailed { + payment_id: Some(payment_id), + payment_hash: payment_hash.clone(), + rejected_by_dest: !payment_retryable, + network_update, + all_paths_failed, + path: path.clone(), + short_channel_id, + retry, + #[cfg(test)] + error_code: onion_error_code, + #[cfg(test)] + error_data: onion_error_data + } } }, &HTLCFailReason::Reason { @@ -6631,6 +6693,7 @@ impl Writeable f (5, self.our_network_pubkey, required), (7, self.fake_scid_rand_bytes, required), (9, htlc_purposes, vec_type), + (11, self.probing_cookie_secret, required), }); Ok(()) @@ -6927,6 +6990,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> let mut pending_outbound_payments = None; 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), @@ -6934,11 +6998,16 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> (5, received_network_pubkey, option), (7, fake_scid_rand_bytes, option), (9, claimable_htlc_purposes, vec_type), + (11, probing_cookie_secret, option), }); if fake_scid_rand_bytes.is_none() { fake_scid_rand_bytes = Some(args.keys_manager.get_secure_random_bytes()); } + if probing_cookie_secret.is_none() { + probing_cookie_secret = Some(args.keys_manager.get_secure_random_bytes()); + } + if pending_outbound_payments.is_none() && pending_outbound_payments_no_retry.is_none() { pending_outbound_payments = Some(pending_outbound_payments_compat); } else if pending_outbound_payments.is_none() { @@ -7144,6 +7213,8 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> outbound_scid_aliases: Mutex::new(outbound_scid_aliases), fake_scid_rand_bytes: fake_scid_rand_bytes.unwrap(), + probing_cookie_secret: probing_cookie_secret.unwrap(), + our_network_key, our_network_pubkey, secp_ctx, diff --git a/lightning/src/ln/mod.rs b/lightning/src/ln/mod.rs index 9522e83d18..28c86b7959 100644 --- a/lightning/src/ln/mod.rs +++ b/lightning/src/ln/mod.rs @@ -80,15 +80,15 @@ pub use self::peer_channel_encryptor::LN_MAX_MSG_LEN; /// payment_hash type, use to cross-lock hop /// (C-not exported) as we just use [u8; 32] directly #[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)] -pub struct PaymentHash(pub [u8;32]); +pub struct PaymentHash(pub [u8; 32]); /// payment_preimage type, use to route payment between hop /// (C-not exported) as we just use [u8; 32] directly #[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)] -pub struct PaymentPreimage(pub [u8;32]); +pub struct PaymentPreimage(pub [u8; 32]); /// payment_secret type, use to authenticate sender to the receiver and tie MPP HTLCs together /// (C-not exported) as we just use [u8; 32] directly #[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)] -pub struct PaymentSecret(pub [u8;32]); +pub struct PaymentSecret(pub [u8; 32]); use prelude::*; use bitcoin::bech32; diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 7f725a4214..99c8682738 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -845,3 +845,132 @@ fn get_ldk_payment_preimage() { pass_along_path(&nodes[0], &[&nodes[1]], amt_msat, payment_hash, Some(payment_secret), events.pop().unwrap(), true, Some(payment_preimage)); claim_payment_along_route(&nodes[0], &[&[&nodes[1]]], false, payment_preimage); } + +#[test] +fn sent_probe_is_probe_of_sending_node() { + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); + create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()); + + // First check we refuse to build a single-hop probe + let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[1], 100_000); + assert!(nodes[0].node.send_probe(route.paths[0].clone()).is_err()); + + // Then build an actual two-hop probing path + let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], 100_000); + + match nodes[0].node.send_probe(route.paths[0].clone()) { + Ok((payment_hash, payment_id)) => { + assert!(nodes[0].node.payment_is_probe(&payment_hash, &payment_id)); + assert!(!nodes[1].node.payment_is_probe(&payment_hash, &payment_id)); + assert!(!nodes[2].node.payment_is_probe(&payment_hash, &payment_id)); + }, + _ => panic!(), + } + + get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + check_added_monitors!(nodes[0], 1); +} + +#[test] +fn successful_probe_yields_event() { + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); + create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()); + + let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], 100_000); + + let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap(); + + // node[0] -- update_add_htlcs -> node[1] + check_added_monitors!(nodes[0], 1); + let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates); + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]); + check_added_monitors!(nodes[1], 0); + commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false); + expect_pending_htlcs_forwardable!(nodes[1]); + + // node[1] -- update_add_htlcs -> node[2] + check_added_monitors!(nodes[1], 1); + let updates = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id()); + let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates); + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &probe_event.msgs[0]); + check_added_monitors!(nodes[2], 0); + commitment_signed_dance!(nodes[2], nodes[1], probe_event.commitment_msg, true, true); + + // node[1] <- update_fail_htlcs -- node[2] + let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id()); + nodes[1].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &updates.update_fail_htlcs[0]); + check_added_monitors!(nodes[1], 0); + commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, true); + + // node[0] <- update_fail_htlcs -- node[1] + let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]); + check_added_monitors!(nodes[0], 0); + commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false); + + let mut events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events.drain(..).next().unwrap() { + crate::util::events::Event::ProbeSuccessful { payment_id: ev_pid, payment_hash: ev_ph, .. } => { + assert_eq!(payment_id, ev_pid); + assert_eq!(payment_hash, ev_ph); + }, + _ => panic!(), + }; +} + +#[test] +fn failed_probe_yields_event() { + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); + create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 90000000, InitFeatures::known(), InitFeatures::known()); + + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()); + + let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 9_999_000, 42); + + let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap(); + + // node[0] -- update_add_htlcs -> node[1] + check_added_monitors!(nodes[0], 1); + let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates); + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]); + check_added_monitors!(nodes[1], 0); + commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false); + expect_pending_htlcs_forwardable!(nodes[1]); + + // node[0] <- update_fail_htlcs -- node[1] + check_added_monitors!(nodes[1], 1); + let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + // Skip the PendingHTLCsForwardable event + let _events = nodes[1].node.get_and_clear_pending_events(); + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]); + check_added_monitors!(nodes[0], 0); + commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false); + + let mut events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events.drain(..).next().unwrap() { + crate::util::events::Event::ProbeFailed { payment_id: ev_pid, payment_hash: ev_ph, .. } => { + assert_eq!(payment_id, ev_pid); + assert_eq!(payment_hash, ev_ph); + }, + _ => panic!(), + }; +} diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index e5377dcbb4..92a78829e2 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1851,6 +1851,10 @@ fn build_route_from_hops_internal( fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} fn payment_path_successful(&mut self, _path: &[&RouteHop]) {} + + fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} + + fn probe_successful(&mut self, _path: &[&RouteHop]) {} } impl<'a> Writeable for HopScorer { @@ -5316,6 +5320,8 @@ mod tests { fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} fn payment_path_successful(&mut self, _path: &[&RouteHop]) {} + fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} + fn probe_successful(&mut self, _path: &[&RouteHop]) {} } struct BadNodeScorer { @@ -5334,6 +5340,8 @@ mod tests { fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} fn payment_path_successful(&mut self, _path: &[&RouteHop]) {} + fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} + fn probe_successful(&mut self, _path: &[&RouteHop]) {} } #[test] diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs index 524f0ed315..95ad581105 100644 --- a/lightning/src/routing/scoring.rs +++ b/lightning/src/routing/scoring.rs @@ -102,6 +102,12 @@ pub trait Score $(: $supertrait)* { /// Handles updating channel penalties after successfully routing along a path. fn payment_path_successful(&mut self, path: &[&RouteHop]); + + /// Handles updating channel penalties after a probe over the given path failed. + fn probe_failed(&mut self, path: &[&RouteHop], short_channel_id: u64); + + /// Handles updating channel penalties after a probe over the given path succeeded. + fn probe_successful(&mut self, path: &[&RouteHop]); } impl $(+ $supertrait)*> Score for T { @@ -118,6 +124,14 @@ impl $(+ $supertrait)*> Score for T { fn payment_path_successful(&mut self, path: &[&RouteHop]) { self.deref_mut().payment_path_successful(path) } + + fn probe_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) { + self.deref_mut().probe_failed(path, short_channel_id) + } + + fn probe_successful(&mut self, path: &[&RouteHop]) { + self.deref_mut().probe_successful(path) + } } } } @@ -241,6 +255,10 @@ impl Score for FixedPenaltyScorer { fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} fn payment_path_successful(&mut self, _path: &[&RouteHop]) {} + + fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} + + fn probe_successful(&mut self, _path: &[&RouteHop]) {} } impl Writeable for FixedPenaltyScorer { @@ -811,6 +829,14 @@ impl>, L: Deref, T: Time> Score for Probabilis } } } + + fn probe_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) { + self.payment_path_failed(path, short_channel_id) + } + + fn probe_successful(&mut self, path: &[&RouteHop]) { + self.payment_path_failed(path, u64::max_value()) + } } mod approx { diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index caba7753f3..2282ea55ab 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -382,6 +382,38 @@ pub enum Event { #[cfg(test)] error_data: Option>, }, + /// Indicates that a probe payment we sent returned successful, i.e., only failed at the destination. + ProbeSuccessful { + /// The id returned by [`ChannelManager::send_probe`]. + /// + /// [`ChannelManager::send_probe`]: crate::ln::channelmanager::ChannelManager::send_probe + payment_id: PaymentId, + /// The hash generated by [`ChannelManager::send_probe`]. + /// + /// [`ChannelManager::send_probe`]: crate::ln::channelmanager::ChannelManager::send_probe + payment_hash: PaymentHash, + /// The payment path that was successful. + path: Vec, + }, + /// Indicates that a probe payment we sent failed at an intermediary node on the path. + ProbeFailed { + /// The id returned by [`ChannelManager::send_probe`]. + /// + /// [`ChannelManager::send_probe`]: crate::ln::channelmanager::ChannelManager::send_probe + payment_id: PaymentId, + /// The hash generated by [`ChannelManager::send_probe`]. + /// + /// [`ChannelManager::send_probe`]: crate::ln::channelmanager::ChannelManager::send_probe + payment_hash: PaymentHash, + /// The payment path that failed. + path: Vec, + /// The channel responsible for the failed probe. + /// + /// Note that for route hints or for the first hop in a path this may be an SCID alias and + /// may not refer to a channel in the public network graph. These aliases may also collide + /// with channels in the public network graph. + short_channel_id: Option, + }, /// Used to indicate that [`ChannelManager::process_pending_htlc_forwards`] should be called at /// a time in the future. /// @@ -635,6 +667,23 @@ impl Writeable for Event { (4, amount_msat, required), }); }, + &Event::ProbeSuccessful { ref payment_id, ref payment_hash, ref path } => { + 21u8.write(writer)?; + write_tlv_fields!(writer, { + (0, payment_id, required), + (2, payment_hash, required), + (4, path, vec_type) + }) + }, + &Event::ProbeFailed { ref payment_id, ref payment_hash, ref path, ref short_channel_id } => { + 23u8.write(writer)?; + write_tlv_fields!(writer, { + (0, payment_id, required), + (2, payment_hash, required), + (4, path, vec_type), + (6, short_channel_id, option), + }) + }, // Note that, going forward, all new events must only write data inside of // `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write // data via `write_tlv_fields`. @@ -854,6 +903,45 @@ impl MaybeReadable for Event { }; f() }, + 21u8 => { + let f = || { + let mut payment_id = PaymentId([0; 32]); + let mut payment_hash = PaymentHash([0; 32]); + let mut path: Option> = Some(vec![]); + read_tlv_fields!(reader, { + (0, payment_id, required), + (2, payment_hash, required), + (4, path, vec_type), + }); + Ok(Some(Event::ProbeSuccessful { + payment_id, + payment_hash, + path: path.unwrap(), + })) + }; + f() + }, + 23u8 => { + let f = || { + let mut payment_id = PaymentId([0; 32]); + let mut payment_hash = PaymentHash([0; 32]); + let mut path: Option> = Some(vec![]); + let mut short_channel_id = None; + read_tlv_fields!(reader, { + (0, payment_id, required), + (2, payment_hash, required), + (4, path, vec_type), + (6, short_channel_id, option), + }); + Ok(Some(Event::ProbeFailed{ + payment_id, + payment_hash, + path: path.unwrap(), + short_channel_id, + })) + }; + f() + }, // Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue. // Version 0.0.100 failed to properly ignore odd types, possibly resulting in corrupt // reads.