Skip to content

Commit 77948db

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #1166 from TheBlueMatt/2021-11-chan-size-scoring
2 parents e1ad422 + 42ebf77 commit 77948db

File tree

9 files changed

+211
-138
lines changed

9 files changed

+211
-138
lines changed

‎fuzz/src/full_stack.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ use lightning::ln::msgs::DecodeError;
3939
use lightning::ln::script::ShutdownScript;
4040
use lightning::routing::network_graph::{NetGraphMsgHandler, NetworkGraph};
4141
use lightning::routing::router::{find_route, Payee, RouteParameters};
42-
use lightning::routing::scorer::Scorer;
42+
use lightning::routing::scoring::Scorer;
4343
use lightning::util::config::UserConfig;
4444
use lightning::util::errors::APIError;
4545
use lightning::util::events::Event;

‎fuzz/src/router.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use lightning::ln::channelmanager::{ChannelDetails, ChannelCounterparty};
1717
use lightning::ln::features::InitFeatures;
1818
use lightning::ln::msgs;
1919
use lightning::routing::router::{find_route, Payee, RouteHint, RouteHintHop, RouteParameters};
20-
use lightning::routing::scorer::Scorer;
20+
use lightning::routing::scoring::Scorer;
2121
use lightning::util::logger::Logger;
2222
use lightning::util::ser::Readable;
2323
use lightning::routing::network_graph::{NetworkGraph, RoutingFees};

