Merge pull request #3808 from elnosh/payer-note-hrn

Allow setting a `payer_note` on `pay_for_offer_from_human_readable_name`

Author: tnull

ci/ci-tests.sh

@@ -74,6 +74,11 @@ for DIR in "${WORKSPACE_MEMBERS[@]}"; do
 	cargo doc -p "$DIR" --document-private-items
 done
 
+echo -e "\n\nChecking and testing lightning with features"
+cargo test -p lightning --verbose --color always --features dnssec
+cargo check -p lightning --verbose --color always --features dnssec
+cargo doc -p lightning --document-private-items --features dnssec
+
 echo -e "\n\nChecking and testing Block Sync Clients with features"
 
 cargo test -p lightning-block-sync --verbose --color always --features rest-client

lightning-dns-resolver/src/lib.rs

@@ -182,6 +182,7 @@ mod test {
 		commitment_signed_dance, expect_payment_claimed, expect_pending_htlcs_forwardable,
 		get_htlc_update_msgs,
 	};
+	use lightning_types::string::UntrustedString;
 
 	use std::ops::Deref;
 	use std::sync::Mutex;
@@ -396,7 +397,7 @@ mod test {
 		// When we get the proof back, override its contents to an offer from nodes[1]
 		let bs_offer = nodes[1].node.create_offer_builder(None).unwrap().build().unwrap();
 		let proof_override = &nodes[0].node.testing_dnssec_proof_offer_resolution_override;
-		proof_override.lock().unwrap().insert(name.clone(), bs_offer);
+		proof_override.lock().unwrap().insert(name.clone(), bs_offer.clone());
 
 		let payment_id = PaymentId([42; 32]);
 		let resolvers = vec![Destination::Node(resolver_id)];
@@ -405,7 +406,86 @@ mod test {
 		let params = RouteParametersConfig::default();
 		nodes[0]
 			.node
-			.pay_for_offer_from_human_readable_name(name, amt, payment_id, retry, params, resolvers)
+			.pay_for_offer_from_human_readable_name(
+				name.clone(),
+				amt,
+				payment_id,
+				None,
+				retry,
+				params,
+				resolvers.clone(),
+			)
+			.unwrap();
+
+		let query = nodes[0].onion_messenger.next_onion_message_for_peer(resolver_id).unwrap();
+		resolver_messenger.get_om().handle_onion_message(payer_id, &query);
+
+		assert!(resolver_messenger.get_om().next_onion_message_for_peer(payer_id).is_none());
+		let start = Instant::now();
+		let response = loop {
+			tokio::time::sleep(Duration::from_millis(10)).await;
+			if let Some(msg) = resolver_messenger.get_om().next_onion_message_for_peer(payer_id) {
+				break msg;
+			}
+			assert!(start.elapsed() < Duration::from_secs(10), "Resolution took too long");
+		};
+
+		nodes[0].onion_messenger.handle_onion_message(resolver_id, &response);
+
+		let invreq = nodes[0].onion_messenger.next_onion_message_for_peer(payee_id).unwrap();
+		nodes[1].onion_messenger.handle_onion_message(payer_id, &invreq);
+
+		let inv = nodes[1].onion_messenger.next_onion_message_for_peer(payer_id).unwrap();
+		nodes[0].onion_messenger.handle_onion_message(payee_id, &inv);
+
+		check_added_monitors(&nodes[0], 1);
+		let updates = get_htlc_update_msgs!(nodes[0], payee_id);
+		nodes[1].node.handle_update_add_htlc(payer_id, &updates.update_add_htlcs[0]);
+		commitment_signed_dance!(nodes[1], nodes[0], updates.commitment_signed, false);
+		expect_pending_htlcs_forwardable!(nodes[1]);
+
+		let claimable_events = nodes[1].node.get_and_clear_pending_events();
+		assert_eq!(claimable_events.len(), 1);
+		let our_payment_preimage;
+		if let Event::PaymentClaimable { purpose, amount_msat, .. } = &claimable_events[0] {
+			assert_eq!(*amount_msat, amt);
+			if let PaymentPurpose::Bolt12OfferPayment {
+				payment_preimage, payment_context, ..
+			} = purpose
+			{
+				our_payment_preimage = payment_preimage.unwrap();
+				nodes[1].node.claim_funds(our_payment_preimage);
+				let payment_hash: PaymentHash = our_payment_preimage.into();
+				expect_payment_claimed!(nodes[1], payment_hash, amt);
+				assert_eq!(payment_context.invoice_request.payer_note_truncated, None);
+			} else {
+				panic!();
+			}
+		} else {
+			panic!();
+		}
+
+		check_added_monitors(&nodes[1], 1);
+		let updates = get_htlc_update_msgs!(nodes[1], payer_id);
+		nodes[0].node.handle_update_fulfill_htlc(payee_id, &updates.update_fulfill_htlcs[0]);
+		commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false);
+
+		expect_payment_sent(&nodes[0], our_payment_preimage, None, true, true);
+
+		// Pay offer with payer_note
+		let proof_override = &nodes[0].node.testing_dnssec_proof_offer_resolution_override;
+		proof_override.lock().unwrap().insert(name.clone(), bs_offer);
+		nodes[0]
+			.node
+			.pay_for_offer_from_human_readable_name(
+				name,
+				amt,
+				PaymentId([21; 32]),
+				Some("foo".into()),
+				retry,
+				params,
+				resolvers,
+			)
 			.unwrap();
 
 		let query = nodes[0].onion_messenger.next_onion_message_for_peer(resolver_id).unwrap();
@@ -440,11 +520,18 @@ mod test {
 		let our_payment_preimage;
 		if let Event::PaymentClaimable { purpose, amount_msat, .. } = &claimable_events[0] {
 			assert_eq!(*amount_msat, amt);
-			if let PaymentPurpose::Bolt12OfferPayment { payment_preimage, .. } = purpose {
+			if let PaymentPurpose::Bolt12OfferPayment {
+				payment_preimage, payment_context, ..
+			} = purpose
+			{
 				our_payment_preimage = payment_preimage.unwrap();
 				nodes[1].node.claim_funds(our_payment_preimage);
 				let payment_hash: PaymentHash = our_payment_preimage.into();
 				expect_payment_claimed!(nodes[1], payment_hash, amt);
+				assert_eq!(
+					payment_context.invoice_request.payer_note_truncated,
+					Some(UntrustedString("foo".into()))
+				);
 			} else {
 				panic!();
 			}

lightning/src/ln/channelmanager.rs

@@ -4876,7 +4876,7 @@ where
 	///
 	/// # Custom Routing Parameters
 	/// Users can customize routing parameters via [`RouteParametersConfig`].
-	/// To use default settings, call the function with `RouteParametersConfig::default()`.
+	/// To use default settings, call the function with [`RouteParametersConfig::default`].
 	pub fn pay_for_bolt11_invoice(
 		&self, invoice: &Bolt11Invoice, payment_id: PaymentId, amount_msats: Option<u64>,
 		route_params_config: RouteParametersConfig, retry_strategy: Retry
@@ -10390,8 +10390,10 @@ where
 	/// - `amount_msats` if overpaying what is required for the given `quantity` is desired, and
 	/// - `payer_note` for [`InvoiceRequest::payer_note`].
 	///
-	/// If `max_total_routing_fee_msat` is not specified, The default from
-	/// [`RouteParameters::from_payment_params_and_value`] is applied.
+	/// # Custom Routing Parameters
+	///
+	/// Users can customize routing parameters via [`RouteParametersConfig`].
+	/// To use default settings, call the function with [`RouteParametersConfig::default`].
 	///
 	/// # Payment
 	///
@@ -10545,15 +10547,17 @@ where
 	}
 
 	/// Pays for an [`Offer`] looked up using [BIP 353] Human Readable Names resolved by the DNS
-	/// resolver(s) at `dns_resolvers` which resolve names according to bLIP 32.
+	/// resolver(s) at `dns_resolvers` which resolve names according to [bLIP 32].
 	///
 	/// If the wallet supports paying on-chain schemes, you should instead use
 	/// [`OMNameResolver::resolve_name`] and [`OMNameResolver::handle_dnssec_proof_for_uri`] (by
 	/// implementing [`DNSResolverMessageHandler`]) directly to look up a URI and then delegate to
 	/// your normal URI handling.
 	///
-	/// If `max_total_routing_fee_msat` is not specified, the default from
-	/// [`RouteParameters::from_payment_params_and_value`] is applied.
+	/// # Custom Routing Parameters
+	///
+	/// Users can customize routing parameters via [`RouteParametersConfig`].
+	/// To use default settings, call the function with [`RouteParametersConfig::default`].
 	///
 	/// # Payment
 	///
@@ -10563,40 +10567,46 @@ where
 	///
 	/// To revoke the request, use [`ChannelManager::abandon_payment`] prior to receiving the
 	/// invoice. If abandoned, or an invoice isn't received in a reasonable amount of time, the
-	/// payment will fail with an [`Event::InvoiceRequestFailed`].
+	/// payment will fail with an [`PaymentFailureReason::UserAbandoned`] or
+	/// [`PaymentFailureReason::InvoiceRequestExpired`], respectively.
 	///
 	/// # Privacy
 	///
 	/// For payer privacy, uses a derived payer id and uses [`MessageRouter::create_blinded_paths`]
-	/// to construct a [`BlindedPath`] for the reply path. For further privacy implications, see the
+	/// to construct a [`BlindedMessagePath`] for the reply path. For further privacy implications, see the
 	/// docs of the parameterized [`Router`], which implements [`MessageRouter`].
 	///
 	/// # Limitations
 	///
 	/// Requires a direct connection to the given [`Destination`] as well as an introduction node in
-	/// [`Offer::paths`] or to [`Offer::signing_pubkey`], if empty. A similar restriction applies to
+	/// [`Offer::paths`] or to [`Offer::issuer_signing_pubkey`], if empty. A similar restriction applies to
 	/// the responding [`Bolt12Invoice::payment_paths`].
 	///
 	/// # Errors
 	///
 	/// Errors if:
 	/// - a duplicate `payment_id` is provided given the caveats in the aforementioned link,
 	///
+	/// [BIP 353]: https://github.com/bitcoin/bips/blob/master/bip-0353.mediawiki
+	/// [bLIP 32]: https://github.com/lightning/blips/blob/master/blip-0032.md
 	/// [`Bolt12Invoice::payment_paths`]: crate::offers::invoice::Bolt12Invoice::payment_paths
 	/// [`OMNameResolver::resolve_name`]: crate::onion_message::dns_resolution::OMNameResolver::resolve_name
 	/// [`OMNameResolver::handle_dnssec_proof_for_uri`]: crate::onion_message::dns_resolution::OMNameResolver::handle_dnssec_proof_for_uri
 	/// [Avoiding Duplicate Payments]: #avoiding-duplicate-payments
+	/// [`BlindedMessagePath`]: crate::blinded_path::message::BlindedMessagePath
+	/// [`PaymentFailureReason::UserAbandoned`]: crate::events::PaymentFailureReason::UserAbandoned
+	/// [`PaymentFailureReason::InvoiceRequestRejected`]: crate::events::PaymentFailureReason::InvoiceRequestRejected
 	#[cfg(feature = "dnssec")]
 	pub fn pay_for_offer_from_human_readable_name(
-		&self, name: HumanReadableName, amount_msats: u64, payment_id: PaymentId,
+		&self, name: HumanReadableName, amount_msats: u64, payment_id: PaymentId, payer_note: Option<String>,
 		retry_strategy: Retry, route_params_config: RouteParametersConfig,
 		dns_resolvers: Vec<Destination>,
 	) -> Result<(), ()> {
 		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, amount_msats)?;
+		self.pending_outbound_payments.add_new_awaiting_offer(payment_id, expiration, retry_strategy, route_params_config, amount_msats, payer_note)?;
 
 		self.flow.enqueue_dns_onion_message(
 			onion_message, context, dns_resolvers,
@@ -12463,9 +12473,9 @@ where
 					// offer, but tests can deal with that.
 					offer = replacement_offer;
 				}
-				if let Ok(amt_msats) = self.pending_outbound_payments.amt_msats_for_payment_awaiting_offer(payment_id) {
+				if let Ok((amt_msats, payer_note)) = self.pending_outbound_payments.params_for_payment_awaiting_offer(payment_id) {
 					let offer_pay_res =
-						self.pay_for_offer_intern(&offer, None, Some(amt_msats), None, payment_id, Some(name),
+						self.pay_for_offer_intern(&offer, None, Some(amt_msats), payer_note, payment_id, Some(name),
 							|invoice_request, nonce| {
 								let retryable_invoice_request = RetryableInvoiceRequest {
 									invoice_request: invoice_request.clone(),

lightning/src/ln/outbound_payment.rs

@@ -67,6 +67,7 @@ pub(crate) enum PendingOutboundPayment {
 		/// Human Readable Names-originated payments should always specify an explicit amount to
 		/// send up-front, which we track here and enforce once we receive the offer.
 		amount_msats: u64,
+		payer_note: Option<String>
 	},
 	AwaitingInvoice {
 		expiration: StaleExpiration,
@@ -1775,7 +1776,7 @@ impl OutboundPayments {
 	#[cfg(feature = "dnssec")]
 	pub(super) fn add_new_awaiting_offer(
 		&self, payment_id: PaymentId, expiration: StaleExpiration, retry_strategy: Retry,
-		route_params_config: RouteParametersConfig, amount_msats: u64,
+		route_params_config: RouteParametersConfig, amount_msats: u64, payer_note: Option<String>
 	) -> Result<(), ()> {
 		let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
 		match pending_outbounds.entry(payment_id) {
@@ -1786,6 +1787,7 @@ impl OutboundPayments {
 					retry_strategy,
 					route_params_config,
 					amount_msats,
+					payer_note,
 				});
 
 				Ok(())
@@ -1794,10 +1796,10 @@ impl OutboundPayments {
 	}
 
 	#[cfg(feature = "dnssec")]
-	pub(super) fn amt_msats_for_payment_awaiting_offer(&self, payment_id: PaymentId) -> Result<u64, ()> {
+	pub(super) fn params_for_payment_awaiting_offer(&self, payment_id: PaymentId) -> Result<(u64, Option<String>), ()> {
 		match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
 			hash_map::Entry::Occupied(entry) => match entry.get() {
-				PendingOutboundPayment::AwaitingOffer { amount_msats, .. } => Ok(*amount_msats),
+				PendingOutboundPayment::AwaitingOffer { amount_msats, payer_note, .. } => Ok((*amount_msats, payer_note.clone())),
 				_ => Err(()),
 			},
 			_ => Err(()),
@@ -2583,6 +2585,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 			)
 		))),
 		(6, amount_msats, required),
+		(7, payer_note, option),
 	},
 );