Merge pull request #1077 from jkczyz/2021-09-failing-route-hop
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Wed, 13 Oct 2021 01:13:41 +0000 (01:13 +0000)
committerGitHub <noreply@github.com>
Wed, 13 Oct 2021 01:13:41 +0000 (01:13 +0000)
Include short channel id in PaymentPathFailed

13 files changed:
lightning/src/chain/mod.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/mod.rs
lightning/src/ln/payment_tests.rs [new file with mode: 0644]
lightning/src/ln/reorg_tests.rs
lightning/src/ln/shutdown_tests.rs
lightning/src/routing/network_graph.rs
lightning/src/routing/router.rs
lightning/src/util/events.rs

index cec09459233daef7b9c38582e17f984d10fa0d6d..718990c4557bc30676179e13bc3afecbbec07e06 100644 (file)
@@ -203,6 +203,9 @@ pub trait Watch<ChannelSigner: Sign> {
        /// with any spends of outputs returned by [`get_outputs_to_watch`]. In practice, this means
        /// calling [`block_connected`] and [`block_disconnected`] on the monitor.
        ///
+       /// Note: this interface MUST error with `ChannelMonitorUpdateErr::PermanentFailure` if
+       /// the given `funding_txo` has previously been registered via `watch_channel`.
+       ///
        /// [`get_outputs_to_watch`]: channelmonitor::ChannelMonitor::get_outputs_to_watch
        /// [`block_connected`]: channelmonitor::ChannelMonitor::block_connected
        /// [`block_disconnected`]: channelmonitor::ChannelMonitor::block_disconnected
index c3262f17374327b42bc40c987350a05a6d8441d9..30088f2eba6815256990d634afa9e7ec1f2c5d53 100644 (file)
@@ -310,7 +310,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
        let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;
        let logger = test_utils::TestLogger::new();
 
-       let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
+       let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
 
        // Now try to send a second payment which will fail to send
        let (payment_preimage_2, payment_hash_2, payment_secret_2) = get_payment_preimage_hash!(nodes[1]);
@@ -346,8 +346,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
                                let events_3 = nodes[0].node.get_and_clear_pending_events();
                                assert_eq!(events_3.len(), 1);
                                match events_3[0] {
-                                       Event::PaymentSent { ref payment_preimage } => {
+                                       Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                                                assert_eq!(*payment_preimage, payment_preimage_1);
+                                               assert_eq!(*payment_hash, payment_hash_1);
                                        },
                                        _ => panic!("Unexpected event"),
                                }
@@ -438,8 +439,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
                        let events_3 = nodes[0].node.get_and_clear_pending_events();
                        assert_eq!(events_3.len(), 1);
                        match events_3[0] {
-                               Event::PaymentSent { ref payment_preimage } => {
+                               Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                                        assert_eq!(*payment_preimage, payment_preimage_1);
+                                       assert_eq!(*payment_hash, payment_hash_1);
                                },
                                _ => panic!("Unexpected event"),
                        }
@@ -1364,7 +1366,7 @@ fn claim_while_disconnected_monitor_update_fail() {
        let logger = test_utils::TestLogger::new();
 
        // Forward a payment for B to claim
-       let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
+       let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
 
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
@@ -1465,8 +1467,9 @@ fn claim_while_disconnected_monitor_update_fail() {
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
        match events[0] {
-               Event::PaymentSent { ref payment_preimage } => {
+               Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                        assert_eq!(*payment_preimage, payment_preimage_1);
+                       assert_eq!(*payment_hash, payment_hash_1);
                },
                _ => panic!("Unexpected event"),
        }
@@ -1847,7 +1850,7 @@ fn monitor_update_claim_fail_no_response() {
        let logger = test_utils::TestLogger::new();
 
        // Forward a payment for B to claim
-       let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
+       let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
 
        // Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
        let (payment_preimage_2, payment_hash_2, payment_secret_2) = get_payment_preimage_hash!(nodes[1]);
@@ -1889,8 +1892,9 @@ fn monitor_update_claim_fail_no_response() {
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
        match events[0] {
-               Event::PaymentSent { ref payment_preimage } => {
+               Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                        assert_eq!(*payment_preimage, payment_preimage_1);
+                       assert_eq!(*payment_hash, payment_hash_1);
                },
                _ => panic!("Unexpected event"),
        }
index 11995c50d7fa0e1314fe3edcfadaca4993b7ec10..7883d7b29e4806643c16513fcfc3aa751fffd709 100644 (file)
@@ -1903,6 +1903,15 @@ impl<Signer: Sign> Channel<Signer> {
                Ok(())
        }
 
+       /// Returns transaction if there is pending funding transaction that is yet to broadcast
+       pub fn unbroadcasted_funding(&self) -> Option<Transaction> {
+                if self.channel_state & (ChannelState::FundingCreated as u32) != 0 {
+                        self.funding_transaction.clone()
+                } else {
+                        None
+                }
+       }
+
        /// Returns a HTLCStats about inbound pending htlcs
        fn get_inbound_pending_htlc_stats(&self) -> HTLCStats {
                let mut stats = HTLCStats {
@@ -5516,7 +5525,7 @@ mod tests {
        use bitcoin::hashes::hex::FromHex;
        use hex;
        use ln::{PaymentPreimage, PaymentHash};
-       use ln::channelmanager::{HTLCSource, MppId};
+       use ln::channelmanager::{HTLCSource, PaymentId};
        use ln::channel::{Channel,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,HTLCCandidate,HTLCInitiator,TxCreationKeys};
        use ln::channel::MAX_FUNDING_SATOSHIS;
        use ln::features::InitFeatures;
@@ -5690,7 +5699,7 @@ mod tests {
                                path: Vec::new(),
                                session_priv: SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(),
                                first_hop_htlc_msat: 548,
-                               mpp_id: MppId([42; 32]),
+                               payment_id: PaymentId([42; 32]),
                        }
                });
 
index e2584c4716a3528491b9df3f5586c3770674debe..298f88ce0cc018162cf5f2a3201512641b1cd8fd 100644 (file)
@@ -172,20 +172,20 @@ struct ClaimableHTLC {
        onion_payload: OnionPayload,
 }
 
-/// A payment identifier used to correlate an MPP payment's per-path HTLC sources internally.
+/// A payment identifier used to uniquely identify a payment to LDK.
 #[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)]
-pub(crate) struct MppId(pub [u8; 32]);
+pub struct PaymentId(pub [u8; 32]);
 
-impl Writeable for MppId {
+impl Writeable for PaymentId {
        fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
                self.0.write(w)
        }
 }
 
-impl Readable for MppId {
+impl Readable for PaymentId {
        fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
                let buf: [u8; 32] = Readable::read(r)?;
-               Ok(MppId(buf))
+               Ok(PaymentId(buf))
        }
 }
 /// Tracks the inbound corresponding to an outbound HTLC
@@ -198,7 +198,7 @@ pub(crate) enum HTLCSource {
                /// Technically we can recalculate this from the route, but we cache it here to avoid
                /// doing a double-pass on route when we get a failure back
                first_hop_htlc_msat: u64,
-               mpp_id: MppId,
+               payment_id: PaymentId,
        },
 }
 #[cfg(test)]
@@ -208,7 +208,7 @@ impl HTLCSource {
                        path: Vec::new(),
                        session_priv: SecretKey::from_slice(&[1; 32]).unwrap(),
                        first_hop_htlc_msat: 0,
-                       mpp_id: MppId([2; 32]),
+                       payment_id: PaymentId([2; 32]),
                }
        }
 }
@@ -400,6 +400,65 @@ struct PendingInboundPayment {
        min_value_msat: Option<u64>,
 }
 
+/// Stores the session_priv for each part of a payment that is still pending. For versions 0.0.102
+/// and later, also stores information for retrying the payment.
+pub(crate) enum PendingOutboundPayment {
+       Legacy {
+               session_privs: HashSet<[u8; 32]>,
+       },
+       Retryable {
+               session_privs: HashSet<[u8; 32]>,
+               payment_hash: PaymentHash,
+               payment_secret: Option<PaymentSecret>,
+               pending_amt_msat: u64,
+               /// The total payment amount across all paths, used to verify that a retry is not overpaying.
+               total_msat: u64,
+               /// Our best known block height at the time this payment was initiated.
+               starting_block_height: u32,
+       },
+}
+
+impl PendingOutboundPayment {
+       fn remove(&mut self, session_priv: &[u8; 32], part_amt_msat: u64) -> bool {
+               let remove_res = match self {
+                       PendingOutboundPayment::Legacy { session_privs } |
+                       PendingOutboundPayment::Retryable { session_privs, .. } => {
+                               session_privs.remove(session_priv)
+                       }
+               };
+               if remove_res {
+                       if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, .. } = self {
+                               *pending_amt_msat -= part_amt_msat;
+                       }
+               }
+               remove_res
+       }
+
+       fn insert(&mut self, session_priv: [u8; 32], part_amt_msat: u64) -> bool {
+               let insert_res = match self {
+                       PendingOutboundPayment::Legacy { session_privs } |
+                       PendingOutboundPayment::Retryable { session_privs, .. } => {
+                               session_privs.insert(session_priv)
+                       }
+               };
+               if insert_res {
+                       if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, .. } = self {
+                               *pending_amt_msat += part_amt_msat;
+                       }
+               }
+               insert_res
+       }
+
+       fn remaining_parts(&self) -> usize {
+               match self {
+                       PendingOutboundPayment::Legacy { session_privs } |
+                       PendingOutboundPayment::Retryable { session_privs, .. } => {
+                               session_privs.len()
+                       }
+               }
+       }
+}
+
 /// SimpleArcChannelManager is useful when you need a ChannelManager with a static lifetime, e.g.
 /// when you're using lightning-net-tokio (since tokio::spawn requires parameters with static
 /// lifetimes). Other times you can afford a reference, which is more efficient, in which case
@@ -486,7 +545,7 @@ pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref,
        /// Locked *after* channel_state.
        pending_inbound_payments: Mutex<HashMap<PaymentHash, PendingInboundPayment>>,
 
-       /// The session_priv bytes of outbound payments which are pending resolution.
+       /// The session_priv bytes and retry metadata of outbound payments which are pending resolution.
        /// The authoritative state of these HTLCs resides either within Channels or ChannelMonitors
        /// (if the channel has been force-closed), however we track them here to prevent duplicative
        /// PaymentSent/PaymentPathFailed events. Specifically, in the case of a duplicative
@@ -495,11 +554,10 @@ pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref,
        /// which may generate a claim event, we may receive similar duplicate claim/fail MonitorEvents
        /// after reloading from disk while replaying blocks against ChannelMonitors.
        ///
-       /// Each payment has each of its MPP part's session_priv bytes in the HashSet of the map (even
-       /// payments over a single path).
+       /// See `PendingOutboundPayment` documentation for more info.
        ///
        /// Locked *after* channel_state.
-       pending_outbound_payments: Mutex<HashMap<MppId, HashSet<[u8; 32]>>>,
+       pending_outbound_payments: Mutex<HashMap<PaymentId, PendingOutboundPayment>>,
 
        our_network_key: SecretKey,
        our_network_pubkey: PublicKey,
@@ -1326,6 +1384,18 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                self.list_channels_with_filter(|&(_, ref channel)| channel.is_live())
        }
 
+       /// Helper function that issues the channel close events
+       fn issue_channel_close_events(&self, channel: &Channel<Signer>, closure_reason: ClosureReason) {
+               let mut pending_events_lock = self.pending_events.lock().unwrap();
+               match channel.unbroadcasted_funding() {
+                       Some(transaction) => {
+                               pending_events_lock.push(events::Event::DiscardFunding { channel_id: channel.channel_id(), transaction })
+                       },
+                       None => {},
+               }
+               pending_events_lock.push(events::Event::ChannelClosed { channel_id: channel.channel_id(), reason: closure_reason });
+       }
+
        fn close_channel_internal(&self, channel_id: &[u8; 32], target_feerate_sats_per_1000_weight: Option<u32>) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 
@@ -1372,12 +1442,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                msg: channel_update
                                                        });
                                                }
-                                               if let Ok(mut pending_events_lock) = self.pending_events.lock() {
-                                                       pending_events_lock.push(events::Event::ChannelClosed {
-                                                               channel_id: *channel_id,
-                                                               reason: ClosureReason::HolderForceClosed
-                                                       });
-                                               }
+                                               self.issue_channel_close_events(&channel, ClosureReason::HolderForceClosed);
                                        }
                                        break Ok(());
                                },
@@ -1468,13 +1533,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                if let Some(short_id) = chan.get().get_short_channel_id() {
                                        channel_state.short_to_id.remove(&short_id);
                                }
-                               let mut pending_events_lock = self.pending_events.lock().unwrap();
                                if peer_node_id.is_some() {
                                        if let Some(peer_msg) = peer_msg {
-                                               pending_events_lock.push(events::Event::ChannelClosed { channel_id: *channel_id, reason: ClosureReason::CounterpartyForceClosed { peer_msg: peer_msg.to_string() } });
+                                               self.issue_channel_close_events(chan.get(),ClosureReason::CounterpartyForceClosed { peer_msg: peer_msg.to_string() });
                                        }
                                } else {
-                                       pending_events_lock.push(events::Event::ChannelClosed { channel_id: *channel_id, reason: ClosureReason::HolderForceClosed });
+                                       self.issue_channel_close_events(chan.get(),ClosureReason::HolderForceClosed);
                                }
                                chan.remove_entry().1
                        } else {
@@ -1878,7 +1942,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        }
 
        // Only public for testing, this should otherwise never be called direcly
