Mark deprecated send_payment_with_route as test and fuzz only

This method has been deprecated for several versions in favor of
ChannelManager::send_payment, and we want to remove it from the public API
entirely prior to the 0.1 release. However, >150 tests use it so put off
removing the method entirely.

Author: valentinewallace

lightning/src/ln/channelmanager.rs

@@ -4519,24 +4519,32 @@ where
 		}
 	}
 
-	/// Sends a payment along a given route.
-	///
-	/// This method is *DEPRECATED*, use [`Self::send_payment`] instead. If you wish to fix the
-	/// route for a payment, do so by matching the [`PaymentId`] passed to
-	/// [`Router::find_route_with_id`].
-	///
-	/// Value parameters are provided via the last hop in route, see documentation for [`RouteHop`]
-	/// fields for more info.
+	// 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(any(test, fuzzing))]
+	pub fn send_payment_with_route(&self, route: Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId) -> Result<(), PaymentSendFailure> {
+		let best_block_height = self.best_block.read().unwrap().height;
+		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
+		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))
+	}
+
+	/// Sends a payment to the route found using the provided [`RouteParameters`], retrying failed
+	/// payment paths based on the provided `Retry`.
 	///
 	/// May generate [`UpdateHTLCs`] message(s) event on success, which should be relayed (e.g. via
 	/// [`PeerManager::process_events`]).
 	///
 	/// # Avoiding Duplicate Payments
 	///
 	/// If a pending payment is currently in-flight with the same [`PaymentId`] provided, this
-	/// method will error with an [`APIError::InvalidRoute`]. Note, however, that once a payment
-	/// is no longer pending (either via [`ChannelManager::abandon_payment`], or handling of an
-	/// [`Event::PaymentSent`] or [`Event::PaymentFailed`]) LDK will not stop you from sending a
+	/// method will error with [`RetryableSendFailure::DuplicatePayment`]. Note, however, that once a
+	/// payment is no longer pending (either via [`ChannelManager::abandon_payment`], or handling of
+	/// an [`Event::PaymentSent`] or [`Event::PaymentFailed`]) LDK will not stop you from sending a
 	/// second payment with the same [`PaymentId`].
 	///
 	/// Thus, in order to ensure duplicate payments are not sent, you should implement your own
@@ -4550,43 +4558,18 @@ where
 	/// using [`ChannelMonitorUpdateStatus::InProgress`]), the payment may be lost on restart. See
 	/// [`ChannelManager::list_recent_payments`] for more information.
 	///
-	/// # Possible Error States on [`PaymentSendFailure`]
-	///
-	/// Each path may have a different return value, and [`PaymentSendFailure`] may return a `Vec` with
-	/// each entry matching the corresponding-index entry in the route paths, see
-	/// [`PaymentSendFailure`] for more info.
-	///
-	/// In general, a path may raise:
-	///  * [`APIError::InvalidRoute`] when an invalid route or forwarding parameter (cltv_delta, fee,
-	///    node public key) is specified.
-	///  * [`APIError::ChannelUnavailable`] if the next-hop channel is not available as it has been
-	///    closed, doesn't exist, or the peer is currently disconnected.
-	///  * [`APIError::MonitorUpdateInProgress`] if a new monitor update failure prevented sending the
-	///    relevant updates.
-	///
-	/// Note that depending on the type of the [`PaymentSendFailure`] the HTLC may have been
-	/// irrevocably committed to on our end. In such a case, do NOT retry the payment with a
-	/// different route unless you intend to pay twice!
+	/// 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`].
 	///
-	/// [`RouteHop`]: crate::routing::router::RouteHop
 	/// [`Event::PaymentSent`]: events::Event::PaymentSent
 	/// [`Event::PaymentFailed`]: events::Event::PaymentFailed
 	/// [`UpdateHTLCs`]: events::MessageSendEvent::UpdateHTLCs
 	/// [`PeerManager::process_events`]: crate::ln::peer_handler::PeerManager::process_events
 	/// [`ChannelMonitorUpdateStatus::InProgress`]: crate::chain::ChannelMonitorUpdateStatus::InProgress
