Centralize custom TLV validation behind `CustomTlvs`
What changed, and why it matters
This commit is a code cleanup, not a security fix. It moves the existing checks that validate custom Lightning payment data (custom TLVs) from one place in the code into a new dedicated wrapper type called RecipientCustomTlvs. The same rules—type numbers must be large enough, must not duplicate reserved protocol types like keysend, and must not repeat—are preserved. No new restrictions are added and no vulnerability is described.
No security action required. Treat as ordinary refactoring. If reviewing, confirm that all previous call sites now construct RecipientCustomTlvs and that no new path bypasses validation.
Security signals we found
Refactor only: validation logic moved, not changed
Return type of with_custom_tlvs changed from Result to Self because validation now happens earlier in RecipientCustomTlvs::new
Same reserved-type rejections retained: keysend (5482373484) and async-payment invoice request (77_777)
No CVE, advisory, or security-relevant description in commit message or diff
Evidence from the diff
The patch refactors RecipientOnionFields::with_custom_tlvs so that validation/sorting of custom TLVs now lives in a new RecipientCustomTlvs::new constructor. with_custom_tlvs now accepts an already-validated RecipientCustomTlvs and simply copies its inner Vec, changing its return type from Result
Changed components
lightning/src/ln/outbound_payment.rslightning/src/ln/msgs.rslightning/src/ln/blinded_payment_tests.rslightning/src/ln/max_payment_path_len_tests.rslightning/src/ln/payment_tests.rsInspect captured patch +79 / −48
diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs
index 5a7c326..80da764 100644
--- a/lightning/src/ln/blinded_payment_tests.rs
+++ b/lightning/src/ln/blinded_payment_tests.rs
@@ -22,7 +22,7 @@ use crate::ln::msgs::{
};
use crate::ln::onion_payment;
use crate::ln::onion_utils::{self, LocalHTLCFailureReason};
-use crate::ln::outbound_payment::{Retry, IDEMPOTENCY_TIMEOUT_TICKS};
+use crate::ln::outbound_payment::{RecipientCustomTlvs, Retry, IDEMPOTENCY_TIMEOUT_TICKS};
use crate::ln::types::ChannelId;
use crate::offers::invoice::UnsignedBolt12Invoice;
use crate::prelude::*;
@@ -1431,8 +1431,7 @@ fn custom_tlvs_to_blinded_path() {
);
let recipient_onion_fields = RecipientOnionFields::spontaneous_empty()
- .with_custom_tlvs(vec![((1 << 16) + 1, vec![42, 42])])
- .unwrap();
+ .with_custom_tlvs(RecipientCustomTlvs::new(vec![((1 << 16) + 1, vec![42, 42])]).unwrap());
nodes[0].node.send_payment(payment_hash, recipient_onion_fields.clone(),
PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap();
check_added_monitors(&nodes[0], 1);
diff --git a/lightning/src/ln/max_payment_path_len_tests.rs b/lightning/src/ln/max_payment_path_len_tests.rs
index fa7e8d8..b947273 100644
--- a/lightning/src/ln/max_payment_path_len_tests.rs
+++ b/lightning/src/ln/max_payment_path_len_tests.rs
@@ -23,7 +23,9 @@ use crate::ln::msgs;
use crate::ln::msgs::{BaseMessageHandler, OnionMessageHandler};
use crate::ln::onion_utils;
use crate::ln::onion_utils::MIN_FINAL_VALUE_ESTIMATE_WITH_OVERPAY;
-use crate::ln::outbound_payment::{RecipientOnionFields, Retry, RetryableSendFailure};
+use crate::ln::outbound_payment::{
+ RecipientCustomTlvs, RecipientOnionFields, Retry, RetryableSendFailure,
+};
use crate::prelude::*;
use crate::routing::router::{
PaymentParameters, RouteParameters, DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA,
@@ -259,9 +261,9 @@ fn one_hop_blinded_path_with_custom_tlv() {
- final_payload_len_without_custom_tlv;
// Check that we can send the maximum custom TLV with 1 blinded hop.
- let max_sized_onion = RecipientOnionFields::spontaneous_empty()
- .with_custom_tlvs(vec![(CUSTOM_TLV_TYPE, vec![42; max_custom_tlv_len])])
- .unwrap();
+ let max_sized_onion = RecipientOnionFields::spontaneous_empty().with_custom_tlvs(
+ RecipientCustomTlvs::new(vec![(CUSTOM_TLV_TYPE, vec![42; max_custom_tlv_len])]).unwrap(),
+ );
let id = PaymentId(payment_hash.0);
let no_retry = Retry::Attempts(0);
nodes[1]
@@ -385,9 +387,9 @@ fn blinded_path_with_custom_tlv() {
- reserved_packet_bytes_without_custom_tlv;
// Check that we can send the maximum custom TLV size with 0 intermediate unblinded hops.
- let max_sized_onion = RecipientOnionFields::spontaneous_empty()
- .with_custom_tlvs(vec![(CUSTOM_TLV_TYPE, vec![42; max_custom_tlv_len])])
- .unwrap();
+ let max_sized_onion = RecipientOnionFields::spontaneous_empty().with_custom_tlvs(
+ RecipientCustomTlvs::new(vec![(CUSTOM_TLV_TYPE, vec![42; max_custom_tlv_len])]).unwrap(),
+ );
let no_retry = Retry::Attempts(0);
let id = PaymentId(payment_hash.0);
nodes[1]
diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
index 2bb2b24..994443d 100644
--- a/lightning/src/ln/msgs.rs
+++ b/lightning/src/ln/msgs.rs
@@ -3537,7 +3537,7 @@ impl<'a> Writeable for OutboundOnionPayload<'a> {
ref invoice_request,
ref custom_tlvs,
} => {
- // We need to update [`ln::outbound_payment::RecipientOnionFields::with_custom_tlvs`]
+ // We need to update [`ln::outbound_payments::RecipientCustomTlvs::new`]
// to reject any reserved types in the experimental range if new ones are ever
// standardized.
let invoice_request_tlv = invoice_request.map(|invreq| (77_777, invreq.encode())); // TODO: update TLV type once the async payments spec is merged
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 6549382..67dba86 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -677,6 +677,54 @@ pub enum ProbeSendFailure {
DuplicateProbe,
}
+/// A validated, sorted set of custom TLVs for payment recipient onion fields.
+#[derive(Clone)]
+pub struct RecipientCustomTlvs(Vec<(u64, Vec<u8>)>);
+
+impl RecipientCustomTlvs {
+ /// Each TLV is provided as a `(u64, Vec<u8>)` for the type number and
+ /// serialized value respectively. TLV type numbers must be unique and
+ /// within the range reserved for custom types, i.e. >= 2^16, otherwise
+ /// this method will return `Err(())`.
+ ///
+ /// This method will also error for TLV types in the experimental range
+ /// which have since been standardized within the protocol. This currently
+ /// includes 5482373484 (keysend) and 77_777 (invoice requests for async
+ /// payments).
+ pub fn new(mut tlvs: Vec<(u64, Vec<u8>)>) -> Result<Self, ()> {
+ tlvs.sort_unstable_by_key(|(typ, _)| *typ);
+ let mut prev_type = None;
+ for (typ, _) in tlvs.iter() {
+ if *typ < 1 << 16 {
+ return Err(());
+ }
+ if *typ == 5482373484 {
+ return Err(());
+ } // keysend
+ if *typ == 77_777 {
+ return Err(());
+ } // invoice requests for async payments
+ match prev_type {
+ Some(prev) if prev >= *typ => return Err(()),
+ _ => {},
+ }
+ prev_type = Some(*typ);
+ }
+
+ Ok(Self(tlvs))
+ }
+
+ /// Returns the inner TLV list.
+ pub(super) fn into_inner(self) -> Vec<(u64, Vec<u8>)> {
+ self.0
+ }
+
+ /// Borrow the inner TLV list.
+ pub fn as_slice(&self) -> &[(u64, Vec<u8>)] {
+ &self.0
+ }
+}
+
/// Information which is provided, encrypted, to the payment recipient when sending HTLCs.
///
/// This should generally be constructed with data communicated to us from the recipient (via a
@@ -739,31 +787,13 @@ impl RecipientOnionFields {
Self { payment_secret: None, payment_metadata: None, custom_tlvs: Vec::new() }
}
- /// Creates a new [`RecipientOnionFields`] from an existing one, adding custom TLVs. Each
- /// TLV is provided as a `(u64, Vec<u8>)` for the type number and serialized value
- /// respectively. TLV type numbers must be unique and within the range
- /// reserved for custom types, i.e. >= 2^16, otherwise this method will return `Err(())`.
- ///
- /// This method will also error for types in the experimental range which have been
- /// standardized within the protocol, which only includes 5482373484 (keysend) for now.
+ /// Creates a new [`RecipientOnionFields`] from an existing one, adding validated custom TLVs.
///
/// See [`Self::custom_tlvs`] for more info.
#[rustfmt::skip]
- pub fn with_custom_tlvs(mut self, mut custom_tlvs: Vec<(u64, Vec<u8>)>) -> Result<Self, ()> {
- custom_tlvs.sort_unstable_by_key(|(typ, _)| *typ);
- let mut prev_type = None;
- for (typ, _) in custom_tlvs.iter() {
- if *typ < 1 << 16 { return Err(()); }
- if *typ == 5482373484 { return Err(()); } // keysend
- if *typ == 77_777 { return Err(()); } // invoice requests for async payments
- match prev_type {
- Some(prev) if prev >= *typ => return Err(()),
- _ => {},
- }
- prev_type = Some(*typ);
- }
- self.custom_tlvs = custom_tlvs;
- Ok(self)
+ pub fn with_custom_tlvs(mut self, custom_tlvs: RecipientCustomTlvs) -> Self {
+ self.custom_tlvs = custom_tlvs.into_inner();
+ self
}
/// Gets the custom TLVs that will be sent or have been received.
@@ -2815,8 +2845,8 @@ mod tests {
use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
use crate::ln::inbound_payment::ExpandedKey;
use crate::ln::outbound_payment::{
- Bolt12PaymentError, OutboundPayments, PendingOutboundPayment, Retry, RetryableSendFailure,
- StaleExpiration,
+ Bolt12PaymentError, OutboundPayments, PendingOutboundPayment, RecipientCustomTlvs, Retry,
+ RetryableSendFailure, StaleExpiration,
};
#[cfg(feature = "std")]
use crate::offers::invoice::DEFAULT_RELATIVE_EXPIRY;
@@ -2843,22 +2873,23 @@ mod tests {
fn test_recipient_onion_fields_with_custom_tlvs() {
let onion_fields = RecipientOnionFields::spontaneous_empty();
- let bad_type_range_tlvs = vec![
+ let bad_type_range_tlvs = RecipientCustomTlvs::new(vec![
(0, vec![42]),
(1, vec![42; 32]),
- ];
- assert!(onion_fields.clone().with_custom_tlvs(bad_type_range_tlvs).is_err());
+ ]);
+ assert!(bad_type_range_tlvs.is_err());
- let keysend_tlv = vec![
+ let keysend_tlv = RecipientCustomTlvs::new(vec![
(5482373484, vec![42; 32]),
- ];
- assert!(onion_fields.clone().with_custom_tlvs(keysend_tlv).is_err());
+ ]);
+ assert!(keysend_tlv.is_err());
- let good_tlvs = vec![
+ let good_tlvs = RecipientCustomTlvs::new(vec![
((1 << 16) + 1, vec![42]),
((1 << 16) + 3, vec![42; 32]),
- ];
- assert!(onion_fields.with_custom_tlvs(good_tlvs).is_ok());
+ ]);
+ assert!(good_tlvs.is_ok());
+ onion_fields.with_custom_tlvs(good_tlvs.unwrap());
}
#[test]
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 1444623..d3be665 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -32,7 +32,7 @@ use crate::ln::msgs;
use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, MessageSendEvent};
use crate::ln::onion_utils::{self, LocalHTLCFailureReason};
use crate::ln::outbound_payment::{
- ProbeSendFailure, Retry, RetryableSendFailure, IDEMPOTENCY_TIMEOUT_TICKS,
+ ProbeSendFailure, RecipientCustomTlvs, Retry, RetryableSendFailure, IDEMPOTENCY_TIMEOUT_TICKS,
};
use crate::ln::types::ChannelId;
use crate::routing::gossip::{EffectiveCapacity, RoutingFees};
@@ -4539,7 +4539,7 @@ fn test_retry_custom_tlvs() {
let custom_tlvs = vec![((1 << 16) + 1, vec![0x42u8; 16])];
let onion = RecipientOnionFields::secret_only(payment_secret);
- let onion = onion.with_custom_tlvs(custom_tlvs.clone()).unwrap();
+ let onion = onion.with_custom_tlvs(RecipientCustomTlvs::new(custom_tlvs.clone()).unwrap());
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
nodes[0].node.send_payment(hash, onion, id, route_params.clone(), Retry::Attempts(1)).unwrap();
@@ -5079,8 +5079,7 @@ fn peel_payment_onion_custom_tlvs() {
let route_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat);
let route = functional_test_utils::get_route(&nodes[0], &route_params).unwrap();
let mut recipient_onion = RecipientOnionFields::spontaneous_empty()
- .with_custom_tlvs(vec![(414141, vec![42; 1200])])
- .unwrap();
+ .with_custom_tlvs(RecipientCustomTlvs::new(vec![(414141, vec![42; 1200])]).unwrap());
let prng_seed = chanmon_cfgs[0].keys_manager.get_secure_random_bytes();
let session_priv = SecretKey::from_slice(&prng_seed[..]).expect("RNG is busted");
let keysend_preimage = PaymentPreimage([42; 32]);
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.