Merge pull request #2629 from jkczyz/2023-09-invreqfailed
[rust-lightning] / lightning / src / ln / outbound_payment.rs
index 2f1e3b0dd0e3d4358f7b43b5e01d1706f3ec005f..1642f28efc7b36dbfa7739bf886ebb58b9eb7c70 100644 (file)
@@ -215,11 +215,19 @@ impl PendingOutboundPayment {
                                PendingOutboundPayment::InvoiceReceived { .. } => { debug_assert!(false); false },
                };
                if remove_res {
-                       if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self {
-                               let path = path.expect("Fulfilling a payment should always come with a path");
+                       if let PendingOutboundPayment::Retryable {
+                               ref mut pending_amt_msat, ref mut pending_fee_msat,
+                               ref mut remaining_max_total_routing_fee_msat, ..
+                       } = self {
+                               let path = path.expect("Removing a failed payment should always come with a path");
                                *pending_amt_msat -= path.final_value_msat();
+                               let path_fee_msat = path.fee_msat();
                                if let Some(fee_msat) = pending_fee_msat.as_mut() {
-                                       *fee_msat -= path.fee_msat();
+                                       *fee_msat -= path_fee_msat;
+                               }
+
+                               if let Some(max_total_routing_fee_msat) = remaining_max_total_routing_fee_msat.as_mut() {
+                                       *max_total_routing_fee_msat = max_total_routing_fee_msat.saturating_add(path_fee_msat);
                                }
                        }
                }
@@ -238,11 +246,19 @@ impl PendingOutboundPayment {
                        PendingOutboundPayment::Abandoned { .. } => false,
                };
                if insert_res {
-                       if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self {
-                               *pending_amt_msat += path.final_value_msat();
-                               if let Some(fee_msat) = pending_fee_msat.as_mut() {
-                                       *fee_msat += path.fee_msat();
-                               }
+                       if let PendingOutboundPayment::Retryable {
+                               ref mut pending_amt_msat, ref mut pending_fee_msat,
+                               ref mut remaining_max_total_routing_fee_msat, .. 
+                       } = self {
+                                       *pending_amt_msat += path.final_value_msat();
+                                       let path_fee_msat = path.fee_msat();
+                                       if let Some(fee_msat) = pending_fee_msat.as_mut() {
+                                               *fee_msat += path_fee_msat;
+                                       }
+
+                                       if let Some(max_total_routing_fee_msat) = remaining_max_total_routing_fee_msat.as_mut() {
+                                               *max_total_routing_fee_msat = max_total_routing_fee_msat.saturating_sub(path_fee_msat);
+                                       }
                        }
                }
                insert_res
@@ -859,7 +875,7 @@ impl OutboundPayments {
                        }
                }
 
-               let route = router.find_route_with_id(
+               let mut route = router.find_route_with_id(
                        &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params,
                        Some(&first_hops.iter().collect::<Vec<_>>()), inflight_htlcs(),
                        payment_hash, payment_id,
@@ -869,6 +885,14 @@ impl OutboundPayments {
                        RetryableSendFailure::RouteNotFound
                })?;
 
+               if let Some(route_route_params) = route.route_params.as_mut() {
+                       if route_route_params.final_value_msat != route_params.final_value_msat {
+                               debug_assert!(false,
+                                       "Routers are expected to return a route which includes the requested final_value_msat");
+                               route_route_params.final_value_msat = route_params.final_value_msat;
+                       }
+               }
+
                let onion_session_privs = self.add_new_pending_payment(payment_hash,
                        recipient_onion.clone(), payment_id, keysend_preimage, &route, Some(retry_strategy),
                        Some(route_params.payment_params.clone()), entropy_source, best_block_height)
@@ -910,7 +934,7 @@ impl OutboundPayments {
                        }
                }
 
-               let route = match router.find_route_with_id(
+               let mut route = match router.find_route_with_id(
                        &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params,
                        Some(&first_hops.iter().collect::<Vec<_>>()), inflight_htlcs(),
                        payment_hash, payment_id,
@@ -922,6 +946,15 @@ impl OutboundPayments {
                                return
                        }
                };
+
+               if let Some(route_route_params) = route.route_params.as_mut() {
+                       if route_route_params.final_value_msat != route_params.final_value_msat {
+                               debug_assert!(false,
+                                       "Routers are expected to return a route which includes the requested final_value_msat");
+                               route_route_params.final_value_msat = route_params.final_value_msat;
+                       }
+               }
+
                for path in route.paths.iter() {
                        if path.hops.len() == 0 {
                                log_error!(logger, "Unusable path in route (path.hops.len() must be at least 1");
@@ -1321,30 +1354,44 @@ impl OutboundPayments {
                }
                let mut has_ok = false;
                let mut has_err = false;
-               let mut pending_amt_unsent = 0;
+               let mut has_unsent = false;
+               let mut total_ok_fees_msat = 0;
+               let mut total_ok_amt_sent_msat = 0;
                for (res, path) in results.iter().zip(route.paths.iter()) {
-                       if res.is_ok() { has_ok = true; }
+                       if res.is_ok() {
+                               has_ok = true;
+                               total_ok_fees_msat += path.fee_msat();
+                               total_ok_amt_sent_msat += path.final_value_msat();
+                       }
                        if res.is_err() { has_err = true; }
                        if let &Err(APIError::MonitorUpdateInProgress) = res {
                                // MonitorUpdateInProgress is inherently unsafe to retry, so we call it a
                                // PartialFailure.
                                has_err = true;
                                has_ok = true;
+                               total_ok_fees_msat += path.fee_msat();
+                               total_ok_amt_sent_msat += path.final_value_msat();
                        } else if res.is_err() {
-                               pending_amt_unsent += path.final_value_msat();
+                               has_unsent = true;
                        }
                }
                if has_err && has_ok {
                        Err(PaymentSendFailure::PartialFailure {
                                results,
                                payment_id,
-                               failed_paths_retry: if pending_amt_unsent != 0 {
-                                       if let Some(payment_params) = route.route_params.as_ref().map(|p| p.payment_params.clone()) {
-                                               Some(RouteParameters {
-                                                       payment_params,
-                                                       final_value_msat: pending_amt_unsent,
-                                                       max_total_routing_fee_msat: None,
-                                               })
+                               failed_paths_retry: if has_unsent {
+                                       if let Some(route_params) = &route.route_params {
+                                               let mut route_params = route_params.clone();
+                                               // We calculate the leftover fee budget we're allowed to spend by
+                                               // subtracting the used fee from the total fee budget.
+                                               route_params.max_total_routing_fee_msat = route_params
+                                                       .max_total_routing_fee_msat.map(|m| m.saturating_sub(total_ok_fees_msat));
+
+                                               // We calculate the remaining target amount by subtracting the succeded
+                                               // path values.
+                                               route_params.final_value_msat = route_params.final_value_msat
+                                                       .saturating_sub(total_ok_amt_sent_msat);
+                                               Some(route_params)
                                        } else { None }
                                } else { None },
                        })
@@ -1455,6 +1502,9 @@ impl OutboundPayments {
                &self, pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>)
        {
                let mut pending_outbound_payments = self.pending_outbound_payments.lock().unwrap();
+               #[cfg(not(invreqfailed))]
+               let pending_events = pending_events.lock().unwrap();
+               #[cfg(invreqfailed)]
                let mut pending_events = pending_events.lock().unwrap();
                pending_outbound_payments.retain(|payment_id, payment| {
                        // If an outbound payment was completed, and no pending HTLCs remain, we should remove it
@@ -1493,6 +1543,7 @@ impl OutboundPayments {
                                if *timer_ticks_without_response <= INVOICE_REQUEST_TIMEOUT_TICKS {
                                        true
                                } else {
+                                       #[cfg(invreqfailed)]
                                        pending_events.push_back(
                                                (events::Event::InvoiceRequestFailed { payment_id: *payment_id }, None)
                                        );
@@ -1511,10 +1562,11 @@ impl OutboundPayments {
        ) -> bool where L::Target: Logger {
                #[cfg(test)]
                let DecodedOnionFailure {
-                       network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data
+                       network_update, short_channel_id, payment_failed_permanently, onion_error_code,
+                       onion_error_data
                } = onion_error.decode_onion_failure(secp_ctx, logger, &source);
                #[cfg(not(test))]
-               let DecodedOnionFailure { network_update, short_channel_id, payment_retryable } =
+               let DecodedOnionFailure { network_update, short_channel_id, payment_failed_permanently } =
                        onion_error.decode_onion_failure(secp_ctx, logger, &source);
 
                let payment_is_probe = payment_is_probe(payment_hash, &payment_id, probing_cookie_secret);
@@ -1555,8 +1607,8 @@ impl OutboundPayments {
                                payment.get_mut().insert_previously_failed_scid(scid);
                        }
 
-                       if payment_is_probe || !is_retryable_now || !payment_retryable {
-                               let reason = if !payment_retryable {
+                       if payment_is_probe || !is_retryable_now || payment_failed_permanently {
+                               let reason = if payment_failed_permanently {
                                        PaymentFailureReason::RecipientRejected
                                } else {
                                        PaymentFailureReason::RetriesExhausted
@@ -1565,7 +1617,7 @@ impl OutboundPayments {
                                is_retryable_now = false;
                        }
                        if payment.get().remaining_parts() == 0 {
-                               if let PendingOutboundPayment::Abandoned { payment_hash, reason, .. }= payment.get() {
+                               if let PendingOutboundPayment::Abandoned { payment_hash, reason, .. } = payment.get() {
                                        if !payment_is_probe {
                                                full_failure_ev = Some(events::Event::PaymentFailed {
                                                        payment_id: *payment_id,
@@ -1586,7 +1638,7 @@ impl OutboundPayments {
 
                let path_failure = {
                        if payment_is_probe {
-                               if !payment_retryable {
+                               if payment_failed_permanently {
                                        events::Event::ProbeSuccessful {
                                                payment_id: *payment_id,
                                                payment_hash: payment_hash.clone(),
@@ -1610,7 +1662,7 @@ impl OutboundPayments {
                                events::Event::PaymentPathFailed {
                                        payment_id: Some(*payment_id),
                                        payment_hash: payment_hash.clone(),
-                                       payment_failed_permanently: !payment_retryable,
+                                       payment_failed_permanently,
                                        failure: events::PathFailure::OnPath { network_update },
                                        path: path.clone(),
                                        short_channel_id,
@@ -1644,6 +1696,7 @@ impl OutboundPayments {
                                        payment.remove();
                                }
                        } else if let PendingOutboundPayment::AwaitingInvoice { .. } = payment.get() {
+                               #[cfg(invreqfailed)]
                                pending_events.lock().unwrap().push_back((events::Event::InvoiceRequestFailed {
                                        payment_id,
                                }, None));
@@ -1734,7 +1787,9 @@ mod tests {
        use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
        use crate::ln::features::{ChannelFeatures, NodeFeatures};
        use crate::ln::msgs::{ErrorAction, LightningError};
-       use crate::ln::outbound_payment::{Bolt12PaymentError, INVOICE_REQUEST_TIMEOUT_TICKS, OutboundPayments, Retry, RetryableSendFailure};
+       use crate::ln::outbound_payment::{Bolt12PaymentError, OutboundPayments, Retry, RetryableSendFailure};
+       #[cfg(invreqfailed)]
+       use crate::ln::outbound_payment::INVOICE_REQUEST_TIMEOUT_TICKS;
        use crate::offers::invoice::DEFAULT_RELATIVE_EXPIRY;
        use crate::offers::offer::OfferBuilder;
        use crate::offers::test_utils::*;
@@ -1937,6 +1992,7 @@ mod tests {
        }
 
        #[test]
+       #[cfg(invreqfailed)]
        fn removes_stale_awaiting_invoice() {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
@@ -1975,6 +2031,7 @@ mod tests {
        }
 
        #[test]
+       #[cfg(invreqfailed)]
        fn removes_abandoned_awaiting_invoice() {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
@@ -2059,11 +2116,6 @@ mod tests {
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
 
-               assert!(
-                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
-               );
-               assert!(outbound_payments.has_pending_payments());
-
                let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
                        .amount_msats(1000)
                        .build().unwrap()
@@ -2074,6 +2126,12 @@ mod tests {
                        .build().unwrap()
                        .sign(recipient_sign).unwrap();
 
+               assert!(outbound_payments.add_new_awaiting_invoice(
+                               payment_id, Retry::Attempts(0), Some(invoice.amount_msats() / 100 + 50_000))
+                       .is_ok()
+               );
+               assert!(outbound_payments.has_pending_payments());
+
                router.expect_find_route(
                        RouteParameters::from_payment_params_and_value(
                                PaymentParameters::from_bolt12_invoice(&invoice),
@@ -2114,11 +2172,6 @@ mod tests {
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
 
-               assert!(
-                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
-               );
-               assert!(outbound_payments.has_pending_payments());
-
                let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
                        .amount_msats(1000)
                        .build().unwrap()
@@ -2129,6 +2182,12 @@ mod tests {
                        .build().unwrap()
                        .sign(recipient_sign).unwrap();
 
+               assert!(outbound_payments.add_new_awaiting_invoice(
+                               payment_id, Retry::Attempts(0), Some(invoice.amount_msats() / 100 + 50_000))
+                       .is_ok()
+               );
+               assert!(outbound_payments.has_pending_payments());
+
                let route_params = RouteParameters::from_payment_params_and_value(
                        PaymentParameters::from_bolt12_invoice(&invoice),
                        invoice.amount_msats(),