-	#[cfg_attr(not(any(test, feature = "_test_utils")), deprecated(note = "Use `send_payment` instead"))]
-	pub fn send_payment_with_route(&self, route: Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId) -> Result<(), PaymentSendFailure> {
-		let best_block_height = self.best_block.read().unwrap().height;
-		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
-		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))
-	}
-
-	/// Similar to [`ChannelManager::send_payment_with_route`], but will automatically find a route based on
-	/// `route_params` and retry failed payment paths based on `retry_strategy`.
-	pub fn send_payment(&self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<(), RetryableSendFailure> {
+	pub fn send_payment(
+		&self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId,
+		route_params: RouteParameters, retry_strategy: Retry
+	) -> Result<(), RetryableSendFailure> {
 		let best_block_height = self.best_block.read().unwrap().height;
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 		self.pending_outbound_payments
@@ -4801,8 +4784,26 @@ where
 	/// would be able to guess -- otherwise, an intermediate node may claim the payment and it will
 	/// never reach the recipient.
 	///
-	/// See [`send_payment`] documentation for more details on the return value of this function
-	/// and idempotency guarantees provided by the [`PaymentId`] key.
+	/// See [`send_payment`] documentation for more details on the idempotency guarantees provided by
+	/// the [`PaymentId`] key.
+	///
+	/// # Possible Error States on [`PaymentSendFailure`]
+	///
+	/// Each path may have a different return value, and [`PaymentSendFailure`] may return a `Vec` with
+	/// each entry matching the corresponding-index entry in the route paths, see
+	/// [`PaymentSendFailure`] for more info.
+	///
+	/// In general, a path may raise:
+	///  * [`APIError::InvalidRoute`] when an invalid route or forwarding parameter (cltv_delta, fee,
+	///    node public key) is specified.
+	///  * [`APIError::ChannelUnavailable`] if the next-hop channel is not available as it has been
+	///    closed, doesn't exist, or the peer is currently disconnected.
+	///  * [`APIError::MonitorUpdateInProgress`] if a new monitor update failure prevented sending the
+	///    relevant updates.
+	///
+	/// Note that depending on the type of the [`PaymentSendFailure`] the HTLC may have been
+	/// irrevocably committed to on our end. In such a case, do NOT retry the payment with a
+	/// different route unless you intend to pay twice!
 	///
 	/// Similar to regular payments, you MUST NOT reuse a `payment_preimage` value. See
 	/// [`send_payment`] for more information about the risks of duplicate preimage usage.

lightning/src/ln/functional_test_utils.rs

@@ -17,17 +17,17 @@ use crate::events::{ClaimedHTLC, ClosureReason, Event, HTLCDestination, MessageS
 use crate::events::bump_transaction::{BumpTransactionEvent, BumpTransactionEventHandler, Wallet, WalletSource};
 use crate::ln::types::ChannelId;
 use crate::types::payment::{PaymentPreimage, PaymentHash, PaymentSecret};
-use crate::ln::channelmanager::{AChannelManager, ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, PaymentId, MIN_CLTV_EXPIRY_DELTA};
+use crate::ln::channelmanager::{AChannelManager, ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, RecipientOnionFields, PaymentId, MIN_CLTV_EXPIRY_DELTA};
 use crate::types::features::InitFeatures;
 use crate::ln::msgs;
 use crate::ln::msgs::{ChannelMessageHandler, OnionMessageHandler, RoutingMessageHandler};
+use crate::ln::outbound_payment::Retry;
 use crate::ln::peer_handler::IgnoringMessageHandler;
 use crate::onion_message::messenger::OnionMessenger;
 use crate::routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate};
 use crate::routing::router::{self, PaymentParameters, Route, RouteParameters};
 use crate::sign::{EntropySource, RandomBytes};
 use crate::util::config::{UserConfig, MaxDustHTLCExposure};
-use crate::util::errors::APIError;
 #[cfg(test)]
 use crate::util::logger::Logger;
 use crate::util::scid_utils;
@@ -2600,8 +2600,11 @@ pub fn expect_payment_failed_conditions<'a, 'b, 'c, 'd, 'e>(
 
 pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_paths: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) -> PaymentId {
 	let payment_id = PaymentId(origin_node.keys_manager.backing.get_secure_random_bytes());
-	origin_node.node.send_payment_with_route(route, our_payment_hash,
-		RecipientOnionFields::secret_only(our_payment_secret), payment_id).unwrap();
+	origin_node.router.expect_find_route(route.route_params.clone().unwrap(), Ok(route.clone()));
+	origin_node.node.send_payment(
+		our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), payment_id,
+		route.route_params.unwrap(), Retry::Attempts(0)
+	).unwrap();
 	check_added_monitors!(origin_node, expected_paths.len());
 	pass_along_route(origin_node, expected_paths, recv_value, our_payment_hash, our_payment_secret);
 	payment_id
@@ -3103,30 +3106,6 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
 	(res.0, res.1, res.2, res.3)
 }
 
-pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64)  {
-	let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV)
-		.with_bolt11_features(expected_route.last().unwrap().node.bolt11_invoice_features()).unwrap();
-	let route_params = RouteParameters::from_payment_params_and_value(payment_params, recv_value);
-	let network_graph = origin_node.network_graph.read_only();
-	let scorer = test_utils::TestScorer::new();
-	let seed = [0u8; 32];
-	let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
-	let random_seed_bytes = keys_manager.get_secure_random_bytes();
-	let route = router::get_route(&origin_node.node.get_our_node_id(), &route_params, &network_graph,
-		None, origin_node.logger, &scorer, &Default::default(), &random_seed_bytes).unwrap();
-	assert_eq!(route.paths.len(), 1);
-	assert_eq!(route.paths[0].hops.len(), expected_route.len());
-	for (node, hop) in expected_route.iter().zip(route.paths[0].hops.iter()) {
-		assert_eq!(hop.pubkey, node.node.get_our_node_id());
-	}
-
-	let (_, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(expected_route.last().unwrap());
-	unwrap_send_err!(origin_node.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!(err.contains("Cannot send value that would put us over the max HTLC value in flight our peer will accept")));
-}
-
 pub fn send_payment<'a, 'b, 'c>(origin: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret, PaymentId) {
 	let res = route_payment(&origin, expected_route, recv_value);
 	claim_payment(&origin, expected_route, res.0);

