Merge pull request #1038 from valentinewallace/2021-08-keysend-docs
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Tue, 10 Aug 2021 00:56:25 +0000 (00:56 +0000)
committerGitHub <noreply@github.com>
Tue, 10 Aug 2021 00:56:25 +0000 (00:56 +0000)
1  2 
lightning/src/ln/channelmanager.rs

index 09383d40301b9f387cd00ec6060dffc5d6674374,af48ca0dd23eabe62f0dbd87bda542ee52afbe8a..439b5444a547f5cfda5b90378e4ba325cca40119
@@@ -491,8 -491,6 +491,8 @@@ pub struct ChannelManager<Signer: Sign
        /// Because adding or removing an entry is rare, we usually take an outer read lock and then
        /// operate on the inner value freely. Sadly, this prevents parallel operation when opening a
        /// new channel.
 +      ///
 +      /// If also holding `channel_state` lock, must lock `channel_state` prior to `per_peer_state`.
        per_peer_state: RwLock<HashMap<PublicKey, Mutex<PeerState>>>,
  
        pending_events: Mutex<Vec<events::Event>>,
@@@ -873,18 -871,6 +873,18 @@@ macro_rules! try_chan_entry 
        }
  }
  
 +macro_rules! remove_channel {
 +      ($channel_state: expr, $entry: expr) => {
 +              {
 +                      let channel = $entry.remove_entry().1;
 +                      if let Some(short_id) = channel.get_short_channel_id() {
 +                              $channel_state.short_to_id.remove(&short_id);
 +                      }
 +                      channel
 +              }
 +      }
 +}
 +
  macro_rules! handle_monitor_err {
        ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr) => {
                handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, Vec::new(), Vec::new())
@@@ -1179,18 -1165,8 +1179,18 @@@ impl<Signer: Sign, M: Deref, T: Deref, 
                        return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
                }
  
 -              let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
 -              let channel = Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, channel_value_satoshis, push_msat, user_id, config)?;
 +              let channel = {
 +                      let per_peer_state = self.per_peer_state.read().unwrap();
 +                      match per_peer_state.get(&their_network_key) {
 +                              Some(peer_state) => {
 +                                      let peer_state = peer_state.lock().unwrap();
 +                                      let their_features = &peer_state.latest_features;
 +                                      let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
 +                                      Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_id, config)?
 +                              },
 +                              None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
 +                      }
 +              };
                let res = channel.get_open_channel(self.genesis_hash.clone());
  
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
        pub fn close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
  
 -              let (mut failed_htlcs, chan_option) = {
 +              let counterparty_node_id;
 +              let mut failed_htlcs: Vec<(HTLCSource, PaymentHash)>;
 +              let result: Result<(), _> = loop {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_state_lock;
                        match channel_state.by_id.entry(channel_id.clone()) {
                                hash_map::Entry::Occupied(mut chan_entry) => {
 -                                      let (shutdown_msg, failed_htlcs) = chan_entry.get_mut().get_shutdown()?;
 +                                      counterparty_node_id = chan_entry.get().get_counterparty_node_id();
 +                                      let per_peer_state = self.per_peer_state.read().unwrap();
 +                                      let (shutdown_msg, monitor_update, htlcs) = match per_peer_state.get(&counterparty_node_id) {
 +                                              Some(peer_state) => {
 +                                                      let peer_state = peer_state.lock().unwrap();
 +                                                      let their_features = &peer_state.latest_features;
 +                                                      chan_entry.get_mut().get_shutdown(&self.keys_manager, their_features)?
 +                                              },
 +                                              None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", counterparty_node_id) }),
 +                                      };
 +                                      failed_htlcs = htlcs;
 +
 +                                      // Update the monitor with the shutdown script if necessary.
 +                                      if let Some(monitor_update) = monitor_update {
 +                                              if let Err(e) = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), monitor_update) {
 +                                                      let (result, is_permanent) =
 +                                                              handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, false, false, Vec::new(), Vec::new(), chan_entry.key());
 +                                                      if is_permanent {
 +                                                              remove_channel!(channel_state, chan_entry);
 +                                                              break result;
 +                                                      }
 +                                              }
 +                                      }
 +
                                        channel_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown {
 -                                              node_id: chan_entry.get().get_counterparty_node_id(),
 +                                              node_id: counterparty_node_id,
                                                msg: shutdown_msg
                                        });
 +
                                        if chan_entry.get().is_shutdown() {
 -                                              if let Some(short_id) = chan_entry.get().get_short_channel_id() {
 -                                                      channel_state.short_to_id.remove(&short_id);
 +                                              let channel = remove_channel!(channel_state, chan_entry);
 +                                              if let Ok(channel_update) = self.get_channel_update_for_broadcast(&channel) {
 +                                                      channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
 +                                                              msg: channel_update
 +                                                      });
                                                }
 -                                              (failed_htlcs, Some(chan_entry.remove_entry().1))
 -                                      } else { (failed_htlcs, None) }
 +                                      }
 +                                      break Ok(());
                                },
                                hash_map::Entry::Vacant(_) => return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()})
                        }
                };
 +
                for htlc_source in failed_htlcs.drain(..) {
                        self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() });
                }
 -              let chan_update = if let Some(chan) = chan_option {
 -                      self.get_channel_update_for_broadcast(&chan).ok()
 -              } else { None };
 -
 -              if let Some(update) = chan_update {
 -                      let mut channel_state = self.channel_state.lock().unwrap();
 -                      channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
 -                              msg: update
 -                      });
 -              }
  
 +              let _ = handle_error!(self, result, counterparty_node_id);
                Ok(())
        }
  
                        // for now more than 10 paths likely carries too much one-path failure.
                        return Err(PaymentSendFailure::ParameterError(APIError::RouteError{err: "Sending over more than 10 paths is not currently supported"}));
                }
