X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Foutbound_payment.rs;h=86b4de768f830172d1caf411b5670667be60923e;hb=2e90ca11ca2063f6984191f028d81af1ba53b1b2;hp=e6454b0ae51b6f659a76b4dffdc4f786e1160dc4;hpb=1dcb3ecb6c6f145bd22a64ce149e939c75aeae3d;p=rust-lightning diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index e6454b0a..86b4de76 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -16,7 +16,6 @@ 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, PaymentId}; -use crate::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA as LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA; use crate::ln::onion_utils::HTLCFailReason; use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router}; use crate::util::errors::APIError; @@ -25,6 +24,7 @@ use crate::util::logger::Logger; use crate::util::time::Time; #[cfg(all(not(feature = "no-std"), test))] use crate::util::time::tests::SinceEpoch; +use crate::util::ser::ReadableArgs; use core::cmp; use core::fmt::{self, Display, Formatter}; @@ -276,7 +276,11 @@ pub(crate) struct PaymentAttemptsUsingTime { /// it means the result of the first attempt is not known yet. pub(crate) count: usize, /// This field is only used when retry is `Retry::Timeout` which is only build with feature std - first_attempted_at: T + #[cfg(not(feature = "no-std"))] + first_attempted_at: T, + #[cfg(feature = "no-std")] + phantom: core::marker::PhantomData, + } #[cfg(not(any(feature = "no-std", test)))] @@ -290,7 +294,10 @@ impl PaymentAttemptsUsingTime { pub(crate) fn new() -> Self { PaymentAttemptsUsingTime { count: 0, - first_attempted_at: T::now() + #[cfg(not(feature = "no-std"))] + first_attempted_at: T::now(), + #[cfg(feature = "no-std")] + phantom: core::marker::PhantomData, } } } @@ -528,12 +535,6 @@ impl OutboundPayments { if pending_amt_msat < total_msat { retry_id_route_params = Some((*pmt_id, RouteParameters { final_value_msat: *total_msat - *pending_amt_msat, - final_cltv_expiry_delta: - if let Some(delta) = params.final_cltv_expiry_delta { delta } - else { - debug_assert!(false, "We always set the final_cltv_expiry_delta when a path fails"); - LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA.into() - }, payment_params: params.clone(), })); break @@ -782,13 +783,13 @@ impl OutboundPayments { debug_assert_eq!(paths.len(), path_results.len()); for (path, path_res) in paths.into_iter().zip(path_results) { if let Err(e) = path_res { - let failed_scid = if let APIError::InvalidRoute { .. } = e { - None - } else { + if let APIError::MonitorUpdateInProgress = e { continue } + let mut failed_scid = None; + if let APIError::ChannelUnavailable { .. } = e { let scid = path[0].short_channel_id; + failed_scid = Some(scid); route_params.payment_params.previously_failed_channels.push(scid); - Some(scid) - }; + } events.push(events::Event::PaymentPathFailed { payment_id: Some(payment_id), payment_hash, @@ -976,9 +977,6 @@ impl OutboundPayments { Some(RouteParameters { payment_params: payment_params.clone(), final_value_msat: pending_amt_unsent, - final_cltv_expiry_delta: - if let Some(delta) = payment_params.final_cltv_expiry_delta { delta } - else { max_unsent_cltv_delta }, }) } else { None } } else { None }, @@ -1179,23 +1177,14 @@ impl OutboundPayments { // `payment_params`) back to the user. let path_last_hop = path.last().expect("Outbound payments must have had a valid path"); if let Some(params) = payment.get_mut().payment_parameters() { - if params.final_cltv_expiry_delta.is_none() { - // This should be rare, but a user could provide None for the payment data, and - // we need it when we go to retry the payment, so fill it in. - params.final_cltv_expiry_delta = Some(path_last_hop.cltv_expiry_delta); - } retry = Some(RouteParameters { payment_params: params.clone(), final_value_msat: path_last_hop.fee_msat, - final_cltv_expiry_delta: params.final_cltv_expiry_delta.unwrap(), }); } else if let Some(params) = payment_params { retry = Some(RouteParameters { payment_params: params.clone(), final_value_msat: path_last_hop.fee_msat, - final_cltv_expiry_delta: - if let Some(delta) = params.final_cltv_expiry_delta { delta } - else { path_last_hop.cltv_expiry_delta }, }); } @@ -1330,7 +1319,9 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, (0, session_privs, required), (1, pending_fee_msat, option), (2, payment_hash, required), - (3, payment_params, option), + // Note that while we "default" payment_param's final CLTV expiry delta to 0 we should + // never see it - `payment_params` was added here after the field was added/required. + (3, payment_params, (option: ReadableArgs, 0)), (4, payment_secret, option), (5, keysend_preimage, option), (6, total_msat, required), @@ -1347,18 +1338,19 @@ 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}; use crate::ln::PaymentHash; use crate::ln::channelmanager::PaymentId; + use crate::ln::features::{ChannelFeatures, NodeFeatures}; use crate::ln::msgs::{ErrorAction, LightningError}; use crate::ln::outbound_payment::{OutboundPayments, Retry, RetryableSendFailure}; use crate::routing::gossip::NetworkGraph; - use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteParameters}; + use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters}; use crate::sync::{Arc, Mutex}; - use crate::util::events::Event; + use crate::util::errors::APIError; + use crate::util::events::{Event, PathFailure}; use crate::util::test_utils; #[test] @@ -1371,8 +1363,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(); @@ -1386,7 +1377,6 @@ mod tests { let expired_route_params = RouteParameters { payment_params, final_value_msat: 0, - final_cltv_expiry_delta: 0, }; let pending_events = Mutex::new(Vec::new()); if on_retry { @@ -1417,8 +1407,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(); @@ -1429,7 +1418,6 @@ mod tests { let route_params = RouteParameters { payment_params, final_value_msat: 0, - final_cltv_expiry_delta: 0, }; router.expect_find_route(route_params.clone(), Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError })); @@ -1455,4 +1443,90 @@ mod tests { } else { panic!("Unexpected error"); } } } + + #[test] + fn initial_send_payment_path_failed_evs() { + let outbound_payments = OutboundPayments::new(); + let logger = test_utils::TestLogger::new(); + 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(); + let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet); + + let sender_pk = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); + let receiver_pk = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[43; 32]).unwrap()); + let payment_params = PaymentParameters::from_node_id(sender_pk, 0); + let route_params = RouteParameters { + payment_params: payment_params.clone(), + final_value_msat: 0, + }; + let failed_scid = 42; + let route = Route { + paths: vec![vec![RouteHop { + pubkey: receiver_pk, + node_features: NodeFeatures::empty(), + short_channel_id: failed_scid, + channel_features: ChannelFeatures::empty(), + fee_msat: 0, + cltv_expiry_delta: 0, + }]], + payment_params: Some(payment_params), + }; + router.expect_find_route(route_params.clone(), Ok(route.clone())); + let mut route_params_w_failed_scid = route_params.clone(); + route_params_w_failed_scid.payment_params.previously_failed_channels.push(failed_scid); + router.expect_find_route(route_params_w_failed_scid, Ok(route.clone())); + router.expect_find_route(route_params.clone(), Ok(route.clone())); + router.expect_find_route(route_params.clone(), Ok(route.clone())); + + // Ensure that a ChannelUnavailable error will result in blaming an scid in the + // PaymentPathFailed event. + let pending_events = Mutex::new(Vec::new()); + outbound_payments.send_payment( + PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params.clone(), + &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, + &pending_events, + |_, _, _, _, _, _, _, _, _| Err(APIError::ChannelUnavailable { err: "test".to_owned() })) + .unwrap(); + let mut events = pending_events.lock().unwrap(); + assert_eq!(events.len(), 2); + if let Event::PaymentPathFailed { + short_channel_id, + failure: PathFailure::InitialSend { err: APIError::ChannelUnavailable { .. }}, .. } = events[0] + { + assert_eq!(short_channel_id, Some(failed_scid)); + } else { panic!("Unexpected event"); } + if let Event::PaymentFailed { .. } = events[1] { } else { panic!("Unexpected event"); } + events.clear(); + core::mem::drop(events); + + // Ensure that a MonitorUpdateInProgress "error" will not result in a PaymentPathFailed event. + outbound_payments.send_payment( + PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params.clone(), + &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, + &pending_events, |_, _, _, _, _, _, _, _, _| Err(APIError::MonitorUpdateInProgress)) + .unwrap(); + { + let events = pending_events.lock().unwrap(); + assert_eq!(events.len(), 0); + } + + // Ensure that any other error will result in a PaymentPathFailed event but no blamed scid. + outbound_payments.send_payment( + PaymentHash([0; 32]), &None, PaymentId([1; 32]), Retry::Attempts(0), route_params.clone(), + &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, + &pending_events, + |_, _, _, _, _, _, _, _, _| Err(APIError::APIMisuseError { err: "test".to_owned() })) + .unwrap(); + let events = pending_events.lock().unwrap(); + assert_eq!(events.len(), 2); + if let Event::PaymentPathFailed { + short_channel_id, + failure: PathFailure::InitialSend { err: APIError::APIMisuseError { .. }}, .. } = events[0] + { + assert_eq!(short_channel_id, None); + } else { panic!("Unexpected event"); } + if let Event::PaymentFailed { .. } = events[1] { } else { panic!("Unexpected event"); } + } }