From 7964b9f745a18c1abeabb11b165bb588ef1474f2 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 19 Dec 2022 22:33:01 -0600 Subject: [PATCH 1/5] Correct documentation about Refund::payer_id The docs incorrectly stated that Refund::payer_id is for signing, where it is only used for identifying a node if Refund::paths is not present. --- lightning/src/offers/refund.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index 4e553cb3e6d..d3798463b44 100644 --- a/lightning/src/offers/refund.rs +++ b/lightning/src/offers/refund.rs @@ -102,8 +102,8 @@ pub struct RefundBuilder { } impl RefundBuilder { - /// Creates a new builder for a refund using the [`Refund::payer_id`] for signing invoices. Use - /// a different pubkey per refund to avoid correlating refunds. + /// Creates a new builder for a refund using the [`Refund::payer_id`] for the public node id to + /// send to if no [`Refund::paths`] are set. Otherwise, it may be a transient pubkey. /// /// Additionally, sets the required [`Refund::description`], [`Refund::metadata`], and /// [`Refund::amount_msats`]. @@ -285,7 +285,10 @@ impl Refund { &self.contents.features } - /// A possibly transient pubkey used to sign the refund. + /// A public node id to send to in the case where there are no [`paths`]. Otherwise, a possibly + /// transient pubkey. + /// + /// [`paths`]: Self::paths pub fn payer_id(&self) -> PublicKey { self.contents.payer_id } From 1a437f41502e2f5c768ac30e3976fba7d8c99571 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 14 Dec 2022 17:51:04 -0600 Subject: [PATCH 2/5] Remove Option from InvoiceRequest::signature Refunds don't have signatures and now use their own abstraction. Therefore, signatures can be required in invoice requests as per the spec. --- lightning/src/offers/invoice_request.rs | 37 +++++++++++++++---------- lightning/src/offers/parse.rs | 2 ++ 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index e3fe112112e..690bc8d0bdb 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -223,11 +223,11 @@ impl<'a> UnsignedInvoiceRequest<'a> { unsigned_tlv_stream.write(&mut bytes).unwrap(); let pubkey = self.invoice_request.payer_id; - let signature = Some(merkle::sign_message(sign, SIGNATURE_TAG, &bytes, pubkey)?); + let signature = merkle::sign_message(sign, SIGNATURE_TAG, &bytes, pubkey)?; // Append the signature TLV record to the bytes. let signature_tlv_stream = SignatureTlvStreamRef { - signature: signature.as_ref(), + signature: Some(&signature), }; signature_tlv_stream.write(&mut bytes).unwrap(); @@ -249,7 +249,7 @@ impl<'a> UnsignedInvoiceRequest<'a> { pub struct InvoiceRequest { pub(super) bytes: Vec, contents: InvoiceRequestContents, - signature: Option, + signature: Signature, } /// The contents of an [`InvoiceRequest`], which may be shared with an `Invoice`. @@ -311,7 +311,7 @@ impl InvoiceRequest { /// Signature of the invoice request using [`payer_id`]. /// /// [`payer_id`]: Self::payer_id - pub fn signature(&self) -> Option { + pub fn signature(&self) -> Signature { self.signature } @@ -320,7 +320,7 @@ impl InvoiceRequest { let (payer_tlv_stream, offer_tlv_stream, invoice_request_tlv_stream) = self.contents.as_tlv_stream(); let signature_tlv_stream = SignatureTlvStreamRef { - signature: self.signature.as_ref(), + signature: Some(&self.signature), }; (payer_tlv_stream, offer_tlv_stream, invoice_request_tlv_stream, signature_tlv_stream) } @@ -421,9 +421,11 @@ impl TryFrom> for InvoiceRequest { (payer_tlv_stream, offer_tlv_stream, invoice_request_tlv_stream) )?; - if let Some(signature) = &signature { - merkle::verify_signature(signature, SIGNATURE_TAG, &bytes, contents.payer_id)?; - } + let signature = match signature { + None => return Err(ParseError::InvalidSemantics(SemanticError::MissingSignature)), + Some(signature) => signature, + }; + merkle::verify_signature(&signature, SIGNATURE_TAG, &bytes, contents.payer_id)?; Ok(InvoiceRequest { bytes, contents, signature }) } @@ -471,7 +473,7 @@ impl TryFrom for InvoiceRequestContents { #[cfg(test)] mod tests { - use super::{InvoiceRequest, InvoiceRequestTlvStreamRef}; + use super::{InvoiceRequest, InvoiceRequestTlvStreamRef, SIGNATURE_TAG}; use bitcoin::blockdata::constants::ChainHash; use bitcoin::network::constants::Network; @@ -483,7 +485,7 @@ mod tests { use core::time::Duration; use crate::ln::features::InvoiceRequestFeatures; use crate::ln::msgs::{DecodeError, MAX_VALUE_MSAT}; - use crate::offers::merkle::{SignError, SignatureTlvStreamRef}; + use crate::offers::merkle::{SignError, SignatureTlvStreamRef, self}; use crate::offers::offer::{Amount, OfferBuilder, OfferTlvStreamRef, Quantity}; use crate::offers::parse::{ParseError, SemanticError}; use crate::offers::payer::PayerTlvStreamRef; @@ -536,7 +538,11 @@ mod tests { assert_eq!(invoice_request.quantity(), None); assert_eq!(invoice_request.payer_id(), payer_pubkey()); assert_eq!(invoice_request.payer_note(), None); - assert!(invoice_request.signature().is_some()); + assert!( + merkle::verify_signature( + &invoice_request.signature, SIGNATURE_TAG, &invoice_request.bytes, payer_pubkey() + ).is_ok() + ); assert_eq!( invoice_request.as_tlv_stream(), @@ -563,7 +569,7 @@ mod tests { payer_id: Some(&payer_pubkey()), payer_note: None, }, - SignatureTlvStreamRef { signature: invoice_request.signature().as_ref() }, + SignatureTlvStreamRef { signature: Some(&invoice_request.signature()) }, ), ); @@ -1222,7 +1228,7 @@ mod tests { } #[test] - fn parses_invoice_request_without_signature() { + fn fails_parsing_invoice_request_without_signature() { let mut buffer = Vec::new(); OfferBuilder::new("foo".into(), recipient_pubkey()) .amount_msats(1000) @@ -1232,8 +1238,9 @@ mod tests { .invoice_request .write(&mut buffer).unwrap(); - if let Err(e) = InvoiceRequest::try_from(buffer) { - panic!("error parsing invoice_request: {:?}", e); + match InvoiceRequest::try_from(buffer) { + Ok(_) => panic!("expected error"), + Err(e) => assert_eq!(e, ParseError::InvalidSemantics(SemanticError::MissingSignature)), } } diff --git a/lightning/src/offers/parse.rs b/lightning/src/offers/parse.rs index b462e686910..deada66b05c 100644 --- a/lightning/src/offers/parse.rs +++ b/lightning/src/offers/parse.rs @@ -159,6 +159,8 @@ pub enum SemanticError { MissingPayerMetadata, /// A payer id was expected but was missing. MissingPayerId, + /// A signature was expected but was missing. + MissingSignature, } impl From for ParseError { From ea1a68c6e6098e8d330792204b05c20d705a372c Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 16 Dec 2022 13:35:50 -0600 Subject: [PATCH 3/5] Use explicit WithoutLength for BOLT 12 features Most BOLT 12 features are used as the value of a TLV record and thus don't use an explicit length. One exception is the features inside the blinded payinfo subtype since the TLV record contains a list of them. However, these features are also used in the BOLT 4 encrypted_data_tlv TLV stream as a single record, where the length is implicit. Implement Readable and Writeable for Features wrapped in WithoutLength such that either serialization can be used where required. --- lightning/src/ln/features.rs | 42 ++++++++++++++++++++----- lightning/src/offers/invoice_request.rs | 2 +- lightning/src/offers/offer.rs | 2 +- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 1f455471a9f..9d34433161a 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -65,7 +65,7 @@ use core::marker::PhantomData; use bitcoin::bech32; use bitcoin::bech32::{Base32Len, FromBase32, ToBase32, u5, WriteBase32}; use crate::ln::msgs::DecodeError; -use crate::util::ser::{Readable, Writeable, Writer}; +use crate::util::ser::{Readable, WithoutLength, Writeable, Writer}; mod sealed { use crate::prelude::*; @@ -725,26 +725,40 @@ macro_rules! impl_feature_tlv_write { ($features: ident) => { impl Writeable for $features { fn write(&self, w: &mut W) -> Result<(), io::Error> { - self.write_be(w) + WithoutLength(self).write(w) } } impl Readable for $features { fn read(r: &mut R) -> Result { - let v = io_extras::read_to_end(r)?; - Ok(Self::from_be_bytes(v)) + Ok(WithoutLength::::read(r)?.0) } } } } impl_feature_tlv_write!(ChannelTypeFeatures); -impl_feature_tlv_write!(OfferFeatures); -impl_feature_tlv_write!(InvoiceRequestFeatures); + +// Some features may appear both in a TLV record and as part of a TLV subtype sequence. The latter +// requires a length but the former does not. + +impl Writeable for WithoutLength<&Features> { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + self.0.write_be(w) + } +} + +impl Readable for WithoutLength> { + fn read(r: &mut R) -> Result { + let v = io_extras::read_to_end(r)?; + Ok(WithoutLength(Features::::from_be_bytes(v))) + } +} #[cfg(test)] mod tests { - use super::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, InvoiceFeatures, NodeFeatures, sealed}; + use super::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, InvoiceFeatures, NodeFeatures, OfferFeatures, sealed}; use bitcoin::bech32::{Base32Len, FromBase32, ToBase32, u5}; + use crate::util::ser::{Readable, WithoutLength, Writeable}; #[test] fn sanity_test_unknown_bits() { @@ -838,6 +852,20 @@ mod tests { assert!(features.supports_payment_secret()); } + #[test] + fn encodes_features_without_length() { + let features = OfferFeatures::from_le_bytes(vec![1, 2, 3, 4, 5, 42, 100, 101]); + assert_eq!(features.flags.len(), 8); + + let mut serialized_features = Vec::new(); + WithoutLength(&features).write(&mut serialized_features).unwrap(); + assert_eq!(serialized_features.len(), 8); + + let deserialized_features = + WithoutLength::::read(&mut &serialized_features[..]).unwrap().0; + assert_eq!(features, deserialized_features); + } + #[test] fn invoice_features_encoding() { let features_as_u5s = vec![ diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 690bc8d0bdb..fd5ecda558a 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -371,7 +371,7 @@ impl Writeable for InvoiceRequestContents { tlv_stream!(InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef, 80..160, { (80, chain: ChainHash), (82, amount: (u64, HighZeroBytesDroppedBigSize)), - (84, features: InvoiceRequestFeatures), + (84, features: (InvoiceRequestFeatures, WithoutLength)), (86, quantity: (u64, HighZeroBytesDroppedBigSize)), (88, payer_id: PublicKey), (89, payer_note: (String, WithoutLength)), diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index 6451d9431a1..e6550044600 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -567,7 +567,7 @@ tlv_stream!(OfferTlvStream, OfferTlvStreamRef, 1..80, { (6, currency: CurrencyCode), (8, amount: (u64, HighZeroBytesDroppedBigSize)), (10, description: (String, WithoutLength)), - (12, features: OfferFeatures), + (12, features: (OfferFeatures, WithoutLength)), (14, absolute_expiry: (u64, HighZeroBytesDroppedBigSize)), (16, paths: (Vec, WithoutLength)), (18, issuer: (String, WithoutLength)), From d985ced7e31bde8273a934c2b620604ee4fb2111 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 14 Dec 2022 21:18:13 -0600 Subject: [PATCH 4/5] Define BOLT 12 invoice features with MPP support --- lightning/src/ln/features.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 9d34433161a..4cfe5d8d57d 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -159,6 +159,14 @@ mod sealed { ]); define_context!(OfferContext, []); define_context!(InvoiceRequestContext, []); + define_context!(Bolt12InvoiceContext, [ + // Byte 0 + , + // Byte 1 + , + // Byte 2 + BasicMPP, + ]); // This isn't a "real" feature context, and is only used in the channel_type field in an // `OpenChannel` message. define_context!(ChannelTypeContext, [ @@ -342,7 +350,7 @@ mod sealed { define_feature!(15, PaymentSecret, [InitContext, NodeContext, InvoiceContext], "Feature flags for `payment_secret`.", set_payment_secret_optional, set_payment_secret_required, supports_payment_secret, requires_payment_secret); - define_feature!(17, BasicMPP, [InitContext, NodeContext, InvoiceContext], + define_feature!(17, BasicMPP, [InitContext, NodeContext, InvoiceContext, Bolt12InvoiceContext], "Feature flags for `basic_mpp`.", set_basic_mpp_optional, set_basic_mpp_required, supports_basic_mpp, requires_basic_mpp); define_feature!(19, Wumbo, [InitContext, NodeContext], @@ -369,7 +377,7 @@ mod sealed { #[cfg(test)] define_feature!(123456789, UnknownFeature, - [NodeContext, ChannelContext, InvoiceContext, OfferContext, InvoiceRequestContext], + [NodeContext, ChannelContext, InvoiceContext, OfferContext, InvoiceRequestContext, Bolt12InvoiceContext], "Feature flags for an unknown feature used in testing.", set_unknown_feature_optional, set_unknown_feature_required, supports_unknown_test_feature, requires_unknown_test_feature); } @@ -432,6 +440,8 @@ pub type InvoiceFeatures = Features; pub type OfferFeatures = Features; /// Features used within an `invoice_request`. pub type InvoiceRequestFeatures = Features; +/// Features used within an `invoice`. +pub type Bolt12InvoiceFeatures = Features; /// Features used within the channel_type field in an OpenChannel message. /// From b50fc4e32c8b19566fe699cb7cccfe3887928fa3 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 16 Dec 2022 14:06:33 -0600 Subject: [PATCH 5/5] Define blinded hop features for use in BOLT 12 BOLT 12 invoices may contain blinded_payinfo for each hop in a blinded path. Each blinded_payinfo contains features, whose length must be encoded since there may be multiple hops. Note these features are also needed in the BOLT 4 encrypted_data_tlv stream. But since they are a single TLV record, the length must *not* be encoded there. --- lightning/src/ln/features.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 4cfe5d8d57d..a3c1b7f623d 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -167,6 +167,7 @@ mod sealed { // Byte 2 BasicMPP, ]); + define_context!(BlindedHopContext, []); // This isn't a "real" feature context, and is only used in the channel_type field in an // `OpenChannel` message. define_context!(ChannelTypeContext, [ @@ -377,7 +378,7 @@ mod sealed { #[cfg(test)] define_feature!(123456789, UnknownFeature, - [NodeContext, ChannelContext, InvoiceContext, OfferContext, InvoiceRequestContext, Bolt12InvoiceContext], + [NodeContext, ChannelContext, InvoiceContext, OfferContext, InvoiceRequestContext, Bolt12InvoiceContext, BlindedHopContext], "Feature flags for an unknown feature used in testing.", set_unknown_feature_optional, set_unknown_feature_required, supports_unknown_test_feature, requires_unknown_test_feature); } @@ -442,6 +443,8 @@ pub type OfferFeatures = Features; pub type InvoiceRequestFeatures = Features; /// Features used within an `invoice`. pub type Bolt12InvoiceFeatures = Features; +/// Features used within BOLT 4 encrypted_data_tlv and BOLT 12 blinded_payinfo +pub type BlindedHopFeatures = Features; /// Features used within the channel_type field in an OpenChannel message. /// @@ -729,6 +732,7 @@ impl_feature_len_prefixed_write!(InitFeatures); impl_feature_len_prefixed_write!(ChannelFeatures); impl_feature_len_prefixed_write!(NodeFeatures); impl_feature_len_prefixed_write!(InvoiceFeatures); +impl_feature_len_prefixed_write!(BlindedHopFeatures); // Some features only appear inside of TLVs, so they don't have a length prefix when serialized. macro_rules! impl_feature_tlv_write {