‎lightning-invoice/src/payment.rs

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
//! # use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
3232
//! # use lightning::ln::channelmanager::{ChannelDetails, PaymentId, PaymentSendFailure};
3333
//! # use lightning::ln::msgs::LightningError;
34-
//! # use lightning::routing;
34+
//! # use lightning::routing::scoring::Score;
3535
//! # use lightning::routing::network_graph::NodeId;
3636
//! # use lightning::routing::router::{Route, RouteHop, RouteParameters};
3737
//! # use lightning::util::events::{Event, EventHandler, EventsProvider};
@@ -63,17 +63,17 @@
6363
//! # }
6464
//! #
6565
//! # struct FakeRouter {};
66-
//! # impl<S: routing::Score> Router<S> for FakeRouter {
66+
//! # impl<S: Score> Router<S> for FakeRouter {
6767
//! # fn find_route(
6868
//! # &self, payer: &PublicKey, params: &RouteParameters, payment_hash: &PaymentHash,
6969
//! # first_hops: Option<&[&ChannelDetails]>, scorer: &S
7070
//! # ) -> Result<Route, LightningError> { unimplemented!() }
7171
//! # }
7272
//! #
7373
//! # struct FakeScorer {};
74-
//! # impl routing::Score for FakeScorer {
74+
//! # impl Score for FakeScorer {
7575
//! # fn channel_penalty_msat(
76-
//! # &self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId
76+
//! # &self, _short_channel_id: u64, _send_amt: u64, _chan_amt: Option<u64>, _source: &NodeId, _target: &NodeId
7777
//! # ) -> u64 { 0 }
7878
//! # fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
7979
//! # }
@@ -122,8 +122,7 @@ use bitcoin_hashes::sha256::Hash as Sha256;
122122
use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
123123
use lightning::ln::channelmanager::{ChannelDetails, PaymentId, PaymentSendFailure};
124124
use lightning::ln::msgs::LightningError;
125-
use lightning::routing;
126-
use lightning::routing::{LockableScore, Score};
125+
use lightning::routing::scoring::{LockableScore, Score};
127126
use lightning::routing::router::{Payee, Route, RouteParameters};
128127
use lightning::util::events::{Event, EventHandler};
129128
use lightning::util::logger::Logger;
@@ -139,8 +138,8 @@ use std::time::{Duration, SystemTime};
139138
pub struct InvoicePayer<P: Deref, R, S: Deref, L: Deref, E>
140139
where
141140
P::Target: Payer,
142-
R: for <'a> Router<<<S as Deref>::Target as routing::LockableScore<'a>>::Locked>,
143-
S::Target: for <'a> routing::LockableScore<'a>,
141+
R: for <'a> Router<<<S as Deref>::Target as LockableScore<'a>>::Locked>,
142+
S::Target: for <'a> LockableScore<'a>,
144143
L::Target: Logger,
145144
E: EventHandler,
146145
{
@@ -177,7 +176,7 @@ pub trait Payer {
177176
}
178177

179178
/// A trait defining behavior for routing an [`Invoice`] payment.
180-
pub trait Router<S: routing::Score> {
179+
pub trait Router<S: Score> {
181180
/// Finds a [`Route`] between `payer` and `payee` for a payment with the given values.
182181
fn find_route(
183182
&self, payer: &PublicKey, params: &RouteParameters, payment_hash: &PaymentHash,
@@ -207,8 +206,8 @@ pub enum PaymentError {
207206
impl<P: Deref, R, S: Deref, L: Deref, E> InvoicePayer<P, R, S, L, E>
208207
where
209208
P::Target: Payer,
210-
R: for <'a> Router<<<S as Deref>::Target as routing::LockableScore<'a>>::Locked>,
211-
S::Target: for <'a> routing::LockableScore<'a>,
209+
R: for <'a> Router<<<S as Deref>::Target as LockableScore<'a>>::Locked>,
210+
S::Target: for <'a> LockableScore<'a>,
212211
L::Target: Logger,
213212
E: EventHandler,
214213
{
@@ -441,8 +440,8 @@ fn has_expired(params: &RouteParameters) -> bool {
441440
impl<P: Deref, R, S: Deref, L: Deref, E> EventHandler for InvoicePayer<P, R, S, L, E>
442441
where
443442
P::Target: Payer,
444-
R: for <'a> Router<<<S as Deref>::Target as routing::LockableScore<'a>>::Locked>,
445-
S::Target: for <'a> routing::LockableScore<'a>,
443+
R: for <'a> Router<<<S as Deref>::Target as LockableScore<'a>>::Locked>,
444+
S::Target: for <'a> LockableScore<'a>,
446445
L::Target: Logger,
447446
E: EventHandler,
448447
{
@@ -1186,7 +1185,7 @@ mod tests {
11861185
}
11871186
}
11881187

1189-
impl<S: routing::Score> Router<S> for TestRouter {
1188+
impl<S: Score> Router<S> for TestRouter {
11901189
fn find_route(
11911190
&self, _payer: &PublicKey, params: &RouteParameters, _payment_hash: &PaymentHash,
11921191
_first_hops: Option<&[&ChannelDetails]>, _scorer: &S
@@ -1199,7 +1198,7 @@ mod tests {
11991198

12001199
struct FailingRouter;
12011200

1202-
impl<S: routing::Score> Router<S> for FailingRouter {
1201+
impl<S: Score> Router<S> for FailingRouter {
12031202
fn find_route(
12041203
&self, _payer: &PublicKey, _params: &RouteParameters, _payment_hash: &PaymentHash,
12051204
_first_hops: Option<&[&ChannelDetails]>, _scorer: &S
@@ -1225,9 +1224,9 @@ mod tests {
12251224
}
12261225
}
12271226

1228-
impl routing::Score for TestScorer {
1227+
impl Score for TestScorer {
12291228
fn channel_penalty_msat(
1230-
&self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId
1229+
&self, _short_channel_id: u64, _send_amt: u64, _chan_amt: Option<u64>, _source: &NodeId, _target: &NodeId
12311230
) -> u64 { 0 }
12321231

12331232
fn payment_path_failed(&mut self, _path: &[&RouteHop], short_channel_id: u64) {
@@ -1364,7 +1363,7 @@ mod tests {
13641363
// *** Full Featured Functional Tests with a Real ChannelManager ***
13651364
struct ManualRouter(RefCell<VecDeque<Result<Route, LightningError>>>);
13661365

1367-
impl<S: routing::Score> Router<S> for ManualRouter {
1366+
impl<S: Score> Router<S> for ManualRouter {
13681367
fn find_route(
13691368
&self, _payer: &PublicKey, _params: &RouteParameters, _payment_hash: &PaymentHash,
13701369
_first_hops: Option<&[&ChannelDetails]>, _scorer: &S

‎lightning-invoice/src/utils.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use lightning::chain::keysinterface::{Sign, KeysInterface};
1111
use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
1212
use lightning::ln::channelmanager::{ChannelDetails, ChannelManager, PaymentId, PaymentSendFailure, MIN_FINAL_CLTV_EXPIRY};
1313
use lightning::ln::msgs::LightningError;
14-
use lightning::routing;
14+
use lightning::routing::scoring::Score;
1515
use lightning::routing::network_graph::{NetworkGraph, RoutingFees};
1616
use lightning::routing::router::{Route, RouteHint, RouteHintHop, RouteParameters, find_route};
1717
use lightning::util::logger::Logger;
@@ -109,7 +109,7 @@ impl<G, L: Deref> DefaultRouter<G, L> where G: Deref<Target = NetworkGraph>, L::
109109
}
110110
}
111111

112-
impl<G, L: Deref, S: routing::Score> Router<S> for DefaultRouter<G, L>
112+
impl<G, L: Deref, S: Score> Router<S> for DefaultRouter<G, L>
113113
where G: Deref<Target = NetworkGraph>, L::Target: Logger {
114114
fn find_route(
115115
&self, payer: &PublicKey, params: &RouteParameters, _payment_hash: &PaymentHash,

‎lightning/src/ln/channelmanager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6552,7 +6552,7 @@ pub mod bench {
65526552
use ln::msgs::{ChannelMessageHandler, Init};
65536553
use routing::network_graph::NetworkGraph;
65546554
use routing::router::{Payee, get_route};
6555-
use routing::scorer::Scorer;
6555+
use routing::scoring::Scorer;
65566556
use util::test_utils;
65576557
use util::config::UserConfig;
65586558
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose};

‎lightning/src/routing/mod.rs

Lines changed: 1 addition & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -11,65 +11,4 @@
1111
1212
pub mod network_graph;
1313
pub mod router;
14-
pub mod scorer;
15-
16-
use routing::network_graph::NodeId;
17-
use routing::router::RouteHop;
18-
19-
use core::cell::{RefCell, RefMut};
20-
use core::ops::DerefMut;
21-
use sync::{Mutex, MutexGuard};
22-
23-
/// An interface used to score payment channels for path finding.
24-
///
25-
/// Scoring is in terms of fees willing to be paid in order to avoid routing through a channel.
26-
pub trait Score {
27-
/// Returns the fee in msats willing to be paid to avoid routing through the given channel
28-
/// in the direction from `source` to `target`.
29-
fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId) -> u64;
30-
31-
/// Handles updating channel penalties after failing to route through a channel.
32-
fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64);
33-
}
34-
35-
/// A scorer that is accessed under a lock.
36-
///
37-
/// Needed so that calls to [`Score::channel_penalty_msat`] in [`find_route`] can be made while
38-
/// having shared ownership of a scorer but without requiring internal locking in [`Score`]
39-
/// implementations. Internal locking would be detrimental to route finding performance and could
40-
/// result in [`Score::channel_penalty_msat`] returning a different value for the same channel.
41-
///
42-
/// [`find_route`]: crate::routing::router::find_route
43-
pub trait LockableScore<'a> {
44-
/// The locked [`Score`] type.
45-
type Locked: 'a + Score;
46-
47-
/// Returns the locked scorer.
48-
fn lock(&'a self) -> Self::Locked;
49-
}
50-
51-
impl<'a, T: 'a + Score> LockableScore<'a> for Mutex<T> {
52-
type Locked = MutexGuard<'a, T>;
53-
54-
fn lock(&'a self) -> MutexGuard<'a, T> {
55-
Mutex::lock(self).unwrap()
56-
}
57-
}
58-
59-
impl<'a, T: 'a + Score> LockableScore<'a> for RefCell<T> {
60-
type Locked = RefMut<'a, T>;
61-
62-
fn lock(&'a self) -> RefMut<'a, T> {
63-
self.borrow_mut()
64-
}
65-
}
66-
67-
impl<S: Score, T: DerefMut<Target=S>> Score for T {
68-
fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId) -> u64 {
69-
self.deref().channel_penalty_msat(short_channel_id, source, target)
70-
}
71-
72-
fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
73-
self.deref_mut().payment_path_failed(path, short_channel_id)
74-
}
75-
}
14+
pub mod scoring;

‎lightning/src/routing/router.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use bitcoin::secp256k1::key::PublicKey;
1717
use ln::channelmanager::ChannelDetails;
1818
use ln::features::{ChannelFeatures, InvoiceFeatures, NodeFeatures};
1919
use ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
20-
use routing;
20+
use routing::scoring::Score;
2121
use routing::network_graph::{NetworkGraph, NodeId, RoutingFees};
2222
use util::ser::{Writeable, Readable};
2323
use util::logger::{Level, Logger};
@@ -529,7 +529,7 @@ fn compute_fees(amount_msat: u64, channel_fees: RoutingFees) -> Option<u64> {
529529
///
530530
/// [`ChannelManager::list_usable_channels`]: crate::ln::channelmanager::ChannelManager::list_usable_channels
531531
/// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed
532-
pub fn find_route<L: Deref, S: routing::Score>(
532+
pub fn find_route<L: Deref, S: Score>(
533533
our_node_pubkey: &PublicKey, params: &RouteParameters, network: &NetworkGraph,
534534
first_hops: Option<&[&ChannelDetails]>, logger: L, scorer: &S
535535
) -> Result<Route, LightningError>
@@ -540,7 +540,7 @@ where L::Target: Logger {
540540
)
541541
}
542542

543-
pub(crate) fn get_route<L: Deref, S: routing::Score>(
543+
pub(crate) fn get_route<L: Deref, S: Score>(
544544
our_node_pubkey: &PublicKey, payee: &Payee, network: &NetworkGraph,
545545
first_hops: Option<&[&ChannelDetails]>, final_value_msat: u64, final_cltv_expiry_delta: u32,
546546
logger: L, scorer: &S
@@ -892,9 +892,9 @@ where L::Target: Logger {
892892
}
893893
}
894894

895-
let path_penalty_msat = $next_hops_path_penalty_msat
896-
.checked_add(scorer.channel_penalty_msat($chan_id.clone(), &$src_node_id, &$dest_node_id))
897-
.unwrap_or_else(|| u64::max_value());
895+
let path_penalty_msat = $next_hops_path_penalty_msat.checked_add(
896+
scorer.channel_penalty_msat($chan_id.clone(), amount_to_transfer_over_msat, Some(*available_liquidity_msat),
897+
&$src_node_id, &$dest_node_id)).unwrap_or_else(|| u64::max_value());
898898
let new_graph_node = RouteGraphNode {
899899
node_id: $src_node_id,
900900
lowest_fee_to_peer_through_node: total_fee_msat,
@@ -1121,7 +1121,7 @@ where L::Target: Logger {
11211121
let src_node_id = NodeId::from_pubkey(&hop.src_node_id);
11221122
let dest_node_id = NodeId::from_pubkey(&prev_hop_id);
11231123
aggregate_next_hops_path_penalty_msat = aggregate_next_hops_path_penalty_msat
1124-
.checked_add(scorer.channel_penalty_msat(hop.short_channel_id, &src_node_id, &dest_node_id))
1124+
.checked_add(scorer.channel_penalty_msat(hop.short_channel_id, final_value_msat, None, &src_node_id, &dest_node_id))
11251125
.unwrap_or_else(|| u64::max_value());
11261126

11271127
// We assume that the recipient only included route hints for routes which had
@@ -1472,7 +1472,7 @@ where L::Target: Logger {
14721472

14731473
#[cfg(test)]
14741474
mod tests {
1475-
use routing;
1475+
use routing::scoring::Score;
14761476
use routing::network_graph::{NetworkGraph, NetGraphMsgHandler, NodeId};
14771477
use routing::router::{get_route, Payee, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees};
14781478
use chain::transaction::OutPoint;
@@ -4549,8 +4549,8 @@ mod tests {
45494549
short_channel_id: u64,
45504550
}
45514551

4552-
impl routing::Score for BadChannelScorer {
4553-
fn channel_penalty_msat(&self, short_channel_id: u64, _source: &NodeId, _target: &NodeId) -> u64 {
4552+
impl Score for BadChannelScorer {
4553+
fn channel_penalty_msat(&self, short_channel_id: u64, _send_amt: u64, _chan_amt: Option<u64>, _source: &NodeId, _target: &NodeId) -> u64 {
45544554
if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 }
45554555
}
45564556

@@ -4561,8 +4561,8 @@ mod tests {
45614561
node_id: NodeId,
45624562
}
45634563

4564-
impl routing::Score for BadNodeScorer {
4565-
fn channel_penalty_msat(&self, _short_channel_id: u64, _source: &NodeId, target: &NodeId) -> u64 {
4564+
impl Score for BadNodeScorer {
4565+
fn channel_penalty_msat(&self, _short_channel_id: u64, _send_amt: u64, _chan_amt: Option<u64>, _source: &NodeId, target: &NodeId) -> u64 {
45664566
if *target == self.node_id { u64::max_value() } else { 0 }
45674567
}
45684568

@@ -4787,7 +4787,7 @@ pub(crate) mod test_utils {
47874787
#[cfg(all(test, feature = "unstable", not(feature = "no-std")))]
47884788
mod benches {
47894789
use super::*;
4790-
use routing::scorer::Scorer;
4790+
use routing::scoring::Scorer;
47914791
use util::logger::{Logger, Record};
47924792

47934793
use test::Bencher;

0 commit comments

Comments
 (0)