Skip to content

Properly consider blinded paths in InFlightHtlcs #4072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Properly consider blinded paths in InFlightHtlcs #4072

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
66e59f9
Make `InFlightHtlcs` a named-field struct instead of a tuple struct
TheBlueMatt Sep 15, 2025
a5d0767
Drop `Readable`/`Writeable` for `InFlightHtlcs`
TheBlueMatt Sep 15, 2025
4cbd556
Properly consider blinded paths in `InFlightHtlcs`
TheBlueMatt Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 144 additions & 7 deletions lightning/src/routing/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,18 @@ where
type ScoreParams = <S::Target as ScoreLookUp>::ScoreParams;
#[rustfmt::skip]
fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams) -> u64 {
if let CandidateRouteHop::Blinded(blinded_candidate) = candidate {
if let Some(used_liquidity) = self.inflight_htlcs.used_blinded_liquidity_msat(
*blinded_candidate.source_node_id, blinded_candidate.hint.blinding_point(),
) {
let usage = ChannelUsage {
inflight_htlc_msat: usage.inflight_htlc_msat.saturating_add(used_liquidity),
..usage
};

return self.scorer.channel_penalty_msat(candidate, usage, score_params);
}
}
let target = match candidate.target() {
Some(target) => target,
None => return self.scorer.channel_penalty_msat(candidate, usage, score_params),
Expand Down Expand Up @@ -356,12 +368,16 @@ pub struct InFlightHtlcs {
// key is less than its destination. See `InFlightHtlcs::used_liquidity_msat` for more
// details.
unblinded_hops: HashMap<(u64, bool), u64>,
/// A map with liquidity value (in msat) keyed by the introduction point of a blinded path and
/// the blinding point. In general blinding points should be globally unique, but just in case
/// we add the introduction point as well.
blinded_hops: HashMap<(NodeId, PublicKey), u64>,
}

impl InFlightHtlcs {
/// Constructs an empty `InFlightHtlcs`.
pub fn new() -> Self {
InFlightHtlcs { unblinded_hops: new_hash_map() }
InFlightHtlcs { unblinded_hops: new_hash_map(), blinded_hops: new_hash_map() }
}

/// Takes in a path with payer's node id and adds the path's details to `InFlightHtlcs`.
Expand All @@ -373,6 +389,19 @@ impl InFlightHtlcs {
let mut cumulative_msat = 0;
if let Some(tail) = &path.blinded_tail {
cumulative_msat += tail.final_value_msat;
if tail.hops.len() > 1 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this method should take into account trampoline hops, might be easy to do here

// Single-hop blinded paths aren't really "blinded" paths, as they terminate at the
// introduction point. In that case, we don't need to track anything.
Comment on lines +392 to +394
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm ok I would have thought a single-hop blinded path is [intro] -> [destination]; need to read up further.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, its confusing...

let last_hop = path.hops.last().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last unblinded hop could be the last trampoline hop instead, IIUC

let intro_node = NodeId::from_pubkey(&last_hop.pubkey);
// The amount we send into the blinded path is the sum of the blinded path final
// amount and the fee we pay in it, which is the `fee_msat` of the last hop.
let blinded_path_sent_amt = last_hop.fee_msat + cumulative_msat;
self.blinded_hops
.entry((intro_node, tail.blinding_point))
.and_modify(|used_liquidity_msat| *used_liquidity_msat += blinded_path_sent_amt)
.or_insert(blinded_path_sent_amt);
}
}

// total_inflight_map needs to be direction-sensitive when keeping track of the HTLC value
Expand Down Expand Up @@ -414,6 +443,13 @@ impl InFlightHtlcs {
) -> Option<u64> {
self.unblinded_hops.get(&(channel_scid, source < target)).map(|v| *v)
}

/// Returns liquidity in msat given the blinded path introduction point and blinding point.
pub fn used_blinded_liquidity_msat(
&self, introduction_point: NodeId, blinding_point: PublicKey,
) -> Option<u64> {
self.blinded_hops.get(&(introduction_point, blinding_point)).map(|v| *v)
}
}

/// A hop in a route, and additional metadata about it. "Hop" is defined as a node and the channel
Expand Down Expand Up @@ -3890,8 +3926,9 @@ mod tests {
use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId, P2PGossipSync};
use crate::routing::router::{
add_random_cltv_offset, build_route_from_hops_internal, default_node_features, get_route,
BlindedTail, CandidateRouteHop, InFlightHtlcs, Path, PaymentParameters, PublicHopCandidate,
Route, RouteHint, RouteHintHop, RouteHop, RouteParameters, RoutingFees,
BlindedPathCandidate, BlindedTail, CandidateRouteHop, InFlightHtlcs, Path,
PaymentParameters, PublicHopCandidate, Route, RouteHint, RouteHintHop, RouteHop,
RouteParameters, RoutingFees, ScorerAccountingForInFlightHtlcs,
DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, MAX_PATH_LENGTH_ESTIMATE,
};
use crate::routing::scoring::{
Expand Down Expand Up @@ -3923,7 +3960,7 @@ mod tests {

use crate::io::Cursor;
use crate::prelude::*;
use crate::sync::Arc;
use crate::sync::{Arc, Mutex};

#[rustfmt::skip]
fn get_channel_details(short_channel_id: Option<u64>, node_id: PublicKey,
Expand Down Expand Up @@ -7960,9 +7997,9 @@ mod tests {

#[test]
#[rustfmt::skip]
fn blinded_path_inflight_processing() {
// Ensure we'll score the channel that's inbound to a blinded path's introduction node, and
// account for the blinded tail's final amount_msat.
fn one_hop_blinded_path_inflight_processing() {
// Ensure we'll score the channel that's inbound to a one-hop blinded path's introduction
// node, and account for the blinded tail's final amount_msat.
let mut inflight_htlcs = InFlightHtlcs::new();
let path = Path {
hops: vec![RouteHop {
Expand Down Expand Up @@ -7994,6 +8031,106 @@ mod tests {
inflight_htlcs.process_path(&path, ln_test_utils::pubkey(44));
assert_eq!(*inflight_htlcs.unblinded_hops.get(&(42, true)).unwrap(), 301);
assert_eq!(*inflight_htlcs.unblinded_hops.get(&(43, false)).unwrap(), 201);
assert!(inflight_htlcs.blinded_hops.is_empty());
}

struct UsageTrackingScorer(Mutex<Option<ChannelUsage>>);

impl ScoreLookUp for UsageTrackingScorer {
type ScoreParams = ();
fn channel_penalty_msat(&self, _: &CandidateRouteHop, usage: ChannelUsage, _: &()) -> u64 {
let mut inner = self.0.lock().unwrap();
assert!(inner.is_none());
*inner = Some(usage);
0
}
}

#[test]
fn blinded_path_inflight_processing() {
// Ensure we'll score the channel that's inbound to a blinded path's introduction node, and
// account for the blinded tail's final amount_msat as well as track the blinded path
// in-flight.
let mut inflight_htlcs = InFlightHtlcs::new();
let blinding_point = ln_test_utils::pubkey(48);
let mut blinded_hops = Vec::new();
for i in 0..2 {
blinded_hops.push(BlindedHop {
blinded_node_id: ln_test_utils::pubkey(49 + i as u8),
encrypted_payload: Vec::new(),
});
}
let intro_point = ln_test_utils::pubkey(43);
let path = Path {
hops: vec![
RouteHop {
pubkey: ln_test_utils::pubkey(42),
node_features: NodeFeatures::empty(),
short_channel_id: 42,
channel_features: ChannelFeatures::empty(),
fee_msat: 100,
cltv_expiry_delta: 0,
maybe_announced_channel: false,
},
RouteHop {
pubkey: intro_point,
node_features: NodeFeatures::empty(),
short_channel_id: 43,
channel_features: ChannelFeatures::empty(),
fee_msat: 1,
cltv_expiry_delta: 0,
maybe_announced_channel: false,
},
],
blinded_tail: Some(BlindedTail {
trampoline_hops: vec![],
hops: blinded_hops.clone(),
blinding_point,
excess_final_cltv_expiry_delta: 0,
final_value_msat: 200,
}),
};
inflight_htlcs.process_path(&path, ln_test_utils::pubkey(44));
assert_eq!(*inflight_htlcs.unblinded_hops.get(&(42, true)).unwrap(), 301);
assert_eq!(*inflight_htlcs.unblinded_hops.get(&(43, false)).unwrap(), 201);
let intro_node_id = NodeId::from_pubkey(&ln_test_utils::pubkey(43));
assert_eq!(
*inflight_htlcs.blinded_hops.get(&(intro_node_id, blinding_point)).unwrap(),
201
);

let tracking_scorer = UsageTrackingScorer(Mutex::new(None));
let inflight_scorer =
ScorerAccountingForInFlightHtlcs::new(&tracking_scorer, &inflight_htlcs);

let blinded_payinfo = BlindedPayInfo {
fee_base_msat: 100,
fee_proportional_millionths: 500,
htlc_minimum_msat: 1000,
htlc_maximum_msat: 100_000_000,
cltv_expiry_delta: 15,
features: BlindedHopFeatures::empty(),
};
let blinded_path = BlindedPaymentPath::from_blinded_path_and_payinfo(
intro_point,
blinding_point,
blinded_hops,
blinded_payinfo,
);

let candidate = CandidateRouteHop::Blinded(BlindedPathCandidate {
source_node_id: &intro_node_id,
hint: &blinded_path,
hint_idx: 0,
source_node_counter: 0,
});
let empty_usage = ChannelUsage {
amount_msat: 42,
inflight_htlc_msat: 0,
effective_capacity: EffectiveCapacity::HintMaxHTLC { amount_msat: 500 },
};
inflight_scorer.channel_penalty_msat(&candidate, empty_usage, &());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this value be checked?

assert_eq!(tracking_scorer.0.lock().unwrap().unwrap().inflight_htlc_msat, 201);
}

#[test]
Expand Down
13 changes: 12 additions & 1 deletion lightning/src/routing/scoring.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use crate::prelude::hash_map::Entry;
use crate::prelude::*;
use crate::routing::gossip::{DirectedChannelInfo, EffectiveCapacity, NetworkGraph, NodeId};
use crate::routing::log_approx;
use crate::routing::router::{CandidateRouteHop, Path, PublicHopCandidate};
use crate::routing::router::{BlindedPathCandidate, CandidateRouteHop, Path, PublicHopCandidate};
use crate::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
use crate::util::logger::Logger;
use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer};
Expand Down Expand Up @@ -1682,6 +1682,17 @@ where
CandidateRouteHop::PublicHop(PublicHopCandidate { info, short_channel_id }) => {
(short_channel_id, info.target())
},
CandidateRouteHop::Blinded(BlindedPathCandidate { hint, .. }) => {
let total_inflight_amount_msat =
usage.amount_msat.saturating_add(usage.inflight_htlc_msat);
if usage.amount_msat > hint.payinfo.htlc_maximum_msat {
return u64::max_value();
} else if total_inflight_amount_msat > hint.payinfo.htlc_maximum_msat {
return score_params.considered_impossible_penalty_msat;
Comment on lines +1688 to +1691
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In both cases, total amount flowing over the channel exceeds our current estimate of the channel's available liquidity, so why return u64::max_value() vs score_params.considered_impossible_penalty_msat ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is consistent with what we do elsewhere - we only use u64::MAX when the amount for this HTLC is too large (which shouldn't happen, the router should never try to do that) whereas we use considered_impossible_penalty_msat if we have existing HTLCs that might saturate a channel. considered_impossible_penalty_msat is high enough that we should always prefer any other possible route, but allows us to still try to send if we don't have any other option. In the case of in-flight HTLCs pushing us over a limit, we might as well try cause those other attempts may fail before the new HTLC gets there.

Of course for blinded paths it maybe shouldn't matter anyway - the blinded path candidates should be within the context of a single payment (because we should have a unique blinded path for each payment) so if we have no option but to over-saturate a blinded path probably we're screwed. Its still worth trying, imo, because the recipient may have given us a blinded path with a too-low htlc_maximum_msat (ie cause they rounded-down for privacy) or could have re-used a blinded path across payments.

} else {
return 0;
}
},
_ => return 0,
};
let source = candidate.source();
Expand Down
Loading