util: add default_value_vec for defaults without LengthReadable

[carlaKC]

⚠️ Clauded and did not review this - just looking for a concept ack and I"ll clean it up properly !

---

In lightningdevkit/ldk-node#825, I ran into the orphan rule when trying to migrate from a single incoming/outgoing HTLC to a vec of HTLCLocator types to allow expressing multiple in/out HTLCs. This issue stems from the following:

• We want to declare our own HTLCLocator type in LDK-Node, so that we can have niceties like a typed user_channel_id.
• We don't want to break out of the impl_writeable_tlv_based_enum! macro for LDK-Node's Event type.
• To be forwards compatible (use the legacy single-htlc fields), we should use default_value to fill the old fields when the new ones aren't present.

However:

• default_value uses required under the hood, which expects the field to be LengthReadable.
• We cannot implement LengthReadable for Vec<HTLCLocator> in LDK-Node due to the orphan rule (we don't own Vec<T>, even if T is our type).
  • In LDK, this is when we use impl_for_vec! to implement this.

This change introduces a new default_value_vec to our macro which uses required_vec (and thus WithoutLength) under the hood, so that we don't need LengthReadable. There are a few ugly workarounds (like using custom), but wanting to write a Vec with a default seems like a common enough one to justify a change to our macros.

[ldk-reviews-bot]

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.11%. Comparing base (1f0763f) to head (5330f9f).
⚠️ Report is 55 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #4507 +/- ##
==========================================
+ Coverage 86.18% 87.11% +0.93% 
==========================================
Files 160 163 +3 Lines 107537 108765 +1228 Branches 107537 108765 +1228 ==========================================
+ Hits 92681 94755 +2074 + Misses 12229 11526 -703 + Partials 2627 2484 -143 

Flag	Coverage Δ
fuzzing	40.22% <ø> (?)
tests	86.21% <100.00%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tnull]

nit: Might make sense to use a dummy default value here rather than the empty Vec to check this is actually what's used rather than Vec::default() in general?

[TheBlueMatt]

default_value uses required under the hood, which expects the field to be LengthReadable.
We cannot implement LengthReadable for Vec in LDK-Node due to the orphan rule (we don't own Vec, even if T is our type).
In LDK, this is when we use impl_for_vec! to implement this.

Hmm, I guess we could use custom for this, no? In both impls wrap with WithoutLength and call it a day?

I'm not a huge fan of yet more write variants, ideally we'd drop some, really, but I do see why this is useful, and its not much of an extension on what we already have, so if custom isn't super clean I'm fine with this.

[carlaKC]

Hmm, I guess we could use custom for this, no? In both impls wrap with WithoutLength and call it a day?

Yeah we can, it's just pretty ugly, worth having the option IMO.

[TheBlueMatt]

Yeah we can, it's just pretty lightningdevkit/ldk-node#825 (comment), worth having the option IMO.

Oof, yea, that's not great. Happy to see this move forward.

[ldk-claude-review-bot]

I've performed a thorough re-review of this PR, tracing through all 7 macro dispatch points (_encode_tlv, _get_varint_length_prefixed_tlv_length, _check_decoded_tlv_order, _check_missing_tlv, _decode_tlv, _init_tlv_based_struct_field, _init_tlv_field_var), the generic dispatch macros (_check_encoded_tlv_order, _decode_tlv_stream_match_check), the proc macros (drop_legacy_field_definition, skip_legacy_fields), and the full encode/decode flow including the _decode_tlv_stream_range loop.

No issues found.

Specifically verified:

• The type chain is consistent: RequiredWrapper<Vec<T>> is initialized as None, set to Some(decoded_vec) on decode, set to Some($default) via .into() when missing, and safely unwrapped via .0.unwrap() since all paths guarantee Some.
• The encode chain default_value_vec -> required_vec -> WithoutLength($field) -> required correctly writes elements without a count prefix, always emitting the TLV.
• The legacy-to-vec migration pattern correctly uses lazy default evaluation — the default expression referencing the legacy field variable is only evaluated when the new TLV is absent.
• Empty vec round-trips correctly (TLV present with 0-byte content, reads back as empty vec, default not applied).
• Test hex values are correct.

[jkczyz]

If we use RequiredWrapper then we can re-use other patterns in all but one place.

diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs
index 52ee09505..751bf8422 100644
--- a/lightning/src/util/ser_macros.rs+++ b/lightning/src/util/ser_macros.rs@@ -308,13 +308,7 @@ macro_rules! _check_decoded_tlv_order {
}
}};
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (default_value_vec, $default: expr)) => {{
- // Note that $type may be 0 making the second comparison always false- #[allow(unused_comparisons)]- let invalid_order =- ($last_seen_type.is_none() || $last_seen_type.unwrap() < $type) && $typ.0 > $type;- if invalid_order {- $field = Some($default);- }+ _check_decoded_tlv_order!($last_seen_type, $typ, $type, $field, (default_value, $default));
}};
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (static_value, $value: expr)) => {};
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, required) => {{
@@ -388,12 +382,7 @@ macro_rules! _check_missing_tlv {
}
}};
($last_seen_type: expr, $type: expr, $field: ident, (default_value_vec, $default: expr)) => {{
- // Note that $type may be 0 making the second comparison always false- #[allow(unused_comparisons)]- let missing_req_type = $last_seen_type.is_none() || $last_seen_type.unwrap() < $type;- if missing_req_type {- $field = Some($default);- }+ _check_missing_tlv!($last_seen_type, $type, $field, (default_value, $default));
}};
($last_seen_type: expr, $type: expr, $field: expr, (static_value, $value: expr)) => {
$field = $value;
@@ -465,7 +454,7 @@ macro_rules! _decode_tlv {
}};
($outer_reader: expr, $reader: expr, $field: ident, (default_value_vec, $default: expr)) => {{
let f: $crate::util::ser::WithoutLength<Vec<_>> = $crate::util::ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?;
- $field = Some(f.0);+ $field = $crate::util::ser::RequiredWrapper(Some(f.0));
}};
($outer_reader: expr, $reader: expr, $field: ident, (static_value, $value: expr)) => {{
}};
@@ -882,7 +871,7 @@ macro_rules! _init_tlv_based_struct_field {
$field.0.unwrap()
};
($field: ident, (default_value_vec, $default: expr)) => {
- $field.unwrap()+ $crate::_init_tlv_based_struct_field!($field, (default_value, $default))
};
($field: ident, (static_value, $value: expr)) => {
$field
@@ -936,7 +925,7 @@ macro_rules! _init_tlv_field_var {
let mut $field = $crate::util::ser::RequiredWrapper(None);
};
($field: ident, (default_value_vec, $default: expr)) => {
- let mut $field: Option<Vec<_>> = None;+ $crate::_init_tlv_field_var!($field, (default_value, $default));
};
($field: ident, (static_value, $value: expr)) => {
let $field;

[tnull]

Put this on the 0.3 milestone as it would be if this makes it into the release and we can address the follow-up in LDK Node.

[carlaKC]

Sorry for delay - was OOO last week - will get to this today!

[carlaKC]

Pushed suggested changes to use RequiredWrapper - diff here.
Also updated lightningdevkit/ldk-node#850 to point to new changes.

[jkczyz]

nit: can make another struct for this.