Skip to content

Allow setting a payer_note on pay_for_offer_from_human_readable_name #3808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 4, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ci/ci-tests.sh
Original file line number Diff line number Diff line change
@@ -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
93 changes: 90 additions & 3 deletions lightning-dns-resolver/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be okay for now because kinda pre-existing, but going forward we'll probably want to DRY up that test code instead of copy/pasting the same flow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, happy to review a followup DRYing up these tests

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!();
}
36 changes: 23 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
@@ -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(),
9 changes: 6 additions & 3 deletions lightning/src/ln/outbound_payment.rs
Original file line number Diff line number Diff line change
@@ -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),
},
);