← Watch feed
Make `AttributionData` actually pub since its used in the public API
Public commit record
85/100 · Strong
What the developer wrote
Authored by Matt Corallo
Make `AttributionData` actually pub since its used in the public API
`AttributionData` is a part of the public `UpdateFulfillHTLC` and
`UpdateFailHTLC` messages, but its not actually `pub`. Yet again
re-exports bite us and leave us with a broken public API - we
ended up accidentally sealing `AttributionData`.
Instead, here, we just make `onion_utils` `pub` so that we avoid
making the same mistake in the future.
Note that this still leaves us with arather useless public
`AttributionData` API - it can't be created, updated, or decoded,
it can only be serialized and deserialized, but at least it exists.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This commit fixes a Rust library API visibility problem. A data type called AttributionData was supposed to be publicly usable because it appears in public Lightning protocol messages, but it was accidentally kept internal due to how Rust re-exports work. The change makes the containing module public so users can actually reference the type. It is a usability/API completeness fix, not a security vulnerability fix.
Recommended action
No security action required. Treat as a normal API-correctness patch. Downstream users who previously could not name AttributionData directly can now do so via lightning::ln::onion_utils::AttributionData.
Security signals we found
01
API visibility fix for a type referenced in public protocol messages
02
No change to cryptographic logic, parsing, or validation behavior
03
Constants and helper methods narrowed from pub to pub(crate), reducing public surface
04
Fuzzing-only functions remain gated behind #[cfg(fuzzing)]
Evidence from the diff
The commit changes lightning/src/ln/onion_utils from pub(crate) to pub and removes conditional pub re-exports in lightning/src/ln/mod.rs. AttributionData, used in public UpdateFulfillHTLC and UpdateFailHTLC message fields, was effectively sealed (not nameable by downstream crates). The patch exposes onion_utils publicly, adds a module-level doc comment, makes several constants pub(crate) instead of pub, and moves process_onion_failure and decode_fulfill_attribution_data behind a #[cfg(fuzzing)] internal module. The test in msgs.rs switches from constructing AttributionData with raw arrays to using AttributionData::new(), and the expected hex encoding changes from filled 3s to zeros because the constructor initializes to zero.
Changed components
lightning/src/ln/onion_utils.rslightning/src/ln/mod.rslightning/src/ln/msgs.rslightning/src/events/mod.rsfuzz/src/process_onion_failure.rsInspect captured patch +97 / −96
diff --git a/fuzz/src/process_onion_failure.rs b/fuzz/src/process_onion_failure.rs
index 1bc9900..ac70562 100644
--- a/fuzz/src/process_onion_failure.rs
+++ b/fuzz/src/process_onion_failure.rs
@@ -9,10 +9,12 @@ use lightning::{
ln::{
channelmanager::{HTLCSource, PaymentId},
msgs::OnionErrorPacket,
+ onion_utils,
},
routing::router::{BlindedTail, Path, RouteHop, TrampolineHop},
types::features::{ChannelFeatures, NodeFeatures},
util::logger::Logger,
+ util::ser::Readable,
};
// Imports that need to be added manually
@@ -126,19 +128,18 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
let failure_data = get_slice!(failure_len);
let attribution_data = if get_bool!() {
- Some(lightning::ln::AttributionData {
- hold_times: get_slice!(80).try_into().unwrap(),
- hmacs: get_slice!(840).try_into().unwrap(),
- })
+ let mut bytes = get_slice!(80 + 840);
+ let data: onion_utils::AttributionData = Readable::read(&mut bytes).unwrap();
+ Some(data)
} else {
None
};
let encrypted_packet =
OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
- lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
+ onion_utils::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
if let Some(attribution_data) = attribution_data {
- lightning::ln::decode_fulfill_attribution_data(
+ onion_utils::decode_fulfill_attribution_data(
&secp_ctx,
&logger,
&path,
diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index b9c4b1c..d97ae60 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -25,8 +25,9 @@ use crate::blinded_path::payment::{
use crate::chain::transaction;
use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS;
use crate::ln::channelmanager::{InterceptId, PaymentId, RecipientOnionFields};
+use crate::ln::msgs;
+use crate::ln::onion_utils::LocalHTLCFailureReason;
use crate::ln::types::ChannelId;
-use crate::ln::{msgs, LocalHTLCFailureReason};
use crate::offers::invoice::Bolt12Invoice;
use crate::offers::invoice_request::InvoiceRequest;
use crate::offers::static_invoice::StaticInvoice;
diff --git a/lightning/src/ln/mod.rs b/lightning/src/ln/mod.rs
index 9473142..04aa818 100644
--- a/lightning/src/ln/mod.rs
+++ b/lightning/src/ln/mod.rs
@@ -41,26 +41,17 @@ pub mod channel;
#[cfg(not(fuzzing))]
pub(crate) mod channel;
-pub(crate) mod onion_utils;
+pub mod onion_utils;
mod outbound_payment;
pub mod wire;
#[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled.
pub(crate) mod interactivetxs;
-pub use onion_utils::{create_payment_onion, LocalHTLCFailureReason};
// Older rustc (which we support) refuses to let us call the get_payment_preimage_hash!() macro
// without the node parameter being mut. This is incorrect, and thus newer rustcs will complain
// about an unnecessary mut. Thus, we silence the unused_mut warning in two test modules below.
-#[cfg(fuzzing)]
-pub use onion_utils::decode_fulfill_attribution_data;
-#[cfg(fuzzing)]
-pub use onion_utils::process_onion_failure;
-
-#[cfg(fuzzing)]
-pub use onion_utils::AttributionData;
-
#[cfg(test)]
#[allow(unused_mut)]
mod async_payments_tests;
diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
index 0484ebe..f237d73 100644
--- a/lightning/src/ln/msgs.rs
+++ b/lightning/src/ln/msgs.rs
@@ -4366,7 +4366,7 @@ mod tests {
InboundOnionForwardPayload, InboundOnionReceivePayload, OutboundTrampolinePayload,
TrampolineOnionPacket,
};
- use crate::ln::onion_utils::{AttributionData, HMAC_COUNT, HMAC_LEN, HOLD_TIME_LEN, MAX_HOPS};
+ use crate::ln::onion_utils::AttributionData;
use crate::ln::types::ChannelId;
use crate::routing::gossip::{NodeAlias, NodeId};
use crate::types::features::{
@@ -5899,13 +5899,10 @@ mod tests {
channel_id: ChannelId::from_bytes([2; 32]),
htlc_id: 2316138423780173,
reason: [1; 32].to_vec(),
- attribution_data: Some(AttributionData {
- hold_times: [3; MAX_HOPS * HOLD_TIME_LEN],
- hmacs: [3; HMAC_LEN * HMAC_COUNT],
- }),
+ attribution_data: Some(AttributionData::new()),
};
let encoded_value = update_fail_htlc.encode();
- let target_value = <Vec<u8>>::from_hex("020202020202020202020202020202020202020202020202020202020202020200083a840000034d0020010101010101010101010101010101010101010101010101010101010101010101fd03980303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303").unwrap();
+ let target_value = <Vec<u8>>::from_hex("020202020202020202020202020202020202020202020202020202020202020200083a840000034d0020010101010101010101010101010101010101010101010101010101010101010101fd03980000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000").unwrap();
assert_eq!(encoded_value, target_value);
}
diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs
index 18aa43e..dbc2ebc 100644
--- a/lightning/src/ln/onion_utils.rs
+++ b/lightning/src/ln/onion_utils.rs
@@ -7,6 +7,8 @@
// You may not use this file except in accordance with one or both of these
// licenses.
+//! Low-level onion manipulation logic and fields
+
use super::msgs::OnionErrorPacket;
use crate::blinded_path::BlindedHop;
use crate::crypto::chacha20::ChaCha20;
@@ -979,27 +981,79 @@ mod fuzzy_onion_utils {
#[cfg(test)]
pub(crate) attribution_failed_channel: Option<u64>,
}
+
+ pub fn process_onion_failure<T: secp256k1::Signing, L: Deref>(
+ secp_ctx: &Secp256k1<T>, logger: &L, htlc_source: &HTLCSource,
+ encrypted_packet: OnionErrorPacket,
+ ) -> DecodedOnionFailure
+ where
+ L::Target: Logger,
+ {
+ let (path, session_priv) = match htlc_source {
+ HTLCSource::OutboundRoute { ref path, ref session_priv, .. } => (path, session_priv),
+ _ => unreachable!(),
+ };
+
+ process_onion_failure_inner(secp_ctx, logger, path, &session_priv, None, encrypted_packet)
+ }
+
+ /// Decodes the attribution data that we got back from upstream on a payment we sent.
+ pub fn decode_fulfill_attribution_data<T: secp256k1::Signing, L: Deref>(
+ secp_ctx: &Secp256k1<T>, logger: &L, path: &Path, outer_session_priv: &SecretKey,
+ mut attribution_data: AttributionData,
+ ) -> Vec<u32>
+ where
+ L::Target: Logger,
+ {
+ let mut hold_times = Vec::new();
+
+ // Only consider hops in the regular path for attribution data. Blinded path attribution data isn't accessible.
+ let shared_secrets =
+ construct_onion_keys_generic(secp_ctx, &path.hops, None, outer_session_priv)
+ .map(|(shared_secret, _, _, _, _)| shared_secret);
+
+ // Path length can reach 27 hops, but attribution data can only be conveyed back to the sender from the first 20
+ // hops. Determine the number of hops to be used for attribution data.
+ let attributable_hop_count = usize::min(path.hops.len(), MAX_HOPS);
+
+ for (route_hop_idx, shared_secret) in
+ shared_secrets.enumerate().take(attributable_hop_count)
+ {
+ attribution_data.crypt(shared_secret.as_ref());
+
+ // Calculate position relative to the last attributable hop. The last attributable hop is at position 0. We need
+ // to look at the chain of HMACs that does include all data up to the last attributable hop. Hold times beyond
+ // the last attributable hop will not be available.
+ let position = attributable_hop_count - route_hop_idx - 1;
+ let res = attribution_data.verify(&Vec::new(), shared_secret.as_ref(), position);
+ match res {
+ Ok(hold_time) => {
+ hold_times.push(hold_time);
+
+ // Shift attribution data to prepare for processing the next hop.
+ attribution_data.shift_left();
+ },
+ Err(()) => {
+ // We will hit this if there is a node on the path that does not support fulfill attribution data.
+ log_debug!(
+ logger,
+ "Invalid fulfill HMAC in attribution data for node at pos {}",
+ route_hop_idx
+ );
+
+ break;
+ },
+ }
+ }
+
+ hold_times
+ }
}
#[cfg(fuzzing)]
pub use self::fuzzy_onion_utils::*;
#[cfg(not(fuzzing))]
pub(crate) use self::fuzzy_onion_utils::*;
-pub fn process_onion_failure<T: secp256k1::Signing, L: Deref>(
- secp_ctx: &Secp256k1<T>, logger: &L, htlc_source: &HTLCSource,
- encrypted_packet: OnionErrorPacket,
-) -> DecodedOnionFailure
-where
- L::Target: Logger,
-{
- let (path, session_priv) = match htlc_source {
- HTLCSource::OutboundRoute { ref path, ref session_priv, .. } => (path, session_priv),
- _ => unreachable!(),
- };
-
- process_onion_failure_inner(secp_ctx, logger, path, &session_priv, None, encrypted_packet)
-}
-
/// Process failure we got back from upstream on a payment we sent (implying htlc_source is an
/// OutboundRoute).
fn process_onion_failure_inner<T: secp256k1::Signing, L: Deref>(
@@ -1449,56 +1503,6 @@ where
}
}
-/// Decodes the attribution data that we got back from upstream on a payment we sent.
-pub fn decode_fulfill_attribution_data<T: secp256k1::Signing, L: Deref>(
- secp_ctx: &Secp256k1<T>, logger: &L, path: &Path, outer_session_priv: &SecretKey,
- mut attribution_data: AttributionData,
-) -> Vec<u32>
-where
- L::Target: Logger,
-{
- let mut hold_times = Vec::new();
-
- // Only consider hops in the regular path for attribution data. Blinded path attribution data isn't accessible.
- let shared_secrets =
- construct_onion_keys_generic(secp_ctx, &path.hops, None, outer_session_priv)
- .map(|(shared_secret, _, _, _, _)| shared_secret);
-
- // Path length can reach 27 hops, but attribution data can only be conveyed back to the sender from the first 20
- // hops. Determine the number of hops to be used for attribution data.
- let attributable_hop_count = usize::min(path.hops.len(), MAX_HOPS);
-
- for (route_hop_idx, shared_secret) in shared_secrets.enumerate().take(attributable_hop_count) {
- attribution_data.crypt(shared_secret.as_ref());
-
- // Calculate position relative to the last attributable hop. The last attributable hop is at position 0. We need
- // to look at the chain of HMACs that does include all data up to the last attributable hop. Hold times beyond
- // the last attributable hop will not be available.
- let position = attributable_hop_count - route_hop_idx - 1;
- let res = attribution_data.verify(&Vec::new(), shared_secret.as_ref(), position);
- match res {
- Ok(hold_time) => {
- hold_times.push(hold_time);
-
- // Shift attribution data to prepare for processing the next hop.
- attribution_data.shift_left();
- },
- Err(()) => {
- // We will hit this if there is a node on the path that does not support fulfill attribution data.
- log_debug!(
- logger,
- "Invalid fulfill HMAC in attribution data for node at pos {}",
- route_hop_idx
- );
-
- break;
- },
- }
- }
-
- hold_times
-}
-
const BADONION: u16 = 0x8000;
const PERM: u16 = 0x4000;
const NODE: u16 = 0x2000;
@@ -2522,6 +2526,7 @@ where
}
/// Build a payment onion, returning the first hop msat and cltv values as well.
+///
/// `cur_block_height` should be set to the best known block height + 1.
pub fn create_payment_onion<T: secp256k1::Signing>(
secp_ctx: &Secp256k1<T>, path: &Path, session_priv: &SecretKey, total_msat: u64,
@@ -2711,22 +2716,28 @@ fn decode_next_hop<T, R: ReadableArgs<T>, N: NextPacketBytes>(
}
}
-pub const HOLD_TIME_LEN: usize = 4;
-pub const MAX_HOPS: usize = 20;
-pub const HMAC_LEN: usize = 4;
+pub(crate) const HOLD_TIME_LEN: usize = 4;
+pub(crate) const MAX_HOPS: usize = 20;
+pub(crate) const HMAC_LEN: usize = 4;
// Define the number of HMACs in the attributable data block. For the first node, there are 20 HMACs, and then for every
// subsequent node, the number of HMACs decreases by 1. 20 + 19 + 18 + ... + 1 = 20 * 21 / 2 = 210.
-pub const HMAC_COUNT: usize = MAX_HOPS * (MAX_HOPS + 1) / 2;
+pub(crate) const HMAC_COUNT: usize = MAX_HOPS * (MAX_HOPS + 1) / 2;
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
+/// Attribution data allows the sender of an HTLC to identify which hop failed an HTLC robustly,
+/// preventing earlier hops from corrupting the HTLC failure information (or at least allowing the
+/// sender to identify the earliest hop which corrupted HTLC failure information).
+///
+/// Additionally, it allows a sender to identify how long each hop along a path held an HTLC, with
+/// 100ms granularity.
pub struct AttributionData {
- pub hold_times: [u8; MAX_HOPS * HOLD_TIME_LEN],
- pub hmacs: [u8; HMAC_LEN * HMAC_COUNT],
+ hold_times: [u8; MAX_HOPS * HOLD_TIME_LEN],
+ hmacs: [u8; HMAC_LEN * HMAC_COUNT],
}
impl AttributionData {
- pub fn new() -> Self {
+ pub(crate) fn new() -> Self {
Self { hold_times: [0; MAX_HOPS * HOLD_TIME_LEN], hmacs: [0; HMAC_LEN * HMAC_COUNT] }
}
}
@@ -2775,7 +2786,7 @@ impl AttributionData {
/// Writes the HMACs corresponding to the given position that have been added already by downstream hops. Position is
/// relative to the final node. The final node is at position 0.
- pub fn write_downstream_hmacs(&self, position: usize, w: &mut HmacEngine<Sha256>) {
+ pub(crate) fn write_downstream_hmacs(&self, position: usize, w: &mut HmacEngine<Sha256>) {
// Set the index to the first downstream HMAC that we need to include. Note that we skip the first MAX_HOPS HMACs
// because this is space reserved for the HMACs that we are producing for the current node.
let mut hmac_idx = MAX_HOPS + MAX_HOPS - position - 1;
Risk score
Our methodology →Why this scored 19/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.