+               if payment_secret.is_none() && route.paths.len() > 1 {
+                       return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError{err: "Payment secret is required for multi-path payments".to_string()}));
+               }
                let mut total_value = 0;
                let our_node_id = self.get_our_node_id();
                let mut path_errs = Vec::with_capacity(route.paths.len());
        /// would be able to guess -- otherwise, an intermediate node may claim the payment and it will
        /// never reach the recipient.
        ///
+       /// See [`send_payment`] documentation for more details on the return value of this function.
+       ///
        /// Similar to regular payments, you MUST NOT reuse a `payment_preimage` value. See
        /// [`send_payment`] for more information about the risks of duplicate preimage usage.
        ///
+       /// Note that `route` must have exactly one path.
+       ///
        /// [`send_payment`]: Self::send_payment
        pub fn send_spontaneous_payment(&self, route: &Route, payment_preimage: Option<PaymentPreimage>) -> Result<PaymentHash, PaymentSendFailure> {
                let preimage = match payment_preimage {
                        return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash".to_owned(), msg.temporary_channel_id.clone()));
                }
  
 -              let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(), their_features, msg, 0, &self.default_configuration)
 +              let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(), &their_features, msg, 0, &self.default_configuration)
                        .map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id))?;
                let mut channel_state_lock = self.channel_state.lock().unwrap();
                let channel_state = &mut *channel_state_lock;
                                        if chan.get().get_counterparty_node_id() != *counterparty_node_id {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.temporary_channel_id));
                                        }
 -                                      try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration, their_features), channel_state, chan);
 +                                      try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration, &their_features), channel_state, chan);
                                        (chan.get().get_value_satoshis(), chan.get().get_funding_redeemscript().to_v0_p2wsh(), chan.get().get_user_id())
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.temporary_channel_id))
        }
  
        fn internal_shutdown(&self, counterparty_node_id: &PublicKey, their_features: &InitFeatures, msg: &msgs::Shutdown) -> Result<(), MsgHandleErrInternal> {
 -              let (mut dropped_htlcs, chan_option) = {
 +              let mut dropped_htlcs: Vec<(HTLCSource, PaymentHash)>;
 +              let result: Result<(), _> = loop {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_state_lock;
  
                                        if chan_entry.get().get_counterparty_node_id() != *counterparty_node_id {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
                                        }
 -                                      let (shutdown, closing_signed, dropped_htlcs) = try_chan_entry!(self, chan_entry.get_mut().shutdown(&self.fee_estimator, &their_features, &msg), channel_state, chan_entry);
 +
 +                                      let (shutdown, closing_signed, monitor_update, htlcs) = try_chan_entry!(self, chan_entry.get_mut().shutdown(&self.fee_estimator, &self.keys_manager, &their_features, &msg), channel_state, chan_entry);
 +                                      dropped_htlcs = htlcs;
 +
 +                                      // Update the monitor with the shutdown script if necessary.
 +                                      if let Some(monitor_update) = monitor_update {
 +                                              if let Err(e) = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), monitor_update) {
 +                                                      let (result, is_permanent) =
 +                                                              handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, false, false, Vec::new(), Vec::new(), chan_entry.key());
 +                                                      if is_permanent {
 +                                                              remove_channel!(channel_state, chan_entry);
 +                                                              break result;
 +                                                      }
 +                                              }
 +                                      }
 +
                                        if let Some(msg) = shutdown {
                                                channel_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown {
 -                                                      node_id: counterparty_node_id.clone(),
 +                                                      node_id: *counterparty_node_id,
                                                        msg,
                                                });
                                        }
                                        if let Some(msg) = closing_signed {
 +                                              // TODO: Do not send this if the monitor update failed.
                                                channel_state.pending_msg_events.push(events::MessageSendEvent::SendClosingSigned {
 -                                                      node_id: counterparty_node_id.clone(),
 +                                                      node_id: *counterparty_node_id,
                                                        msg,
                                                });
                                        }
 -                                      if chan_entry.get().is_shutdown() {
 -                                              if let Some(short_id) = chan_entry.get().get_short_channel_id() {
 -                                                      channel_state.short_to_id.remove(&short_id);
 -                                              }
 -                                              (dropped_htlcs, Some(chan_entry.remove_entry().1))
 -                                      } else { (dropped_htlcs, None) }
 +
 +                                      break Ok(());
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
                        }
                for htlc_source in dropped_htlcs.drain(..) {
                        self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() });
                }
 -              if let Some(chan) = chan_option {
 -                      if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
 -                              let mut channel_state = self.channel_state.lock().unwrap();
 -                              channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
 -                                      msg: update
 -                              });
 -                      }
 -              }
 +
 +              let _ = handle_error!(self, result, *counterparty_node_id);
                Ok(())
        }
  
