Move reasonable-default arguments out of pay_for_offer.

`ChannelManager::pay_for_offer` and
`ChannelManager::pay_for_offer_from_human_readable_name` have
accumulated a handful of arguments, most of which we expect users
to never actually set.

Instead, here, we move `quantity` and `payer_note` (which we expect
users to never set) as well as `route_params_config` and
`retry_strategy` (which we expect users to generally stick with the
defaults on) into a new struct, which implements `Default`.

This cleans up a good bit of cruft on payment calls.

Author: TheBlueMatt

lightning-dns-resolver/src/lib.rs

@@ -162,7 +162,7 @@ mod test {
 	use lightning::blinded_path::message::{BlindedMessagePath, MessageContext};
 	use lightning::blinded_path::NodeIdLookUp;
 	use lightning::events::{Event, PaymentPurpose};
-	use lightning::ln::channelmanager::{PaymentId, Retry};
+	use lightning::ln::channelmanager::{OptionalOfferPaymentInfo, PaymentId};
 	use lightning::ln::functional_test_utils::*;
 	use lightning::ln::msgs::{
 		BaseMessageHandler, ChannelMessageHandler, Init, OnionMessageHandler,
@@ -173,7 +173,6 @@ mod test {
 	use lightning::onion_message::messenger::{
 		AOnionMessenger, Destination, MessageRouter, OnionMessagePath, OnionMessenger,
 	};
-	use lightning::routing::router::RouteParametersConfig;
 	use lightning::sign::{KeysManager, NodeSigner, Recipient};
 	use lightning::types::features::InitFeatures;
 	use lightning::types::payment::PaymentHash;
@@ -369,23 +368,18 @@ mod test {
 	async fn pay_offer_flow<'a, 'b, 'c>(
 		nodes: &[Node<'a, 'b, 'c>], resolver_messenger: &impl AOnionMessenger,
 		resolver_id: PublicKey, payer_id: PublicKey, payee_id: PublicKey, offer: Offer,
-		name: HumanReadableName, amt: u64, payment_id: PaymentId, payer_note: Option<String>,
-		retry: Retry, params: RouteParametersConfig, resolvers: Vec<Destination>,
+		name: HumanReadableName, payment_id: PaymentId, payer_note: Option<String>,
+		resolvers: Vec<Destination>,
 	) {
 		// Override contents to offer provided
 		let proof_override = &nodes[0].node.testing_dnssec_proof_offer_resolution_override;
 		proof_override.lock().unwrap().insert(name.clone(), offer);
+		let amt = 42_000;
+		let mut opts = OptionalOfferPaymentInfo::default();
+		opts.payer_note = payer_note.clone();
 		nodes[0]
 			.node
-			.pay_for_offer_from_human_readable_name(
-				name,
-				amt,
-				payment_id,
-				payer_note.clone(),
-				retry,
-				params,
-				resolvers,
-			)
+			.pay_for_offer_from_human_readable_name(name, amt, payment_id, opts, resolvers)
 			.unwrap();
 
 		let query = nodes[0].onion_messenger.next_onion_message_for_peer(resolver_id).unwrap();
@@ -482,9 +476,6 @@ mod test {
 
 		let bs_offer = nodes[1].node.create_offer_builder(None).unwrap().build().unwrap();
 		let resolvers = vec![Destination::Node(resolver_id)];
-		let retry = Retry::Attempts(0);
-		let amt = 42_000;
-		let params = RouteParametersConfig::default();
 
 		pay_offer_flow(
 			&nodes,
@@ -494,11 +485,8 @@ mod test {
 			payee_id,
 			bs_offer.clone(),
 			name.clone(),
-			amt,
 			PaymentId([42; 32]),
 			None,
-			retry,
-			params,
 			resolvers.clone(),
 		)
 		.await;
@@ -512,11 +500,8 @@ mod test {
 			payee_id,
 			bs_offer,
 			name,
-			amt,
 			PaymentId([21; 32]),
 			Some("foo".into()),
-			retry,
-			params,
 			resolvers,
 		)
 		.await;

lightning/src/ln/async_payments_tests.rs

@@ -13,7 +13,7 @@ use crate::blinded_path::payment::{AsyncBolt12OfferContext, BlindedPaymentTlvs};
 use crate::chain::channelmonitor::{HTLC_FAIL_BACK_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS};
 use crate::events::{Event, HTLCHandlingFailureType, PaidBolt12Invoice, PaymentFailureReason};
 use crate::ln::blinded_payment_tests::{fail_blinded_htlc_backwards, get_blinded_route_parameters};
-use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
+use crate::ln::channelmanager::{OptionalOfferPaymentInfo, PaymentId, RecipientOnionFields};
 use crate::ln::functional_test_utils::*;
 use crate::ln::msgs;
 use crate::ln::msgs::{
@@ -32,7 +32,7 @@ use crate::onion_message::messenger::{Destination, MessageRouter, MessageSendIns
 use crate::onion_message::offers::OffersMessage;
 use crate::onion_message::packet::ParsedOnionMessageContents;
 use crate::prelude::*;
-use crate::routing::router::{Payee, PaymentParameters, RouteParametersConfig};
+use crate::routing::router::{Payee, PaymentParameters};
 use crate::sign::NodeSigner;
 use crate::sync::Mutex;
 use crate::types::features::Bolt12InvoiceFeatures;
@@ -239,10 +239,9 @@ fn static_invoice_unknown_required_features() {
 
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
 	nodes[0]
 		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
+		.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default())
 		.unwrap();
 
 	// Don't forward the invreq since we don't support retrieving the static invoice from the
@@ -300,10 +299,9 @@ fn ignore_unexpected_static_invoice() {
 		create_static_invoice(&nodes[1], &nodes[2], None, &secp_ctx);
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
 	nodes[0]
 		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
+		.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default())
 		.unwrap();
 
 	// Don't forward the invreq since we don't support retrieving the static invoice from the
@@ -418,10 +416,9 @@ fn async_receive_flow_success() {
 
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
 	nodes[0]
 		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
+		.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default())
 		.unwrap();
 	let release_held_htlc_om =
 		pass_async_payments_oms(static_invoice.clone(), &nodes[0], &nodes[1], &nodes[2]).1;
@@ -470,10 +467,9 @@ fn expired_static_invoice_fail() {
 
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
 	nodes[0]
 		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
+		.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default())
 		.unwrap();
 
 	let invreq_om = nodes[0]
@@ -561,10 +557,9 @@ fn async_receive_mpp() {
 	// the different MPP parts to not be unique.
 	let amt_msat = 15_000_000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
 	nodes[0]
 		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(1), params)
+		.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default())
 		.unwrap();
 	let release_held_htlc_om_3_0 =
 		pass_async_payments_oms(static_invoice, &nodes[0], &nodes[1], &nodes[3]).1;
@@ -652,10 +647,9 @@ fn amount_doesnt_match_invreq() {
 
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
 	nodes[0]
 		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(1), params)
+		.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default())
 		.unwrap();
 	let release_held_htlc_om_3_0 =
 		pass_async_payments_oms(static_invoice, &nodes[0], &nodes[1], &nodes[3]).1;
@@ -888,10 +882,9 @@ fn invalid_async_receive_with_retry<F1, F2>(
 	let payment_hash: PaymentHash = keysend_preimage.into();
 	*nodes[0].keys_manager.override_random_bytes.lock().unwrap() = Some(hardcoded_random_bytes);
 
-	let params = RouteParametersConfig::default();
 	nodes[0]
 		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(2), params)
+		.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default())
 		.unwrap();
 	let release_held_htlc_om_2_0 =
 		pass_async_payments_oms(static_invoice, &nodes[0], &nodes[1], &nodes[2]).1;
@@ -978,10 +971,9 @@ fn expired_static_invoice_message_path() {
 
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
 	nodes[0]
 		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(1), params)
+		.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default())
 		.unwrap();
 
 	// While the invoice is unexpired, respond with release_held_htlc.
@@ -1083,11 +1075,9 @@ fn expired_static_invoice_payment_path() {
 	let (offer, static_invoice) = create_static_invoice(&nodes[1], &nodes[2], None, &secp_ctx);
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
-	nodes[0]
-		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
-		.unwrap();
+	let mut params: OptionalOfferPaymentInfo = Default::default();
+	params.retry_strategy = Retry::Attempts(0);
+	nodes[0].node.pay_for_offer(&offer, Some(amt_msat), payment_id, params).unwrap();
 	let release_held_htlc_om =
 		pass_async_payments_oms(static_invoice, &nodes[0], &nodes[1], &nodes[2]).1;
 	nodes[0]

lightning/src/ln/channelmanager.rs

@@ -687,6 +687,53 @@ impl Readable for InterceptId {
 	}
 }
 
+/// Optional arguments to [`ChannelManager::pay_for_offer`]
+#[cfg_attr(
+	feature = "dnssec",
+	doc = "and [`ChannelManager::pay_for_offer_from_human_readable_name`]"
+)]
+/// .
+///
+/// These fields will often not need to be set, and the provided [`Self::default`] can be used.
+pub struct OptionalOfferPaymentInfo {
+	/// The quantity of the offer which we wish to pay for. This is communicated to the recipient
+	/// and determines the minimum value which we must pay.
+	///
+	/// This must be set if we're paying for an offer that has [`Offer::expects_quantity`].
+	///
+	#[cfg_attr(
+		feature = "dnssec",
+		doc = "Note that setting this will cause [`ChannelManager::pay_for_offer_from_human_readable_name`] to fail."
+	)]
+	pub quantity: Option<u64>,
+	/// A note which is communicated to the recipient about this payment via
+	/// [`InvoiceRequest::payer_note`].
+	pub payer_note: Option<String>,
+	/// Pathfinding options which tweak how the path is constructed to the recipient.
+	pub route_params_config: RouteParametersConfig,
+	/// The number of tries or time during which we'll retry this payment if some paths to the
+	/// recipient fail.
+	///
+	/// Once the retry limit is reached, further path failures will not be retried and the payment
+	/// will ultimately fail once all pending paths have failed (generating an
+	/// [`Event::PaymentFailed`]).
+	pub retry_strategy: Retry,
+}
+
+impl Default for OptionalOfferPaymentInfo {
+	fn default() -> Self {
+		Self {
+			quantity: None,
+			payer_note: None,
+			route_params_config: Default::default(),
+			#[cfg(feature = "std")]
+			retry_strategy: Retry::Timeout(core::time::Duration::from_secs(2)),
+			#[cfg(not(feature = "std"))]
+			retry_strategy: Retry::Attempts(3),
+		}
+	}
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 /// Uniquely describes an HTLC by its source. Just the guaranteed-unique subset of [`HTLCSource`].
 pub(crate) enum SentHTLCId {
@@ -2256,18 +2303,16 @@ where
 ///
 /// ```
 /// # use lightning::events::{Event, EventsProvider};
-/// # use lightning::ln::channelmanager::{AChannelManager, PaymentId, RecentPaymentDetails, Retry};
+/// # use lightning::ln::channelmanager::{AChannelManager, PaymentId, RecentPaymentDetails};
 /// # use lightning::offers::offer::Offer;
-/// # use lightning::routing::router::RouteParametersConfig;
 /// #
 /// # fn example<T: AChannelManager>(
-/// #     channel_manager: T, offer: &Offer, quantity: Option<u64>, amount_msats: Option<u64>,
-/// #     payer_note: Option<String>, retry: Retry, route_params_config: RouteParametersConfig
+/// #     channel_manager: T, offer: &Offer, amount_msats: Option<u64>,
 /// # ) {
 /// # let channel_manager = channel_manager.get_cm();
 /// let payment_id = PaymentId([42; 32]);
 /// match channel_manager.pay_for_offer(
-///     offer, quantity, amount_msats, payer_note, payment_id, retry, route_params_config
+///     offer, amount_msats, payment_id, Default::default(),
 /// ) {
 ///     Ok(()) => println!("Requesting invoice for offer"),
 ///     Err(e) => println!("Unable to request invoice for offer: {:?}", e),
@@ -5024,6 +5069,9 @@ where
 	/// Sends a payment to the route found using the provided [`RouteParameters`], retrying failed
 	/// payment paths based on the provided `Retry`.
 	///
+	/// You should likely prefer [`Self::pay_for_bolt11_invoice`] or [`Self::pay_for_offer`] in
+	/// general, however this method may allow for slightly more customization.
+	///
 	/// May generate [`UpdateHTLCs`] message(s) event on success, which should be relayed (e.g. via
 	/// [`PeerManager::process_events`]).
 	///
@@ -11128,16 +11176,9 @@ where
 	///
 	/// Uses [`InvoiceRequestBuilder`] such that the [`InvoiceRequest`] it builds is recognized by
 	/// the [`ChannelManager`] when handling a [`Bolt12Invoice`] message in response to the request.
-	/// The optional parameters are used in the builder, if `Some`:
-	/// - `quantity` for [`InvoiceRequest::quantity`] which must be set if
-	///   [`Offer::expects_quantity`] is `true`.
-	/// - `amount_msats` if overpaying what is required for the given `quantity` is desired, and
-	/// - `payer_note` for [`InvoiceRequest::payer_note`].
 	///
-	/// # Custom Routing Parameters
-	///
-	/// Users can customize routing parameters via [`RouteParametersConfig`].
-	/// To use default settings, call the function with [`RouteParametersConfig::default`].
+	/// `amount_msats` allows you to overpay what is required to satisfy the offer, or may be
+	/// required if the offer does not require a specific amount.
 	///
 	/// # Payment
 	///
@@ -11179,9 +11220,8 @@ where
 	/// [`Bolt12Invoice::payment_paths`]: crate::offers::invoice::Bolt12Invoice::payment_paths
 	/// [Avoiding Duplicate Payments]: #avoiding-duplicate-payments
 	pub fn pay_for_offer(
-		&self, offer: &Offer, quantity: Option<u64>, amount_msats: Option<u64>,
-		payer_note: Option<String>, payment_id: PaymentId, retry_strategy: Retry,
-		route_params_config: RouteParametersConfig,
+		&self, offer: &Offer, amount_msats: Option<u64>, payment_id: PaymentId,
+		optional_info: OptionalOfferPaymentInfo,
 	) -> Result<(), Bolt12SemanticError> {
 		let create_pending_payment_fn = |invoice_request: &InvoiceRequest, nonce| {
 			let expiration = StaleExpiration::TimerTicks(1);
@@ -11194,18 +11234,18 @@ where
 				.add_new_awaiting_invoice(
 					payment_id,
 					expiration,
-					retry_strategy,
-					route_params_config,
+					optional_info.retry_strategy,
+					optional_info.route_params_config,
 					Some(retryable_invoice_request),
 				)
 				.map_err(|_| Bolt12SemanticError::DuplicatePaymentId)
 		};
 
 		self.pay_for_offer_intern(
 			offer,
-			quantity,
+			optional_info.quantity,
 			amount_msats,
-			payer_note,
+			optional_info.payer_note,
 			payment_id,
 			None,
 			create_pending_payment_fn,
@@ -11313,11 +11353,6 @@ where
 	/// implementing [`DNSResolverMessageHandler`]) directly to look up a URI and then delegate to
 	/// your normal URI handling.
 	///
-	/// # Custom Routing Parameters
-	///
-	/// Users can customize routing parameters via [`RouteParametersConfig`].
-	/// To use default settings, call the function with [`RouteParametersConfig::default`].
-	///
 	/// # Payment
 	///
 	/// The provided `payment_id` is used to ensure that only one invoice is paid for the request
@@ -11345,6 +11380,7 @@ where
 	///
 	/// Errors if:
 	/// - a duplicate `payment_id` is provided given the caveats in the aforementioned link,
+	/// - [`OptionalOfferPaymentInfo.quantity`] is set.
 	///
 	/// [BIP 353]: https://github.com/bitcoin/bips/blob/master/bip-0353.mediawiki
 	/// [bLIP 32]: https://github.com/lightning/blips/blob/master/blip-0032.md
@@ -11358,20 +11394,23 @@ where
 	#[cfg(feature = "dnssec")]
 	pub fn pay_for_offer_from_human_readable_name(
 		&self, name: HumanReadableName, amount_msats: u64, payment_id: PaymentId,
-		payer_note: Option<String>, retry_strategy: Retry,
-		route_params_config: RouteParametersConfig, dns_resolvers: Vec<Destination>,
+		optional_info: OptionalOfferPaymentInfo, dns_resolvers: Vec<Destination>,
 	) -> Result<(), ()> {
+		if optional_info.quantity.is_some() {
+			return Err(());
+		}
+
 		let (onion_message, context) =
 			self.flow.hrn_resolver.resolve_name(payment_id, name, &*self.entropy_source)?;
 
 		let expiration = StaleExpiration::TimerTicks(1);
 		self.pending_outbound_payments.add_new_awaiting_offer(
 			payment_id,
 			expiration,
-			retry_strategy,
-			route_params_config,
+			optional_info.retry_strategy,
+			optional_info.route_params_config,
 			amount_msats,
-			payer_note,
+			optional_info.payer_note,
 		)?;
 
 		self.flow