Add a read closure to the legacy TLV variant

Update the `legacy` TLV read/write variant signature from
`(legacy, $fieldty, $write)` to `(legacy, $fieldty, $read, $write)`,
adding a read closure parameter matching the `custom` variant's
signature.

The read closure is applied in `_check_missing_tlv!` after all TLV
fields are read but before `static_value` fields consume legacy
values. This preserves backwards compatibility with `static_value` and
`default_value` expressions that reference legacy field variables as
`Option<$fieldty>` during TLV reading.

The read closure signature matches `custom`:
`FnOnce(Option<$fieldty>) -> Result<Option<$fieldty>, DecodeError>`.
All existing usage sites use `Ok` as their read closure (identity/
no-op).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

Author: jkczyz

lightning/src/chain/package.rs

@@ -183,7 +183,7 @@ impl_writeable_tlv_based!(RevokedOutput, {
 	(12, on_counterparty_tx_csv, required),
 	// Unused since 0.1, this setting causes downgrades to before 0.1 to refuse to
 	// aggregate `RevokedOutput` claims, which is the more conservative stance.
-	(14, is_counterparty_balance_on_anchors, (legacy, (), |_| Some(()))),
+	(14, is_counterparty_balance_on_anchors, (legacy, (), Ok, |_| Some(()))),
 	(15, channel_parameters, (option: ReadableArgs, None)), // Added in 0.2.
 });
 

lightning/src/ln/channel_state.rs