@@@ -5212,11 -5167,13 +5219,13 @@@ mod tests 
        use bitcoin::hashes::sha256::Hash as Sha256;
        use core::time::Duration;
        use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
+       use ln::channelmanager::PaymentSendFailure;
        use ln::features::{InitFeatures, InvoiceFeatures};
        use ln::functional_test_utils::*;
        use ln::msgs;
        use ln::msgs::ChannelMessageHandler;
        use routing::router::{get_keysend_route, get_route};
+       use util::errors::APIError;
        use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
        use util::test_utils;
  
  
                nodes[1].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "We don't support MPP keysend payments".to_string(), 1);
        }
+       #[test]
+       fn test_multi_hop_missing_secret() {
+               let chanmon_cfgs = create_chanmon_cfgs(4);
+               let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
+               let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
+               let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
+               let chan_1_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
+               let chan_2_id = create_announced_chan_between_nodes(&nodes, 0, 2, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
+               let chan_3_id = create_announced_chan_between_nodes(&nodes, 1, 3, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
+               let chan_4_id = create_announced_chan_between_nodes(&nodes, 2, 3, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
+               let logger = test_utils::TestLogger::new();
+               // Marshall an MPP route.
+               let (_, payment_hash, _) = get_payment_preimage_hash!(&nodes[3]);
+               let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
+               let mut route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[3].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &[], 100000, TEST_FINAL_CLTV, &logger).unwrap();
+               let path = route.paths[0].clone();
+               route.paths.push(path);
+               route.paths[0][0].pubkey = nodes[1].node.get_our_node_id();
+               route.paths[0][0].short_channel_id = chan_1_id;
+               route.paths[0][1].short_channel_id = chan_3_id;
+               route.paths[1][0].pubkey = nodes[2].node.get_our_node_id();
+               route.paths[1][0].short_channel_id = chan_2_id;
+               route.paths[1][1].short_channel_id = chan_4_id;
+               match nodes[0].node.send_payment(&route, payment_hash, &None).unwrap_err() {
+                       PaymentSendFailure::ParameterError(APIError::APIMisuseError { ref err }) => {
+                               assert!(regex::Regex::new(r"Payment secret is required for multi-path payments").unwrap().is_match(err))                        },
+                       _ => panic!("unexpected error")
+               }
+       }
  }
  
  #[cfg(all(any(test, feature = "_test_utils"), feature = "unstable"))]
@@@ -5615,7 -5605,7 +5657,7 @@@ pub mod bench 
        use ln::channelmanager::{BestBlock, ChainParameters, ChannelManager, PaymentHash, PaymentPreimage};
        use ln::features::{InitFeatures, InvoiceFeatures};
        use ln::functional_test_utils::*;
 -      use ln::msgs::ChannelMessageHandler;
 +      use ln::msgs::{ChannelMessageHandler, Init};
        use routing::network_graph::NetworkGraph;
        use routing::router::get_route;
        use util::test_utils;
                });
                let node_b_holder = NodeHolder { node: &node_b };
  
 +              node_a.peer_connected(&node_b.get_our_node_id(), &Init { features: InitFeatures::known() });
 +              node_b.peer_connected(&node_a.get_our_node_id(), &Init { features: InitFeatures::known() });
                node_a.create_channel(node_b.get_our_node_id(), 8_000_000, 100_000_000, 42, None).unwrap();
                node_b.handle_open_channel(&node_a.get_our_node_id(), InitFeatures::known(), &get_event_msg!(node_a_holder, MessageSendEvent::SendOpenChannel, node_b.get_our_node_id()));
                node_a.handle_accept_channel(&node_b.get_our_node_id(), InitFeatures::known(), &get_event_msg!(node_b_holder, MessageSendEvent::SendAcceptChannel, node_a.get_our_node_id()));