From c36bf9249978dce2b72b3b67d01e22c6c8d78431 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 30 Nov 2021 17:16:05 -0600 Subject: [PATCH] Score successful payment paths Expand the Score trait with a payment_path_successful function for scoring successful payment paths. Called by InvoicePayer's EventHandler implementation when processing PaymentPathSuccessful events. May be used by Score implementations to revert any channel penalties that were applied by calls to payment_path_failed. --- lightning-invoice/src/payment.rs | 100 +++++++++++++++++++++++++++---- lightning/src/routing/router.rs | 2 + lightning/src/routing/scoring.rs | 9 +++ 3 files changed, 101 insertions(+), 10 deletions(-) diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index a141eb2c3..ce942ef57 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -15,6 +15,10 @@ //! and payee using information provided by the payer and from the payee's [`Invoice`], when //! applicable. //! +//! [`InvoicePayer`] is parameterized by a [`LockableScore`], which it uses for scoring failed and +//! successful payment paths upon receiving [`Event::PaymentPathFailed`] and +//! [`Event::PaymentPathSuccessful`] events, respectively. +//! //! [`InvoicePayer`] is capable of retrying failed payments. It accomplishes this by implementing //! [`EventHandler`] which decorates a user-provided handler. It will intercept any //! [`Event::PaymentPathFailed`] events and retry the failed paths for a fixed number of total @@ -80,6 +84,7 @@ //! # &self, _short_channel_id: u64, _send_amt: u64, _chan_amt: Option, _source: &NodeId, _target: &NodeId //! # ) -> u64 { 0 } //! # fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} +//! # fn payment_path_successful(&mut self, _path: &[&RouteHop]) {} //! # } //! # //! # struct FakeLogger {}; @@ -139,6 +144,10 @@ use std::sync::Mutex; use std::time::{Duration, SystemTime}; /// A utility for paying [`Invoice`]s and sending spontaneous payments. +/// +/// See [module-level documentation] for details. +/// +/// [module-level documentation]: crate::payment pub struct InvoicePayer where P::Target: Payer, @@ -473,6 +482,10 @@ where if *all_paths_failed { self.payment_cache.lock().unwrap().remove(payment_hash); } }, + Event::PaymentPathSuccessful { path, .. } => { + let path = path.iter().collect::>(); + self.scorer.lock().payment_path_successful(&path); + }, Event::PaymentSent { payment_hash, .. } => { let mut payment_cache = self.payment_cache.lock().unwrap(); let attempts = payment_cache @@ -1127,7 +1140,9 @@ mod tests { .expect_send(Amount::ForInvoice(final_value_msat)) .expect_send(Amount::OnRetry(final_value_msat / 2)); let router = TestRouter {}; - let scorer = RefCell::new(TestScorer::new().expect_channel_failure(short_channel_id.unwrap())); + let scorer = RefCell::new(TestScorer::new().expect(PaymentPath::Failure { + path: path.clone(), short_channel_id: path[0].short_channel_id, + })); let logger = TestLogger::new(); let invoice_payer = InvoicePayer::new(&payer, router, &scorer, &logger, event_handler, RetryAttempts(2)); @@ -1146,6 +1161,39 @@ mod tests { invoice_payer.handle_event(&event); } + #[test] + fn scores_successful_channels() { + let event_handled = core::cell::RefCell::new(false); + let event_handler = |_: &_| { *event_handled.borrow_mut() = true; }; + + let payment_preimage = PaymentPreimage([1; 32]); + let invoice = invoice(payment_preimage); + let payment_hash = Some(PaymentHash(invoice.payment_hash().clone().into_inner())); + let final_value_msat = invoice.amount_milli_satoshis().unwrap(); + let route = TestRouter::route_for_value(final_value_msat); + + // Expect that scorer is given short_channel_id upon handling the event. + let payer = TestPayer::new().expect_send(Amount::ForInvoice(final_value_msat)); + let router = TestRouter {}; + let scorer = RefCell::new(TestScorer::new() + .expect(PaymentPath::Success { path: route.paths[0].clone() }) + .expect(PaymentPath::Success { path: route.paths[1].clone() }) + ); + let logger = TestLogger::new(); + let invoice_payer = + InvoicePayer::new(&payer, router, &scorer, &logger, event_handler, RetryAttempts(2)); + + let payment_id = invoice_payer.pay_invoice(&invoice).unwrap(); + let event = Event::PaymentPathSuccessful { + payment_id, payment_hash, path: route.paths[0].clone() + }; + invoice_payer.handle_event(&event); + let event = Event::PaymentPathSuccessful { + payment_id, payment_hash, path: route.paths[1].clone() + }; + invoice_payer.handle_event(&event); + } + struct TestRouter; impl TestRouter { @@ -1212,18 +1260,24 @@ mod tests { } struct TestScorer { - expectations: VecDeque, + expectations: Option>, + } + + #[derive(Debug)] + enum PaymentPath { + Failure { path: Vec, short_channel_id: u64 }, + Success { path: Vec }, } impl TestScorer { fn new() -> Self { Self { - expectations: VecDeque::new(), + expectations: None, } } - fn expect_channel_failure(mut self, short_channel_id: u64) -> Self { - self.expectations.push_back(short_channel_id); + fn expect(mut self, expectation: PaymentPath) -> Self { + self.expectations.get_or_insert_with(|| VecDeque::new()).push_back(expectation); self } } @@ -1232,14 +1286,38 @@ mod tests { impl lightning::util::ser::Writeable for TestScorer { fn write(&self, _: &mut W) -> Result<(), std::io::Error> { unreachable!(); } } + impl Score for TestScorer { fn channel_penalty_msat( &self, _short_channel_id: u64, _send_amt: u64, _chan_amt: Option, _source: &NodeId, _target: &NodeId ) -> u64 { 0 } - fn payment_path_failed(&mut self, _path: &[&RouteHop], short_channel_id: u64) { - if let Some(expected_short_channel_id) = self.expectations.pop_front() { - assert_eq!(short_channel_id, expected_short_channel_id); + fn payment_path_failed(&mut self, actual_path: &[&RouteHop], actual_short_channel_id: u64) { + if let Some(expectations) = &mut self.expectations { + match expectations.pop_front() { + Some(PaymentPath::Failure { path, short_channel_id }) => { + assert_eq!(actual_path, &path.iter().collect::>()[..]); + assert_eq!(actual_short_channel_id, short_channel_id); + }, + Some(PaymentPath::Success { path }) => { + panic!("Unexpected successful payment path: {:?}", path) + }, + None => panic!("Unexpected payment_path_failed call: {:?}", actual_path), + } + } + } + + fn payment_path_successful(&mut self, actual_path: &[&RouteHop]) { + if let Some(expectations) = &mut self.expectations { + match expectations.pop_front() { + Some(PaymentPath::Failure { path, .. }) => { + panic!("Unexpected payment path failure: {:?}", path) + }, + Some(PaymentPath::Success { path }) => { + assert_eq!(actual_path, &path.iter().collect::>()[..]); + }, + None => panic!("Unexpected payment_path_successful call: {:?}", actual_path), + } } } } @@ -1250,8 +1328,10 @@ mod tests { return; } - if !self.expectations.is_empty() { - panic!("Unsatisfied channel failure expectations: {:?}", self.expectations); + if let Some(expectations) = &self.expectations { + if !expectations.is_empty() { + panic!("Unsatisfied scorer expectations: {:?}", expectations); + } } } } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 00c3ee6bc..cd7d62e66 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -4665,6 +4665,7 @@ mod tests { } fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} + fn payment_path_successful(&mut self, _path: &[&RouteHop]) {} } struct BadNodeScorer { @@ -4682,6 +4683,7 @@ mod tests { } fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {} + fn payment_path_successful(&mut self, _path: &[&RouteHop]) {} } #[test] diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs index e478dbcbe..967f3f3b5 100644 --- a/lightning/src/routing/scoring.rs +++ b/lightning/src/routing/scoring.rs @@ -95,6 +95,9 @@ pub trait Score $(: $supertrait)* { /// Handles updating channel penalties after failing to route through a channel. fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64); + + /// Handles updating channel penalties after successfully routing along a path. + fn payment_path_successful(&mut self, path: &[&RouteHop]); } impl $(+ $supertrait)*> Score for T { @@ -105,6 +108,10 @@ impl $(+ $supertrait)*> Score for T { fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) { self.deref_mut().payment_path_failed(path, short_channel_id) } + + fn payment_path_successful(&mut self, path: &[&RouteHop]) { + self.deref_mut().payment_path_successful(path) + } } } } @@ -377,6 +384,8 @@ impl Score for ScorerUsingTime { .and_modify(|failure| failure.add_penalty(failure_penalty_msat, half_life)) .or_insert_with(|| ChannelFailure::new(failure_penalty_msat)); } + + fn payment_path_successful(&mut self, _path: &[&RouteHop]) {} } impl Writeable for ScorerUsingTime { -- 2.39.5