From 82a2c845d825d8b07a739f509759444e974979fc Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 14 Jan 2025 14:27:02 -0500 Subject: [PATCH] Reinstate ChannelManager::send_payment_with_route API Support more ergonomically sending payments to specific routes. We removed the original version of this API because it was hard to work with, but the concept of sending a payment to a specific route is still useful. Previously, users were able to do this via manually matching the payment id in their router, but that's cumbersome when we could just handle it internally. --- fuzz/src/chanmon_consistency.rs | 31 +++------- lightning/src/chain/channelmonitor.rs | 11 ++-- lightning/src/ln/chanmon_update_fail_tests.rs | 33 ++++------ lightning/src/ln/channelmanager.rs | 61 +++++++++++-------- lightning/src/ln/functional_test_utils.rs | 41 ++++++------- lightning/src/ln/functional_tests.rs | 42 ++++++------- lightning/src/ln/outbound_payment.rs | 17 ------ lightning/src/ln/payment_tests.rs | 43 ++++++++++--- lightning/src/ln/shutdown_tests.rs | 6 +- lightning/src/routing/router.rs | 40 ++++++++++++ 10 files changed, 179 insertions(+), 146 deletions(-) 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.