Merge pull request #1553 from wvanlint/dns_hostname
[rust-lightning] / lightning / src / routing / gossip.rs
index 9030719934eb80cf5180694b0a72d06e38793e03..716ca2b305f3a48da3b5c698c571b11fed2fcdd0 100644 (file)
@@ -28,7 +28,7 @@ use ln::msgs::{DecodeError, ErrorAction, Init, LightningError, RoutingMessageHan
 use ln::msgs::{ChannelAnnouncement, ChannelUpdate, NodeAnnouncement, OptionalField, GossipTimestampFilter};
 use ln::msgs::{QueryChannelRange, ReplyChannelRange, QueryShortChannelIds, ReplyShortChannelIdsEnd};
 use ln::msgs;
-use util::ser::{Writeable, Readable, Writer};
+use util::ser::{Readable, ReadableArgs, Writeable, Writer};
 use util::logger::{Logger, Level};
 use util::events::{Event, EventHandler, MessageSendEvent, MessageSendEventsProvider};
 use util::scid_utils::{block_from_scid, scid_from_parts, MAX_SCID_BLOCK};
@@ -122,30 +122,16 @@ impl Readable for NodeId {
 }
 
 /// Represents the network as nodes and channels between them
-pub struct NetworkGraph {
+pub struct NetworkGraph<L: Deref> where L::Target: Logger {
        secp_ctx: Secp256k1<secp256k1::VerifyOnly>,
        last_rapid_gossip_sync_timestamp: Mutex<Option<u32>>,
        genesis_hash: BlockHash,
+       logger: L,
        // Lock order: channels -> nodes
        channels: RwLock<BTreeMap<u64, ChannelInfo>>,
        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();
-               let last_rapid_gossip_sync_timestamp = self.get_last_rapid_gossip_sync_timestamp();
-               Self {
-                       secp_ctx: Secp256k1::verification_only(),
-                       genesis_hash: self.genesis_hash.clone(),
-                       channels: RwLock::new(channels.clone()),
-                       nodes: RwLock::new(nodes.clone()),
-                       last_rapid_gossip_sync_timestamp: Mutex::new(last_rapid_gossip_sync_timestamp)
-               }
-       }
-}
-
 /// A read-only view of [`NetworkGraph`].
 pub struct ReadOnlyNetworkGraph<'a> {
        channels: RwLockReadGuard<'a, BTreeMap<u64, ChannelInfo>>,
@@ -198,17 +184,6 @@ impl_writeable_tlv_based_enum_upgradable!(NetworkUpdate,
        },
 );
 
-impl<G: Deref<Target=NetworkGraph>, C: Deref, L: Deref> EventHandler for P2PGossipSync<G, C, L>
-where C::Target: chain::Access, L::Target: Logger {
-       fn handle_event(&self, event: &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);
-                       }
-               }
-       }
-}
-
 /// Receives and validates network updates from peers,
 /// stores authentic and relevant data as a network graph.
 /// This network graph is then used for routing payments.
@@ -217,7 +192,7 @@ where C::Target: chain::Access, L::Target: Logger {
 ///
 /// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentPathFailed`] to the
 /// [`NetworkGraph`].
-pub struct P2PGossipSync<G: Deref<Target=NetworkGraph>, C: Deref, L: Deref>
+pub struct P2PGossipSync<G: Deref<Target=NetworkGraph<L>>, C: Deref, L: Deref>
 where C::Target: chain::Access, L::Target: Logger
 {
        network_graph: G,
@@ -227,7 +202,7 @@ where C::Target: chain::Access, L::Target: Logger
        logger: L,
 }
 
-impl<G: Deref<Target=NetworkGraph>, C: Deref, L: Deref> P2PGossipSync<G, C, L>
+impl<G: Deref<Target=NetworkGraph<L>>, C: Deref, L: Deref> P2PGossipSync<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,
@@ -271,27 +246,32 @@ where C::Target: chain::Access, L::Target: Logger
                        false
                }
        }
+}
 
