From cc72f032dca9a0a203bcc5da2892fc68c46c1420 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 3 Oct 2024 16:53:56 +0000 Subject: [PATCH 1/5] Marginally reduce allocations in `lightning-invoice` In aa2f6b47df312f026213d0ceaaff20ffe955c377 we refactored `lightning-invoice` de/serialization to use the new version of `bech32`, also reducing some trivial unnecessary allocations when we did so. Here we drop a few additional allocations which came up in review. --- lightning-invoice/src/de.rs | 2 +- lightning-invoice/src/ser.rs | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lightning-invoice/src/de.rs b/lightning-invoice/src/de.rs index 7c425441f4e..446fd5c71c0 100644 --- a/lightning-invoice/src/de.rs +++ b/lightning-invoice/src/de.rs @@ -712,7 +712,7 @@ impl FromBase32 for PrivateRoute { return Err(Bolt11ParseError::UnexpectedEndOfTaggedFields); } - let mut route_hops = Vec::::new(); + let mut route_hops = Vec::with_capacity(bytes.len() / 51); let mut bytes = bytes.as_slice(); while !bytes.is_empty() { diff --git a/lightning-invoice/src/ser.rs b/lightning-invoice/src/ser.rs index 4000241f557..3d79a253314 100644 --- a/lightning-invoice/src/ser.rs +++ b/lightning-invoice/src/ser.rs @@ -218,19 +218,20 @@ impl Display for SiPrefix { } /// Encode an integer to base32, big endian, without leading zeros -fn encode_int_be_base32(int: u64) -> Vec { +fn encode_int_be_base32(int: u64) -> impl ExactSizeIterator { let base = 32u64; // (64 + 4) / 5 == 13 - let mut out_vec = Vec::::with_capacity(13); + let mut out = [Fe32::Q; 13]; + let mut out_pos = 0; let mut rem_int = int; while rem_int != 0 { - out_vec.push(Fe32::try_from((rem_int % base) as u8).expect("always <32")); + out[out_pos] = Fe32::try_from((rem_int % base) as u8).expect("always <32"); + out_pos += 1; rem_int /= base; } - out_vec.reverse(); - out_vec + out.into_iter().take(out_pos).rev() } /// The length of the output of `encode_int_be_base32`. @@ -252,7 +253,7 @@ impl Base32Iterable for PositiveTimestamp { let fes = encode_int_be_base32(self.as_unix_timestamp()); debug_assert!(fes.len() <= 7, "Invalid timestamp length"); let to_pad = 7 - fes.len(); - Box::new(core::iter::repeat(Fe32::Q).take(to_pad).chain(fes.into_iter())) + Box::new(core::iter::repeat(Fe32::Q).take(to_pad).chain(fes)) } } @@ -305,7 +306,7 @@ impl Base32Len for PayeePubKey { impl Base32Iterable for ExpiryTime { fn fe_iter<'s>(&'s self) -> Box + 's> { - Box::new(encode_int_be_base32(self.as_seconds()).into_iter()) + Box::new(encode_int_be_base32(self.as_seconds())) } } @@ -317,7 +318,7 @@ impl Base32Len for ExpiryTime { impl Base32Iterable for MinFinalCltvExpiryDelta { fn fe_iter<'s>(&'s self) -> Box + 's> { - Box::new(encode_int_be_base32(self.0).into_iter()) + Box::new(encode_int_be_base32(self.0)) } } @@ -504,6 +505,6 @@ mod test { .map(|v| Fe32::try_from(v).expect("<= 31")) .collect::>(); - assert_eq!(expected_out, encode_int_be_base32(input)); + assert_eq!(expected_out, encode_int_be_base32(input).collect::>()); } } From 0abf068ad1a8021261f915f4cba426f2ef24ed16 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 3 Oct 2024 16:54:03 +0000 Subject: [PATCH 2/5] Drop one unnecessary allocation added in aa2f6b47df312f026213d0ceaa In aa2f6b47df312f026213d0ceaaff20ffe955c377 we refactored `lightning-invoice` de/serialization to use the new version of `bech32`, but ended up adding one unnecessary allocation in our offers logic, which we drop here. --- lightning/src/offers/parse.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/offers/parse.rs b/lightning/src/offers/parse.rs index a46e90de6be..7c9d80387de 100644 --- a/lightning/src/offers/parse.rs +++ b/lightning/src/offers/parse.rs @@ -58,7 +58,7 @@ mod sealed { let parsed = CheckedHrpstring::new::(encoded.as_ref())?; let hrp = parsed.hrp(); - if hrp.to_string() != Self::BECH32_HRP { + if hrp.as_str() != Self::BECH32_HRP { return Err(Bolt12ParseError::InvalidBech32Hrp); } From c5658f6cc2cfa6eee2a45d206b08933d30f1a275 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 3 Oct 2024 16:54:10 +0000 Subject: [PATCH 3/5] Check that the HRPs generated in BOLT 11 `RawHrp` are always valid ...in `debug_assertions`. --- lightning-invoice/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 36370896052..f696e3545f9 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -313,6 +313,7 @@ impl RawHrp { pub fn to_hrp(&self) -> bech32::Hrp { let hrp_str = self.to_string(); let s = core::str::from_utf8(&hrp_str.as_bytes()).expect("HRP bytes should be ASCII"); + debug_assert!(bech32::Hrp::parse(s).is_ok(), "We should always build BIP 173-valid HRPs"); bech32::Hrp::parse_unchecked(s) } } From 052e7c3df06012e0787b65b11d740843e26c3c1a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 3 Oct 2024 16:54:14 +0000 Subject: [PATCH 4/5] Marginally reduce allocations in `lightning-invoice` In aa2f6b47df312f026213d0ceaaff20ffe955c377 we refactored `lightning-invoice` de/serialization to use the new version of `bech32`, but in order to keep the public API the same we introduced one allocation we could have skipped. Instead, here, we replace the public `Utf8Error` with `FromUtf8Error` which contains the original data which failed conversion, removing an allocation in the process. --- lightning-invoice/src/de.rs | 6 +++--- lightning-invoice/src/lib.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lightning-invoice/src/de.rs b/lightning-invoice/src/de.rs index 446fd5c71c0..da43edc5b6b 100644 --- a/lightning-invoice/src/de.rs +++ b/lightning-invoice/src/de.rs @@ -1,3 +1,4 @@ +use alloc::string; #[cfg(feature = "std")] use std::error; #[cfg(not(feature = "std"))] @@ -5,7 +6,6 @@ use core::convert::TryFrom; use core::fmt; use core::fmt::{Display, Formatter}; use core::num::ParseIntError; -use core::str; use core::str::FromStr; use bech32::primitives::decode::{CheckedHrpstring, CheckedHrpstringError}; @@ -613,7 +613,7 @@ impl FromBase32 for Description { fn from_base32(field_data: &[Fe32]) -> Result { let bytes = Vec::::from_base32(field_data)?; - let description = String::from(str::from_utf8(&bytes)?); + let description = String::from_utf8(bytes)?; Ok(Description::new(description).expect( "Max len is 639=floor(1023*5/8) since the len field is only 10bits long" )) @@ -824,7 +824,7 @@ macro_rules! from_error { from_error!(Bolt11ParseError::MalformedSignature, bitcoin::secp256k1::Error); from_error!(Bolt11ParseError::ParseAmountError, ParseIntError); -from_error!(Bolt11ParseError::DescriptionDecodeError, str::Utf8Error); +from_error!(Bolt11ParseError::DescriptionDecodeError, string::FromUtf8Error); impl From for Bolt11ParseError { fn from(e: CheckedHrpstringError) -> Self { diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index f696e3545f9..8baedbe26cd 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -51,7 +51,7 @@ use core::num::ParseIntError; use core::ops::Deref; use core::slice::Iter; use core::time::Duration; -use core::str; +use alloc::string; #[cfg(feature = "serde")] use serde::{Deserialize, Deserializer,Serialize, Serializer, de::Error}; @@ -98,7 +98,7 @@ pub enum Bolt11ParseError { MalformedHRP, TooShortDataPart, UnexpectedEndOfTaggedFields, - DescriptionDecodeError(str::Utf8Error), + DescriptionDecodeError(string::FromUtf8Error), PaddingError, IntegerOverflowError, InvalidSegWitProgramLength, From 5c1440afec0a509c0cb83fc650cabe9655509322 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 3 Oct 2024 16:54:20 +0000 Subject: [PATCH 5/5] Hold a reference to byte arrays when serializing to bech32 When we serialize from a byte array to bech32 in `lightning-invoice`, we can either copy the array itself into the iterator or hold a reference to the array and iterate through that. In aa2f6b47df312f026213d0ceaaff20ffe955c377 we opted to copy the array into the iterator, which is fine for the current array sizes we're working with, but does result in additional memory on the stack if, in the future, we end up writing large arrays. Instead, here, we switch to using the slice serialization code when writing arrays, (very marginally) reducing code size and reducing stack usage. --- lightning-invoice/src/ser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning-invoice/src/ser.rs b/lightning-invoice/src/ser.rs index 3d79a253314..66571ee35f9 100644 --- a/lightning-invoice/src/ser.rs +++ b/lightning-invoice/src/ser.rs @@ -29,7 +29,7 @@ pub(crate) trait Base32Len: Base32Iterable { impl Base32Iterable for [u8; N] { fn fe_iter<'s>(&'s self) -> Box + 's> { - Box::new((*self).into_iter().bytes_to_fes()) + self[..].fe_iter() } }