-       pub(crate) fn send_payment_along_path(&self, path: &Vec<RouteHop>, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, mpp_id: MppId, keysend_preimage: &Option<PaymentPreimage>) -> Result<(), APIError> {
+       pub(crate) fn send_payment_along_path(&self, path: &Vec<RouteHop>, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>) -> Result<(), APIError> {
                log_trace!(self.logger, "Attempting to send payment for path with next hop {}", path.first().unwrap().short_channel_id);
                let prng_seed = self.keys_manager.get_secure_random_bytes();
                let session_priv_bytes = self.keys_manager.get_secure_random_bytes();
@@ -1893,9 +1957,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash);
 
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
-               let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
-               let sessions = pending_outbounds.entry(mpp_id).or_insert(HashSet::new());
-               assert!(sessions.insert(session_priv_bytes));
 
                let err: Result<(), _> = loop {
                        let mut channel_lock = self.channel_state.lock().unwrap();
@@ -1913,12 +1974,27 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        if !chan.get().is_live() {
                                                return Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected/pending monitor update!".to_owned()});
                                        }
-                                       break_chan_entry!(self, chan.get_mut().send_htlc_and_commit(htlc_msat, payment_hash.clone(), htlc_cltv, HTLCSource::OutboundRoute {
-                                               path: path.clone(),
-                                               session_priv: session_priv.clone(),
-                                               first_hop_htlc_msat: htlc_msat,
-                                               mpp_id,
-                                       }, onion_packet, &self.logger), channel_state, chan)
+                                       let send_res = break_chan_entry!(self, chan.get_mut().send_htlc_and_commit(
+                                               htlc_msat, payment_hash.clone(), htlc_cltv, HTLCSource::OutboundRoute {
+                                                       path: path.clone(),
+                                                       session_priv: session_priv.clone(),
+                                                       first_hop_htlc_msat: htlc_msat,
+                                                       payment_id,
+                                               }, onion_packet, &self.logger),
+                                       channel_state, chan);
+
+                                       let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
+                                       let payment = pending_outbounds.entry(payment_id).or_insert_with(|| PendingOutboundPayment::Retryable {
+                                               session_privs: HashSet::new(),
+                                               pending_amt_msat: 0,
+                                               payment_hash: *payment_hash,
+                                               payment_secret: *payment_secret,
+                                               starting_block_height: self.best_block.read().unwrap().height(),
+                                               total_msat: total_value,
+                                       });
+                                       assert!(payment.insert(session_priv_bytes, path.last().unwrap().fee_msat));
+
+                                       send_res
                                } {
                                        Some((update_add, commitment_signed, monitor_update)) => {
                                                if let Err(e) = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update) {
@@ -1997,11 +2073,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// If a payment_secret *is* provided, we assume that the invoice had the payment_secret feature
        /// bit set (either as required or as available). If multiple paths are present in the Route,
        /// we assume the invoice had the basic_mpp feature set.
-       pub fn send_payment(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>) -> Result<(), PaymentSendFailure> {
-               self.send_payment_internal(route, payment_hash, payment_secret, None)
+       pub fn send_payment(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>) -> Result<PaymentId, PaymentSendFailure> {
+               self.send_payment_internal(route, payment_hash, payment_secret, None, None, None)
        }
 
-       fn send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, keysend_preimage: Option<PaymentPreimage>) -> Result<(), PaymentSendFailure> {
+       fn send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, keysend_preimage: Option<PaymentPreimage>, payment_id: Option<PaymentId>, recv_value_msat: Option<u64>) -> Result<PaymentId, PaymentSendFailure> {
                if route.paths.len() < 1 {
                        return Err(PaymentSendFailure::ParameterError(APIError::RouteError{err: "There must be at least one path to send over"}));
                }
@@ -2017,7 +2093,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                let mut total_value = 0;
                let our_node_id = self.get_our_node_id();
                let mut path_errs = Vec::with_capacity(route.paths.len());
-               let mpp_id = MppId(self.keys_manager.get_secure_random_bytes());
+               let payment_id = if let Some(id) = payment_id { id } else { PaymentId(self.keys_manager.get_secure_random_bytes()) };
                'path_check: for path in route.paths.iter() {
                        if path.len() < 1 || path.len() > 20 {
                                path_errs.push(Err(APIError::RouteError{err: "Path didn't go anywhere/had bogus size"}));
@@ -2035,11 +2111,15 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                if path_errs.iter().any(|e| e.is_err()) {
                        return Err(PaymentSendFailure::PathParameterError(path_errs));
                }
+               if let Some(amt_msat) = recv_value_msat {
+                       debug_assert!(amt_msat >= total_value);
+                       total_value = amt_msat;
+               }
 
                let cur_height = self.best_block.read().unwrap().height() + 1;
                let mut results = Vec::new();
                for path in route.paths.iter() {
-                       results.push(self.send_payment_along_path(&path, &payment_hash, payment_secret, total_value, cur_height, mpp_id, &keysend_preimage));
+                       results.push(self.send_payment_along_path(&path, &payment_hash, payment_secret, total_value, cur_height, payment_id, &keysend_preimage));
                }
                let mut has_ok = false;
                let mut has_err = false;
@@ -2059,10 +2139,58 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                } else if has_err {
                        Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
                } else {
-                       Ok(())
+                       Ok(payment_id)
                }
        }
 
+       /// Retries a payment along the given [`Route`].
+       ///
+       /// Errors returned are a superset of those returned from [`send_payment`], so see
+       /// [`send_payment`] documentation for more details on errors. This method will also error if the
+       /// retry amount puts the payment more than 10% over the payment's total amount, or if the payment
+       /// for the given `payment_id` cannot be found (likely due to timeout or success).
+       ///
+       /// [`send_payment`]: [`ChannelManager::send_payment`]
+       pub fn retry_payment(&self, route: &Route, payment_id: PaymentId) -> Result<(), PaymentSendFailure> {
+               const RETRY_OVERFLOW_PERCENTAGE: u64 = 10;
+               for path in route.paths.iter() {
+                       if path.len() == 0 {
+                               return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+                                       err: "length-0 path in route".to_string()
+                               }))
+                       }
+               }
+
+               let (total_msat, payment_hash, payment_secret) = {
+                       let outbounds = self.pending_outbound_payments.lock().unwrap();
+                       if let Some(payment) = outbounds.get(&payment_id) {
+                               match payment {
+                                       PendingOutboundPayment::Retryable {
+                                               total_msat, payment_hash, payment_secret, pending_amt_msat, ..
+                                       } => {
+                                               let retry_amt_msat: u64 = route.paths.iter().map(|path| path.last().unwrap().fee_msat).sum();
+                                               if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 {
+                                                       return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+                                                               err: format!("retry_amt_msat of {} will put pending_amt_msat (currently: {}) more than 10% over total_payment_amt_msat of {}", retry_amt_msat, pending_amt_msat, total_msat).to_string()
+                                                       }))
+                                               }
+                                               (*total_msat, *payment_hash, *payment_secret)
+                                       },
+                                       PendingOutboundPayment::Legacy { .. } => {
+                                               return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+                                                       err: "Unable to retry payments that were initially sent on LDK versions prior to 0.0.102".to_string()
+                                               }))
+                                       }
+                               }
+                       } else {
+                               return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+                                       err: format!("Payment with ID {} not found", log_bytes!(payment_id.0)),
+                               }))
+                       }
+               };
+               return self.send_payment_internal(route, payment_hash, &payment_secret, None, Some(payment_id), Some(total_msat)).map(|_| ())
+       }
+
        /// Send a spontaneous payment, which is a payment that does not require the recipient to have
        /// generated an invoice. Optionally, you may specify the preimage. If you do choose to specify
        /// the preimage, it must be a cryptographically secure random value that no intermediate node
@@ -2077,14 +2205,14 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// 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> {
+       pub fn send_spontaneous_payment(&self, route: &Route, payment_preimage: Option<PaymentPreimage>) -> Result<(PaymentHash, PaymentId), PaymentSendFailure> {
                let preimage = match payment_preimage {
                        Some(p) => p,
                        None => PaymentPreimage(self.keys_manager.get_secure_random_bytes()),
                };
                let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
-               match self.send_payment_internal(route, payment_hash, &None, Some(preimage)) {
-                       Ok(()) => Ok(payment_hash),
+               match self.send_payment_internal(route, payment_hash, &None, Some(preimage), None, None) {
+                       Ok(payment_id) => Ok((payment_hash, payment_id)),
                        Err(e) => Err(e)
                }
        }
@@ -2875,18 +3003,18 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        self.fail_htlc_backwards_internal(channel_state,
                                                htlc_src, &payment_hash, HTLCFailReason::Reason { failure_code, data: onion_failure_data});
                                },
-                               HTLCSource::OutboundRoute { session_priv, mpp_id, path, .. } => {
+                               HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } => {
                                        let mut session_priv_bytes = [0; 32];
                                        session_priv_bytes.copy_from_slice(&session_priv[..]);
                                        let mut outbounds = self.pending_outbound_payments.lock().unwrap();
-                                       if let hash_map::Entry::Occupied(mut sessions) = outbounds.entry(mpp_id) {
-                                               if sessions.get_mut().remove(&session_priv_bytes) {
+                                       if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
+                                               if payment.get_mut().remove(&session_priv_bytes, path.last().unwrap().fee_msat) {
                                                        self.pending_events.lock().unwrap().push(
                                                                events::Event::PaymentPathFailed {
                                                                        payment_hash,
                                                                        rejected_by_dest: false,
                                                                        network_update: None,
-                                                                       all_paths_failed: sessions.get().len() == 0,
+                                                                       all_paths_failed: payment.get().remaining_parts() == 0,
                                                                        path: path.clone(),
                                                                        short_channel_id: None,
                                                                        #[cfg(test)]
@@ -2895,9 +3023,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                        error_data: None,
                                                                }
                                                        );
-                                                       if sessions.get().len() == 0 {
-                                                               sessions.remove();
-                                                       }
                                                }
                                        } else {
                                                log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@@ -2923,19 +3048,18 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                // from block_connected which may run during initialization prior to the chain_monitor
                // being fully configured. See the docs for `ChannelManagerReadArgs` for more.
                match source {
-                       HTLCSource::OutboundRoute { ref path, session_priv, mpp_id, .. } => {
+                       HTLCSource::OutboundRoute { ref path, session_priv, payment_id, .. } => {
                                let mut session_priv_bytes = [0; 32];
                                session_priv_bytes.copy_from_slice(&session_priv[..]);
                                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
                                let mut all_paths_failed = false;
-                               if let hash_map::Entry::Occupied(mut sessions) = outbounds.entry(mpp_id) {
-                                       if !sessions.get_mut().remove(&session_priv_bytes) {
+                               if let hash_map::Entry::Occupied(mut sessions) = outbounds.entry(payment_id) {
+                                       if !sessions.get_mut().remove(&session_priv_bytes, path.last().unwrap().fee_msat) {
                                                log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
                                                return;
                                        }
-                                       if sessions.get().len() == 0 {
+                                       if sessions.get().remaining_parts() == 0 {
                                                all_paths_failed = true;
-                                               sessions.remove();
                                        }
                                } else {
                                        log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@@ -3184,17 +3308,21 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
        fn claim_funds_internal(&self, mut channel_state_lock: MutexGuard<ChannelHolder<Signer>>, source: HTLCSource, payment_preimage: PaymentPreimage, forwarded_htlc_value_msat: Option<u64>, from_onchain: bool) {
                match source {
-                       HTLCSource::OutboundRoute { session_priv, mpp_id, .. } => {
+                       HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } => {
                                mem::drop(channel_state_lock);
                                let mut session_priv_bytes = [0; 32];
                                session_priv_bytes.copy_from_slice(&session_priv[..]);
                                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
-                               let found_payment = if let Some(mut sessions) = outbounds.remove(&mpp_id) {
-                                       sessions.remove(&session_priv_bytes)
+                               let found_payment = if let Some(mut sessions) = outbounds.remove(&payment_id) {
+                                       sessions.remove(&session_priv_bytes, path.last().unwrap().fee_msat)
                                } else { false };
                                if found_payment {
+                                       let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
                                        self.pending_events.lock().unwrap().push(
-                                               events::Event::PaymentSent { payment_preimage }
+                                               events::Event::PaymentSent {
+                                                       payment_preimage,
+                                                       payment_hash: payment_hash
+                                               }
                                        );
                                } else {
                                        log_trace!(self.logger, "Received duplicative fulfill for HTLC with payment_preimage {}", log_bytes!(payment_preimage.0));
@@ -3434,7 +3562,16 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                Err(e) => try_chan_entry!(self, Err(e), channel_state, chan),
                                        };
                                        if let Err(e) = self.chain_monitor.watch_channel(chan.get().get_funding_txo().unwrap(), monitor) {
-                                               return_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::RevokeAndACKFirst, false, false);
+                                               let mut res = handle_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::RevokeAndACKFirst, false, false);
+                                               if let Err(MsgHandleErrInternal { ref mut shutdown_finish, .. }) = res {
+                                                       // We weren't able to watch the channel to begin with, so no updates should be made on
+                                                       // it. Previously, full_stack_target found an (unreachable) panic when the
+                                                       // monitor update contained within `shutdown_finish` was applied.
+                                                       if let Some((ref mut shutdown_finish, _)) = shutdown_finish {
+                                                               shutdown_finish.0.take();
+                                                       }
+                                               }
+                                               return res
                                        }
                                        funding_tx
                                },
@@ -3577,7 +3714,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        msg: update
                                });
                        }
-                       self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: msg.channel_id,  reason: ClosureReason::CooperativeClosure });
+                       self.issue_channel_close_events(&chan, ClosureReason::CooperativeClosure);
                }
                Ok(())
        }
@@ -3989,7 +4126,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                msg: update
                                                        });
                                                }
-                                               self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(),  reason: ClosureReason::CommitmentTxConfirmed });
+                                               self.issue_channel_close_events(&chan, ClosureReason::CommitmentTxConfirmed);
                                                pending_msg_events.push(events::MessageSendEvent::HandleError {
                                                        node_id: chan.get_counterparty_node_id(),
                                                        action: msgs::ErrorAction::SendErrorMessage {
@@ -4105,12 +4242,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                });
                                                        }
 
-                                                       if let Ok(mut pending_events_lock) = self.pending_events.lock() {
-                                                               pending_events_lock.push(events::Event::ChannelClosed {
-                                                                       channel_id: *channel_id,
-                                                                       reason: ClosureReason::CooperativeClosure
-                                                               });
-                                                       }
+                                                       self.issue_channel_close_events(chan, ClosureReason::CooperativeClosure);
 
                                                        log_info!(self.logger, "Broadcasting {}", log_tx!(tx));
                                                        self.tx_broadcaster.broadcast_transaction(&tx);
@@ -4264,6 +4396,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                self.process_pending_events(&event_handler);
                events.into_inner()
        }
+
+       #[cfg(test)]
+       pub fn has_pending_payments(&self) -> bool {
+               !self.pending_outbound_payments.lock().unwrap().is_empty()
+       }
 }
 
 impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> MessageSendEventsProvider for ChannelManager<Signer, M, T, K, F, L>
@@ -4439,6 +4576,16 @@ where
                payment_secrets.retain(|_, inbound_payment| {
                        inbound_payment.expiry_time > header.time as u64
                });
+
+               let mut outbounds = self.pending_outbound_payments.lock().unwrap();
+               outbounds.retain(|_, payment| {
+                       const PAYMENT_EXPIRY_BLOCKS: u32 = 3;
+                       if payment.remaining_parts() != 0 { return true }
+                       if let PendingOutboundPayment::Retryable { starting_block_height, .. } = payment {
+                               return *starting_block_height + PAYMENT_EXPIRY_BLOCKS > height
+                       }
+                       true
+               });
        }
 
        fn get_relevant_txids(&self) -> Vec<Txid> {
@@ -4532,7 +4679,7 @@ where
                                                        msg: update
                                                });
                                        }
-                                       self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: channel.channel_id(),  reason: ClosureReason::CommitmentTxConfirmed });
+                                       self.issue_channel_close_events(channel, ClosureReason::CommitmentTxConfirmed);
                                        pending_msg_events.push(events::MessageSendEvent::HandleError {
                                                node_id: channel.get_counterparty_node_id(),
                                                action: msgs::ErrorAction::SendErrorMessage { msg: e },
@@ -4723,7 +4870,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
                                                                msg: update
                                                        });
                                                }