-       /// Applies changes to the [`NetworkGraph`] from the given update.
-       fn handle_network_update(&self, update: &NetworkUpdate) {
-               match *update {
-                       NetworkUpdate::ChannelUpdateMessage { ref msg } => {
-                               let short_channel_id = msg.contents.short_channel_id;
-                               let is_enabled = msg.contents.flags & (1 << 1) != (1 << 1);
-                               let status = if is_enabled { "enabled" } else { "disabled" };
-                               log_debug!(self.logger, "Updating channel with channel_update from a payment failure. Channel {} is {}.", short_channel_id, status);
-                               let _ = self.network_graph.update_channel(msg);
-                       },
-                       NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
-                               let action = if is_permanent { "Removing" } else { "Disabling" };
-                               log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
-                               self.network_graph.channel_failed(short_channel_id, is_permanent);
-                       },
-                       NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
-                               let action = if is_permanent { "Removing" } else { "Disabling" };
-                               log_debug!(self.logger, "{} node graph entry for {} due to a payment failure.", action, node_id);
-                               self.network_graph.node_failed(node_id, is_permanent);
-                       },
+impl<L: Deref> EventHandler for NetworkGraph<L> where L::Target: Logger {
+       fn handle_event(&self, event: &Event) {
+               if let Event::PaymentPathFailed { network_update, .. } = event {
+                       if let Some(network_update) = network_update {
+                               match *network_update {
+                                       NetworkUpdate::ChannelUpdateMessage { ref msg } => {
+                                               let short_channel_id = msg.contents.short_channel_id;
+                                               let is_enabled = msg.contents.flags & (1 << 1) != (1 << 1);
+                                               let status = if is_enabled { "enabled" } else { "disabled" };
+                                               log_debug!(self.logger, "Updating channel with channel_update from a payment failure. Channel {} is {}.", short_channel_id, status);
+                                               let _ = self.update_channel(msg);
+                                       },
+                                       NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
+                                               let action = if is_permanent { "Removing" } else { "Disabling" };
+                                               log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
+                                               self.channel_failed(short_channel_id, is_permanent);
+                                       },
+                                       NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
+                                               let action = if is_permanent { "Removing" } else { "Disabling" };
+                                               log_debug!(self.logger, "{} node graph entry for {} due to a payment failure.", action, node_id);
+                                               self.node_failed(node_id, is_permanent);
+                                       },
+                               }
+                       }
                }
        }
 }
@@ -316,7 +296,7 @@ macro_rules! secp_verify_sig {
        };
 }
 
-impl<G: Deref<Target=NetworkGraph>, C: Deref, L: Deref> RoutingMessageHandler for P2PGossipSync<G, C, L>
+impl<G: Deref<Target=NetworkGraph<L>>, C: Deref, L: Deref> RoutingMessageHandler for P2PGossipSync<G, C, L>
 where C::Target: chain::Access, L::Target: Logger
 {
        fn handle_node_announcement(&self, msg: &msgs::NodeAnnouncement) -> Result<bool, LightningError> {
@@ -605,7 +585,7 @@ where C::Target: chain::Access, L::Target: Logger
        }
 }
 
-impl<G: Deref<Target=NetworkGraph>, C: Deref, L: Deref> MessageSendEventsProvider for P2PGossipSync<G, C, L>
+impl<G: Deref<Target=NetworkGraph<L>>, C: Deref, L: Deref> MessageSendEventsProvider for P2PGossipSync<G, C, L>
 where
        C::Target: chain::Access,
        L::Target: Logger,
@@ -765,13 +745,13 @@ impl<'a> DirectedChannelInfo<'a> {
                let (htlc_maximum_msat, effective_capacity) = match (htlc_maximum_msat, capacity_msat) {
                        (Some(amount_msat), Some(capacity_msat)) => {
                                let htlc_maximum_msat = cmp::min(amount_msat, capacity_msat);
-                               (htlc_maximum_msat, EffectiveCapacity::Total { capacity_msat })
+                               (htlc_maximum_msat, EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: Some(htlc_maximum_msat) })
                        },
                        (Some(amount_msat), None) => {
                                (amount_msat, EffectiveCapacity::MaximumHTLC { amount_msat })
                        },
                        (None, Some(capacity_msat)) => {
-                               (capacity_msat, EffectiveCapacity::Total { capacity_msat })
+                               (capacity_msat, EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: None })
                        },
                        (None, None) => (EffectiveCapacity::Unknown.as_msat(), EffectiveCapacity::Unknown),
                };
