diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 26c9fb2177d..2bbcac2fd60 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -48,7 +48,7 @@ use lightning::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; use lightning::ln::channel_state::ChannelDetails; use lightning::ln::channelmanager::{ ChainParameters, ChannelManager, ChannelManagerReadArgs, PaymentId, RecentPaymentDetails, - RecipientOnionFields, Retry, + RecipientOnionFields, }; use lightning::ln::functional_test_utils::*; use lightning::ln::inbound_payment::ExpandedKey; @@ -82,7 +82,6 @@ use bitcoin::secp256k1::{self, Message, PublicKey, Scalar, Secp256k1, SecretKey} use lightning::io::Cursor; use std::cmp::{self, Ordering}; -use std::collections::VecDeque; use std::mem; use std::sync::atomic; use std::sync::{Arc, Mutex}; @@ -113,22 +112,14 @@ impl FeeEstimator for FuzzEstimator { } } -struct FuzzRouter { - pub next_routes: Mutex>, -} +struct FuzzRouter {} impl Router for FuzzRouter { fn find_route( &self, _payer: &PublicKey, _params: &RouteParameters, _first_hops: Option<&[&ChannelDetails]>, _inflight_htlcs: InFlightHtlcs, ) -> Result { - if let Some(route) = self.next_routes.lock().unwrap().pop_front() { - return Ok(route); - } - Err(msgs::LightningError { - err: String::from("Not implemented"), - action: msgs::ErrorAction::IgnoreError, - }) + unreachable!() } fn create_blinded_payment_paths( @@ -518,7 +509,7 @@ fn send_payment( PaymentParameters::from_node_id(source.get_our_node_id(), TEST_FINAL_CLTV), amt, ); - source.router.next_routes.lock().unwrap().push_back(Route { + let route = Route { paths: vec![Path { hops: vec![RouteHop { pubkey: dest.get_our_node_id(), @@ -532,11 +523,10 @@ fn send_payment( blinded_tail: None, }], route_params: Some(route_params.clone()), - }); + }; let onion = RecipientOnionFields::secret_only(payment_secret); let payment_id = PaymentId(payment_id); - let res = - source.send_payment(payment_hash, onion, payment_id, route_params, Retry::Attempts(0)); + let res = source.send_payment_with_route(route, payment_hash, onion, payment_id); match res { Err(err) => { panic!("Errored with {:?} on initial payment send", err); @@ -592,7 +582,7 @@ fn send_hop_payment( PaymentParameters::from_node_id(source.get_our_node_id(), TEST_FINAL_CLTV), amt, ); - source.router.next_routes.lock().unwrap().push_back(Route { + let route = Route { paths: vec![Path { hops: vec![ RouteHop { @@ -617,11 +607,10 @@ fn send_hop_payment( blinded_tail: None, }], route_params: Some(route_params.clone()), - }); + }; let onion = RecipientOnionFields::secret_only(payment_secret); let payment_id = PaymentId(payment_id); - let res = - source.send_payment(payment_hash, onion, payment_id, route_params, Retry::Attempts(0)); + let res = source.send_payment_with_route(route, payment_hash, onion, payment_id); match res { Err(err) => { panic!("Errored with {:?} on initial payment send", err); @@ -640,7 +629,7 @@ fn send_hop_payment( pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { let out = SearchingOutput::new(underlying_out); let broadcast = Arc::new(TestBroadcaster {}); - let router = FuzzRouter { next_routes: Mutex::new(VecDeque::new()) }; + let router = FuzzRouter {}; macro_rules! make_node { ($node_id: expr, $fee_estimator: expr) => {{ diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 2d5667a15bd..997a9317e71 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -5096,7 +5096,7 @@ mod tests { use crate::chain::chaininterface::LowerBoundedFeeEstimator; use super::ChannelMonitorUpdateStep; - use crate::{check_added_monitors, check_spends, get_local_commitment_txn, get_monitor, get_route_and_payment_hash, unwrap_send_err}; + use crate::{check_added_monitors, check_spends, get_local_commitment_txn, get_monitor, get_route_and_payment_hash}; use crate::chain::{BestBlock, Confirm}; use crate::chain::channelmonitor::{ChannelMonitor, WithChannelMonitor}; use crate::chain::package::{weight_offered_htlc, weight_received_htlc, weight_revoked_offered_htlc, weight_revoked_received_htlc, WEIGHT_REVOKED_OUTPUT}; @@ -5106,10 +5106,9 @@ mod tests { use crate::types::payment::{PaymentPreimage, PaymentHash}; use crate::ln::channel_keys::{DelayedPaymentBasepoint, DelayedPaymentKey, HtlcBasepoint, RevocationBasepoint, RevocationKey}; use crate::ln::chan_utils::{self,HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters}; - use crate::ln::channelmanager::{PaymentSendFailure, PaymentId, RecipientOnionFields}; + use crate::ln::channelmanager::{PaymentId, RecipientOnionFields}; use crate::ln::functional_test_utils::*; use crate::ln::script::ShutdownScript; - use crate::util::errors::APIError; use crate::util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator}; use crate::util::ser::{ReadableArgs, Writeable}; use crate::util::logger::Logger; @@ -5170,9 +5169,9 @@ mod tests { // If the ChannelManager tries to update the channel, however, the ChainMonitor will pass // the update through to the ChannelMonitor which will refuse it (as the channel is closed). let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 100_000); - unwrap_send_err!(nodes[1].node.send_payment_with_route(route, payment_hash, - RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0) - ), false, APIError::MonitorUpdateInProgress, {}); + nodes[1].node.send_payment_with_route(route, payment_hash, + RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0) + ).unwrap(); check_added_monitors!(nodes[1], 1); // Build a new ChannelMonitorUpdate which contains both the failing commitment tx update diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 175891bcd87..c081937650e 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -19,13 +19,12 @@ use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ChannelMonitor}; use crate::chain::transaction::OutPoint; use crate::chain::{ChannelMonitorUpdateStatus, Listen, Watch}; use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose, ClosureReason, HTLCDestination}; -use crate::ln::channelmanager::{PaymentId, PaymentSendFailure, RAACommitmentOrder, RecipientOnionFields}; +use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, RecipientOnionFields}; use crate::ln::channel::AnnouncementSigsState; use crate::ln::msgs; use crate::ln::types::ChannelId; use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler}; use crate::util::test_channel_signer::TestChannelSigner; -use crate::util::errors::APIError; use crate::util::ser::{ReadableArgs, Writeable}; use crate::util::test_utils::TestBroadcaster; @@ -133,9 +132,9 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) { chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); { - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, payment_hash_1, - RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0) - ), false, APIError::MonitorUpdateInProgress, {}); + nodes[0].node.send_payment_with_route(route, payment_hash_1, + RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0) + ).unwrap(); check_added_monitors!(nodes[0], 1); } @@ -190,9 +189,9 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) { let (route, payment_hash_2, _, payment_secret_2) = get_route_and_payment_hash!(&nodes[0], nodes[1], 1000000); { chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, payment_hash_2, - RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0) - ), false, APIError::MonitorUpdateInProgress, {}); + nodes[0].node.send_payment_with_route(route, payment_hash_2, + RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0) + ).unwrap(); check_added_monitors!(nodes[0], 1); } @@ -257,9 +256,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) { let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000); { chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, payment_hash_2, - RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0) - ), false, APIError::MonitorUpdateInProgress, {}); + nodes[0].node.send_payment_with_route(route, payment_hash_2, + RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0) + ).unwrap(); check_added_monitors!(nodes[0], 1); } @@ -2004,16 +2003,10 @@ fn test_path_paused_mpp() { chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); - // Now check that we get the right return value, indicating that the first path succeeded but - // the second got a MonitorUpdateInProgress err. This implies - // PaymentSendFailure::PartialFailure as some paths succeeded, preventing retry. - if let Err(PaymentSendFailure::PartialFailure { results, ..}) = nodes[0].node.send_payment_with_route( + // The first path should have succeeded with the second getting a MonitorUpdateInProgress err. + nodes[0].node.send_payment_with_route( route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0) - ) { - assert_eq!(results.len(), 2); - if let Ok(()) = results[0] {} else { panic!(); } - if let Err(APIError::MonitorUpdateInProgress) = results[1] {} else { panic!(); } - } else { panic!(); } + ).unwrap(); check_added_monitors!(nodes[0], 2); chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 82d56fb4ae9..7defad8ce65 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -55,9 +55,7 @@ use crate::ln::channel_state::ChannelDetails; use crate::types::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures}; #[cfg(any(feature = "_test_utils", test))] use crate::types::features::Bolt11InvoiceFeatures; -use crate::routing::router::{BlindedTail, InFlightHtlcs, Path, Payee, PaymentParameters, RouteParameters, Router}; -#[cfg(test)] -use crate::routing::router::Route; +use crate::routing::router::{BlindedTail, FixedRouter, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteParameters, Router}; use crate::ln::onion_payment::{check_incoming_htlc_cltv, create_recv_pending_htlc_info, create_fwd_pending_htlc_info, decode_incoming_update_add_htlc_onion, InboundHTLCErr, NextPacketDetails}; use crate::ln::msgs; use crate::ln::onion_utils; @@ -2398,9 +2396,6 @@ where fee_estimator: LowerBoundedFeeEstimator, chain_monitor: M, tx_broadcaster: T, - #[cfg(fuzzing)] - pub router: R, - #[cfg(not(fuzzing))] router: R, message_router: MR, @@ -4583,21 +4578,31 @@ where } } - // Deprecated send method, for testing use [`Self::send_payment`] and - // [`TestRouter::expect_find_route`] instead. - // - // [`TestRouter::expect_find_route`]: crate::util::test_utils::TestRouter::expect_find_route - #[cfg(test)] - pub(crate) fn send_payment_with_route( - &self, route: Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, + /// Sends a payment along a given route. See [`Self::send_payment`] for more info. + /// + /// LDK will not automatically retry this payment, though it may be manually re-sent after an + /// [`Event::PaymentFailed`] is generated. + pub fn send_payment_with_route( + &self, mut route: Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId - ) -> Result<(), PaymentSendFailure> { + ) -> Result<(), RetryableSendFailure> { let best_block_height = self.best_block.read().unwrap().height; let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); + let route_params = route.route_params.clone().unwrap_or_else(|| { + // Create a dummy route params since they're a required parameter but unused in this case + let (payee_node_id, cltv_delta) = route.paths.first() + .and_then(|path| path.hops.last().map(|hop| (hop.pubkey, hop.cltv_expiry_delta as u32))) + .unwrap_or_else(|| (PublicKey::from_slice(&[2; 32]).unwrap(), MIN_FINAL_CLTV_EXPIRY_DELTA as u32)); + let dummy_payment_params = PaymentParameters::from_node_id(payee_node_id, cltv_delta); + RouteParameters::from_payment_params_and_value(dummy_payment_params, route.get_total_amount()) + }); + if route.route_params.is_none() { route.route_params = Some(route_params.clone()); } + let router = FixedRouter::new(route); self.pending_outbound_payments - .send_payment_with_route(&route, payment_hash, recipient_onion, payment_id, - &self.entropy_source, &self.node_signer, best_block_height, - |args| self.send_payment_along_path(args)) + .send_payment(payment_hash, recipient_onion, payment_id, Retry::Attempts(0), + route_params, &&router, self.list_usable_channels(), || self.compute_inflight_htlcs(), + &self.entropy_source, &self.node_signer, best_block_height, &self.logger, + &self.pending_events, |args| self.send_payment_along_path(args)) } /// Sends a payment to the route found using the provided [`RouteParameters`], retrying failed @@ -4626,7 +4631,8 @@ where /// [`ChannelManager::list_recent_payments`] for more information. /// /// Routes are automatically found using the [`Router] provided on startup. To fix a route for a - /// particular payment, match the [`PaymentId`] passed to [`Router::find_route_with_id`]. + /// particular payment, use [`Self::send_payment_with_route`] or match the [`PaymentId`] passed to + /// [`Router::find_route_with_id`]. /// /// [`Event::PaymentSent`]: events::Event::PaymentSent /// [`Event::PaymentFailed`]: events::Event::PaymentFailed @@ -14391,7 +14397,7 @@ mod tests { use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason}; use crate::ln::types::ChannelId; use crate::types::payment::{PaymentPreimage, PaymentHash, PaymentSecret}; - use crate::ln::channelmanager::{create_recv_pending_htlc_info, HTLCForwardInfo, inbound_payment, PaymentId, PaymentSendFailure, RecipientOnionFields, InterceptId}; + use crate::ln::channelmanager::{create_recv_pending_htlc_info, HTLCForwardInfo, inbound_payment, PaymentId, RecipientOnionFields, InterceptId}; use crate::ln::functional_test_utils::*; use crate::ln::msgs::{self, ErrorAction}; use crate::ln::msgs::ChannelMessageHandler; @@ -14822,14 +14828,17 @@ mod tests { route.paths[1].hops[0].short_channel_id = chan_2_id; route.paths[1].hops[1].short_channel_id = chan_4_id; - match nodes[0].node.send_payment_with_route(route, payment_hash, - RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0)) - .unwrap_err() { - PaymentSendFailure::ParameterError(APIError::APIMisuseError { ref err }) => { - assert!(regex::Regex::new(r"Payment secret is required for multi-path payments").unwrap().is_match(err)) - }, - _ => panic!("unexpected error") + nodes[0].node.send_payment_with_route(route, payment_hash, + RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0)).unwrap(); + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { reason, .. } => { + assert_eq!(reason.unwrap(), crate::events::PaymentFailureReason::UnexpectedError); + } + _ => panic!() } + nodes[0].logger.assert_log_contains("lightning::ln::outbound_payment", "Payment secret is required for multi-path payments", 2); assert!(nodes[0].node.list_recent_payments().is_empty()); } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 738e53230e0..385c14e3a09 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1069,30 +1069,27 @@ macro_rules! get_local_commitment_txn { /// Check the error from attempting a payment. #[macro_export] macro_rules! unwrap_send_err { - ($res: expr, $all_failed: expr, $type: pat, $check: expr) => { - match &$res { - &Err(PaymentSendFailure::AllFailedResendSafe(ref fails)) if $all_failed => { - assert_eq!(fails.len(), 1); - match fails[0] { - $type => { $check }, - _ => panic!(), - } - }, - &Err(PaymentSendFailure::PartialFailure { ref results, .. }) if !$all_failed => { - assert_eq!(results.len(), 1); - match results[0] { - Err($type) => { $check }, - _ => panic!(), - } - }, - &Err(PaymentSendFailure::PathParameterError(ref result)) if !$all_failed => { - assert_eq!(result.len(), 1); - match result[0] { - Err($type) => { $check }, - _ => panic!(), + ($node: expr, $res: expr, $all_failed: expr, $type: pat, $check: expr) => { + assert!($res.is_ok()); + let events = $node.node.get_and_clear_pending_events(); + assert!(events.len() == 2); + match &events[0] { + crate::events::Event::PaymentPathFailed { failure, .. } => { + match failure { + crate::events::PathFailure::InitialSend { err } => { + match err { + $type => { $check }, + _ => panic!() + } + }, + _ => panic!() } }, - _ => {panic!()}, + _ => panic!() + } + match &events[1] { + crate::events::Event::PaymentFailed { .. } => {}, + _ => panic!() } } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 177306cfd7b..5711f6ea64b 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -23,7 +23,7 @@ use crate::events::{Event, FundingInfo, MessageSendEvent, MessageSendEventsProvi use crate::ln::types::ChannelId; use crate::types::payment::{PaymentPreimage, PaymentSecret, PaymentHash}; use crate::ln::channel::{CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel, COINBASE_MATURITY, Channel}; -use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, BREAKDOWN_TIMEOUT, ENABLE_GOSSIP_TICKS, DISABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA}; +use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, RecipientOnionFields, BREAKDOWN_TIMEOUT, ENABLE_GOSSIP_TICKS, DISABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA}; use crate::ln::channel::{DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, ChannelError}; use crate::ln::{chan_utils, onion_utils}; use crate::ln::chan_utils::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, OFFERED_HTLC_SCRIPT_WEIGHT, htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment}; @@ -1181,7 +1181,7 @@ fn holding_cell_htlc_counting() { // the holding cell waiting on B's RAA to send. At this point we should not be able to add // another HTLC. { - unwrap_send_err!(nodes[1].node.send_payment_with_route(route, payment_hash_1, + unwrap_send_err!(nodes[1], nodes[1].node.send_payment_with_route(route, payment_hash_1, RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0) ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); @@ -1405,14 +1405,8 @@ fn test_basic_channel_reserve() { get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send); route.paths[0].hops.last_mut().unwrap().fee_msat += 1; let err = nodes[0].node.send_payment_with_route(route, our_payment_hash, - RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap(); - match err { - PaymentSendFailure::AllFailedResendSafe(ref fails) => { - if let &APIError::ChannelUnavailable { .. } = &fails[0] {} - else { panic!("Unexpected error variant"); } - }, - _ => panic!("Unexpected error variant"), - } + RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0)); + unwrap_send_err!(nodes[0], err, true, APIError::ChannelUnavailable { .. }, {} ); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); send_payment(&nodes[0], &vec![&nodes[1]], max_can_send); @@ -1594,7 +1588,7 @@ fn test_chan_reserve_violation_outbound_htlc_inbound_chan() { } // However one more HTLC should be significantly over the reserve amount and fail. - unwrap_send_err!(nodes[1].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[1], nodes[1].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); @@ -1694,7 +1688,7 @@ fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() { let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], dust_amt); route.paths[0].hops[0].fee_msat += 1; - unwrap_send_err!(nodes[1].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[1], nodes[1].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::ChannelUnavailable { .. }, {}); } @@ -1906,7 +1900,7 @@ fn test_channel_reserve_holding_cell_htlcs() { route.paths[0].hops.last_mut().unwrap().fee_msat += 1; assert!(route.paths[0].hops.iter().rev().skip(1).all(|h| h.fee_msat == feemsat)); - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); @@ -1978,7 +1972,7 @@ fn test_channel_reserve_holding_cell_htlcs() { let mut route = route_1.clone(); route.paths[0].hops.last_mut().unwrap().fee_msat = recv_value_2 + 1; let (_, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(nodes[2]); - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); @@ -2008,7 +2002,7 @@ fn test_channel_reserve_holding_cell_htlcs() { let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], recv_value_22); route.paths[0].hops.last_mut().unwrap().fee_msat += 1; - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); @@ -6481,7 +6475,7 @@ fn test_payment_route_reaching_same_channel_twice() { let cloned_hops = route.paths[0].hops.clone(); route.paths[0].hops.extend_from_slice(&cloned_hops); - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), false, APIError::InvalidRoute { ref err }, assert_eq!(err, &"Path went through the same channel twice")); @@ -6504,7 +6498,7 @@ fn test_update_add_htlc_bolt2_sender_value_below_minimum_msat() { let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100000); route.paths[0].hops[0].fee_msat = 100; - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); @@ -6521,13 +6515,13 @@ fn test_update_add_htlc_bolt2_sender_zero_value_msat() { let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100000); route.paths[0].hops[0].fee_msat = 0; - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0)), true, APIError::ChannelUnavailable { ref err }, assert_eq!(err, "Cannot send 0-msat HTLC")); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send 0-msat HTLC", 1); + nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send 0-msat HTLC", 2); } #[test] @@ -6568,7 +6562,7 @@ fn test_update_add_htlc_bolt2_sender_cltv_expiry_too_high() { .with_bolt11_features(nodes[1].node.bolt11_invoice_features()).unwrap(); let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], payment_params, 100000000); route.paths[0].hops.last_mut().unwrap().cltv_expiry_delta = 500000001; - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::InvalidRoute { ref err }, assert_eq!(err, &"Channel CLTV overflowed?")); @@ -6612,7 +6606,7 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_num_and_htlc_id_increment() expect_pending_htlcs_forwardable!(nodes[1]); expect_payment_claimable!(nodes[1], our_payment_hash, our_payment_secret, 100000); } - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::ChannelUnavailable { .. }, {}); @@ -6636,7 +6630,7 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_value_in_flight() { // Manually create a route over our max in flight (which our router normally automatically // limits us to. route.paths[0].hops[0].fee_msat = max_in_flight + 1; - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, our_payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); @@ -10299,11 +10293,11 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e if on_holder_tx { dust_outbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat + 1 }; // With default dust exposure: 5000 sats if on_holder_tx { - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0) ), true, APIError::ChannelUnavailable { .. }, {}); } else { - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0) ), true, APIError::ChannelUnavailable { .. }, {}); } diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 0c02af49763..f51d3b8f11f 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -801,23 +801,6 @@ impl OutboundPayments { best_block_height, logger, pending_events, &send_payment_along_path) } - #[cfg(test)] - pub(super) fn send_payment_with_route( - &self, route: &Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, - payment_id: PaymentId, entropy_source: &ES, node_signer: &NS, best_block_height: u32, - send_payment_along_path: F - ) -> Result<(), PaymentSendFailure> - where - ES::Target: EntropySource, - NS::Target: NodeSigner, - F: Fn(SendAlongPathArgs) -> Result<(), APIError> - { - let onion_session_privs = self.add_new_pending_payment(payment_hash, recipient_onion.clone(), payment_id, None, route, None, None, entropy_source, best_block_height)?; - self.pay_route_internal(route, payment_hash, &recipient_onion, None, None, payment_id, None, - &onion_session_privs, node_signer, best_block_height, &send_payment_along_path) - .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e }) - } - pub(super) fn send_spontaneous_payment( &self, payment_preimage: Option, recipient_onion: RecipientOnionFields, payment_id: PaymentId, retry_strategy: Retry, route_params: RouteParameters, router: &R, diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 029ed37a906..66322cf107e 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -16,7 +16,7 @@ use crate::chain::channelmonitor::{ANTI_REORG_DELAY, HTLC_FAIL_BACK_BUFFER, LATE use crate::sign::EntropySource; use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentFailureReason, PaymentPurpose}; use crate::ln::channel::{EXPIRE_PREV_CONFIG_TICKS, get_holder_selected_channel_reserve_satoshis, ANCHOR_OUTPUT_VALUE_SATOSHI}; -use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, RecentPaymentDetails, RecipientOnionFields, HTLCForwardInfo, PendingHTLCRouting, PendingAddHTLCInfo}; +use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, RecentPaymentDetails, RecipientOnionFields, HTLCForwardInfo, PendingHTLCRouting, PendingAddHTLCInfo}; use crate::types::features::{Bolt11InvoiceFeatures, ChannelTypeFeatures}; use crate::ln::msgs; use crate::ln::types::ChannelId; @@ -603,7 +603,7 @@ fn no_pending_leak_on_initial_send_failure() { nodes[0].node.peer_disconnected(nodes[1].node.get_our_node_id()); nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id()); - unwrap_send_err!(nodes[0].node.send_payment_with_route(route, payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0) ), true, APIError::ChannelUnavailable { ref err }, assert_eq!(err, "Peer for first hop currently disconnected")); @@ -956,7 +956,7 @@ fn do_test_completed_payment_not_retryable_on_reload(use_dust: bool) { // confirming, we will fail as it's considered still-pending... let (new_route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[2], if use_dust { 1_000 } else { 1_000_000 }); match nodes[0].node.send_payment_with_route(new_route.clone(), payment_hash, RecipientOnionFields::secret_only(payment_secret), payment_id) { - Err(PaymentSendFailure::DuplicatePayment) => {}, + Err(RetryableSendFailure::DuplicatePayment) => {}, _ => panic!("Unexpected error") } assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); @@ -995,7 +995,7 @@ fn do_test_completed_payment_not_retryable_on_reload(use_dust: bool) { claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage); match nodes[0].node.send_payment_with_route(new_route.clone(), payment_hash, RecipientOnionFields::secret_only(payment_secret), payment_id) { - Err(PaymentSendFailure::DuplicatePayment) => {}, + Err(RetryableSendFailure::DuplicatePayment) => {}, _ => panic!("Unexpected error") } assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); @@ -1014,7 +1014,7 @@ fn do_test_completed_payment_not_retryable_on_reload(use_dust: bool) { reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1])); match nodes[0].node.send_payment_with_route(new_route, payment_hash, RecipientOnionFields::secret_only(payment_secret), payment_id) { - Err(PaymentSendFailure::DuplicatePayment) => {}, + Err(RetryableSendFailure::DuplicatePayment) => {}, _ => panic!("Unexpected error") } assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); @@ -1558,7 +1558,7 @@ fn claimed_send_payment_idempotent() { let send_result = nodes[0].node.send_payment_with_route(route.clone(), second_payment_hash, RecipientOnionFields::secret_only(second_payment_secret), payment_id); match send_result { - Err(PaymentSendFailure::DuplicatePayment) => {}, + Err(RetryableSendFailure::DuplicatePayment) => {}, _ => panic!("Unexpected send result: {:?}", send_result), } @@ -1637,7 +1637,7 @@ fn abandoned_send_payment_idempotent() { let send_result = nodes[0].node.send_payment_with_route(route.clone(), second_payment_hash, RecipientOnionFields::secret_only(second_payment_secret), payment_id); match send_result { - Err(PaymentSendFailure::DuplicatePayment) => {}, + Err(RetryableSendFailure::DuplicatePayment) => {}, _ => panic!("Unexpected send result: {:?}", send_result), } @@ -4486,3 +4486,32 @@ fn remove_pending_outbound_probe_on_buggy_path() { ); assert!(nodes[0].node.list_recent_payments().is_empty()); } + +#[test] +fn pay_route_without_params() { + // Make sure we can use ChannelManager::send_payment_with_route to pay a route where + // Route::route_parameters is None. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + 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); + + let amt_msat = 10_000; + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) + .with_bolt11_features(nodes[1].node.bolt11_invoice_features()).unwrap(); + let (mut route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], payment_params, amt_msat); + route.route_params.take(); + nodes[0].node.send_payment_with_route( + route, payment_hash, RecipientOnionFields::secret_only(payment_secret), + PaymentId(payment_hash.0) + ).unwrap(); + check_added_monitors!(nodes[0], 1); + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); + pass_along_path(&nodes[0], &[&nodes[1]], amt_msat, payment_hash, Some(payment_secret), node_1_msgs, true, None); + claim_payment_along_route( + ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1]]], payment_preimage) + ); +} diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index ba682f68375..f6e1cd74f02 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -15,7 +15,7 @@ use crate::chain::ChannelMonitorUpdateStatus; use crate::chain::transaction::OutPoint; use crate::events::{Event, MessageSendEvent, HTLCDestination, MessageSendEventsProvider, ClosureReason}; use crate::ln::channel_state::{ChannelDetails, ChannelShutdownState}; -use crate::ln::channelmanager::{self, PaymentSendFailure, PaymentId, RecipientOnionFields, Retry}; +use crate::ln::channelmanager::{self, PaymentId, RecipientOnionFields, Retry}; use crate::routing::router::{PaymentParameters, get_route, RouteParameters}; use crate::ln::msgs; use crate::ln::types::ChannelId; @@ -364,10 +364,10 @@ fn updates_shutdown_wait() { let route_params = RouteParameters::from_payment_params_and_value(payment_params_2, 100_000); let route_2 = get_route(&nodes[1].node.get_our_node_id(), &route_params, &nodes[1].network_graph.read_only(), None, &logger, &scorer, &Default::default(), &random_seed_bytes).unwrap(); - unwrap_send_err!(nodes[0].node.send_payment_with_route(route_1, payment_hash, + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route_1, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0) ), true, APIError::ChannelUnavailable {..}, {}); - unwrap_send_err!(nodes[1].node.send_payment_with_route(route_2, payment_hash, + unwrap_send_err!(nodes[1], nodes[1].node.send_payment_with_route(route_2, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0) ), true, APIError::ChannelUnavailable {..}, {}); diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index a257d2b4abe..d87b5597c48 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -25,6 +25,7 @@ use crate::offers::invoice::Bolt12Invoice; use crate::routing::gossip::{DirectedChannelInfo, EffectiveCapacity, ReadOnlyNetworkGraph, NetworkGraph, NodeId}; use crate::routing::scoring::{ChannelUsage, LockableScore, ScoreLookUp}; use crate::sign::EntropySource; +use crate::sync::Mutex; use crate::util::ser::{Writeable, Readable, ReadableArgs, Writer}; use crate::util::logger::{Level, Logger}; use crate::crypto::chacha20::ChaCha20; @@ -185,6 +186,45 @@ impl>, L: Deref, ES: Deref, S: Deref, SP: Size } } +/// A `Router` that returns a fixed route one time, erroring otherwise. Useful for +/// `ChannelManager::send_payment_with_route` to support sending to specific routes without +/// requiring a custom `Router` implementation. +pub(crate) struct FixedRouter { + // Use an `Option` to avoid needing to clone the route when `find_route` is called. + route: Mutex>, +} + +impl FixedRouter { + pub(crate) fn new(route: Route) -> Self { + Self { route: Mutex::new(Some(route)) } + } +} + +impl Router for FixedRouter { + fn find_route( + &self, _payer: &PublicKey, _route_params: &RouteParameters, + _first_hops: Option<&[&ChannelDetails]>, _inflight_htlcs: InFlightHtlcs + ) -> Result { + self.route.lock().unwrap().take().ok_or_else(|| { + LightningError { + err: "Can't use this router to return multiple routes".to_owned(), + action: ErrorAction::IgnoreError, + } + }) + } + + fn create_blinded_payment_paths< + T: secp256k1::Signing + secp256k1::Verification + > ( + &self, _recipient: PublicKey, _first_hops: Vec, _tlvs: ReceiveTlvs, + _amount_msats: Option, _secp_ctx: &Secp256k1 + ) -> Result, ()> { + // Should be unreachable as this router is only intended to provide a one-time payment route. + debug_assert!(false); + Err(()) + } +} + /// A trait defining behavior for routing a payment. pub trait Router { /// Finds a [`Route`] for a payment between the given `payer` and a payee.