From a3a4e614276cbfe7c2fbb9eeb39d32982f1e6bb0 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 23 Aug 2021 23:55:28 -0500 Subject: [PATCH 1/9] Expose log_bytes! macro for use in other crates Needed to log PaymentHash in the lightning-invoice crate when retrying payments. --- lightning/src/util/logger.rs | 12 ++++++++++++ lightning/src/util/macro_logger.rs | 15 +++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lightning/src/util/logger.rs b/lightning/src/util/logger.rs index b34d1f0b782..01b024065b2 100644 --- a/lightning/src/util/logger.rs +++ b/lightning/src/util/logger.rs @@ -120,6 +120,18 @@ pub trait Logger { fn log(&self, record: &Record); } +/// Wrapper for logging byte slices in hex format. +#[doc(hidden)] +pub struct DebugBytes<'a>(pub &'a [u8]); +impl<'a> core::fmt::Display for DebugBytes<'a> { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + for i in self.0 { + write!(f, "{:02x}", i)?; + } + Ok(()) + } +} + #[cfg(test)] mod tests { use util::logger::{Logger, Level}; diff --git a/lightning/src/util/macro_logger.rs b/lightning/src/util/macro_logger.rs index d7849d77ae0..788bc5eca11 100644 --- a/lightning/src/util/macro_logger.rs +++ b/lightning/src/util/macro_logger.rs @@ -16,6 +16,7 @@ use bitcoin::secp256k1::key::PublicKey; use routing::router::Route; use ln::chan_utils::HTLCType; +use util::logger::DebugBytes; pub(crate) struct DebugPubKey<'a>(pub &'a PublicKey); impl<'a> core::fmt::Display for DebugPubKey<'a> { @@ -32,18 +33,11 @@ macro_rules! log_pubkey { } } -pub(crate) struct DebugBytes<'a>(pub &'a [u8]); -impl<'a> core::fmt::Display for DebugBytes<'a> { - fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { - for i in self.0 { - write!(f, "{:02x}", i)?; - } - Ok(()) - } -} +/// Logs a byte slice in hex format. +#[macro_export] macro_rules! log_bytes { ($obj: expr) => { - ::util::macro_logger::DebugBytes(&$obj) + $crate::util::logger::DebugBytes(&$obj) } } @@ -157,6 +151,7 @@ macro_rules! log_spendable { /// Create a new Record and log it. You probably don't want to use this macro directly, /// but it needs to be exported so `log_trace` etc can use it in external crates. +#[doc(hidden)] #[macro_export] macro_rules! log_internal { ($logger: expr, $lvl:expr, $($arg:tt)+) => ( From 3410f1803a77f7114c4f4e2195597578c608087f Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Sun, 17 Oct 2021 17:21:01 -0500 Subject: [PATCH 2/9] Add PaymentId to PaymentSent event The payment_hash may not uniquely identify the payment if it has been reused. Include the payment_id in PaymentSent events so it can correlated with the send_payment call. --- lightning/src/ln/chanmon_update_fail_tests.rs | 8 ++++---- lightning/src/ln/channelmanager.rs | 4 +++- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/functional_tests.rs | 16 ++++++++-------- lightning/src/ln/shutdown_tests.rs | 4 ++-- lightning/src/util/events.rs | 13 ++++++++++++- 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 0b1c16d3577..e9b93237e32 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -304,7 +304,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) { let events_3 = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events_3.len(), 1); match events_3[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(*payment_preimage, payment_preimage_1); assert_eq!(*payment_hash, payment_hash_1); }, @@ -397,7 +397,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) { let events_3 = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events_3.len(), 1); match events_3[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(*payment_preimage, payment_preimage_1); assert_eq!(*payment_hash, payment_hash_1); }, @@ -1399,7 +1399,7 @@ fn claim_while_disconnected_monitor_update_fail() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(*payment_preimage, payment_preimage_1); assert_eq!(*payment_hash, payment_hash_1); }, @@ -1806,7 +1806,7 @@ fn monitor_update_claim_fail_no_response() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(*payment_preimage, payment_preimage_1); assert_eq!(*payment_hash, payment_hash_1); }, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 4eb67af2e51..8e911876fa9 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3475,6 +3475,7 @@ impl ChannelMana let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner()); self.pending_events.lock().unwrap().push( events::Event::PaymentSent { + payment_id: Some(payment_id), payment_preimage, payment_hash: payment_hash } @@ -6256,7 +6257,8 @@ mod tests { // further events will be generated for subsequence path successes. let events = nodes[0].node.get_and_clear_pending_events(); match events[0] { - Event::PaymentSent { payment_preimage: ref preimage, payment_hash: ref hash } => { + Event::PaymentSent { payment_id: ref id, payment_preimage: ref preimage, payment_hash: ref hash } => { + assert_eq!(Some(payment_id), *id); assert_eq!(payment_preimage, *preimage); assert_eq!(our_payment_hash, *hash); }, diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 13fe7f73c31..014cf3b7ddb 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1085,7 +1085,7 @@ macro_rules! expect_payment_sent { let expected_payment_hash = PaymentHash(Sha256::hash(&$expected_payment_preimage.0).into_inner()); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!($expected_payment_preimage, *payment_preimage); assert_eq!(expected_payment_hash, *payment_hash); }, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 7c8bd075fa3..d92b914b0b5 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2659,7 +2659,7 @@ fn test_htlc_on_chain_success() { let mut first_claimed = false; for event in events { match event { - Event::PaymentSent { payment_preimage, payment_hash } => { + Event::PaymentSent { payment_id: _, payment_preimage, payment_hash } => { if payment_preimage == our_payment_preimage && payment_hash == payment_hash_1 { assert!(!first_claimed); first_claimed = true; @@ -3350,7 +3350,7 @@ fn test_simple_peer_disconnect() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match events[0] { - Event::PaymentSent { payment_preimage, payment_hash } => { + Event::PaymentSent { payment_id: _, payment_preimage, payment_hash } => { assert_eq!(payment_preimage, payment_preimage_3); assert_eq!(payment_hash, payment_hash_3); }, @@ -3514,7 +3514,7 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken let events_4 = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events_4.len(), 1); match events_4[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(payment_preimage_1, *payment_preimage); assert_eq!(payment_hash_1, *payment_hash); }, @@ -3555,7 +3555,7 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken let events_4 = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events_4.len(), 1); match events_4[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(payment_preimage_1, *payment_preimage); assert_eq!(payment_hash_1, *payment_hash); }, @@ -3790,7 +3790,7 @@ fn test_drop_messages_peer_disconnect_dual_htlc() { let events_3 = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events_3.len(), 1); match events_3[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(*payment_preimage, payment_preimage_1); assert_eq!(*payment_hash, payment_hash_1); }, @@ -5059,7 +5059,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() { let events = nodes[0].node.get_and_clear_pending_events(); match events[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(*payment_preimage, our_payment_preimage); assert_eq!(*payment_hash, duplicate_payment_hash); } @@ -5572,7 +5572,7 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentSent { payment_preimage, payment_hash } => { + Event::PaymentSent { payment_id: _, payment_preimage, payment_hash } => { assert_eq!(payment_preimage, our_payment_preimage); assert_eq!(payment_hash, our_payment_hash); }, @@ -6000,7 +6000,7 @@ fn test_free_and_fail_holding_cell_htlcs() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(*payment_preimage, payment_preimage_1); assert_eq!(*payment_hash, payment_hash_1); } diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index cbf288962f5..388104bf023 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -131,7 +131,7 @@ fn updates_shutdown_wait() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(our_payment_preimage, *payment_preimage); assert_eq!(our_payment_hash, *payment_hash); }, @@ -309,7 +309,7 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentSent { ref payment_preimage, ref payment_hash } => { + Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash } => { assert_eq!(our_payment_preimage, *payment_preimage); assert_eq!(our_payment_hash, *payment_hash); }, diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 70a6a2dd4d1..a311bd52c82 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -15,6 +15,7 @@ //! few other things. use chain::keysinterface::SpendableOutputDescriptor; +use ln::channelmanager::PaymentId; use ln::msgs; use ln::msgs::DecodeError; use ln::{PaymentPreimage, PaymentHash, PaymentSecret}; @@ -178,6 +179,12 @@ pub enum Event { /// Note for MPP payments: in rare cases, this event may be preceded by a `PaymentPathFailed` /// event. In this situation, you SHOULD treat this payment as having succeeded. PaymentSent { + /// The id returned by [`ChannelManager::send_payment`] and used with + /// [`ChannelManager::retry_payment`]. + /// + /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment + /// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment + payment_id: Option, /// The preimage to the hash given to ChannelManager::send_payment. /// Note that this serves as a payment receipt, if you wish to have such a thing, you must /// store it somehow! @@ -322,11 +329,12 @@ impl Writeable for Event { (8, payment_preimage, option), }); }, - &Event::PaymentSent { ref payment_preimage, ref payment_hash} => { + &Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash} => { 2u8.write(writer)?; write_tlv_fields!(writer, { (0, payment_preimage, required), (1, payment_hash, required), + (3, payment_id, option), }); }, &Event::PaymentPathFailed { @@ -435,14 +443,17 @@ impl MaybeReadable for Event { let f = || { let mut payment_preimage = PaymentPreimage([0; 32]); let mut payment_hash = None; + let mut payment_id = None; read_tlv_fields!(reader, { (0, payment_preimage, required), (1, payment_hash, option), + (3, payment_id, option), }); if payment_hash.is_none() { payment_hash = Some(PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner())); } Ok(Some(Event::PaymentSent { + payment_id, payment_preimage, payment_hash: payment_hash.unwrap(), })) From 73f601fd3d1c9defe553392264ca6dbcc53e4334 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 30 Sep 2021 15:29:44 -0700 Subject: [PATCH 3/9] Add PaymentId to PaymentPathFailed event The PaymentId is needed when retrying payments. Include it in the PaymentPathFailed event so it can be used in that manner. --- lightning/src/ln/channelmanager.rs | 3 +++ lightning/src/ln/functional_tests.rs | 10 ++++++---- lightning/src/ln/onion_route_tests.rs | 2 +- lightning/src/routing/network_graph.rs | 3 +++ lightning/src/util/events.rs | 12 +++++++++++- 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8e911876fa9..b93e6671ad6 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3120,6 +3120,7 @@ impl ChannelMana } else { None }; self.pending_events.lock().unwrap().push( events::Event::PaymentPathFailed { + payment_id: Some(payment_id), payment_hash, rejected_by_dest: false, network_update: None, @@ -3200,6 +3201,7 @@ impl ChannelMana // next-hop is needlessly blaming us! self.pending_events.lock().unwrap().push( events::Event::PaymentPathFailed { + payment_id: Some(payment_id), payment_hash: payment_hash.clone(), rejected_by_dest: !payment_retryable, network_update, @@ -3229,6 +3231,7 @@ impl ChannelMana // channel here as we apparently can't relay through them anyway. self.pending_events.lock().unwrap().push( events::Event::PaymentPathFailed { + payment_id: Some(payment_id), payment_hash: payment_hash.clone(), rejected_by_dest: path.len() == 1, network_update: None, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index d92b914b0b5..798a6f4435d 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -5844,7 +5844,7 @@ fn test_fail_holding_cell_htlc_upon_free() { let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send); // Send a payment which passes reserve checks but gets stuck in the holding cell. - nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap(); + let our_payment_id = nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap(); chan_stat = get_channel_value_stat!(nodes[0], chan.2); assert_eq!(chan_stat.holding_cell_outbound_amount_msat, max_can_send); @@ -5869,7 +5869,8 @@ fn test_fail_holding_cell_htlc_upon_free() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match &events[0] { - &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, path: _, ref short_channel_id, retry: _, ref error_code, ref error_data } => { + &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, ref error_data, .. } => { + assert_eq!(our_payment_id, *payment_id.as_ref().unwrap()); assert_eq!(our_payment_hash.clone(), *payment_hash); assert_eq!(*rejected_by_dest, false); assert_eq!(*all_paths_failed, true); @@ -5927,7 +5928,7 @@ fn test_free_and_fail_holding_cell_htlcs() { nodes[0].node.send_payment(&route_1, payment_hash_1, &Some(payment_secret_1)).unwrap(); chan_stat = get_channel_value_stat!(nodes[0], chan.2); assert_eq!(chan_stat.holding_cell_outbound_amount_msat, amt_1); - nodes[0].node.send_payment(&route_2, payment_hash_2, &Some(payment_secret_2)).unwrap(); + let payment_id_2 = nodes[0].node.send_payment(&route_2, payment_hash_2, &Some(payment_secret_2)).unwrap(); chan_stat = get_channel_value_stat!(nodes[0], chan.2); assert_eq!(chan_stat.holding_cell_outbound_amount_msat, amt_1 + amt_2); @@ -5953,7 +5954,8 @@ fn test_free_and_fail_holding_cell_htlcs() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match &events[0] { - &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, path: _, ref short_channel_id, retry: _, ref error_code, ref error_data } => { + &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, ref error_data, .. } => { + assert_eq!(payment_id_2, *payment_id.as_ref().unwrap()); assert_eq!(payment_hash_2.clone(), *payment_hash); assert_eq!(*rejected_by_dest, false); assert_eq!(*all_paths_failed, true); diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 419d5c158c6..f3681946d19 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -162,7 +162,7 @@ fn run_onion_failure_test_with_fail_intercept(_name: &str, test_case: let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); - if let &Event::PaymentPathFailed { payment_hash: _, ref rejected_by_dest, ref network_update, ref all_paths_failed, path: _, ref short_channel_id, retry: _, ref error_code, error_data: _ } = &events[0] { + if let &Event::PaymentPathFailed { ref rejected_by_dest, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, .. } = &events[0] { assert_eq!(*rejected_by_dest, !expected_retryable); assert_eq!(*all_paths_failed, true); assert_eq!(*error_code, expected_error_code); diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index 216cb45d4bd..364c9fd8865 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -1806,6 +1806,7 @@ mod tests { assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none()); net_graph_msg_handler.handle_event(&Event::PaymentPathFailed { + payment_id: None, payment_hash: PaymentHash([0; 32]), rejected_by_dest: false, all_paths_failed: true, @@ -1832,6 +1833,7 @@ mod tests { }; net_graph_msg_handler.handle_event(&Event::PaymentPathFailed { + payment_id: None, payment_hash: PaymentHash([0; 32]), rejected_by_dest: false, all_paths_failed: true, @@ -1857,6 +1859,7 @@ mod tests { // Permanent closing deletes a channel { net_graph_msg_handler.handle_event(&Event::PaymentPathFailed { + payment_id: None, payment_hash: PaymentHash([0; 32]), rejected_by_dest: false, all_paths_failed: true, diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index a311bd52c82..148bd096b2b 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -197,6 +197,12 @@ pub enum Event { /// Indicates an outbound payment we made failed. Probably some intermediary node dropped /// something. You may wish to retry with a different route. PaymentPathFailed { + /// The id returned by [`ChannelManager::send_payment`] and used with + /// [`ChannelManager::retry_payment`]. + /// + /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment + /// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment + payment_id: Option, /// The hash which was given to ChannelManager::send_payment. payment_hash: PaymentHash, /// Indicates the payment was rejected for some reason by the recipient. This implies that @@ -338,7 +344,7 @@ impl Writeable for Event { }); }, &Event::PaymentPathFailed { - ref payment_hash, ref rejected_by_dest, ref network_update, + ref payment_id, ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, ref path, ref short_channel_id, ref retry, #[cfg(test)] ref error_code, @@ -358,6 +364,7 @@ impl Writeable for Event { (5, path, vec_type), (7, short_channel_id, option), (9, retry, option), + (11, payment_id, option), }); }, &Event::PendingHTLCsForwardable { time_forwardable: _ } => { @@ -473,6 +480,7 @@ impl MaybeReadable for Event { let mut path: Option> = Some(vec![]); let mut short_channel_id = None; let mut retry = None; + let mut payment_id = None; read_tlv_fields!(reader, { (0, payment_hash, required), (1, network_update, ignorable), @@ -481,8 +489,10 @@ impl MaybeReadable for Event { (5, path, vec_type), (7, short_channel_id, option), (9, retry, option), + (11, payment_id, option), }); Ok(Some(Event::PaymentPathFailed { + payment_id, payment_hash, rejected_by_dest, network_update, From 5feb4dc9c4aea77be83427c66f7ec93551753d26 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 4 Oct 2021 09:20:49 -0500 Subject: [PATCH 4/9] Rewrite Invoice's interface in terms of msats InvoiceBuilder's interface was changed recently to work in terms of msats. Update Invoice's interface to return the amount in msats, too, and make amount_pico_btc private. --- lightning-invoice/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 0f125362c3a..f81af862364 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -1219,8 +1219,13 @@ impl Invoice { self.signed_invoice.currency() } + /// Returns the amount if specified in the invoice as millisatoshis. + pub fn amount_milli_satoshis(&self) -> Option { + self.signed_invoice.amount_pico_btc().map(|v| v / 10) + } + /// Returns the amount if specified in the invoice as pico . - pub fn amount_pico_btc(&self) -> Option { + fn amount_pico_btc(&self) -> Option { self.signed_invoice.amount_pico_btc() } } @@ -1867,6 +1872,7 @@ mod test { assert!(invoice.check_signature().is_ok()); assert_eq!(invoice.tagged_fields().count(), 10); + assert_eq!(invoice.amount_milli_satoshis(), Some(123)); assert_eq!(invoice.amount_pico_btc(), Some(1230)); assert_eq!(invoice.currency(), Currency::BitcoinTestnet); assert_eq!( From 2d102a3065d7a438d5c88fffcad36545a0822bfa Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 25 Oct 2021 18:48:52 -0500 Subject: [PATCH 5/9] Unify route finding methods An upcoming Router interface will be used for finding a Route both when initially sending a payment and also when retrying failed payment paths. Unify the three varieties of get_route so the interface can consist of a single method implemented by the new `find_route` method. Give get_route pub(crate) visibility so it can still be used in tests. --- fuzz/src/full_stack.rs | 20 +++++-- fuzz/src/router.rs | 12 ++-- lightning-invoice/src/utils.rs | 22 ++++--- lightning/src/ln/channelmanager.rs | 53 +++++++++++------ lightning/src/ln/features.rs | 8 +-- lightning/src/ln/functional_tests.rs | 22 ++++--- lightning/src/routing/router.rs | 86 ++++++++++++---------------- lightning/src/routing/scorer.rs | 10 ++-- lightning/src/util/events.rs | 10 ++-- 9 files changed, 135 insertions(+), 108 deletions(-) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index b2feaa695a5..b01506871ec 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -38,7 +38,7 @@ use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor,Ig use lightning::ln::msgs::DecodeError; use lightning::ln::script::ShutdownScript; use lightning::routing::network_graph::{NetGraphMsgHandler, NetworkGraph}; -use lightning::routing::router::{get_route, Payee}; +use lightning::routing::router::{find_route, Payee, RouteParameters}; use lightning::routing::scorer::Scorer; use lightning::util::config::UserConfig; use lightning::util::errors::APIError; @@ -437,9 +437,14 @@ pub fn do_test(data: &[u8], logger: &Arc) { } }, 4 => { - let value = slice_to_be24(get_slice!(3)) as u64; + let final_value_msat = slice_to_be24(get_slice!(3)) as u64; let payee = Payee::new(get_pubkey!()); - let route = match get_route(&our_id, &payee, &net_graph_msg_handler.network_graph, None, value, 42, Arc::clone(&logger), &scorer) { + let params = RouteParameters { + payee, + final_value_msat, + final_cltv_expiry_delta: 42, + }; + let route = match find_route(&our_id, ¶ms, &net_graph_msg_handler.network_graph, None, Arc::clone(&logger), &scorer) { Ok(route) => route, Err(_) => return, }; @@ -455,9 +460,14 @@ pub fn do_test(data: &[u8], logger: &Arc) { } }, 15 => { - let value = slice_to_be24(get_slice!(3)) as u64; + let final_value_msat = slice_to_be24(get_slice!(3)) as u64; let payee = Payee::new(get_pubkey!()); - let mut route = match get_route(&our_id, &payee, &net_graph_msg_handler.network_graph, None, value, 42, Arc::clone(&logger), &scorer) { + let params = RouteParameters { + payee, + final_value_msat, + final_cltv_expiry_delta: 42, + }; + let mut route = match find_route(&our_id, ¶ms, &net_graph_msg_handler.network_graph, None, Arc::clone(&logger), &scorer) { Ok(route) => route, Err(_) => return, }; diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 6f54aa02df7..7f7d9585cc2 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -16,7 +16,7 @@ use lightning::chain::transaction::OutPoint; use lightning::ln::channelmanager::{ChannelDetails, ChannelCounterparty}; use lightning::ln::features::InitFeatures; use lightning::ln::msgs; -use lightning::routing::router::{get_route, Payee, RouteHint, RouteHintHop}; +use lightning::routing::router::{find_route, Payee, RouteHint, RouteHintHop, RouteParameters}; use lightning::routing::scorer::Scorer; use lightning::util::logger::Logger; use lightning::util::ser::Readable; @@ -250,10 +250,14 @@ pub fn do_test(data: &[u8], out: Out) { } let scorer = Scorer::new(0); for target in node_pks.iter() { - let payee = Payee::new(*target).with_route_hints(last_hops.clone()); - let _ = get_route(&our_pubkey, &payee, &net_graph, + let params = RouteParameters { + payee: Payee::new(*target).with_route_hints(last_hops.clone()), + final_value_msat: slice_to_be64(get_slice!(8)), + final_cltv_expiry_delta: slice_to_be32(get_slice!(4)), + }; + let _ = find_route(&our_pubkey, ¶ms, &net_graph, first_hops.map(|c| c.iter().collect::>()).as_ref().map(|a| a.as_slice()), - slice_to_be64(get_slice!(8)), slice_to_be32(get_slice!(4)), Arc::clone(&logger), &scorer); + Arc::clone(&logger), &scorer); } }, } diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index c9046abc062..02bb31dfaf4 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -97,7 +97,7 @@ mod test { use lightning::ln::functional_test_utils::*; use lightning::ln::features::InitFeatures; use lightning::ln::msgs::ChannelMessageHandler; - use lightning::routing::router; + use lightning::routing::router::{Payee, RouteParameters, find_route}; use lightning::routing::scorer::Scorer; use lightning::util::events::MessageSendEventsProvider; use lightning::util::test_utils; @@ -113,23 +113,21 @@ mod test { assert_eq!(invoice.min_final_cltv_expiry(), MIN_FINAL_CLTV_EXPIRY as u64); assert_eq!(invoice.description(), InvoiceDescription::Direct(&Description("test".to_string()))); - let payee = router::Payee::new(invoice.recover_payee_pub_key()) + let payee = Payee::new(invoice.recover_payee_pub_key()) .with_features(invoice.features().unwrap().clone()) .with_route_hints(invoice.route_hints()); - let amt_msat = invoice.amount_pico_btc().unwrap() / 10; + let params = RouteParameters { + payee, + final_value_msat: invoice.amount_milli_satoshis().unwrap(), + final_cltv_expiry_delta: invoice.min_final_cltv_expiry() as u32, + }; let first_hops = nodes[0].node.list_usable_channels(); let network_graph = &nodes[0].net_graph_msg_handler.network_graph; let logger = test_utils::TestLogger::new(); let scorer = Scorer::new(0); - let route = router::get_route( - &nodes[0].node.get_our_node_id(), - &payee, - network_graph, - Some(&first_hops.iter().collect::>()), - amt_msat, - invoice.min_final_cltv_expiry() as u32, - &logger, - &scorer, + let route = find_route( + &nodes[0].node.get_our_node_id(), ¶ms, network_graph, + Some(&first_hops.iter().collect::>()), &logger, &scorer, ).unwrap(); let payment_event = { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b93e6671ad6..90f1a2a91b2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -45,7 +45,7 @@ use chain::transaction::{OutPoint, TransactionData}; use ln::{PaymentHash, PaymentPreimage, PaymentSecret}; use ln::channel::{Channel, ChannelError, ChannelUpdateStatus, UpdateFulfillCommitFetch}; use ln::features::{InitFeatures, NodeFeatures}; -use routing::router::{Payee, PaymentPathRetry, Route, RouteHop}; +use routing::router::{Payee, Route, RouteHop, RouteParameters}; use ln::msgs; use ln::msgs::NetAddress; use ln::onion_utils; @@ -3112,7 +3112,7 @@ impl ChannelMana !payment.get().is_fulfilled() { let retry = if let Some(payee_data) = payee { - Some(PaymentPathRetry { + Some(RouteParameters { payee: payee_data, final_value_msat: path_last_hop.fee_msat, final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta, @@ -3183,7 +3183,7 @@ impl ChannelMana } mem::drop(channel_state_lock); let retry = if let Some(payee_data) = payee { - Some(PaymentPathRetry { + Some(RouteParameters { payee: payee_data.clone(), final_value_msat: path_last_hop.fee_msat, final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta, @@ -6036,15 +6036,14 @@ mod tests { use core::time::Duration; use ln::{PaymentPreimage, PaymentHash, PaymentSecret}; use ln::channelmanager::{PaymentId, PaymentSendFailure}; - use ln::features::{InitFeatures, InvoiceFeatures}; + use ln::features::InitFeatures; use ln::functional_test_utils::*; use ln::msgs; use ln::msgs::ChannelMessageHandler; - use routing::router::{Payee, get_keysend_route, get_route}; + use routing::router::{Payee, RouteParameters, find_route}; use routing::scorer::Scorer; use util::errors::APIError; use util::events::{Event, MessageSendEvent, MessageSendEventsProvider}; - use util::test_utils; #[cfg(feature = "std")] #[test] @@ -6280,7 +6279,6 @@ mod tests { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); - let logger = test_utils::TestLogger::new(); let scorer = Scorer::new(0); // To start (1), send a regular payment but don't claim it. @@ -6288,9 +6286,15 @@ mod tests { let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &expected_route, 100_000); // Next, attempt a keysend payment and make sure it fails. - let payee = Payee::new(expected_route.last().unwrap().node.get_our_node_id()) - .with_features(InvoiceFeatures::known()); - let route = get_route(&nodes[0].node.get_our_node_id(), &payee, &nodes[0].net_graph_msg_handler.network_graph, None, 100_000, TEST_FINAL_CLTV, &logger, &scorer).unwrap(); + let params = RouteParameters { + payee: Payee::for_keysend(expected_route.last().unwrap().node.get_our_node_id()), + final_value_msat: 100_000, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + let route = find_route( + &nodes[0].node.get_our_node_id(), ¶ms, + &nodes[0].net_graph_msg_handler.network_graph, None, nodes[0].logger, &scorer + ).unwrap(); nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap(); check_added_monitors!(nodes[0], 1); let mut events = nodes[0].node.get_and_clear_pending_msg_events(); @@ -6318,7 +6322,10 @@ mod tests { // To start (2), send a keysend payment but don't claim it. let payment_preimage = PaymentPreimage([42; 32]); - let route = get_route(&nodes[0].node.get_our_node_id(), &payee, &nodes[0].net_graph_msg_handler.network_graph, None, 100_000, TEST_FINAL_CLTV, &logger, &scorer).unwrap(); + let route = find_route( + &nodes[0].node.get_our_node_id(), ¶ms, + &nodes[0].net_graph_msg_handler.network_graph, None, nodes[0].logger, &scorer + ).unwrap(); let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap(); check_added_monitors!(nodes[0], 1); let mut events = nodes[0].node.get_and_clear_pending_msg_events(); @@ -6370,12 +6377,18 @@ mod tests { nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known() }); let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known()); + let params = RouteParameters { + payee: Payee::for_keysend(payee_pubkey), + final_value_msat: 10000, + final_cltv_expiry_delta: 40, + }; let network_graph = &nodes[0].net_graph_msg_handler.network_graph; let first_hops = nodes[0].node.list_usable_channels(); let scorer = Scorer::new(0); - let route = get_keysend_route(&payer_pubkey, network_graph, &payee_pubkey, - Some(&first_hops.iter().collect::>()), &vec![], 10000, 40, - nodes[0].logger, &scorer).unwrap(); + let route = find_route( + &payer_pubkey, ¶ms, network_graph, Some(&first_hops.iter().collect::>()), + nodes[0].logger, &scorer + ).unwrap(); let test_preimage = PaymentPreimage([42; 32]); let mismatch_payment_hash = PaymentHash([43; 32]); @@ -6407,12 +6420,18 @@ mod tests { nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known() }); let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known()); + let params = RouteParameters { + payee: Payee::for_keysend(payee_pubkey), + final_value_msat: 10000, + final_cltv_expiry_delta: 40, + }; let network_graph = &nodes[0].net_graph_msg_handler.network_graph; let first_hops = nodes[0].node.list_usable_channels(); let scorer = Scorer::new(0); - let route = get_keysend_route(&payer_pubkey, network_graph, &payee_pubkey, - Some(&first_hops.iter().collect::>()), &vec![], 10000, 40, - nodes[0].logger, &scorer).unwrap(); + let route = find_route( + &payer_pubkey, ¶ms, network_graph, Some(&first_hops.iter().collect::>()), + nodes[0].logger, &scorer + ).unwrap(); let test_preimage = PaymentPreimage([42; 32]); let test_secret = PaymentSecret([43; 32]); diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 8f251d8478c..888dcd3ac0c 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -432,11 +432,11 @@ impl InvoiceFeatures { /// Getting a route for a keysend payment to a private node requires providing the payee's /// features (since they were not announced in a node announcement). However, keysend payments /// don't have an invoice to pull the payee's features from, so this method is provided for use in - /// [`get_keysend_route`], thus omitting the need for payers to manually construct an - /// `InvoiceFeatures` for [`get_route`]. + /// [`Payee::for_keysend`], thus omitting the need for payers to manually construct an + /// `InvoiceFeatures` for [`find_route`]. /// - /// [`get_keysend_route`]: crate::routing::router::get_keysend_route - /// [`get_route`]: crate::routing::router::get_route + /// [`Payee::for_keysend`]: crate::routing::router::Payee::for_keysend + /// [`find_route`]: crate::routing::router::find_route pub(crate) fn for_keysend() -> InvoiceFeatures { InvoiceFeatures::empty().set_variable_length_onion_optional() } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 798a6f4435d..564a9e5a090 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -24,7 +24,7 @@ use ln::channel::{Channel, ChannelError}; use ln::{chan_utils, onion_utils}; use ln::chan_utils::HTLC_SUCCESS_TX_WEIGHT; use routing::network_graph::{NetworkUpdate, RoutingFees}; -use routing::router::{Payee, Route, RouteHop, RouteHint, RouteHintHop, get_route, get_keysend_route}; +use routing::router::{Payee, Route, RouteHop, RouteHint, RouteHintHop, RouteParameters, find_route, get_route}; use routing::scorer::Scorer; use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures}; use ln::msgs; @@ -9056,10 +9056,13 @@ fn test_keysend_payments_to_public_node() { let network_graph = &nodes[0].net_graph_msg_handler.network_graph; let payer_pubkey = nodes[0].node.get_our_node_id(); let payee_pubkey = nodes[1].node.get_our_node_id(); + let params = RouteParameters { + payee: Payee::for_keysend(payee_pubkey), + final_value_msat: 10000, + final_cltv_expiry_delta: 40, + }; let scorer = Scorer::new(0); - let route = get_keysend_route( - &payer_pubkey, &network_graph, &payee_pubkey, None, &vec![], 10000, 40, nodes[0].logger, &scorer - ).unwrap(); + let route = find_route(&payer_pubkey, ¶ms, &network_graph, None, nodes[0].logger, &scorer).unwrap(); let test_preimage = PaymentPreimage([42; 32]); let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(test_preimage)).unwrap(); @@ -9085,12 +9088,17 @@ fn test_keysend_payments_to_private_node() { nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known() }); let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known()); + let params = RouteParameters { + payee: Payee::for_keysend(payee_pubkey), + final_value_msat: 10000, + final_cltv_expiry_delta: 40, + }; let network_graph = &nodes[0].net_graph_msg_handler.network_graph; let first_hops = nodes[0].node.list_usable_channels(); let scorer = Scorer::new(0); - let route = get_keysend_route( - &payer_pubkey, &network_graph, &payee_pubkey, Some(&first_hops.iter().collect::>()), - &vec![], 10000, 40, nodes[0].logger, &scorer + let route = find_route( + &payer_pubkey, ¶ms, &network_graph, Some(&first_hops.iter().collect::>()), + nodes[0].logger, &scorer ).unwrap(); let test_preimage = PaymentPreimage([42; 32]); diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 5de71fb6cf1..ec49b21058b 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -70,7 +70,7 @@ pub struct Route { /// given path is variable, keeping the length of any path to less than 20 should currently /// ensure it is viable. pub paths: Vec>, - /// The `payee` parameter passed to [`get_route`]. + /// The `payee` parameter passed to [`find_route`]. /// This is used by `ChannelManager` to track information which may be required for retries, /// provided back to you via [`Event::PaymentPathFailed`]. /// @@ -82,7 +82,7 @@ impl Route { /// Returns the total amount of fees paid on this [`Route`]. /// /// This doesn't include any extra payment made to the recipient, which can happen in excess of - /// the amount passed to [`get_route`]'s `final_value_msat`. + /// the amount passed to [`find_route`]'s `params.final_value_msat`. pub fn get_total_fees(&self) -> u64 { // Do not count last hop of each path since that's the full value of the payment return self.paths.iter() @@ -140,13 +140,14 @@ impl Readable for Route { } } -/// Parameters needed to re-compute a [`Route`] for retrying a failed payment path. +/// Parameters needed to find a [`Route`] for paying a [`Payee`]. /// -/// Provided in [`Event::PaymentPathFailed`] and passed to [`get_retry_route`]. +/// Passed to [`find_route`] and also provided in [`Event::PaymentPathFailed`] for retrying a failed +/// payment path. /// /// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed #[derive(Clone, Debug)] -pub struct PaymentPathRetry { +pub struct RouteParameters { /// The recipient of the failed payment path. pub payee: Payee, @@ -157,7 +158,7 @@ pub struct PaymentPathRetry { pub final_cltv_expiry_delta: u32, } -impl_writeable_tlv_based!(PaymentPathRetry, { +impl_writeable_tlv_based!(RouteParameters, { (0, payee, required), (2, final_value_msat, required), (4, final_cltv_expiry_delta, required), @@ -479,59 +480,46 @@ fn compute_fees(amount_msat: u64, channel_fees: RoutingFees) -> Option { } } -/// Gets a keysend route from us (payer) to the given target node (payee). This is needed because -/// keysend payments do not have an invoice from which to pull the payee's supported features, which -/// makes it tricky to otherwise supply the `payee` parameter of `get_route`. -pub fn get_keysend_route( - our_node_pubkey: &PublicKey, network: &NetworkGraph, payee: &PublicKey, - first_hops: Option<&[&ChannelDetails]>, last_hops: &[&RouteHint], final_value_msat: u64, - final_cltv_expiry_delta: u32, logger: L, scorer: &S -) -> Result -where L::Target: Logger { - let route_hints = last_hops.iter().map(|hint| (*hint).clone()).collect(); - let payee = Payee::for_keysend(*payee).with_route_hints(route_hints); - get_route( - our_node_pubkey, &payee, network, first_hops, final_value_msat, final_cltv_expiry_delta, - logger, scorer - ) -} - -/// Gets a route suitable for retrying a failed payment path. +/// Finds a route from us (payer) to the given target node (payee). +/// +/// If the payee provided features in their invoice, they should be provided via `params.payee`. +/// Without this, MPP will only be used if the payee's features are available in the network graph. +/// +/// Private routing paths between a public node and the target may be included in `params.payee`. +/// +/// If some channels aren't announced, it may be useful to fill in `first_hops` with the results +/// from [`ChannelManager::list_usable_channels`]. If it is filled in, the view of our local +/// channels from [`NetworkGraph`] will be ignored, and only those in `first_hops` will be used. /// -/// Used to re-compute a [`Route`] when handling a [`Event::PaymentPathFailed`]. Any adjustments to -/// the [`NetworkGraph`] and channel scores should be made prior to calling this function. +/// The fees on channels from us to the next hop are ignored as they are assumed to all be equal. +/// However, the enabled/disabled bit on such channels as well as the `htlc_minimum_msat` / +/// `htlc_maximum_msat` *are* checked as they may change based on the receiving node. /// +/// # Note +/// +/// May be used to re-compute a [`Route`] when handling a [`Event::PaymentPathFailed`]. Any +/// adjustments to the [`NetworkGraph`] and channel scores should be made prior to calling this +/// function. +/// +/// # Panics +/// +/// Panics if first_hops contains channels without short_channel_ids; +/// [`ChannelManager::list_usable_channels`] will never include such channels. +/// +/// [`ChannelManager::list_usable_channels`]: crate::ln::channelmanager::ChannelManager::list_usable_channels /// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed -pub fn get_retry_route( - our_node_pubkey: &PublicKey, retry: &PaymentPathRetry, network: &NetworkGraph, +pub fn find_route( + our_node_pubkey: &PublicKey, params: &RouteParameters, network: &NetworkGraph, first_hops: Option<&[&ChannelDetails]>, logger: L, scorer: &S ) -> Result where L::Target: Logger { get_route( - our_node_pubkey, &retry.payee, network, first_hops, retry.final_value_msat, - retry.final_cltv_expiry_delta, logger, scorer + our_node_pubkey, ¶ms.payee, network, first_hops, params.final_value_msat, + params.final_cltv_expiry_delta, logger, scorer ) } -/// Gets a route from us (payer) to the given target node (payee). -/// -/// If the payee provided features in their invoice, they should be provided via `payee`. Without -/// this, MPP will only be used if the payee's features are available in the network graph. -/// -/// Private routing paths between a public node and the target may be included in `payee`. -/// -/// If some channels aren't announced, it may be useful to fill in a first_hops with the -/// results from a local ChannelManager::list_usable_channels() call. If it is filled in, our -/// view of our local channels (from net_graph_msg_handler) will be ignored, and only those -/// in first_hops will be used. -/// -/// Panics if first_hops contains channels without short_channel_ids -/// (ChannelManager::list_usable_channels will never include such channels). -/// -/// The fees on channels from us to next-hops are ignored (as they are assumed to all be -/// equal), however the enabled/disabled bit on such channels as well as the -/// htlc_minimum_msat/htlc_maximum_msat *are* checked as they may change based on the receiving node. -pub fn get_route( +pub(crate) fn get_route( our_node_pubkey: &PublicKey, payee: &Payee, network: &NetworkGraph, first_hops: Option<&[&ChannelDetails]>, final_value_msat: u64, final_cltv_expiry_delta: u32, logger: L, scorer: &S diff --git a/lightning/src/routing/scorer.rs b/lightning/src/routing/scorer.rs index 841d4bdbaba..e3f5c8679b6 100644 --- a/lightning/src/routing/scorer.rs +++ b/lightning/src/routing/scorer.rs @@ -9,7 +9,7 @@ //! Utilities for scoring payment channels. //! -//! [`Scorer`] may be given to [`get_route`] to score payment channels during path finding when a +//! [`Scorer`] may be given to [`find_route`] to score payment channels during path finding when a //! custom [`routing::Score`] implementation is not needed. //! //! # Example @@ -18,7 +18,7 @@ //! # extern crate secp256k1; //! # //! # use lightning::routing::network_graph::NetworkGraph; -//! # use lightning::routing::router::{Payee, get_route}; +//! # use lightning::routing::router::{RouteParameters, find_route}; //! # use lightning::routing::scorer::Scorer; //! # use lightning::util::logger::{Logger, Record}; //! # use secp256k1::key::PublicKey; @@ -27,7 +27,7 @@ //! # impl Logger for FakeLogger { //! # fn log(&self, record: &Record) { unimplemented!() } //! # } -//! # fn find_scored_route(payer: PublicKey, payee: Payee, network_graph: NetworkGraph) { +//! # fn find_scored_route(payer: PublicKey, params: RouteParameters, network_graph: NetworkGraph) { //! # let logger = FakeLogger {}; //! # //! // Use the default channel penalty. @@ -36,11 +36,11 @@ //! // Or use a custom channel penalty. //! let scorer = Scorer::new(1_000); //! -//! let route = get_route(&payer, &payee, &network_graph, None, 1_000, 42, &logger, &scorer); +//! let route = find_route(&payer, ¶ms, &network_graph, None, &logger, &scorer); //! # } //! ``` //! -//! [`get_route`]: crate::routing::router::get_route +//! [`find_route`]: crate::routing::router::find_route use routing; diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 148bd096b2b..e9f456cfe69 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -21,7 +21,7 @@ use ln::msgs::DecodeError; use ln::{PaymentPreimage, PaymentHash, PaymentSecret}; use routing::network_graph::NetworkUpdate; use util::ser::{BigSize, FixedLengthReader, Writeable, Writer, MaybeReadable, Readable, VecReadWrapper, VecWriteWrapper}; -use routing::router::{PaymentPathRetry, RouteHop}; +use routing::router::{RouteHop, RouteParameters}; use bitcoin::blockdata::script::Script; use bitcoin::hashes::Hash; @@ -229,13 +229,13 @@ pub enum Event { /// If this is `Some`, then the corresponding channel should be avoided when the payment is /// retried. May be `None` for older [`Event`] serializations. short_channel_id: Option, - /// Parameters needed to re-compute a [`Route`] for retrying the failed path. + /// Parameters needed to compute a new [`Route`] when retrying the failed payment path. /// - /// See [`get_retry_route`] for details. + /// See [`find_route`] for details. /// /// [`Route`]: crate::routing::router::Route - /// [`get_retry_route`]: crate::routing::router::get_retry_route - retry: Option, + /// [`find_route`]: crate::routing::router::find_route + retry: Option, #[cfg(test)] error_code: Option, #[cfg(test)] From ad4f16b3e60bbc074423639fe5cfdb9f7174e1e9 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 24 Aug 2021 00:08:15 -0500 Subject: [PATCH 6/9] Add InvoicePayer for retrying failed payments When a payment fails, it's useful to retry the payment once the network graph and channel scores are updated. InvoicePayer is a utility for making payments which will retry any failed payment paths for a payment up to a configured number of total attempts. It is parameterized by a Payer and Router for ease of customization and testing. Implement EventHandler for InvoicePayer as a decorator that intercepts PaymentPathFailed events and retries that payment using the parameters from the event. It delegates to the decorated EventHandler after retries have been exhausted and for other events. --- lightning-invoice/src/lib.rs | 3 +- lightning-invoice/src/payment.rs | 846 +++++++++++++++++++++++++++++++ lightning/src/util/events.rs | 9 +- 3 files changed, 856 insertions(+), 2 deletions(-) create mode 100644 lightning-invoice/src/payment.rs diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index f81af862364..5388321694d 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -15,11 +15,12 @@ //! * For parsing use `str::parse::(&self)` (see the docs of `impl FromStr for Invoice`) //! * For constructing invoices use the `InvoiceBuilder` //! * For serializing invoices use the `Display`/`ToString` traits +pub mod payment; pub mod utils; extern crate bech32; extern crate bitcoin_hashes; -extern crate lightning; +#[macro_use] extern crate lightning; extern crate num_traits; extern crate secp256k1; diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs new file mode 100644 index 00000000000..61a0e350ad1 --- /dev/null +++ b/lightning-invoice/src/payment.rs @@ -0,0 +1,846 @@ +// 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. + +//! A module for paying Lightning invoices. +//! +//! Defines an [`InvoicePayer`] utility for paying invoices, parameterized by [`Payer`] and +//! [`Router`] traits. Implementations of [`Payer`] provide the payer's node id, channels, and means +//! to send a payment over a [`Route`]. Implementations of [`Router`] find a [`Route`] between payer +//! and payee using information provided by the payer and from the payee's [`Invoice`]. +//! +//! [`InvoicePayer`] is capable of retrying failed payments. It accomplishes this by implementing +//! [`EventHandler`] which decorates a user-provided handler. It will intercept any +//! [`Event::PaymentPathFailed`] events and retry the failed paths for a fixed number of total +//! attempts or until retry is no longer possible. In such a situation, [`InvoicePayer`] will pass +//! along the events to the user-provided handler. +//! +//! # Example +//! +//! ``` +//! # extern crate lightning; +//! # extern crate lightning_invoice; +//! # extern crate secp256k1; +//! # +//! # use lightning::ln::{PaymentHash, PaymentSecret}; +//! # use lightning::ln::channelmanager::{ChannelDetails, PaymentId, PaymentSendFailure}; +//! # use lightning::ln::msgs::LightningError; +//! # use lightning::routing::router::{Route, RouteParameters}; +//! # use lightning::util::events::{Event, EventHandler, EventsProvider}; +//! # use lightning::util::logger::{Logger, Record}; +//! # use lightning_invoice::Invoice; +//! # use lightning_invoice::payment::{InvoicePayer, Payer, RetryAttempts, Router}; +//! # use secp256k1::key::PublicKey; +//! # use std::ops::Deref; +//! # +//! # struct FakeEventProvider {} +//! # impl EventsProvider for FakeEventProvider { +//! # fn process_pending_events(&self, handler: H) where H::Target: EventHandler {} +//! # } +//! # +//! # struct FakePayer {} +//! # impl Payer for FakePayer { +//! # fn node_id(&self) -> PublicKey { unimplemented!() } +//! # fn first_hops(&self) -> Vec { unimplemented!() } +//! # fn send_payment( +//! # &self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option +//! # ) -> Result { unimplemented!() } +//! # fn retry_payment( +//! # &self, route: &Route, payment_id: PaymentId +//! # ) -> Result<(), PaymentSendFailure> { unimplemented!() } +//! # } +//! # +//! # struct FakeRouter {}; +//! # impl Router for FakeRouter { +//! # fn find_route( +//! # &self, payer: &PublicKey, params: &RouteParameters, +//! # first_hops: Option<&[&ChannelDetails]> +//! # ) -> Result { unimplemented!() } +//! # } +//! # +//! # struct FakeLogger {}; +//! # impl Logger for FakeLogger { +//! # fn log(&self, record: &Record) { unimplemented!() } +//! # } +//! # +//! # fn main() { +//! let event_handler = |event: &Event| { +//! match event { +//! Event::PaymentPathFailed { .. } => println!("payment failed after retries"), +//! Event::PaymentSent { .. } => println!("payment successful"), +//! _ => {}, +//! } +//! }; +//! # let payer = FakePayer {}; +//! # let router = FakeRouter {}; +//! # let logger = FakeLogger {}; +//! let invoice_payer = InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(2)); +//! +//! let invoice = "..."; +//! let invoice = invoice.parse::().unwrap(); +//! invoice_payer.pay_invoice(&invoice).unwrap(); +//! +//! # let event_provider = FakeEventProvider {}; +//! loop { +//! event_provider.process_pending_events(&invoice_payer); +//! } +//! # } +//! ``` +//! +//! # Note +//! +//! The [`Route`] is computed before each payment attempt. Any updates affecting path finding such +//! as updates to the network graph or changes to channel scores should be applied prior to +//! retries, typically by way of composing [`EventHandler`]s accordingly. + +use crate::Invoice; + +use bitcoin_hashes::Hash; + +use lightning::ln::{PaymentHash, PaymentSecret}; +use lightning::ln::channelmanager::{ChannelDetails, PaymentId, PaymentSendFailure}; +use lightning::ln::msgs::LightningError; +use lightning::routing::router::{Payee, Route, RouteParameters}; +use lightning::util::events::{Event, EventHandler}; +use lightning::util::logger::Logger; + +use secp256k1::key::PublicKey; + +use std::collections::hash_map::{self, HashMap}; +use std::ops::Deref; +use std::sync::Mutex; + +/// A utility for paying [`Invoice]`s. +pub struct InvoicePayer +where + P::Target: Payer, + R: Router, + L::Target: Logger, + E: EventHandler, +{ + payer: P, + router: R, + logger: L, + event_handler: E, + payment_cache: Mutex>, + retry_attempts: RetryAttempts, +} + +/// A trait defining behavior of an [`Invoice`] payer. +pub trait Payer { + /// Returns the payer's node id. + fn node_id(&self) -> PublicKey; + + /// Returns the payer's channels. + fn first_hops(&self) -> Vec; + + /// Sends a payment over the Lightning Network using the given [`Route`]. + fn send_payment( + &self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option + ) -> Result; + + /// Retries a failed payment path for the [`PaymentId`] using the given [`Route`]. + fn retry_payment(&self, route: &Route, payment_id: PaymentId) -> Result<(), PaymentSendFailure>; +} + +/// A trait defining behavior for routing an [`Invoice`] payment. +pub trait Router { + /// Finds a [`Route`] between `payer` and `payee` for a payment with the given values. + fn find_route( + &self, payer: &PublicKey, params: &RouteParameters, first_hops: Option<&[&ChannelDetails]> + ) -> Result; +} + +/// Number of attempts to retry payment path failures for an [`Invoice`]. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub struct RetryAttempts(pub usize); + +/// An error that may occur when making a payment. +#[derive(Clone, Debug)] +pub enum PaymentError { + /// An error resulting from the provided [`Invoice`] or payment hash. + Invoice(&'static str), + /// An error occurring when finding a route. + Routing(LightningError), + /// An error occurring when sending a payment. + Sending(PaymentSendFailure), +} + +impl InvoicePayer +where + P::Target: Payer, + R: Router, + L::Target: Logger, + E: EventHandler, +{ + /// Creates an invoice payer that retries failed payment paths. + /// + /// Will forward any [`Event::PaymentPathFailed`] events to the decorated `event_handler` once + /// `retry_attempts` has been exceeded for a given [`Invoice`]. + pub fn new( + payer: P, router: R, logger: L, event_handler: E, retry_attempts: RetryAttempts + ) -> Self { + Self { + payer, + router, + logger, + event_handler, + payment_cache: Mutex::new(HashMap::new()), + retry_attempts, + } + } + + /// Pays the given [`Invoice`], caching it for later use in case a retry is needed. + pub fn pay_invoice(&self, invoice: &Invoice) -> Result { + let payment_hash = PaymentHash(invoice.payment_hash().clone().into_inner()); + let mut payment_cache = self.payment_cache.lock().unwrap(); + match payment_cache.entry(payment_hash) { + hash_map::Entry::Vacant(entry) => { + let payer = self.payer.node_id(); + let mut payee = Payee::new(invoice.recover_payee_pub_key()) + .with_route_hints(invoice.route_hints()); + if let Some(features) = invoice.features() { + payee = payee.with_features(features.clone()); + } + let final_value_msat = invoice.amount_milli_satoshis() + .ok_or(PaymentError::Invoice("amount missing"))?; + let params = RouteParameters { + payee, + final_value_msat, + final_cltv_expiry_delta: invoice.min_final_cltv_expiry() as u32, + }; + let first_hops = self.payer.first_hops(); + let route = self.router.find_route( + &payer, + ¶ms, + Some(&first_hops.iter().collect::>()), + ).map_err(|e| PaymentError::Routing(e))?; + + let payment_hash = PaymentHash(invoice.payment_hash().clone().into_inner()); + let payment_secret = Some(invoice.payment_secret().clone()); + let payment_id = self.payer.send_payment(&route, payment_hash, &payment_secret) + .map_err(|e| PaymentError::Sending(e))?; + entry.insert(0); + Ok(payment_id) + }, + hash_map::Entry::Occupied(_) => Err(PaymentError::Invoice("payment pending")), + } + } + + fn retry_payment( + &self, payment_id: PaymentId, params: &RouteParameters + ) -> Result<(), PaymentError> { + let payer = self.payer.node_id(); + let first_hops = self.payer.first_hops(); + let route = self.router.find_route( + &payer, ¶ms, Some(&first_hops.iter().collect::>()) + ).map_err(|e| PaymentError::Routing(e))?; + self.payer.retry_payment(&route, payment_id).map_err(|e| PaymentError::Sending(e)) + } + + /// Removes the payment cached by the given payment hash. + /// + /// Should be called once a payment has failed or succeeded if not using [`InvoicePayer`] as an + /// [`EventHandler`]. Otherwise, calling this method is unnecessary. + pub fn remove_cached_payment(&self, payment_hash: &PaymentHash) { + self.payment_cache.lock().unwrap().remove(payment_hash); + } +} + +impl EventHandler for InvoicePayer +where + P::Target: Payer, + R: Router, + L::Target: Logger, + E: EventHandler, +{ + fn handle_event(&self, event: &Event) { + match event { + Event::PaymentPathFailed { payment_id, payment_hash, rejected_by_dest, retry, .. } => { + let mut payment_cache = self.payment_cache.lock().unwrap(); + let entry = loop { + let entry = payment_cache.entry(*payment_hash); + match entry { + hash_map::Entry::Occupied(_) => break entry, + hash_map::Entry::Vacant(entry) => entry.insert(0), + }; + }; + if let hash_map::Entry::Occupied(mut entry) = entry { + let max_payment_attempts = self.retry_attempts.0 + 1; + let attempts = entry.get_mut(); + *attempts += 1; + + if *rejected_by_dest { + log_trace!(self.logger, "Payment {} rejected by destination; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts); + } else if payment_id.is_none() { + log_trace!(self.logger, "Payment {} has no id; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts); + } else if *attempts >= max_payment_attempts { + log_trace!(self.logger, "Payment {} exceeded maximum attempts; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts); + } else if retry.is_none() { + log_trace!(self.logger, "Payment {} missing retry params; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts); + } else if self.retry_payment(*payment_id.as_ref().unwrap(), retry.as_ref().unwrap()).is_err() { + log_trace!(self.logger, "Error retrying payment {}; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts); + } else { + log_trace!(self.logger, "Payment {} failed; retrying (attempts: {})", log_bytes!(payment_hash.0), attempts); + return; + } + + // Either the payment was rejected, the maximum attempts were exceeded, or an + // error occurred when attempting to retry. + entry.remove(); + } else { + unreachable!(); + } + }, + Event::PaymentSent { payment_hash, .. } => { + let mut payment_cache = self.payment_cache.lock().unwrap(); + let attempts = payment_cache + .remove(payment_hash) + .map_or(1, |attempts| attempts + 1); + log_trace!(self.logger, "Payment {} succeeded (attempts: {})", log_bytes!(payment_hash.0), attempts); + }, + _ => {}, + } + + // Delegate to the decorated event handler unless the payment is retried. + self.event_handler.handle_event(event) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{InvoiceBuilder, Currency}; + use bitcoin_hashes::sha256::Hash as Sha256; + use lightning::ln::PaymentPreimage; + use lightning::ln::features::{ChannelFeatures, NodeFeatures}; + use lightning::ln::msgs::{ErrorAction, LightningError}; + use lightning::routing::router::{Route, RouteHop}; + use lightning::util::test_utils::TestLogger; + use lightning::util::errors::APIError; + use lightning::util::events::Event; + use secp256k1::{SecretKey, PublicKey, Secp256k1}; + + fn invoice(payment_preimage: PaymentPreimage) -> Invoice { + let payment_hash = Sha256::hash(&payment_preimage.0); + let private_key = SecretKey::from_slice(&[42; 32]).unwrap(); + InvoiceBuilder::new(Currency::Bitcoin) + .description("test".into()) + .payment_hash(payment_hash) + .payment_secret(PaymentSecret([0; 32])) + .current_timestamp() + .min_final_cltv_expiry(144) + .amount_milli_satoshis(128) + .build_signed(|hash| { + Secp256k1::new().sign_recoverable(hash, &private_key) + }) + .unwrap() + } + + #[test] + fn pays_invoice_on_first_attempt() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + let payment_hash = PaymentHash(invoice.payment_hash().clone().into_inner()); + + let payer = TestPayer::new(); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(0)); + + let payment_id = Some(invoice_payer.pay_invoice(&invoice).unwrap()); + assert_eq!(*payer.attempts.borrow(), 1); + + invoice_payer.handle_event(&Event::PaymentSent { + payment_id, payment_preimage, payment_hash + }); + assert_eq!(*event_handled.borrow(), true); + assert_eq!(*payer.attempts.borrow(), 1); + } + + #[test] + fn pays_invoice_on_retry() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + let payment_hash = PaymentHash(invoice.payment_hash().clone().into_inner()); + let final_value_msat = invoice.amount_milli_satoshis().unwrap(); + + let payer = TestPayer::new() + .expect_value_msat(final_value_msat) + .expect_value_msat(final_value_msat / 2); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(2)); + + let payment_id = Some(invoice_payer.pay_invoice(&invoice).unwrap()); + assert_eq!(*payer.attempts.borrow(), 1); + + let event = Event::PaymentPathFailed { + payment_id, + payment_hash, + network_update: None, + rejected_by_dest: false, + all_paths_failed: false, + path: TestRouter::path_for_value(final_value_msat), + short_channel_id: None, + retry: Some(TestRouter::retry_for_invoice(&invoice)), + }; + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), false); + assert_eq!(*payer.attempts.borrow(), 2); + + invoice_payer.handle_event(&Event::PaymentSent { + payment_id, payment_preimage, payment_hash + }); + assert_eq!(*event_handled.borrow(), true); + assert_eq!(*payer.attempts.borrow(), 2); + } + + #[test] + fn retries_payment_path_for_unknown_payment() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + let payment_hash = PaymentHash(invoice.payment_hash().clone().into_inner()); + let final_value_msat = invoice.amount_milli_satoshis().unwrap(); + + let payer = TestPayer::new(); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(2)); + + let payment_id = Some(PaymentId([1; 32])); + let event = Event::PaymentPathFailed { + payment_id, + payment_hash, + network_update: None, + rejected_by_dest: false, + all_paths_failed: false, + path: TestRouter::path_for_value(final_value_msat), + short_channel_id: None, + retry: Some(TestRouter::retry_for_invoice(&invoice)), + }; + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), false); + assert_eq!(*payer.attempts.borrow(), 1); + + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), false); + assert_eq!(*payer.attempts.borrow(), 2); + + invoice_payer.handle_event(&Event::PaymentSent { + payment_id, payment_preimage, payment_hash + }); + assert_eq!(*event_handled.borrow(), true); + assert_eq!(*payer.attempts.borrow(), 2); + } + + #[test] + fn fails_paying_invoice_after_max_retries() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + let final_value_msat = invoice.amount_milli_satoshis().unwrap(); + + let payer = TestPayer::new() + .expect_value_msat(final_value_msat) + .expect_value_msat(final_value_msat / 2) + .expect_value_msat(final_value_msat / 2); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(2)); + + let payment_id = Some(invoice_payer.pay_invoice(&invoice).unwrap()); + assert_eq!(*payer.attempts.borrow(), 1); + + let event = Event::PaymentPathFailed { + payment_id, + payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), + network_update: None, + rejected_by_dest: false, + all_paths_failed: true, + path: TestRouter::path_for_value(final_value_msat), + short_channel_id: None, + retry: Some(TestRouter::retry_for_invoice(&invoice)), + }; + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), false); + assert_eq!(*payer.attempts.borrow(), 2); + + let event = Event::PaymentPathFailed { + payment_id, + payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), + network_update: None, + rejected_by_dest: false, + all_paths_failed: false, + path: TestRouter::path_for_value(final_value_msat / 2), + short_channel_id: None, + retry: Some(RouteParameters { + final_value_msat: final_value_msat / 2, ..TestRouter::retry_for_invoice(&invoice) + }), + }; + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), false); + assert_eq!(*payer.attempts.borrow(), 3); + + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), true); + assert_eq!(*payer.attempts.borrow(), 3); + } + + #[test] + fn fails_paying_invoice_with_missing_retry_params() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payer = TestPayer::new(); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(2)); + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + let payment_id = Some(invoice_payer.pay_invoice(&invoice).unwrap()); + assert_eq!(*payer.attempts.borrow(), 1); + + let event = Event::PaymentPathFailed { + payment_id, + payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), + network_update: None, + rejected_by_dest: false, + all_paths_failed: false, + path: vec![], + short_channel_id: None, + retry: None, + }; + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), true); + assert_eq!(*payer.attempts.borrow(), 1); + } + + #[test] + fn fails_paying_invoice_after_retry_error() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + let final_value_msat = invoice.amount_milli_satoshis().unwrap(); + + let payer = TestPayer::new() + .fails_on_attempt(2) + .expect_value_msat(final_value_msat); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(2)); + + let payment_id = Some(invoice_payer.pay_invoice(&invoice).unwrap()); + assert_eq!(*payer.attempts.borrow(), 1); + + let event = Event::PaymentPathFailed { + payment_id, + payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), + network_update: None, + rejected_by_dest: false, + all_paths_failed: false, + path: TestRouter::path_for_value(final_value_msat / 2), + short_channel_id: None, + retry: Some(TestRouter::retry_for_invoice(&invoice)), + }; + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), true); + assert_eq!(*payer.attempts.borrow(), 2); + } + + #[test] + fn fails_paying_invoice_after_rejected_by_payee() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payer = TestPayer::new(); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(2)); + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + let payment_id = Some(invoice_payer.pay_invoice(&invoice).unwrap()); + assert_eq!(*payer.attempts.borrow(), 1); + + let event = Event::PaymentPathFailed { + payment_id, + payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), + network_update: None, + rejected_by_dest: true, + all_paths_failed: false, + path: vec![], + short_channel_id: None, + retry: Some(TestRouter::retry_for_invoice(&invoice)), + }; + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), true); + assert_eq!(*payer.attempts.borrow(), 1); + } + + #[test] + fn fails_repaying_invoice_with_pending_payment() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payer = TestPayer::new(); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(0)); + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + let payment_id = Some(invoice_payer.pay_invoice(&invoice).unwrap()); + + // Cannot repay an invoice pending payment. + match invoice_payer.pay_invoice(&invoice) { + Err(PaymentError::Invoice("payment pending")) => {}, + Err(_) => panic!("unexpected error"), + Ok(_) => panic!("expected invoice error"), + } + + // Can repay an invoice once cleared from cache. + let payment_hash = PaymentHash(invoice.payment_hash().clone().into_inner()); + invoice_payer.remove_cached_payment(&payment_hash); + assert!(invoice_payer.pay_invoice(&invoice).is_ok()); + + // Cannot retry paying an invoice if cleared from cache. + invoice_payer.remove_cached_payment(&payment_hash); + let event = Event::PaymentPathFailed { + payment_id, + payment_hash, + network_update: None, + rejected_by_dest: false, + all_paths_failed: false, + path: vec![], + short_channel_id: None, + retry: Some(TestRouter::retry_for_invoice(&invoice)), + }; + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), true); + } + + #[test] + fn fails_paying_invoice_with_routing_errors() { + let payer = TestPayer::new(); + let router = FailingRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, |_: &_| {}, RetryAttempts(0)); + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + match invoice_payer.pay_invoice(&invoice) { + Err(PaymentError::Routing(_)) => {}, + Err(_) => panic!("unexpected error"), + Ok(_) => panic!("expected routing error"), + } + } + + #[test] + fn fails_paying_invoice_with_sending_errors() { + let payer = TestPayer::new().fails_on_attempt(1); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, |_: &_| {}, RetryAttempts(0)); + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + match invoice_payer.pay_invoice(&invoice) { + Err(PaymentError::Sending(_)) => {}, + Err(_) => panic!("unexpected error"), + Ok(_) => panic!("expected sending error"), + } + } + + struct TestRouter; + + impl TestRouter { + fn route_for_value(final_value_msat: u64) -> Route { + Route { + paths: vec![ + vec![RouteHop { + pubkey: PublicKey::from_slice(&hex::decode("02eec7245d6b7d2ccb30380bfbe2a3648cd7a942653f5aa340edcea1f283686619").unwrap()[..]).unwrap(), + channel_features: ChannelFeatures::empty(), + node_features: NodeFeatures::empty(), + short_channel_id: 0, fee_msat: final_value_msat / 2, cltv_expiry_delta: 144 + }], + vec![RouteHop { + pubkey: PublicKey::from_slice(&hex::decode("0324653eac434488002cc06bbfb7f10fe18991e35f9fe4302dbea6d2353dc0ab1c").unwrap()[..]).unwrap(), + channel_features: ChannelFeatures::empty(), + node_features: NodeFeatures::empty(), + short_channel_id: 1, fee_msat: final_value_msat / 2, cltv_expiry_delta: 144 + }], + ], + payee: None, + } + } + + fn path_for_value(final_value_msat: u64) -> Vec { + TestRouter::route_for_value(final_value_msat).paths[0].clone() + } + + fn retry_for_invoice(invoice: &Invoice) -> RouteParameters { + let mut payee = Payee::new(invoice.recover_payee_pub_key()) + .with_route_hints(invoice.route_hints()); + if let Some(features) = invoice.features() { + payee = payee.with_features(features.clone()); + } + let final_value_msat = invoice.amount_milli_satoshis().unwrap() / 2; + RouteParameters { + payee, + final_value_msat, + final_cltv_expiry_delta: invoice.min_final_cltv_expiry() as u32, + } + } + } + + impl Router for TestRouter { + fn find_route( + &self, + _payer: &PublicKey, + params: &RouteParameters, + _first_hops: Option<&[&ChannelDetails]>, + ) -> Result { + Ok(Route { + payee: Some(params.payee.clone()), ..Self::route_for_value(params.final_value_msat) + }) + } + } + + struct FailingRouter; + + impl Router for FailingRouter { + fn find_route( + &self, + _payer: &PublicKey, + _params: &RouteParameters, + _first_hops: Option<&[&ChannelDetails]>, + ) -> Result { + Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }) + } + } + + struct TestPayer { + expectations: core::cell::RefCell>, + attempts: core::cell::RefCell, + failing_on_attempt: Option, + } + + impl TestPayer { + fn new() -> Self { + Self { + expectations: core::cell::RefCell::new(std::collections::VecDeque::new()), + attempts: core::cell::RefCell::new(0), + failing_on_attempt: None, + } + } + + fn expect_value_msat(self, value_msat: u64) -> Self { + self.expectations.borrow_mut().push_back(value_msat); + self + } + + fn fails_on_attempt(self, attempt: usize) -> Self { + Self { + expectations: core::cell::RefCell::new(self.expectations.borrow().clone()), + attempts: core::cell::RefCell::new(0), + failing_on_attempt: Some(attempt), + } + } + + fn check_attempts(&self) -> bool { + let mut attempts = self.attempts.borrow_mut(); + *attempts += 1; + match self.failing_on_attempt { + None => true, + Some(attempt) if attempt != *attempts => true, + Some(_) => false, + } + } + + fn check_value_msats(&self, route: &Route) { + let expected_value_msats = self.expectations.borrow_mut().pop_front(); + if let Some(expected_value_msats) = expected_value_msats { + let actual_value_msats = route.get_total_amount(); + assert_eq!(actual_value_msats, expected_value_msats); + } + } + } + + impl Drop for TestPayer { + fn drop(&mut self) { + if std::thread::panicking() { + return; + } + + if !self.expectations.borrow().is_empty() { + panic!("Unsatisfied payment expectations: {:?}", self.expectations.borrow()); + } + } + } + + impl Payer for TestPayer { + fn node_id(&self) -> PublicKey { + let secp_ctx = Secp256k1::new(); + PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()) + } + + fn first_hops(&self) -> Vec { + Vec::new() + } + + fn send_payment( + &self, + route: &Route, + _payment_hash: PaymentHash, + _payment_secret: &Option + ) -> Result { + if self.check_attempts() { + self.check_value_msats(route); + Ok(PaymentId([1; 32])) + } else { + Err(PaymentSendFailure::ParameterError(APIError::MonitorUpdateFailed)) + } + } + + fn retry_payment( + &self, route: &Route, _payment_id: PaymentId + ) -> Result<(), PaymentSendFailure> { + if self.check_attempts() { + self.check_value_msats(route); + Ok(()) + } else { + Err(PaymentSendFailure::ParameterError(APIError::MonitorUpdateFailed)) + } + } + } +} diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index e9f456cfe69..bcd84ba42a6 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -23,6 +23,7 @@ use routing::network_graph::NetworkUpdate; use util::ser::{BigSize, FixedLengthReader, Writeable, Writer, MaybeReadable, Readable, VecReadWrapper, VecWriteWrapper}; use routing::router::{RouteHop, RouteParameters}; +use bitcoin::Transaction; use bitcoin::blockdata::script::Script; use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; @@ -32,7 +33,7 @@ use io; use prelude::*; use core::time::Duration; use core::ops::Deref; -use bitcoin::Transaction; +use sync::Arc; /// Some information provided on receipt of payment depends on whether the payment received is a /// spontaneous payment or a "conventional" lightning payment that's paying an invoice. @@ -781,3 +782,9 @@ impl EventHandler for F where F: Fn(&Event) { self(event) } } + +impl EventHandler for Arc { + fn handle_event(&self, event: &Event) { + self.deref().handle_event(event) + } +} From e523e581522e3c1e1a13a79f339931b5662ca13f Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 4 Oct 2021 11:39:59 -0500 Subject: [PATCH 7/9] Support paying zero-value invoices --- lightning-invoice/src/payment.rs | 91 ++++++++++++++++++++++++++++++-- 1 file changed, 88 insertions(+), 3 deletions(-) diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index 61a0e350ad1..e639b679950 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -197,6 +197,29 @@ where /// Pays the given [`Invoice`], caching it for later use in case a retry is needed. pub fn pay_invoice(&self, invoice: &Invoice) -> Result { + if invoice.amount_milli_satoshis().is_none() { + Err(PaymentError::Invoice("amount missing")) + } else { + self.pay_invoice_internal(invoice, None) + } + } + + /// Pays the given zero-value [`Invoice`] using the given amount, caching it for later use in + /// case a retry is needed. + pub fn pay_zero_value_invoice( + &self, invoice: &Invoice, amount_msats: u64 + ) -> Result { + if invoice.amount_milli_satoshis().is_some() { + Err(PaymentError::Invoice("amount unexpected")) + } else { + self.pay_invoice_internal(invoice, Some(amount_msats)) + } + } + + fn pay_invoice_internal( + &self, invoice: &Invoice, amount_msats: Option + ) -> Result { + debug_assert!(invoice.amount_milli_satoshis().is_some() ^ amount_msats.is_some()); let payment_hash = PaymentHash(invoice.payment_hash().clone().into_inner()); let mut payment_cache = self.payment_cache.lock().unwrap(); match payment_cache.entry(payment_hash) { @@ -207,11 +230,9 @@ where if let Some(features) = invoice.features() { payee = payee.with_features(features.clone()); } - let final_value_msat = invoice.amount_milli_satoshis() - .ok_or(PaymentError::Invoice("amount missing"))?; let params = RouteParameters { payee, - final_value_msat, + final_value_msat: invoice.amount_milli_satoshis().or(amount_msats).unwrap(), final_cltv_expiry_delta: invoice.min_final_cltv_expiry() as u32, }; let first_hops = self.payer.first_hops(); @@ -342,6 +363,21 @@ mod tests { .unwrap() } + fn zero_value_invoice(payment_preimage: PaymentPreimage) -> Invoice { + let payment_hash = Sha256::hash(&payment_preimage.0); + let private_key = SecretKey::from_slice(&[42; 32]).unwrap(); + InvoiceBuilder::new(Currency::Bitcoin) + .description("test".into()) + .payment_hash(payment_hash) + .payment_secret(PaymentSecret([0; 32])) + .current_timestamp() + .min_final_cltv_expiry(144) + .build_signed(|hash| { + Secp256k1::new().sign_recoverable(hash, &private_key) + }) + .unwrap() + } + #[test] fn pays_invoice_on_first_attempt() { let event_handled = core::cell::RefCell::new(false); @@ -681,6 +717,55 @@ mod tests { } } + #[test] + fn pays_zero_value_invoice_using_amount() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = zero_value_invoice(payment_preimage); + let payment_hash = PaymentHash(invoice.payment_hash().clone().into_inner()); + let final_value_msat = 100; + + let payer = TestPayer::new().expect_value_msat(final_value_msat); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(0)); + + let payment_id = + Some(invoice_payer.pay_zero_value_invoice(&invoice, final_value_msat).unwrap()); + assert_eq!(*payer.attempts.borrow(), 1); + + invoice_payer.handle_event(&Event::PaymentSent { + payment_id, payment_preimage, payment_hash + }); + assert_eq!(*event_handled.borrow(), true); + assert_eq!(*payer.attempts.borrow(), 1); + } + + #[test] + fn fails_paying_zero_value_invoice_with_amount() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payer = TestPayer::new(); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(0)); + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + + // Cannot repay an invoice pending payment. + match invoice_payer.pay_zero_value_invoice(&invoice, 100) { + Err(PaymentError::Invoice("amount unexpected")) => {}, + Err(_) => panic!("unexpected error"), + Ok(_) => panic!("expected invoice error"), + } + } + struct TestRouter; impl TestRouter { From d9b9916601cf688dabb0ad4dcddcb98c4c8467e5 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 30 Sep 2021 11:30:24 -0700 Subject: [PATCH 8/9] Fail payment retry if Invoice is expired According to BOLT 11: - after the `timestamp` plus `expiry` has passed - SHOULD NOT attempt a payment Add a convenience method for checking if an Invoice has expired, and use it to short-circuit payment retries. --- lightning-invoice/src/lib.rs | 41 +++++++++++++++++++ lightning-invoice/src/payment.rs | 67 +++++++++++++++++++++++++++++++- lightning/src/routing/router.rs | 10 +++++ 3 files changed, 117 insertions(+), 1 deletion(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 5388321694d..3492b74f423 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -1188,6 +1188,19 @@ impl Invoice { .unwrap_or(Duration::from_secs(DEFAULT_EXPIRY_TIME)) } + /// Returns whether the invoice has expired. + pub fn is_expired(&self) -> bool { + Self::is_expired_from_epoch(self.timestamp(), self.expiry_time()) + } + + /// Returns whether the expiry time from the given epoch has passed. + pub(crate) fn is_expired_from_epoch(epoch: &SystemTime, expiry_time: Duration) -> bool { + match epoch.elapsed() { + Ok(elapsed) => elapsed > expiry_time, + Err(_) => false, + } + } + /// Returns the invoice's `min_final_cltv_expiry` time, if present, otherwise /// [`DEFAULT_MIN_FINAL_CLTV_EXPIRY`]. pub fn min_final_cltv_expiry(&self) -> u64 { @@ -1920,5 +1933,33 @@ mod test { assert_eq!(invoice.min_final_cltv_expiry(), DEFAULT_MIN_FINAL_CLTV_EXPIRY); assert_eq!(invoice.expiry_time(), Duration::from_secs(DEFAULT_EXPIRY_TIME)); + assert!(!invoice.is_expired()); + } + + #[test] + fn test_expiration() { + use ::*; + use secp256k1::Secp256k1; + use secp256k1::key::SecretKey; + + let timestamp = SystemTime::now() + .checked_sub(Duration::from_secs(DEFAULT_EXPIRY_TIME * 2)) + .unwrap(); + let signed_invoice = InvoiceBuilder::new(Currency::Bitcoin) + .description("Test".into()) + .payment_hash(sha256::Hash::from_slice(&[0;32][..]).unwrap()) + .payment_secret(PaymentSecret([0; 32])) + .timestamp(timestamp) + .build_raw() + .unwrap() + .sign::<_, ()>(|hash| { + let privkey = SecretKey::from_slice(&[41; 32]).unwrap(); + let secp_ctx = Secp256k1::new(); + Ok(secp_ctx.sign_recoverable(hash, &privkey)) + }) + .unwrap(); + let invoice = Invoice::from_signed(signed_invoice).unwrap(); + + assert!(invoice.is_expired()); } } diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index e639b679950..7e931d66f15 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -114,6 +114,7 @@ use secp256k1::key::PublicKey; use std::collections::hash_map::{self, HashMap}; use std::ops::Deref; use std::sync::Mutex; +use std::time::{Duration, SystemTime}; /// A utility for paying [`Invoice]`s. pub struct InvoicePayer @@ -226,6 +227,7 @@ where hash_map::Entry::Vacant(entry) => { let payer = self.payer.node_id(); let mut payee = Payee::new(invoice.recover_payee_pub_key()) + .with_expiry_time(expiry_time_from_unix_epoch(&invoice).as_secs()) .with_route_hints(invoice.route_hints()); if let Some(features) = invoice.features() { payee = payee.with_features(features.clone()); @@ -273,6 +275,15 @@ where } } +fn expiry_time_from_unix_epoch(invoice: &Invoice) -> Duration { + invoice.timestamp().duration_since(SystemTime::UNIX_EPOCH).unwrap() + invoice.expiry_time() +} + +fn has_expired(params: &RouteParameters) -> bool { + let expiry_time = Duration::from_secs(params.payee.expiry_time.unwrap()); + Invoice::is_expired_from_epoch(&SystemTime::UNIX_EPOCH, expiry_time) +} + impl EventHandler for InvoicePayer where P::Target: Payer, @@ -304,6 +315,8 @@ where log_trace!(self.logger, "Payment {} exceeded maximum attempts; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts); } else if retry.is_none() { log_trace!(self.logger, "Payment {} missing retry params; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts); + } else if has_expired(retry.as_ref().unwrap()) { + log_trace!(self.logger, "Invoice expired for payment {}; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts); } else if self.retry_payment(*payment_id.as_ref().unwrap(), retry.as_ref().unwrap()).is_err() { log_trace!(self.logger, "Error retrying payment {}; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts); } else { @@ -336,7 +349,7 @@ where #[cfg(test)] mod tests { use super::*; - use crate::{InvoiceBuilder, Currency}; + use crate::{DEFAULT_EXPIRY_TIME, InvoiceBuilder, Currency}; use bitcoin_hashes::sha256::Hash as Sha256; use lightning::ln::PaymentPreimage; use lightning::ln::features::{ChannelFeatures, NodeFeatures}; @@ -346,6 +359,7 @@ mod tests { use lightning::util::errors::APIError; use lightning::util::events::Event; use secp256k1::{SecretKey, PublicKey, Secp256k1}; + use std::time::{SystemTime, Duration}; fn invoice(payment_preimage: PaymentPreimage) -> Invoice { let payment_hash = Sha256::hash(&payment_preimage.0); @@ -378,6 +392,25 @@ mod tests { .unwrap() } + fn expired_invoice(payment_preimage: PaymentPreimage) -> Invoice { + let payment_hash = Sha256::hash(&payment_preimage.0); + let private_key = SecretKey::from_slice(&[42; 32]).unwrap(); + let timestamp = SystemTime::now() + .checked_sub(Duration::from_secs(DEFAULT_EXPIRY_TIME * 2)) + .unwrap(); + InvoiceBuilder::new(Currency::Bitcoin) + .description("test".into()) + .payment_hash(payment_hash) + .payment_secret(PaymentSecret([0; 32])) + .timestamp(timestamp) + .min_final_cltv_expiry(144) + .amount_milli_satoshis(128) + .build_signed(|hash| { + Secp256k1::new().sign_recoverable(hash, &private_key) + }) + .unwrap() + } + #[test] fn pays_invoice_on_first_attempt() { let event_handled = core::cell::RefCell::new(false); @@ -574,6 +607,37 @@ mod tests { assert_eq!(*payer.attempts.borrow(), 1); } + #[test] + fn fails_paying_invoice_after_expiration() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payer = TestPayer::new(); + let router = TestRouter {}; + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &logger, event_handler, RetryAttempts(2)); + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = expired_invoice(payment_preimage); + let payment_id = Some(invoice_payer.pay_invoice(&invoice).unwrap()); + assert_eq!(*payer.attempts.borrow(), 1); + + let event = Event::PaymentPathFailed { + payment_id, + payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), + network_update: None, + rejected_by_dest: false, + all_paths_failed: false, + path: vec![], + short_channel_id: None, + retry: Some(TestRouter::retry_for_invoice(&invoice)), + }; + invoice_payer.handle_event(&event); + assert_eq!(*event_handled.borrow(), true); + assert_eq!(*payer.attempts.borrow(), 1); + } + #[test] fn fails_paying_invoice_after_retry_error() { let event_handled = core::cell::RefCell::new(false); @@ -795,6 +859,7 @@ mod tests { fn retry_for_invoice(invoice: &Invoice) -> RouteParameters { let mut payee = Payee::new(invoice.recover_payee_pub_key()) + .with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs()) .with_route_hints(invoice.route_hints()); if let Some(features) = invoice.features() { payee = payee.with_features(features.clone()); diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index ec49b21058b..545ff9f24ce 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -180,12 +180,16 @@ pub struct Payee { /// Hints for routing to the payee, containing channels connecting the payee to public nodes. pub route_hints: Vec, + + /// Expiration of a payment to the payee, in seconds relative to the UNIX epoch. + pub expiry_time: Option, } impl_writeable_tlv_based!(Payee, { (0, pubkey, required), (2, features, option), (4, route_hints, vec_type), + (6, expiry_time, option), }); impl Payee { @@ -195,6 +199,7 @@ impl Payee { pubkey, features: None, route_hints: vec![], + expiry_time: None, } } @@ -216,6 +221,11 @@ impl Payee { pub fn with_route_hints(self, route_hints: Vec) -> Self { Self { route_hints, ..self } } + + /// Includes a payment expiration in seconds relative to the UNIX epoch. + pub fn with_expiry_time(self, expiry_time: u64) -> Self { + Self { expiry_time: Some(expiry_time), ..self } + } } /// A list of hops along a payment path terminating with a channel to the recipient. From 010436d07d79672c96f5fecc8e948ba66303b735 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 24 Aug 2021 00:14:10 -0500 Subject: [PATCH 9/9] Implement Payer and Router for lightning crate Implements Payer for ChannelManager and Rotuer for find_route, which can be used to parameterize InvoicePayer when needing payment retries. --- lightning-invoice/src/utils.rs | 65 ++++++++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 3 deletions(-) diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index 02bb31dfaf4..ef885f20381 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -1,14 +1,21 @@ //! Convenient utilities to create an invoice. + use {Currency, DEFAULT_EXPIRY_TIME, Invoice, InvoiceBuilder, SignOrCreationError, RawInvoice}; +use payment::{Payer, Router}; + use bech32::ToBase32; use bitcoin_hashes::Hash; use lightning::chain; use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator}; use lightning::chain::keysinterface::{Sign, KeysInterface}; -use lightning::ln::channelmanager::{ChannelManager, MIN_FINAL_CLTV_EXPIRY}; -use lightning::routing::network_graph::RoutingFees; -use lightning::routing::router::{RouteHint, RouteHintHop}; +use lightning::ln::{PaymentHash, PaymentSecret}; +use lightning::ln::channelmanager::{ChannelDetails, ChannelManager, PaymentId, PaymentSendFailure, MIN_FINAL_CLTV_EXPIRY}; +use lightning::ln::msgs::LightningError; +use lightning::routing::network_graph::{NetworkGraph, RoutingFees}; +use lightning::routing::router::{Route, RouteHint, RouteHintHop, RouteParameters, find_route}; +use lightning::routing::scorer::Scorer; use lightning::util::logger::Logger; +use secp256k1::key::PublicKey; use std::convert::TryInto; use std::ops::Deref; @@ -89,6 +96,58 @@ where } } +/// A [`Router`] implemented using [`find_route`]. +pub struct DefaultRouter where G: Deref, L::Target: Logger { + network_graph: G, + logger: L, +} + +impl DefaultRouter where G: Deref, L::Target: Logger { + /// Creates a new router using the given [`NetworkGraph`] and [`Logger`]. + pub fn new(network_graph: G, logger: L) -> Self { + Self { network_graph, logger } + } +} + +impl Router for DefaultRouter +where G: Deref, L::Target: Logger { + fn find_route( + &self, payer: &PublicKey, params: &RouteParameters, first_hops: Option<&[&ChannelDetails]>, + ) -> Result { + let scorer = Scorer::default(); + find_route(payer, params, &*self.network_graph, first_hops, &*self.logger, &scorer) + } +} + +impl Payer for ChannelManager +where + M::Target: chain::Watch, + T::Target: BroadcasterInterface, + K::Target: KeysInterface, + F::Target: FeeEstimator, + L::Target: Logger, +{ + fn node_id(&self) -> PublicKey { + self.get_our_node_id() + } + + fn first_hops(&self) -> Vec { + self.list_usable_channels() + } + + fn send_payment( + &self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option + ) -> Result { + self.send_payment(route, payment_hash, payment_secret) + } + + fn retry_payment( + &self, route: &Route, payment_id: PaymentId + ) -> Result<(), PaymentSendFailure> { + self.retry_payment(route, payment_id) + } +} + #[cfg(test)] mod test { use {Currency, Description, InvoiceDescription};