Skip to content

Commit a635fb3

Browse files
committed
Deduce probing state from channel manager
Previously we tried to store the total amount of funds locked and/or exact probes in flight which was difficult to persist and restore after restart. Now it is completely deduced from the channel manager.
1 parent 6e06fab commit a635fb3

3 files changed

Lines changed: 38 additions & 57 deletions

File tree

src/builder.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use std::convert::TryInto;
1010
use std::default::Default;
1111
use std::net::ToSocketAddrs;
1212
use std::path::PathBuf;
13-
use std::sync::atomic::AtomicU64;
1413
use std::sync::{Arc, Mutex, Once, RwLock};
1514
use std::time::SystemTime;
1615
use std::{fmt, fs};
@@ -1146,7 +1145,9 @@ impl ArcedNodeBuilder {
11461145

11471146
/// Configures background probing.
11481147
///
1149-
/// See [`ProbingConfig`] for details.
1148+
/// Use [`ProbingConfigBuilder`] to build the configuration.
1149+
///
1150+
/// [`ProbingConfigBuilder`]: crate::probing::ProbingConfigBuilder
11501151
pub fn set_probing_config(&self, config: Arc<ProbingConfig>) {
11511152
self.inner.write().expect("lock").set_probing_config((*config).clone());
11521153
}
@@ -2111,8 +2112,6 @@ fn build_with_store_internal(
21112112
strategy,
21122113
interval: probing_cfg.interval,
21132114
max_locked_msat: probing_cfg.max_locked_msat,
2114-
locked_msat: Arc::new(AtomicU64::new(0)),
2115-
inflight_probes: Mutex::new(HashMap::new()),
21162115
})
21172116
});
21182117

