Fix computing in-flight HTLCs in between retries + test

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index cfb462b0120cac784c2393e4e27450c14747a6bc..234d072a8bd4da9e3bcb45c80bf45c996feb08a7 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -2546,7 +2546,7 @@ where
                let best_block_height = self.best_block.read().unwrap().height();
                self.pending_outbound_payments
                        .send_payment(payment_hash, payment_secret, payment_id, retry_strategy, route_params,
-                               &self.router, self.list_usable_channels(), self.compute_inflight_htlcs(),
+                               &self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
                                &self.entropy_source, &self.node_signer, best_block_height, &self.logger,
                                |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv|
                                self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv))
@@ -2646,7 +2646,7 @@ where
                let best_block_height = self.best_block.read().unwrap().height();
                self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, payment_id,
                        retry_strategy, route_params, &self.router, self.list_usable_channels(),
-                       self.compute_inflight_htlcs(),  &self.entropy_source, &self.node_signer, best_block_height,
+                       || self.compute_inflight_htlcs(),  &self.entropy_source, &self.node_signer, best_block_height,
                        &self.logger,
                        |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv|
                        self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv))

diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index b1b610bf8c9703445bea47aff84c4214bcd0852f..31ba9cb371f9c2c3036a5f4779cc9f24c429467e 100644 (file)
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -402,22 +402,23 @@ impl OutboundPayments {
                }
        }
 
-       pub(super) fn send_payment<R: Deref, ES: Deref, NS: Deref, F, L: Deref>(
+       pub(super) fn send_payment<R: Deref, ES: Deref, NS: Deref, IH, SP, L: Deref>(
                &self, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, payment_id: PaymentId,
                retry_strategy: Retry, route_params: RouteParameters, router: &R,
-               first_hops: Vec<ChannelDetails>, inflight_htlcs: InFlightHtlcs, entropy_source: &ES,
-               node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: F,
+               first_hops: Vec<ChannelDetails>, compute_inflight_htlcs: IH, entropy_source: &ES,
+               node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: SP,
        ) -> Result<(), PaymentSendFailure>
        where
                R::Target: Router,
                ES::Target: EntropySource,
                NS::Target: NodeSigner,
                L::Target: Logger,
-               F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
+               IH: Fn() -> InFlightHtlcs,
+               SP: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
                         u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
        {
                self.pay_internal(payment_id, Some((payment_hash, payment_secret, None, retry_strategy)),
-                       route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer,
+                       route_params, router, first_hops, &compute_inflight_htlcs, entropy_source, node_signer,
                        best_block_height, logger, &send_payment_along_path)
                        .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
        }
@@ -439,25 +440,26 @@ impl OutboundPayments {
                        .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
        }
 
-       pub(super) fn send_spontaneous_payment<R: Deref, ES: Deref, NS: Deref, F, L: Deref>(
+       pub(super) fn send_spontaneous_payment<R: Deref, ES: Deref, NS: Deref, IH, SP, L: Deref>(
                &self, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId,
                retry_strategy: Retry, route_params: RouteParameters, router: &R,
-               first_hops: Vec<ChannelDetails>, inflight_htlcs: InFlightHtlcs, entropy_source: &ES,
-               node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: F
+               first_hops: Vec<ChannelDetails>, inflight_htlcs: IH, entropy_source: &ES,
+               node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: SP
        ) -> Result<PaymentHash, PaymentSendFailure>
        where
                R::Target: Router,
                ES::Target: EntropySource,
                NS::Target: NodeSigner,
                L::Target: Logger,
-               F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
+               IH: Fn() -> InFlightHtlcs,
+               SP: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
                         u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
        {
                let preimage = payment_preimage
                        .unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes()));
                let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
                self.pay_internal(payment_id, Some((payment_hash, &None, Some(preimage), retry_strategy)),
-                       route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer,
+                       route_params, router, first_hops, &inflight_htlcs, entropy_source, node_signer,
                        best_block_height, logger, &send_payment_along_path)
                        .map(|()| payment_hash)
                        .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
@@ -525,26 +527,27 @@ impl OutboundPayments {
                        }
                        if let Some((payment_id, route_params)) = retry_id_route_params {
                                core::mem::drop(outbounds);
-                               if let Err(e) = self.pay_internal(payment_id, None, route_params, router, first_hops(), inflight_htlcs(), entropy_source, node_signer, best_block_height, logger, &send_payment_along_path) {
+                               if let Err(e) = self.pay_internal(payment_id, None, route_params, router, first_hops(), &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, &send_payment_along_path) {
                                        log_info!(logger, "Errored retrying payment: {:?}", e);
                                }
                        } else { break }
                }
        }
 
