From 236342332ea272d47be1043197a3817d5d0482da Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 23 Apr 2021 00:25:55 +0000 Subject: [PATCH 1/4] Do not return a reference to a u64 in rust-lightning-invoices There is generally never a reason to return a non-mutable reference to a u64 vs just copying it, same applies here. It makes the API slightly less consistent, but is easier to map in bindings and just makes more sense. --- lightning-invoice/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 3d75d4d65dc..5a9c094c63b 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -1029,8 +1029,8 @@ impl Invoice { } /// Returns the invoice's `min_cltv_expiry` time if present - pub fn min_final_cltv_expiry(&self) -> Option<&u64> { - self.signed_invoice.min_final_cltv_expiry().map(|x| &x.0) + pub fn min_final_cltv_expiry(&self) -> Option { + self.signed_invoice.min_final_cltv_expiry().map(|x| x.0) } /// Returns a list of all fallback addresses @@ -1591,7 +1591,7 @@ mod test { ); assert_eq!(invoice.payee_pub_key(), Some(&public_key)); assert_eq!(invoice.expiry_time(), Duration::from_secs(54321)); - assert_eq!(invoice.min_final_cltv_expiry(), Some(&144)); + assert_eq!(invoice.min_final_cltv_expiry(), Some(144)); assert_eq!(invoice.fallbacks(), vec![&Fallback::PubKeyHash([0;20])]); assert_eq!(invoice.routes(), vec![&RouteHint(route_1), &RouteHint(route_2)]); assert_eq!( From 3d3147fe836f7a1fa09991f13bdbe556e8222592 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 29 Apr 2021 15:47:08 +0000 Subject: [PATCH 2/4] Rename lightning_invoice::Signature to InvoiceSignature This prevents aliasing the global secp256k1::Signature name in C bindings and also makes it a little more explicit that the object is different from other signature types. --- lightning-invoice/src/de.rs | 16 ++++++++-------- lightning-invoice/src/lib.rs | 18 +++++++++--------- lightning-invoice/src/ser.rs | 2 +- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lightning-invoice/src/de.rs b/lightning-invoice/src/de.rs index c06281e4b4d..52a2201bdfd 100644 --- a/lightning-invoice/src/de.rs +++ b/lightning-invoice/src/de.rs @@ -264,7 +264,7 @@ impl FromStr for SignedRawInvoice { hrp.as_bytes(), &data[..data.len()-104] ), - signature: Signature::from_base32(&data[data.len()-104..])?, + signature: InvoiceSignature::from_base32(&data[data.len()-104..])?, }) } } @@ -338,17 +338,17 @@ impl FromBase32 for PositiveTimestamp { } } -impl FromBase32 for Signature { +impl FromBase32 for InvoiceSignature { type Err = ParseError; fn from_base32(signature: &[u5]) -> Result { if signature.len() != 104 { - return Err(ParseError::InvalidSliceLength("Signature::from_base32()".into())); + return Err(ParseError::InvalidSliceLength("InvoiceSignature::from_base32()".into())); } let recoverable_signature_bytes = Vec::::from_base32(signature)?; let signature = &recoverable_signature_bytes[0..64]; let recovery_id = RecoveryId::from_i32(recoverable_signature_bytes[64] as i32)?; - Ok(Signature(RecoverableSignature::from_compact( + Ok(InvoiceSignature(RecoverableSignature::from_compact( signature, recovery_id )?)) @@ -999,7 +999,7 @@ mod test { use lightning::ln::features::InvoiceFeatures; use secp256k1::recovery::{RecoveryId, RecoverableSignature}; use TaggedField::*; - use {SiPrefix, SignedRawInvoice, Signature, RawInvoice, RawHrp, RawDataPart, + use {SiPrefix, SignedRawInvoice, InvoiceSignature, RawInvoice, RawHrp, RawDataPart, Currency, Sha256, PositiveTimestamp}; // Feature bits 9, 15, and 99 are set. @@ -1025,7 +1025,7 @@ mod test { hash: [0xb1, 0x96, 0x46, 0xc3, 0xbc, 0x56, 0x76, 0x1d, 0x20, 0x65, 0x6e, 0x0e, 0x32, 0xec, 0xd2, 0x69, 0x27, 0xb7, 0x62, 0x6e, 0x2a, 0x8b, 0xe6, 0x97, 0x71, 0x9f, 0xf8, 0x7e, 0x44, 0x54, 0x55, 0xb9], - signature: Signature(RecoverableSignature::from_compact( + signature: InvoiceSignature(RecoverableSignature::from_compact( &[0xd7, 0x90, 0x4c, 0xc4, 0xb7, 0x4a, 0x22, 0x26, 0x9c, 0x68, 0xc1, 0xdf, 0x68, 0xa9, 0x6c, 0x21, 0x4d, 0x65, 0x1b, 0x93, 0x76, 0xe9, 0xf1, 0x64, 0xd3, 0x60, 0x4d, 0xa4, 0xb7, 0xde, 0xcc, 0xce, 0x0e, 0x82, 0xaa, 0xab, 0x4c, 0x85, 0xd3, @@ -1045,7 +1045,7 @@ mod test { fn test_raw_signed_invoice_deserialization() { use TaggedField::*; use secp256k1::recovery::{RecoveryId, RecoverableSignature}; - use {SignedRawInvoice, Signature, RawInvoice, RawHrp, RawDataPart, Currency, Sha256, + use {SignedRawInvoice, InvoiceSignature, RawInvoice, RawHrp, RawDataPart, Currency, Sha256, PositiveTimestamp}; assert_eq!( @@ -1078,7 +1078,7 @@ mod test { 0x7b, 0x1d, 0x85, 0x8d, 0xb1, 0xd1, 0xf7, 0xab, 0x71, 0x37, 0xdc, 0xb7, 0x83, 0x5d, 0xb2, 0xec, 0xd5, 0x18, 0xe1, 0xc9 ], - signature: Signature(RecoverableSignature::from_compact( + signature: InvoiceSignature(RecoverableSignature::from_compact( & [ 0x38u8, 0xec, 0x68, 0x91, 0x34, 0x5e, 0x20, 0x41, 0x45, 0xbe, 0x8a, 0x3a, 0x99, 0xde, 0x38, 0xe9, 0x8a, 0x39, 0xd6, 0xa5, 0x69, 0x43, diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 5a9c094c63b..086169ed7cb 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -207,7 +207,7 @@ pub struct SignedRawInvoice { hash: [u8; 32], /// signature of the payment request - signature: Signature, + signature: InvoiceSignature, } /// Represents an syntactically correct Invoice for a payment on the lightning network, @@ -381,7 +381,7 @@ pub enum Fallback { /// Recoverable signature #[derive(Eq, PartialEq, Debug, Clone)] -pub struct Signature(pub RecoverableSignature); +pub struct InvoiceSignature(pub RecoverableSignature); /// Private routing information /// @@ -630,7 +630,7 @@ impl SignedRawInvoice { /// 1. raw invoice /// 2. hash of the raw invoice /// 3. signature - pub fn into_parts(self) -> (RawInvoice, [u8; 32], Signature) { + pub fn into_parts(self) -> (RawInvoice, [u8; 32], InvoiceSignature) { (self.raw_invoice, self.hash, self.signature) } @@ -644,8 +644,8 @@ impl SignedRawInvoice { &self.hash } - /// Signature for the invoice. - pub fn signature(&self) -> &Signature { + /// InvoiceSignature for the invoice. + pub fn signature(&self) -> &InvoiceSignature { &self.signature } @@ -771,7 +771,7 @@ impl RawInvoice { Ok(SignedRawInvoice { raw_invoice: self, hash: raw_hash, - signature: Signature(signature), + signature: InvoiceSignature(signature), }) } @@ -1192,7 +1192,7 @@ impl Deref for RouteHint { } } -impl Deref for Signature { +impl Deref for InvoiceSignature { type Target = RecoverableSignature; fn deref(&self) -> &RecoverableSignature { @@ -1354,7 +1354,7 @@ mod test { use secp256k1::Secp256k1; use secp256k1::recovery::{RecoveryId, RecoverableSignature}; use secp256k1::key::{SecretKey, PublicKey}; - use {SignedRawInvoice, Signature, RawInvoice, RawHrp, RawDataPart, Currency, Sha256, + use {SignedRawInvoice, InvoiceSignature, RawInvoice, RawHrp, RawDataPart, Currency, Sha256, PositiveTimestamp}; let invoice = SignedRawInvoice { @@ -1383,7 +1383,7 @@ mod test { 0x7b, 0x1d, 0x85, 0x8d, 0xb1, 0xd1, 0xf7, 0xab, 0x71, 0x37, 0xdc, 0xb7, 0x83, 0x5d, 0xb2, 0xec, 0xd5, 0x18, 0xe1, 0xc9 ], - signature: Signature(RecoverableSignature::from_compact( + signature: InvoiceSignature(RecoverableSignature::from_compact( & [ 0x38u8, 0xec, 0x68, 0x91, 0x34, 0x5e, 0x20, 0x41, 0x45, 0xbe, 0x8a, 0x3a, 0x99, 0xde, 0x38, 0xe9, 0x8a, 0x39, 0xd6, 0xa5, 0x69, 0x43, diff --git a/lightning-invoice/src/ser.rs b/lightning-invoice/src/ser.rs index fdff7f4f61c..cfc4313b94b 100644 --- a/lightning-invoice/src/ser.rs +++ b/lightning-invoice/src/ser.rs @@ -461,7 +461,7 @@ impl ToBase32 for TaggedField { } } -impl ToBase32 for Signature { +impl ToBase32 for InvoiceSignature { fn write_base32(&self, writer: &mut W) -> Result<(), ::Err> { let mut converter = BytesToBase32::new(writer); let (recovery_id, signature) = self.0.serialize_compact(); From 2484c1afc284f1f5f296489fbd0e59f0813f176d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 16 Apr 2021 22:32:49 +0000 Subject: [PATCH 3/4] Add no-export tags for lightning-invoice where we can't map to C --- lightning-invoice/src/lib.rs | 24 ++++++++++++++++++++++++ lightning/src/ln/features.rs | 1 + 2 files changed, 25 insertions(+) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 086169ed7cb..b33b1b374d8 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -151,6 +151,8 @@ pub fn check_platform() { /// * `D`: exactly one `Description` or `DescriptionHash` /// * `H`: exactly one `PaymentHash` /// * `T`: the timestamp is set +/// +/// (C-not exported) as we likely need to manually select one set of boolean type parameters. #[derive(Eq, PartialEq, Debug, Clone)] pub struct InvoiceBuilder { currency: Currency, @@ -178,6 +180,9 @@ pub struct Invoice { /// Represents the description of an invoice which has to be either a directly included string or /// a hash of a description provided out of band. +/// +/// (C-not exported) As we don't have a good way to map the reference lifetimes making this +/// practically impossible to use safely in languages like C. #[derive(Eq, PartialEq, Debug, Clone)] pub enum InvoiceDescription<'f> { /// Reference to the directly supplied description in the invoice @@ -225,6 +230,8 @@ pub struct RawInvoice { } /// Data of the `RawInvoice` that is encoded in the human readable part +/// +/// (C-not exported) As we don't yet support Option #[derive(Eq, PartialEq, Debug, Clone)] pub struct RawHrp { /// The currency deferred from the 3rd and 4th character of the bech32 transaction @@ -283,6 +290,9 @@ impl SiPrefix { /// Returns all enum variants of `SiPrefix` sorted in descending order of their associated /// multiplier. + /// + /// (C-not exported) As we don't yet support a slice of enums, and also because this function + /// isn't the most critical to expose. pub fn values_desc() -> &'static [SiPrefix] { use SiPrefix::*; static VALUES: [SiPrefix; 4] = [Milli, Micro, Nano, Pico]; @@ -760,6 +770,9 @@ impl RawInvoice { /// Signs the invoice using the supplied `sign_function`. This function MAY fail with an error /// of type `E`. Since the signature of a `SignedRawInvoice` is not required to be valid there /// are no constraints regarding the validity of the produced signature. + /// + /// (C-not exported) As we don't currently support passing function pointers into methods + /// explicitly. pub fn sign(self, sign_method: F) -> Result where F: FnOnce(&Message) -> Result { @@ -776,6 +789,8 @@ impl RawInvoice { } /// Returns an iterator over all tagged fields with known semantics. + /// + /// (C-not exported) As there is not yet a manual mapping for a FilterMap pub fn known_tagged_fields(&self) -> FilterMap, fn(&RawTaggedField) -> Option<&TaggedField>> { @@ -824,6 +839,7 @@ impl RawInvoice { find_extract!(self.known_tagged_fields(), TaggedField::Features(ref x), x) } + /// (C-not exported) as we don't support Vec<&NonOpaqueType> pub fn fallbacks(&self) -> Vec<&Fallback> { self.known_tagged_fields().filter_map(|tf| match tf { &TaggedField::Fallback(ref f) => Some(f), @@ -981,6 +997,8 @@ impl Invoice { } /// Returns an iterator over all tagged fields of this Invoice. + /// + /// (C-not exported) As there is not yet a manual mapping for a FilterMap pub fn tagged_fields(&self) -> FilterMap, fn(&RawTaggedField) -> Option<&TaggedField>> { self.signed_invoice.raw_invoice().known_tagged_fields() @@ -992,6 +1010,8 @@ impl Invoice { } /// Return the description or a hash of it for longer ones + /// + /// (C-not exported) because we don't yet export InvoiceDescription pub fn description(&self) -> InvoiceDescription { if let Some(ref direct) = self.signed_invoice.description() { return InvoiceDescription::Direct(direct); @@ -1034,6 +1054,8 @@ impl Invoice { } /// Returns a list of all fallback addresses + /// + /// (C-not exported) as we don't support Vec<&NonOpaqueType> pub fn fallbacks(&self) -> Vec<&Fallback> { self.signed_invoice.fallbacks() } @@ -1277,6 +1299,8 @@ impl std::error::Error for SemanticError { } /// When signing using a fallible method either an user-supplied `SignError` or a `CreationError` /// may occur. +/// +/// (C-not exported) As we don't support unbounded generics #[derive(Eq, PartialEq, Debug, Clone)] pub enum SignOrCreationError { /// An error occurred during signing diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index dd6ac3307f8..02e403b46d9 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -502,6 +502,7 @@ impl Features { /// Create a Features given a set of flags, in little-endian. This is in reverse byte order from /// most on-the-wire encodings. + /// (C-not exported) as we don't support export across multiple T pub fn from_le_bytes(flags: Vec) -> Features { Features { flags, From c9afea2d16f6cf57c60e85ee7b766f2b2e78bfc7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 29 Apr 2021 16:46:20 +0000 Subject: [PATCH 4/4] Drop redundant generic parameter bounds on ChainMonitor trait impls The ChannelSigner bounds are specified both in `impl<>` and in the `where` clause, which the C bindings generator doesn't like. There is no reason to have them specified twice. --- lightning/src/chain/chainmonitor.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 0f0958ee8c3..5055305d3c5 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -144,7 +144,6 @@ where C::Target: chain::Filter, impl chain::Listen for ChainMonitor where - ChannelSigner: Sign, C::Target: chain::Filter, T::Target: BroadcasterInterface, F::Target: FeeEstimator, @@ -172,7 +171,6 @@ where impl chain::Confirm for ChainMonitor where - ChannelSigner: Sign, C::Target: chain::Filter, T::Target: BroadcasterInterface, F::Target: FeeEstimator,