From 6ce239cdb940eba2f07e0f94004e4b430220e1e2 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 21 Dec 2021 05:23:15 +0000 Subject: [PATCH 01/15] Define the `PaymentMetadata` feature to be used in invoices --- lightning/src/ln/features.rs | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index b4a5c5ae52d..82242c71d05 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -160,6 +160,14 @@ mod sealed { VariableLengthOnion | PaymentSecret, // Byte 2 BasicMPP, + // Byte 3 + , + // Byte 4 + , + // Byte 5 + , + // Byte 6 + PaymentMetadata, ]); define_context!(OfferContext, []); define_context!(InvoiceRequestContext, []); @@ -376,6 +384,9 @@ mod sealed { define_feature!(47, SCIDPrivacy, [InitContext, NodeContext, ChannelTypeContext], "Feature flags for only forwarding with SCID aliasing. Called `option_scid_alias` in the BOLTs", set_scid_privacy_optional, set_scid_privacy_required, supports_scid_privacy, requires_scid_privacy); + define_feature!(49, PaymentMetadata, [InvoiceContext], + "Feature flags for payment metadata in invoices.", set_payment_metadata_optional, + set_payment_metadata_required, supports_payment_metadata, requires_payment_metadata); define_feature!(51, ZeroConf, [InitContext, NodeContext, ChannelTypeContext], "Feature flags for accepting channels with zero confirmations. Called `option_zeroconf` in the BOLTs", set_zero_conf_optional, set_zero_conf_required, supports_zero_conf, requires_zero_conf); @@ -885,13 +896,13 @@ mod tests { #[test] fn convert_to_context_with_unknown_flags() { // Ensure the `from` context has fewer known feature bytes than the `to` context. - assert!(::KNOWN_FEATURE_MASK.len() < - ::KNOWN_FEATURE_MASK.len()); - let mut invoice_features = InvoiceFeatures::empty(); - invoice_features.set_unknown_feature_optional(); - assert!(invoice_features.supports_unknown_bits()); - let node_features: NodeFeatures = invoice_features.to_context(); - assert!(!node_features.supports_unknown_bits()); + assert!(::KNOWN_FEATURE_MASK.len() < + ::KNOWN_FEATURE_MASK.len()); + let mut channel_features = ChannelFeatures::empty(); + channel_features.set_unknown_feature_optional(); + assert!(channel_features.supports_unknown_bits()); + let invoice_features: InvoiceFeatures = channel_features.to_context_internal(); + assert!(!invoice_features.supports_unknown_bits()); } #[test] From 35b597a4c27e54fcdd1feeb1c7dc77356c45c11b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 28 Mar 2023 20:46:11 +0000 Subject: [PATCH 02/15] Add features module-level documentation for missing features --- lightning/src/ln/features.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 82242c71d05..47e1cbcc4cb 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -45,17 +45,25 @@ //! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md) for more information). //! - `OnionMessages` - requires/supports forwarding onion messages //! (see [BOLT-7](https://github.com/lightning/bolts/pull/759/files) for more information). -//! TODO: update link +// TODO: update link //! - `ChannelType` - node supports the channel_type field in open/accept //! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md) for more information). //! - `SCIDPrivacy` - supply channel aliases for routing //! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md) for more information). +//! - `PaymentMetadata` - include additional data in invoices which is passed to recipients in the +//! onion. +//! (see [BOLT-11](https://github.com/lightning/bolts/blob/master/11-payment-encoding.md) for +//! more). +//! - `ZeroConf` - supports accepting HTLCs and using channels prior to funding confirmation +//! (see +//! [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#the-channel_ready-message) +//! for more info). //! - `Keysend` - send funds to a node without an invoice //! (see the [`Keysend` feature assignment proposal](https://github.com/lightning/bolts/issues/605#issuecomment-606679798) for more information). //! - `AnchorsZeroFeeHtlcTx` - requires/supports that commitment transactions include anchor outputs -//! and HTLC transactions are pre-signed with zero fee (see -//! [BOLT-3](https://github.com/lightning/bolts/blob/master/03-transactions.md) for more -//! information). +//! and HTLC transactions are pre-signed with zero fee (see +//! [BOLT-3](https://github.com/lightning/bolts/blob/master/03-transactions.md) for more +//! information). //! //! [BOLT #9]: https://github.com/lightning/bolts/blob/master/09-features.md //! [messages]: crate::ln::msgs @@ -393,6 +401,7 @@ mod sealed { define_feature!(55, Keysend, [NodeContext], "Feature flags for keysend payments.", set_keysend_optional, set_keysend_required, supports_keysend, requires_keysend); + // Note: update the module-level docs when a new feature bit is added! #[cfg(test)] define_feature!(123456789, UnknownFeature, From 1b29a55e17b50ab86edef70d4d9e1feb27af839a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 21 Dec 2021 05:23:51 +0000 Subject: [PATCH 03/15] Unset the optional bit for a feature when setting the required bit There is no reason to set both, and this currently makes testing the new BOLT invoice tests slightly harder, so we just unset it. --- lightning/src/ln/features.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 47e1cbcc4cb..cf375603b37 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -275,6 +275,7 @@ mod sealed { } flags[Self::BYTE_OFFSET] |= Self::REQUIRED_MASK; + flags[Self::BYTE_OFFSET] &= !Self::OPTIONAL_MASK; } /// Sets the feature's optional (odd) bit in the given flags. From 928c9b806eb3b2ef7b144d02f730a30d18a19deb Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 21 Dec 2021 05:25:18 +0000 Subject: [PATCH 04/15] Support reading the new `payment_metadata` field in invoices This adds support for reading the new `PaymentMetadata` BOLT11 invoice field, giving us access to the `Vec` storing arbitrary bytes we have to send to the recipient. --- lightning-invoice/src/de.rs | 2 ++ lightning-invoice/src/lib.rs | 12 ++++++++++++ lightning-invoice/src/ser.rs | 3 +++ 3 files changed, 17 insertions(+) diff --git a/lightning-invoice/src/de.rs b/lightning-invoice/src/de.rs index 925d7265c55..01adf67d1af 100644 --- a/lightning-invoice/src/de.rs +++ b/lightning-invoice/src/de.rs @@ -460,6 +460,8 @@ impl FromBase32 for TaggedField { Ok(TaggedField::PrivateRoute(PrivateRoute::from_base32(field_data)?)), constants::TAG_PAYMENT_SECRET => Ok(TaggedField::PaymentSecret(PaymentSecret::from_base32(field_data)?)), + constants::TAG_PAYMENT_METADATA => + Ok(TaggedField::PaymentMetadata(Vec::::from_base32(field_data)?)), constants::TAG_FEATURES => Ok(TaggedField::Features(InvoiceFeatures::from_base32(field_data)?)), _ => { diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 097bdc458ce..e0ba4dda398 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -419,6 +419,7 @@ pub enum TaggedField { Fallback(Fallback), PrivateRoute(PrivateRoute), PaymentSecret(PaymentSecret), + PaymentMetadata(Vec), Features(InvoiceFeatures), } @@ -483,6 +484,7 @@ pub mod constants { pub const TAG_FALLBACK: u8 = 9; pub const TAG_PRIVATE_ROUTE: u8 = 3; pub const TAG_PAYMENT_SECRET: u8 = 16; + pub const TAG_PAYMENT_METADATA: u8 = 27; pub const TAG_FEATURES: u8 = 5; } @@ -954,6 +956,10 @@ impl RawInvoice { find_extract!(self.known_tagged_fields(), TaggedField::PaymentSecret(ref x), x) } + pub fn payment_metadata(&self) -> Option<&Vec> { + find_extract!(self.known_tagged_fields(), TaggedField::PaymentMetadata(ref x), x) + } + pub fn features(&self) -> Option<&InvoiceFeatures> { find_extract!(self.known_tagged_fields(), TaggedField::Features(ref x), x) } @@ -1225,6 +1231,11 @@ impl Invoice { self.signed_invoice.payment_secret().expect("was checked by constructor") } + /// Get the payment metadata blob if one was included in the invoice + pub fn payment_metadata(&self) -> Option<&Vec> { + self.signed_invoice.payment_metadata() + } + /// Get the invoice features if they were included in the invoice pub fn features(&self) -> Option<&InvoiceFeatures> { self.signed_invoice.features() @@ -1374,6 +1385,7 @@ impl TaggedField { TaggedField::Fallback(_) => constants::TAG_FALLBACK, TaggedField::PrivateRoute(_) => constants::TAG_PRIVATE_ROUTE, TaggedField::PaymentSecret(_) => constants::TAG_PAYMENT_SECRET, + TaggedField::PaymentMetadata(_) => constants::TAG_PAYMENT_METADATA, TaggedField::Features(_) => constants::TAG_FEATURES, }; diff --git a/lightning-invoice/src/ser.rs b/lightning-invoice/src/ser.rs index 29a2ca074ed..0dca180cab3 100644 --- a/lightning-invoice/src/ser.rs +++ b/lightning-invoice/src/ser.rs @@ -446,6 +446,9 @@ impl ToBase32 for TaggedField { TaggedField::PaymentSecret(ref payment_secret) => { write_tagged_field(writer, constants::TAG_PAYMENT_SECRET, payment_secret) }, + TaggedField::PaymentMetadata(ref payment_metadata) => { + write_tagged_field(writer, constants::TAG_PAYMENT_METADATA, payment_metadata) + }, TaggedField::Features(ref features) => { write_tagged_field(writer, constants::TAG_FEATURES, features) }, From 8ed6e64913c3559ee897442db61b41ce129a20ec Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 21 Dec 2021 06:03:07 +0000 Subject: [PATCH 05/15] Support setting the new payment metadata field in invoices This adds support for setting the new payment metadata field in BOLT11 invoices, using a new type flag on the builder to enforce transition correctness. We allow users to set the payment metadata as either optional or required, defaulting to optional so that invoice parsing does not fail if the sender does not support payment metadata fields. --- lightning-invoice/src/lib.rs | 106 ++++++++++++++++++++++-------- lightning-invoice/tests/ser_de.rs | 50 ++++++++++++++ 2 files changed, 130 insertions(+), 26 deletions(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index e0ba4dda398..8177e835a1e 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -218,10 +218,13 @@ pub const DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA: u64 = 18; /// * `D`: exactly one [`TaggedField::Description`] or [`TaggedField::DescriptionHash`] /// * `H`: exactly one [`TaggedField::PaymentHash`] /// * `T`: the timestamp is set +/// * `C`: the CLTV expiry is set +/// * `S`: the payment secret is set +/// * `M`: payment metadata is set /// /// This is not exported to bindings users as we likely need to manually select one set of boolean type parameters. #[derive(Eq, PartialEq, Debug, Clone)] -pub struct InvoiceBuilder { +pub struct InvoiceBuilder { currency: Currency, amount: Option, si_prefix: Option, @@ -234,6 +237,7 @@ pub struct InvoiceBuilder, phantom_c: core::marker::PhantomData, phantom_s: core::marker::PhantomData, + phantom_m: core::marker::PhantomData, } /// Represents a syntactically and semantically correct lightning BOLT11 invoice. @@ -488,7 +492,7 @@ pub mod constants { pub const TAG_FEATURES: u8 = 5; } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Construct new, empty `InvoiceBuilder`. All necessary fields have to be filled first before /// `InvoiceBuilder::build(self)` becomes available. pub fn new(currrency: Currency) -> Self { @@ -505,14 +509,15 @@ impl InvoiceBuilder { phantom_t: core::marker::PhantomData, phantom_c: core::marker::PhantomData, phantom_s: core::marker::PhantomData, + phantom_m: core::marker::PhantomData, } } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Helper function to set the completeness flags. - fn set_flags(self) -> InvoiceBuilder { - InvoiceBuilder:: { + fn set_flags(self) -> InvoiceBuilder { + InvoiceBuilder:: { currency: self.currency, amount: self.amount, si_prefix: self.si_prefix, @@ -525,6 +530,7 @@ impl InvoiceBui phantom_t: core::marker::PhantomData, phantom_c: core::marker::PhantomData, phantom_s: core::marker::PhantomData, + phantom_m: core::marker::PhantomData, } } @@ -569,7 +575,7 @@ impl InvoiceBui } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Builds a [`RawInvoice`] if no [`CreationError`] occurred while construction any of the /// fields. pub fn build_raw(self) -> Result { @@ -603,9 +609,9 @@ impl InvoiceBuilder InvoiceBuilder { +impl InvoiceBuilder { /// Set the description. This function is only available if no description (hash) was set. - pub fn description(mut self, description: String) -> InvoiceBuilder { + pub fn description(mut self, description: String) -> InvoiceBuilder { match Description::new(description) { Ok(d) => self.tagged_fields.push(TaggedField::Description(d)), Err(e) => self.error = Some(e), @@ -614,13 +620,13 @@ impl InvoiceBuilder InvoiceBuilder { + pub fn description_hash(mut self, description_hash: sha256::Hash) -> InvoiceBuilder { self.tagged_fields.push(TaggedField::DescriptionHash(Sha256(description_hash))); self.set_flags() } /// Set the description or description hash. This function is only available if no description (hash) was set. - pub fn invoice_description(self, description: InvoiceDescription) -> InvoiceBuilder { + pub fn invoice_description(self, description: InvoiceDescription) -> InvoiceBuilder { match description { InvoiceDescription::Direct(desc) => { self.description(desc.clone().into_inner()) @@ -632,18 +638,18 @@ impl InvoiceBuilder InvoiceBuilder { +impl InvoiceBuilder { /// Set the payment hash. This function is only available if no payment hash was set. - pub fn payment_hash(mut self, hash: sha256::Hash) -> InvoiceBuilder { + pub fn payment_hash(mut self, hash: sha256::Hash) -> InvoiceBuilder { self.tagged_fields.push(TaggedField::PaymentHash(Sha256(hash))); self.set_flags() } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Sets the timestamp to a specific [`SystemTime`]. #[cfg(feature = "std")] - pub fn timestamp(mut self, time: SystemTime) -> InvoiceBuilder { + pub fn timestamp(mut self, time: SystemTime) -> InvoiceBuilder { match PositiveTimestamp::from_system_time(time) { Ok(t) => self.timestamp = Some(t), Err(e) => self.error = Some(e), @@ -654,7 +660,7 @@ impl InvoiceBuilder InvoiceBuilder { + pub fn duration_since_epoch(mut self, time: Duration) -> InvoiceBuilder { match PositiveTimestamp::from_duration_since_epoch(time) { Ok(t) => self.timestamp = Some(t), Err(e) => self.error = Some(e), @@ -665,34 +671,82 @@ impl InvoiceBuilder InvoiceBuilder { + pub fn current_timestamp(mut self) -> InvoiceBuilder { let now = PositiveTimestamp::from_system_time(SystemTime::now()); self.timestamp = Some(now.expect("for the foreseeable future this shouldn't happen")); self.set_flags() } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Sets `min_final_cltv_expiry_delta`. - pub fn min_final_cltv_expiry_delta(mut self, min_final_cltv_expiry_delta: u64) -> InvoiceBuilder { + pub fn min_final_cltv_expiry_delta(mut self, min_final_cltv_expiry_delta: u64) -> InvoiceBuilder { self.tagged_fields.push(TaggedField::MinFinalCltvExpiryDelta(MinFinalCltvExpiryDelta(min_final_cltv_expiry_delta))); self.set_flags() } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Sets the payment secret and relevant features. - pub fn payment_secret(mut self, payment_secret: PaymentSecret) -> InvoiceBuilder { - let mut features = InvoiceFeatures::empty(); - features.set_variable_length_onion_required(); - features.set_payment_secret_required(); + pub fn payment_secret(mut self, payment_secret: PaymentSecret) -> InvoiceBuilder { + let mut found_features = false; + for field in self.tagged_fields.iter_mut() { + if let TaggedField::Features(f) = field { + found_features = true; + f.set_variable_length_onion_required(); + f.set_payment_secret_required(); + } + } self.tagged_fields.push(TaggedField::PaymentSecret(payment_secret)); - self.tagged_fields.push(TaggedField::Features(features)); + if !found_features { + let mut features = InvoiceFeatures::empty(); + features.set_variable_length_onion_required(); + features.set_payment_secret_required(); + self.tagged_fields.push(TaggedField::Features(features)); + } self.set_flags() } } -impl InvoiceBuilder { +impl InvoiceBuilder { + /// Sets the payment metadata. + /// + /// By default features are set to *optionally* allow the sender to include the payment metadata. + /// If you wish to require that the sender include the metadata (and fail to parse the invoice if + /// they don't support payment metadata fields), you need to call + /// [`InvoiceBuilder::require_payment_metadata`] after this. + pub fn payment_metadata(mut self, payment_metadata: Vec) -> InvoiceBuilder { + self.tagged_fields.push(TaggedField::PaymentMetadata(payment_metadata)); + let mut found_features = false; + for field in self.tagged_fields.iter_mut() { + if let TaggedField::Features(f) = field { + found_features = true; + f.set_payment_metadata_optional(); + } + } + if !found_features { + let mut features = InvoiceFeatures::empty(); + features.set_payment_metadata_optional(); + self.tagged_fields.push(TaggedField::Features(features)); + } + self.set_flags() + } +} + +impl InvoiceBuilder { + /// Sets forwarding of payment metadata as required. A reader of the invoice which does not + /// support sending payment metadata will fail to read the invoice. + pub fn require_payment_metadata(mut self) -> InvoiceBuilder { + for field in self.tagged_fields.iter_mut() { + if let TaggedField::Features(f) = field { + f.set_payment_metadata_required(); + } + } + self + } +} + +impl InvoiceBuilder { /// Sets the `basic_mpp` feature as optional. pub fn basic_mpp(mut self) -> Self { for field in self.tagged_fields.iter_mut() { @@ -704,7 +758,7 @@ impl InvoiceBuilder { +impl InvoiceBuilder { /// Builds and signs an invoice using the supplied `sign_function`. This function MAY NOT fail /// and MUST produce a recoverable signature valid for the given hash and if applicable also for /// the included payee public key. diff --git a/lightning-invoice/tests/ser_de.rs b/lightning-invoice/tests/ser_de.rs index 73e2ea8384a..ef5a4d32b30 100644 --- a/lightning-invoice/tests/ser_de.rs +++ b/lightning-invoice/tests/ser_de.rs @@ -332,6 +332,56 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, bool, bool)> { true, // Different features than set in InvoiceBuilder true, // Some unknown fields ), + ( // Older version of the payment metadata test with a payment_pubkey set + "lnbc10m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdp9wpshjmt9de6zqmt9w3skgct5vysxjmnnd9jx2mq8q8a04uqnp4q0n326hr8v9zprg8gsvezcch06gfaqqhde2aj730yg0durunfhv66sp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q2gqqqqqqsgqy9gw6ymamd20jumvdgpfphkhp8fzhhdhycw36egcmla5vlrtrmhs9t7psfy3hkkdqzm9eq64fjg558znccds5nhsfmxveha5xe0dykgpspdha0".to_owned(), + InvoiceBuilder::new(Currency::Bitcoin) + .amount_milli_satoshis(1_000_000_000) + .duration_since_epoch(Duration::from_secs(1496314658)) + .payment_hash(sha256::Hash::from_hex( + "0001020304050607080900010203040506070809000102030405060708090102" + ).unwrap()) + .description("payment metadata inside".to_owned()) + .payment_metadata(hex::decode("01fafaf0").unwrap()) + .require_payment_metadata() + .payee_pub_key(PublicKey::from_slice(&hex::decode( + "03e7156ae33b0a208d0744199163177e909e80176e55d97a2f221ede0f934dd9ad" + ).unwrap()).unwrap()) + .payment_secret(PaymentSecret([0x11; 32])) + .build_raw() + .unwrap() + .sign(|_| { + RecoverableSignature::from_compact( + &hex::decode("2150ed137ddb54f9736c6a0290ded709d22bddb7261d1d6518dffb467c6b1eef02afc182491bdacd00b65c83554c914a1c53c61b0a4ef04eccccdfb4365ed259").unwrap(), + RecoveryId::from_i32(1).unwrap() + ) + }).unwrap(), + false, // Different features than set in InvoiceBuilder + true, // Some unknown fields + ), + ( + "lnbc10m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdp9wpshjmt9de6zqmt9w3skgct5vysxjmnnd9jx2mq8q8a04uqsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q2gqqqqqqsgq7hf8he7ecf7n4ffphs6awl9t6676rrclv9ckg3d3ncn7fct63p6s365duk5wrk202cfy3aj5xnnp5gs3vrdvruverwwq7yzhkf5a3xqpd05wjc".to_owned(), + InvoiceBuilder::new(Currency::Bitcoin) + .amount_milli_satoshis(1_000_000_000) + .duration_since_epoch(Duration::from_secs(1496314658)) + .payment_hash(sha256::Hash::from_hex( + "0001020304050607080900010203040506070809000102030405060708090102" + ).unwrap()) + .description("payment metadata inside".to_owned()) + .payment_metadata(hex::decode("01fafaf0").unwrap()) + .require_payment_metadata() + .payment_secret(PaymentSecret([0x11; 32])) + .build_raw() + .unwrap() + .sign(|_| { + RecoverableSignature::from_compact( + &hex::decode("f5d27be7d9c27d3aa521bc35d77cabd6bda18f1f61716445b19e27e4e17a887508ea8de5a8e1d94f561248f65434e61a221160dac1f1991b9c0f1057b269d898").unwrap(), + RecoveryId::from_i32(1).unwrap() + ) + }).unwrap(), + false, // Different features than set in InvoiceBuilder + true, // Some unknown fields + ), + ] } From a90a35bcbb10b7e53340fba85222867499e84a0f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 24 Mar 2023 17:07:07 +0000 Subject: [PATCH 06/15] Deserialize payment metadata fields in the onion final hop data --- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/ln/msgs.rs | 15 ++++++++++++--- lightning/src/ln/onion_utils.rs | 1 + 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 28b279aac95..7590d6f2f22 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2163,7 +2163,7 @@ where msg: "Got non final data with an HMAC of 0", }); }, - msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage } => { + msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage, .. } => { // TODO: expose the payment_metadata to the user if payment_data.is_some() && keysend_preimage.is_some() { return Err(ReceiveError { err_code: 0x4000|22, diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 2557454b462..c5177828f49 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -42,7 +42,7 @@ use crate::io_extras::read_to_end; use crate::events::{MessageSendEventsProvider, OnionMessageProvider}; use crate::util::logger; -use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname}; +use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, WithoutLength, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname}; use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret}; @@ -1168,6 +1168,7 @@ mod fuzzy_internal_msgs { }, FinalNode { payment_data: Option, + payment_metadata: Option>, keysend_preimage: Option, }, } @@ -1661,11 +1662,12 @@ impl Writeable for OnionHopData { (6, short_channel_id, required) }); }, - OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => { + OnionHopDataFormat::FinalNode { ref payment_data, ref payment_metadata, ref keysend_preimage } => { _encode_varint_length_prefixed_tlv!(w, { (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required), (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required), (8, payment_data, option), + (16, payment_metadata.as_ref().map(|m| WithoutLength(m)), option), (5482373484, keysend_preimage, option) }); }, @@ -1680,29 +1682,33 @@ impl Readable for OnionHopData { let mut cltv_value = HighZeroBytesDroppedBigSize(0u32); let mut short_id: Option = None; let mut payment_data: Option = None; + let mut payment_metadata: Option>> = None; let mut keysend_preimage: Option = None; read_tlv_fields!(r, { (2, amt, required), (4, cltv_value, required), (6, short_id, option), (8, payment_data, option), + (16, payment_metadata, option), // See https://github.com/lightning/blips/blob/master/blip-0003.md (5482373484, keysend_preimage, option) }); let format = if let Some(short_channel_id) = short_id { if payment_data.is_some() { return Err(DecodeError::InvalidValue); } + if payment_metadata.is_some() { return Err(DecodeError::InvalidValue); } OnionHopDataFormat::NonFinalNode { short_channel_id, } } else { - if let &Some(ref data) = &payment_data { + if let Some(data) = &payment_data { if data.total_msat > MAX_VALUE_MSAT { return Err(DecodeError::InvalidValue); } } OnionHopDataFormat::FinalNode { payment_data, + payment_metadata: payment_metadata.map(|w| w.0), keysend_preimage, } }; @@ -2880,6 +2886,7 @@ mod tests { let mut msg = msgs::OnionHopData { format: OnionHopDataFormat::FinalNode { payment_data: None, + payment_metadata: None, keysend_preimage: None, }, amt_to_forward: 0x0badf00d01020304, @@ -2903,6 +2910,7 @@ mod tests { payment_secret: expected_payment_secret, total_msat: 0x1badca1f }), + payment_metadata: None, keysend_preimage: None, }, amt_to_forward: 0x0badf00d01020304, @@ -2917,6 +2925,7 @@ mod tests { payment_secret, total_msat: 0x1badca1f }), + payment_metadata: None, keysend_preimage: None, } = msg.format { assert_eq!(payment_secret, expected_payment_secret); diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index 2c20fb17734..a751cb092da 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -170,6 +170,7 @@ pub(super) fn build_onion_payloads(path: &Vec, total_msat: u64, mut re total_msat, }) } else { None }, + payment_metadata: None, keysend_preimage: *keysend_preimage, } } else { From ee9afd315d22151e314aff2ca826561569ac4d03 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 24 Mar 2023 01:31:14 +0000 Subject: [PATCH 07/15] Add a `payment_metadata` field to `RecipientOnionFields` This adds the new `payment_metadata` to `RecipientOnionFields`, passing the metadata from BOLT11 invoices through the send pipeline and finally copying them info the onion when sending HTLCs. This completes send-side support for the new payment metadata feature. --- lightning-invoice/src/payment.rs | 6 ++++-- lightning/src/ln/channelmanager.rs | 1 + lightning/src/ln/onion_utils.rs | 2 +- lightning/src/ln/outbound_payment.rs | 23 ++++++++++++++++++++--- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index 00b17db7899..ce0d37ee399 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -145,8 +145,10 @@ fn pay_invoice_using_amount( payer: P ) -> Result<(), PaymentError> where P::Target: Payer { let payment_hash = PaymentHash((*invoice.payment_hash()).into_inner()); - let payment_secret = Some(*invoice.payment_secret()); - let recipient_onion = RecipientOnionFields { payment_secret }; + let recipient_onion = RecipientOnionFields { + payment_secret: Some(*invoice.payment_secret()), + payment_metadata: invoice.payment_metadata().map(|v| v.clone()), + }; let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(), invoice.min_final_cltv_expiry_delta() as u32) .with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs()) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7590d6f2f22..4ca93418582 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7603,6 +7603,7 @@ where session_privs: [session_priv_bytes].iter().map(|a| *a).collect(), payment_hash: htlc.payment_hash, payment_secret: None, // only used for retries, and we'll never retry on startup + payment_metadata: None, // only used for retries, and we'll never retry on startup keysend_preimage: None, // only used for retries, and we'll never retry on startup pending_amt_msat: path_amt, pending_fee_msat: Some(path_fee), diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index a751cb092da..b9f36bbd8da 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -170,7 +170,7 @@ pub(super) fn build_onion_payloads(path: &Vec, total_msat: u64, mut re total_msat, }) } else { None }, - payment_metadata: None, + payment_metadata: recipient_onion.payment_metadata.take(), keysend_preimage: *keysend_preimage, } } else { diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 3dde7f24387..738c1d1952f 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -46,6 +46,7 @@ pub(crate) enum PendingOutboundPayment { session_privs: HashSet<[u8; 32]>, payment_hash: PaymentHash, payment_secret: Option, + payment_metadata: Option>, keysend_preimage: Option, pending_amt_msat: u64, /// Used to track the fee paid. Only present if the payment was serialized on 0.0.103+. @@ -422,6 +423,19 @@ pub struct RecipientOnionFields { /// receives, thus you should generally never be providing a secret here for spontaneous /// payments. pub payment_secret: Option, + /// The payment metadata serves a similar purpose as [`Self::payment_secret`] but is of + /// arbitrary length. This gives recipients substantially more flexibility to receive + /// additional data. + /// + /// In LDK, while the [`Self::payment_secret`] is fixed based on an internal authentication + /// scheme to authenticate received payments against expected payments and invoices, this field + /// is not used in LDK for received payments, and can be used to store arbitrary data in + /// invoices which will be received with the payment. + /// + /// Note that this field was added to the lightning specification more recently than + /// [`Self::payment_secret`] and while nearly all lightning senders support secrets, metadata + /// may not be supported as universally. + pub payment_metadata: Option>, } impl RecipientOnionFields { @@ -429,7 +443,7 @@ impl RecipientOnionFields { /// set of onion fields for today's BOLT11 invoices - most nodes require a [`PaymentSecret`] /// but do not require or provide any further data. pub fn secret_only(payment_secret: PaymentSecret) -> Self { - Self { payment_secret: Some(payment_secret) } + Self { payment_secret: Some(payment_secret), payment_metadata: None } } /// Creates a new [`RecipientOnionFields`] with no fields. This generally does not create @@ -438,7 +452,7 @@ impl RecipientOnionFields { /// /// [`ChannelManager::send_spontaneous_payment`]: super::channelmanager::ChannelManager::send_spontaneous_payment pub fn spontaneous_empty() -> Self { - Self { payment_secret: None } + Self { payment_secret: None, payment_metadata: None } } } @@ -719,7 +733,7 @@ impl OutboundPayments { hash_map::Entry::Occupied(mut payment) => { let res = match payment.get() { PendingOutboundPayment::Retryable { - total_msat, keysend_preimage, payment_secret, pending_amt_msat, .. + total_msat, keysend_preimage, payment_secret, payment_metadata, pending_amt_msat, .. } => { let retry_amt_msat: u64 = route.paths.iter().map(|path| path.last().unwrap().fee_msat).sum(); if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 { @@ -729,6 +743,7 @@ impl OutboundPayments { } (*total_msat, RecipientOnionFields { payment_secret: *payment_secret, + payment_metadata: payment_metadata.clone(), }, *keysend_preimage) }, PendingOutboundPayment::Legacy { .. } => { @@ -912,6 +927,7 @@ impl OutboundPayments { pending_fee_msat: Some(0), payment_hash, payment_secret: recipient_onion.payment_secret, + payment_metadata: recipient_onion.payment_metadata, keysend_preimage, starting_block_height: best_block_height, total_msat: route.get_total_amount(), @@ -1345,6 +1361,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, (4, payment_secret, option), (5, keysend_preimage, option), (6, total_msat, required), + (7, payment_metadata, option), (8, pending_amt_msat, required), (10, starting_block_height, required), (not_written, retry_strategy, (static_value, None)), From f57221be60da4ceaa15b7c7b6d63a3d89414ca8f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 7 Apr 2023 20:19:03 +0000 Subject: [PATCH 08/15] Make `claimable_payments` map value a struct, rather than a tuple This makes the `claimable_payments` code more upgradable allowing us to add new fields in the coming commit(s). --- lightning/src/ln/channelmanager.rs | 101 +++++++++++++++++------------ 1 file changed, 61 insertions(+), 40 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 4ca93418582..d51209d7e0e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -470,6 +470,11 @@ impl_writeable_tlv_based!(ClaimingPayment, { (4, receiver_node_id, required), }); +struct ClaimablePayment { + purpose: events::PaymentPurpose, + htlcs: Vec, +} + /// Information about claimable or being-claimed payments struct ClaimablePayments { /// Map from payment hash to the payment data and any HTLCs which are to us and can be @@ -480,7 +485,7 @@ struct ClaimablePayments { /// /// When adding to the map, [`Self::pending_claiming_payments`] must also be checked to ensure /// we don't get a duplicate payment. - claimable_htlcs: HashMap)>, + claimable_payments: HashMap, /// Map from payment hash to the payment data for HTLCs which we have begun claiming, but which /// are waiting on a [`ChannelMonitorUpdate`] to complete in order to be surfaced to the user @@ -1668,7 +1673,7 @@ where pending_inbound_payments: Mutex::new(HashMap::new()), pending_outbound_payments: OutboundPayments::new(), forward_htlcs: Mutex::new(HashMap::new()), - claimable_payments: Mutex::new(ClaimablePayments { claimable_htlcs: HashMap::new(), pending_claiming_payments: HashMap::new() }), + claimable_payments: Mutex::new(ClaimablePayments { claimable_payments: HashMap::new(), pending_claiming_payments: HashMap::new() }), pending_intercepted_htlcs: Mutex::new(HashMap::new()), id_to_peer: Mutex::new(HashMap::new()), short_to_chan_info: FairRwLock::new(HashMap::new()), @@ -3349,8 +3354,13 @@ where fail_htlc!(claimable_htlc, payment_hash); continue } - let (_, ref mut htlcs) = claimable_payments.claimable_htlcs.entry(payment_hash) - .or_insert_with(|| (purpose(), Vec::new())); + let ref mut claimable_payment = claimable_payments.claimable_payments + .entry(payment_hash) + // Note that if we insert here we MUST NOT fail_htlc!() + .or_insert_with(|| ClaimablePayment { + purpose: purpose(), htlcs: Vec::new() + }); + let ref mut htlcs = &mut claimable_payment.htlcs; if htlcs.len() == 1 { if let OnionPayload::Spontaneous(_) = htlcs[0].onion_payload { log_trace!(self.logger, "Failing new HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash", log_bytes!(payment_hash.0)); @@ -3445,13 +3455,16 @@ where fail_htlc!(claimable_htlc, payment_hash); continue } - match claimable_payments.claimable_htlcs.entry(payment_hash) { + match claimable_payments.claimable_payments.entry(payment_hash) { hash_map::Entry::Vacant(e) => { let amount_msat = claimable_htlc.value; claimable_htlc.total_value_received = Some(amount_msat); let claim_deadline = Some(claimable_htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER); let purpose = events::PaymentPurpose::SpontaneousPayment(preimage); - e.insert((purpose.clone(), vec![claimable_htlc])); + e.insert(ClaimablePayment { + purpose: purpose.clone(), + htlcs: vec![claimable_htlc], + }); let prev_channel_id = prev_funding_outpoint.to_channel_id(); new_events.push(events::Event::PaymentClaimable { receiver_node_id: Some(receiver_node_id), @@ -3708,24 +3721,27 @@ where } } - self.claimable_payments.lock().unwrap().claimable_htlcs.retain(|payment_hash, (_, htlcs)| { - if htlcs.is_empty() { + self.claimable_payments.lock().unwrap().claimable_payments.retain(|payment_hash, payment| { + if payment.htlcs.is_empty() { // This should be unreachable debug_assert!(false); return false; } - if let OnionPayload::Invoice { .. } = htlcs[0].onion_payload { + if let OnionPayload::Invoice { .. } = payment.htlcs[0].onion_payload { // Check if we've received all the parts we need for an MPP (the value of the parts adds to total_msat). // In this case we're not going to handle any timeouts of the parts here. // This condition determining whether the MPP is complete here must match // exactly the condition used in `process_pending_htlc_forwards`. - if htlcs[0].total_msat <= htlcs.iter().fold(0, |total, htlc| total + htlc.sender_intended_value) { + if payment.htlcs[0].total_msat <= payment.htlcs.iter() + .fold(0, |total, htlc| total + htlc.sender_intended_value) + { return true; - } else if htlcs.into_iter().any(|htlc| { + } else if payment.htlcs.iter_mut().any(|htlc| { htlc.timer_ticks += 1; return htlc.timer_ticks >= MPP_TIMEOUT_TICKS }) { - timed_out_mpp_htlcs.extend(htlcs.drain(..).map(|htlc: ClaimableHTLC| (htlc.prev_hop, *payment_hash))); + timed_out_mpp_htlcs.extend(payment.htlcs.drain(..) + .map(|htlc: ClaimableHTLC| (htlc.prev_hop, *payment_hash))); return false; } } @@ -3780,9 +3796,9 @@ where pub fn fail_htlc_backwards_with_reason(&self, payment_hash: &PaymentHash, failure_code: FailureCode) { let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); - let removed_source = self.claimable_payments.lock().unwrap().claimable_htlcs.remove(payment_hash); - if let Some((_, mut sources)) = removed_source { - for htlc in sources.drain(..) { + let removed_source = self.claimable_payments.lock().unwrap().claimable_payments.remove(payment_hash); + if let Some(payment) = removed_source { + for htlc in payment.htlcs { let reason = self.get_htlc_fail_reason_from_failure_code(failure_code, &htlc); let source = HTLCSource::PreviousHopData(htlc.prev_hop); let receiver = HTLCDestination::FailedPayment { payment_hash: *payment_hash }; @@ -3959,9 +3975,9 @@ where let mut sources = { let mut claimable_payments = self.claimable_payments.lock().unwrap(); - if let Some((payment_purpose, sources)) = claimable_payments.claimable_htlcs.remove(&payment_hash) { + if let Some(payment) = claimable_payments.claimable_payments.remove(&payment_hash) { let mut receiver_node_id = self.our_network_pubkey; - for htlc in sources.iter() { + for htlc in payment.htlcs.iter() { if htlc.prev_hop.phantom_shared_secret.is_some() { let phantom_pubkey = self.node_signer.get_node_id(Recipient::PhantomNode) .expect("Failed to get node_id for phantom node recipient"); @@ -3971,15 +3987,15 @@ where } let dup_purpose = claimable_payments.pending_claiming_payments.insert(payment_hash, - ClaimingPayment { amount_msat: sources.iter().map(|source| source.value).sum(), - payment_purpose, receiver_node_id, + ClaimingPayment { amount_msat: payment.htlcs.iter().map(|source| source.value).sum(), + payment_purpose: payment.purpose, receiver_node_id, }); if dup_purpose.is_some() { debug_assert!(false, "Shouldn't get a duplicate pending claim event ever"); log_error!(self.logger, "Got a duplicate pending claimable event on payment hash {}! Please report this bug", log_bytes!(payment_hash.0)); } - sources + payment.htlcs } else { return; } }; debug_assert!(!sources.is_empty()); @@ -6091,8 +6107,8 @@ where } if let Some(height) = height_opt { - self.claimable_payments.lock().unwrap().claimable_htlcs.retain(|payment_hash, (_, htlcs)| { - htlcs.retain(|htlc| { + self.claimable_payments.lock().unwrap().claimable_payments.retain(|payment_hash, payment| { + payment.htlcs.retain(|htlc| { // If height is approaching the number of blocks we think it takes us to get // our commitment transaction confirmed before the HTLC expires, plus the // number of blocks we generally consider it to take to do a commitment update, @@ -6107,7 +6123,7 @@ where false } else { true } }); - !htlcs.is_empty() // Only retain this entry if htlcs has at least one entry. + !payment.htlcs.is_empty() // Only retain this entry if htlcs has at least one entry. }); let mut intercepted_htlcs = self.pending_intercepted_htlcs.lock().unwrap(); @@ -7028,14 +7044,14 @@ where let pending_outbound_payments = self.pending_outbound_payments.pending_outbound_payments.lock().unwrap(); let mut htlc_purposes: Vec<&events::PaymentPurpose> = Vec::new(); - (claimable_payments.claimable_htlcs.len() as u64).write(writer)?; - for (payment_hash, (purpose, previous_hops)) in claimable_payments.claimable_htlcs.iter() { + (claimable_payments.claimable_payments.len() as u64).write(writer)?; + for (payment_hash, payment) in claimable_payments.claimable_payments.iter() { payment_hash.write(writer)?; - (previous_hops.len() as u64).write(writer)?; - for htlc in previous_hops.iter() { + (payment.htlcs.len() as u64).write(writer)?; + for htlc in payment.htlcs.iter() { htlc.write(writer)?; } - htlc_purposes.push(purpose); + htlc_purposes.push(&payment.purpose); } let mut monitor_update_blocked_actions_per_peer = None; @@ -7688,22 +7704,25 @@ where let inbound_pmt_key_material = args.node_signer.get_inbound_payment_key_material(); let expanded_inbound_key = inbound_payment::ExpandedKey::new(&inbound_pmt_key_material); - let mut claimable_htlcs = HashMap::with_capacity(claimable_htlcs_list.len()); + let mut claimable_payments = HashMap::with_capacity(claimable_htlcs_list.len()); if let Some(mut purposes) = claimable_htlc_purposes { if purposes.len() != claimable_htlcs_list.len() { return Err(DecodeError::InvalidValue); } - for (purpose, (payment_hash, previous_hops)) in purposes.drain(..).zip(claimable_htlcs_list.drain(..)) { - claimable_htlcs.insert(payment_hash, (purpose, previous_hops)); + for (purpose, (payment_hash, htlcs)) in purposes.drain(..).zip(claimable_htlcs_list.drain(..)) { + let existing_payment = claimable_payments.insert(payment_hash, ClaimablePayment { + purpose, htlcs, + }); + if existing_payment.is_some() { return Err(DecodeError::InvalidValue); } } } else { // LDK versions prior to 0.0.107 did not write a `pending_htlc_purposes`, but do // include a `_legacy_hop_data` in the `OnionPayload`. - for (payment_hash, previous_hops) in claimable_htlcs_list.drain(..) { - if previous_hops.is_empty() { + for (payment_hash, htlcs) in claimable_htlcs_list.drain(..) { + if htlcs.is_empty() { return Err(DecodeError::InvalidValue); } - let purpose = match &previous_hops[0].onion_payload { + let purpose = match &htlcs[0].onion_payload { OnionPayload::Invoice { _legacy_hop_data } => { if let Some(hop_data) = _legacy_hop_data { events::PaymentPurpose::InvoicePayment { @@ -7724,7 +7743,9 @@ where OnionPayload::Spontaneous(payment_preimage) => events::PaymentPurpose::SpontaneousPayment(*payment_preimage), }; - claimable_htlcs.insert(payment_hash, (purpose, previous_hops)); + claimable_payments.insert(payment_hash, ClaimablePayment { + purpose, htlcs, + }); } } @@ -7776,17 +7797,17 @@ where for (_, monitor) in args.channel_monitors.iter() { for (payment_hash, payment_preimage) in monitor.get_stored_preimages() { - if let Some((payment_purpose, claimable_htlcs)) = claimable_htlcs.remove(&payment_hash) { + if let Some(payment) = claimable_payments.remove(&payment_hash) { log_info!(args.logger, "Re-claiming HTLCs with payment hash {} as we've released the preimage to a ChannelMonitor!", log_bytes!(payment_hash.0)); let mut claimable_amt_msat = 0; let mut receiver_node_id = Some(our_network_pubkey); - let phantom_shared_secret = claimable_htlcs[0].prev_hop.phantom_shared_secret; + let phantom_shared_secret = payment.htlcs[0].prev_hop.phantom_shared_secret; if phantom_shared_secret.is_some() { let phantom_pubkey = args.node_signer.get_node_id(Recipient::PhantomNode) .expect("Failed to get node_id for phantom node recipient"); receiver_node_id = Some(phantom_pubkey) } - for claimable_htlc in claimable_htlcs { + for claimable_htlc in payment.htlcs { claimable_amt_msat += claimable_htlc.value; // Add a holding-cell claim of the payment to the Channel, which should be @@ -7820,7 +7841,7 @@ where pending_events_read.push(events::Event::PaymentClaimed { receiver_node_id, payment_hash, - purpose: payment_purpose, + purpose: payment.purpose, amount_msat: claimable_amt_msat, }); } @@ -7851,7 +7872,7 @@ where pending_intercepted_htlcs: Mutex::new(pending_intercepted_htlcs.unwrap()), forward_htlcs: Mutex::new(forward_htlcs), - claimable_payments: Mutex::new(ClaimablePayments { claimable_htlcs, pending_claiming_payments: pending_claiming_payments.unwrap() }), + claimable_payments: Mutex::new(ClaimablePayments { claimable_payments, pending_claiming_payments: pending_claiming_payments.unwrap() }), outbound_scid_aliases: Mutex::new(outbound_scid_aliases), id_to_peer: Mutex::new(id_to_peer), short_to_chan_info: FairRwLock::new(short_to_chan_info), From c1e6a74e0b8309bc85e683824f832ac539806dc5 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 19 Apr 2023 14:51:45 +0000 Subject: [PATCH 09/15] Add a debug_assert the newly-documented (but existing) requirement If we add an entry to `claimable_payments` we have to ensure we actually accept the HTLC we're considering, otherwise we'll end up with an empty `claimable_payments` entry. --- lightning/src/ln/channelmanager.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d51209d7e0e..845a922ee56 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3315,8 +3315,11 @@ where onion_payload, }; + let mut committed_to_claimable = false; + macro_rules! fail_htlc { ($htlc: expr, $payment_hash: expr) => { + debug_assert!(!committed_to_claimable); let mut htlc_msat_height_data = $htlc.value.to_be_bytes().to_vec(); htlc_msat_height_data.extend_from_slice( &self.best_block.read().unwrap().height().to_be_bytes(), @@ -3357,8 +3360,11 @@ where let ref mut claimable_payment = claimable_payments.claimable_payments .entry(payment_hash) // Note that if we insert here we MUST NOT fail_htlc!() - .or_insert_with(|| ClaimablePayment { - purpose: purpose(), htlcs: Vec::new() + .or_insert_with(|| { + committed_to_claimable = true; + ClaimablePayment { + purpose: purpose(), htlcs: Vec::new() + } }); let ref mut htlcs = &mut claimable_payment.htlcs; if htlcs.len() == 1 { @@ -3394,6 +3400,9 @@ where log_bytes!(payment_hash.0)); fail_htlc!(claimable_htlc, payment_hash); } else if total_value >= $payment_data.total_msat { + #[allow(unused_assignments)] { + committed_to_claimable = true; + } let prev_channel_id = prev_funding_outpoint.to_channel_id(); htlcs.push(claimable_htlc); let amount_msat = htlcs.iter().map(|htlc| htlc.value).sum(); @@ -3413,6 +3422,9 @@ where // payment value yet, wait until we receive more // MPP parts. htlcs.push(claimable_htlc); + #[allow(unused_assignments)] { + committed_to_claimable = true; + } } payment_claimable_generated }} From 3dd05ab2619f91f94db4b0de09453019a4064d32 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 7 Apr 2023 20:41:53 +0000 Subject: [PATCH 10/15] `continue` automatically after `fail_htlc` in receiving an HTLC If we receive an HTLC and are processing it a potential MPP part, we always continue in the per-HTLC loop if we call the `fail_htlc` macro, thus its nice to actually do the `continue` therein rather than at the callsites. --- lightning/src/ln/channelmanager.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 845a922ee56..a330380a40f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3276,7 +3276,7 @@ where } } } else { - for forward_info in pending_forwards.drain(..) { + 'next_forwardable_htlc: for forward_info in pending_forwards.drain(..) { match forward_info { HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id, @@ -3334,6 +3334,7 @@ where HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data), HTLCDestination::FailedPayment { payment_hash: $payment_hash }, )); + continue 'next_forwardable_htlc; } } let phantom_shared_secret = claimable_htlc.prev_hop.phantom_shared_secret; @@ -3355,7 +3356,6 @@ where let mut claimable_payments = self.claimable_payments.lock().unwrap(); if claimable_payments.pending_claiming_payments.contains_key(&payment_hash) { fail_htlc!(claimable_htlc, payment_hash); - continue } let ref mut claimable_payment = claimable_payments.claimable_payments .entry(payment_hash) @@ -3371,7 +3371,6 @@ where if let OnionPayload::Spontaneous(_) = htlcs[0].onion_payload { log_trace!(self.logger, "Failing new HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash", log_bytes!(payment_hash.0)); fail_htlc!(claimable_htlc, payment_hash); - continue } } let mut total_value = claimable_htlc.sender_intended_value; @@ -3447,7 +3446,6 @@ where Err(()) => { log_trace!(self.logger, "Failing new HTLC with payment_hash {} as payment verification failed", log_bytes!(payment_hash.0)); fail_htlc!(claimable_htlc, payment_hash); - continue } }; if let Some(min_final_cltv_expiry_delta) = min_final_cltv_expiry_delta { @@ -3456,7 +3454,6 @@ where log_trace!(self.logger, "Failing new HTLC with payment_hash {} as its CLTV expiry was too soon (had {}, earliest expected {})", log_bytes!(payment_hash.0), cltv_expiry, expected_min_expiry_height); fail_htlc!(claimable_htlc, payment_hash); - continue; } } check_total_value!(payment_data, payment_preimage); @@ -3465,7 +3462,6 @@ where let mut claimable_payments = self.claimable_payments.lock().unwrap(); if claimable_payments.pending_claiming_payments.contains_key(&payment_hash) { fail_htlc!(claimable_htlc, payment_hash); - continue } match claimable_payments.claimable_payments.entry(payment_hash) { hash_map::Entry::Vacant(e) => { @@ -3500,7 +3496,6 @@ where if payment_data.is_none() { log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} because we already have an inbound payment with the same payment hash", log_bytes!(payment_hash.0)); fail_htlc!(claimable_htlc, payment_hash); - continue }; let payment_data = payment_data.unwrap(); if inbound_payment.get().payment_secret != payment_data.payment_secret { From 9c55adaa4a15a2eb9eedc65831cdb74ae33e8a94 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 7 Apr 2023 20:43:54 +0000 Subject: [PATCH 11/15] Pipe received `payment_metadata` through the HTLC receipt pipeline When we receive an HTLC, we want to pass the `payment_metadata` through to the `PaymentClaimable` event. This does most of the internal refactoring required to do so - storing a `RecipientOnionFields` in the inbound HTLC tracking structs, including the `payment_metadata`. In the future this struct will allow us to do MPP keysend receipts (as it now stores an Optional `payment_secret` for all inbound payments) as well as custom TLV receipts (as the struct is extensible to store additional fields and the internal API supports filtering for fields which are consistent across HTLCs). --- lightning/src/ln/channelmanager.rs | 68 ++++++++++++++++++++++------ lightning/src/ln/outbound_payment.rs | 19 ++++++++ 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a330380a40f..9f0861ce3c6 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -106,11 +106,13 @@ pub(super) enum PendingHTLCRouting { }, Receive { payment_data: msgs::FinalOnionHopData, + payment_metadata: Option>, incoming_cltv_expiry: u32, // Used to track when we should expire pending HTLCs that go unclaimed phantom_shared_secret: Option<[u8; 32]>, }, ReceiveKeysend { payment_preimage: PaymentPreimage, + payment_metadata: Option>, incoming_cltv_expiry: u32, // Used to track when we should expire pending HTLCs that go unclaimed }, } @@ -472,6 +474,7 @@ impl_writeable_tlv_based!(ClaimingPayment, { struct ClaimablePayment { purpose: events::PaymentPurpose, + onion_fields: Option, htlcs: Vec, } @@ -2168,7 +2171,7 @@ where msg: "Got non final data with an HMAC of 0", }); }, - msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage, .. } => { // TODO: expose the payment_metadata to the user + msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage, payment_metadata } => { if payment_data.is_some() && keysend_preimage.is_some() { return Err(ReceiveError { err_code: 0x4000|22, @@ -2178,6 +2181,7 @@ where } else if let Some(data) = payment_data { PendingHTLCRouting::Receive { payment_data: data, + payment_metadata, incoming_cltv_expiry: hop_data.outgoing_cltv_value, phantom_shared_secret, } @@ -2198,6 +2202,7 @@ where PendingHTLCRouting::ReceiveKeysend { payment_preimage, + payment_metadata, incoming_cltv_expiry: hop_data.outgoing_cltv_value, } } else { @@ -3284,13 +3289,19 @@ where routing, incoming_shared_secret, payment_hash, incoming_amt_msat, outgoing_amt_msat, .. } }) => { - let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret) = match routing { - PendingHTLCRouting::Receive { payment_data, incoming_cltv_expiry, phantom_shared_secret } => { + let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret, mut onion_fields) = match routing { + PendingHTLCRouting::Receive { payment_data, payment_metadata, incoming_cltv_expiry, phantom_shared_secret } => { let _legacy_hop_data = Some(payment_data.clone()); - (incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data }, Some(payment_data), phantom_shared_secret) + let onion_fields = + RecipientOnionFields { payment_secret: Some(payment_data.payment_secret), payment_metadata }; + (incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data }, + Some(payment_data), phantom_shared_secret, onion_fields) + }, + PendingHTLCRouting::ReceiveKeysend { payment_preimage, payment_metadata, incoming_cltv_expiry } => { + let onion_fields = RecipientOnionFields { payment_secret: None, payment_metadata }; + (incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage), + None, None, onion_fields) }, - PendingHTLCRouting::ReceiveKeysend { payment_preimage, incoming_cltv_expiry } => - (incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage), None, None), _ => { panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive"); } @@ -3363,9 +3374,16 @@ where .or_insert_with(|| { committed_to_claimable = true; ClaimablePayment { - purpose: purpose(), htlcs: Vec::new() + purpose: purpose(), htlcs: Vec::new(), onion_fields: None, } }); + if let Some(earlier_fields) = &mut claimable_payment.onion_fields { + if earlier_fields.check_merge(&mut onion_fields).is_err() { + fail_htlc!(claimable_htlc, payment_hash); + } + } else { + claimable_payment.onion_fields = Some(onion_fields); + } let ref mut htlcs = &mut claimable_payment.htlcs; if htlcs.len() == 1 { if let OnionPayload::Spontaneous(_) = htlcs[0].onion_payload { @@ -3471,6 +3489,7 @@ where let purpose = events::PaymentPurpose::SpontaneousPayment(preimage); e.insert(ClaimablePayment { purpose: purpose.clone(), + onion_fields: Some(onion_fields.clone()), htlcs: vec![claimable_htlc], }); let prev_channel_id = prev_funding_outpoint.to_channel_id(); @@ -6715,10 +6734,12 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting, (0, payment_data, required), (1, phantom_shared_secret, option), (2, incoming_cltv_expiry, required), + (3, payment_metadata, option), }, (2, ReceiveKeysend) => { (0, payment_preimage, required), (2, incoming_cltv_expiry, required), + (3, payment_metadata, option), }, ;); @@ -7051,6 +7072,7 @@ where let pending_outbound_payments = self.pending_outbound_payments.pending_outbound_payments.lock().unwrap(); let mut htlc_purposes: Vec<&events::PaymentPurpose> = Vec::new(); + let mut htlc_onion_fields: Vec<&_> = Vec::new(); (claimable_payments.claimable_payments.len() as u64).write(writer)?; for (payment_hash, payment) in claimable_payments.claimable_payments.iter() { payment_hash.write(writer)?; @@ -7059,6 +7081,7 @@ where htlc.write(writer)?; } htlc_purposes.push(&payment.purpose); + htlc_onion_fields.push(&payment.onion_fields); } let mut monitor_update_blocked_actions_per_peer = None; @@ -7173,6 +7196,7 @@ where (7, self.fake_scid_rand_bytes, required), (9, htlc_purposes, vec_type), (11, self.probing_cookie_secret, required), + (13, htlc_onion_fields, optional_vec), }); Ok(()) @@ -7551,6 +7575,7 @@ where let mut fake_scid_rand_bytes: Option<[u8; 32]> = None; let mut probing_cookie_secret: Option<[u8; 32]> = None; let mut claimable_htlc_purposes = None; + let mut claimable_htlc_onion_fields = None; let mut pending_claiming_payments = Some(HashMap::new()); let mut monitor_update_blocked_actions_per_peer = Some(Vec::new()); read_tlv_fields!(reader, { @@ -7563,6 +7588,7 @@ where (7, fake_scid_rand_bytes, option), (9, claimable_htlc_purposes, vec_type), (11, probing_cookie_secret, option), + (13, claimable_htlc_onion_fields, optional_vec), }); if fake_scid_rand_bytes.is_none() { fake_scid_rand_bytes = Some(args.entropy_source.get_secure_random_bytes()); @@ -7712,15 +7738,29 @@ where let expanded_inbound_key = inbound_payment::ExpandedKey::new(&inbound_pmt_key_material); let mut claimable_payments = HashMap::with_capacity(claimable_htlcs_list.len()); - if let Some(mut purposes) = claimable_htlc_purposes { + if let Some(purposes) = claimable_htlc_purposes { if purposes.len() != claimable_htlcs_list.len() { return Err(DecodeError::InvalidValue); } - for (purpose, (payment_hash, htlcs)) in purposes.drain(..).zip(claimable_htlcs_list.drain(..)) { - let existing_payment = claimable_payments.insert(payment_hash, ClaimablePayment { - purpose, htlcs, - }); - if existing_payment.is_some() { return Err(DecodeError::InvalidValue); } + if let Some(onion_fields) = claimable_htlc_onion_fields { + if onion_fields.len() != claimable_htlcs_list.len() { + return Err(DecodeError::InvalidValue); + } + for (purpose, (onion, (payment_hash, htlcs))) in + purposes.into_iter().zip(onion_fields.into_iter().zip(claimable_htlcs_list.into_iter())) + { + let existing_payment = claimable_payments.insert(payment_hash, ClaimablePayment { + purpose, htlcs, onion_fields: onion, + }); + if existing_payment.is_some() { return Err(DecodeError::InvalidValue); } + } + } else { + for (purpose, (payment_hash, htlcs)) in purposes.into_iter().zip(claimable_htlcs_list.into_iter()) { + let existing_payment = claimable_payments.insert(payment_hash, ClaimablePayment { + purpose, htlcs, onion_fields: None, + }); + if existing_payment.is_some() { return Err(DecodeError::InvalidValue); } + } } } else { // LDK versions prior to 0.0.107 did not write a `pending_htlc_purposes`, but do @@ -7751,7 +7791,7 @@ where events::PaymentPurpose::SpontaneousPayment(*payment_preimage), }; claimable_payments.insert(payment_hash, ClaimablePayment { - purpose, htlcs, + purpose, htlcs, onion_fields: None, }); } } diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 738c1d1952f..59efbcfa8c3 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -438,6 +438,11 @@ pub struct RecipientOnionFields { pub payment_metadata: Option>, } +impl_writeable_tlv_based!(RecipientOnionFields, { + (0, payment_secret, option), + (2, payment_metadata, option), +}); + impl RecipientOnionFields { /// Creates a [`RecipientOnionFields`] from only a [`PaymentSecret`]. This is the most common /// set of onion fields for today's BOLT11 invoices - most nodes require a [`PaymentSecret`] @@ -454,6 +459,20 @@ impl RecipientOnionFields { pub fn spontaneous_empty() -> Self { Self { payment_secret: None, payment_metadata: None } } + + /// When we have received some HTLC(s) towards an MPP payment, as we receive further HTLC(s) we + /// have to make sure that some fields match exactly across the parts. For those that aren't + /// required to match, if they don't match we should remove them so as to not expose data + /// that's dependent on the HTLC receive order to users. + /// + /// Here we implement this, first checking compatibility then mutating two objects and then + /// dropping any remaining non-matching fields from both. + pub(super) fn check_merge(&mut self, further_htlc_fields: &mut Self) -> Result<(), ()> { + if self.payment_secret != further_htlc_fields.payment_secret { return Err(()); } + if self.payment_metadata != further_htlc_fields.payment_metadata { return Err(()); } + // For custom TLVs we should just drop non-matching ones, but not reject the payment. + Ok(()) + } } pub(super) struct OutboundPayments { From e1e79031bef6ab2d3be4347677a1cccc81c55fef Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 7 Apr 2023 20:48:01 +0000 Subject: [PATCH 12/15] Expose the `RecipientOnionFields` in `Event::PaymentClaimable` This finally completes the piping of the `payment_metadata` from from the BOLT11 invoice on the sending side all the way through the onion sending + receiving ends to the user on the receive events. --- lightning/src/events/mod.rs | 16 ++++++++++++++-- lightning/src/ln/channelmanager.rs | 2 ++ lightning/src/ln/functional_test_utils.rs | 11 ++++++++--- lightning/src/ln/outbound_payment.rs | 2 +- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index b482996b916..18d2929178f 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -21,7 +21,7 @@ pub mod bump_transaction; pub use bump_transaction::BumpTransactionEvent; use crate::chain::keysinterface::SpendableOutputDescriptor; -use crate::ln::channelmanager::{InterceptId, PaymentId}; +use crate::ln::channelmanager::{InterceptId, PaymentId, RecipientOnionFields}; use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS; use crate::ln::features::ChannelTypeFeatures; use crate::ln::msgs; @@ -342,6 +342,11 @@ pub enum Event { /// The hash for which the preimage should be handed to the ChannelManager. Note that LDK will /// not stop you from registering duplicate payment hashes for inbound payments. payment_hash: PaymentHash, + /// The fields in the onion which were received with each HTLC. Only fields which were + /// identical in each HTLC involved in the payment will be included here. + /// + /// Payments received on LDK versions prior to 0.0.115 will have this field unset. + onion_fields: Option, /// The value, in thousandths of a satoshi, that this payment is for. amount_msat: u64, /// Information for claiming this received payment, based on whether the purpose of the @@ -780,7 +785,10 @@ impl Writeable for Event { // We never write out FundingGenerationReady events as, upon disconnection, peers // drop any channels which have not yet exchanged funding_signed. }, - &Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id, ref claim_deadline } => { + &Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose, + ref receiver_node_id, ref via_channel_id, ref via_user_channel_id, + ref claim_deadline, ref onion_fields + } => { 1u8.write(writer)?; let mut payment_secret = None; let payment_preimage; @@ -803,6 +811,7 @@ impl Writeable for Event { (6, 0u64, required), // user_payment_id required for compatibility with 0.0.103 and earlier (7, claim_deadline, option), (8, payment_preimage, option), + (9, onion_fields, option), }); }, &Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => { @@ -1002,6 +1011,7 @@ impl MaybeReadable for Event { let mut via_channel_id = None; let mut claim_deadline = None; let mut via_user_channel_id = None; + let mut onion_fields = None; read_tlv_fields!(reader, { (0, payment_hash, required), (1, receiver_node_id, option), @@ -1012,6 +1022,7 @@ impl MaybeReadable for Event { (6, _user_payment_id, option), (7, claim_deadline, option), (8, payment_preimage, option), + (9, onion_fields, option), }); let purpose = match payment_secret { Some(secret) => PaymentPurpose::InvoicePayment { @@ -1029,6 +1040,7 @@ impl MaybeReadable for Event { via_channel_id, via_user_channel_id, claim_deadline, + onion_fields, })) }; f() diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9f0861ce3c6..6b3d3dacfb5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3432,6 +3432,7 @@ where via_channel_id: Some(prev_channel_id), via_user_channel_id: Some(prev_user_channel_id), claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER), + onion_fields: claimable_payment.onion_fields.clone(), }); payment_claimable_generated = true; } else { @@ -3501,6 +3502,7 @@ where via_channel_id: Some(prev_channel_id), via_user_channel_id: Some(prev_user_channel_id), claim_deadline, + onion_fields: Some(onion_fields), }); }, hash_map::Entry::Occupied(_) => { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 16a2fd486bd..7421601d953 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1981,21 +1981,26 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p let events_2 = node.node.get_and_clear_pending_events(); if payment_claimable_expected { assert_eq!(events_2.len(), 1); - match events_2[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_id, ref via_user_channel_id, claim_deadline } => { + match &events_2[0] { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, + receiver_node_id, ref via_channel_id, ref via_user_channel_id, + claim_deadline, onion_fields, + } => { assert_eq!(our_payment_hash, *payment_hash); assert_eq!(node.node.get_our_node_id(), receiver_node_id.unwrap()); + assert!(onion_fields.is_some()); match &purpose { PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { assert_eq!(expected_preimage, *payment_preimage); assert_eq!(our_payment_secret.unwrap(), *payment_secret); + assert_eq!(Some(*payment_secret), onion_fields.as_ref().unwrap().payment_secret); }, PaymentPurpose::SpontaneousPayment(payment_preimage) => { assert_eq!(expected_preimage.unwrap(), *payment_preimage); assert!(our_payment_secret.is_none()); }, } - assert_eq!(amount_msat, recv_value); + assert_eq!(*amount_msat, recv_value); assert!(node.node.list_channels().iter().any(|details| details.channel_id == via_channel_id.unwrap())); assert!(node.node.list_channels().iter().any(|details| details.user_channel_id == via_user_channel_id.unwrap())); assert!(claim_deadline.unwrap() > node.best_block_info().1); diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 59efbcfa8c3..345046c2daa 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -409,7 +409,7 @@ pub enum PaymentSendFailure { /// /// This should generally be constructed with data communicated to us from the recipient (via a /// BOLT11 or BOLT12 invoice). -#[derive(Clone)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct RecipientOnionFields { /// The [`PaymentSecret`] is an arbitrary 32 bytes provided by the recipient for us to repeat /// in the onion. It is unrelated to `payment_hash` (or [`PaymentPreimage`]) and exists to From a41d75fb08a870395588df4fc49c03384772eee9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 8 Apr 2023 01:17:51 +0000 Subject: [PATCH 13/15] Add some tests of payment metadata being sent and received --- lightning-invoice/src/payment.rs | 50 +++++++++ lightning/src/ln/channelmanager.rs | 5 + lightning/src/ln/outbound_payment.rs | 12 +++ lightning/src/ln/payment_tests.rs | 148 +++++++++++++++++++++++++++ 4 files changed, 215 insertions(+) diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index ce0d37ee399..11757be2e3a 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -215,6 +215,8 @@ mod tests { use super::*; use crate::{InvoiceBuilder, Currency}; use bitcoin_hashes::sha256::Hash as Sha256; + use lightning::events::Event; + use lightning::ln::msgs::ChannelMessageHandler; use lightning::ln::{PaymentPreimage, PaymentSecret}; use lightning::ln::functional_test_utils::*; use secp256k1::{SecretKey, Secp256k1}; @@ -352,4 +354,52 @@ mod tests { _ => panic!() } } + + #[test] + #[cfg(feature = "std")] + fn payment_metadata_end_to_end() { + // Test that a payment metadata read from an invoice passed to `pay_invoice` makes it all + // the way out through the `PaymentClaimable` event. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + create_announced_chan_between_nodes(&nodes, 0, 1); + + let payment_metadata = vec![42, 43, 44, 45, 46, 47, 48, 49, 42]; + + let (payment_hash, payment_secret) = + nodes[1].node.create_inbound_payment(None, 7200, None).unwrap(); + + let invoice = InvoiceBuilder::new(Currency::Bitcoin) + .description("test".into()) + .payment_hash(Sha256::from_slice(&payment_hash.0).unwrap()) + .payment_secret(payment_secret) + .current_timestamp() + .min_final_cltv_expiry_delta(144) + .amount_milli_satoshis(50_000) + .payment_metadata(payment_metadata.clone()) + .build_signed(|hash| { + Secp256k1::new().sign_ecdsa_recoverable(hash, + &nodes[1].keys_manager.backing.get_node_secret_key()) + }) + .unwrap(); + + pay_invoice(&invoice, Retry::Attempts(0), nodes[0].node).unwrap(); + check_added_monitors(&nodes[0], 1); + let send_event = SendEvent::from_node(&nodes[0]); + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send_event.msgs[0]); + commitment_signed_dance!(nodes[1], nodes[0], &send_event.commitment_msg, false); + + expect_pending_htlcs_forwardable!(nodes[1]); + + let mut events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events.pop().unwrap() { + Event::PaymentClaimable { onion_fields, .. } => { + assert_eq!(Some(payment_metadata), onion_fields.unwrap().payment_metadata); + }, + _ => panic!("Unexpected event") + } + } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6b3d3dacfb5..4bb830f0c6f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2702,6 +2702,11 @@ where self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, recipient_onion, payment_id, route, None, &self.entropy_source, best_block_height) } + #[cfg(test)] + pub(crate) fn test_set_payment_metadata(&self, payment_id: PaymentId, new_payment_metadata: Option>) { + self.pending_outbound_payments.test_set_payment_metadata(payment_id, new_payment_metadata); + } + /// Signals that no further retries for the given payment should occur. Useful if you have a /// pending outbound payment with retries remaining, but wish to stop retrying the payment before diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 345046c2daa..d41c62d1d8a 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -915,6 +915,18 @@ impl OutboundPayments { } } + #[cfg(test)] + pub(super) fn test_set_payment_metadata( + &self, payment_id: PaymentId, new_payment_metadata: Option> + ) { + match self.pending_outbound_payments.lock().unwrap().get_mut(&payment_id).unwrap() { + PendingOutboundPayment::Retryable { payment_metadata, .. } => { + *payment_metadata = new_payment_metadata; + }, + _ => panic!("Need a retryable payment to update metadata on"), + } + } + #[cfg(test)] pub(super) fn test_add_new_pending_payment( &self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId, diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 8fc582745a8..2965f271348 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -3013,3 +3013,151 @@ fn claim_from_closed_chan() { do_claim_from_closed_chan(true); do_claim_from_closed_chan(false); } + +fn do_test_payment_metadata_consistency(do_reload: bool, do_modify: bool) { + // Check that a payment metadata received on one HTLC that doesn't match the one received on + // another results in the HTLC being rejected. + // + // We first set up a diamond shaped network, allowing us to split a payment into two HTLCs, the + // first of which we'll deliver and the second of which we'll fail and then re-send with + // modified payment metadata, which will in turn result in it being failed by the recipient. + let chanmon_cfgs = create_chanmon_cfgs(4); + let node_cfgs = create_node_cfgs(4, &chanmon_cfgs); + let mut config = test_default_channel_config(); + config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 50; + let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, Some(config), Some(config), Some(config)]); + + let persister; + let new_chain_monitor; + let nodes_0_deserialized; + + let mut nodes = create_network(4, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + let chan_id_bd = create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 1_000_000, 0).2; + create_announced_chan_between_nodes_with_value(&nodes, 0, 2, 1_000_000, 0); + let chan_id_cd = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0).2; + + // Pay more than half of each channel's max, requiring MPP + let amt_msat = 750_000_000; + let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[3], Some(amt_msat)); + let payment_id = PaymentId(payment_hash.0); + let payment_metadata = vec![44, 49, 52, 142]; + + let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV) + .with_features(nodes[1].node.invoice_features()); + let mut route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + }; + + // Send the MPP payment, delivering the updated commitment state to nodes[1]. + nodes[0].node.send_payment(payment_hash, RecipientOnionFields { + payment_secret: Some(payment_secret), payment_metadata: Some(payment_metadata), + }, payment_id, route_params.clone(), Retry::Attempts(1)).unwrap(); + check_added_monitors!(nodes[0], 2); + + let mut send_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(send_events.len(), 2); + let first_send = SendEvent::from_event(send_events.pop().unwrap()); + let second_send = SendEvent::from_event(send_events.pop().unwrap()); + + let (b_recv_ev, c_recv_ev) = if first_send.node_id == nodes[1].node.get_our_node_id() { + (&first_send, &second_send) + } else { + (&second_send, &first_send) + }; + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &b_recv_ev.msgs[0]); + commitment_signed_dance!(nodes[1], nodes[0], b_recv_ev.commitment_msg, false, true); + + expect_pending_htlcs_forwardable!(nodes[1]); + check_added_monitors(&nodes[1], 1); + let b_forward_ev = SendEvent::from_node(&nodes[1]); + nodes[3].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &b_forward_ev.msgs[0]); + commitment_signed_dance!(nodes[3], nodes[1], b_forward_ev.commitment_msg, false, true); + + expect_pending_htlcs_forwardable!(nodes[3]); + + // Before delivering the second MPP HTLC to nodes[2], disconnect nodes[2] and nodes[3], which + // will result in nodes[2] failing the HTLC back. + nodes[2].node.peer_disconnected(&nodes[3].node.get_our_node_id()); + nodes[3].node.peer_disconnected(&nodes[2].node.get_our_node_id()); + + nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &c_recv_ev.msgs[0]); + commitment_signed_dance!(nodes[2], nodes[0], c_recv_ev.commitment_msg, false, true); + + let cs_fail = get_htlc_update_msgs(&nodes[2], &nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &cs_fail.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[0], nodes[2], cs_fail.commitment_signed, false, true); + + let payment_fail_retryable_evs = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(payment_fail_retryable_evs.len(), 2); + if let Event::PaymentPathFailed { .. } = payment_fail_retryable_evs[0] {} else { panic!(); } + if let Event::PendingHTLCsForwardable { .. } = payment_fail_retryable_evs[1] {} else { panic!(); } + + // Before we allow the HTLC to be retried, optionally change the payment_metadata we have + // stored for our payment. + if do_modify { + nodes[0].node.test_set_payment_metadata(payment_id, Some(Vec::new())); + } + + // Optionally reload nodes[3] to check that the payment_metadata is properly serialized with + // the payment state. + if do_reload { + let mon_bd = get_monitor!(nodes[3], chan_id_bd).encode(); + let mon_cd = get_monitor!(nodes[3], chan_id_cd).encode(); + reload_node!(nodes[3], config, &nodes[3].node.encode(), &[&mon_bd, &mon_cd], + persister, new_chain_monitor, nodes_0_deserialized); + nodes[1].node.peer_disconnected(&nodes[3].node.get_our_node_id()); + reconnect_nodes(&nodes[1], &nodes[3], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false)); + } + reconnect_nodes(&nodes[2], &nodes[3], (true, true), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false)); + + // Create a new channel between C and D as A will refuse to retry on the existing one because + // it just failed. + let chan_id_cd_2 = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0).2; + + // Now retry the failed HTLC. + nodes[0].node.process_pending_htlc_forwards(); + check_added_monitors(&nodes[0], 1); + let as_resend = SendEvent::from_node(&nodes[0]); + nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &as_resend.msgs[0]); + commitment_signed_dance!(nodes[2], nodes[0], as_resend.commitment_msg, false, true); + + expect_pending_htlcs_forwardable!(nodes[2]); + check_added_monitors(&nodes[2], 1); + let cs_forward = SendEvent::from_node(&nodes[2]); + nodes[3].node.handle_update_add_htlc(&nodes[2].node.get_our_node_id(), &cs_forward.msgs[0]); + commitment_signed_dance!(nodes[3], nodes[2], cs_forward.commitment_msg, false, true); + + // Finally, check that nodes[3] does the correct thing - either accepting the payment or, if + // the payment metadata was modified, failing only the one modified HTLC and retaining the + // other. + if do_modify { + expect_pending_htlcs_forwardable_ignore!(nodes[3]); + nodes[3].node.process_pending_htlc_forwards(); + expect_pending_htlcs_forwardable_conditions(nodes[3].node.get_and_clear_pending_events(), + &[HTLCDestination::FailedPayment {payment_hash}]); + nodes[3].node.process_pending_htlc_forwards(); + + check_added_monitors(&nodes[3], 1); + let ds_fail = get_htlc_update_msgs(&nodes[3], &nodes[2].node.get_our_node_id()); + + nodes[2].node.handle_update_fail_htlc(&nodes[3].node.get_our_node_id(), &ds_fail.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[2], nodes[3], ds_fail.commitment_signed, false, true); + expect_pending_htlcs_forwardable_conditions(nodes[2].node.get_and_clear_pending_events(), + &[HTLCDestination::NextHopChannel { node_id: Some(nodes[3].node.get_our_node_id()), channel_id: chan_id_cd_2 }]); + } else { + expect_pending_htlcs_forwardable!(nodes[3]); + expect_payment_claimable!(nodes[3], payment_hash, payment_secret, amt_msat); + claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage); + } +} + +#[test] +fn test_payment_metadata_consistency() { + do_test_payment_metadata_consistency(true, true); + do_test_payment_metadata_consistency(true, false); + do_test_payment_metadata_consistency(false, true); + do_test_payment_metadata_consistency(false, false); +} From 934a5349f8bf54eb5d9a36f5574d52254fed82e7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 10 Apr 2023 23:24:01 +0000 Subject: [PATCH 14/15] Update and clarify the reasons for HTLCDestination::FailedPayment --- lightning/src/events/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 18d2929178f..4e7919df57b 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -232,8 +232,11 @@ pub enum HTLCDestination { /// /// Some of the reasons may include: /// * HTLC Timeouts - /// * Expected MPP amount has already been reached - /// * Claimable amount does not match expected amount + /// * Excess HTLCs for a payment that we have already fully received, over-paying for the + /// payment, + /// * The counterparty node modified the HTLC in transit, + /// * A probing attack where an intermediary node is trying to detect if we are the ultimate + /// recipient for a payment. FailedPayment { /// The payment hash of the payment we attempted to process. payment_hash: PaymentHash From ef8e3770a96da083b5663c57a09cc5b0efce5df3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 17 Apr 2023 14:54:32 +0000 Subject: [PATCH 15/15] Fix variable name typo --- lightning-invoice/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 8177e835a1e..f1d3d973acf 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -495,9 +495,9 @@ pub mod constants { impl InvoiceBuilder { /// Construct new, empty `InvoiceBuilder`. All necessary fields have to be filled first before /// `InvoiceBuilder::build(self)` becomes available. - pub fn new(currrency: Currency) -> Self { + pub fn new(currency: Currency) -> Self { InvoiceBuilder { - currency: currrency, + currency, amount: None, si_prefix: None, timestamp: None,