X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Foutbound_payment.rs;h=361adfaf66c3eab2b4a78678a6d7b5ffe5dfb81c;hb=d7c818a3addfe21151e468044540d702857daa94;hp=346190e2085271ea83047112f72ecba3ce737bbc;hpb=5c6d8a7cb8c2252d112568e03496f6adaa16f774;p=rust-lightning diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 346190e2..361adfaf 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -15,7 +15,7 @@ use bitcoin::secp256k1::{self, Secp256k1, SecretKey}; use crate::chain::keysinterface::{EntropySource, NodeSigner, Recipient}; use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; -use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, MIN_HTLC_RELAY_HOLDING_CELL_MILLIS, PaymentId}; +use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId}; use crate::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA as LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA; use crate::ln::msgs::DecodeError; use crate::ln::onion_utils::HTLCFailReason; @@ -30,7 +30,6 @@ use crate::util::time::tests::SinceEpoch; use core::cmp; use core::fmt::{self, Display, Formatter}; use core::ops::Deref; -use core::time::Duration; use crate::prelude::*; use crate::sync::Mutex; @@ -360,13 +359,13 @@ pub enum PaymentSendFailure { /// [`Event::PaymentSent`]: crate::util::events::Event::PaymentSent /// [`Event::PaymentFailed`]: crate::util::events::Event::PaymentFailed DuplicatePayment, - /// Some paths which were attempted failed to send, though possibly not all. At least some - /// paths have irrevocably committed to the HTLC. + /// Some paths that were attempted failed to send, though some paths may have succeeded. At least + /// some paths have irrevocably committed to the HTLC. /// - /// The results here are ordered the same as the paths in the route object which was passed to + /// The results here are ordered the same as the paths in the route object that was passed to /// send_payment. /// - /// Any entries which contain `Err(APIError::MonitorUpdateInprogress)` will send once a + /// Any entries that contain `Err(APIError::MonitorUpdateInprogress)` will send once a /// [`MonitorEvent::Completed`] is provided for the next-hop channel with the latest update_id. /// /// [`MonitorEvent::Completed`]: crate::chain::channelmonitor::MonitorEvent::Completed @@ -383,12 +382,14 @@ pub enum PaymentSendFailure { pub(super) struct OutboundPayments { pub(super) pending_outbound_payments: Mutex>, + pub(super) retry_lock: Mutex<()>, } impl OutboundPayments { pub(super) fn new() -> Self { Self { - pending_outbound_payments: Mutex::new(HashMap::new()) + pending_outbound_payments: Mutex::new(HashMap::new()), + retry_lock: Mutex::new(()), } } @@ -494,6 +495,7 @@ impl OutboundPayments { FH: Fn() -> Vec, L::Target: Logger, { + let _single_thread = self.retry_lock.lock().unwrap(); loop { let mut outbounds = self.pending_outbound_payments.lock().unwrap(); let mut retry_id_route_params = None; @@ -543,6 +545,12 @@ impl OutboundPayments { }); } + pub(super) fn needs_abandon(&self) -> bool { + let outbounds = self.pending_outbound_payments.lock().unwrap(); + outbounds.iter().any(|(_, pmt)| + !pmt.is_auto_retryable_now() && pmt.remaining_parts() == 0 && !pmt.is_fulfilled()) + } + /// Will return `Ok(())` iff at least one HTLC is sent for the payment. fn pay_internal( &self, payment_id: PaymentId, @@ -1003,12 +1011,13 @@ impl OutboundPayments { }); } + // Returns a bool indicating whether a PendingHTLCsForwardable event should be generated. pub(super) fn fail_htlc( &self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason, path: &Vec, session_priv: &SecretKey, payment_id: &PaymentId, payment_params: &Option, probing_cookie_secret: [u8; 32], secp_ctx: &Secp256k1, pending_events: &Mutex>, logger: &L - ) where L::Target: Logger { + ) -> bool where L::Target: Logger { #[cfg(test)] let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_error.decode_onion_failure(secp_ctx, logger, &source); #[cfg(not(test))] @@ -1018,18 +1027,33 @@ impl OutboundPayments { let mut session_priv_bytes = [0; 32]; session_priv_bytes.copy_from_slice(&session_priv[..]); let mut outbounds = self.pending_outbound_payments.lock().unwrap(); + + // If any payments already need retry, there's no need to generate a redundant + // `PendingHTLCsForwardable`. + let already_awaiting_retry = outbounds.iter().any(|(_, pmt)| { + let mut awaiting_retry = false; + if pmt.is_auto_retryable_now() { + if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, .. } = pmt { + if pending_amt_msat < total_msat { + awaiting_retry = true; + } + } + } + awaiting_retry + }); + let mut all_paths_failed = false; let mut full_failure_ev = None; - let mut pending_retry_ev = None; + let mut pending_retry_ev = false; let mut retry = None; let attempts_remaining = if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) { if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) { log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0)); - return + return false } if payment.get().is_fulfilled() { log_trace!(logger, "Received failure of HTLC with payment_hash {} after payment completion", log_bytes!(payment_hash.0)); - return + return false } let mut is_retryable_now = payment.get().is_auto_retryable_now(); if let Some(scid) = short_channel_id { @@ -1062,24 +1086,26 @@ impl OutboundPayments { }); } - if !payment_is_probe && (!is_retryable_now || !payment_retryable || retry.is_none()) { + if payment_is_probe || !is_retryable_now || !payment_retryable || retry.is_none() { let _ = payment.get_mut().mark_abandoned(); // we'll only Err if it's a legacy payment is_retryable_now = false; } if payment.get().remaining_parts() == 0 { all_paths_failed = true; if payment.get().abandoned() { - full_failure_ev = Some(events::Event::PaymentFailed { - payment_id: *payment_id, - payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"), - }); + if !payment_is_probe { + full_failure_ev = Some(events::Event::PaymentFailed { + payment_id: *payment_id, + payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"), + }); + } payment.remove(); } } is_retryable_now } else { log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0)); - return + return false }; core::mem::drop(outbounds); log_trace!(logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0)); @@ -1109,11 +1135,9 @@ impl OutboundPayments { } // If we miss abandoning the payment above, we *must* generate an event here or else the // payment will sit in our outbounds forever. - if attempts_remaining { + if attempts_remaining && !already_awaiting_retry { debug_assert!(full_failure_ev.is_none()); - pending_retry_ev = Some(events::Event::PendingHTLCsForwardable { - time_forwardable: Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS), - }); + pending_retry_ev = true; } events::Event::PaymentPathFailed { payment_id: Some(*payment_id), @@ -1134,7 +1158,7 @@ impl OutboundPayments { let mut pending_events = pending_events.lock().unwrap(); pending_events.push(path_failure); if let Some(ev) = full_failure_ev { pending_events.push(ev); } - if let Some(ev) = pending_retry_ev { pending_events.push(ev); } + pending_retry_ev } pub(super) fn abandon_payment( @@ -1212,7 +1236,6 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, #[cfg(test)] mod tests { - use bitcoin::blockdata::constants::genesis_block; use bitcoin::network::constants::Network; use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; @@ -1236,8 +1259,7 @@ mod tests { fn do_fails_paying_after_expiration(on_retry: bool) { let outbound_payments = OutboundPayments::new(); let logger = test_utils::TestLogger::new(); - let genesis_hash = genesis_block(Network::Testnet).header.block_hash(); - let network_graph = Arc::new(NetworkGraph::new(genesis_hash, &logger)); + let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger)); let scorer = Mutex::new(test_utils::TestScorer::new()); let router = test_utils::TestRouter::new(network_graph, &scorer); let secp_ctx = Secp256k1::new(); @@ -1276,8 +1298,7 @@ mod tests { fn do_find_route_error(on_retry: bool) { let outbound_payments = OutboundPayments::new(); let logger = test_utils::TestLogger::new(); - let genesis_hash = genesis_block(Network::Testnet).header.block_hash(); - let network_graph = Arc::new(NetworkGraph::new(genesis_hash, &logger)); + let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger)); let scorer = Mutex::new(test_utils::TestScorer::new()); let router = test_utils::TestRouter::new(network_graph, &scorer); let secp_ctx = Secp256k1::new();