Skip to content

Commit 579f14f

Browse files
jkczyzclaude
jkczyz
and
claude
committed
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 called in the same three macro sites as `custom`: 1. `_decode_tlv!` — TLV present: `$read(Some(decoded_value))` 2. `_check_decoded_tlv_order!` — TLV skipped mid-stream: `$read(None)` 3. `_check_missing_tlv!` — TLV missing at end: `$read(None)` The closure receives `Some(value)` when the TLV is present and `None` when it was skipped or missing. The result is assigned back to `$field` because other fields may reference legacy field variables in their `default_value` or `static_value` expressions (e.g., `_user_channel_id_low` in `ChannelDetails`). 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 579f14f

5 files changed

Lines changed: 47 additions & 27 deletions

File tree

‎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: 37 additions & 17 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,8 +320,15 @@ 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)) => {{
324-
// no-op
323+
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {{
324+
// Note that $type may be 0 making the second comparison always false
325+
#[allow(unused_comparisons)]
326+
let invalid_order =
327+
($last_seen_type.is_none() || $last_seen_type.unwrap() < $type) && $typ.0 > $type;
328+
if invalid_order {
329+
let read_result: Result<Option<$fieldty>, DecodeError> = $read(None);
330+
$field = read_result?;
331+
}
325332
}};
326333
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (custom, $fieldty: ty, $read: expr, $write: expr) $(, $self: ident)?) => {{
327334
// Note that $type may be 0 making the second comparison always false
@@ -398,8 +405,16 @@ macro_rules! _check_missing_tlv {
398405
($last_seen_type: expr, $type: expr, $field: ident, (option, explicit_type: $fieldty: ty)) => {{
399406
// no-op
400407
}};
401-
($last_seen_type: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $write: expr)) => {{
402-
// no-op
408+
($last_seen_type: expr, $type: expr, $field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {{
409+
// Note that $type may be 0 making the second comparison always false
410+
#[allow(unused_comparisons)]
411+
let missing_tlv = $last_seen_type.is_none() || $last_seen_type.unwrap() < $type;
412+
if missing_tlv {
413+
let read_result: Result<Option<$fieldty>, DecodeError> = $read(None);
414+
$field = read_result?;
415+
}
416+
// Suppress unused_variables warning since legacy fields are not included in the struct.
417+
let _ = &$field;
403418
}};
404419
($last_seen_type: expr, $type: expr, $field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {{
405420
// Note that $type may be 0 making the second comparison always false
@@ -463,8 +478,11 @@ macro_rules! _decode_tlv {
463478
let _field: &Option<$fieldty> = &$field;
464479
$crate::_decode_tlv!($outer_reader, $reader, $field, option);
465480
}};
466-
($outer_reader: expr, $reader: expr, $field: ident, (legacy, $fieldty: ty, $write: expr)) => {{
467-
$crate::_decode_tlv!($outer_reader, $reader, $field, (option, explicit_type: $fieldty));
481+
($outer_reader: expr, $reader: expr, $field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {{
482+
let read_field: $fieldty;
483+
$crate::_decode_tlv!($outer_reader, $reader, read_field, required);
484+
let read_result: Result<Option<$fieldty>, DecodeError> = $read(Some(read_field));
485+
$field = read_result?;
468486
}};
469487
($outer_reader: expr, $reader: expr, $field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {{
470488
let read_field: $fieldty;
@@ -858,7 +876,7 @@ macro_rules! _init_tlv_based_struct_field {
858876
($field: ident, option) => {
859877
$field
860878
};
861-
($field: ident, (legacy, $fieldty: ty, $write: expr)) => {
879+
($field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {
862880
$crate::_init_tlv_based_struct_field!($field, option)
863881
};
864882
($field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {
@@ -927,7 +945,7 @@ macro_rules! _init_tlv_field_var {
927945
($field: ident, (option, explicit_type: $fieldty: ty)) => {
928946
let mut $field: Option<$fieldty> = None;
929947
};
930-
($field: ident, (legacy, $fieldty: ty, $write: expr)) => {
948+
($field: ident, (legacy, $fieldty: ty, $read: expr, $write: expr)) => {
931949
$crate::_init_tlv_field_var!($field, (option, explicit_type: $fieldty));
932950
};
933951
($field: ident, (custom, $fieldty: ty, $read: expr, $write: expr)) => {
@@ -1012,10 +1030,12 @@ macro_rules! _decode_and_build {
10121030
/// [`MaybeReadable`], requiring the TLV to be present.
10131031
/// If `$fieldty` is `optional_vec`, then `$field` is a [`Vec`], which needs to have its individual elements serialized.
10141032
/// 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
1033+
/// If `$fieldty` is `(legacy, $ty, $read, $write)` then, when writing, the function $write will be
10161034
/// 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).
1035+
/// `Some`. When reading, an optional field of type `$ty` is read, and after all TLV fields are
1036+
/// read, the `$read` closure is called with the `Option<$ty>` value. The `$read` closure should
1037+
/// return a `Result<Option<$ty>, DecodeError>`. Legacy field values can be used in later
1038+
/// `default_value` or `static_value` fields by referring to the value by name.
10191039
/// If `$fieldty` is `(custom, $ty, $read, $write)` then, when writing, the same behavior as
10201040
/// `legacy`, above is used. When reading, if a TLV is present, it is read as `$ty` and the
10211041
/// `$read` method is called with `Some(decoded_$ty_object)`. If no TLV is present, the field
@@ -1039,7 +1059,7 @@ macro_rules! _decode_and_build {
10391059
/// (1, tlv_default_integer, (default_value, 7)),
10401060
/// (2, tlv_optional_integer, option),
10411061
/// (3, tlv_vec_type_integer, optional_vec),
1042-
/// (4, unwritten_type, (legacy, u32, |us: &LightningMessage| Some(us.tlv_integer))),
1062+
/// (4, unwritten_type, (legacy, u32, Ok, |us: &LightningMessage| Some(us.tlv_integer))),
10431063
/// (_unused, tlv_upgraded_integer, (static_value, unwritten_type.unwrap_or(0) * 2))
10441064
/// });
10451065
/// ```
@@ -1931,7 +1951,7 @@ mod tests {
19311951
new_field: (u8, u8),
19321952
}
19331953
impl_writeable_tlv_based!(ExpandedField, {
1934-
(0, old_field, (legacy, u8, |us: &ExpandedField| Some(us.new_field.0))),
1954+
(0, old_field, (legacy, u8, Ok, |us: &ExpandedField| Some(us.new_field.0))),
19351955
(1, new_field, (default_value, (old_field.ok_or(DecodeError::InvalidValue)?, 0))),
19361956
});
19371957

0 commit comments

Comments
 (0)