-                                               self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(),  reason: ClosureReason::DisconnectedPeer });
+                                               self.issue_channel_close_events(chan, ClosureReason::DisconnectedPeer);
                                                false
                                        } else {
                                                true
@@ -4738,7 +4885,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
                                                        if let Some(short_id) = chan.get_short_channel_id() {
                                                                short_to_id.remove(&short_id);
                                                        }
-                                                       self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(),  reason: ClosureReason::DisconnectedPeer });
+                                                       self.issue_channel_close_events(chan, ClosureReason::DisconnectedPeer);
                                                        return false;
                                                } else {
                                                        no_channels_remain = false;
@@ -5082,23 +5229,23 @@ impl Readable for HTLCSource {
                                let mut session_priv: ::util::ser::OptionDeserWrapper<SecretKey> = ::util::ser::OptionDeserWrapper(None);
                                let mut first_hop_htlc_msat: u64 = 0;
                                let mut path = Some(Vec::new());
-                               let mut mpp_id = None;
+                               let mut payment_id = None;
                                read_tlv_fields!(reader, {
                                        (0, session_priv, required),
-                                       (1, mpp_id, option),
+                                       (1, payment_id, option),
                                        (2, first_hop_htlc_msat, required),
                                        (4, path, vec_type),
                                });
-                               if mpp_id.is_none() {
-                                       // For backwards compat, if there was no mpp_id written, use the session_priv bytes
+                               if payment_id.is_none() {
+                                       // For backwards compat, if there was no payment_id written, use the session_priv bytes
                                        // instead.
-                                       mpp_id = Some(MppId(*session_priv.0.unwrap().as_ref()));
+                                       payment_id = Some(PaymentId(*session_priv.0.unwrap().as_ref()));
                                }
                                Ok(HTLCSource::OutboundRoute {
                                        session_priv: session_priv.0.unwrap(),
                                        first_hop_htlc_msat: first_hop_htlc_msat,
                                        path: path.unwrap(),
-                                       mpp_id: mpp_id.unwrap(),
+                                       payment_id: payment_id.unwrap(),
                                })
                        }
                        1 => Ok(HTLCSource::PreviousHopData(Readable::read(reader)?)),
@@ -5110,12 +5257,12 @@ impl Readable for HTLCSource {
 impl Writeable for HTLCSource {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::io::Error> {
                match self {
-                       HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, mpp_id } => {
+                       HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id } => {
                                0u8.write(writer)?;
-                               let mpp_id_opt = Some(mpp_id);
+                               let payment_id_opt = Some(payment_id);
                                write_tlv_fields!(writer, {
                                        (0, session_priv, required),
-                                       (1, mpp_id_opt, option),
+                                       (1, payment_id_opt, option),
                                        (2, first_hop_htlc_msat, required),
                                        (4, path, vec_type),
                                 });
@@ -5160,6 +5307,20 @@ impl_writeable_tlv_based!(PendingInboundPayment, {
        (8, min_value_msat, required),
 });
 
+impl_writeable_tlv_based_enum!(PendingOutboundPayment,
+       (0, Legacy) => {
+               (0, session_privs, required),
+       },
+       (2, Retryable) => {
+               (0, session_privs, required),
+               (2, payment_hash, required),
+               (4, payment_secret, option),
+               (6, total_msat, required),
+               (8, pending_amt_msat, required),
+               (10, starting_block_height, required),
+       },
+;);
+
 impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable for ChannelManager<Signer, M, T, K, F, L>
        where M::Target: chain::Watch<Signer>,
         T::Target: BroadcasterInterface,
@@ -5250,18 +5411,34 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
                let pending_outbound_payments = self.pending_outbound_payments.lock().unwrap();
                // For backwards compat, write the session privs and their total length.
                let mut num_pending_outbounds_compat: u64 = 0;
-               for (_, outbounds) in pending_outbound_payments.iter() {
-                       num_pending_outbounds_compat += outbounds.len() as u64;
+               for (_, outbound) in pending_outbound_payments.iter() {
+                       num_pending_outbounds_compat += outbound.remaining_parts() as u64;
                }
                num_pending_outbounds_compat.write(writer)?;
-               for (_, outbounds) in pending_outbound_payments.iter() {
-                       for outbound in outbounds.iter() {
-                               outbound.write(writer)?;
+               for (_, outbound) in pending_outbound_payments.iter() {
+                       match outbound {
+                               PendingOutboundPayment::Legacy { session_privs } |
+                               PendingOutboundPayment::Retryable { session_privs, .. } => {
+                                       for session_priv in session_privs.iter() {
+                                               session_priv.write(writer)?;
+                                       }
+                               }
                        }
                }
 
+               // Encode without retry info for 0.0.101 compatibility.
+               let mut pending_outbound_payments_no_retry: HashMap<PaymentId, HashSet<[u8; 32]>> = HashMap::new();
+               for (id, outbound) in pending_outbound_payments.iter() {
+                       match outbound {
+                               PendingOutboundPayment::Legacy { session_privs } |
+                               PendingOutboundPayment::Retryable { session_privs, .. } => {
+                                       pending_outbound_payments_no_retry.insert(*id, session_privs.clone());
+                               }
+                       }
+               }
                write_tlv_fields!(writer, {
-                       (1, pending_outbound_payments, required),
+                       (1, pending_outbound_payments_no_retry, required),
+                       (3, pending_outbound_payments, required),
                });
 
                Ok(())
@@ -5499,6 +5676,16 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                None => continue,
                        }
                }
+               if forward_htlcs_count > 0 {
+                       // If we have pending HTLCs to forward, assume we either dropped a
+                       // `PendingHTLCsForwardable` or the user received it but never processed it as they
+                       // shut down before the timer hit. Either way, set the time_forwardable to a small
+                       // constant as enough time has likely passed that we should simply handle the forwards
+                       // now, or at least after the user gets a chance to reconnect to our peers.
+                       pending_events_read.push(events::Event::PendingHTLCsForwardable {
+                               time_forwardable: Duration::from_secs(2),
+                       });
+               }
 
                let background_event_count: u64 = Readable::read(reader)?;
                let mut pending_background_events_read: Vec<BackgroundEvent> = Vec::with_capacity(cmp::min(background_event_count as usize, MAX_ALLOC_SIZE/mem::size_of::<BackgroundEvent>()));
@@ -5521,21 +5708,33 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                }
 
                let pending_outbound_payments_count_compat: u64 = Readable::read(reader)?;
-               let mut pending_outbound_payments_compat: HashMap<MppId, HashSet<[u8; 32]>> =
+               let mut pending_outbound_payments_compat: HashMap<PaymentId, PendingOutboundPayment> =
                        HashMap::with_capacity(cmp::min(pending_outbound_payments_count_compat as usize, MAX_ALLOC_SIZE/32));
                for _ in 0..pending_outbound_payments_count_compat {
                        let session_priv = Readable::read(reader)?;
-                       if pending_outbound_payments_compat.insert(MppId(session_priv), [session_priv].iter().cloned().collect()).is_some() {
+                       let payment = PendingOutboundPayment::Legacy {
+                               session_privs: [session_priv].iter().cloned().collect()
+                       };
+                       if pending_outbound_payments_compat.insert(PaymentId(session_priv), payment).is_some() {
                                return Err(DecodeError::InvalidValue)
                        };
                }
 
+               // pending_outbound_payments_no_retry is for compatibility with 0.0.101 clients.
+               let mut pending_outbound_payments_no_retry: Option<HashMap<PaymentId, HashSet<[u8; 32]>>> = None;
                let mut pending_outbound_payments = None;
                read_tlv_fields!(reader, {
-                       (1, pending_outbound_payments, option),
+                       (1, pending_outbound_payments_no_retry, option),
+                       (3, pending_outbound_payments, option),
                });
-               if pending_outbound_payments.is_none() {
+               if pending_outbound_payments.is_none() && pending_outbound_payments_no_retry.is_none() {
                        pending_outbound_payments = Some(pending_outbound_payments_compat);
+               } else if pending_outbound_payments.is_none() {
+                       let mut outbounds = HashMap::new();
+                       for (id, session_privs) in pending_outbound_payments_no_retry.unwrap().drain() {
+                               outbounds.insert(id, PendingOutboundPayment::Legacy { session_privs });
+                       }
+                       pending_outbound_payments = Some(outbounds);
                }
 
                let mut secp_ctx = Secp256k1::new();
@@ -5599,7 +5798,7 @@ mod tests {
        use bitcoin::hashes::sha256::Hash as Sha256;
        use core::time::Duration;
        use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
-       use ln::channelmanager::{MppId, PaymentSendFailure};
+       use ln::channelmanager::{PaymentId, PaymentSendFailure};
        use ln::features::{InitFeatures, InvoiceFeatures};
        use ln::functional_test_utils::*;
        use ln::msgs;
@@ -5750,11 +5949,11 @@ mod tests {
                let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
                let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph, &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
                let (payment_preimage, our_payment_hash, payment_secret) = get_payment_preimage_hash!(&nodes[1]);
-               let mpp_id = MppId([42; 32]);
+               let payment_id = PaymentId([42; 32]);
                // Use the utility function send_payment_along_path to send the payment with MPP data which
                // indicates there are more HTLCs coming.
                let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match.
-               nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, mpp_id, &None).unwrap();
+               nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -5784,7 +5983,7 @@ mod tests {
                expect_payment_failed!(nodes[0], our_payment_hash, true);
 
                // Send the second half of the original MPP payment.
-               nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, mpp_id, &None).unwrap();
+               nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -5826,8 +6025,9 @@ mod tests {
                // further events will be generated for subsequence path successes.
                let events = nodes[0].node.get_and_clear_pending_events();
                match events[0] {
-                       Event::PaymentSent { payment_preimage: ref preimage } => {
+                       Event::PaymentSent { payment_preimage: ref preimage, payment_hash: ref hash } => {
                                assert_eq!(payment_preimage, *preimage);
+                               assert_eq!(our_payment_hash, *hash);
                        },
                        _ => panic!("Unexpected event"),
                }
@@ -5880,7 +6080,7 @@ mod tests {
                // To start (2), send a keysend payment but don't claim it.
                let payment_preimage = PaymentPreimage([42; 32]);
                let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph, &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
-               let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
+               let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -5939,7 +6139,7 @@ mod tests {
 
                let test_preimage = PaymentPreimage([42; 32]);
                let mismatch_payment_hash = PaymentHash([43; 32]);
-               let _ = nodes[0].node.send_payment_internal(&route, mismatch_payment_hash, &None, Some(test_preimage)).unwrap();
+               let _ = nodes[0].node.send_payment_internal(&route, mismatch_payment_hash, &None, Some(test_preimage), None, None).unwrap();
                check_added_monitors!(nodes[0], 1);
 
                let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
@@ -5976,7 +6176,7 @@ mod tests {
                let test_preimage = PaymentPreimage([42; 32]);
                let test_secret = PaymentSecret([43; 32]);
                let payment_hash = PaymentHash(Sha256::hash(&test_preimage.0).into_inner());
-               let _ = nodes[0].node.send_payment_internal(&route, payment_hash, &Some(test_secret), Some(test_preimage)).unwrap();
+               let _ = nodes[0].node.send_payment_internal(&route, payment_hash, &Some(test_secret), Some(test_preimage), None, None).unwrap();
                check_added_monitors!(nodes[0], 1);
 
                let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
index 9fbbcfff39c9701306f0c06e0dcc47c0fdd7728b..0d01961cd1b3f023f7bec7c3a6559b330d4bf34d 100644 (file)
@@ -763,21 +763,29 @@ macro_rules! check_closed_broadcast {
        }}
 }
 
-/// Check that a channel's closing channel event has been issued
+/// Check that a channel's closing channel events has been issued
 #[macro_export]
 macro_rules! check_closed_event {
-       ($node: expr, $events: expr, $reason: expr) => {{
+       ($node: expr, $events: expr, $reason: expr) => {
+               check_closed_event!($node, $events, $reason, false);
+       };
+       ($node: expr, $events: expr, $reason: expr, $is_check_discard_funding: expr) => {{
                let events = $node.node.get_and_clear_pending_events();
                assert_eq!(events.len(), $events);
                let expected_reason = $reason;
+               let mut issues_discard_funding = false;
                for event in events {
                        match event {
                                Event::ChannelClosed { ref reason, .. } => {
                                        assert_eq!(*reason, expected_reason);
                                },
+                               Event::DiscardFunding { .. } => {
+                                       issues_discard_funding = true;
+                               }
                                _ => panic!("Unexpected event"),
                        }
                }
+               assert_eq!($is_check_discard_funding, issues_discard_funding);
        }}
 }
 
@@ -965,8 +973,9 @@ macro_rules! commitment_signed_dance {
 macro_rules! get_payment_preimage_hash {
        ($dest_node: expr) => {
                {
-                       let payment_preimage = PaymentPreimage([*$dest_node.network_payment_count.borrow(); 32]);
-                       *$dest_node.network_payment_count.borrow_mut() += 1;
+                       let mut payment_count = $dest_node.network_payment_count.borrow_mut();
+                       let payment_preimage = PaymentPreimage([*payment_count; 32]);
+                       *payment_count += 1;
                        let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
                        let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, None, 7200, 0).unwrap();
                        (payment_preimage, payment_hash, payment_secret)
@@ -981,7 +990,9 @@ macro_rules! get_route_and_payment_hash {
                let net_graph_msg_handler = &$send_node.net_graph_msg_handler;
                let route = get_route(&$send_node.node.get_our_node_id(),
                        &net_graph_msg_handler.network_graph,
-                       &$recv_node.node.get_our_node_id(), None, None, &Vec::new(), $recv_value, TEST_FINAL_CLTV, $send_node.logger).unwrap();
+                       &$recv_node.node.get_our_node_id(), None,
+                       Some(&$send_node.node.list_usable_channels().iter().map(|a| a).collect::<Vec<_>>()),
+                       &Vec::new(), $recv_value, TEST_FINAL_CLTV, $send_node.logger).unwrap();
                (route, payment_hash, payment_preimage, payment_secret)
        }}
 }
@@ -1043,10 +1054,12 @@ macro_rules! expect_payment_received {
 macro_rules! expect_payment_sent {
        ($node: expr, $expected_payment_preimage: expr) => {
                let events = $node.node.get_and_clear_pending_events();
+               let expected_payment_hash = PaymentHash(Sha256::hash(&$expected_payment_preimage.0).into_inner());
                assert_eq!(events.len(), 1);
                match events[0] {
-                       Event::PaymentSent { ref payment_preimage } => {
+                       Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                                assert_eq!($expected_payment_preimage, *payment_preimage);
+                               assert_eq!(expected_payment_hash, *payment_hash);
                        },
                        _ => panic!("Unexpected event"),
                }
index 8d0bb1ec38069d8c53770b148dd80654f1fb0526..9817d6bed62a20b1c5818bfceaf077f40a36b08e 100644 (file)
@@ -19,7 +19,7 @@ use chain::transaction::OutPoint;
 use chain::keysinterface::BaseSign;
 use ln::{PaymentPreimage, PaymentSecret, PaymentHash};
 use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC};
-use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, MppId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA};
+use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA};
 use ln::channel::{Channel, ChannelError};
 use ln::{chan_utils, onion_utils};
 use ln::chan_utils::HTLC_SUCCESS_TX_WEIGHT;
@@ -2521,8 +2521,8 @@ fn test_htlc_on_chain_success() {
        send_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 8000000);
        send_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 8000000);
 
-       let (our_payment_preimage, _payment_hash, _payment_secret) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), 3000000);
-       let (our_payment_preimage_2, _payment_hash_2, _payment_secret_2) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), 3000000);
+       let (our_payment_preimage, payment_hash_1, _payment_secret) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), 3000000);
+       let (our_payment_preimage_2, payment_hash_2, _payment_secret_2) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), 3000000);
 
        // Broadcast legit commitment tx from C on B's chain
        // Broadcast HTLC Success transaction by C on received output from C's commitment tx on B's chain
@@ -2682,12 +2682,13 @@ fn test_htlc_on_chain_success() {
        let mut first_claimed = false;
        for event in events {
                match event {
-                       Event::PaymentSent { payment_preimage } => {
-                               if payment_preimage == our_payment_preimage {
+                       Event::PaymentSent { payment_preimage, payment_hash } => {
+                               if payment_preimage == our_payment_preimage && payment_hash == payment_hash_1 {
                                        assert!(!first_claimed);
                                        first_claimed = true;
                                } else {
                                        assert_eq!(payment_preimage, our_payment_preimage_2);
+                                       assert_eq!(payment_hash, payment_hash_2);
                                }
                        },
                        Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } => {},
@@ -3370,7 +3371,7 @@ fn test_simple_peer_disconnect() {
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
        reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
-       let payment_preimage_3 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).0;
+       let (payment_preimage_3, payment_hash_3, _) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000);
        let payment_preimage_4 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).0;
        let payment_hash_5 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).1;
        let payment_hash_6 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).1;
@@ -3386,8 +3387,9 @@ fn test_simple_peer_disconnect() {
                let events = nodes[0].node.get_and_clear_pending_events();
                assert_eq!(events.len(), 2);
                match events[0] {
-                       Event::PaymentSent { payment_preimage } => {
+                       Event::PaymentSent { payment_preimage, payment_hash } => {
                                assert_eq!(payment_preimage, payment_preimage_3);
+                               assert_eq!(payment_hash, payment_hash_3);
                        },
                        _ => panic!("Unexpected event"),
                }
@@ -3554,8 +3556,9 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
                let events_4 = nodes[0].node.get_and_clear_pending_events();
                assert_eq!(events_4.len(), 1);
                match events_4[0] {
-                       Event::PaymentSent { ref payment_preimage } => {
+                       Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                                assert_eq!(payment_preimage_1, *payment_preimage);
+                               assert_eq!(payment_hash_1, *payment_hash);
                        },
                        _ => panic!("Unexpected event"),
                }
@@ -3594,8 +3597,9 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
                        let events_4 = nodes[0].node.get_and_clear_pending_events();
                        assert_eq!(events_4.len(), 1);
                        match events_4[0] {
-                               Event::PaymentSent { ref payment_preimage } => {
+                               Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                                        assert_eq!(payment_preimage_1, *payment_preimage);
+                                       assert_eq!(payment_hash_1, *payment_hash);
                                },
                                _ => panic!("Unexpected event"),
                        }
@@ -3800,7 +3804,7 @@ fn test_drop_messages_peer_disconnect_dual_htlc() {
        create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
        let logger = test_utils::TestLogger::new();
 
-       let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
+       let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
 
        // Now try to send a second payment which will fail to send
        let (payment_preimage_2, payment_hash_2, payment_secret_2) = get_payment_preimage_hash!(nodes[1]);
@@ -3834,8 +3838,9 @@ fn test_drop_messages_peer_disconnect_dual_htlc() {
                        let events_3 = nodes[0].node.get_and_clear_pending_events();
                        assert_eq!(events_3.len(), 1);
                        match events_3[0] {
-                               Event::PaymentSent { ref payment_preimage } => {
+                               Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                                        assert_eq!(*payment_preimage, payment_preimage_1);
+                                       assert_eq!(*payment_hash, payment_hash_1);
                                },
                                _ => panic!("Unexpected event"),
                        }
@@ -3957,8 +3962,8 @@ fn do_test_htlc_timeout(send_partial_mpp: bool) {
                // Use the utility function send_payment_along_path to send the payment with MPP data which
                // indicates there are more HTLCs coming.
                let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match.
-               let mpp_id = MppId([42; 32]);
-               nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200000, cur_height, mpp_id, &None).unwrap();
+               let payment_id = PaymentId([42; 32]);
+               nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200000, cur_height, payment_id, &None).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -5251,8 +5256,9 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
 
        let events = nodes[0].node.get_and_clear_pending_events();
        match events[0] {
-               Event::PaymentSent { ref payment_preimage } => {
+               Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                        assert_eq!(*payment_preimage, our_payment_preimage);
+                       assert_eq!(*payment_hash, duplicate_payment_hash);
                }
                _ => panic!("Unexpected event"),
        }
@@ -5754,7 +5760,7 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) {
        let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
        let chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
 
-       let (our_payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1]], if use_dust { 50000 } else { 3000000 });
+       let (our_payment_preimage, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], if use_dust { 50000 } else { 3000000 });
 
        // Claim the payment, but don't deliver A's commitment_signed, resulting in the HTLC only being
        // present in B's local commitment transaction, but none of A's commitment transactions.
@@ -5766,8 +5772,9 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) {
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
        match events[0] {
-               Event::PaymentSent { payment_preimage } => {
+               Event::PaymentSent { payment_preimage, payment_hash } => {
                        assert_eq!(payment_preimage, our_payment_preimage);
+                       assert_eq!(payment_hash, our_payment_hash);
                },
                _ => panic!("Unexpected event"),
        }
@@ -6203,8 +6210,9 @@ fn test_free_and_fail_holding_cell_htlcs() {
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
        match events[0] {
-               Event::PaymentSent { ref payment_preimage } => {
+               Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                        assert_eq!(*payment_preimage, payment_preimage_1);
+                       assert_eq!(*payment_hash, payment_hash_1);
                }
                _ => panic!("Unexpected event"),
        }
@@ -8634,7 +8642,7 @@ fn test_pre_lockin_no_chan_closed_update() {
        let channel_id = ::chain::transaction::OutPoint { txid: funding_created_msg.funding_txid, index: funding_created_msg.funding_output_index }.to_channel_id();
        nodes[0].node.handle_error(&nodes[1].node.get_our_node_id(), &msgs::ErrorMessage { channel_id, data: "Hi".to_owned() });
        assert!(nodes[0].chain_monitor.added_monitors.lock().unwrap().is_empty());
-       check_closed_event!(nodes[0], 1, ClosureReason::CounterpartyForceClosed { peer_msg: "Hi".to_string() });
+       check_closed_event!(nodes[0], 2, ClosureReason::CounterpartyForceClosed { peer_msg: "Hi".to_string() }, true);
 }
 
 #[test]
@@ -9210,6 +9218,125 @@ fn test_tx_confirmed_skipping_blocks_immediate_broadcast() {
        do_test_tx_confirmed_skipping_blocks_immediate_broadcast(true);
 }
 
+#[test]
+fn test_forwardable_regen() {
+       // Tests that if we reload a ChannelManager while forwards are pending we will regenerate the
+       // PendingHTLCsForwardable event automatically, ensuring we don't forget to forward/receive
+       // HTLCs.
+       // We test it for both payment receipt and payment forwarding.
+
+       let chanmon_cfgs = create_chanmon_cfgs(3);
+       let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+       let persister: test_utils::TestPersister;
+       let new_chain_monitor: test_utils::TestChainMonitor;
+       let nodes_1_deserialized: ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
+       let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+       create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+       create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
+
+       // First send a payment to nodes[1]
+       let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
+       nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
+       check_added_monitors!(nodes[0], 1);
+
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 1);
+       let payment_event = SendEvent::from_event(events.pop().unwrap());
+       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+       commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
+
+       expect_pending_htlcs_forwardable_ignore!(nodes[1]);
+
+       // Next send a payment which is forwarded by nodes[1]
+       let (route_2, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[2], 200_000);
+       nodes[0].node.send_payment(&route_2, payment_hash_2, &Some(payment_secret_2)).unwrap();
+       check_added_monitors!(nodes[0], 1);
+
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 1);
+       let payment_event = SendEvent::from_event(events.pop().unwrap());
+       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+       commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
+
+       // There is already a PendingHTLCsForwardable event "pending" so another one will not be
+       // generated
+       assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+
+       // Now restart nodes[1] and make sure it regenerates a single PendingHTLCsForwardable
+       nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+       nodes[2].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+
+       let nodes_1_serialized = nodes[1].node.encode();
+       let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new());
+       let mut chan_1_monitor_serialized = test_utils::TestVecWriter(Vec::new());
+       {
+               let monitors = nodes[1].chain_monitor.chain_monitor.monitors.read().unwrap();
+               let mut monitor_iter = monitors.iter();
+               monitor_iter.next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
+               monitor_iter.next().unwrap().1.write(&mut chan_1_monitor_serialized).unwrap();
+       }
+
+       persister = test_utils::TestPersister::new();
+       let keys_manager = &chanmon_cfgs[1].keys_manager;
+       new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[1].chain_source), nodes[1].tx_broadcaster.clone(), nodes[1].logger, node_cfgs[1].fee_estimator, &persister, keys_manager);
+       nodes[1].chain_monitor = &new_chain_monitor;
+
+       let mut chan_0_monitor_read = &chan_0_monitor_serialized.0[..];
+       let (_, mut chan_0_monitor) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
+               &mut chan_0_monitor_read, keys_manager).unwrap();
+       assert!(chan_0_monitor_read.is_empty());
+       let mut chan_1_monitor_read = &chan_1_monitor_serialized.0[..];
+       let (_, mut chan_1_monitor) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
+               &mut chan_1_monitor_read, keys_manager).unwrap();
+       assert!(chan_1_monitor_read.is_empty());
+
+       let mut nodes_1_read = &nodes_1_serialized[..];
+       let (_, nodes_1_deserialized_tmp) = {
+               let mut channel_monitors = HashMap::new();
+               channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor);
+               channel_monitors.insert(chan_1_monitor.get_funding_txo().0, &mut chan_1_monitor);
+               <(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_1_read, ChannelManagerReadArgs {
+                       default_config: UserConfig::default(),
+                       keys_manager,
+                       fee_estimator: node_cfgs[1].fee_estimator,
+                       chain_monitor: nodes[1].chain_monitor,
+                       tx_broadcaster: nodes[1].tx_broadcaster.clone(),
+                       logger: nodes[1].logger,
+                       channel_monitors,
+               }).unwrap()
+       };
+       nodes_1_deserialized = nodes_1_deserialized_tmp;
+       assert!(nodes_1_read.is_empty());
+
+       assert!(nodes[1].chain_monitor.watch_channel(chan_0_monitor.get_funding_txo().0, chan_0_monitor).is_ok());
+       assert!(nodes[1].chain_monitor.watch_channel(chan_1_monitor.get_funding_txo().0, chan_1_monitor).is_ok());
+       nodes[1].node = &nodes_1_deserialized;
+       check_added_monitors!(nodes[1], 2);
+
+       reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+       // Note that nodes[1] and nodes[2] resend their funding_locked here since they haven't updated
+       // the commitment state.
+       reconnect_nodes(&nodes[1], &nodes[2], (true, true), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+
+       assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+       expect_pending_htlcs_forwardable!(nodes[1]);
+       expect_payment_received!(nodes[1], payment_hash, payment_secret, 100_000);
+       check_added_monitors!(nodes[1], 1);
+
+       let mut events = nodes[1].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 1);
+       let payment_event = SendEvent::from_event(events.pop().unwrap());
+       nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]);
+       commitment_signed_dance!(nodes[2], nodes[1], payment_event.commitment_msg, false);
+       expect_pending_htlcs_forwardable!(nodes[2]);
+       expect_payment_received!(nodes[2], payment_hash_2, payment_secret_2, 200_000);
+
+       claim_payment(&nodes[0], &[&nodes[1]], payment_preimage);
+       claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage_2);
+}
+
 #[test]
 fn test_keysend_payments_to_public_node() {
        let chanmon_cfgs = create_chanmon_cfgs(2);
@@ -9226,7 +9353,7 @@ fn test_keysend_payments_to_public_node() {
                         nodes[0].logger).unwrap();
 
        let test_preimage = PaymentPreimage([42; 32]);
-       let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(test_preimage)).unwrap();
+       let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(test_preimage)).unwrap();
        check_added_monitors!(nodes[0], 1);
        let mut events = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(events.len(), 1);
