Drop `Box`ing of iterators during BOLT 11 invoice serialization
What changed, and why it matters
This commit is a routine code cleanup in the Lightning Dev Kit's BOLT 11 invoice serialization code. It replaces heap-allocated boxed iterators with Rust's newer 'impl Trait' return type feature, eliminating unnecessary memory allocations. There is no security-relevant change here.
No security action required. This is a normal refactoring commit.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors the Base32Iterable trait and its implementations in lightning-invoice/src/ser.rs to return impl Iterator<Item = Fe32> + 's instead of Box<dyn Iterator<Item = Fe32> + 's>. It introduces a macro define_iterator_enum! to create small enum-based iterator adapters where multiple concrete iterator types must be unified. The hash_from_parts helper in lib.rs is similarly generic over the iterator type. This is a pure performance/maintainability refactor enabled by a higher Minimum Supported Rust Version (MSRV).
Changed components
lightning-invoice/src/ser.rslightning-invoice/src/lib.rsInspect captured patch +71 / −70
diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs
index 47f9293..60d413c 100644
--- a/lightning-invoice/src/lib.rs
+++ b/lightning-invoice/src/lib.rs
@@ -40,7 +40,6 @@ use bitcoin::secp256k1::ecdsa::RecoverableSignature;
use bitcoin::secp256k1::PublicKey;
use bitcoin::secp256k1::{Message, Secp256k1};
-use alloc::boxed::Box;
use alloc::string;
use core::cmp::Ordering;
use core::fmt::{self, Display, Formatter};
@@ -1081,8 +1080,8 @@ macro_rules! find_all_extract {
#[allow(missing_docs)]
impl RawBolt11Invoice {
/// Hash the HRP (as bytes) and signatureless data part (as Fe32 iterator)
- fn hash_from_parts<'s>(
- hrp_bytes: &[u8], data_without_signature: Box<dyn Iterator<Item = Fe32> + 's>,
+ fn hash_from_parts<'s, I: Iterator<Item = Fe32> + 's>(
+ hrp_bytes: &[u8], data_without_signature: I,
) -> [u8; 32] {
use crate::bech32::Fe32IterExt;
use bitcoin::hashes::HashEngine;
diff --git a/lightning-invoice/src/ser.rs b/lightning-invoice/src/ser.rs
index 5c93fa8..853accd 100644
--- a/lightning-invoice/src/ser.rs
+++ b/lightning-invoice/src/ser.rs
@@ -1,4 +1,3 @@
-use alloc::boxed::Box;
use core::fmt;
use core::fmt::{Display, Formatter};
use core::{array, iter};
@@ -13,14 +12,28 @@ use super::{
SignedRawBolt11Invoice, TaggedField,
};
+macro_rules! define_iterator_enum {
+ ($name: ident, $($n: ident),*) => {
+ enum $name<$($n: Iterator<Item = Fe32>,)*> {
+ $($n($n),)*
+ }
+ impl<$($n: Iterator<Item = Fe32>,)*> Iterator for $name<$($n,)*> {
+ type Item = Fe32;
+ fn next(&mut self) -> Option<Fe32> {
+ match self {
+ $(Self::$n(iter) => iter.next(),)*
+ }
+ }
+ }
+ }
+}
+
/// Objects that can be encoded to base32 (bech32).
///
-/// Private to this crate to avoid polluting the API.
+/// Private to this crate (except in fuzzing) to avoid polluting the API.
pub trait Base32Iterable {
- /// apoelstra: In future we want to replace this Box<dyn Iterator> with an explicit
- /// associated type, to avoid the allocation. But we cannot do this until
- /// Rust 1.65 and GATs since the iterator may contain a reference to self.
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's>;
+ /// Serialize this object, returning an iterator over bech32 field elements.
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's;
}
/// Interface to calculate the length of the base32 representation before actually serializing
@@ -32,7 +45,7 @@ pub(crate) trait Base32Len: Base32Iterable {
// Base32Iterable & Base32Len implementations are here, because the traits are in this module.
impl<const N: usize> Base32Iterable for [u8; N] {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
self[..].fe_iter()
}
}
@@ -45,8 +58,8 @@ impl<const N: usize> Base32Len for [u8; N] {
}
impl Base32Iterable for [u8] {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
- Box::new(self.iter().copied().bytes_to_fes())
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
+ self.iter().copied().bytes_to_fes()
}
}
@@ -58,8 +71,8 @@ impl Base32Len for [u8] {
}
impl Base32Iterable for Vec<u8> {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
- Box::new(self.iter().copied().bytes_to_fes())
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
+ self.iter().copied().bytes_to_fes()
}
}
@@ -71,8 +84,8 @@ impl Base32Len for Vec<u8> {
}
impl Base32Iterable for PaymentSecret {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
- Box::new(self.0[..].fe_iter())
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
+ self.0[..].fe_iter()
}
}
@@ -88,7 +101,7 @@ impl Base32Iterable for Bolt11InvoiceFeatures {
/// starting from the rightmost bit,
/// and taking the resulting 5-bit values in reverse (left-to-right),
/// with the leading 0's skipped.
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
// Fe32 conversion cannot be used, because this packs from right, right-to-left
let mut input_iter = self.le_flags().iter();
// Carry bits, 0..7 bits
@@ -126,7 +139,7 @@ impl Base32Iterable for Bolt11InvoiceFeatures {
output.push(Fe32::try_from(next_out8 & 31u8).expect("<32"))
}
// Take result in reverse order, and skip leading 0s
- Box::new(output.into_iter().rev().skip_while(|e| *e == Fe32::Q))
+ output.into_iter().rev().skip_while(|e| *e == Fe32::Q)
}
}
@@ -241,36 +254,35 @@ fn encoded_int_be_base32_size(int: u64) -> usize {
}
impl Base32Iterable for RawDataPart {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
let ts_iter = self.timestamp.fe_iter();
let fields_iter = self.tagged_fields.iter().map(RawTaggedField::fe_iter).flatten();
- Box::new(ts_iter.chain(fields_iter))
+ ts_iter.chain(fields_iter)
}
}
impl Base32Iterable for PositiveTimestamp {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
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))
+ core::iter::repeat(Fe32::Q).take(to_pad).chain(fes)
}
}
impl Base32Iterable for RawTaggedField {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
- // Annoyingly, when we move to explicit types, we will need an
- // explicit enum holding the two iterator variants.
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
+ define_iterator_enum!(TwoIters, A, B);
match *self {
- RawTaggedField::UnknownSemantics(ref content) => Box::new(content.iter().copied()),
- RawTaggedField::KnownSemantics(ref tagged_field) => tagged_field.fe_iter(),
+ RawTaggedField::UnknownSemantics(ref content) => TwoIters::A(content.iter().copied()),
+ RawTaggedField::KnownSemantics(ref tagged_field) => TwoIters::B(tagged_field.fe_iter()),
}
}
}
impl Base32Iterable for Sha256 {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
- Box::new(self.0[..].fe_iter())
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
+ self.0[..].fe_iter()
}
}
@@ -281,8 +293,8 @@ impl Base32Len for Sha256 {
}
impl Base32Iterable for Description {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
- Box::new(self.0 .0.as_bytes().fe_iter())
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
+ self.0 .0.as_bytes().fe_iter()
}
}
@@ -293,8 +305,8 @@ impl Base32Len for Description {
}
impl Base32Iterable for PayeePubKey {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
- Box::new(self.serialize().into_iter().bytes_to_fes())
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
+ self.serialize().into_iter().bytes_to_fes()
}
}
@@ -305,8 +317,8 @@ impl Base32Len for PayeePubKey {
}
impl Base32Iterable for ExpiryTime {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
- Box::new(encode_int_be_base32(self.as_seconds()))
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
+ encode_int_be_base32(self.as_seconds())
}
}
@@ -317,8 +329,8 @@ impl Base32Len for ExpiryTime {
}
impl Base32Iterable for MinFinalCltvExpiryDelta {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
- Box::new(encode_int_be_base32(self.0))
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
+ encode_int_be_base32(self.0)
}
}
@@ -329,8 +341,8 @@ impl Base32Len for MinFinalCltvExpiryDelta {
}
impl Base32Iterable for Fallback {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
- Box::new(match *self {
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
+ match *self {
Fallback::SegWitProgram { version: v, program: ref p } => {
let v = Fe32::try_from(v.to_num()).expect("valid version");
core::iter::once(v).chain(p[..].fe_iter())
@@ -343,7 +355,7 @@ impl Base32Iterable for Fallback {
// 18 'J'
core::iter::once(Fe32::J).chain(hash[..].fe_iter())
},
- })
+ }
}
}
@@ -371,7 +383,7 @@ type RouteHintHopIter = iter::Chain<
>;
impl Base32Iterable for PrivateRoute {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
fn serialize_to_iter(hop: &RouteHintHop) -> RouteHintHopIter {
let i1 = hop.src_node_id.serialize().into_iter();
let i2 = u64::to_be_bytes(hop.short_channel_id).into_iter();
@@ -381,7 +393,7 @@ impl Base32Iterable for PrivateRoute {
i1.chain(i2).chain(i3).chain(i4).chain(i5)
}
- Box::new(self.0 .0.iter().map(serialize_to_iter).flatten().bytes_to_fes())
+ self.0 .0.iter().map(serialize_to_iter).flatten().bytes_to_fes()
}
}
@@ -391,16 +403,11 @@ impl Base32Len for PrivateRoute {
}
}
-// Shorthand type
-type TaggedFieldIter<I> = core::iter::Chain<core::array::IntoIter<Fe32, 3>, I>;
-
impl Base32Iterable for TaggedField {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
/// Writes a tagged field: tag, length and data. `tag` should be in `0..32` otherwise the
/// function will panic.
- fn write_tagged_field<'s, P>(
- tag: u8, payload: &'s P,
- ) -> TaggedFieldIter<Box<dyn Iterator<Item = Fe32> + 's>>
+ fn write_tagged_field<'s, P>(tag: u8, payload: &'s P) -> impl Iterator<Item = Fe32> + 's
where
P: Base32Iterable + Base32Len + ?Sized,
{
@@ -416,54 +423,49 @@ impl Base32Iterable for TaggedField {
.chain(payload.fe_iter())
}
- // we will also need a giant enum for this
- Box::new(match *self {
+ define_iterator_enum!(ManyIters, A, B, C, D, E, F, G, H, I, J, K);
+ match *self {
TaggedField::PaymentHash(ref hash) => {
- write_tagged_field(constants::TAG_PAYMENT_HASH, hash)
+ ManyIters::A(write_tagged_field(constants::TAG_PAYMENT_HASH, hash))
},
TaggedField::Description(ref description) => {
- write_tagged_field(constants::TAG_DESCRIPTION, description)
+ ManyIters::B(write_tagged_field(constants::TAG_DESCRIPTION, description))
},
TaggedField::PayeePubKey(ref pub_key) => {
- write_tagged_field(constants::TAG_PAYEE_PUB_KEY, pub_key)
+ ManyIters::C(write_tagged_field(constants::TAG_PAYEE_PUB_KEY, pub_key))
},
TaggedField::DescriptionHash(ref hash) => {
- write_tagged_field(constants::TAG_DESCRIPTION_HASH, hash)
+ ManyIters::D(write_tagged_field(constants::TAG_DESCRIPTION_HASH, hash))
},
TaggedField::ExpiryTime(ref duration) => {
- write_tagged_field(constants::TAG_EXPIRY_TIME, duration)
+ ManyIters::E(write_tagged_field(constants::TAG_EXPIRY_TIME, duration))
},
TaggedField::MinFinalCltvExpiryDelta(ref expiry) => {
- write_tagged_field(constants::TAG_MIN_FINAL_CLTV_EXPIRY_DELTA, expiry)
+ ManyIters::F(write_tagged_field(constants::TAG_MIN_FINAL_CLTV_EXPIRY_DELTA, expiry))
},
TaggedField::Fallback(ref fallback_address) => {
- write_tagged_field(constants::TAG_FALLBACK, fallback_address)
+ ManyIters::G(write_tagged_field(constants::TAG_FALLBACK, fallback_address))
},
TaggedField::PrivateRoute(ref route_hops) => {
- write_tagged_field(constants::TAG_PRIVATE_ROUTE, route_hops)
+ ManyIters::H(write_tagged_field(constants::TAG_PRIVATE_ROUTE, route_hops))
},
TaggedField::PaymentSecret(ref payment_secret) => {
- write_tagged_field(constants::TAG_PAYMENT_SECRET, payment_secret)
+ ManyIters::I(write_tagged_field(constants::TAG_PAYMENT_SECRET, payment_secret))
},
TaggedField::PaymentMetadata(ref payment_metadata) => {
- write_tagged_field(constants::TAG_PAYMENT_METADATA, payment_metadata)
+ ManyIters::J(write_tagged_field(constants::TAG_PAYMENT_METADATA, payment_metadata))
},
TaggedField::Features(ref features) => {
- write_tagged_field(constants::TAG_FEATURES, features)
+ ManyIters::K(write_tagged_field(constants::TAG_FEATURES, features))
},
- })
+ }
}
}
impl Base32Iterable for Bolt11InvoiceSignature {
- fn fe_iter<'s>(&'s self) -> Box<dyn Iterator<Item = Fe32> + 's> {
+ fn fe_iter<'s>(&'s self) -> impl Iterator<Item = Fe32> + 's {
let (recovery_id, signature) = self.0.serialize_compact();
- Box::new(
- signature
- .into_iter()
- .chain(core::iter::once(recovery_id.to_i32() as u8))
- .bytes_to_fes(),
- )
+ signature.into_iter().chain(core::iter::once(recovery_id.to_i32() as u8)).bytes_to_fes()
}
}
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.