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

@@ -164,7 +164,7 @@ mod test {
 	};
 	use lightning::blinded_path::NodeIdLookUp;
 	use lightning::events::{Event, PaymentPurpose};
-	use lightning::ln::channelmanager::{PaymentId, Retry};
+	use lightning::ln::channelmanager::{OptionalOfferPaymentParams, PaymentId};
 	use lightning::ln::functional_test_utils::*;
 	use lightning::ln::msgs::{
 		BaseMessageHandler, ChannelMessageHandler, Init, OnionMessageHandler,
@@ -175,7 +175,6 @@ mod test {
 	use lightning::onion_message::messenger::{
 		AOnionMessenger, Destination, MessageRouter, OnionMessagePath, OnionMessenger,
 	};
-	use lightning::routing::router::RouteParametersConfig;
 	use lightning::sign::{KeysManager, NodeSigner, ReceiveAuthKey, Recipient};
 	use lightning::types::features::InitFeatures;
 	use lightning::types::payment::PaymentHash;
@@ -380,23 +379,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 = OptionalOfferPaymentParams::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();
@@ -493,9 +487,6 @@ mod test {
 
 		let bs_offer = nodes[1].node.create_offer_builder().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,
@@ -505,11 +496,8 @@ mod test {
 			payee_id,
 			bs_offer.clone(),
 			name.clone(),
-			amt,
 			PaymentId([42; 32]),
 			None,
-			retry,
-			params,
 			resolvers.clone(),
 		)
 		.await;
@@ -523,11 +511,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

@@ -15,7 +15,9 @@ use crate::events::{
 	Event, HTLCHandlingFailureType, PaidBolt12Invoice, PaymentFailureReason, PaymentPurpose,
 };
 use crate::ln::blinded_payment_tests::{fail_blinded_htlc_backwards, get_blinded_route_parameters};
-use crate::ln::channelmanager::{Bolt12PaymentError, PaymentId, RecipientOnionFields};
+use crate::ln::channelmanager::{
+	Bolt12PaymentError, OptionalOfferPaymentParams, PaymentId, RecipientOnionFields,
+};
 use crate::ln::functional_test_utils::*;
 use crate::ln::inbound_payment;
 use crate::ln::msgs;
@@ -49,7 +51,7 @@ use crate::onion_message::messenger::{
 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;
@@ -498,11 +500,7 @@ fn static_invoice_unknown_required_features() {
 	// 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)
-		.unwrap();
+	nodes[0].node.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default()).unwrap();
 
 	// Don't forward the invreq since the invoice was created outside of the normal flow, instead
 	// manually construct the response.
@@ -574,11 +572,7 @@ fn ignore_unexpected_static_invoice() {
 
 	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();
+	nodes[0].node.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default()).unwrap();
 
 	let invreq_om = nodes[0]
 		.onion_messenger
@@ -707,11 +701,7 @@ fn ignore_duplicate_invoice() {
 	let offer = async_recipient.node.get_async_receive_offer().unwrap();
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
-	sender
-		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
-		.unwrap();
+	sender.node.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default()).unwrap();
 
 	let sender_node_id = sender.node.get_our_node_id();
 	let always_online_node_id = always_online_node.node.get_our_node_id();
@@ -809,10 +799,7 @@ fn ignore_duplicate_invoice() {
 
 	// Now handle case where the sender pays regular invoice and ignores static invoice.
 	let payment_id = PaymentId([2; 32]);
-	sender
-		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
-		.unwrap();
+	sender.node.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default()).unwrap();
 
 	let invreq_om =
 		sender.onion_messenger.next_onion_message_for_peer(always_online_node_id).unwrap();
@@ -918,11 +905,7 @@ fn async_receive_flow_success() {
 	let offer = nodes[2].node.get_async_receive_offer().unwrap();
 	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();
+	nodes[0].node.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],
@@ -982,11 +965,7 @@ 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)
-		.unwrap();
+	nodes[0].node.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default()).unwrap();
 
 	let invreq_om = nodes[0]
 		.onion_messenger
@@ -1069,11 +1048,7 @@ fn timeout_unreleased_payment() {
 
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
-	sender
-		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
-		.unwrap();
+	sender.node.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default()).unwrap();
 
 	let invreq_om =
 		sender.onion_messenger.next_onion_message_for_peer(server.node.get_our_node_id()).unwrap();
@@ -1166,11 +1141,7 @@ fn async_receive_mpp() {
 
 	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)
-		.unwrap();
+	nodes[0].node.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],
@@ -1268,11 +1239,7 @@ 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)
-		.unwrap();
+	nodes[0].node.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],
@@ -1515,11 +1482,7 @@ fn invalid_async_receive_with_retry<F1, F2>(
 	let static_invoice = invoice_flow_res.invoice;
 	let offer = nodes[2].node.get_async_receive_offer().unwrap();
 
-	let params = RouteParametersConfig::default();
-	nodes[0]
-		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(2), params)
-		.unwrap();
+	nodes[0].node.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],
@@ -1613,11 +1576,7 @@ 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)
-		.unwrap();
+	nodes[0].node.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default()).unwrap();
 
 	// While the invoice is unexpired, respond with release_held_htlc.
 	let (held_htlc_available_om, _release_held_htlc_om) = pass_async_payments_oms(
@@ -1730,11 +1689,9 @@ fn expired_static_invoice_payment_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(0), params)
-		.unwrap();
+	let mut params: OptionalOfferPaymentParams = 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],
@@ -2178,11 +2135,7 @@ fn refresh_static_invoices_for_used_offers() {
 	let offer = recipient.node.get_async_receive_offer().unwrap();
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
-	sender
-		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
-		.unwrap();
+	sender.node.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default()).unwrap();
 
 	let release_held_htlc_om = pass_async_payments_oms(
 		updated_invoice.clone(),
@@ -2514,11 +2467,7 @@ fn invoice_server_is_not_channel_peer() {
 	let offer = recipient.node.get_async_receive_offer().unwrap();
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
-	sender
-		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
-		.unwrap();
+	sender.node.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default()).unwrap();
 
 	// Do the held_htlc_available --> release_held_htlc dance.
 	let release_held_htlc_om = pass_async_payments_oms(
@@ -2580,11 +2529,7 @@ fn invoice_request_forwarded_to_async_recipient() {
 	let offer = async_recipient.node.get_async_receive_offer().unwrap();
 	let amt_msat = 5000;
 	let payment_id = PaymentId([1; 32]);
-	let params = RouteParametersConfig::default();
-	sender
-		.node
-		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
-		.unwrap();
+	sender.node.pay_for_offer(&offer, Some(amt_msat), payment_id, Default::default()).unwrap();
 
 	let sender_node_id = sender.node.get_our_node_id();