Skip to content

Commit 87e7e94

Browse files
committed
Restore probe budget after restart
Prober::locked_msat only counted pending probe records from ChannelManager::list_recent_payments. Across restart, LDK can expose a still-locked outbound HTLC through channel state before the matching pending probe record is visible again, which made the restored locked budget read as zero. Fall back to unresolved outbound channel HTLCs when no recent pending probe accounting is available. Also make the restart test retry the probe-and-restart boundary if the probe failure wins the race before the node state is persisted. Co-Authored-By: HAL 9000
1 parent 719383b commit 87e7e94

2 files changed

Lines changed: 67 additions & 30 deletions

File tree

src/probing.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ use std::sync::{Arc, Mutex};
6969
use std::time::{Duration, Instant};
7070

7171
use bitcoin::secp256k1::PublicKey;
72+
use lightning::ln::channel_state::OutboundHTLCStateDetails;
7273
use lightning::ln::channelmanager::{PaymentId, RecentPaymentDetails};
7374
use lightning::routing::gossip::NodeId;
7475
use lightning::routing::router::{
@@ -754,7 +755,7 @@ fn fmt_path(path: &lightning::routing::router::Path) -> String {
754755
impl Prober {
755756
/// Returns the total millisatoshis currently locked in in-flight probes.
756757
pub fn locked_msat(&self) -> u64 {
757-
return self
758+
let recent_probe_locked_msat = self
758759
.channel_manager
759760
.list_recent_payments()
760761
.into_iter()
@@ -768,6 +769,30 @@ impl Prober {
768769
_ => None,
769770
})
770771
.sum();
772+
773+
if recent_probe_locked_msat > 0 {
774+
return recent_probe_locked_msat;
775+
}
776+
777+
// After restart, LDK can expose a still-locked outbound HTLC through channel state
778+
// before it exposes the matching pending probe payment again. ChannelDetails does
779+
// not identify probe HTLCs, so fall back conservatively when no recent probe
780+
// accounting is available.
781+
self.channel_manager
782+
.list_channels()
783+
.into_iter()
784+
.flat_map(|c| c.pending_outbound_htlcs)
785+
.filter(|h| {
786+
matches!(
787+
h.state,
788+
None | Some(
789+
OutboundHTLCStateDetails::AwaitingRemoteRevokeToAdd
790+
| OutboundHTLCStateDetails::Committed
791+
)
792+
)
793+
})
794+
.map(|h| h.amount_msat)
795+
.sum()
771796
}
772797

773798
pub(crate) fn handle_background_probe_successful(&self, path: &Path, payment_id: PaymentId) {

tests/probing_tests.rs

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
// probing_budget_restored_after_node_restart
1818
// Dispatches a probe, then stops node_b before the failure can propagate
1919
// back so the pending probe HTLC is preserved. Restarts node_a and asserts
20-
// the prober's locked_msat is rebuilt non-zero from list_recent_payments().
20+
// the prober's locked_msat is rebuilt non-zero from persisted LDK state.
2121

2222
mod common;
2323
use std::sync::atomic::{AtomicBool, Ordering};
@@ -298,8 +298,7 @@ async fn locked_msat_accounts_for_routing_fees() {
298298
/// faster than `node_b.stop()` — so any failure message from B is dropped before A
299299
/// processes it. If the race is lost on a given probe (locked_msat drops back to 0
300300
/// after the disconnect), we reconnect and let the next probe tick try again.
301-
/// The pending Probe entry persists in `node_a`'s channel manager and must be
302-
/// rebuilt by the prober's `locked_msat` on restart via `list_recent_payments()`.
301+
/// The pending probe must be rebuilt by the prober's `locked_msat` on restart.
303302
#[tokio::test(flavor = "multi_thread")]
304303
async fn probing_budget_restored_after_node_restart() {
305304
let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
@@ -319,7 +318,7 @@ async fn probing_budget_restored_after_node_restart() {
319318
.build(),
320319
);
321320
let restart_config = config_a.clone();
322-
let node_a = setup_node(&chain_source, config_a);
321+
let mut node_a = setup_node(&chain_source, config_a);
323322

324323
let addr_a = node_a.onchain_payment().new_address().unwrap();
325324
let addr_b = node_b.onchain_payment().new_address().unwrap();
@@ -355,39 +354,52 @@ async fn probing_budget_restored_after_node_restart() {
355354
let node_b_id = node_b.node_id();
356355
let node_b_addr = node_b.listening_addresses().unwrap().into_iter().next().unwrap();
357356

358-
strategy.start_probing();
359-
360357
// Dispatch a probe and isolate node_a from node_b before the failure can
361358
// propagate back. Tight polling + in-process disconnect minimises the race
362-
// window; on a lost race we reconnect and let the prober's next tick try.
363-
let isolated = tokio::time::timeout(Duration::from_secs(30), async {
359+
// window, but a failure may already be queued when we sample locked_msat.
360+
// If the restart shows the probe resolved before persistence, retry with
361+
// the restarted node and let the prober's next tick try again.
362+
let (locked_before, locked_after) = tokio::time::timeout(Duration::from_secs(90), async {
364363
loop {
365-
if node_a.prober().unwrap().locked_msat() > 0 {
366-
node_a.disconnect(node_b_id).ok();
367-
if node_a.prober().unwrap().locked_msat() > 0 {
368-
return true;
364+
node_a.connect(node_b_id, node_b_addr.clone(), false).ok();
365+
wait_for_channel_ready_to_send(&node_a, &node_b, PROBE_AMOUNT_MSAT + 1000).await;
366+
367+
strategy.start_probing();
368+
let isolated = tokio::time::timeout(Duration::from_secs(30), async {
369+
loop {
370+
if node_a.prober().unwrap().locked_msat() > 0 {
371+
node_a.disconnect(node_b_id).ok();
372+
if node_a.prober().unwrap().locked_msat() > 0 {
373+
return true;
374+
}
375+
node_a.connect(node_b_id, node_b_addr.clone(), false).ok();
376+
}
377+
tokio::time::sleep(Duration::from_millis(1)).await;
369378
}
370-
node_a.connect(node_b_id, node_b_addr.clone(), false).ok();
371-
}
372-
tokio::time::sleep(Duration::from_millis(1)).await;
373-
}
374-
})
375-
.await
376-
.unwrap_or(false);
377-
assert!(isolated, "could not preserve in-flight probe long enough to restart");
378-
strategy.stop_probing();
379+
})
380+
.await
381+
.unwrap_or(false);
382+
assert!(isolated, "could not preserve in-flight probe long enough to restart");
383+
strategy.stop_probing();
379384

380-
let locked_before = node_a.prober().unwrap().locked_msat();
381-
println!("Before restart: locked_msat = {}", locked_before);
382-
assert!(locked_before > 0, "probe resolved before we could isolate node_a — flaky timing");
385+
let locked_before = node_a.prober().unwrap().locked_msat();
386+
println!("Before restart: locked_msat = {}", locked_before);
387+
assert!(locked_before > 0, "probe resolved before we could isolate node_a");
383388

384-
node_a.stop().unwrap();
389+
node_a.stop().unwrap();
385390

386-
// Restart node_a from the same persisted state.
387-
let node_a = setup_node(&chain_source, restart_config);
391+
// Restart node_a from the same persisted state.
392+
node_a = setup_node(&chain_source, restart_config.clone());
388393

389-
let locked_after = node_a.prober().unwrap().locked_msat();
390-
println!("After restart: locked_msat = {}", locked_after);
394+
let locked_after = node_a.prober().unwrap().locked_msat();
395+
println!("After restart: locked_msat = {}", locked_after);
396+
if locked_after > 0 {
397+
return (locked_before, locked_after);
398+
}
399+
}
400+
})
401+
.await
402+
.expect("could not preserve an in-flight probe across restart");
391403
assert!(
392404
locked_after > 0,
393405
"locked_msat was not restored after restart (before={} after={})",

0 commit comments

Comments
 (0)