@@ -9256,7 +9383,7 @@ fn test_keysend_payments_to_private_node() {
                                 nodes[0].logger).unwrap();
 
        let test_preimage = PaymentPreimage([42; 32]);
-       let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(test_preimage)).unwrap();
+       let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(test_preimage)).unwrap();
        check_added_monitors!(nodes[0], 1);
        let mut events = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(events.len(), 1);
index b5e433270a51ac9d6a363c9152f4f485fbbe39b2..a2a0b4efee2f32418fcdf503a1c5c4b0db0daccc 100644 (file)
@@ -51,6 +51,9 @@ pub mod wire;
 mod functional_tests;
 #[cfg(test)]
 #[allow(unused_mut)]
+mod payment_tests;
+#[cfg(test)]
+#[allow(unused_mut)]
 mod chanmon_update_fail_tests;
 #[cfg(test)]
 #[allow(unused_mut)]
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
new file mode 100644 (file)
index 0000000..f8ff35f
--- /dev/null
@@ -0,0 +1,254 @@
+// This file is Copyright its original authors, visible in version control
+// history.
+//
+// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
+// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your option.
+// You may not use this file except in accordance with one or both of these
+// licenses.
+
+//! Tests that test the payment retry logic in ChannelManager, including various edge-cases around
+//! serialization ordering between ChannelManager/ChannelMonitors and ensuring we can still retry
+//! payments thereafter.
+
+use ln::{PaymentPreimage, PaymentHash};
+use ln::channelmanager::{PaymentId, PaymentSendFailure};
+use routing::router::get_route;
+use ln::features::{InitFeatures, InvoiceFeatures};
+use ln::msgs;
+use ln::msgs::ChannelMessageHandler;
+use util::test_utils;
+use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
+use util::errors::APIError;
+
+use bitcoin::hashes::sha256::Hash as Sha256;
+use bitcoin::hashes::Hash;
+
+use prelude::*;
+
+use ln::functional_test_utils::*;
+
+#[test]
+fn retry_single_path_payment() {
+       let chanmon_cfgs = create_chanmon_cfgs(3);
+       let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+       let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+       let _chan_0 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+       let _chan_1 = create_announced_chan_between_nodes(&nodes, 2, 1, InitFeatures::known(), InitFeatures::known());
+       // Rebalance to find a route
+       send_payment(&nodes[2], &vec!(&nodes[1])[..], 3_000_000);
+
+       let logger = test_utils::TestLogger::new();
+       let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[2]);
+       let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
+       let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph, &nodes[2].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
+
+       // Rebalance so that the first hop fails.
+       send_payment(&nodes[1], &vec!(&nodes[2])[..], 2_000_000);
+
+       // Make sure the payment fails on the first hop.
+       let payment_id = nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 1);
+       let mut payment_event = SendEvent::from_event(events.pop().unwrap());
+       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+       check_added_monitors!(nodes[1], 0);
+       commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
+       expect_pending_htlcs_forwardable!(nodes[1]);
+       expect_pending_htlcs_forwardable!(&nodes[1]);
+       let htlc_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+       assert!(htlc_updates.update_add_htlcs.is_empty());
+       assert_eq!(htlc_updates.update_fail_htlcs.len(), 1);
+       assert!(htlc_updates.update_fulfill_htlcs.is_empty());
+       assert!(htlc_updates.update_fail_malformed_htlcs.is_empty());
+       check_added_monitors!(nodes[1], 1);
+       nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]);
+       commitment_signed_dance!(nodes[0], nodes[1], htlc_updates.commitment_signed, false);
+       expect_payment_failed!(nodes[0], payment_hash, false);
+
+       // Rebalance the channel so the retry succeeds.
+       send_payment(&nodes[2], &vec!(&nodes[1])[..], 3_000_000);
+
+       // Mine two blocks (we expire retries after 3, so this will check that we don't expire early)
+       connect_blocks(&nodes[0], 2);
+
+       // Retry the payment and make sure it succeeds.
+       nodes[0].node.retry_payment(&route, payment_id).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 1);
+       pass_along_path(&nodes[0], &[&nodes[1], &nodes[2]], 100_000, payment_hash, Some(payment_secret), events.pop().unwrap(), true, None);
+       claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], false, payment_preimage);
+}
+
+#[test]
+fn mpp_retry() {
+       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, 3, 2, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
+       let logger = test_utils::TestLogger::new();
+       // Rebalance
+       send_payment(&nodes[3], &vec!(&nodes[2])[..], 1_500_000);
+
+       let (payment_preimage, payment_hash, payment_secret) = 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, &nodes[3].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &[], 1_000_000, 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;
+
+       // Initiate the MPP payment.
+       let payment_id = nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
+       check_added_monitors!(nodes[0], 2); // one monitor per path
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 2);
+
+       // Pass half of the payment along the success path.
+       let success_path_msgs = events.remove(0);
+       pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 2_000_000, payment_hash, Some(payment_secret), success_path_msgs, false, None);
+
+       // Add the HTLC along the first hop.
+       let fail_path_msgs_1 = events.remove(0);
+       let (update_add, commitment_signed) = match fail_path_msgs_1 {
+               MessageSendEvent::UpdateHTLCs { node_id: _, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, ref update_fee, ref commitment_signed } } => {
+                       assert_eq!(update_add_htlcs.len(), 1);
+                       assert!(update_fail_htlcs.is_empty());
+                       assert!(update_fulfill_htlcs.is_empty());
+                       assert!(update_fail_malformed_htlcs.is_empty());
+                       assert!(update_fee.is_none());
+                       (update_add_htlcs[0].clone(), commitment_signed.clone())
+               },
+               _ => panic!("Unexpected event"),
+       };
+       nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add);
+       commitment_signed_dance!(nodes[2], nodes[0], commitment_signed, false);
+
+       // Attempt to forward the payment and complete the 2nd path's failure.
+       expect_pending_htlcs_forwardable!(&nodes[2]);
+       expect_pending_htlcs_forwardable!(&nodes[2]);
+       let htlc_updates = get_htlc_update_msgs!(nodes[2], nodes[0].node.get_our_node_id());
+       assert!(htlc_updates.update_add_htlcs.is_empty());
+       assert_eq!(htlc_updates.update_fail_htlcs.len(), 1);
+       assert!(htlc_updates.update_fulfill_htlcs.is_empty());
+       assert!(htlc_updates.update_fail_malformed_htlcs.is_empty());
+       check_added_monitors!(nodes[2], 1);
+       nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]);
+       commitment_signed_dance!(nodes[0], nodes[2], htlc_updates.commitment_signed, false);
+       expect_payment_failed!(nodes[0], payment_hash, false);
+
+       // Rebalance the channel so the second half of the payment can succeed.
+       send_payment(&nodes[3], &vec!(&nodes[2])[..], 1_500_000);
+
+       // Make sure it errors as expected given a too-large amount.
+       if let Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { err })) = nodes[0].node.retry_payment(&route, payment_id) {
+               assert!(err.contains("over total_payment_amt_msat"));
+       } else { panic!("Unexpected error"); }
+
+       // Make sure it errors as expected given the wrong payment_id.
+       if let Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { err })) = nodes[0].node.retry_payment(&route, PaymentId([0; 32])) {
+               assert!(err.contains("not found"));
+       } else { panic!("Unexpected error"); }
+
+       // Retry the second half of the payment and make sure it succeeds.
+       let mut path = route.clone();
+       path.paths.remove(0);
+       nodes[0].node.retry_payment(&path, payment_id).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 1);
+       pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 2_000_000, payment_hash, Some(payment_secret), events.pop().unwrap(), true, None);
+       claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
+}
+
+#[test]
+fn retry_expired_payment() {
+       let chanmon_cfgs = create_chanmon_cfgs(3);
+       let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+       let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+       let _chan_0 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+       let _chan_1 = create_announced_chan_between_nodes(&nodes, 2, 1, InitFeatures::known(), InitFeatures::known());
+       // Rebalance to find a route
+       send_payment(&nodes[2], &vec!(&nodes[1])[..], 3_000_000);
+
+       let logger = test_utils::TestLogger::new();
+       let (_payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[2]);
+       let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
+       let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph, &nodes[2].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
+
+       // Rebalance so that the first hop fails.
+       send_payment(&nodes[1], &vec!(&nodes[2])[..], 2_000_000);
+
+       // Make sure the payment fails on the first hop.
+       let payment_id = nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
+       check_added_monitors!(nodes[0], 1);
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 1);
+       let mut payment_event = SendEvent::from_event(events.pop().unwrap());
+       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+       check_added_monitors!(nodes[1], 0);
+       commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
+       expect_pending_htlcs_forwardable!(nodes[1]);
+       expect_pending_htlcs_forwardable!(&nodes[1]);
+       let htlc_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+       assert!(htlc_updates.update_add_htlcs.is_empty());
+       assert_eq!(htlc_updates.update_fail_htlcs.len(), 1);
+       assert!(htlc_updates.update_fulfill_htlcs.is_empty());
+       assert!(htlc_updates.update_fail_malformed_htlcs.is_empty());
+       check_added_monitors!(nodes[1], 1);
+       nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]);
+       commitment_signed_dance!(nodes[0], nodes[1], htlc_updates.commitment_signed, false);
+       expect_payment_failed!(nodes[0], payment_hash, false);
+
+       // Mine blocks so the payment will have expired.
+       connect_blocks(&nodes[0], 3);
+
+       // Retry the payment and make sure it errors as expected.
+       if let Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { err })) = nodes[0].node.retry_payment(&route, payment_id) {
+               assert!(err.contains("not found"));
+       } else {
+               panic!("Unexpected error");
+       }
+}
+
+#[test]
+fn no_pending_leak_on_initial_send_failure() {
+       // In an earlier version of our payment tracking, we'd have a retry entry even when the initial
+       // HTLC for payment failed to send due to local channel errors (e.g. peer disconnected). In this
+       // case, the user wouldn't have a PaymentId to retry the payment with, but we'd think we have a
+       // pending payment forever and never time it out.
+       // Here we test exactly that - retrying a payment when a peer was disconnected on the first
+       // try, and then check that no pending payment is being tracked.
+       let chanmon_cfgs = create_chanmon_cfgs(2);
+       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+       let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+       create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+
+       let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
+
+       nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+       nodes[1].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+
+       unwrap_send_err!(nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)),
+               true, APIError::ChannelUnavailable { ref err },
+               assert_eq!(err, "Peer for first hop currently disconnected/pending monitor update!"));
+
+       assert!(!nodes[0].node.has_pending_payments());
+}
index aca67c8f9f0ebe40980777f0c24f14799b742660..0409396db11696310cb4ad1a61b7d83870121970 100644 (file)
@@ -12,6 +12,7 @@
 use chain::channelmonitor::{ANTI_REORG_DELAY, ChannelMonitor};
 use chain::transaction::OutPoint;
 use chain::{Confirm, Watch};
