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 a141eb2c3e1445b3e3f08ce2afbd411989b32389..ce942ef574e4c9d570900d96bb7e5e2db7f9b72c 100644 (file)
--- 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<u64>, _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<P: Deref, R, S: Deref, L: Deref, E>
 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::<Vec<_>>();
+                               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<u64>,
+               expectations: Option<VecDeque<PaymentPath>>,
+       }
+
+       #[derive(Debug)]
+       enum PaymentPath {
+               Failure { path: Vec<RouteHop>, short_channel_id: u64 },
+               Success { path: Vec<RouteHop> },
        }
 
        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<W: lightning::util::ser::Writer>(&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<u64>, _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::<Vec<_>>()[..]);
+                                               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::<Vec<_>>()[..]);
+                                       },
+                                       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 00c3ee6bcebc947c5ba44b09ab1b5812f14a7406..cd7d62e6686ed9628d455f5b548ff17a1cfdf091 100644 (file)
--- 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 e478dbcbededa9ff5541dd709d1f951e7073fde0..967f3f3b5cbda150e73e97483f2405df9b223ca4 100644 (file)
--- 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<S: Score, T: DerefMut<Target=S> $(+ $supertrait)*> Score for T {
@@ -105,6 +108,10 @@ impl<S: Score, T: DerefMut<Target=S> $(+ $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<T: Time> Score for ScorerUsingTime<T> {
                        .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<T: Time> Writeable for ScorerUsingTime<T> {