Skip to content

Commit

Permalink
Consume node_id from CandidateRouteHop
Browse files Browse the repository at this point in the history
  • Loading branch information
@jbesraa
jbesraa committed Sep 27, 2023
1 parent 51ae105 commit 443626d
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 25 deletions.
54 changes: 29 additions & 25 deletions lightning/src/routing/router.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1052,8 +1052,6 @@ pub enum CandidateRouteHop<'a> {
/// Provided to uniquely identify a hop as we are
/// route building.
hint_idx: usize,
/// The node id of the payer.
source_node_id: NodeId,
},
/// Similar to [`Self::Blinded`], but the path here has 1 blinded hop. `BlindedPayInfo` provided
/// for 1-hop blinded paths is ignored because it is meant to apply to the hops *between* the
Expand All @@ -1069,8 +1067,6 @@ pub enum CandidateRouteHop<'a> {
/// Provided to uniquely identify a hop as we are
/// route building.
hint_idx: usize,
/// The node id of the payer.
source_node_id: NodeId,
},
}

Expand Down Expand Up @@ -1183,8 +1179,8 @@ impl<'a> CandidateRouteHop<'a> {
*source_node_id
},
CandidateRouteHop::PrivateHop { hint, .. } => hint.src_node_id.into(),
CandidateRouteHop::Blinded { source_node_id, .. } => *source_node_id,
CandidateRouteHop::OneHopBlinded { source_node_id, .. } => *source_node_id,
CandidateRouteHop::Blinded { hint, .. } => hint.1.introduction_node_id.into(),
CandidateRouteHop::OneHopBlinded { hint, .. } => hint.1.introduction_node_id.into(),
}
}
/// Returns the target node id of this hop, if known.
Expand Down Expand Up @@ -1249,7 +1245,7 @@ fn iter_equal<I1: Iterator, I2: Iterator>(mut iter_a: I1, mut iter_b: I2)
pub struct PathBuildingHop<'a> {
// Note that this should be dropped in favor of loading it from CandidateRouteHop, but doing so
// is a larger refactor and will require careful performance analysis.
node_id: NodeId,
// node_id: NodeId,
candidate: CandidateRouteHop<'a>,
fee_msat: u64,