+use ln::PaymentHash;
 use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs};
 use ln::features::InitFeatures;
 use ln::msgs::{ChannelMessageHandler, ErrorAction};
@@ -24,6 +25,8 @@ use util::ser::{ReadableArgs, Writeable};
 use bitcoin::blockdata::block::{Block, BlockHeader};
 use bitcoin::blockdata::script::Builder;
 use bitcoin::blockdata::opcodes;
+use bitcoin::hashes::sha256::Hash as Sha256;
+use bitcoin::hashes::Hash;
 use bitcoin::hash_types::BlockHash;
 use bitcoin::secp256k1::Secp256k1;
 
index d13b7d2df12fece9a5d78092a21caa73e816451b..c2908e2652ed141cc70d0838e3d976c8709feab8 100644 (file)
@@ -82,7 +82,7 @@ fn updates_shutdown_wait() {
        let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
        let logger = test_utils::TestLogger::new();
 
-       let (our_payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000);
+       let (our_payment_preimage, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000);
 
        nodes[0].node.close_channel(&chan_1.2).unwrap();
        let node_0_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id());
@@ -127,8 +127,9 @@ fn updates_shutdown_wait() {
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
        match events[0] {
-               Event::PaymentSent { ref payment_preimage } => {
+               Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                        assert_eq!(our_payment_preimage, *payment_preimage);
+                       assert_eq!(our_payment_hash, *payment_hash);
                },
                _ => panic!("Unexpected event"),
        }
@@ -242,7 +243,7 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) {
        let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
        let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
 
-       let (our_payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000);
+       let (our_payment_preimage, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000);
 
        nodes[1].node.close_channel(&chan_1.2).unwrap();
        let node_1_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id());