@@ -870,6 +850,8 @@ pub enum EffectiveCapacity {
        Total {
                /// The funding amount denominated in millisatoshi.
                capacity_msat: u64,
+               /// The maximum HTLC amount denominated in millisatoshi.
+               htlc_maximum_msat: Option<u64>
        },
        /// A capacity sufficient to route any payment, typically used for private channels provided by
        /// an invoice.
@@ -889,7 +871,7 @@ impl EffectiveCapacity {
                match self {
                        EffectiveCapacity::ExactLiquidity { liquidity_msat } => *liquidity_msat,
                        EffectiveCapacity::MaximumHTLC { amount_msat } => *amount_msat,
-                       EffectiveCapacity::Total { capacity_msat } => *capacity_msat,
+                       EffectiveCapacity::Total { capacity_msat, .. } => *capacity_msat,
                        EffectiveCapacity::Infinite => u64::max_value(),
                        EffectiveCapacity::Unknown => UNKNOWN_CHANNEL_CAPACITY_MSAT,
                }
@@ -924,7 +906,7 @@ pub struct NodeAnnouncementInfo {
        /// Moniker assigned to the node.
        /// May be invalid or malicious (eg control chars),
        /// should not be exposed to the user.
-       pub alias: [u8; 32],
+       pub alias: NodeAlias,
        /// Internet-level addresses via which one can connect to the node
        pub addresses: Vec<NetAddress>,
        /// An initial announcement of the node
@@ -943,6 +925,51 @@ impl_writeable_tlv_based!(NodeAnnouncementInfo, {
        (10, addresses, vec_type),
 });
 
+/// A user-defined name for a node, which may be used when displaying the node in a graph.
+///
+/// Since node aliases are provided by third parties, they are a potential avenue for injection
+/// attacks. Care must be taken when processing.
+#[derive(Clone, Debug, PartialEq)]
+pub struct NodeAlias(pub [u8; 32]);
+
+impl fmt::Display for NodeAlias {
+       fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+               let control_symbol = core::char::REPLACEMENT_CHARACTER;
+               let first_null = self.0.iter().position(|b| *b == 0).unwrap_or(self.0.len());
+               let bytes = self.0.split_at(first_null).0;
+               match core::str::from_utf8(bytes) {
+                       Ok(alias) => {
+                               for c in alias.chars() {
+                                       let mut bytes = [0u8; 4];
+                                       let c = if !c.is_control() { c } else { control_symbol };
+                                       f.write_str(c.encode_utf8(&mut bytes))?;
+                               }
+                       },
+                       Err(_) => {
+                               for c in bytes.iter().map(|b| *b as char) {
+                                       // Display printable ASCII characters
+                                       let mut bytes = [0u8; 4];
+                                       let c = if c >= '\x20' && c <= '\x7e' { c } else { control_symbol };
+                                       f.write_str(c.encode_utf8(&mut bytes))?;
+                               }
+                       },
+               };
+               Ok(())
+       }
+}
+
+impl Writeable for NodeAlias {
+       fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
+               self.0.write(w)
+       }
+}
+
+impl Readable for NodeAlias {
+       fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+               Ok(NodeAlias(Readable::read(r)?))
+       }
+}
+
 #[derive(Clone, Debug, PartialEq)]
 /// Details about a node in the network, known from the network announcement.
 pub struct NodeInfo {
@@ -975,7 +1002,7 @@ impl_writeable_tlv_based!(NodeInfo, {
 const SERIALIZATION_VERSION: u8 = 1;
 const MIN_SERIALIZATION_VERSION: u8 = 1;
 
-impl Writeable for NetworkGraph {
+impl<L: Deref> Writeable for NetworkGraph<L> where L::Target: Logger {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
                write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
 
@@ -1001,8 +1028,8 @@ impl Writeable for NetworkGraph {
        }
 }
 
-impl Readable for NetworkGraph {
-       fn read<R: io::Read>(reader: &mut R) -> Result<NetworkGraph, DecodeError> {
+impl<L: Deref> ReadableArgs<L> for NetworkGraph<L> where L::Target: Logger {
+       fn read<R: io::Read>(reader: &mut R, logger: L) -> Result<NetworkGraph<L>, DecodeError> {
                let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
                let genesis_hash: BlockHash = Readable::read(reader)?;
@@ -1029,6 +1056,7 @@ impl Readable for NetworkGraph {
                Ok(NetworkGraph {
                        secp_ctx: Secp256k1::verification_only(),
                        genesis_hash,
+                       logger,
                        channels: RwLock::new(channels),
                        nodes: RwLock::new(nodes),
                        last_rapid_gossip_sync_timestamp: Mutex::new(last_rapid_gossip_sync_timestamp),
@@ -1036,7 +1064,7 @@ impl Readable for NetworkGraph {
        }
 }
 
-impl fmt::Display for NetworkGraph {
+impl<L: Deref> fmt::Display for NetworkGraph<L> where L::Target: Logger {
        fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
                writeln!(f, "Network map\n[Channels]")?;
                for (key, val) in self.channels.read().unwrap().iter() {
@@ -1050,7 +1078,7 @@ impl fmt::Display for NetworkGraph {
        }
 }
 
-impl PartialEq for NetworkGraph {
+impl<L: Deref> PartialEq for NetworkGraph<L> where L::Target: Logger {
        fn eq(&self, other: &Self) -> bool {
                self.genesis_hash == other.genesis_hash &&
                        *self.channels.read().unwrap() == *other.channels.read().unwrap() &&
@@ -1058,12 +1086,13 @@ impl PartialEq for NetworkGraph {
        }
 }
 
-impl NetworkGraph {
+impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
        /// Creates a new, empty, network graph.
-       pub fn new(genesis_hash: BlockHash) -> NetworkGraph {
+       pub fn new(genesis_hash: BlockHash, logger: L) -> NetworkGraph<L> {
                Self {
                        secp_ctx: Secp256k1::verification_only(),
                        genesis_hash,
+                       logger,
                        channels: RwLock::new(BTreeMap::new()),
                        nodes: RwLock::new(BTreeMap::new()),
                        last_rapid_gossip_sync_timestamp: Mutex::new(None),
@@ -1144,7 +1173,7 @@ impl NetworkGraph {
                                        features: msg.features.clone(),
                                        last_update: msg.timestamp,
                                        rgb: msg.rgb,
-                                       alias: msg.alias,
+                                       alias: NodeAlias(msg.alias),
                                        addresses: msg.addresses.clone(),
                                        announcement_message: if should_relay { full_msg.cloned() } else { None },
                                });
@@ -1645,13 +1674,12 @@ mod tests {
        use chain;
        use ln::PaymentHash;
        use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
-       use routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate, MAX_EXCESS_BYTES_FOR_RELAY};
+       use routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate, NodeAlias, MAX_EXCESS_BYTES_FOR_RELAY};
        use ln::msgs::{Init, OptionalField, RoutingMessageHandler, UnsignedNodeAnnouncement, NodeAnnouncement,
                UnsignedChannelAnnouncement, ChannelAnnouncement, UnsignedChannelUpdate, ChannelUpdate,
                ReplyChannelRange, QueryChannelRange, QueryShortChannelIds, MAX_VALUE_MSAT};
        use util::test_utils;
-       use util::logger::Logger;
-       use util::ser::{Readable, Writeable};
+       use util::ser::{ReadableArgs, Writeable};
        use util::events::{Event, EventHandler, MessageSendEvent, MessageSendEventsProvider};
        use util::scid_utils::scid_from_parts;
 
@@ -1675,13 +1703,15 @@ mod tests {
        use prelude::*;
        use sync::Arc;
 
-       fn create_network_graph() -> NetworkGraph {
+       fn create_network_graph() -> NetworkGraph<Arc<test_utils::TestLogger>> {
                let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
-               NetworkGraph::new(genesis_hash)
+               let logger = Arc::new(test_utils::TestLogger::new());
+               NetworkGraph::new(genesis_hash, logger)
        }
 
-       fn create_gossip_sync(network_graph: &NetworkGraph) -> (
-               Secp256k1<All>, P2PGossipSync<&NetworkGraph, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>
+       fn create_gossip_sync(network_graph: &NetworkGraph<Arc<test_utils::TestLogger>>) -> (
+               Secp256k1<All>, P2PGossipSync<&NetworkGraph<Arc<test_utils::TestLogger>>,
+               Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>
        ) {
                let secp_ctx = Secp256k1::new();
                let logger = Arc::new(test_utils::TestLogger::new());
@@ -1845,7 +1875,7 @@ mod tests {
        #[test]
        fn handling_channel_announcements() {
                let secp_ctx = Secp256k1::new();
-               let logger: Arc<Logger> = Arc::new(test_utils::TestLogger::new());
+               let logger = test_utils::TestLogger::new();
 
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
                let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
@@ -1854,8 +1884,9 @@ mod tests {
                let valid_announcement = get_signed_channel_announcement(|_| {}, node_1_privkey, node_2_privkey, &secp_ctx);
 
                // Test if the UTXO lookups were not supported
-               let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash());
-               let mut gossip_sync = P2PGossipSync::new(&network_graph, None, Arc::clone(&logger));
+               let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
+               let network_graph = NetworkGraph::new(genesis_hash, &logger);
+               let mut gossip_sync = P2PGossipSync::new(&network_graph, None, &logger);
                match gossip_sync.handle_channel_announcement(&valid_announcement) {
                        Ok(res) => assert!(res),
                        _ => panic!()
@@ -1876,10 +1907,10 @@ mod tests {
                };
 
                // Test if an associated transaction were not on-chain (or not confirmed).
-               let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
+               let chain_source = 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());
-               gossip_sync = P2PGossipSync::new(&network_graph, Some(chain_source.clone()), Arc::clone(&logger));
+               let network_graph = NetworkGraph::new(genesis_hash, &logger);
+               gossip_sync = P2PGossipSync::new(&network_graph, Some(&chain_source), &logger);
 
                let valid_announcement = get_signed_channel_announcement(|unsigned_announcement| {
                        unsigned_announcement.short_channel_id += 1;
@@ -1960,10 +1991,11 @@ mod tests {
        #[test]
        fn handling_channel_update() {
                let secp_ctx = Secp256k1::new();
-               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 gossip_sync = P2PGossipSync::new(&network_graph, Some(chain_source.clone()), Arc::clone(&logger));
+               let logger = test_utils::TestLogger::new();
+               let chain_source = test_utils::TestChainSource::new(Network::Testnet);
+               let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
+               let network_graph = NetworkGraph::new(genesis_hash, &logger);
+               let gossip_sync = P2PGossipSync::new(&network_graph, Some(&chain_source), &logger);
 
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
                let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
@@ -2063,10 +2095,8 @@ mod tests {
        #[test]
        fn handling_network_update() {
                let logger = test_utils::TestLogger::new();
-               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 gossip_sync = P2PGossipSync::new(&network_graph, Some(chain_source.clone()), &logger);
+               let network_graph = NetworkGraph::new(genesis_hash, &logger);
                let secp_ctx = Secp256k1::new();
 
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
@@ -2089,7 +2119,7 @@ mod tests {
                        let valid_channel_update = get_signed_channel_update(|_| {}, node_1_privkey, &secp_ctx);
                        assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none());
 
-                       gossip_sync.handle_event(&Event::PaymentPathFailed {
+                       network_graph.handle_event(&Event::PaymentPathFailed {
                                payment_id: None,
                                payment_hash: PaymentHash([0; 32]),
                                rejected_by_dest: false,
@@ -2116,7 +2146,7 @@ mod tests {
                                }
                        };
 
-                       gossip_sync.handle_event(&Event::PaymentPathFailed {
+                       network_graph.handle_event(&Event::PaymentPathFailed {
                                payment_id: None,
                                payment_hash: PaymentHash([0; 32]),
                                rejected_by_dest: false,
@@ -2141,7 +2171,7 @@ mod tests {
                }
 
                // Permanent closing deletes a channel
-               gossip_sync.handle_event(&Event::PaymentPathFailed {
+               network_graph.handle_event(&Event::PaymentPathFailed {
                        payment_id: None,
                        payment_hash: PaymentHash([0; 32]),
                        rejected_by_dest: false,
@@ -2167,10 +2197,10 @@ mod tests {
        fn test_channel_timeouts() {
                // Test the removal of channels with `remove_stale_channels`.
                let logger = test_utils::TestLogger::new();
-               let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
+               let chain_source = test_utils::TestChainSource::new(Network::Testnet);
                let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
-               let network_graph = NetworkGraph::new(genesis_hash);
-               let gossip_sync = P2PGossipSync::new(&network_graph, Some(chain_source.clone()), &logger);
+               let network_graph = NetworkGraph::new(genesis_hash, &logger);
+               let gossip_sync = P2PGossipSync::new(&network_graph, Some(&chain_source), &logger);
                let secp_ctx = Secp256k1::new();
 
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
@@ -2382,7 +2412,9 @@ mod tests {
                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);
+
+               let logger = Arc::new(test_utils::TestLogger::new());
+               assert!(<NetworkGraph<_>>::read(&mut io::Cursor::new(&w.0), logger).unwrap() == network_graph);
        }
 
        #[test]
@@ -2392,7 +2424,9 @@ mod tests {
 
                let mut w = test_utils::TestVecWriter(Vec::new());
                network_graph.write(&mut w).unwrap();
-               let reassembled_network_graph: NetworkGraph = Readable::read(&mut io::Cursor::new(&w.0)).unwrap();
+
+               let logger = Arc::new(test_utils::TestLogger::new());
+               let reassembled_network_graph: NetworkGraph<_> = ReadableArgs::read(&mut io::Cursor::new(&w.0), logger).unwrap();
                assert!(reassembled_network_graph == network_graph);
                assert_eq!(reassembled_network_graph.get_last_rapid_gossip_sync_timestamp().unwrap(), 42);
        }
@@ -2681,7 +2715,7 @@ mod tests {
        }
 
        fn do_handling_query_channel_range(
-               gossip_sync: &P2PGossipSync<&NetworkGraph, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
+               gossip_sync: &P2PGossipSync<&NetworkGraph<Arc<test_utils::TestLogger>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
                test_node_id: &PublicKey,
                msg: QueryChannelRange,
                expected_ok: bool,
@@ -2743,6 +2777,29 @@ mod tests {
                });
                assert!(result.is_err());
        }
+
+       #[test]
+       fn displays_node_alias() {
+               let format_str_alias = |alias: &str| {
+                       let mut bytes = [0u8; 32];
+                       bytes[..alias.as_bytes().len()].copy_from_slice(alias.as_bytes());
+                       format!("{}", NodeAlias(bytes))
+               };
+
+               assert_eq!(format_str_alias("I\u{1F496}LDK! \u{26A1}"), "I\u{1F496}LDK! \u{26A1}");
+               assert_eq!(format_str_alias("I\u{1F496}LDK!\0\u{26A1}"), "I\u{1F496}LDK!");
+               assert_eq!(format_str_alias("I\u{1F496}LDK!\t\u{26A1}"), "I\u{1F496}LDK!\u{FFFD}\u{26A1}");
+
+               let format_bytes_alias = |alias: &[u8]| {
+                       let mut bytes = [0u8; 32];
+                       bytes[..alias.len()].copy_from_slice(alias);
+                       format!("{}", NodeAlias(bytes))
+               };
+
+               assert_eq!(format_bytes_alias(b"\xFFI <heart> LDK!"), "\u{FFFD}I <heart> LDK!");
+               assert_eq!(format_bytes_alias(b"\xFFI <heart>\0LDK!"), "\u{FFFD}I <heart>");
+               assert_eq!(format_bytes_alias(b"\xFFI <heart>\tLDK!"), "\u{FFFD}I <heart>\u{FFFD}LDK!");
+       }
 }
 
 #[cfg(all(test, feature = "_bench_unstable"))]
@@ -2754,18 +2811,20 @@ mod benches {
 
        #[bench]
        fn read_network_graph(bench: &mut Bencher) {
+               let logger = ::util::test_utils::TestLogger::new();
                let mut d = ::routing::router::test_utils::get_route_file().unwrap();
                let mut v = Vec::new();
                d.read_to_end(&mut v).unwrap();
                bench.iter(|| {
-                       let _ = NetworkGraph::read(&mut std::io::Cursor::new(&v)).unwrap();
+                       let _ = NetworkGraph::read(&mut std::io::Cursor::new(&v), &logger).unwrap();
                });
        }
 
        #[bench]
        fn write_network_graph(bench: &mut Bencher) {
+               let logger = ::util::test_utils::TestLogger::new();
                let mut d = ::routing::router::test_utils::get_route_file().unwrap();
-               let net_graph = NetworkGraph::read(&mut d).unwrap();
+               let net_graph = NetworkGraph::read(&mut d, &logger).unwrap();
                bench.iter(|| {
                        let _ = net_graph.encode();
                });