← Watch feed
Generate TLV write length impls
Public commit record
68/100 · Adequate
What the developer wrote
Authored by Joost Jager
Generate TLV write length impls
Add a writeable TLV helper macro that emits both write and
serialized_length from the same field list. Reuse the shared TLV length
helper from impl_ser_tlv_based so the generated read/write path and the
new custom-read path stay aligned.
Use the new helper for the hot channel funding and commitment
transaction TLV writers while leaving their custom read implementations
unchanged.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a code cleanup and performance improvement. It introduces a new Rust macro that automatically generates both the 'write' and 'serialized_length' functions for certain data structures from a single field list. Previously, these two functions were written separately, which made it easier for them to become inconsistent. The change applies this new macro to hot-path channel funding and commitment transaction serialization, but leaves the corresponding read/deserialization code untouched. There is no indication of a security bug being fixed.
Recommended action
No security action required. This is a refactoring/performance change. Normal code review and regression testing are sufficient.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch adds impl_writeable_tlv_based! and _tlv_fields_serialized_length! macros in lightning/src/util/ser_macros.rs. These macros derive Writeable::write and Writeable::serialized_length from the same TLV field list, reusing the existing _get_varint_length_prefixed_tlv_length! helper. impl_ser_tlv_based! is refactored to use the shared length helper. ChannelTransactionParameters, CommitmentTransaction, and FundingScope are converted from hand-written Writeable impls to the new macro. The custom Readable/ReadableArgs implementations are preserved. The change reduces code duplication and ensures the write path and length calculation stay aligned.
Changed components
lightning/src/util/ser_macros.rslightning/src/ln/chan_utils.rslightning/src/ln/channel.rsInspect captured patch +97 / −65
diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs
index 238ef71..dd33477 100644
--- a/lightning/src/ln/chan_utils.rs
+++ b/lightning/src/ln/chan_utils.rs
@@ -1169,24 +1169,21 @@ impl_ser_tlv_based!(CounterpartyChannelTransactionParameters, {
(2, selected_contest_delay, required),
});
-impl Writeable for ChannelTransactionParameters {
- #[rustfmt::skip]
- fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
- let legacy_deserialization_prevention_marker = legacy_deserialization_prevention_marker_for_channel_type_features(&self.channel_type_features);
- write_tlv_fields!(writer, {
- (0, self.holder_pubkeys, required),
- (2, self.holder_selected_contest_delay, required),
- (4, self.is_outbound_from_holder, required),
- (6, self.counterparty_parameters, option),
- (8, self.funding_outpoint, option),
- (10, legacy_deserialization_prevention_marker, option),
- (11, self.channel_type_features, required),
- (12, self.splice_parent_funding_txid, option),
- (13, self.channel_value_satoshis, required),
- });
- Ok(())
- }
-}
+impl_writeable_tlv_based!(ChannelTransactionParameters, self, {
+ (0, self.holder_pubkeys, required),
+ (2, self.holder_selected_contest_delay, required),
+ (4, self.is_outbound_from_holder, required),
+ (6, self.counterparty_parameters, option),
+ (8, self.funding_outpoint, option),
+ (
+ 10,
+ legacy_deserialization_prevention_marker_for_channel_type_features(&self.channel_type_features),
+ option
+ ),
+ (11, self.channel_type_features, required),
+ (12, self.splice_parent_funding_txid, option),
+ (13, self.channel_value_satoshis, required),
+});
impl ReadableArgs<Option<u64>> for ChannelTransactionParameters {
#[rustfmt::skip]
@@ -1634,25 +1631,22 @@ impl PartialEq for CommitmentTransaction {
}
}
-impl Writeable for CommitmentTransaction {
- #[rustfmt::skip]
- fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
- let legacy_deserialization_prevention_marker = legacy_deserialization_prevention_marker_for_channel_type_features(&self.channel_type_features);
- write_tlv_fields!(writer, {
- (0, self.commitment_number, required),
- (1, self.to_broadcaster_delay, option),
- (2, self.to_broadcaster_value_sat, required),
- (4, self.to_countersignatory_value_sat, required),
- (6, self.feerate_per_kw, required),
- (8, self.keys, required),
- (10, self.built, required),
- (12, self.nondust_htlcs, required_vec),
- (14, legacy_deserialization_prevention_marker, option),
- (15, self.channel_type_features, required),
- });
- Ok(())
- }
-}
+impl_writeable_tlv_based!(CommitmentTransaction, self, {
+ (0, self.commitment_number, required),
+ (1, self.to_broadcaster_delay, option),
+ (2, self.to_broadcaster_value_sat, required),
+ (4, self.to_countersignatory_value_sat, required),
+ (6, self.feerate_per_kw, required),
+ (8, self.keys, required),
+ (10, self.built, required),
+ (12, self.nondust_htlcs, required_vec),
+ (
+ 14,
+ legacy_deserialization_prevention_marker_for_channel_type_features(&self.channel_type_features),
+ option
+ ),
+ (15, self.channel_type_features, required),
+});
impl Readable for CommitmentTransaction {
#[rustfmt::skip]
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index cfa7304..5d43af0 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2596,22 +2596,17 @@ pub(super) struct FundingScope {
minimum_depth_override: Option<u32>,
}
-impl Writeable for FundingScope {
- fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
- write_tlv_fields!(writer, {
- (1, self.value_to_self_msat, required),
- (3, self.counterparty_selected_channel_reserve_satoshis, option),
- (5, self.holder_selected_channel_reserve_satoshis, required),
- (7, self.channel_transaction_parameters, (required: ReadableArgs, None)),
- (9, self.funding_transaction, option),
- (11, self.funding_tx_confirmed_in, option),
- (13, self.funding_tx_confirmation_height, required),
- (15, self.short_channel_id, option),
- (17, self.minimum_depth_override, option),
- });
- Ok(())
- }
-}
+impl_writeable_tlv_based!(FundingScope, self, {
+ (1, self.value_to_self_msat, required),
+ (3, self.counterparty_selected_channel_reserve_satoshis, option),
+ (5, self.holder_selected_channel_reserve_satoshis, required),
+ (7, self.channel_transaction_parameters, (required: ReadableArgs, None)),
+ (9, self.funding_transaction, option),
+ (11, self.funding_tx_confirmed_in, option),
+ (13, self.funding_tx_confirmation_height, required),
+ (15, self.short_channel_id, option),
+ (17, self.minimum_depth_override, option),
+});
impl Readable for FundingScope {
#[rustfmt::skip]
diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs
index 716be68..e6f558b 100644
--- a/lightning/src/util/ser_macros.rs
+++ b/lightning/src/util/ser_macros.rs
@@ -839,6 +839,58 @@ macro_rules! write_tlv_fields {
}
}
+#[doc(hidden)]
+#[macro_export]
+macro_rules! _tlv_fields_serialized_length {
+ ({$(($type: expr, $field: expr, $fieldty: tt $(, $self: ident)?)),* $(,)*}) => { {
+ use $crate::util::ser::BigSize;
+ let len = {
+ #[allow(unused_mut)]
+ let mut len = $crate::util::ser::LengthCalculatingWriter(0);
+ $(
+ $crate::_get_varint_length_prefixed_tlv_length!(len, $type, &$field, $fieldty $(, $self)?);
+ )*
+ len.0
+ };
+ let mut len_calc = $crate::util::ser::LengthCalculatingWriter(0);
+ BigSize(len as u64).write(&mut len_calc).expect("No in-memory data may fail to serialize");
+ len + len_calc.0
+ } }
+}
+
+/// Implements [`Writeable`] for a type serialized as a length-prefixed TLV stream.
+///
+/// This is useful for types that share the TLV-writing format used by
+/// [`impl_ser_tlv_based`] but need a custom read implementation. The field list uses the
+/// same entries accepted by [`write_tlv_fields`], and the macro derives both `write` and
+/// `serialized_length` from that list so the two paths stay aligned.
+///
+/// The `$self` argument names the generated `self` binding, allowing field expressions to refer
+/// to it explicitly.
+///
+/// [`Writeable`]: crate::util::ser::Writeable
+/// [`impl_ser_tlv_based`]: crate::impl_ser_tlv_based
+/// [`write_tlv_fields`]: crate::write_tlv_fields
+macro_rules! impl_writeable_tlv_based {
+ ($st: ty, $self: ident, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => {
+ impl $crate::util::ser::Writeable for $st {
+ fn write<W: $crate::util::ser::Writer>(&$self, writer: &mut W) -> Result<(), $crate::io::Error> {
+ write_tlv_fields!(writer, {
+ $(($type, $field, $fieldty)),*
+ });
+ Ok(())
+ }
+
+ #[inline]
+ fn serialized_length(&$self) -> usize {
+ $crate::_tlv_fields_serialized_length!({
+ $(($type, $field, $fieldty)),*
+ })
+ }
+ }
+ }
+}
+
/// Reads a prefix added by [`write_ver_prefix`], above. Takes the current version of the
/// serialization logic for this object. This is compared against the
/// `$min_version_that_can_read_this` added by [`write_ver_prefix`].
@@ -1095,18 +1147,9 @@ macro_rules! impl_ser_tlv_based {
#[inline]
fn serialized_length(&self) -> usize {
- use $crate::util::ser::BigSize;
- let len = {
- #[allow(unused_mut)]
- let mut len = $crate::util::ser::LengthCalculatingWriter(0);
- $(
- $crate::_get_varint_length_prefixed_tlv_length!(len, $type, &self.$field, $fieldty, self);
- )*
- len.0
- };
- let mut len_calc = $crate::util::ser::LengthCalculatingWriter(0);
- BigSize(len as u64).write(&mut len_calc).expect("No in-memory data may fail to serialize");
- len + len_calc.0
+ $crate::_tlv_fields_serialized_length!({
+ $(($type, self.$field, $fieldty, self)),*
+ })
}
}
Risk score
Our methodology →Why this scored 13/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.