Skip to content

Commit 8136a98

Browse files
committed
Distinguish between background and user probes
Previously when calculated currently locked amount, we didn't account for preflight probes sent on a payment which could result in an incorrect value of probe locked_msat. Now Prober saves the PaymentId of probes it sent and tracks them on release, ignoring the user-sent ones.
1 parent 172ca85 commit 8136a98

3 files changed

Lines changed: 26 additions & 9 deletions

File tree

src/builder.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2106,6 +2106,7 @@ fn build_with_store_internal(
21062106
interval: probing_cfg.interval,
21072107
max_locked_msat: probing_cfg.max_locked_msat,
21082108
locked_msat: Arc::new(AtomicU64::new(0)),
2109+
inflight_probes: Mutex::new(HashMap::new()),
21092110
})
21102111
});
21112112

src/event.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,14 +1189,22 @@ where
11891189

11901190
LdkEvent::PaymentPathSuccessful { .. } => {},
11911191
LdkEvent::PaymentPathFailed { .. } => {},
1192-
LdkEvent::ProbeSuccessful { path, .. } => {
1192+
LdkEvent::ProbeSuccessful { path, payment_id, .. } => {
11931193
if let Some(prober) = &self.prober {
1194-
prober.handle_probe_successful(&path);
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+
}
11951199
}
11961200
},
1197-
LdkEvent::ProbeFailed { path, .. } => {
1201+
LdkEvent::ProbeFailed { path, payment_id, .. } => {
11981202
if let Some(prober) = &self.prober {
1199-
prober.handle_probe_failed(&path);
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+
}
12001208
}
12011209
},
12021210
LdkEvent::HTLCHandlingFailed { failure_type, .. } => {

src/probing.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use std::sync::{Arc, Mutex};
1616
use std::time::{Duration, Instant};
1717

1818
use bitcoin::secp256k1::PublicKey;
19+
use lightning::ln::channelmanager::PaymentId;
1920
use lightning::routing::gossip::NodeId;
2021
use lightning::routing::router::{
2122
Path, PaymentParameters, RouteHop, RouteParameters, MAX_PATH_LENGTH_ESTIMATE,
@@ -654,6 +655,7 @@ pub struct Prober {
654655
/// Maximum total millisatoshis that may be locked in in-flight probes at any time.
655656
pub max_locked_msat: u64,
656657
pub(crate) locked_msat: Arc<AtomicU64>,
658+
pub(crate) inflight_probes: Mutex<HashMap<PaymentId, u64>>,
657659
}
658660

659661
fn fmt_path(path: &lightning::routing::router::Path) -> String {
@@ -670,8 +672,7 @@ impl Prober {
670672
self.locked_msat.load(Ordering::Relaxed)
671673
}
672674

673-
pub(crate) fn handle_probe_successful(&self, path: &lightning::routing::router::Path) {
674-
let amount: u64 = path.hops.iter().map(|h| h.fee_msat).sum();
675+
pub(crate) fn handle_background_probe_successful(&self, path: &Path, amount: u64) {
675676
let prev = self
676677
.locked_msat
677678
.fetch_update(Ordering::AcqRel, Ordering::Acquire, |v| Some(v.saturating_sub(amount)))
@@ -687,8 +688,7 @@ impl Prober {
687688
);
688689
}
689690

690-
pub(crate) fn handle_probe_failed(&self, path: &lightning::routing::router::Path) {
691-
let amount: u64 = path.hops.iter().map(|h| h.fee_msat).sum();
691+
pub(crate) fn handle_background_probe_failed(&self, path: &Path, amount: u64) {
692692
let prev = self
693693
.locked_msat
694694
.fetch_update(Ordering::AcqRel, Ordering::Acquire, |v| Some(v.saturating_sub(amount)))
@@ -727,9 +727,16 @@ pub(crate) async fn run_prober(prober: Arc<Prober>, mut stop_rx: tokio::sync::wa
727727
log_debug!(prober.logger, "Skipping probe: locked-msat budget exceeded.");
728728
continue;
729729
}
730+
// Hold `inflight_probes` across `send_probe` so the event handler in
731+
// `event.rs` (which acquires the same lock to remove the entry) cannot
732+
// observe a `ProbeSuccessful`/`ProbeFailed` for a payment_id we have not
733+
// yet inserted, which would leave `locked_msat` permanently incremented.
734+
let mut inflight = prober.inflight_probes.lock().expect("lock");
730735
match prober.channel_manager.send_probe(path.clone()) {
731-
Ok(_) => {
736+
Ok((_, payment_id)) => {
737+
inflight.insert(payment_id, amount);
732738
prober.locked_msat.fetch_add(amount, Ordering::Release);
739+
drop(inflight);
733740
log_debug!(
734741
prober.logger,
735742
"Probe sent: locked {} msat, path: {}",
@@ -738,6 +745,7 @@ pub(crate) async fn run_prober(prober: Arc<Prober>, mut stop_rx: tokio::sync::wa
738745
);
739746
}
740747
Err(e) => {
748+
drop(inflight);
741749
log_debug!(
742750
prober.logger,
743751
"Probe send failed: {:?}, path: {}",

0 commit comments

Comments
 (0)