Skip to content

Commit

Permalink
Fix scoring methods to score Paths instead of Vec<RouteHop>s
Browse files Browse the repository at this point in the history
  • Loading branch information
@valentinewallace
valentinewallace committed Apr 5, 2023
1 parent b0d9cdd commit 6ed1e5f
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 115 deletions.
35 changes: 15 additions & 20 deletions lightning-background-processor/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,25 +236,20 @@ fn update_scorer<'a, S: 'static + Deref<Target = SC> + Send + Sync, SC: 'a + Wri
let mut score = scorer.lock();
match event {
Event::PaymentPathFailed { ref path, short_channel_id: Some(scid), .. } => {
let path = path.iter().collect::<Vec<_>>();
score.payment_path_failed(&path, *scid);
},
Event::PaymentPathFailed { ref path, payment_failed_permanently: true, .. } => {
// Reached if the destination explicitly failed it back. We treat this as a successful probe
// because the payment made it all the way to the destination with sufficient liquidity.
let path = path.iter().collect::<Vec<_>>();
score.probe_successful(&path);
},
Event::PaymentPathSuccessful { path, .. } => {
let path = path.iter().collect::<Vec<_>>();
score.payment_path_successful(&path);
},
Event::ProbeSuccessful { path, .. } => {
let path = path.iter().collect::<Vec<_>>();
score.probe_successful(&path);
},
Event::ProbeFailed { path, short_channel_id: Some(scid), .. } => {
let path = path.iter().collect::<Vec<_>>();
score.probe_failed(&path, *scid);
},
_ => {},
Expand Down Expand Up @@ -751,7 +746,7 @@ mod tests {
use lightning::ln::msgs::{ChannelMessageHandler, Init};
use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler};
use lightning::routing::gossip::{NetworkGraph, NodeId, P2PGossipSync};
use lightning::routing::router::{DefaultRouter, RouteHop};
use lightning::routing::router::{DefaultRouter, Path, RouteHop};
use lightning::routing::scoring::{ChannelUsage, Score};
use lightning::util::config::UserConfig;
use lightning::util::ser::Writeable;
Expand Down Expand Up @@ -891,10 +886,10 @@ mod tests {

#[derive(Debug)]
enum TestResult {
PaymentFailure { path: Vec<RouteHop>, short_channel_id: u64 },
PaymentSuccess { path: Vec<RouteHop> },
ProbeFailure { path: Vec<RouteHop> },
ProbeSuccess { path: Vec<RouteHop> },
PaymentFailure { path: Path, short_channel_id: u64 },
PaymentSuccess { path: Path },
ProbeFailure { path: Path },
ProbeSuccess { path: Path },
}

impl TestScorer {
Expand All @@ -916,11 +911,11 @@ mod tests {
&self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId, _usage: ChannelUsage
) -> u64 { unimplemented!(); }

fn payment_path_failed(&mut self, actual_path: &[&RouteHop], actual_short_channel_id: u64) {
fn payment_path_failed(&mut self, actual_path: &Path, actual_short_channel_id: u64) {
if let Some(expectations) = &mut self.event_expectations {
match expectations.pop_front().unwrap() {
TestResult::PaymentFailure { path, short_channel_id } => {
assert_eq!(actual_path, &path.iter().collect::<Vec<_>>()[..]);
assert_eq!(actual_path, &path);
assert_eq!(actual_short_channel_id, short_channel_id);
},
TestResult::PaymentSuccess { path } => {
Expand All @@ -936,14 +931,14 @@ mod tests {
}
}

fn payment_path_successful(&mut self, actual_path: &[&RouteHop]) {
fn payment_path_successful(&mut self, actual_path: &Path) {
if let Some(expectations) = &mut self.event_expectations {
match expectations.pop_front().unwrap() {
TestResult::PaymentFailure { path, .. } => {
panic!("Unexpected payment path failure: {:?}", path)
},
TestResult::PaymentSuccess { path } => {
assert_eq!(actual_path, &path.iter().collect::<Vec<_>>()[..]);
assert_eq!(actual_path, &path);
},
TestResult::ProbeFailure { path } => {
panic!("Unexpected probe failure: {:?}", path)
Expand All @@ -955,7 +950,7 @@ mod tests {
}
}

fn probe_failed(&mut self, actual_path: &[&RouteHop], _: u64) {
fn probe_failed(&mut self, actual_path: &Path, _: u64) {
if let Some(expectations) = &mut self.event_expectations {
match expectations.pop_front().unwrap() {
TestResult::PaymentFailure { path, .. } => {
Expand All @@ -965,15 +960,15 @@ mod tests {
panic!("Unexpected payment path success: {:?}", path)
},
TestResult::ProbeFailure { path } => {
assert_eq!(actual_path, &path.iter().collect::<Vec<_>>()[..]);
assert_eq!(actual_path, &path);
},
TestResult::ProbeSuccess { path } => {
panic!("Unexpected probe success: {:?}", path)
}
}
}
}
fn probe_successful(&mut self, actual_path: &[&RouteHop]) {
fn probe_successful(&mut self, actual_path: &Path) {
if let Some(expectations) = &mut self.event_expectations {
match expectations.pop_front().unwrap() {
TestResult::PaymentFailure { path, .. } => {
Expand All @@ -986,7 +981,7 @@ mod tests {
panic!("Unexpected probe failure: {:?}", path)
},
TestResult::ProbeSuccess { path } => {
assert_eq!(actual_path, &path.iter().collect::<Vec<_>>()[..]);
assert_eq!(actual_path, &path);
}
}
}
Expand Down Expand Up @@ -1510,14 +1505,14 @@ mod tests {
let node_1_privkey = SecretKey::from_slice(&[42; 32]).unwrap();
let node_1_id = PublicKey::from_secret_key(&secp_ctx, &node_1_privkey);

let path = vec![RouteHop {
let path = Path { hops: vec![RouteHop {
pubkey: node_1_id,
node_features: NodeFeatures::empty(),
short_channel_id: scored_scid,
channel_features: ChannelFeatures::empty(),
fee_msat: 0,
cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA as u32,
}];
}], blinded_tail: None };

$nodes[0].scorer.lock().unwrap().expect(TestResult::PaymentFailure { path: path.clone(), short_channel_id: scored_scid });
$nodes[0].node.push_pending_event(Event::PaymentPathFailed {
Expand Down
38 changes: 18 additions & 20 deletions lightning/src/routing/router.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,19 +137,19 @@ impl<'a, S: Score> Score for ScorerAccountingForInFlightHtlcs<'a, S> {
}
}

fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
fn payment_path_failed(&mut self, path: &Path, short_channel_id: u64) {
self.scorer.payment_path_failed(path, short_channel_id)
}

fn payment_path_successful(&mut self, path: &[&RouteHop]) {
fn payment_path_successful(&mut self, path: &Path) {
self.scorer.payment_path_successful(path)
}

fn probe_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
fn probe_failed(&mut self, path: &Path, short_channel_id: u64) {
self.scorer.probe_failed(path, short_channel_id)
}

fn probe_successful(&mut self, path: &[&RouteHop]) {
fn probe_successful(&mut self, path: &Path) {
self.scorer.probe_successful(path)
}
}
Expand Down Expand Up @@ -2251,13 +2251,13 @@ fn build_route_from_hops_internal<L: Deref>(
u64::max_value()
}

fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
fn payment_path_failed(&mut self, _path: &Path, _short_channel_id: u64) {}

fn payment_path_successful(&mut self, _path: &[&RouteHop]) {}
fn payment_path_successful(&mut self, _path: &Path) {}

fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
fn probe_failed(&mut self, _path: &Path, _short_channel_id: u64) {}

fn probe_successful(&mut self, _path: &[&RouteHop]) {}
fn probe_successful(&mut self, _path: &Path) {}
}

impl<'a> Writeable for HopScorer {
Expand Down Expand Up @@ -5268,10 +5268,10 @@ mod tests {
if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 }
}

fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
fn payment_path_successful(&mut self, _path: &[&RouteHop]) {}
fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
fn probe_successful(&mut self, _path: &[&RouteHop]) {}
fn payment_path_failed(&mut self, _path: &Path, _short_channel_id: u64) {}
fn payment_path_successful(&mut self, _path: &Path) {}
fn probe_failed(&mut self, _path: &Path, _short_channel_id: u64) {}
fn probe_successful(&mut self, _path: &Path) {}
}

struct BadNodeScorer {
Expand All @@ -5288,10 +5288,10 @@ mod tests {
if *target == self.node_id { u64::max_value() } else { 0 }
}

fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
fn payment_path_successful(&mut self, _path: &[&RouteHop]) {}
fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
fn probe_successful(&mut self, _path: &[&RouteHop]) {}
fn payment_path_failed(&mut self, _path: &Path, _short_channel_id: u64) {}
fn payment_path_successful(&mut self, _path: &Path) {}
fn probe_failed(&mut self, _path: &Path, _short_channel_id: u64) {}
fn probe_successful(&mut self, _path: &Path) {}
}

#[test]
Expand Down Expand Up @@ -5955,14 +5955,12 @@ mod benches {
let amount = route.get_total_amount();
if amount < 250_000 {
for path in route.paths {
// fixed to use the whole Path in an upcoming commit
scorer.payment_path_successful(&path.hops.iter().collect::<Vec<_>>());
scorer.payment_path_successful(&path);
}
} else if amount > 750_000 {
for path in route.paths {
let short_channel_id = path.hops[path.hops.len() / 2].short_channel_id;
// fixed to use the whole Path in an upcoming commit
scorer.payment_path_failed(&path.hops.iter().collect::<Vec<_>>(), short_channel_id);
scorer.payment_path_failed(&path, short_channel_id);
}
}
}
Expand Down
Loading

0 comments on commit 6ed1e5f

Please sign in to comment.