@@ -307,8 +308,9 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) {
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
        match events[0] {
-               Event::PaymentSent { ref payment_preimage } => {
+               Event::PaymentSent { ref payment_preimage, ref payment_hash } => {
                        assert_eq!(our_payment_preimage, *payment_preimage);
+                       assert_eq!(our_payment_hash, *payment_hash);
                },
                _ => panic!("Unexpected event"),
        }
index 19786947f98d987c465ffdce7dcd0ad4dc281405..961939cb7e6dedd4126c8a9daf7e4d839135bc6e 100644 (file)
@@ -9,6 +9,7 @@
 
 //! The top-level network map tracking logic lives here.
 
+use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE;
 use bitcoin::secp256k1::key::PublicKey;
 use bitcoin::secp256k1::Secp256k1;
 use bitcoin::secp256k1;
@@ -50,12 +51,75 @@ const MAX_EXCESS_BYTES_FOR_RELAY: usize = 1024;
 /// This value ensures a reply fits within the 65k payload limit and is consistent with other implementations.
 const MAX_SCIDS_PER_REPLY: usize = 8000;
 
+/// Represents the compressed public key of a node
+#[derive(Clone, Copy)]
+pub struct NodeId([u8; PUBLIC_KEY_SIZE]);
+
+impl NodeId {
+       /// Create a new NodeId from a public key
+       pub fn from_pubkey(pubkey: &PublicKey) -> Self {
+               NodeId(pubkey.serialize())
+       }
+       
+       /// Get the public key slice from this NodeId
+       pub fn as_slice(&self) -> &[u8] {
+               &self.0
+       }
+}
+
+impl fmt::Debug for NodeId {
+       fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+               write!(f, "NodeId({})", log_bytes!(self.0))
+       }
+}
+
+impl core::hash::Hash for NodeId {
+       fn hash<H: core::hash::Hasher>(&self, hasher: &mut H) {
+               self.0.hash(hasher);
+       }
+}
+
+impl Eq for NodeId {}
+
+impl PartialEq for NodeId {
+       fn eq(&self, other: &Self) -> bool {
+               self.0[..] == other.0[..]
+       }
+}
+
+impl cmp::PartialOrd for NodeId {
+       fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
+               Some(self.cmp(other))
+       }
+}
+
+impl Ord for NodeId {
+       fn cmp(&self, other: &Self) -> cmp::Ordering {
+               self.0[..].cmp(&other.0[..])
+       }
+}
+
+impl Writeable for NodeId {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               writer.write_all(&self.0)?;
+               Ok(())
+       }
+}
+
+impl Readable for NodeId {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+               let mut buf = [0; PUBLIC_KEY_SIZE];
+               reader.read_exact(&mut buf)?;
+               Ok(Self(buf))
+       }
+}
+
 /// Represents the network as nodes and channels between them
 pub struct NetworkGraph {
        genesis_hash: BlockHash,
        // Lock order: channels -> nodes
        channels: RwLock<BTreeMap<u64, ChannelInfo>>,
-       nodes: RwLock<BTreeMap<PublicKey, NodeInfo>>,
+       nodes: RwLock<BTreeMap<NodeId, NodeInfo>>,
 }
 
 impl Clone for NetworkGraph {
@@ -73,7 +137,7 @@ impl Clone for NetworkGraph {
 /// A read-only view of [`NetworkGraph`].
 pub struct ReadOnlyNetworkGraph<'a> {
        channels: RwLockReadGuard<'a, BTreeMap<u64, ChannelInfo>>,
-       nodes: RwLockReadGuard<'a, BTreeMap<PublicKey, NodeInfo>>,
+       nodes: RwLockReadGuard<'a, BTreeMap<NodeId, NodeInfo>>,
 }
 
 /// Update to the [`NetworkGraph`] based on payment failure information conveyed via the Onion
@@ -277,11 +341,11 @@ where C::Target: chain::Access, L::Target: Logger
                let mut result = Vec::with_capacity(batch_amount as usize);
                let nodes = self.network_graph.nodes.read().unwrap();
                let mut iter = if let Some(pubkey) = starting_point {
-                               let mut iter = nodes.range((*pubkey)..);
+                               let mut iter = nodes.range(NodeId::from_pubkey(pubkey)..);
                                iter.next();
                                iter
                        } else {
-                               nodes.range(..)
+                               nodes.range::<NodeId, _>(..)
                        };
                while result.len() < batch_amount as usize {
                        if let Some((_, ref node)) = iter.next() {
@@ -314,7 +378,7 @@ where C::Target: chain::Access, L::Target: Logger
                }
 
                // Check if we need to perform a full synchronization with this peer
-               if !self.should_request_full_sync(their_node_id) {
+               if !self.should_request_full_sync(&their_node_id) {
                        return ();
                }
 
@@ -551,11 +615,11 @@ pub struct ChannelInfo {
        /// Protocol features of a channel communicated during its announcement
        pub features: ChannelFeatures,
        /// Source node of the first direction of a channel
-       pub node_one: PublicKey,
+       pub node_one: NodeId,
        /// Details about the first direction of a channel
        pub one_to_two: Option<DirectionalChannelInfo>,
        /// Source node of the second direction of a channel
-       pub node_two: PublicKey,
+       pub node_two: NodeId,
        /// Details about the second direction of a channel
        pub two_to_one: Option<DirectionalChannelInfo>,
        /// The channel capacity as seen on-chain, if chain lookup is available.
@@ -570,7 +634,7 @@ pub struct ChannelInfo {
 impl fmt::Display for ChannelInfo {
        fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
                write!(f, "features: {}, node_one: {}, one_to_two: {:?}, node_two: {}, two_to_one: {:?}",
-                  log_bytes!(self.features.encode()), log_pubkey!(self.node_one), self.one_to_two, log_pubkey!(self.node_two), self.two_to_one)?;
+                  log_bytes!(self.features.encode()), log_bytes!(self.node_one.as_slice()), self.one_to_two, log_bytes!(self.node_two.as_slice()), self.two_to_one)?;
                Ok(())
        }
 }
@@ -724,8 +788,8 @@ impl fmt::Display for NetworkGraph {
                        writeln!(f, " {}: {}", key, val)?;
                }
                writeln!(f, "[Nodes]")?;
-               for (key, val) in self.nodes.read().unwrap().iter() {
-                       writeln!(f, " {}: {}", log_pubkey!(key), val)?;
+               for (&node_id, val) in self.nodes.read().unwrap().iter() {
+                       writeln!(f, " {}: {}", log_bytes!(node_id.as_slice()), val)?;
                }
                Ok(())
        }
@@ -780,7 +844,7 @@ impl NetworkGraph {
        }
 
        fn update_node_from_announcement_intern(&self, msg: &msgs::UnsignedNodeAnnouncement, full_msg: Option<&msgs::NodeAnnouncement>) -> Result<(), LightningError> {
-               match self.nodes.write().unwrap().get_mut(&msg.node_id) {
+               match self.nodes.write().unwrap().get_mut(&NodeId::from_pubkey(&msg.node_id)) {
                        None => Err(LightningError{err: "No existing channels for node_announcement".to_owned(), action: ErrorAction::IgnoreError}),
                        Some(node) => {
                                if let Some(node_info) = node.announcement_info.as_ref() {
@@ -886,9 +950,9 @@ impl NetworkGraph {
 
                let chan_info = ChannelInfo {
                                features: msg.features.clone(),
-                               node_one: msg.node_id_1.clone(),
+                               node_one: NodeId::from_pubkey(&msg.node_id_1),
                                one_to_two: None,
-                               node_two: msg.node_id_2.clone(),
+                               node_two: NodeId::from_pubkey(&msg.node_id_2),
                                two_to_one: None,
                                capacity_sats: utxo_value,
                                announcement_message: if msg.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY
@@ -939,8 +1003,8 @@ impl NetworkGraph {
                        };
                }
 
-               add_channel_to_node!(msg.node_id_1);
-               add_channel_to_node!(msg.node_id_2);
+               add_channel_to_node!(NodeId::from_pubkey(&msg.node_id_1));
+               add_channel_to_node!(NodeId::from_pubkey(&msg.node_id_2));
 
                Ok(())
        }
@@ -1050,13 +1114,19 @@ impl NetworkGraph {
                                if msg.flags & 1 == 1 {
                                        dest_node_id = channel.node_one.clone();
                                        if let Some((sig, ctx)) = sig_info {
-                                               secp_verify_sig!(ctx, &msg_hash, &sig, &channel.node_two);
+                                               secp_verify_sig!(ctx, &msg_hash, &sig, &PublicKey::from_slice(channel.node_two.as_slice()).map_err(|_| LightningError{
+                                                       err: "Couldn't parse source node pubkey".to_owned(),
+                                                       action: ErrorAction::IgnoreAndLog(Level::Debug)
+                                               })?);
                                        }
                                        maybe_update_channel_info!(channel.two_to_one, channel.node_two);
                                } else {
                                        dest_node_id = channel.node_two.clone();
                                        if let Some((sig, ctx)) = sig_info {
-                                               secp_verify_sig!(ctx, &msg_hash, &sig, &channel.node_one);
+                                               secp_verify_sig!(ctx, &msg_hash, &sig, &PublicKey::from_slice(channel.node_one.as_slice()).map_err(|_| LightningError{
+                                                       err: "Couldn't parse destination node pubkey".to_owned(),
+                                                       action: ErrorAction::IgnoreAndLog(Level::Debug)
+                                               })?);
                                        }
                                        maybe_update_channel_info!(channel.one_to_two, channel.node_one);
                                }
@@ -1104,7 +1174,7 @@ impl NetworkGraph {
                Ok(())
        }
 
-       fn remove_channel_in_nodes(nodes: &mut BTreeMap<PublicKey, NodeInfo>, chan: &ChannelInfo, short_channel_id: u64) {
+       fn remove_channel_in_nodes(nodes: &mut BTreeMap<NodeId, NodeInfo>, chan: &ChannelInfo, short_channel_id: u64) {
                macro_rules! remove_from_node {
                        ($node_id: expr) => {
                                if let BtreeEntry::Occupied(mut entry) = nodes.entry($node_id) {
@@ -1136,7 +1206,7 @@ impl ReadOnlyNetworkGraph<'_> {
        /// Returns all known nodes' public keys along with announced node info.
        ///
        /// (C-not exported) because we have no mapping for `BTreeMap`s
-       pub fn nodes(&self) -> &BTreeMap<PublicKey, NodeInfo> {
+       pub fn nodes(&self) -> &BTreeMap<NodeId, NodeInfo> {
                &*self.nodes
        }
 
@@ -1146,7 +1216,7 @@ impl ReadOnlyNetworkGraph<'_> {
        ///
        /// (C-not exported) as there is no practical way to track lifetimes of returned values.
        pub fn get_addresses(&self, pubkey: &PublicKey) -> Option<&Vec<NetAddress>> {
-               if let Some(node) = self.nodes.get(pubkey) {
+               if let Some(node) = self.nodes.get(&NodeId::from_pubkey(&pubkey)) {
                        if let Some(node_info) = node.announcement_info.as_ref() {
                                return Some(&node_info.addresses)
                        }
index 325920b778151bafbfa98c8d610af77b7cb7ca41..d6adb889d5060fe7e4122ba42e733a5e6615584a 100644 (file)
@@ -17,9 +17,9 @@ use bitcoin::secp256k1::key::PublicKey;
 use ln::channelmanager::ChannelDetails;
 use ln::features::{ChannelFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
-use routing::network_graph::{NetworkGraph, RoutingFees};
+use routing::network_graph::{NetworkGraph, RoutingFees, NodeId};
 use util::ser::{Writeable, Readable};
-use util::logger::Logger;
+use util::logger::{Level, Logger};
 
 use io;
 use prelude::*;
@@ -151,7 +151,7 @@ pub struct RouteHintHop {
 
 #[derive(Eq, PartialEq)]
 struct RouteGraphNode {
-       pubkey: PublicKey,
+       node_id: NodeId,
        lowest_fee_to_peer_through_node: u64,
        lowest_fee_to_node: u64,
        // The maximum value a yet-to-be-constructed payment path might flow through this node.
@@ -169,7 +169,7 @@ impl cmp::Ord for RouteGraphNode {
        fn cmp(&self, other: &RouteGraphNode) -> cmp::Ordering {
                let other_score = cmp::max(other.lowest_fee_to_peer_through_node, other.path_htlc_minimum_msat);
                let self_score = cmp::max(self.lowest_fee_to_peer_through_node, self.path_htlc_minimum_msat);
-               other_score.cmp(&self_score).then_with(|| other.pubkey.serialize().cmp(&self.pubkey.serialize()))
+               other_score.cmp(&self_score).then_with(|| other.node_id.cmp(&self.node_id))
        }
 }
 
@@ -194,7 +194,7 @@ struct DummyDirectionalChannelInfo {
 struct PathBuildingHop<'a> {
        // The RouteHintHop fields which will eventually be used if this hop is used in a final Route.
        // Note that node_features is calculated separately after our initial graph walk.
-       pubkey: PublicKey,
+       node_id: NodeId,
        short_channel_id: u64,
        channel_features: &'a ChannelFeatures,
        fee_msat: u64,
@@ -352,12 +352,12 @@ fn compute_fees(amount_msat: u64, channel_fees: RoutingFees) -> Option<u64> {
 /// Gets a keysend route from us (payer) to the given target node (payee). This is needed because
 /// keysend payments do not have an invoice from which to pull the payee's supported features, which
 /// makes it tricky to otherwise supply the `payee_features` parameter of `get_route`.
-pub fn get_keysend_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, payee:
+pub fn get_keysend_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph, payee:
                        &PublicKey, first_hops: Option<&[&ChannelDetails]>, last_hops: &[&RouteHint],
                        final_value_msat: u64, final_cltv: u32, logger: L) -> Result<Route,
                        LightningError> where L::Target: Logger {
        let invoice_features = InvoiceFeatures::for_keysend();
-       get_route(our_node_id, network, payee, Some(invoice_features), first_hops, last_hops,
+       get_route(our_node_pubkey, network, payee, Some(invoice_features), first_hops, last_hops,
             final_value_msat, final_cltv, logger)
 }
 
@@ -380,11 +380,14 @@ pub fn get_keysend_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGra
 /// The fees on channels from us to next-hops are ignored (as they are assumed to all be
 /// equal), however the enabled/disabled bit on such channels as well as the
 /// htlc_minimum_msat/htlc_maximum_msat *are* checked as they may change based on the receiving node.
-pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, payee: &PublicKey, payee_features: Option<InvoiceFeatures>, first_hops: Option<&[&ChannelDetails]>,
+pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph, payee: &PublicKey, payee_features: Option<InvoiceFeatures>, first_hops: Option<&[&ChannelDetails]>,
        last_hops: &[&RouteHint], final_value_msat: u64, final_cltv: u32, logger: L) -> Result<Route, LightningError> where L::Target: Logger {
+       let payee_node_id = NodeId::from_pubkey(&payee);
+       let our_node_id = NodeId::from_pubkey(&our_node_pubkey);
+
        // TODO: Obviously *only* using total fee cost sucks. We should consider weighting by
        // uptime/success in using a node in the past.
-       if *payee == *our_node_id {
+       if payee_node_id == our_node_id {
                return Err(LightningError{err: "Cannot generate a route to ourselves".to_owned(), action: ErrorAction::IgnoreError});
        }
 
@@ -482,25 +485,28 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
        // work reliably.
        let allow_mpp = if let Some(features) = &payee_features {
                features.supports_basic_mpp()
-       } else if let Some(node) = network_nodes.get(&payee) {
+       } else if let Some(node) = network_nodes.get(&payee_node_id) {
                if let Some(node_info) = node.announcement_info.as_ref() {
                        node_info.features.supports_basic_mpp()
                } else { false }
        } else { false };
+       log_trace!(logger, "Searching for a route from payer {} to payee {} {} MPP", our_node_pubkey, payee,
+               if allow_mpp { "with" } else { "without" });
 
        // Step (1).
        // Prepare the data we'll use for payee-to-payer search by
        // inserting first hops suggested by the caller as targets.
        // Our search will then attempt to reach them while traversing from the payee node.
-       let mut first_hop_targets: HashMap<_, (_, ChannelFeatures, _, NodeFeatures)> =
+       let mut first_hop_targets: HashMap<_, Vec<(_, ChannelFeatures, _, NodeFeatures)>> =
                HashMap::with_capacity(if first_hops.is_some() { first_hops.as_ref().unwrap().len() } else { 0 });
        if let Some(hops) = first_hops {
                for chan in hops {
                        let short_channel_id = chan.short_channel_id.expect("first_hops should be filled in with usable channels, not pending ones");
-                       if chan.counterparty.node_id == *our_node_id {
-                               return Err(LightningError{err: "First hop cannot have our_node_id as a destination.".to_owned(), action: ErrorAction::IgnoreError});
+                       if chan.counterparty.node_id == *our_node_pubkey {
+                               return Err(LightningError{err: "First hop cannot have our_node_pubkey as a destination.".to_owned(), action: ErrorAction::IgnoreError});
                        }
-                       first_hop_targets.insert(chan.counterparty.node_id, (short_channel_id, chan.counterparty.features.to_context(), chan.outbound_capacity_msat, chan.counterparty.features.to_context()));
+                       first_hop_targets.entry(NodeId::from_pubkey(&chan.counterparty.node_id)).or_insert(Vec::new())
+                               .push((short_channel_id, chan.counterparty.features.to_context(), chan.outbound_capacity_msat, chan.counterparty.features.to_context()));
                }
                if first_hop_targets.is_empty() {
                        return Err(LightningError{err: "Cannot route when there are no outbound routes away from us".to_owned(), action: ErrorAction::IgnoreError});
@@ -544,7 +550,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
        // - when we want to stop looking for new paths.
        let mut already_collected_value_msat = 0;
 
-       log_trace!(logger, "Building path from {} (payee) to {} (us/payer) for value {} msat.", payee, our_node_id, final_value_msat);
+       log_trace!(logger, "Building path from {} (payee) to {} (us/payer) for value {} msat.", payee, our_node_pubkey, final_value_msat);
 
        macro_rules! add_entry {
                // Adds entry which goes from $src_node_id to $dest_node_id
@@ -644,7 +650,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                        Some(Some(value_msat)) => cmp::max(value_msat, $directional_info.htlc_minimum_msat),
                                                        _ => u64::max_value()
                                                };
-                                               let hm_entry = dist.entry(&$src_node_id);
+                                               let hm_entry = dist.entry($src_node_id);
                                                let old_entry = hm_entry.or_insert_with(|| {
                                                        // If there was previously no known way to access
                                                        // the source node (recall it goes payee-to-payer) of $chan_id, first add
@@ -658,7 +664,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                                fee_proportional_millionths = fees.proportional_millionths;
                                                        }
                                                        PathBuildingHop {
-                                                               pubkey: $dest_node_id.clone(),
+                                                               node_id: $dest_node_id.clone(),
                                                                short_channel_id: 0,
                                                                channel_features: $chan_features,
                                                                fee_msat: 0,
@@ -694,7 +700,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
 
                                                        // Ignore hop_use_fee_msat for channel-from-us as we assume all channels-from-us
                                                        // will have the same effective-fee
-                                                       if $src_node_id != *our_node_id {
+                                                       if $src_node_id != our_node_id {
                                                                match compute_fees(amount_to_transfer_over_msat, $directional_info.fees) {
                                                                        // max_value means we'll always fail
                                                                        // the old_entry.total_fee_msat > total_fee_msat check
@@ -725,7 +731,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                        }
 
                                                        let new_graph_node = RouteGraphNode {
-                                                               pubkey: $src_node_id,
+                                                               node_id: $src_node_id,
                                                                lowest_fee_to_peer_through_node: total_fee_msat,
                                                                lowest_fee_to_node: $next_hops_fee_msat as u64 + hop_use_fee_msat,
                                                                value_contribution_msat: value_contribution_msat,
@@ -756,7 +762,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                                old_entry.next_hops_fee_msat = $next_hops_fee_msat;
                                                                old_entry.hop_use_fee_msat = hop_use_fee_msat;
                                                                old_entry.total_fee_msat = total_fee_msat;
-                                                               old_entry.pubkey = $dest_node_id.clone();
+                                                               old_entry.node_id = $dest_node_id.clone();
                                                                old_entry.short_channel_id = $chan_id.clone();
                                                                old_entry.channel_features = $chan_features;
                                                                old_entry.fee_msat = 0; // This value will be later filled with hop_use_fee_msat of the following channel
@@ -811,7 +817,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
        // This data can later be helpful to optimize routing (pay lower fees).
        macro_rules! add_entries_to_cheapest_to_target_node {
                ( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr ) => {
-                       let skip_node = if let Some(elem) = dist.get_mut($node_id) {
+                       let skip_node = if let Some(elem) = dist.get_mut(&$node_id) {
                                let was_processed = elem.was_processed;
                                elem.was_processed = true;
                                was_processed
@@ -819,14 +825,14 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                // Entries are added to dist in add_entry!() when there is a channel from a node.
                                // Because there are no channels from payee, it will not have a dist entry at this point.
                                // If we're processing any other node, it is always be the result of a channel from it.
-                               assert_eq!($node_id, payee);
+                               assert_eq!($node_id, payee_node_id);
                                false
                        };
 
                        if !skip_node {
-                               if first_hops.is_some() {
-                                       if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&$node_id) {
-                                               add_entry!(first_hop, *our_node_id, $node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
+                               if let Some(first_channels) = first_hop_targets.get(&$node_id) {
+                                       for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
+                                               add_entry!(first_hop, our_node_id, $node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
                                        }
                                }
 
@@ -840,9 +846,9 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                        for chan_id in $node.channels.iter() {
                                                let chan = network_channels.get(chan_id).unwrap();
                                                if !chan.features.requires_unknown_bits() {
-                                                       if chan.node_one == *$node_id {
+                                                       if chan.node_one == $node_id {
                                                                // ie $node is one, ie next hop in A* is two, via the two_to_one channel
-                                                               if first_hops.is_none() || chan.node_two != *our_node_id {
+                                                               if first_hops.is_none() || chan.node_two != our_node_id {
                                                                        if let Some(two_to_one) = chan.two_to_one.as_ref() {
                                                                                if two_to_one.enabled {
                                                                                        add_entry!(chan_id, chan.node_two, chan.node_one, two_to_one, chan.capacity_sats, &chan.features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
@@ -850,7 +856,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                                        }
                                                                }
                                                        } else {
-                                                               if first_hops.is_none() || chan.node_one != *our_node_id {
+                                                               if first_hops.is_none() || chan.node_one != our_node_id{
                                                                        if let Some(one_to_two) = chan.one_to_two.as_ref() {
                                                                                if one_to_two.enabled {
                                                                                        add_entry!(chan_id, chan.node_one, chan.node_two, one_to_two, chan.capacity_sats, &chan.features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
@@ -878,26 +884,27 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
 
                // If first hop is a private channel and the only way to reach the payee, this is the only
                // place where it could be added.
-               if first_hops.is_some() {
-                       if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&payee) {
-                               add_entry!(first_hop, *our_node_id, payee, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, 0, path_value_msat, 0);
+               if let Some(first_channels) = first_hop_targets.get(&payee_node_id) {
+                       for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
+                               let added = add_entry!(first_hop, our_node_id, payee_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, 0, path_value_msat, 0);
+                               log_trace!(logger, "{} direct route to payee via SCID {}", if added { "Added" } else { "Skipped" }, first_hop);
                        }
                }
 
                // Add the payee as a target, so that the payee-to-payer
                // search algorithm knows what to start with.
-               match network_nodes.get(payee) {
+               match network_nodes.get(&payee_node_id) {
                        // The payee is not in our network graph, so nothing to add here.
                        // There is still a chance of reaching them via last_hops though,
                        // so don't yet fail the payment here.
                        // If not, targets.pop() will not even let us enter the loop in step 2.
                        None => {},
                        Some(node) => {
-                               add_entries_to_cheapest_to_target_node!(node, payee, 0, path_value_msat, 0);
+                               add_entries_to_cheapest_to_target_node!(node, payee_node_id, 0, path_value_msat, 0);
                        },
                }
 
-               // Step (1).
+               // Step (2).
                // If a caller provided us with last hops, add them to routing targets. Since this happens
                // earlier than general path finding, they will be somewhat prioritized, although currently
                // it matters only if the fees are exactly the same.
@@ -907,8 +914,8 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                // Only add the hops in this route to our candidate set if either
                                // we have a direct channel to the first hop or the first hop is
                                // in the regular network graph.
-                               first_hop_targets.get(&first_hop_in_route.src_node_id).is_some() ||
-                               network_nodes.get(&first_hop_in_route.src_node_id).is_some();
+                               first_hop_targets.get(&NodeId::from_pubkey(&first_hop_in_route.src_node_id)).is_some() ||
+                               network_nodes.get(&NodeId::from_pubkey(&first_hop_in_route.src_node_id)).is_some();
                        if have_hop_src_in_graph {
                                // We start building the path from reverse, i.e., from payee
                                // to the first RouteHintHop in the path.
@@ -936,12 +943,11 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                _ => aggregate_next_hops_fee_msat.checked_add(999).unwrap_or(u64::max_value())
                                        }) { Some( val / 1000 ) } else { break; }; // converting from msat or breaking if max ~ infinity
 
-
                                        // We assume that the recipient only included route hints for routes which had
                                        // sufficient value to route `final_value_msat`. Note that in the case of "0-value"
                                        // invoices where the invoice does not specify value this may not be the case, but
                                        // better to include the hints than not.
-                                       if !add_entry!(hop.short_channel_id, hop.src_node_id, prev_hop_id, directional_info, reqd_channel_cap, &empty_channel_features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat) {
+                                       if !add_entry!(hop.short_channel_id, NodeId::from_pubkey(&hop.src_node_id), NodeId::from_pubkey(&prev_hop_id), directional_info, reqd_channel_cap, &empty_channel_features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat) {
                                                // If this hop was not used then there is no use checking the preceding hops
                                                // in the RouteHint. We can break by just searching for a direct channel between
                                                // last checked hop and first_hop_targets
@@ -949,8 +955,10 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                        }
 
                                        // Searching for a direct channel between last checked hop and first_hop_targets
-                                       if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&prev_hop_id) {
-                                               add_entry!(first_hop, *our_node_id , prev_hop_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat);
+                                       if let Some(first_channels) = first_hop_targets.get(&NodeId::from_pubkey(&prev_hop_id)) {
+                                               for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
+                                                       add_entry!(first_hop, our_node_id , NodeId::from_pubkey(&prev_hop_id), dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat);
+                                               }
                                        }
 
                                        if !hop_used {
@@ -981,8 +989,10 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                // Note that we *must* check if the last hop was added as `add_entry`
                                                // always assumes that the third argument is a node to which we have a
                                                // path.
-                                               if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&hop.src_node_id) {
-                                                       add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat);
+                                               if let Some(first_channels) = first_hop_targets.get(&NodeId::from_pubkey(&hop.src_node_id)) {
+                                                       for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
+                                                               add_entry!(first_hop, our_node_id , NodeId::from_pubkey(&hop.src_node_id), dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat);
+                                                       }
                                                }
                                        }
                                }
@@ -995,7 +1005,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                // last hops communicated by the caller, and the payment receiver.
                let mut found_new_path = false;
 
-               // Step (2).
+               // Step (3).
                // If this loop terminates due the exhaustion of targets, two situations are possible:
                // - not enough outgoing liquidity:
                //   0 < already_collected_value_msat < final_value_msat
@@ -1004,40 +1014,50 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                // Both these cases (and other cases except reaching recommended_value_msat) mean that
                // paths_collection will be stopped because found_new_path==false.
                // This is not necessarily a routing failure.
-               'path_construction: while let Some(RouteGraphNode { pubkey, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat, .. }) = targets.pop() {
+               'path_construction: while let Some(RouteGraphNode { node_id, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat, .. }) = targets.pop() {
 
                        // Since we're going payee-to-payer, hitting our node as a target means we should stop
                        // traversing the graph and arrange the path out of what we found.
-                       if pubkey == *our_node_id {
+                       if node_id == our_node_id {
                                let mut new_entry = dist.remove(&our_node_id).unwrap();
                                let mut ordered_hops = vec!((new_entry.clone(), NodeFeatures::empty()));
 
                                'path_walk: loop {
-                                       if let Some(&(_, _, _, ref features)) = first_hop_targets.get(&ordered_hops.last().unwrap().0.pubkey) {
-                                               ordered_hops.last_mut().unwrap().1 = features.clone();
-                                       } else if let Some(node) = network_nodes.get(&ordered_hops.last().unwrap().0.pubkey) {
-                                               if let Some(node_info) = node.announcement_info.as_ref() {
-                                                       ordered_hops.last_mut().unwrap().1 = node_info.features.clone();
+                                       let mut features_set = false;
+                                       if let Some(first_channels) = first_hop_targets.get(&ordered_hops.last().unwrap().0.node_id) {
+                                               for (scid, _, _, ref features) in first_channels {
+                                                       if *scid == ordered_hops.last().unwrap().0.short_channel_id {
+                                                               ordered_hops.last_mut().unwrap().1 = features.clone();
+                                                               features_set = true;
+                                                               break;
+                                                       }
+                                               }
+                                       }
+                                       if !features_set {
+                                               if let Some(node) = network_nodes.get(&ordered_hops.last().unwrap().0.node_id) {
+                                                       if let Some(node_info) = node.announcement_info.as_ref() {
+                                                               ordered_hops.last_mut().unwrap().1 = node_info.features.clone();
+                                                       } else {
+                                                               ordered_hops.last_mut().unwrap().1 = NodeFeatures::empty();
+                                                       }
                                                } else {
-                                                       ordered_hops.last_mut().unwrap().1 = NodeFeatures::empty();
+                                                       // We should be able to fill in features for everything except the last
+                                                       // hop, if the last hop was provided via a BOLT 11 invoice (though we
+                                                       // should be able to extend it further as BOLT 11 does have feature
+                                                       // flags for the last hop node itself).
+                                                       assert!(ordered_hops.last().unwrap().0.node_id == payee_node_id);
                                                }
-                                       } else {
-                                               // We should be able to fill in features for everything except the last
-                                               // hop, if the last hop was provided via a BOLT 11 invoice (though we
-                                               // should be able to extend it further as BOLT 11 does have feature
-                                               // flags for the last hop node itself).
-                                               assert!(ordered_hops.last().unwrap().0.pubkey == *payee);
                                        }
 
                                        // Means we succesfully traversed from the payer to the payee, now
                                        // save this path for the payment route. Also, update the liquidity
                                        // remaining on the used hops, so that we take them into account
                                        // while looking for more paths.
-                                       if ordered_hops.last().unwrap().0.pubkey == *payee {
+                                       if ordered_hops.last().unwrap().0.node_id == payee_node_id {
                                                break 'path_walk;
                                        }
 
-                                       new_entry = match dist.remove(&ordered_hops.last().unwrap().0.pubkey) {
+                                       new_entry = match dist.remove(&ordered_hops.last().unwrap().0.node_id) {
                                                Some(payment_hop) => payment_hop,
                                                // We can't arrive at None because, if we ever add an entry to targets,
                                                // we also fill in the entry in dist (see add_entry!).
@@ -1112,15 +1132,15 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                        // If we found a path back to the payee, we shouldn't try to process it again. This is
                        // the equivalent of the `elem.was_processed` check in
                        // add_entries_to_cheapest_to_target_node!() (see comment there for more info).
-                       if pubkey == *payee { continue 'path_construction; }
+                       if node_id == payee_node_id { continue 'path_construction; }
 
                        // Otherwise, since the current target node is not us,
                        // keep "unrolling" the payment graph from payee to payer by
                        // finding a way to reach the current target from the payer side.
-                       match network_nodes.get(&pubkey) {
+                       match network_nodes.get(&node_id) {
                                None => {},
                                Some(node) => {
-                                       add_entries_to_cheapest_to_target_node!(node, &pubkey, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat);
+                                       add_entries_to_cheapest_to_target_node!(node, node_id, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat);
                                },
                        }
                }
@@ -1130,7 +1150,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                        break 'paths_collection;
                }
 
-               // Step (3).
+               // Step (4).
                // Stop either when the recommended value is reached or if no new path was found in this
                // iteration.
                // In the latter case, making another path finding attempt won't help,
@@ -1154,7 +1174,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                }
        }
 
-       // Step (4).
+       // Step (5).
        if payment_paths.len() == 0 {
                return Err(LightningError{err: "Failed to find a path to the given destination".to_owned(), action: ErrorAction::IgnoreError});
        }
@@ -1175,12 +1195,12 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                let mut cur_route = Vec::<PaymentPath>::new();
                let mut aggregate_route_value_msat = 0;
 
-               // Step (5).
+               // Step (6).
                // TODO: real random shuffle
                // Currently just starts with i_th and goes up to i-1_th in a looped way.
                let cur_payment_paths = [&payment_paths[i..], &payment_paths[..i]].concat();
 
-               // Step (6).
+               // Step (7).
                for payment_path in cur_payment_paths {
                        cur_route.push(payment_path.clone());
                        aggregate_route_value_msat += payment_path.get_value_msat();
@@ -1219,7 +1239,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
 
                                assert!(cur_route.len() > 0);
 
-                               // Step (7).
+                               // Step (8).
                                // Now, substract the overpaid value from the most-expensive path.
                                // TODO: this could also be optimized by also sorting by feerate_per_sat_routed,
                                // so that the sender pays less fees overall. And also htlc_minimum_msat.
@@ -1236,30 +1256,32 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                drawn_routes.push(cur_route);
        }
 
-       // Step (8).
+       // Step (9).
        // Select the best route by lowest total fee.
        drawn_routes.sort_by_key(|paths| paths.iter().map(|path| path.get_total_fee_paid_msat()).sum::<u64>());
-       let mut selected_paths = Vec::<Vec<RouteHop>>::new();
+       let mut selected_paths = Vec::<Vec<Result<RouteHop, LightningError>>>::new();
        for payment_path in drawn_routes.first().unwrap() {
                selected_paths.push(payment_path.hops.iter().map(|(payment_hop, node_features)| {
-                       RouteHop {
-                               pubkey: payment_hop.pubkey,
+                       Ok(RouteHop {
+                               pubkey: PublicKey::from_slice(payment_hop.node_id.as_slice()).map_err(|_| LightningError{err: format!("Public key {:?} is invalid", &payment_hop.node_id), action: ErrorAction::IgnoreAndLog(Level::Trace)})?,
                                node_features: node_features.clone(),
                                short_channel_id: payment_hop.short_channel_id,
                                channel_features: payment_hop.channel_features.clone(),
                                fee_msat: payment_hop.fee_msat,
                                cltv_expiry_delta: payment_hop.cltv_expiry_delta,
-                       }
+                       })
                }).collect());
        }
 
        if let Some(features) = &payee_features {
                for path in selected_paths.iter_mut() {
-                       path.last_mut().unwrap().node_features = features.to_context();
+                       if let Ok(route_hop) = path.last_mut().unwrap() {
+                               route_hop.node_features = features.to_context();
+                       }
                }
        }
 
-       let route = Route { paths: selected_paths };
+       let route = Route { paths: selected_paths.into_iter().map(|path| path.into_iter().collect()).collect::<Result<Vec<_>, _>>()? };
        log_info!(logger, "Got route to {}: {}", payee, log_route!(route));
        Ok(route)
 }
@@ -1765,7 +1787,7 @@ mod tests {
                let our_chans = vec![get_channel_details(Some(2), our_id, InitFeatures::from_le_bytes(vec![0b11]), 100000)];
 
                if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)) {
-                       assert_eq!(err, "First hop cannot have our_node_id as a destination.");
+                       assert_eq!(err, "First hop cannot have our_node_pubkey as a destination.");
                } else { panic!(); }
 
                let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
@@ -2184,7 +2206,7 @@ mod tests {
                        proportional_millionths: 0,
                };
                vec![RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[3].clone(),
+                       src_node_id: nodes[3],
                        short_channel_id: 8,
                        fees: zero_fees,
                        cltv_expiry_delta: (8 << 8) | 1,
@@ -2192,7 +2214,7 @@ mod tests {
                        htlc_maximum_msat: None,
                }
                ]), RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[4].clone(),
+                       src_node_id: nodes[4],
                        short_channel_id: 9,
                        fees: RoutingFees {
                                base_msat: 1001,
@@ -2202,7 +2224,7 @@ mod tests {
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
                }]), RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[5].clone(),
+                       src_node_id: nodes[5],
                        short_channel_id: 10,
                        fees: zero_fees,
                        cltv_expiry_delta: (10 << 8) | 1,
@@ -2217,7 +2239,7 @@ mod tests {
                        proportional_millionths: 0,
                };
                vec![RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[2].clone(),
+                       src_node_id: nodes[2],
                        short_channel_id: 5,
                        fees: RoutingFees {
                                base_msat: 100,
@@ -2227,7 +2249,7 @@ mod tests {
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
                }, RouteHintHop {
-                       src_node_id: nodes[3].clone(),
+                       src_node_id: nodes[3],
                        short_channel_id: 8,
                        fees: zero_fees,
                        cltv_expiry_delta: (8 << 8) | 1,
@@ -2235,7 +2257,7 @@ mod tests {
                        htlc_maximum_msat: None,
                }
                ]), RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[4].clone(),
+                       src_node_id: nodes[4],
                        short_channel_id: 9,
                        fees: RoutingFees {
                                base_msat: 1001,
@@ -2245,7 +2267,7 @@ mod tests {
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
                }]), RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[5].clone(),
+                       src_node_id: nodes[5],
                        short_channel_id: 10,
                        fees: zero_fees,
                        cltv_expiry_delta: (10 << 8) | 1,
@@ -2331,7 +2353,7 @@ mod tests {
                        proportional_millionths: 0,
                };
                vec![RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[3].clone(),
+                       src_node_id: nodes[3],
                        short_channel_id: 8,
                        fees: zero_fees,
                        cltv_expiry_delta: (8 << 8) | 1,
@@ -2340,7 +2362,7 @@ mod tests {
                }]), RouteHint(vec![
 
                ]), RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[5].clone(),
+                       src_node_id: nodes[5],
                        short_channel_id: 10,
                        fees: zero_fees,
                        cltv_expiry_delta: (10 << 8) | 1,
@@ -2403,7 +2425,7 @@ mod tests {
                        proportional_millionths: 0,
                };
                vec![RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[2].clone(),
+                       src_node_id: nodes[2],
                        short_channel_id: 5,
                        fees: RoutingFees {
                                base_msat: 100,
@@ -2413,14 +2435,14 @@ mod tests {
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
                }, RouteHintHop {
-                       src_node_id: nodes[3].clone(),
+                       src_node_id: nodes[3],
                        short_channel_id: 8,
                        fees: zero_fees,
                        cltv_expiry_delta: (8 << 8) | 1,
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
                }]), RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[5].clone(),
+                       src_node_id: nodes[5],
                        short_channel_id: 10,
                        fees: zero_fees,
                        cltv_expiry_delta: (10 << 8) | 1,
@@ -2500,21 +2522,21 @@ mod tests {
                        proportional_millionths: 0,
                };
                vec![RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[4].clone(),
+                       src_node_id: nodes[4],
                        short_channel_id: 11,
                        fees: zero_fees,
                        cltv_expiry_delta: (11 << 8) | 1,
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
                }, RouteHintHop {
-                       src_node_id: nodes[3].clone(),
+                       src_node_id: nodes[3],
                        short_channel_id: 8,
                        fees: zero_fees,
                        cltv_expiry_delta: (8 << 8) | 1,
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
                }]), RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[4].clone(),
+                       src_node_id: nodes[4],
                        short_channel_id: 9,
                        fees: RoutingFees {
                                base_msat: 1001,
@@ -2524,7 +2546,7 @@ mod tests {
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
                }]), RouteHint(vec![RouteHintHop {
-                       src_node_id: nodes[5].clone(),
+                       src_node_id: nodes[5],
                        short_channel_id: 10,
                        fees: zero_fees,
                        cltv_expiry_delta: (10 << 8) | 1,
@@ -4220,6 +4242,50 @@ mod tests {
                }
        }
 
+       #[test]
+       fn multiple_direct_first_hops() {
+               // Previously we'd only ever considered one first hop path per counterparty.
+               // However, as we don't restrict users to one channel per peer, we really need to support
+               // looking at all first hop paths.
+               // Here we test that we do not ignore all-but-the-last first hop paths per counterparty (as
+               // we used to do by overwriting the `first_hop_targets` hashmap entry) and that we can MPP
+               // route over multiple channels with the same first hop.
+               let secp_ctx = Secp256k1::new();
+               let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
+               let logger = Arc::new(test_utils::TestLogger::new());
+               let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash());
+
+               {
+                       let route = get_route(&our_id, &network_graph, &nodes[0], Some(InvoiceFeatures::known()), Some(&[
+                               &get_channel_details(Some(3), nodes[0], InitFeatures::known(), 200_000),
+                               &get_channel_details(Some(2), nodes[0], InitFeatures::known(), 10_000),
+                       ]), &[], 100_000, 42, Arc::clone(&logger)).unwrap();
+                       assert_eq!(route.paths.len(), 1);
+                       assert_eq!(route.paths[0].len(), 1);
+
+                       assert_eq!(route.paths[0][0].pubkey, nodes[0]);
+                       assert_eq!(route.paths[0][0].short_channel_id, 3);
+                       assert_eq!(route.paths[0][0].fee_msat, 100_000);
+               }
+               {
+                       let route = get_route(&our_id, &network_graph, &nodes[0], Some(InvoiceFeatures::known()), Some(&[
+                               &get_channel_details(Some(3), nodes[0], InitFeatures::known(), 50_000),
+                               &get_channel_details(Some(2), nodes[0], InitFeatures::known(), 50_000),
+                       ]), &[], 100_000, 42, Arc::clone(&logger)).unwrap();
+                       assert_eq!(route.paths.len(), 2);
+                       assert_eq!(route.paths[0].len(), 1);
+                       assert_eq!(route.paths[1].len(), 1);
+
+                       assert_eq!(route.paths[0][0].pubkey, nodes[0]);
+                       assert_eq!(route.paths[0][0].short_channel_id, 3);
+                       assert_eq!(route.paths[0][0].fee_msat, 50_000);
+
+                       assert_eq!(route.paths[1][0].pubkey, nodes[0]);
+                       assert_eq!(route.paths[1][0].short_channel_id, 2);
+                       assert_eq!(route.paths[1][0].fee_msat, 50_000);
+               }
+       }
+
        #[test]
        fn total_fees_single_path() {
                let route = Route {
@@ -4318,9 +4384,9 @@ mod tests {
                'load_endpoints: for _ in 0..10 {
                        loop {
                                seed = seed.overflowing_mul(0xdeadbeef).0;
-                               let src = nodes.keys().skip(seed % nodes.len()).next().unwrap();
+                               let src = &PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                seed = seed.overflowing_mul(0xdeadbeef).0;
-                               let dst = nodes.keys().skip(seed % nodes.len()).next().unwrap();
+                               let dst = &PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                let amt = seed as u64 % 200_000_000;
                                if get_route(src, &graph, dst, None, None, &[], amt, 42, &test_utils::TestLogger::new()).is_ok() {
                                        continue 'load_endpoints;
@@ -4347,9 +4413,9 @@ mod tests {
                'load_endpoints: for _ in 0..10 {
                        loop {
                                seed = seed.overflowing_mul(0xdeadbeef).0;
-                               let src = nodes.keys().skip(seed % nodes.len()).next().unwrap();
+                               let src = &PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                seed = seed.overflowing_mul(0xdeadbeef).0;
-                               let dst = nodes.keys().skip(seed % nodes.len()).next().unwrap();
+                               let dst = &PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                let amt = seed as u64 % 200_000_000;
                                if get_route(src, &graph, dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &test_utils::TestLogger::new()).is_ok() {
                                        continue 'load_endpoints;
@@ -4410,11 +4476,11 @@ mod benches {
                'load_endpoints: for _ in 0..100 {
                        loop {
                                seed *= 0xdeadbeef;
-                               let src = nodes.keys().skip(seed % nodes.len()).next().unwrap();
+                               let src = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                seed *= 0xdeadbeef;
-                               let dst = nodes.keys().skip(seed % nodes.len()).next().unwrap();
+                               let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                let amt = seed as u64 % 1_000_000;
-                               if get_route(src, &graph, dst, None, None, &[], amt, 42, &DummyLogger{}).is_ok() {
+                               if get_route(&src, &graph, &dst, None, None, &[], amt, 42, &DummyLogger{}).is_ok() {
                                        path_endpoints.push((src, dst, amt));
                                        continue 'load_endpoints;
                                }
@@ -4425,7 +4491,7 @@ mod benches {
                let mut idx = 0;
                bench.iter(|| {
                        let (src, dst, amt) = path_endpoints[idx % path_endpoints.len()];
-                       assert!(get_route(src, &graph, dst, None, None, &[], amt, 42, &DummyLogger{}).is_ok());
+                       assert!(get_route(&src, &graph, &dst, None, None, &[], amt, 42, &DummyLogger{}).is_ok());
                        idx += 1;
                });
        }
@@ -4442,11 +4508,11 @@ mod benches {
                'load_endpoints: for _ in 0..100 {
                        loop {
                                seed *= 0xdeadbeef;
-                               let src = nodes.keys().skip(seed % nodes.len()).next().unwrap();
+                               let src = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                seed *= 0xdeadbeef;
-                               let dst = nodes.keys().skip(seed % nodes.len()).next().unwrap();
+                               let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                let amt = seed as u64 % 1_000_000;
-                               if get_route(src, &graph, dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &DummyLogger{}).is_ok() {
+                               if get_route(&src, &graph, &dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &DummyLogger{}).is_ok() {
                                        path_endpoints.push((src, dst, amt));
                                        continue 'load_endpoints;
                                }
@@ -4457,7 +4523,7 @@ mod benches {
                let mut idx = 0;
                bench.iter(|| {
                        let (src, dst, amt) = path_endpoints[idx % path_endpoints.len()];
-                       assert!(get_route(src, &graph, dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &DummyLogger{}).is_ok());
+                       assert!(get_route(&src, &graph, &dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &DummyLogger{}).is_ok());
                        idx += 1;
                });
        }
index fcc625db047e679117aff18af238634401ea1cf1..9f7f4b4e5e00130a3581c5dd397497b3be0ba8ff 100644 (file)
@@ -23,13 +23,15 @@ use util::ser::{BigSize, FixedLengthReader, Writeable, Writer, MaybeReadable, Re
 use routing::router::RouteHop;
 
 use bitcoin::blockdata::script::Script;
-
+use bitcoin::hashes::Hash;
+use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::secp256k1::key::PublicKey;
 
 use io;
 use prelude::*;
 use core::time::Duration;
 use core::ops::Deref;
+use bitcoin::Transaction;
 
 /// Some information provided on receipt of payment depends on whether the payment received is a
 /// spontaneous payment or a "conventional" lightning payment that's paying an invoice.
@@ -148,21 +150,20 @@ pub enum Event {
                user_channel_id: u64,
        },
        /// Indicates we've received money! Just gotta dig out that payment preimage and feed it to
-       /// ChannelManager::claim_funds to get it....
-       /// Note that if the preimage is not known or the amount paid is incorrect, you should call
-       /// ChannelManager::fail_htlc_backwards to free up resources for this HTLC and avoid
+       /// [`ChannelManager::claim_funds`] to get it....
+       /// Note that if the preimage is not known, you should call
+       /// [`ChannelManager::fail_htlc_backwards`] to free up resources for this HTLC and avoid
        /// network congestion.
-       /// The amount paid should be considered 'incorrect' when it is less than or more than twice
-       /// the amount expected.
-       /// If you fail to call either ChannelManager::claim_funds or
-       /// ChannelManager::fail_htlc_backwards within the HTLC's timeout, the HTLC will be
+       /// If you fail to call either [`ChannelManager::claim_funds`] or
+       /// [`ChannelManager::fail_htlc_backwards`] within the HTLC's timeout, the HTLC will be
        /// automatically failed.
+       ///
+       /// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds
+       /// [`ChannelManager::fail_htlc_backwards`]: crate::ln::channelmanager::ChannelManager::fail_htlc_backwards
        PaymentReceived {
                /// The hash for which the preimage should be handed to the ChannelManager.
                payment_hash: PaymentHash,
-               /// The value, in thousandths of a satoshi, that this payment is for. Note that you must
-               /// compare this to the expected value before accepting the payment (as otherwise you are
-               /// providing proof-of-payment for less than the value you expected!).
+               /// The value, in thousandths of a satoshi, that this payment is for.
                amt: u64,
                /// Information for claiming this received payment, based on whether the purpose of the
                /// payment is to pay an invoice or to send a spontaneous payment.
@@ -178,6 +179,10 @@ pub enum Event {
                /// Note that this serves as a payment receipt, if you wish to have such a thing, you must
                /// store it somehow!
                payment_preimage: PaymentPreimage,
+               /// The hash which was given to [`ChannelManager::send_payment`].
+               ///
+               /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
+               payment_hash: PaymentHash,
        },
        /// Indicates an outbound payment we made failed. Probably some intermediary node dropped
        /// something. You may wish to retry with a different route.
@@ -259,6 +264,14 @@ pub enum Event {
                channel_id: [u8; 32],
                /// The reason the channel was closed.
                reason: ClosureReason
+       },
+       /// Used to indicate to the user that they can abandon the funding transaction and recycle the
+       /// inputs for another purpose.
+       DiscardFunding {
+               /// The channel_id of the channel which has been closed.
+               channel_id: [u8; 32],
+               /// The full transaction received from the user
+               transaction: Transaction
        }
 }
 
@@ -293,10 +306,11 @@ impl Writeable for Event {
                                        (8, payment_preimage, option),
                                });
                        },
-                       &Event::PaymentSent { ref payment_preimage } => {
+                       &Event::PaymentSent { ref payment_preimage, ref payment_hash} => {
                                2u8.write(writer)?;
                                write_tlv_fields!(writer, {
                                        (0, payment_preimage, required),
+                                       (1, payment_hash, required),
                                });
                        },
                        &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update,
@@ -322,9 +336,8 @@ impl Writeable for Event {
                        },
                        &Event::PendingHTLCsForwardable { time_forwardable: _ } => {
                                4u8.write(writer)?;
-                               write_tlv_fields!(writer, {});
-                               // We don't write the time_fordwardable out at all, as we presume when the user
-                               // deserializes us at least that much time has elapsed.
+                               // Note that we now ignore these on the read end as we'll re-generate them in
+                               // ChannelManager, we write them here only for backwards compatibility.
                        },
                        &Event::SpendableOutputs { ref outputs } => {
                                5u8.write(writer)?;
@@ -346,6 +359,13 @@ impl Writeable for Event {
                                        (2, reason, required)
                                });
                        },
+                       &Event::DiscardFunding { ref channel_id, ref transaction } => {
+                               11u8.write(writer)?;
+                               write_tlv_fields!(writer, {
+                                       (0, channel_id, required),
+                                       (2, transaction, required)
+                               })
+                       },
                        // Note that, going forward, all new events must only write data inside of
                        // `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write
                        // data via `write_tlv_fields`.
@@ -395,11 +415,17 @@ impl MaybeReadable for Event {
                        2u8 => {
                                let f = || {
                                        let mut payment_preimage = PaymentPreimage([0; 32]);
+                                       let mut payment_hash = None;
                                        read_tlv_fields!(reader, {
                                                (0, payment_preimage, required),
+                                               (1, payment_hash, option),
                                        });
+                                       if payment_hash.is_none() {
+                                               payment_hash = Some(PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner()));
+                                       }
                                        Ok(Some(Event::PaymentSent {
                                                payment_preimage,
+                                               payment_hash: payment_hash.unwrap(),
                                        }))
                                };
                                f()
@@ -439,15 +465,7 @@ impl MaybeReadable for Event {
                                };
                                f()
                        },
-                       4u8 => {
-                               let f = || {
-                                       read_tlv_fields!(reader, {});
-                                       Ok(Some(Event::PendingHTLCsForwardable {
-                                               time_forwardable: Duration::from_secs(0)
-                                       }))
-                               };
-                               f()
-                       },
+                       4u8 => Ok(None),
                        5u8 => {
                                let f = || {
                                        let mut outputs = VecReadWrapper(Vec::new());
@@ -471,14 +489,29 @@ impl MaybeReadable for Event {
                                f()
                        },
                        9u8 => {
-                               let mut channel_id = [0; 32];
-                               let mut reason = None;
-                               read_tlv_fields!(reader, {
-                                       (0, channel_id, required),
-                                       (2, reason, ignorable),
-                               });
-                               if reason.is_none() { return Ok(None); }
-                               Ok(Some(Event::ChannelClosed { channel_id, reason: reason.unwrap() }))
+                               let f = || {
+                                       let mut channel_id = [0; 32];
+                                       let mut reason = None;
+                                       read_tlv_fields!(reader, {
+                                               (0, channel_id, required),
+                                               (2, reason, ignorable),
+                                       });
+                                       if reason.is_none() { return Ok(None); }
+                                       Ok(Some(Event::ChannelClosed { channel_id, reason: reason.unwrap() }))
+                               };
+                               f()
+                       },
+                       11u8 => {
+                               let f = || {
+                                       let mut channel_id = [0; 32];
+                                       let mut transaction = Transaction{ version: 2, lock_time: 0, input: Vec::new(), output: Vec::new() };
+                                       read_tlv_fields!(reader, {
+                                               (0, channel_id, required),
+                                               (2, transaction, required),
+                                       });
+                                       Ok(Some(Event::DiscardFunding { channel_id, transaction } ))
+                               };
+                               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