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
Show file tree
Hide file tree
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
Expand Up @@ -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
Expand Down
93 changes: 90 additions & 3 deletions lightning-dns-resolver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)];
Expand All @@ -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();
Expand Down Expand Up @@ -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!();
}
Expand Down
36 changes: 23 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
///
Expand Down Expand Up @@ -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
///
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
9 changes: 6 additions & 3 deletions lightning/src/ln/outbound_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -1786,6 +1787,7 @@ impl OutboundPayments {
retry_strategy,
route_params_config,
amount_msats,
payer_note,
});

Ok(())
Expand All @@ -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(()),
Expand Down Expand Up @@ -2583,6 +2585,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
)
))),
(6, amount_msats, required),
(7, payer_note, option),
},
);

Expand Down
Loading