Add a comment describing the timestamp field's use
[rust-lightning] / lightning / src / routing / network_graph.rs
index 16ff80189e96b7dd249d8d96b5112b305e2f272b..10c0ba57b58b05e2a6875795773e4a19188c6cb1 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,18 +51,93 @@ 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 {
+       fn clone(&self) -> Self {
+               let channels = self.channels.read().unwrap();
+               let nodes = self.nodes.read().unwrap();
+               Self {
+                       genesis_hash: self.genesis_hash.clone(),
+                       channels: RwLock::new(channels.clone()),
+                       nodes: RwLock::new(nodes.clone()),
+               }
+       }
 }
 
 /// 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
@@ -110,10 +186,10 @@ impl_writeable_tlv_based_enum_upgradable!(NetworkUpdate,
        },
 );
 
-impl<C: Deref, L: Deref> EventHandler for NetGraphMsgHandler<C, L>
+impl<G: Deref<Target=NetworkGraph>, C: Deref, L: Deref> EventHandler for NetGraphMsgHandler<G, C, L>
 where C::Target: chain::Access, L::Target: Logger {
        fn handle_event(&self, event: &Event) {
-               if let Event::PaymentFailed { payment_hash: _, rejected_by_dest: _, network_update, .. } = event {
+               if let Event::PaymentPathFailed { payment_hash: _, rejected_by_dest: _, network_update, .. } = event {
                        if let Some(network_update) = network_update {
                                self.handle_network_update(network_update);
                        }
@@ -127,21 +203,20 @@ where C::Target: chain::Access, L::Target: Logger {
 /// Provides interface to help with initial routing sync by
 /// serving historical announcements.
 ///
-/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentFailed`] to the
+/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentPathFailed`] to the
 /// [`NetworkGraph`].
-pub struct NetGraphMsgHandler<C: Deref, L: Deref>
+pub struct NetGraphMsgHandler<G: Deref<Target=NetworkGraph>, C: Deref, L: Deref>
 where C::Target: chain::Access, L::Target: Logger
 {
        secp_ctx: Secp256k1<secp256k1::VerifyOnly>,
-       /// Representation of the payment channel network
-       pub network_graph: NetworkGraph,
+       network_graph: G,
        chain_access: Option<C>,
        full_syncs_requested: AtomicUsize,
        pending_events: Mutex<Vec<MessageSendEvent>>,
        logger: L,
 }
 
-impl<C: Deref, L: Deref> NetGraphMsgHandler<C, L>
+impl<G: Deref<Target=NetworkGraph>, C: Deref, L: Deref> NetGraphMsgHandler<G, C, L>
 where C::Target: chain::Access, L::Target: Logger
 {
        /// Creates a new tracker of the actual state of the network of channels and nodes,
@@ -149,7 +224,7 @@ where C::Target: chain::Access, L::Target: Logger
        /// Chain monitor is used to make sure announced channels exist on-chain,
        /// channel data is correct, and that the announcement is signed with
        /// channel owners' keys.
-       pub fn new(network_graph: NetworkGraph, chain_access: Option<C>, logger: L) -> Self {
+       pub fn new(network_graph: G, chain_access: Option<C>, logger: L) -> Self {
                NetGraphMsgHandler {
                        secp_ctx: Secp256k1::verification_only(),
                        network_graph,
@@ -212,7 +287,7 @@ macro_rules! secp_verify_sig {
        };
 }
 
-impl<C: Deref, L: Deref> RoutingMessageHandler for NetGraphMsgHandler<C, L>
+impl<G: Deref<Target=NetworkGraph>, C: Deref, L: Deref> RoutingMessageHandler for NetGraphMsgHandler<G, C, L>
 where C::Target: chain::Access, L::Target: Logger
 {
        fn handle_node_announcement(&self, msg: &msgs::NodeAnnouncement) -> Result<bool, LightningError> {
@@ -224,7 +299,7 @@ where C::Target: chain::Access, L::Target: Logger
 
        fn handle_channel_announcement(&self, msg: &msgs::ChannelAnnouncement) -> Result<bool, LightningError> {
                self.network_graph.update_channel_from_announcement(msg, &self.chain_access, &self.secp_ctx)?;
-               log_trace!(self.logger, "Added channel_announcement for {}{}", msg.contents.short_channel_id, if !msg.contents.excess_data.is_empty() { " with excess uninterpreted data!" } else { "" });
+               log_gossip!(self.logger, "Added channel_announcement for {}{}", msg.contents.short_channel_id, if !msg.contents.excess_data.is_empty() { " with excess uninterpreted data!" } else { "" });
                Ok(msg.contents.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY)
        }
 
@@ -265,11 +340,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() {
@@ -302,7 +377,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 ();
                }
 
@@ -478,7 +553,7 @@ where C::Target: chain::Access, L::Target: Logger
        }
 }
 
-impl<C: Deref, L: Deref> MessageSendEventsProvider for NetGraphMsgHandler<C, L>
+impl<G: Deref<Target=NetworkGraph>, C: Deref, L: Deref> MessageSendEventsProvider for NetGraphMsgHandler<G, C, L>
 where
        C::Target: chain::Access,
        L::Target: Logger,
@@ -539,11 +614,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.
@@ -558,7 +633,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(())
        }
 }
@@ -712,8 +787,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(())
        }
@@ -768,12 +843,17 @@ 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() {
-                                       if node_info.last_update  >= msg.timestamp {
-                                               return Err(LightningError{err: "Update older than last processed update".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Trace)});
+                                       // The timestamp field is somewhat of a misnomer - the BOLTs use it to order
+                                       // updates to ensure you always have the latest one, only vaguely suggesting
+                                       // that it be at least the current time.
+                                       if node_info.last_update  > msg.timestamp {
+                                               return Err(LightningError{err: "Update older than last processed update".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Gossip)});
+                                       } else if node_info.last_update  == msg.timestamp {
+                                               return Err(LightningError{err: "Update had the same timestamp as last processed update".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
                                        }
                                }
 
@@ -874,9 +954,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
@@ -902,7 +982,7 @@ impl NetworkGraph {
                                        Self::remove_channel_in_nodes(&mut nodes, &entry.get(), msg.short_channel_id);
                                        *entry.get_mut() = chan_info;
                                } else {
-                                       return Err(LightningError{err: "Already have knowledge of channel".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Trace)})
+                                       return Err(LightningError{err: "Already have knowledge of channel".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
                                }
                        },
                        BtreeEntry::Vacant(entry) => {
@@ -927,8 +1007,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(())
        }
@@ -1007,8 +1087,16 @@ impl NetworkGraph {
                                macro_rules! maybe_update_channel_info {
                                        ( $target: expr, $src_node: expr) => {
                                                if let Some(existing_chan_info) = $target.as_ref() {
-                                                       if existing_chan_info.last_update >= msg.timestamp {
-                                                               return Err(LightningError{err: "Update older than last processed update".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Trace)});
+                                                       // The timestamp field is somewhat of a misnomer - the BOLTs use it to
+                                                       // order updates to ensure you always have the latest one, only
+                                                       // suggesting  that it be at least the current time. For
+                                                       // channel_updates specifically, the BOLTs discuss the possibility of
+                                                       // pruning based on the timestamp field being more than two weeks old,
+                                                       // but only in the non-normative section.
+                                                       if existing_chan_info.last_update > msg.timestamp {
+                                                               return Err(LightningError{err: "Update older than last processed update".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Gossip)});
+                                                       } else if existing_chan_info.last_update == msg.timestamp {
+                                                               return Err(LightningError{err: "Update had same timestamp as last processed update".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
                                                        }
                                                        chan_was_enabled = existing_chan_info.enabled;
                                                } else {
@@ -1038,13 +1126,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);
                                }
@@ -1092,7 +1186,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) {
@@ -1124,19 +1218,17 @@ 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
        }
 
        /// Get network addresses by node id.
        /// Returns None if the requested node is completely unknown,
        /// or if node announcement for the node was never received.
-       ///
-       /// (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) {
+       pub fn get_addresses(&self, pubkey: &PublicKey) -> Option<Vec<NetAddress>> {
+               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)
+                               return Some(node_info.addresses.clone())
                        }
                }
                None
@@ -1175,18 +1267,24 @@ mod tests {
        use prelude::*;
        use sync::Arc;
 
-       fn create_net_graph_msg_handler() -> (Secp256k1<All>, NetGraphMsgHandler<Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>) {
+       fn create_network_graph() -> NetworkGraph {
+               let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
+               NetworkGraph::new(genesis_hash)
+       }
+
+       fn create_net_graph_msg_handler(network_graph: &NetworkGraph) -> (
+               Secp256k1<All>, NetGraphMsgHandler<&NetworkGraph, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>
+       ) {
                let secp_ctx = Secp256k1::new();
                let logger = Arc::new(test_utils::TestLogger::new());
-               let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
-               let network_graph = NetworkGraph::new(genesis_hash);
                let net_graph_msg_handler = NetGraphMsgHandler::new(network_graph, None, Arc::clone(&logger));
                (secp_ctx, net_graph_msg_handler)
        }
 
        #[test]
        fn request_full_sync_finite_times() {
-               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+               let network_graph = create_network_graph();
+               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
                let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&hex::decode("0202020202020202020202020202020202020202020202020202020202020202").unwrap()[..]).unwrap());
 
                assert!(net_graph_msg_handler.should_request_full_sync(&node_id));
@@ -1199,7 +1297,8 @@ mod tests {
 
        #[test]
        fn handling_node_announcements() {
-               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+               let network_graph = create_network_graph();
+               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
 
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
                let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
@@ -1342,15 +1441,14 @@ mod tests {
 
                // Test if the UTXO lookups were not supported
                let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash());
-               let mut net_graph_msg_handler = NetGraphMsgHandler::new(network_graph, None, Arc::clone(&logger));
+               let mut net_graph_msg_handler = NetGraphMsgHandler::new(&network_graph, None, Arc::clone(&logger));
                match net_graph_msg_handler.handle_channel_announcement(&valid_announcement) {
                        Ok(res) => assert!(res),
                        _ => panic!()
                };
 
                {
-                       let network = &net_graph_msg_handler.network_graph;
-                       match network.read_only().channels().get(&unsigned_announcement.short_channel_id) {
+                       match network_graph.read_only().channels().get(&unsigned_announcement.short_channel_id) {
                                None => panic!(),
                                Some(_) => ()
                        };
@@ -1367,7 +1465,7 @@ mod tests {
                let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
                *chain_source.utxo_ret.lock().unwrap() = Err(chain::AccessError::UnknownTx);
                let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash());
-               net_graph_msg_handler = NetGraphMsgHandler::new(network_graph, Some(chain_source.clone()), Arc::clone(&logger));
+               net_graph_msg_handler = NetGraphMsgHandler::new(&network_graph, Some(chain_source.clone()), Arc::clone(&logger));
                unsigned_announcement.short_channel_id += 1;
 
                msghash = hash_to_message!(&Sha256dHash::hash(&unsigned_announcement.encode()[..])[..]);
@@ -1402,8 +1500,7 @@ mod tests {
                };
 
                {
-                       let network = &net_graph_msg_handler.network_graph;
-                       match network.read_only().channels().get(&unsigned_announcement.short_channel_id) {
+                       match network_graph.read_only().channels().get(&unsigned_announcement.short_channel_id) {
                                None => panic!(),
                                Some(_) => ()
                        };
@@ -1433,8 +1530,7 @@ mod tests {
                        _ => panic!()
                };
                {
-                       let network = &net_graph_msg_handler.network_graph;
-                       match network.read_only().channels().get(&unsigned_announcement.short_channel_id) {
+                       match network_graph.read_only().channels().get(&unsigned_announcement.short_channel_id) {
                                Some(channel_entry) => {
                                        assert_eq!(channel_entry.features, ChannelFeatures::empty());
                                },
@@ -1492,7 +1588,7 @@ mod tests {
                let logger: Arc<Logger> = Arc::new(test_utils::TestLogger::new());
                let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
                let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash());
-               let net_graph_msg_handler = NetGraphMsgHandler::new(network_graph, Some(chain_source.clone()), Arc::clone(&logger));
+               let net_graph_msg_handler = NetGraphMsgHandler::new(&network_graph, Some(chain_source.clone()), Arc::clone(&logger));
 
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
                let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
@@ -1564,8 +1660,7 @@ mod tests {
                };
 
                {
-                       let network = &net_graph_msg_handler.network_graph;
-                       match network.read_only().channels().get(&short_channel_id) {
+                       match network_graph.read_only().channels().get(&short_channel_id) {
                                None => panic!(),
                                Some(channel_info) => {
                                        assert_eq!(channel_info.one_to_two.as_ref().unwrap().cltv_expiry_delta, 144);
@@ -1638,7 +1733,7 @@ mod tests {
 
                match net_graph_msg_handler.handle_channel_update(&valid_channel_update) {
                        Ok(_) => panic!(),
-                       Err(e) => assert_eq!(e.err, "Update older than last processed update")
+                       Err(e) => assert_eq!(e.err, "Update had same timestamp as last processed update")
                };
                unsigned_channel_update.timestamp += 500;
 
@@ -1661,7 +1756,7 @@ mod tests {
                let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
                let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
                let network_graph = NetworkGraph::new(genesis_hash);
-               let net_graph_msg_handler = NetGraphMsgHandler::new(network_graph, Some(chain_source.clone()), &logger);
+               let net_graph_msg_handler = NetGraphMsgHandler::new(&network_graph, Some(chain_source.clone()), &logger);
                let secp_ctx = Secp256k1::new();
 
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
@@ -1673,7 +1768,6 @@ mod tests {
 
                let short_channel_id = 0;
                let chain_hash = genesis_block(Network::Testnet).header.block_hash();
-               let network_graph = &net_graph_msg_handler.network_graph;
 
                {
                        // There is no nodes in the table at the beginning.
@@ -1725,13 +1819,17 @@ mod tests {
 
                        assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none());
 
-                       net_graph_msg_handler.handle_event(&Event::PaymentFailed {
+                       net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
+                               payment_id: None,
                                payment_hash: PaymentHash([0; 32]),
                                rejected_by_dest: false,
                                all_paths_failed: true,
+                               path: vec![],
                                network_update: Some(NetworkUpdate::ChannelUpdateMessage {
                                        msg: valid_channel_update,
                                }),
+                               short_channel_id: None,
+                               retry: None,
                                error_code: None,
                                error_data: None,
                        });
@@ -1748,14 +1846,18 @@ mod tests {
                                }
                        };
 
-                       net_graph_msg_handler.handle_event(&Event::PaymentFailed {
+                       net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
+                               payment_id: None,
                                payment_hash: PaymentHash([0; 32]),
                                rejected_by_dest: false,
                                all_paths_failed: true,
+                               path: vec![],
                                network_update: Some(NetworkUpdate::ChannelClosed {
                                        short_channel_id,
                                        is_permanent: false,
                                }),
+                               short_channel_id: None,
+                               retry: None,
                                error_code: None,
                                error_data: None,
                        });
@@ -1770,14 +1872,18 @@ mod tests {
 
                // Permanent closing deletes a channel
                {
-                       net_graph_msg_handler.handle_event(&Event::PaymentFailed {
+                       net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
+                               payment_id: None,
                                payment_hash: PaymentHash([0; 32]),
                                rejected_by_dest: false,
                                all_paths_failed: true,
+                               path: vec![],
                                network_update: Some(NetworkUpdate::ChannelClosed {
                                        short_channel_id,
                                        is_permanent: true,
                                }),
+                               short_channel_id: None,
+                               retry: None,
                                error_code: None,
                                error_data: None,
                        });
@@ -1791,7 +1897,8 @@ mod tests {
 
        #[test]
        fn getting_next_channel_announcements() {
-               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+               let network_graph = create_network_graph();
+               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
                let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
                let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_1_privkey);
@@ -1925,7 +2032,8 @@ mod tests {
 
        #[test]
        fn getting_next_node_announcements() {
-               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+               let network_graph = create_network_graph();
+               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
                let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
                let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_1_privkey);
@@ -2042,7 +2150,8 @@ mod tests {
 
        #[test]
        fn network_graph_serialization() {
-               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+               let network_graph = create_network_graph();
+               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
 
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
                let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
@@ -2099,17 +2208,17 @@ mod tests {
                        Err(_) => panic!()
                };
 
-               let network = &net_graph_msg_handler.network_graph;
                let mut w = test_utils::TestVecWriter(Vec::new());
-               assert!(!network.read_only().nodes().is_empty());
-               assert!(!network.read_only().channels().is_empty());
-               network.write(&mut w).unwrap();
-               assert!(<NetworkGraph>::read(&mut io::Cursor::new(&w.0)).unwrap() == *network);
+               assert!(!network_graph.read_only().nodes().is_empty());
+               assert!(!network_graph.read_only().channels().is_empty());
+               network_graph.write(&mut w).unwrap();
+               assert!(<NetworkGraph>::read(&mut io::Cursor::new(&w.0)).unwrap() == network_graph);
        }
 
        #[test]
        fn calling_sync_routing_table() {
-               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+               let network_graph = create_network_graph();
+               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
                let node_privkey_1 = &SecretKey::from_slice(&[42; 32]).unwrap();
                let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_privkey_1);
 
@@ -2146,7 +2255,8 @@ mod tests {
                // The initial implementation allows syncing with the first 5 peers after
                // which should_request_full_sync will return false
                {
-                       let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+                       let network_graph = create_network_graph();
+                       let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
                        let init_msg = Init { features: InitFeatures::known() };
                        for n in 1..7 {
                                let node_privkey = &SecretKey::from_slice(&[n; 32]).unwrap();
@@ -2165,7 +2275,8 @@ mod tests {
 
        #[test]
        fn handling_reply_channel_range() {
-               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+               let network_graph = create_network_graph();
+               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
                let node_privkey_1 = &SecretKey::from_slice(&[42; 32]).unwrap();
                let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_privkey_1);
 
@@ -2213,7 +2324,8 @@ mod tests {
 
        #[test]
        fn handling_reply_short_channel_ids() {
-               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+               let network_graph = create_network_graph();
+               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
                let node_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
                let node_id = PublicKey::from_secret_key(&secp_ctx, node_privkey);
 
@@ -2242,7 +2354,8 @@ mod tests {
 
        #[test]
        fn handling_query_channel_range() {
-               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+               let network_graph = create_network_graph();
+               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
 
                let chain_hash = genesis_block(Network::Testnet).header.block_hash();
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
@@ -2504,7 +2617,7 @@ mod tests {
        }
 
        fn do_handling_query_channel_range(
-               net_graph_msg_handler: &NetGraphMsgHandler<Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
+               net_graph_msg_handler: &NetGraphMsgHandler<&NetworkGraph, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
                test_node_id: &PublicKey,
                msg: QueryChannelRange,
                expected_ok: bool,
@@ -2553,7 +2666,8 @@ mod tests {
 
        #[test]
        fn handling_query_short_channel_ids() {
-               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler();
+               let network_graph = create_network_graph();
+               let (secp_ctx, net_graph_msg_handler) = create_net_graph_msg_handler(&network_graph);
                let node_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
                let node_id = PublicKey::from_secret_key(&secp_ctx, node_privkey);