Merge pull request #1702 from TheBlueMatt/2022-09-one-hop-retryable
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Thu, 8 Sep 2022 17:34:04 +0000 (17:34 +0000)
committerGitHub <noreply@github.com>
Thu, 8 Sep 2022 17:34:04 +0000 (17:34 +0000)
Mark failed one-hop HTLCs as retrably

1  2 
lightning/src/ln/channelmanager.rs
lightning/src/util/events.rs

index 81bcb3b8c0a1b409e9bef3dfaeb6bdd96644effc,2d1a3c7efdc30a1913bd2819ee32010a161eeb74..80d0fea5b6ca12d5d1db9185560cff11709735c1
@@@ -3844,7 -3844,7 +3844,7 @@@ impl<Signer: Sign, M: Deref, T: Deref, 
                                                        pending_events.push(events::Event::PaymentPathFailed {
                                                                payment_id: Some(payment_id),
                                                                payment_hash,
-                                                               rejected_by_dest: false,
+                                                               payment_failed_permanently: false,
                                                                network_update: None,
                                                                all_paths_failed: payment.get().remaining_parts() == 0,
                                                                path: path.clone(),
                                                        events::Event::PaymentPathFailed {
                                                                payment_id: Some(payment_id),
                                                                payment_hash: payment_hash.clone(),
-                                                               rejected_by_dest: !payment_retryable,
+                                                               payment_failed_permanently: !payment_retryable,
                                                                network_update,
                                                                all_paths_failed,
                                                                path: path.clone(),
                                                events::Event::PaymentPathFailed {
                                                        payment_id: Some(payment_id),
                                                        payment_hash: payment_hash.clone(),
-                                                       rejected_by_dest: path.len() == 1,
+                                                       payment_failed_permanently: false,
                                                        network_update: None,
                                                        all_paths_failed,
                                                        path: path.clone(),
@@@ -5671,6 -5671,10 +5671,6 @@@ wher
        ///
        /// An [`EventHandler`] may safely call back to the provider in order to handle an event.
        /// However, it must not call [`Writeable::write`] as doing so would result in a deadlock.
 -      ///
 -      /// Pending events are persisted as part of [`ChannelManager`]. While these events are cleared
 -      /// when processed, an [`EventHandler`] must be able to handle previously seen events when
 -      /// restarting from an old state.
        fn process_pending_events<H: Deref>(&self, handler: H) where H::Target: EventHandler {
                PersistenceNotifierGuard::optionally_notify(&self.total_consistency_lock, &self.persistence_notifier, || {
                        let mut result = NotifyOption::SkipPersist;
index 0cce3c7f5969b367a36b6ee63ffd817bb10d14b5,47e0c88f549f6f882f310c220d5cc94736dc4f41..be1979c77c93cfb6739ae01ef2da78ba25e75b75
@@@ -376,7 -376,7 +376,7 @@@ pub enum Event 
                /// Indicates the payment was rejected for some reason by the recipient. This implies that
                /// the payment has failed, not just the route in question. If this is not set, you may
                /// retry the payment via a different route.
-               rejected_by_dest: bool,
+               payment_failed_permanently: bool,
                /// Any failure information conveyed via the Onion return packet by a node along the failed
                /// payment route.
                ///
@@@ -643,7 -643,7 +643,7 @@@ impl Writeable for Event 
                                });
                        },
                        &Event::PaymentPathFailed {
-                               ref payment_id, ref payment_hash, ref rejected_by_dest, ref network_update,
+                               ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update,
                                ref all_paths_failed, ref path, ref short_channel_id, ref retry,
                                #[cfg(test)]
                                ref error_code,
                                write_tlv_fields!(writer, {
                                        (0, payment_hash, required),
                                        (1, network_update, option),
-                                       (2, rejected_by_dest, required),
+                                       (2, payment_failed_permanently, required),
                                        (3, all_paths_failed, required),
                                        (5, path, vec_type),
                                        (7, short_channel_id, option),
@@@ -827,7 -827,7 +827,7 @@@ impl MaybeReadable for Event 
                                        #[cfg(test)]
                                        let error_data = Readable::read(reader)?;
                                        let mut payment_hash = PaymentHash([0; 32]);
-                                       let mut rejected_by_dest = false;
+                                       let mut payment_failed_permanently = false;
                                        let mut network_update = None;
                                        let mut all_paths_failed = Some(true);
                                        let mut path: Option<Vec<RouteHop>> = Some(vec![]);
                                        read_tlv_fields!(reader, {
                                                (0, payment_hash, required),
                                                (1, network_update, ignorable),
-                                               (2, rejected_by_dest, required),
+                                               (2, payment_failed_permanently, required),
                                                (3, all_paths_failed, option),
                                                (5, path, vec_type),
                                                (7, short_channel_id, option),
                                        Ok(Some(Event::PaymentPathFailed {
                                                payment_id,
                                                payment_hash,
-                                               rejected_by_dest,
+                                               payment_failed_permanently,
                                                network_update,
                                                all_paths_failed: all_paths_failed.unwrap(),
                                                path: path.unwrap(),
                                                (4, path, vec_type),
                                                (6, short_channel_id, option),
                                        });
 -                                      Ok(Some(Event::ProbeFailed{
 +                                      Ok(Some(Event::ProbeFailed {
                                                payment_id,
                                                payment_hash,
                                                path: path.unwrap(),
                                };
                                f()
                        },
 +                      25u8 => {
 +                              let f = || {
 +                                      let mut prev_channel_id = [0; 32];
 +                                      let mut failed_next_destination_opt = None;
 +                                      read_tlv_fields!(reader, {
 +                                              (0, prev_channel_id, required),
 +                                              (2, failed_next_destination_opt, ignorable),
 +                                      });
 +                                      if let Some(failed_next_destination) = failed_next_destination_opt {
 +                                              Ok(Some(Event::HTLCHandlingFailed {
 +                                                      prev_channel_id,
 +                                                      failed_next_destination,
 +                                              }))
 +                                      } else {
 +                                              // If we fail to read a `failed_next_destination` assume it's because
 +                                              // `MaybeReadable::read` returned `Ok(None)`, though it's also possible we
 +                                              // were simply missing the field.
 +                                              Ok(None)
 +                                      }
 +                              };
 +                              f()
 +                      },
                        // Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue.
                        // Version 0.0.100 failed to properly ignore odd types, possibly resulting in corrupt
                        // reads.
@@@ -1229,17 -1207,11 +1229,17 @@@ pub trait OnionMessageProvider 
  ///
  /// # Requirements
  ///
 -/// See [`process_pending_events`] for requirements around event processing.
 -///
  /// When using this trait, [`process_pending_events`] will call [`handle_event`] for each pending
 -/// event since the last invocation. The handler must either act upon the event immediately
 -/// or preserve it for later handling.
 +/// event since the last invocation.
 +///
 +/// In order to ensure no [`Event`]s are lost, implementors of this trait will persist [`Event`]s
 +/// and replay any unhandled events on startup. An [`Event`] is considered handled when
 +/// [`process_pending_events`] returns, thus handlers MUST fully handle [`Event`]s and persist any
 +/// relevant changes to disk *before* returning.
 +///
 +/// Further, because an application may crash between an [`Event`] being handled and the
 +/// implementor of this trait being re-serialized, [`Event`] handling must be idempotent - in
 +/// effect, [`Event`]s may be replayed.
  ///
  /// Note, handlers may call back into the provider and thus deadlocking must be avoided. Be sure to
  /// consult the provider's documentation on the implication of processing events and how a handler
  pub trait EventsProvider {
        /// Processes any events generated since the last call using the given event handler.
        ///
 -      /// Subsequent calls must only process new events. However, handlers must be capable of handling
 -      /// duplicate events across process restarts. This may occur if the provider was recovered from
 -      /// an old state (i.e., it hadn't been successfully persisted after processing pending events).
 +      /// See the trait-level documentation for requirements.
        fn process_pending_events<H: Deref>(&self, handler: H) where H::Target: EventHandler;
  }