src/event.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,20 +1191,12 @@ where
11911191
LdkEvent::PaymentPathFailed { .. } => {},
11921192
LdkEvent::ProbeSuccessful { path, payment_id, .. } => {
11931193
if let Some(prober) = &self.prober {
1194-
if let Some(amount) =
1195-
prober.inflight_probes.lock().expect("lock").remove(&payment_id)
1196-
{
1197-
prober.handle_background_probe_successful(&path, amount);
1198-
}
1194+
prober.handle_background_probe_successful(&path, payment_id);
11991195
}
12001196
},
12011197
LdkEvent::ProbeFailed { path, payment_id, .. } => {
12021198
if let Some(prober) = &self.prober {
1203-
if let Some(amount) =
1204-
prober.inflight_probes.lock().expect("lock").remove(&payment_id)
1205-
{
1206-
prober.handle_background_probe_failed(&path, amount);
1207-
}
1199+
prober.handle_background_probe_failed(&path, payment_id);
12081200
}
12091201
},
12101202
LdkEvent::HTLCHandlingFailed { failure_type, .. } => {

src/probing.rs

Lines changed: 33 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,15 @@
6262
6363
use std::collections::HashMap;
6464
use std::fmt;
65-
use std::sync::atomic::{AtomicU64, Ordering};
6665
#[cfg(feature = "uniffi")]
6766
use std::sync::RwLock;
6867
use std::sync::{Arc, Mutex};
6968
use std::time::{Duration, Instant};
7069

7170
use bitcoin::secp256k1::PublicKey;
72-
use lightning::ln::channelmanager::PaymentId;
71+
use lightning::ln::channelmanager::{PaymentId, RecentPaymentDetails};
7372
use lightning::routing::gossip::NodeId;
73+
use lightning::routing::router::Router as LdkRouter;
7474
use lightning::routing::router::{
7575
Path, PaymentParameters, RouteHop, RouteParameters, MAX_PATH_LENGTH_ESTIMATE,
7676
};
@@ -85,8 +85,6 @@ use crate::logger::{log_debug, LdkLogger, Logger};
8585
use crate::types::{ChannelManager, Graph, Router};
8686
use crate::util::random_range;
8787

88-
use lightning::routing::router::Router as LdkRouter;
89-
9088
/// Which built-in probing strategy to use, or a custom one.
9189
#[derive(Clone)]
9290
pub(crate) enum ProbingStrategyKind {
@@ -287,7 +285,7 @@ pub struct ArcedProbingConfigBuilder {
287285
#[cfg(feature = "uniffi")]
288286
#[uniffi::export]
289287
impl ArcedProbingConfigBuilder {
290-
/// Creates a builder configured to probe toward the highest-degree nodes in the graph.
288+
/// Start building a config that probes toward the highest-degree nodes in the graph.
291289
///
292290
/// `top_node_count` controls how many of the most-connected nodes are cycled through.
293291
#[uniffi::constructor]
@@ -297,7 +295,7 @@ impl ArcedProbingConfigBuilder {
297295
})
298296
}
299297

300-
/// Creates a builder configured to probe via random graph walks.
298+
/// Start building a config that probes via random graph walks.
301299
///
302300
/// `max_hops` is the upper bound on the number of hops in a randomly constructed path.
303301
/// Values below `2` are clamped to `2`.
@@ -306,7 +304,9 @@ impl ArcedProbingConfigBuilder {
306304
Arc::new(Self { inner: RwLock::new(ProbingConfigBuilder::random_walk(max_hops as usize)) })
307305
}
308306

309-
/// Overrides the interval between probe attempts. Defaults to 10 seconds.
307+
/// Overrides the interval between probe attempts.
308+
///
309+
/// Defaults to 10 seconds.
310310
pub fn set_interval(&self, secs: u64) {
311311
self.inner.write().expect("lock").interval(Duration::from_secs(secs));
312312
}
@@ -324,14 +324,18 @@ impl ArcedProbingConfigBuilder {
324324
/// encouraging path diversity during background probing. The penalty decays
325325
/// quadratically over 24 hours.
326326
///
327+
/// This is only useful for probing strategies that route through the scorer
328+
/// (e.g., [`HighDegreeStrategy`]). Strategies that build paths manually
329+
/// (e.g., [`RandomStrategy`]) bypass the scorer entirely.
330+
///
327331
/// If unset, LDK's default of `0` (no penalty) is used.
328332
pub fn set_diversity_penalty_msat(&self, penalty_msat: u64) {
329333
self.inner.write().expect("lock").diversity_penalty_msat(penalty_msat);
330334
}
331335

332336
/// Sets how long a probed node stays ineligible before being probed again.
333337
///
334-
/// Only applies to the high-degree strategy. Defaults to 1 hour.
338+
/// Only applies to [`HighDegreeStrategy`]. Defaults to 1 hour.
335339
pub fn set_cooldown(&self, secs: u64) {
336340
self.inner.write().expect("lock").cooldown(Duration::from_secs(secs));
337341
}
@@ -737,8 +741,6 @@ pub struct Prober {
737741
pub interval: Duration,
738742
/// Maximum total millisatoshis that may be locked in in-flight probes at any time.
739743
pub max_locked_msat: u64,
740-
pub(crate) locked_msat: Arc<AtomicU64>,
741-
pub(crate) inflight_probes: Mutex<HashMap<PaymentId, u64>>,
742744
}
743745

744746
fn fmt_path(path: &lightning::routing::router::Path) -> String {
@@ -752,37 +754,33 @@ fn fmt_path(path: &lightning::routing::router::Path) -> String {
752754
impl Prober {
753755
/// Returns the total millisatoshis currently locked in in-flight probes.
754756
pub fn locked_msat(&self) -> u64 {
755-
self.locked_msat.load(Ordering::Relaxed)
757+
return self
758+
.channel_manager
759+
.list_recent_payments()
760+
.into_iter()
761+
.filter_map(|p| match p {
762+
RecentPaymentDetails::Pending { is_probe: true, total_msat, .. } => {
763+
Some(total_msat)
764+
},
765+
_ => None,
766+
})
767+
.sum();
756768
}
757769

758-
pub(crate) fn handle_background_probe_successful(&self, path: &Path, amount: u64) {
759-
let prev = self
760-
.locked_msat
761-
.fetch_update(Ordering::AcqRel, Ordering::Acquire, |v| Some(v.saturating_sub(amount)))
762-
.expect("fetch_update closure always returns Some");
763-
let new = prev.saturating_sub(amount);
770+
pub(crate) fn handle_background_probe_successful(&self, path: &Path, payment_id: PaymentId) {
764771
log_debug!(
765772
self.logger,
766-
"Probe successful: released {} msat (locked_msat {} -> {}), path: {}",
767-
amount,
768-
prev,
769-
new,
773+
"Background probe with payment_id: {} succeeded along the path: {}",
774+
payment_id,
770775
fmt_path(path)
771776
);
772777
}
773778

774-
pub(crate) fn handle_background_probe_failed(&self, path: &Path, amount: u64) {
775-
let prev = self
776-
.locked_msat
777-
.fetch_update(Ordering::AcqRel, Ordering::Acquire, |v| Some(v.saturating_sub(amount)))
778-
.expect("fetch_update closure always returns Some");
779-
let new = prev.saturating_sub(amount);
779+
pub(crate) fn handle_background_probe_failed(&self, path: &Path, payment_id: PaymentId) {
780780
log_debug!(
781781
self.logger,
782-
"Probe failed: released {} msat (locked_msat {} -> {}), path: {}",
783-
amount,
784-
prev,
785-
new,
782+
"Background probe with payment_id: {} failed along the path: {}",
783+
payment_id,
786784
fmt_path(path)
787785
);
788786
}
@@ -806,32 +804,24 @@ pub(crate) async fn run_prober(prober: Arc<Prober>, mut stop_rx: tokio::sync::wa
806804
None => continue,
807805
};
808806
let amount: u64 = path.hops.iter().map(|h| h.fee_msat).sum();
809-
if prober.locked_msat.load(Ordering::Acquire) + amount > prober.max_locked_msat {
807+
if prober.locked_msat() + amount > prober.max_locked_msat {
810808
log_debug!(prober.logger, "Skipping probe: locked-msat budget exceeded.");
811809
continue;
812810
}
813-
// Hold `inflight_probes` across `send_probe` so the event handler in
814-
// `event.rs` (which acquires the same lock to remove the entry) cannot
815-
// observe a `ProbeSuccessful`/`ProbeFailed` for a payment_id we have not
816-
// yet inserted, which would leave `locked_msat` permanently incremented.
817-
let mut inflight = prober.inflight_probes.lock().expect("lock");
818811
match prober.channel_manager.send_probe(path.clone()) {
819812
Ok((_, payment_id)) => {
820-
inflight.insert(payment_id, amount);
821-
prober.locked_msat.fetch_add(amount, Ordering::Release);
822-
drop(inflight);
823813
log_debug!(
824814
prober.logger,
825-
"Probe sent: locked {} msat, path: {}",
815+
"Background probe with payment_id {} sent: locked {} msat, path: {}",
816+
payment_id,
826817
amount,
827818
fmt_path(&path)
828819
);
829820
}
830821
Err(e) => {
831-
drop(inflight);
832822
log_debug!(
833823
prober.logger,
834-
"Probe send failed: {:?}, path: {}",
824+
"Background probe send failed: {:?}, path: {}",
835825
e,
836826
fmt_path(&path)
837827
);

0 commit comments

Comments
 (0)