From: Jeffrey Czyz Date: Tue, 30 Nov 2021 23:16:05 +0000 (-0600) Subject: Score successful payment paths X-Git-Tag: v0.0.104~16^2~2 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=c36bf9249978dce2b72b3b67d01e22c6c8d78431;p=rust-lightning 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. --- diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index a141eb2c..ce942ef5 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 00c3ee6b..cd7d62e6 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 e478dbcb..967f3f3b 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 {