-       fn pay_internal<R: Deref, NS: Deref, ES: Deref, F, L: Deref>(
+       fn pay_internal<R: Deref, NS: Deref, ES: Deref, IH, SP, L: Deref>(
                &self, payment_id: PaymentId,
                initial_send_info: Option<(PaymentHash, &Option<PaymentSecret>, Option<PaymentPreimage>, Retry)>,
                route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>,
-               inflight_htlcs: InFlightHtlcs, entropy_source: &ES, node_signer: &NS, best_block_height: u32,
-               logger: &L, send_payment_along_path: &F,
+               inflight_htlcs: &IH, entropy_source: &ES, node_signer: &NS, best_block_height: u32,
+               logger: &L, send_payment_along_path: &SP,
        ) -> Result<(), PaymentSendFailure>
        where
                R::Target: Router,
                ES::Target: EntropySource,
                NS::Target: NodeSigner,
                L::Target: Logger,
-               F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
+               IH: Fn() -> InFlightHtlcs,
+               SP: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
                   u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
        {
                #[cfg(feature = "std")] {
@@ -557,7 +560,7 @@ impl OutboundPayments {
 
                let route = router.find_route(
                        &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params,
-                       Some(&first_hops.iter().collect::<Vec<_>>()), &inflight_htlcs
+                       Some(&first_hops.iter().collect::<Vec<_>>()), &inflight_htlcs(),
                ).map_err(|e| PaymentSendFailure::ParameterError(APIError::APIMisuseError {
                        err: format!("Failed to find a route for payment {}: {:?}", log_bytes!(payment_id.0), e), // TODO: add APIError::RouteNotFound
                }))?;
@@ -1235,12 +1238,12 @@ mod tests {
                };
                let err = if on_retry {
                        outbound_payments.pay_internal(
-                               PaymentId([0; 32]), None, expired_route_params, &&router, vec![], InFlightHtlcs::new(),
+                               PaymentId([0; 32]), None, expired_route_params, &&router, vec![], &|| InFlightHtlcs::new(),
                                &&keys_manager, &&keys_manager, 0, &&logger, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
                } else {
                        outbound_payments.send_payment(
                                PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), expired_route_params,
-                               &&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
+                               &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
                                |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
                };
                if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err {
@@ -1278,12 +1281,12 @@ mod tests {
                                &Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)),
                                Some(route_params.payment_params.clone()), &&keys_manager, 0).unwrap();
                        outbound_payments.pay_internal(
-                               PaymentId([0; 32]), None, route_params, &&router, vec![], InFlightHtlcs::new(),
+                               PaymentId([0; 32]), None, route_params, &&router, vec![], &|| InFlightHtlcs::new(),
                                &&keys_manager, &&keys_manager, 0, &&logger, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
                } else {
                        outbound_payments.send_payment(
                                PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params,
-                               &&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
+                               &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
                                |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
                };
                if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err {

diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index c82534ec8fc27567a594ee570630af9e888aa765..dbd78b6f8e60cb9c2e150aa4e51ce9ddf948bb88 100644 (file)
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -21,8 +21,9 @@ use crate::ln::features::InvoiceFeatures;
 use crate::ln::msgs;
 use crate::ln::msgs::ChannelMessageHandler;
 use crate::ln::outbound_payment::Retry;
-use crate::routing::gossip::RoutingFees;
+use crate::routing::gossip::{EffectiveCapacity, RoutingFees};
 use crate::routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RouteParameters};
+use crate::routing::scoring::ChannelUsage;
 use crate::util::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
 use crate::util::test_utils;
 use crate::util::errors::APIError;
@@ -2175,6 +2176,16 @@ fn retry_multi_path_single_failed_payment() {
                        final_value_msat: 100_000_001, final_cltv_expiry_delta: TEST_FINAL_CLTV
                }, Ok(route.clone()));
 
+       {
+               let scorer = chanmon_cfgs[0].scorer.lock().unwrap();
+               // The initial send attempt, 2 paths
+               scorer.expect_usage(chans[0].short_channel_id.unwrap(), ChannelUsage { amount_msat: 10_000, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown });
+               scorer.expect_usage(chans[1].short_channel_id.unwrap(), ChannelUsage { amount_msat: 100_000_001, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown });
+               // The retry, 2 paths. Ensure that the in-flight HTLC amount is factored in.
+               scorer.expect_usage(chans[0].short_channel_id.unwrap(), ChannelUsage { amount_msat: 50_000_001, inflight_htlc_msat: 10_000, effective_capacity: EffectiveCapacity::Unknown });
+               scorer.expect_usage(chans[1].short_channel_id.unwrap(), ChannelUsage { amount_msat: 50_000_000, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown });
+       }
+
        nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
        let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(htlc_msgs.len(), 2);