lightning/src/ln/functional_tests.rs

@@ -1092,8 +1092,12 @@ fn fake_network_test() {
 	});
 	hops[1].fee_msat = chan_4.1.contents.fee_base_msat as u64 + chan_4.1.contents.fee_proportional_millionths as u64 * hops[2].fee_msat as u64 / 1000000;
 	hops[0].fee_msat = chan_3.0.contents.fee_base_msat as u64 + chan_3.0.contents.fee_proportional_millionths as u64 * hops[1].fee_msat as u64 / 1000000;
+	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 route_params = RouteParameters::from_payment_params_and_value(payment_params, 1000000);
 	let payment_preimage_1 = send_along_route(&nodes[1],
-		Route { paths: vec![Path { hops, blinded_tail: None }], route_params: None },
+		Route { paths: vec![Path { hops, blinded_tail: None }], route_params: Some(route_params.clone()) },
 			&vec!(&nodes[2], &nodes[3], &nodes[1])[..], 1000000).0;
 
 	let mut hops = Vec::with_capacity(3);
@@ -1127,7 +1131,7 @@ fn fake_network_test() {
 	hops[1].fee_msat = chan_2.1.contents.fee_base_msat as u64 + chan_2.1.contents.fee_proportional_millionths as u64 * hops[2].fee_msat as u64 / 1000000;
 	hops[0].fee_msat = chan_3.1.contents.fee_base_msat as u64 + chan_3.1.contents.fee_proportional_millionths as u64 * hops[1].fee_msat as u64 / 1000000;
 	let payment_hash_2 = send_along_route(&nodes[1],
-		Route { paths: vec![Path { hops, blinded_tail: None }], route_params: None },
+		Route { paths: vec![Path { hops, blinded_tail: None }], route_params: Some(route_params) },
 			&vec!(&nodes[3], &nodes[2], &nodes[1])[..], 1000000).1;
 
 	// Claim the rebalances...

lightning/src/ln/outbound_payment.rs

@@ -478,11 +478,9 @@ pub enum RetryableSendFailure {
 	OnionPacketSizeExceeded,
 }
 
-/// If a payment fails to send with [`ChannelManager::send_payment_with_route`], it can be in one
-/// of several states. This enum is returned as the Err() type describing which state the payment
-/// is in, see the description of individual enum states for more.
-///
-/// [`ChannelManager::send_payment_with_route`]: crate::ln::channelmanager::ChannelManager::send_payment_with_route
+/// If a payment fails to send to a route, it can be in one of several states. This enum is returned
+/// as the Err() type describing which state the payment is in, see the description of individual
+/// enum states for more.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum PaymentSendFailure {
 	/// A parameter which was passed to send_payment was invalid, preventing us from attempting to
@@ -776,6 +774,7 @@ impl OutboundPayments {
 			best_block_height, logger, pending_events, &send_payment_along_path)
 	}
 
+	#[cfg(any(test, fuzzing))]
 	pub(super) fn send_payment_with_route<ES: Deref, NS: Deref, F>(
 		&self, route: &Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields,
 		payment_id: PaymentId, entropy_source: &ES, node_signer: &NS, best_block_height: u32,