Skip to content
Open
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
1 change: 1 addition & 0 deletions fuzz/src/invoice_request_deser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
let payment_context = PaymentContext::Bolt12Offer(Bolt12OfferContext {
offer_id: OfferId([42; 32]),
invoice_request: invoice_request_fields,
payment_metadata: None,
});
let payee_tlvs = ReceiveTlvs {
payment_secret: PaymentSecret([42; 32]),
Expand Down
2 changes: 1 addition & 1 deletion fuzz/src/refund_deser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
) -> Result<UnsignedBolt12Invoice, Bolt12SemanticError> {
let entropy_source = Randomness {};
let receive_auth_key = ReceiveAuthKey([41; 32]);
let payment_context = PaymentContext::Bolt12Refund(Bolt12RefundContext {});
let payment_context = PaymentContext::Bolt12Refund(Bolt12RefundContext { payment_metadata: None });
let payee_tlvs = ReceiveTlvs {
payment_secret: PaymentSecret([42; 32]),
payment_constraints: PaymentConstraints {
Expand Down
28 changes: 27 additions & 1 deletion lightning/src/blinded_path/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

//! Data structures and methods for constructing [`BlindedMessagePath`]s to send a message over.

use alloc::collections::BTreeMap;

use bitcoin::secp256k1::{self, PublicKey, Secp256k1, SecretKey};

#[allow(unused_imports)]
Expand All @@ -29,7 +31,9 @@ use crate::routing::gossip::{NodeId, ReadOnlyNetworkGraph};
use crate::sign::{EntropySource, NodeSigner, ReceiveAuthKey, Recipient};
use crate::types::payment::PaymentHash;
use crate::util::scid_utils;
use crate::util::ser::{FixedLengthReader, LengthReadableArgs, Readable, Writeable, Writer};
use crate::util::ser::{
BigSizeKeyedMap, FixedLengthReader, LengthReadableArgs, Readable, Writeable, Writer,
};

use core::time::Duration;
use core::{cmp, mem};
Expand Down Expand Up @@ -391,6 +395,27 @@ pub enum OffersContext {
/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
/// [`Offer`]: crate::offers::offer::Offer
nonce: Nonce,

/// Additional data about this payment which is not used in LDK and can be used for any
/// purpose.
///
/// This is analogous to the BOLT 11 [`RecipientOnionFields::payment_metadata`] (which is
/// provided to payers via [`Bolt11Invoice::payment_metadata`]) and can be used any time data
/// needs to be "stored" by a payment recipient for their own internal use, provided back to
/// them with the payment.
///
/// Payment metadata is stored as a map from a numeric key to an arbitrary byte array value.
/// This allows for several types of metadata to be stored attached to a single payment. In the
/// future some optional features of LDK may use some keys.
///
/// Note that because this is included in the payment onion, its size must be tightly
/// constrained. More than a few hundred bytes and the payment will be entirely unpayable (with
/// limited routing options as size increases). Further, any data placed here will increase
/// the size of the offer which may make it difficult to fit in QR codes.
///
/// [`RecipientOnionFields::payment_metadata`]: crate::ln::outbound_payment::RecipientOnionFields::payment_metadata
/// [`Bolt11Invoice::payment_metadata`]: lightning_invoice::Bolt11Invoice::payment_metadata
payment_metadata: Option<BTreeMap<u64, Vec<u8>>>,
},
/// Context used by a [`BlindedMessagePath`] within the [`Offer`] of an async recipient.
///
Expand Down Expand Up @@ -648,6 +673,7 @@ impl_writeable_tlv_based_enum!(MessageContext,
impl_writeable_tlv_based_enum!(OffersContext,
(0, InvoiceRequest) => {
(0, nonce, required),
(1, payment_metadata, (option, encoding: (BTreeMap<u64, Vec<u8>>, BigSizeKeyedMap))),
},
(1, OutboundPaymentForRefund) => {
(0, payment_id, required),
Expand Down
108 changes: 99 additions & 9 deletions lightning/src/blinded_path/payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

//! Data structures and methods for constructing [`BlindedPaymentPath`]s to send a payment over.

use alloc::collections::BTreeMap;

use bitcoin::secp256k1::ecdh::SharedSecret;
use bitcoin::secp256k1::{self, PublicKey, Secp256k1, SecretKey};

Expand All @@ -29,8 +31,8 @@ use crate::types::features::BlindedHopFeatures;
use crate::types::payment::PaymentSecret;
use crate::types::routing::RoutingFees;
use crate::util::ser::{
FixedLengthReader, HighZeroBytesDroppedBigSize, LengthReadableArgs, Readable, WithoutLength,
Writeable, Writer,
BigSizeKeyedMap, FixedLengthReader, HighZeroBytesDroppedBigSize, LengthReadableArgs, Readable,
WithoutLength, Writeable, Writer,
};

#[allow(unused_imports)]
Expand Down Expand Up @@ -572,6 +574,20 @@ pub enum PaymentContext {
/// [`Refund`]: crate::offers::refund::Refund
Bolt12Refund(Bolt12RefundContext),
}
impl PaymentContext {
/// Returns the additional payment metadata stored alongside this payment context, if any.
///
/// Payment metadata is stored as a map from a numeric key to an arbitrary byte array value.
/// This allows for several types of metadata to be stored attached to a single payment. In the
/// future some optional features of LDK may use some keys.
pub fn payment_metadata(&self) -> Option<&BTreeMap<u64, Vec<u8>>> {
Comment thread
TheBlueMatt marked this conversation as resolved.
match self {
Self::Bolt12Offer(Bolt12OfferContext { payment_metadata, .. })
| Self::AsyncBolt12Offer(AsyncBolt12OfferContext { payment_metadata, .. })
| Self::Bolt12Refund(Bolt12RefundContext { payment_metadata, .. }) => payment_metadata.as_ref(),
}
}
}

// Used when writing PaymentContext in Event::PaymentClaimable to avoid cloning.
pub(crate) enum PaymentContextRef<'a> {
Expand All @@ -594,6 +610,26 @@ pub struct Bolt12OfferContext {
/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
pub invoice_request: InvoiceRequestFields,

/// Additional data about this payment which is not used in LDK and can be used for any
/// purpose.
///
/// This is analogous to the BOLT 11 [`RecipientOnionFields::payment_metadata`] (which is
/// provided to payers via [`Bolt11Invoice::payment_metadata`]) and can be used any time data
/// needs to be "stored" by a payment recipient for their own internal use, provided back to
/// them with the payment.
///
Comment thread
TheBlueMatt marked this conversation as resolved.
/// Payment metadata is stored as a map from a numeric key to an arbitrary byte array value.
/// This allows for several types of metadata to be stored attached to a single payment. In the
/// future some optional features of LDK may use some keys.
///
/// Note that because this is included in the payment onion, its size must be tightly
/// constrained. More than a few hundred bytes and the payment will be entirely unpayable (with
/// limited routing options as size increases).
///
/// [`RecipientOnionFields::payment_metadata`]: crate::ln::outbound_payment::RecipientOnionFields::payment_metadata
/// [`Bolt11Invoice::payment_metadata`]: lightning_invoice::Bolt11Invoice::payment_metadata
pub payment_metadata: Option<BTreeMap<u64, Vec<u8>>>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See above: what are we expecting users to set the u64 to?

Shouldn't we rather use a dedicated PaymentMetadata type for this, or even an Option<Box<dyn Writeable>> or similar?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Answered at #4584 (comment) but I do really want to force users into the K-V-map box here. Can do a type if we want but its a bit awkward that we have a separate BOLT 11 and BOLT 12 PaymentMetadata type...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The Box<dyn Writeable> would get a bit ugly at read time. You'd need to parameterize the onion message parsing with your types.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why use a map? Wouldn't a user's wrapper struct be just as efficient encoding -- or could be even more efficient if each part is of a fixed length? Deserialization would be simpler, too, if they didn't need to iterate a map.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The intention is both to force users to think about forward compatibility from day one (in the sense that its already a K-V) but also because we want to reserve some keys in LDK (eg in #4463).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The intention is both to force users to think about forward compatibility from day one (in the sense that its already a K-V)

Isn't that what using our serialization macros provide? Blinded paths are also not expected to be useable indefinitely, so the compatibility argument isn't as strong, IMO.

FWIW, the rationale given in the commit message only mentions composability.

but also because we want to reserve some keys in LDK (eg in #4463).

Wouldn't it be simpler to add a dedicated field for anything LDK-specific? Then we wouldn't need custom parsing / serialization logic (for each variant of PaymentContext) or need to worry about checking if users took a reserved type. This would need to be added somewhere in the code vs a simple declaration in the impl_writeable_tlv_based uses.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Isn't that what using our serialization macros provide? Blinded paths are also not expected to be useable indefinitely, so the compatibility argument isn't as strong, IMO.

Right, for cases where there's a single top-level struct which the downstream code wants to use.

Wouldn't it be simpler to add a dedicated field for anything LDK-specific?

Yea, this is probably true for #4463, but I'm not sure how this would work, eg, if LDK Node had something it wanted to add? Would we add a field here just for LDK Node's usage? Or would LDK Node define a struct which then had a similar user-defined sub-struct?

I guess its not clear to me how exactly a struct would work, anyway, would we parameterize ~everything by the custom context struct?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Right, for cases where there's a single top-level struct which the downstream code wants to use.

We could advise to include a top-level struct with a field holding a sub-struct with their data. Then they would simply add a new struct / field for another use case. Alternatively, their serialization could have a version number if they ever want to swap-out a top-level struct.

Yea, this is probably true for #4463, but I'm not sure how this would work, eg, if LDK Node had something it wanted to add? Would we add a field here just for LDK Node's usage? Or would LDK Node define a struct which then had a similar user-defined sub-struct?

Right, I guess we'd need to know how LDK Node would communicate to LDK to set the field and how it can be communicated back to LDK Node when handling a message sent over the blinded path. For payment paths, we'd expose it through PaymentClaimable. But how will it be set?

For the LSP case, I thought the idea would be to communicate data through the blinded message path. And then LDK would have logic for setting data in the blinded payment path when handing an invoice request containing that data.

For other use cases, we'd probably force them to use OffersMessageFlow (or a different system / handler if this is not offers-related blinded path usage). We could make LDK Node use OffersMessageFlow, too. Or just provide the tighter integration with LDK as described above.

I guess its not clear to me how exactly a struct would work, anyway, would we parameterize ~everything by the custom context struct?

I was saying to just have a Vec<u8> and allow consumers to parse as needed. They could use a custom struct and parameterize everything with it. But for LDK-specific data, we'd have our own dedicated struct / field.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I was saying to just have a Vec and allow consumers to parse as needed. They could use a custom struct and parameterize everything with it. But for LDK-specific data, we'd have our own dedicated struct / field.

Ah, yea, I really don't love that. At the cost of two extra bytes forcing them to have a specified key effectively forces them to have forward/backwards compat (which, indeed, doesn't matter as much in blinded payment paths but we're also using the same field in offers blinded paths, which are long-lived) and allows user data to live parallel with LDK Node (or other LDK non-lightning-crate) data. It seems very much worth it to have that and I don't really think the API complexity cost is all that high here.

}

/// The context of a payment made for a static invoice requested from a BOLT 12 [`Offer`].
Expand All @@ -606,13 +642,53 @@ pub struct AsyncBolt12OfferContext {
///
/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
pub offer_nonce: Nonce,

/// Additional data about this payment which is not used in LDK and can be used for any
/// purpose.
///
/// This is analogous to the BOLT 11 [`RecipientOnionFields::payment_metadata`] (which is
/// provided to payers via [`Bolt11Invoice::payment_metadata`]) and can be used any time data
/// needs to be "stored" by a payment recipient for their own internal use, provided back to
/// them with the payment.
///
/// Payment metadata is stored as a map from a numeric key to an arbitrary byte array value.
/// This allows for several types of metadata to be stored attached to a single payment. In the
/// future some optional features of LDK may use some keys.
///
/// Note that because this is included in the payment onion, its size must be tightly
/// constrained. More than a few hundred bytes and the payment will be entirely unpayable (with
/// limited routing options as size increases).
///
/// [`RecipientOnionFields::payment_metadata`]: crate::ln::outbound_payment::RecipientOnionFields::payment_metadata
/// [`Bolt11Invoice::payment_metadata`]: lightning_invoice::Bolt11Invoice::payment_metadata
pub payment_metadata: Option<BTreeMap<u64, Vec<u8>>>,
}

/// The context of a payment made for an invoice sent for a BOLT 12 [`Refund`].
///
/// [`Refund`]: crate::offers::refund::Refund
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct Bolt12RefundContext {}
pub struct Bolt12RefundContext {
/// Additional data about this payment which is not used in LDK and can be used for any
/// purpose.
///
/// This is analogous to the BOLT 11 [`RecipientOnionFields::payment_metadata`] (which is
/// provided to payers via [`Bolt11Invoice::payment_metadata`]) and can be used any time data
/// needs to be "stored" by a payment recipient for their own internal use, provided back to
/// them with the payment.
///
/// Payment metadata is stored as a map from a numeric key to an arbitrary byte array value.
/// This allows for several types of metadata to be stored attached to a single payment. In the
/// future some optional features of LDK may use some keys.
///
/// Note that because this is included in the payment onion, its size must be tightly
/// constrained. More than a few hundred bytes and the payment will be entirely unpayable (with
/// limited routing options as size increases).
///
/// [`RecipientOnionFields::payment_metadata`]: crate::ln::outbound_payment::RecipientOnionFields::payment_metadata
/// [`Bolt11Invoice::payment_metadata`]: lightning_invoice::Bolt11Invoice::payment_metadata
pub payment_metadata: Option<BTreeMap<u64, Vec<u8>>>,
}

impl TryFrom<CounterpartyForwardingInfo> for PaymentRelay {
type Error = ();
Expand Down Expand Up @@ -1031,14 +1107,18 @@ impl<'a> Writeable for PaymentContextRef<'a> {

impl_writeable_tlv_based!(Bolt12OfferContext, {
(0, offer_id, required),
(1, payment_metadata, (option, encoding: (BTreeMap<u64, Vec<u8>>, BigSizeKeyedMap))),
(2, invoice_request, required),
});

impl_writeable_tlv_based!(AsyncBolt12OfferContext, {
(0, offer_nonce, required),
(1, payment_metadata, (option, encoding: (BTreeMap<u64, Vec<u8>>, BigSizeKeyedMap))),
});

impl_writeable_tlv_based!(Bolt12RefundContext, {});
impl_writeable_tlv_based!(Bolt12RefundContext, {
(1, payment_metadata, (option, encoding: (BTreeMap<u64, Vec<u8>>, BigSizeKeyedMap))),
});

#[cfg(test)]
mod tests {
Expand Down Expand Up @@ -1097,7 +1177,9 @@ mod tests {
let recv_tlvs = ReceiveTlvs {
payment_secret: PaymentSecret([0; 32]),
payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 1 },
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {
payment_metadata: None,
}),
};
let htlc_maximum_msat = 100_000;
let blinded_payinfo =
Expand All @@ -1115,7 +1197,9 @@ mod tests {
let recv_tlvs = ReceiveTlvs {
payment_secret: PaymentSecret([0; 32]),
payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 1 },
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {
payment_metadata: None,
}),
};
let blinded_payinfo = super::compute_payinfo::<ForwardTlvs>(
&[],
Expand Down Expand Up @@ -1178,7 +1262,9 @@ mod tests {
let recv_tlvs = ReceiveTlvs {
payment_secret: PaymentSecret([0; 32]),
payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 3 },
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {
payment_metadata: None,
}),
};
let htlc_maximum_msat = 100_000;
let blinded_payinfo = super::compute_payinfo(
Expand Down Expand Up @@ -1238,7 +1324,9 @@ mod tests {
let recv_tlvs = ReceiveTlvs {
payment_secret: PaymentSecret([0; 32]),
payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 1 },
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {
payment_metadata: None,
}),
};
let htlc_minimum_msat = 3798;
assert!(super::compute_payinfo(
Expand Down Expand Up @@ -1309,7 +1397,9 @@ mod tests {
let recv_tlvs = ReceiveTlvs {
payment_secret: PaymentSecret([0; 32]),
payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 1 },
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {
payment_metadata: None,
}),
};

let blinded_payinfo = super::compute_payinfo(
Expand Down
Loading
Loading