Expand Down Expand Up @@ -1287,7 +1283,7 @@ impl<'a> core::fmt::Debug for PathBuildingHop<'a> {
fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
let mut debug_struct = f.debug_struct("PathBuildingHop");
debug_struct
.field("node_id", &self.node_id)
.field("node_id", &self.candidate.source())
.field("short_channel_id", &self.candidate.short_channel_id())
.field("total_fee_msat", &self.total_fee_msat)
.field("next_hops_fee_msat", &self.next_hops_fee_msat)
Expand Down Expand Up @@ -1910,7 +1906,7 @@ where L::Target: Logger {
// This will affect our decision on selecting short_channel_id
// as a way to reach the $dest_node_id.
PathBuildingHop {
node_id: $dest_node_id.clone(),
// node_id: $dest_node_id.clone(),
candidate: $candidate.clone(),
fee_msat: 0,
next_hops_fee_msat: u64::max_value(),
Expand Down Expand Up @@ -2001,7 +1997,6 @@ where L::Target: Logger {
old_entry.next_hops_fee_msat = $next_hops_fee_msat;
old_entry.hop_use_fee_msat = hop_use_fee_msat;
old_entry.total_fee_msat = total_fee_msat;
old_entry.node_id = $dest_node_id.clone();
old_entry.candidate = $candidate.clone();
old_entry.fee_msat = 0; // This value will be later filled with hop_use_fee_msat of the following channel
old_entry.path_htlc_minimum_msat = path_htlc_minimum_msat;
Expand Down Expand Up @@ -2167,8 +2162,8 @@ where L::Target: Logger {
network_nodes.get(&intro_node_id).is_some();
if !have_intro_node_in_graph { continue }
let candidate = if hint.1.blinded_hops.len() == 1 {
CandidateRouteHop::OneHopBlinded { hint, hint_idx, source_node_id: our_node_id }
} else { CandidateRouteHop::Blinded { hint, hint_idx, source_node_id: intro_node_id } };
CandidateRouteHop::OneHopBlinded { hint, hint_idx }
} else { CandidateRouteHop::Blinded { hint, hint_idx } };
let mut path_contribution_msat = path_value_msat;
if let Some(hop_used_msat) = add_entry!(&candidate, intro_node_id, maybe_dummy_payee_node_id,
0, path_contribution_msat, 0, 0_u64, 0, 0)
Expand Down Expand Up @@ -2349,7 +2344,11 @@ where L::Target: Logger {

'path_walk: loop {
let mut features_set = false;
if let Some(first_channels) = first_hop_targets.get(&ordered_hops.last().unwrap().0.node_id) {
let target = match ordered_hops.last().unwrap().0.candidate.target() {
Some(target) => target,
None => break,
};
if let Some(first_channels) = first_hop_targets.get(&target) {
for details in first_channels {
if let Some(scid) = ordered_hops.last().unwrap().0.candidate.short_channel_id() {
if details.get_outbound_payment_scid().unwrap() == scid {
Expand All @@ -2361,7 +2360,7 @@ where L::Target: Logger {
}
}
if !features_set {
if let Some(node) = network_nodes.get(&ordered_hops.last().unwrap().0.node_id) {
if let Some(node) = network_nodes.get(&ordered_hops.last().unwrap().0.candidate.target().unwrap()) {
if let Some(node_info) = node.announcement_info.as_ref() {
ordered_hops.last_mut().unwrap().1 = node_info.features.clone();
} else {
Expand All @@ -2378,11 +2377,11 @@ where L::Target: Logger {
// save this path for the payment route. Also, update the liquidity
// remaining on the used hops, so that we take them into account
// while looking for more paths.
if ordered_hops.last().unwrap().0.node_id == maybe_dummy_payee_node_id {
if ordered_hops.last().unwrap().0.candidate.target().unwrap() == maybe_dummy_payee_node_id {
break 'path_walk;
}

new_entry = match dist.remove(&ordered_hops.last().unwrap().0.node_id) {
new_entry = match dist.remove(&ordered_hops.last().unwrap().0.candidate.target().unwrap()) {
Some(payment_hop) => payment_hop,
// We can't arrive at None because, if we ever add an entry to targets,
// we also fill in the entry in dist (see add_entry!).
Expand Down Expand Up @@ -2421,12 +2420,16 @@ where L::Target: Logger {
// Remember that we used these channels so that we don't rely
// on the same liquidity in future paths.
let mut prevented_redundant_path_selection = false;
let prev_hop_iter = core::iter::once(&our_node_id)
.chain(payment_path.hops.iter().map(|(hop, _)| &hop.node_id));
let prev_hop_iter = core::iter::once(our_node_id)
.chain(payment_path.hops.iter().map(|(hop, _)| hop.candidate.target().unwrap()));
for (prev_hop, (hop, _)) in prev_hop_iter.zip(payment_path.hops.iter()) {
let target = match hop.candidate.target() {
Some(target) => target,
None => break,
};
let spent_on_hop_msat = value_contribution_msat + hop.next_hops_fee_msat;
let used_liquidity_msat = used_liquidities
.entry(hop.candidate.id(*prev_hop < hop.node_id))
.entry(hop.candidate.id(prev_hop < target))
.and_modify(|used_liquidity_msat| *used_liquidity_msat += spent_on_hop_msat)
.or_insert(spent_on_hop_msat);
let hop_capacity = hop.candidate.effective_capacity();
Expand Down Expand Up @@ -2591,8 +2594,8 @@ where L::Target: Logger {
});
for idx in 0..(selected_route.len() - 1) {
if idx + 1 >= selected_route.len() { break; }
if iter_equal(selected_route[idx ].hops.iter().map(|h| (h.0.candidate.id(true), h.0.node_id)),
selected_route[idx + 1].hops.iter().map(|h| (h.0.candidate.id(true), h.0.node_id))) {
if iter_equal(selected_route[idx].hops.iter().map(|h| (h.0.candidate.id(true), h.0.candidate.target())),
selected_route[idx + 1].hops.iter().map(|h| (h.0.candidate.id(true), h.0.candidate.target()))) {
let new_value = selected_route[idx].get_value_msat() + selected_route[idx + 1].get_value_msat();
selected_route[idx].update_value_and_recompute_fees(new_value);
selected_route.remove(idx + 1);
Expand All @@ -2617,14 +2620,15 @@ where L::Target: Logger {
// there are announced channels between the endpoints. If so, the hop might be
// referring to any of the announced channels, as its `short_channel_id` might be
// an alias, in which case we don't take any chances here.
network_graph.node(&hop.node_id).map_or(false, |hop_node|
let hop_node_id = hop.candidate.source();
network_graph.node(&hop_node_id).map_or(false, |hop_node|
hop_node.channels.iter().any(|scid| network_graph.channel(*scid)
.map_or(false, |c| c.as_directed_from(&prev_hop_node_id).is_some()))
.map_or(false, |c| c.as_directed_from(&prev_hop_node_id).is_some()))
)
};

hops.push(RouteHop {
pubkey: PublicKey::from_slice(hop.node_id.as_slice()).map_err(|_| LightningError{err: format!("Public key {:?} is invalid", &hop.node_id), action: ErrorAction::IgnoreAndLog(Level::Trace)})?,
pubkey: PublicKey::from_slice(hop.candidate.target().unwrap().as_slice()).map_err(|_| LightningError{err: format!("Public key {:?} is invalid", &hop.candidate.target().unwrap()), action: ErrorAction::IgnoreAndLog(Level::Trace)})?,
node_features: node_features.clone(),
short_channel_id: hop.candidate.short_channel_id().unwrap(),
channel_features: hop.candidate.features(),
Expand All @@ -2633,7 +2637,7 @@ where L::Target: Logger {
maybe_announced_channel,
});

prev_hop_node_id = hop.node_id;
prev_hop_node_id = hop.candidate.source();
}
let mut final_cltv_delta = final_cltv_expiry_delta;
let blinded_tail = payment_path.hops.last().and_then(|(h, _)| {
Expand Down
8 changes: 8 additions & 0 deletions lightning/src/routing/scoring.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3715,6 +3715,14 @@ mod tests {
source_node_id: source,
target_node_id: *target
};
let channel = network_graph.read_only().channel(42).unwrap().to_owned();
let (info, target) = channel.as_directed_from(&source).unwrap();
let candidate: CandidateRouteHop = CandidateRouteHop::PublicHop {
info,
short_channel_id: 42,
source_node_id: source,
target_node_id: *target,
};
// With no historical data the normal liquidity penalty calculation is used, which results
// in a success probability of ~75%.
assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 1269);
Expand Down

0 comments on commit 443626d

Please sign in to comment.