Skip to content

Commit 415befb

Browse files
committed
Polish background probing service
Move probing code in builder before the memory leak checker. Prevent potential overflow due to public channel info fee values. Account aggregated fee potentially being bigger than constructed porbing path's max htlc bound.
1 parent 08efb3a commit 415befb

2 files changed

Lines changed: 32 additions & 21 deletions

File tree

src/builder.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2249,18 +2249,6 @@ fn build_with_store_internal(
22492249

22502250
let pathfinding_scores_sync_url = pathfinding_scores_sync_config.map(|c| c.url.clone());
22512251

2252-
#[cfg(cycle_tests)]
2253-
let mut _leak_checker = crate::LeakChecker(Vec::new());
2254-
#[cfg(cycle_tests)]
2255-
{
2256-
use std::any::Any;
2257-
use std::sync::Weak;
2258-
2259-
_leak_checker.0.push(Arc::downgrade(&channel_manager) as Weak<dyn Any + Send + Sync>);
2260-
_leak_checker.0.push(Arc::downgrade(&network_graph) as Weak<dyn Any + Send + Sync>);
2261-
_leak_checker.0.push(Arc::downgrade(&wallet) as Weak<dyn Any + Send + Sync>);
2262-
}
2263-
22642252
let prober = probing_config.map(|probing_cfg| {
22652253
let strategy: Arc<dyn ProbingStrategy> = match &probing_cfg.kind {
22662254
ProbingStrategyKind::HighDegree { top_node_count } => {
@@ -2306,6 +2294,18 @@ fn build_with_store_internal(
23062294
})
23072295
});
23082296

2297+
#[cfg(cycle_tests)]
2298+
let mut _leak_checker = crate::LeakChecker(Vec::new());
2299+
#[cfg(cycle_tests)]
2300+
{
2301+
use std::any::Any;
2302+
use std::sync::Weak;
2303+
2304+
_leak_checker.0.push(Arc::downgrade(&channel_manager) as Weak<dyn Any + Send + Sync>);
2305+
_leak_checker.0.push(Arc::downgrade(&network_graph) as Weak<dyn Any + Send + Sync>);
2306+
_leak_checker.0.push(Arc::downgrade(&wallet) as Weak<dyn Any + Send + Sync>);
2307+
}
2308+
23092309
Ok(Node {
23102310
runtime,
23112311
stop_sender,

src/probing.rs

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,7 @@ impl RandomWalkStrategy {
595595
// Retrieve the direction-specific update via the public ChannelInfo fields.
596596
// as_directed_from already checked both directions are Some, but we break
597597
// defensively rather than unwrap.
598-
let update = match if directed.source() == &next_channel.node_one {
598+
let channel_update_info = match if directed.source() == &next_channel.node_one {
599599
next_channel.one_to_two.as_ref()
600600
} else {
601601
next_channel.two_to_one.as_ref()
@@ -604,15 +604,15 @@ impl RandomWalkStrategy {
604604
None => break,
605605
};
606606

607-
if !update.enabled {
607+
if !channel_update_info.enabled {
608608
break;
609609
}
610610

611611
route_least_htlc_upper_bound =
612-
route_least_htlc_upper_bound.min(update.htlc_maximum_msat);
612+
route_least_htlc_upper_bound.min(channel_update_info.htlc_maximum_msat);
613613

614614
route_greatest_htlc_lower_bound =
615-
route_greatest_htlc_lower_bound.max(update.htlc_minimum_msat);
615+
route_greatest_htlc_lower_bound.max(channel_update_info.htlc_minimum_msat);
616616

617617
let next_pubkey = match PublicKey::try_from(*next_node_id) {
618618
Ok(pk) => pk,
@@ -682,25 +682,36 @@ impl RandomWalkStrategy {
682682
let (_, next_scid, _) = route[i + 1];
683683
let next_channel = graph.channel(next_scid)?;
684684
let (directed, _) = next_channel.as_directed_from(&node_id)?;
685-
let update = match if directed.source() == &next_channel.node_one {
685+
let channel_update_info = match if directed.source() == &next_channel.node_one {
686686
next_channel.one_to_two.as_ref()
687687
} else {
688688
next_channel.two_to_one.as_ref()
689689
} {
690690
Some(u) => u,
691691
None => return None,
692692
};
693-
let fee = update.fees.base_msat as u64
694-
+ (forwarded * update.fees.proportional_millionths as u64 / 1_000_000);
695-
forwarded += fee;
693+
// Overflow prevention
694+
let proportional_fee = forwarded
695+
.checked_mul(channel_update_info.fees.proportional_millionths as u64)
696+
.map(|v| v / 1_000_000)?;
697+
let fee = (channel_update_info.fees.base_msat as u64).checked_add(proportional_fee)?;
698+
forwarded = forwarded.checked_add(fee)?;
699+
700+
// Fees compound as they're added walking back towards the sender, so re-check the
701+
// now-inflated forwarded amount against this channel's own advertised bounds.
702+
if forwarded > channel_update_info.htlc_maximum_msat
703+
|| forwarded < channel_update_info.htlc_minimum_msat
704+
{
705+
return None;
706+
}
696707

697708
hops.push(RouteHop {
698709
pubkey,
699710
node_features,
700711
short_channel_id: via_scid,
701712
channel_features,
702713
fee_msat: fee,
703-
cltv_expiry_delta: update.cltv_expiry_delta as u32,
714+
cltv_expiry_delta: channel_update_info.cltv_expiry_delta as u32,
704715
maybe_announced_channel,
705716
});
706717
}

0 commit comments

Comments
 (0)