Skip to content

Commit e5a77b4

Browse files
randomloginclaude
andcommitted
refactor(probing)
- Clamp ProbingConfigBuilder::interval to MIN_PROBING_INTERVAL (100ms) in build(). Avoids the tokio::time::interval(Duration::ZERO) panic in run_prober and rules out sub-100ms hot-looping. New constant lives in config.rs alongside DEFAULT_PROBING_INTERVAL_SECS. - Replace .unwrap_or(0) with .expect() on the fetch_update calls in handle_probe_successful / handle_probe_failed. The closure always returns Some, so the Err arm is unreachable; unwrap_or(0) implied a possible failure mode that cannot occur. - Reject RandomStrategy paths whose HTLC bounds force the probe above the user-configured max_amount_msat. Previously the amount could be silently inflated past the user's ceiling. - Document in try_build_path that longer cycles aren't filtered from the random walk; probes fail at the destination by design, so revisiting a node via a different channel is harmless. - Simplify the Debug impl for ProbingConfig by deriving it and giving ProbingStrategyKind its own manual Debug that hides the Custom payload. Replaces a larger hand-written impl on ProbingConfig. - Cache prev.saturating_sub(amount) into `new` in the probe handlers so the log line doesn't recompute it. - Expand ProbingConfig docs with a Caution section noting that stuck intermediate HTLCs can lock outbound liquidity until timeout, and that max_locked_msat is the user-facing backstop for this. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 234ae2f commit e5a77b4

2 files changed

Lines changed: 38 additions & 34 deletions

File tree

src/config.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const DEFAULT_LDK_WALLET_SYNC_INTERVAL_SECS: u64 = 30;
2828
const DEFAULT_FEE_RATE_CACHE_UPDATE_INTERVAL_SECS: u64 = 60 * 10;
2929
const DEFAULT_PROBING_LIQUIDITY_LIMIT_MULTIPLIER: u64 = 3;
3030
pub(crate) const DEFAULT_PROBING_INTERVAL_SECS: u64 = 10;
31+
pub(crate) const MIN_PROBING_INTERVAL: Duration = Duration::from_millis(100);
3132
pub(crate) const DEFAULT_PROBED_NODE_COOLDOWN_SECS: u64 = 60 * 60; // 1 hour
3233
pub(crate) const DEFAULT_MAX_PROBE_LOCKED_MSAT: u64 = 100_000_000; // 100k sats
3334
pub(crate) const MIN_PROBE_AMOUNT_MSAT: u64 = 1_000_000; // 1k sats

src/probing.rs

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ use lightning_invoice::DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA;
2424
use lightning_types::features::{ChannelFeatures, NodeFeatures};
2525

2626
use crate::config::{
27-
DEFAULT_MAX_PROBE_LOCKED_MSAT, DEFAULT_PROBED_NODE_COOLDOWN_SECS, DEFAULT_PROBING_INTERVAL_SECS,
27+
DEFAULT_MAX_PROBE_LOCKED_MSAT, DEFAULT_PROBED_NODE_COOLDOWN_SECS,
28+
DEFAULT_PROBING_INTERVAL_SECS, MIN_PROBING_INTERVAL,
2829
};
2930
use crate::logger::{log_debug, LdkLogger, Logger};
3031
use crate::types::{ChannelManager, Graph, Router};
@@ -40,13 +41,35 @@ pub(crate) enum ProbingStrategyKind {
4041
Custom(Arc<dyn ProbingStrategy>),
4142
}
4243

