Skip to content

Commit 5dec4e7

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 5dec4e7

2 files changed

Lines changed: 33 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: 21 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,37 @@ 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+
// forwarded is the amount sent over next_channel (before this hop's own fee is
694+
// added on top for the preceding channel), so check it against next_channel's
695+
// own advertised bounds before computing the fee.
696+
if forwarded > channel_update_info.htlc_maximum_msat
697+
|| forwarded < channel_update_info.htlc_minimum_msat
698+
{
699+
return None;
700+
}
701+
702+
// Overflow prevention
703+
let proportional_fee = forwarded
704+
.checked_mul(channel_update_info.fees.proportional_millionths as u64)
705+
.map(|v| v / 1_000_000)?;
706+
let fee = (channel_update_info.fees.base_msat as u64).checked_add(proportional_fee)?;
707+
forwarded = forwarded.checked_add(fee)?;
696708

697709
hops.push(RouteHop {
698710
pubkey,
699711
node_features,
700712
short_channel_id: via_scid,
701713
channel_features,
702714
fee_msat: fee,
703-
cltv_expiry_delta: update.cltv_expiry_delta as u32,
715+
cltv_expiry_delta: channel_update_info.cltv_expiry_delta as u32,
704716
maybe_announced_channel,
705717
});
706718
}

0 commit comments

Comments
 (0)