@@ -607,9 +607,9 @@ impl_writeable_tlv_based!(ChannelDetails, {
 	(10, channel_value_satoshis, required),
 	(12, unspendable_punishment_reserve, option),
 	// Note that _user_channel_id_low is used below, but rustc warns anyway
-	(14, _user_channel_id_low, (legacy, u64,
+	(14, _user_channel_id_low, (legacy, u64, Ok,
 		|us: &ChannelDetails| Some(us.user_channel_id as u64))),
-	(16, _balance_msat, (legacy, u64, |us: &ChannelDetails| Some(us.next_outbound_htlc_limit_msat))),
+	(16, _balance_msat, (legacy, u64, Ok, |us: &ChannelDetails| Some(us.next_outbound_htlc_limit_msat))),
 	(18, outbound_capacity_msat, required),
 	(19, next_outbound_htlc_limit_msat, (default_value, outbound_capacity_msat)),
 	(20, inbound_capacity_msat, required),
@@ -623,7 +623,7 @@ impl_writeable_tlv_based!(ChannelDetails, {
 	(33, inbound_htlc_minimum_msat, option),
 	(35, inbound_htlc_maximum_msat, option),
 	// Note that _user_channel_id_high is used below, but rustc warns anyway
-	(37, _user_channel_id_high, (legacy, u64,
+	(37, _user_channel_id_high, (legacy, u64, Ok,
 		|us: &ChannelDetails| Some((us.user_channel_id >> 64) as u64))),
 	(39, feerate_sat_per_1000_weight, option),
 	(41, channel_shutdown_state, option),

lightning/src/ln/onion_utils.rs

@@ -1943,14 +1943,14 @@ impl Readable for HTLCFailReason {
 
 impl_writeable_tlv_based_enum!(HTLCFailReasonRepr,
 	(0, LightningError) => {
-		(0, data, (legacy, Vec<u8>, |us|
+		(0, data, (legacy, Vec<u8>, Ok, |us|
 			if let &HTLCFailReasonRepr::LightningError { err: msgs::OnionErrorPacket { ref data, .. }, .. } = us {
 				Some(data)
 			} else {
 				None
 			})
 		),
-		(1, attribution_data, (legacy, AttributionData, |us|
+		(1, attribution_data, (legacy, AttributionData, Ok, |us|
 			if let &HTLCFailReasonRepr::LightningError { err: msgs::OnionErrorPacket { ref attribution_data, .. }, .. } = us {
 				attribution_data.as_ref()
 			} else {
@@ -1961,7 +1961,7 @@ impl_writeable_tlv_based_enum!(HTLCFailReasonRepr,
 		(_unused, err, (static_value, msgs::OnionErrorPacket { data: data.ok_or(DecodeError::InvalidValue)?, attribution_data })),
 	},
 	(1, Reason) => {
-		(0, _failure_code, (legacy, u16,
+		(0, _failure_code, (legacy, u16, Ok,
 			|r: &HTLCFailReasonRepr| match r {
 				HTLCFailReasonRepr::LightningError{ .. } => None,
 				HTLCFailReasonRepr::Reason{ failure_reason, .. } => Some(failure_reason.failure_code())

lightning/src/ln/outbound_payment.rs

@@ -2731,7 +2731,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 	(5, AwaitingInvoice) => {
 		(0, expiration, required),
 		(2, retry_strategy, required),
-		(4, _max_total_routing_fee_msat, (legacy, u64,
+		(4, _max_total_routing_fee_msat, (legacy, u64, Ok,
 			|us: &PendingOutboundPayment| match us {
 				PendingOutboundPayment::AwaitingInvoice { route_params_config, .. } => route_params_config.max_total_routing_fee_msat,
 				_ => None,
@@ -2748,7 +2748,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 	(7, InvoiceReceived) => {
 		(0, payment_hash, required),
 		(2, retry_strategy, required),
-		(4, _max_total_routing_fee_msat, (legacy, u64,
+		(4, _max_total_routing_fee_msat, (legacy, u64, Ok,
 			|us: &PendingOutboundPayment| match us {
 				PendingOutboundPayment::InvoiceReceived { route_params_config, .. } => route_params_config.max_total_routing_fee_msat,
 				_ => None,
@@ -2779,7 +2779,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 	(11, AwaitingOffer) => {
 		(0, expiration, required),
 		(2, retry_strategy, required),
-		(4, _max_total_routing_fee_msat, (legacy, u64,
+		(4, _max_total_routing_fee_msat, (legacy, u64, Ok,
 			|us: &PendingOutboundPayment| match us {
 				PendingOutboundPayment::AwaitingOffer { route_params_config, .. } => route_params_config.max_total_routing_fee_msat,
 				_ => None,

lightning/src/util/ser_macros.rs

@@ -45,7 +45,7 @@ macro_rules! _encode_tlv {
 			field.write($stream)?;
 		}
 	};
-	($stream: expr, $optional_type: expr, $optional_field: expr, (legacy, $fieldty: ty, $write: expr) $(, $self: ident)?) => { {
+	($stream: expr, $optional_type: expr, $optional_field: expr, (legacy, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => { {
 		let value: Option<_> = $write($($self)?);
 		#[cfg(debug_assertions)]
 		{
@@ -64,7 +64,7 @@ macro_rules! _encode_tlv {
 		$crate::_encode_tlv!($stream, $optional_type, value, option);
 	} };
 	($stream: expr, $optional_type: expr, $optional_field: expr, (custom, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => { {
-		$crate::_encode_tlv!($stream, $optional_type, $optional_field, (legacy, $fieldty, $write) $(, $self)?);
+		$crate::_encode_tlv!($stream, $optional_type, $optional_field, (legacy, $fieldty, $read, $write) $(, $self)?);
 	} };
 	($stream: expr, $type: expr, $field: expr, optional_vec $(, $self: ident)?) => {
 		if !$field.is_empty() {
@@ -232,11 +232,11 @@ macro_rules! _get_varint_length_prefixed_tlv_length {
 			$len.0 += field_len;
 		}
 	};
-	($len: expr, $optional_type: expr, $optional_field: expr, (legacy, $fieldty: ty, $write: expr) $(, $self: ident)?) => {
+	($len: expr, $optional_type: expr, $optional_field: expr, (legacy, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => {
 		$crate::_get_varint_length_prefixed_tlv_length!($len, $optional_type, $write($($self)?), option);
 	};
 	($len: expr, $optional_type: expr, $optional_field: expr, (custom, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => {
-		$crate::_get_varint_length_prefixed_tlv_length!($len, $optional_type, $optional_field, (legacy, $fieldty, $write) $(, $self)?);
+		$crate::_get_varint_length_prefixed_tlv_length!($len, $optional_type, $optional_field, (legacy, $fieldty, $read, $write) $(, $self)?);
 	};
 	($len: expr, $type: expr, $field: expr, optional_vec $(, $self: ident)?) => {
 		if !$field.is_empty() {
@@ -320,7 +320,7 @@ macro_rules! _check_decoded_tlv_order {
 	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (option, explicit_type: $fieldty: ty)) => {{
 		// no-op
 	}};
-	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $write: expr)) => {{
+	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {{
 		// no-op
 	}};
 	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (custom, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => {{
@@ -398,8 +398,13 @@ macro_rules! _check_missing_tlv {
 	($last_seen_type: expr, $type: expr, $field: ident, (option, explicit_type: $fieldty: ty)) => {{
 		// no-op
 	}};
-	($last_seen_type: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $write: expr)) => {{
-		// no-op
+	($last_seen_type: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {{
+		use $crate::ln::msgs::DecodeError;
+		let read_result: Result<_, DecodeError> = $read($field);
+		#[allow(unused_assignments)]
+		{
+			$field = read_result?;
+		}
 	}};
 	($last_seen_type: expr, $type: expr, $field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {{
 		// Note that $type may be 0 making the second comparison always false
@@ -463,7 +468,7 @@ macro_rules! _decode_tlv {
 		let _field: &Option<$fieldty> = &$field;
 		$crate::_decode_tlv!($outer_reader, $reader, $field, option);
 	}};
-	($outer_reader: expr, $reader: expr, $field: ident, (legacy, $fieldty: ty, $write: expr)) => {{
+	($outer_reader: expr, $reader: expr, $field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {{
 		$crate::_decode_tlv!($outer_reader, $reader, $field, (option, explicit_type: $fieldty));
 	}};
 	($outer_reader: expr, $reader: expr, $field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {{
@@ -858,7 +863,7 @@ macro_rules! _init_tlv_based_struct_field {
 	($field: ident, option) => {
 		$field
 	};
-	($field: ident, (legacy, $fieldty: ty, $write: expr)) => {
+	($field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {
 		$crate::_init_tlv_based_struct_field!($field, option)
 	};
 	($field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {
@@ -927,7 +932,7 @@ macro_rules! _init_tlv_field_var {
 	($field: ident, (option, explicit_type: $fieldty: ty)) => {
 		let mut $field: Option<$fieldty> = None;
 	};
-	($field: ident, (legacy, $fieldty: ty, $write: expr)) => {
+	($field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {
 		$crate::_init_tlv_field_var!($field, (option, explicit_type: $fieldty));
 	};
 	($field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {
@@ -1012,10 +1017,12 @@ macro_rules! _decode_and_build {
 ///    [`MaybeReadable`], requiring the TLV to be present.
 /// If `$fieldty` is `optional_vec`, then `$field` is a [`Vec`], which needs to have its individual elements serialized.
 ///    Note that for `optional_vec` no bytes are written if the vec is empty
-/// If `$fieldty` is `(legacy, $ty, $write)` then, when writing, the function $write will be
+/// If `$fieldty` is `(legacy, $ty, $read, $write)` then, when writing, the function $write will be
 ///    called with the object being serialized and a returned `Option` and is written as a TLV if
-///    `Some`. When reading, an optional field of type `$ty` is read (which can be used in later
-///    `default_value` or `static_value` fields by referring to the value by name).
+///    `Some`. When reading, an optional field of type `$ty` is read, and after all TLV fields are
+///    read, the `$read` closure is called with the `Option<$ty>` value. The `$read` closure should
+///    return a `Result<Option<$ty>, DecodeError>`. Legacy field values can be used in later
+///    `default_value` or `static_value` fields by referring to the value by name.
 /// If `$fieldty` is `(custom, $ty, $read, $write)` then, when writing, the same behavior as
 ///    `legacy`, above is used. When reading, if a TLV is present, it is read as `$ty` and the
 ///    `$read` method is called with `Some(decoded_$ty_object)`. If no TLV is present, the field
@@ -1039,7 +1046,7 @@ macro_rules! _decode_and_build {
 /// 	(1, tlv_default_integer, (default_value, 7)),
 /// 	(2, tlv_optional_integer, option),
 /// 	(3, tlv_vec_type_integer, optional_vec),
-/// 	(4, unwritten_type, (legacy, u32, |us: &LightningMessage| Some(us.tlv_integer))),
+/// 	(4, unwritten_type, (legacy, u32, Ok, |us: &LightningMessage| Some(us.tlv_integer))),
 /// 	(_unused, tlv_upgraded_integer, (static_value, unwritten_type.unwrap_or(0) * 2))
 /// });
 /// ```
@@ -1931,7 +1938,7 @@ mod tests {
 		new_field: (u8, u8),
 	}
 	impl_writeable_tlv_based!(ExpandedField, {
-		(0, old_field, (legacy, u8, |us: &ExpandedField| Some(us.new_field.0))),
+		(0, old_field, (legacy, u8, Ok, |us: &ExpandedField| Some(us.new_field.0))),
 		(1, new_field, (default_value, (old_field.ok_or(DecodeError::InvalidValue)?, 0))),
 	});