44+
impl fmt::Debug for ProbingStrategyKind {
45+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
46+
match self {
47+
Self::HighDegree { top_node_count } => {
48+
f.debug_struct("HighDegree").field("top_node_count", top_node_count).finish()
49+
},
50+
Self::Random { max_hops } => {
51+
f.debug_struct("Random").field("max_hops", max_hops).finish()
52+
},
53+
Self::Custom(_) => f.write_str("Custom(<probing strategy>)"),
54+
}
55+
}
56+
}
57+
4358
/// Configuration for the background probing subsystem.
4459
///
4560
/// Construct via [`ProbingConfigBuilder`]. Pick a strategy with
4661
/// [`ProbingConfigBuilder::high_degree`], [`ProbingConfigBuilder::random_walk`], or
4762
/// [`ProbingConfigBuilder::custom`], chain optional setters, and finalize with
4863
/// [`ProbingConfigBuilder::build`].
4964
///
65+
/// # Caution
66+
///
67+
/// Probes send real HTLCs along real paths. If an intermediate hop is offline or
68+
/// misbehaving, the probe HTLC can remain in-flight — locking outbound liquidity
69+
/// on the first-hop channel until the HTLC timeout elapses (potentially hours).
70+
/// `max_locked_msat` caps the total outbound capacity that in-flight probes may
71+
/// hold at any one time; tune it conservatively for nodes with tight liquidity.
72+
///
5073
/// # Example
5174
/// ```ignore
5275
/// let config = ProbingConfigBuilder::high_degree(100)
@@ -56,7 +79,7 @@ pub(crate) enum ProbingStrategyKind {
5679
/// .build();
5780
/// builder.set_probing_config(config);
5881
/// ```
59-
#[derive(Clone)]
82+
#[derive(Clone, Debug)]
6083
#[cfg_attr(feature = "uniffi", derive(uniffi::Object))]
6184
pub struct ProbingConfig {
6285
pub(crate) kind: ProbingStrategyKind,
@@ -66,27 +89,6 @@ pub struct ProbingConfig {
6689
pub(crate) cooldown: Duration,
6790
}
6891

69-
impl fmt::Debug for ProbingConfig {
70-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
71-
let kind_str = match &self.kind {
72-
ProbingStrategyKind::HighDegree { top_node_count } => {
73-
format!("HighDegree {{ top_node_count: {} }}", top_node_count)
74-
},
75-
ProbingStrategyKind::Random { max_hops } => {
76-
format!("Random {{ max_hops: {} }}", max_hops)
77-
},
78-
ProbingStrategyKind::Custom(_) => "Custom(<probing strategy>)".to_string(),
79-
};
80-
f.debug_struct("ProbingConfig")
81-
.field("kind", &kind_str)
82-
.field("interval", &self.interval)
83-
.field("max_locked_msat", &self.max_locked_msat)
84-
.field("diversity_penalty_msat", &self.diversity_penalty_msat)
85-
.field("cooldown", &self.cooldown)
86-
.finish()
87-
}
88-
}
89-
9092
/// Builder for [`ProbingConfig`].
9193
///
9294
/// Pick a strategy with [`high_degree`], [`random_walk`], or [`custom`], chain optional
@@ -178,7 +180,7 @@ impl ProbingConfigBuilder {
178180
pub fn build(&self) -> ProbingConfig {
179181
ProbingConfig {
180182
kind: self.kind.clone(),
181-
interval: self.interval,
183+
interval: self.interval.max(MIN_PROBING_INTERVAL),
182184
max_locked_msat: self.max_locked_msat,
183185
diversity_penalty_msat: self.diversity_penalty_msat,
184186
cooldown: self.cooldown,
@@ -406,10 +408,8 @@ impl ProbingStrategy for HighDegreeStrategy {
406408
///
407409
/// On each tick:
408410
/// 1. Picks one of our confirmed, usable channels to start from.
409-
/// 2. Performs a deterministic walk of a randomly chosen depth (up to
410-
/// [`MAX_PATH_LENGTH_ESTIMATE`]) through the gossip graph, skipping disabled
411-
/// channels and dead-ends.
412-
/// 3. Returns the constructed `Path` so the prober calls `send_probe` directly.
411+
/// 2. Performs a random walk of a chosen depth (up to [`MAX_PATH_LENGTH_ESTIMATE`]) through the
412+
/// gossip graph, skipping disabled channels and dead-ends.
413413
///
414414
/// The probe amount is chosen uniformly at random from `[min_amount_msat, max_amount_msat]`.
415415
///
@@ -484,7 +484,8 @@ impl RandomStrategy {
484484
None => break,
485485
};
486486

487-
// Outward channels: skip the one we arrived on to avoid backtracking.
487+
// Skip the edge we arrived on. Longer cycles aren't filtered — probes fail at
488+
// the destination anyway, so revisiting nodes is harmless.
488489
let candidates: Vec<u64> =
489490
node_info.channels.iter().copied().filter(|&scid| scid != prev_scid).collect();
490491

@@ -542,7 +543,7 @@ impl RandomStrategy {
542543
}
543544
let amount_msat =
544545
amount_msat.max(route_greatest_htlc_lower_bound).min(route_least_htlc_upper_bound);
545-
if amount_msat < self.min_amount_msat {
546+
if amount_msat < self.min_amount_msat || amount_msat > self.max_amount_msat {
546547
return None;
547548
}
548549

@@ -671,13 +672,14 @@ impl Prober {
671672
let prev = self
672673
.locked_msat
673674
.fetch_update(Ordering::AcqRel, Ordering::Acquire, |v| Some(v.saturating_sub(amount)))
674-
.unwrap_or(0);
675+
.expect("fetch_update closure always returns Some");
676+
let new = prev.saturating_sub(amount);
675677
log_debug!(
676678
self.logger,
677679
"Probe successful: released {} msat (locked_msat {} -> {}), path: {}",
678680
amount,
679681
prev,
680-
prev.saturating_sub(amount),
682+
new,
681683
fmt_path(path)
682684
);
683685
}
@@ -687,13 +689,14 @@ impl Prober {
687689
let prev = self
688690
.locked_msat
689691
.fetch_update(Ordering::AcqRel, Ordering::Acquire, |v| Some(v.saturating_sub(amount)))
690-
.unwrap_or(0);
692+
.expect("fetch_update closure always returns Some");
693+
let new = prev.saturating_sub(amount);
691694
log_debug!(
692695
self.logger,
693696
"Probe failed: released {} msat (locked_msat {} -> {}), path: {}",
694697
amount,
695698
prev,
696-
prev.saturating_sub(amount),
699+
new,
697700
fmt_path(path)
698701
);
699702
}

0 commit comments

Comments
 (0)