Skip to content

Commit 49c80a3

Browse files
jkczyzclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 05c9a03 commit 49c80a3

File tree

5 files changed

+29
-25
lines changed

5 files changed

+29
-25
lines changed

lightning/src/chain/package.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ impl_writeable_tlv_based!(RevokedOutput, {
183183
(12, on_counterparty_tx_csv, required),
184184
// Unused since 0.1, this setting causes downgrades to before 0.1 to refuse to
185185
// aggregate `RevokedOutput` claims, which is the more conservative stance.
186-
(14, is_counterparty_balance_on_anchors, (legacy, (), |_| Some(()))),
186+
(14, is_counterparty_balance_on_anchors, (legacy, (), |_| Ok(()), |_| Some(()))),
187187
(15, channel_parameters, (option: ReadableArgs, None)), // Added in 0.2.
188188
});
189189

lightning/src/ln/channel_state.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -607,9 +607,9 @@ impl_writeable_tlv_based!(ChannelDetails, {
607607
(10, channel_value_satoshis, required),
608608
(12, unspendable_punishment_reserve, option),
609609
// Note that _user_channel_id_low is used below, but rustc warns anyway
610-
(14, _user_channel_id_low, (legacy, u64,
610+
(14, _user_channel_id_low, (legacy, u64, |_| Ok(()),
611611
|us: &ChannelDetails| Some(us.user_channel_id as u64))),
612-
(16, _balance_msat, (legacy, u64, |us: &ChannelDetails| Some(us.next_outbound_htlc_limit_msat))),
612+
(16, _balance_msat, (legacy, u64, |_| Ok(()), |us: &ChannelDetails| Some(us.next_outbound_htlc_limit_msat))),
613613
(18, outbound_capacity_msat, required),
614614
(19, next_outbound_htlc_limit_msat, (default_value, outbound_capacity_msat)),
615615
(20, inbound_capacity_msat, required),
@@ -623,7 +623,7 @@ impl_writeable_tlv_based!(ChannelDetails, {
623623
(33, inbound_htlc_minimum_msat, option),
624624
(35, inbound_htlc_maximum_msat, option),
625625
// Note that _user_channel_id_high is used below, but rustc warns anyway
626-
(37, _user_channel_id_high, (legacy, u64,
626+
(37, _user_channel_id_high, (legacy, u64, |_| Ok(()),
627627
|us: &ChannelDetails| Some((us.user_channel_id >> 64) as u64))),
628628
(39, feerate_sat_per_1000_weight, option),
629629
(41, channel_shutdown_state, option),

lightning/src/ln/onion_utils.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1943,14 +1943,14 @@ impl Readable for HTLCFailReason {
19431943

19441944
impl_writeable_tlv_based_enum!(HTLCFailReasonRepr,
19451945
(0, LightningError) => {
1946-
(0, data, (legacy, Vec<u8>, |us|
1946+
(0, data, (legacy, Vec<u8>, |_| Ok(()), |us|
19471947
if let &HTLCFailReasonRepr::LightningError { err: msgs::OnionErrorPacket { ref data, .. }, .. } = us {
19481948
Some(data)
19491949
} else {
19501950
None
19511951
})
19521952
),
1953-
(1, attribution_data, (legacy, AttributionData, |us|
1953+
(1, attribution_data, (legacy, AttributionData, |_| Ok(()), |us|
19541954
if let &HTLCFailReasonRepr::LightningError { err: msgs::OnionErrorPacket { ref attribution_data, .. }, .. } = us {
19551955
attribution_data.as_ref()
19561956
} else {
@@ -1961,7 +1961,7 @@ impl_writeable_tlv_based_enum!(HTLCFailReasonRepr,
19611961
(_unused, err, (static_value, msgs::OnionErrorPacket { data: data.ok_or(DecodeError::InvalidValue)?, attribution_data })),
19621962
},
19631963
(1, Reason) => {
1964-
(0, _failure_code, (legacy, u16,
1964+
(0, _failure_code, (legacy, u16, |_| Ok(()),
19651965
|r: &HTLCFailReasonRepr| match r {
19661966
HTLCFailReasonRepr::LightningError{ .. } => None,
19671967
HTLCFailReasonRepr::Reason{ failure_reason, .. } => Some(failure_reason.failure_code())

lightning/src/ln/outbound_payment.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2731,7 +2731,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
27312731
(5, AwaitingInvoice) => {
27322732
(0, expiration, required),
27332733
(2, retry_strategy, required),
2734-
(4, _max_total_routing_fee_msat, (legacy, u64,
2734+
(4, _max_total_routing_fee_msat, (legacy, u64, |_| Ok(()),
27352735
|us: &PendingOutboundPayment| match us {
27362736
PendingOutboundPayment::AwaitingInvoice { route_params_config, .. } => route_params_config.max_total_routing_fee_msat,
27372737
_ => None,
@@ -2748,7 +2748,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
27482748
(7, InvoiceReceived) => {
27492749
(0, payment_hash, required),
27502750
(2, retry_strategy, required),
2751-
(4, _max_total_routing_fee_msat, (legacy, u64,
2751+
(4, _max_total_routing_fee_msat, (legacy, u64, |_| Ok(()),
27522752
|us: &PendingOutboundPayment| match us {
27532753
PendingOutboundPayment::InvoiceReceived { route_params_config, .. } => route_params_config.max_total_routing_fee_msat,
27542754
_ => None,
@@ -2779,7 +2779,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
27792779
(11, AwaitingOffer) => {
27802780
(0, expiration, required),
27812781
(2, retry_strategy, required),
2782-
(4, _max_total_routing_fee_msat, (legacy, u64,
2782+
(4, _max_total_routing_fee_msat, (legacy, u64, |_| Ok(()),
27832783
|us: &PendingOutboundPayment| match us {
27842784
PendingOutboundPayment::AwaitingOffer { route_params_config, .. } => route_params_config.max_total_routing_fee_msat,
27852785
_ => None,

lightning/src/util/ser_macros.rs

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ macro_rules! _encode_tlv {
4545
field.write($stream)?;
4646
}
4747
};
48-
($stream: expr, $optional_type: expr, $optional_field: expr, (legacy, $fieldty: ty, $write: expr) $(, $self: ident)?) => { {
48+
($stream: expr, $optional_type: expr, $optional_field: expr, (legacy, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => { {
4949
let value: Option<_> = $write($($self)?);
5050
#[cfg(debug_assertions)]
5151
{
@@ -64,7 +64,7 @@ macro_rules! _encode_tlv {
6464
$crate::_encode_tlv!($stream, $optional_type, value, option);
6565
} };
6666
($stream: expr, $optional_type: expr, $optional_field: expr, (custom, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => { {
67-
$crate::_encode_tlv!($stream, $optional_type, $optional_field, (legacy, $fieldty, $write) $(, $self)?);
67+
$crate::_encode_tlv!($stream, $optional_type, $optional_field, (legacy, $fieldty, $read, $write) $(, $self)?);
6868
} };
6969
($stream: expr, $type: expr, $field: expr, optional_vec $(, $self: ident)?) => {
7070
if !$field.is_empty() {
@@ -232,11 +232,11 @@ macro_rules! _get_varint_length_prefixed_tlv_length {
232232
$len.0 += field_len;
233233
}
234234
};
235-
($len: expr, $optional_type: expr, $optional_field: expr, (legacy, $fieldty: ty, $write: expr) $(, $self: ident)?) => {
235+
($len: expr, $optional_type: expr, $optional_field: expr, (legacy, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => {
236236
$crate::_get_varint_length_prefixed_tlv_length!($len, $optional_type, $write($($self)?), option);
237237
};
238238
($len: expr, $optional_type: expr, $optional_field: expr, (custom, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => {
239-
$crate::_get_varint_length_prefixed_tlv_length!($len, $optional_type, $optional_field, (legacy, $fieldty, $write) $(, $self)?);
239+
$crate::_get_varint_length_prefixed_tlv_length!($len, $optional_type, $optional_field, (legacy, $fieldty, $read, $write) $(, $self)?);
240240
};
241241
($len: expr, $type: expr, $field: expr, optional_vec $(, $self: ident)?) => {
242242
if !$field.is_empty() {
@@ -320,7 +320,7 @@ macro_rules! _check_decoded_tlv_order {
320320
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (option, explicit_type: $fieldty: ty)) => {{
321321
// no-op
322322
}};
323-
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $write: expr)) => {{
323+
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {{
324324
// no-op
325325
}};
326326
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (custom, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => {{
@@ -398,8 +398,10 @@ macro_rules! _check_missing_tlv {
398398
($last_seen_type: expr, $type: expr, $field: ident, (option, explicit_type: $fieldty: ty)) => {{
399399
// no-op
400400
}};
401-
($last_seen_type: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $write: expr)) => {{
402-
// no-op
401+
($last_seen_type: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {{
402+
use $crate::ln::msgs::DecodeError;
403+
let read_result: Result<(), DecodeError> = $read($field.as_ref());
404+
read_result?;
403405
}};
404406
($last_seen_type: expr, $type: expr, $field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {{
405407
// Note that $type may be 0 making the second comparison always false
@@ -463,7 +465,7 @@ macro_rules! _decode_tlv {
463465
let _field: &Option<$fieldty> = &$field;
464466
$crate::_decode_tlv!($outer_reader, $reader, $field, option);
465467
}};
466-
($outer_reader: expr, $reader: expr, $field: ident, (legacy, $fieldty: ty, $write: expr)) => {{
468+
($outer_reader: expr, $reader: expr, $field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {{
467469
$crate::_decode_tlv!($outer_reader, $reader, $field, (option, explicit_type: $fieldty));
468470
}};
469471
($outer_reader: expr, $reader: expr, $field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {{
@@ -858,7 +860,7 @@ macro_rules! _init_tlv_based_struct_field {
858860
($field: ident, option) => {
859861
$field
860862
};
861-
($field: ident, (legacy, $fieldty: ty, $write: expr)) => {
863+
($field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {
862864
$crate::_init_tlv_based_struct_field!($field, option)
863865
};
864866
($field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {
@@ -927,7 +929,7 @@ macro_rules! _init_tlv_field_var {
927929
($field: ident, (option, explicit_type: $fieldty: ty)) => {
928930
let mut $field: Option<$fieldty> = None;
929931
};
930-
($field: ident, (legacy, $fieldty: ty, $write: expr)) => {
932+
($field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {
931933
$crate::_init_tlv_field_var!($field, (option, explicit_type: $fieldty));
932934
};
933935
($field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {
@@ -1012,10 +1014,12 @@ macro_rules! _decode_and_build {
10121014
/// [`MaybeReadable`], requiring the TLV to be present.
10131015
/// If `$fieldty` is `optional_vec`, then `$field` is a [`Vec`], which needs to have its individual elements serialized.
10141016
/// Note that for `optional_vec` no bytes are written if the vec is empty
1015-
/// If `$fieldty` is `(legacy, $ty, $write)` then, when writing, the function $write will be
1017+
/// If `$fieldty` is `(legacy, $ty, $read, $write)` then, when writing, the function $write will be
10161018
/// called with the object being serialized and a returned `Option` and is written as a TLV if
1017-
/// `Some`. When reading, an optional field of type `$ty` is read (which can be used in later
1018-
/// `default_value` or `static_value` fields by referring to the value by name).
1019+
/// `Some`. When reading, an optional field of type `$ty` is read, and after all TLV fields are
1020+
/// read, the `$read` closure is called with the `Option<$ty>` value. The `$read` closure should
1021+
/// return a `Result<Option<$ty>, DecodeError>`. Legacy field values can be used in later
1022+
/// `default_value` or `static_value` fields by referring to the value by name.
10191023
/// If `$fieldty` is `(custom, $ty, $read, $write)` then, when writing, the same behavior as
10201024
/// `legacy`, above is used. When reading, if a TLV is present, it is read as `$ty` and the
10211025
/// `$read` method is called with `Some(decoded_$ty_object)`. If no TLV is present, the field
@@ -1039,7 +1043,7 @@ macro_rules! _decode_and_build {
10391043
/// (1, tlv_default_integer, (default_value, 7)),
10401044
/// (2, tlv_optional_integer, option),
10411045
/// (3, tlv_vec_type_integer, optional_vec),
1042-
/// (4, unwritten_type, (legacy, u32, |us: &LightningMessage| Some(us.tlv_integer))),
1046+
/// (4, unwritten_type, (legacy, u32, |_| Ok(()), |us: &LightningMessage| Some(us.tlv_integer))),
10431047
/// (_unused, tlv_upgraded_integer, (static_value, unwritten_type.unwrap_or(0) * 2))
10441048
/// });
10451049
/// ```
@@ -1931,7 +1935,7 @@ mod tests {
19311935
new_field: (u8, u8),
19321936
}
19331937
impl_writeable_tlv_based!(ExpandedField, {
1934-
(0, old_field, (legacy, u8, |us: &ExpandedField| Some(us.new_field.0))),
1938+
(0, old_field, (legacy, u8, |_| Ok(()), |us: &ExpandedField| Some(us.new_field.0))),
19351939
(1, new_field, (default_value, (old_field.ok_or(DecodeError::InvalidValue)?, 0))),
19361940
});
19371941

0 commit comments

Comments
 (0)