Move HTLCFailChannelUpdate handling out-of-band

While this isn't neccessary for message ordering consistency, this
does mean that we won't end up processing an HTLCFailChannelUpdate
from a update_fail_htlc prior to it being fully committed (where if
the peer disconnects/reconnects it could theoretically give us a
different result, eg if their next-hop reconnected to them).

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index 7389a678bee3b5a09971c18c8dd4bf590a9213e6..1e396612cf8abf7f885a5fd962fbf5a44975b2fa 100644 (file)
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -1381,11 +1381,21 @@ impl ChannelManager {
                match source {
                        HTLCSource::OutboundRoute { .. } => {
                                mem::drop(channel_state);
-
-                               let mut pending_events = self.pending_events.lock().unwrap();
-                               pending_events.push(events::Event::PaymentFailed {
-                                       payment_hash: payment_hash.clone()
-                               });
+                               if let &HTLCFailReason::ErrorPacket { ref err } = &onion_error {
+                                       let (channel_update, payment_retryable) = self.process_onion_failure(&source, err.data.clone());
+                                       let mut pending_events = self.pending_events.lock().unwrap();
+                                       if let Some(channel_update) = channel_update {
+                                               pending_events.push(events::Event::PaymentFailureNetworkUpdate {
+                                                       update: channel_update,
+                                               });
+                                       }
+                                       pending_events.push(events::Event::PaymentFailed {
+                                               payment_hash: payment_hash.clone(),
+                                               rejected_by_dest: !payment_retryable,
+                                       });
+                               } else {
+                                       panic!("should have onion error packet here");
+                               }
                        },
                        HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret }) => {
                                let err_packet = match onion_error {
@@ -1996,9 +2006,9 @@ impl ChannelManager {
                } else { ((None, true)) }
        }
 
-       fn internal_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<Option<msgs::HTLCFailChannelUpdate>, MsgHandleErrInternal> {
+       fn internal_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<(), MsgHandleErrInternal> {
                let mut channel_state = self.channel_state.lock().unwrap();
-               let htlc_source = match channel_state.by_id.get_mut(&msg.channel_id) {
+               match channel_state.by_id.get_mut(&msg.channel_id) {
                        Some(chan) => {
                                if chan.get_their_node_id() != *their_node_id {
                                        //TODO: here and below MsgHandleErrInternal, #153 case
@@ -2009,17 +2019,7 @@ impl ChannelManager {
                        },
                        None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
                }?;
-
-               // we are the origin node and update route information
-               // also determine if the payment is retryable
-               if let &HTLCSource::OutboundRoute { .. } = htlc_source {
-                       let (channel_update, _payment_retry) = self.process_onion_failure(htlc_source, msg.reason.data.clone());
-                       Ok(channel_update)
-                       // TODO: include pyament_retry info in PaymentFailed event that will be
-                       // fired when receiving revoke_and_ack
-               } else {
-                       Ok(None)
-               }
+               Ok(())
        }
 
        fn internal_update_fail_malformed_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailMalformedHTLC) -> Result<(), MsgHandleErrInternal> {
@@ -2424,7 +2424,7 @@ impl ChannelMessageHandler for ChannelManager {
                handle_error!(self, self.internal_update_fulfill_htlc(their_node_id, msg), their_node_id)
        }
 
-       fn handle_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<Option<msgs::HTLCFailChannelUpdate>, HandleError> {
+       fn handle_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<(), HandleError> {
                handle_error!(self, self.internal_update_fail_htlc(their_node_id, msg), their_node_id)
        }
 
@@ -3296,8 +3296,9 @@ mod tests {
                        let events = origin_node.node.get_and_clear_pending_events();
                        assert_eq!(events.len(), 1);
                        match events[0] {
-                               Event::PaymentFailed { payment_hash } => {
+                               Event::PaymentFailed { payment_hash, rejected_by_dest } => {
                                        assert_eq!(payment_hash, our_payment_hash);
+                                       assert!(rejected_by_dest);
                                },
                                _ => panic!("Unexpected event"),
                        }
@@ -5064,8 +5065,9 @@ mod tests {
                                _ => panic!("Unexpected event"),
                        }
                        match events[1] {
-                               Event::PaymentFailed { payment_hash } => {
+                               Event::PaymentFailed { payment_hash, rejected_by_dest } => {
                                        assert_eq!(payment_hash, payment_hash_5);
+                                       assert!(rejected_by_dest);
                                },
                                _ => panic!("Unexpected event"),
                        }

diff --git a/src/ln/msgs.rs b/src/ln/msgs.rs
index 610158503e6656a462d90f25a41e32bd1cbcd6a7..7b0254bddfd5fbb470cb9e4e6ee3ba8b1162cc52 100644 (file)
--- a/src/ln/msgs.rs
+++ b/src/ln/msgs.rs
@@ -529,7 +529,7 @@ pub trait ChannelMessageHandler : events::EventsProvider + Send + Sync {
        /// Handle an incoming update_fulfill_htlc message from the given peer.
        fn handle_update_fulfill_htlc(&self, their_node_id: &PublicKey, msg: &UpdateFulfillHTLC) -> Result<(), HandleError>;
        /// Handle an incoming update_fail_htlc message from the given peer.
-       fn handle_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &UpdateFailHTLC) -> Result<Option<HTLCFailChannelUpdate>, HandleError>;
+       fn handle_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &UpdateFailHTLC) -> Result<(), HandleError>;
        /// Handle an incoming update_fail_malformed_htlc message from the given peer.
        fn handle_update_fail_malformed_htlc(&self, their_node_id: &PublicKey, msg: &UpdateFailMalformedHTLC) -> Result<(), HandleError>;
        /// Handle an incoming commitment_signed message from the given peer.

diff --git a/src/ln/peer_handler.rs b/src/ln/peer_handler.rs
index 76cc35f64c550b9b401faab6272a9d163383e821..81ef41cf789d05aff4f25f659e97f1716e324d5d 100644 (file)
--- a/src/ln/peer_handler.rs
+++ b/src/ln/peer_handler.rs
@@ -615,10 +615,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                                                        },
                                                                                        131 => {
                                                                                                let msg = try_potential_decodeerror!(msgs::UpdateFailHTLC::read(&mut reader));
-                                                                                               let chan_update = try_potential_handleerror!(self.message_handler.chan_handler.handle_update_fail_htlc(&peer.their_node_id.unwrap(), &msg));
-                                                                                               if let Some(update) = chan_update {
-                                                                                                       self.message_handler.route_handler.handle_htlc_fail_channel_update(&update);
-                                                                                               }
+                                                                                               try_potential_handleerror!(self.message_handler.chan_handler.handle_update_fail_htlc(&peer.their_node_id.unwrap(), &msg));
                                                                                        },
                                                                                        135 => {
                                                                                                let msg = try_potential_decodeerror!(msgs::UpdateFailMalformedHTLC::read(&mut reader));
@@ -906,6 +903,10 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                }
                                                continue;
                                        },
+                                       Event::PaymentFailureNetworkUpdate { ref update } => {
+                                               self.message_handler.route_handler.handle_htlc_fail_channel_update(update);
+                                               continue;
+                                       },
                                        Event::HandleError { ref node_id, ref action } => {
                                                if let Some(ref action) = *action {
                                                        match *action {

diff --git a/src/util/events.rs b/src/util/events.rs
index a317f076658acac1f984f2df6c8b6cbcb2c5ff6e..51417c63c75ef0fd7f5e4694d9d35fca7a9d3cfe 100644 (file)
--- a/src/util/events.rs
+++ b/src/util/events.rs
@@ -75,6 +75,10 @@ pub enum Event {
        PaymentFailed {
                /// The hash which was given to ChannelManager::send_payment.
                payment_hash: [u8; 32],
+               /// 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,
        },
        /// Used to indicate that ChannelManager::process_pending_htlc_forwards should be called at a
        /// time in the future.
@@ -161,6 +165,14 @@ pub enum Event {
                node_id: PublicKey,
                /// The action which should be taken.
                action: Option<msgs::ErrorAction>
+       },
+       /// When a payment fails we may receive updates back from the hop where it failed. In such
+       /// cases this event is generated so that we can inform the router of this information.
+       ///
+       /// This event is handled by PeerManager::process_events if you are using a PeerManager.
+       PaymentFailureNetworkUpdate {
+               /// The channel/node update which should be sent to router
+               update: msgs::HTLCFailChannelUpdate,
        }
 }
 

diff --git a/src/util/test_utils.rs b/src/util/test_utils.rs
index 49aa269b3b44cbe018401cc0386afe5478ccb2aa..f8341e885872a8db2d9aa78ff23fb2124383f814 100644 (file)
--- a/src/util/test_utils.rs
+++ b/src/util/test_utils.rs
@@ -110,7 +110,7 @@ impl msgs::ChannelMessageHandler for TestChannelMessageHandler {
        fn handle_update_fulfill_htlc(&self, _their_node_id: &PublicKey, _msg: &msgs::UpdateFulfillHTLC) -> Result<(), HandleError> {
                Err(HandleError { err: "", action: None })
        }
-       fn handle_update_fail_htlc(&self, _their_node_id: &PublicKey, _msg: &msgs::UpdateFailHTLC) -> Result<Option<msgs::HTLCFailChannelUpdate>, HandleError> {
+       fn handle_update_fail_htlc(&self, _their_node_id: &PublicKey, _msg: &msgs::UpdateFailHTLC) -> Result<(), HandleError> {
                Err(HandleError { err: "", action: None })
        }
        fn handle_update_fail_malformed_htlc(&self, _their_node_id: &PublicKey, _msg: &msgs::UpdateFailMalformedHTLC) -> Result<(), HandleError> {