Merge pull request #1413 from ViktorTigerstrom/2022-04-default-to-bolt4-tlv-onions
[rust-lightning] / lightning / src / ln / msgs.rs
index a810731c691f40c58a12272b2784a4dd48722245..1478f8b6bfb8272795dcd06d8272db926876a959 100644 (file)
@@ -30,27 +30,27 @@ use bitcoin::secp256k1;
 use bitcoin::blockdata::script::Script;
 use bitcoin::hash_types::{Txid, BlockHash};
 
-use ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
-use ln::onion_utils;
-use onion_message;
+use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
+use crate::ln::onion_utils;
+use crate::onion_message;
 
-use prelude::*;
+use crate::prelude::*;
 use core::fmt;
 use core::fmt::Debug;
-use io::{self, Read};
-use io_extras::read_to_end;
+use crate::io::{self, Read};
+use crate::io_extras::read_to_end;
 
-use util::events::{MessageSendEventsProvider, OnionMessageProvider};
-use util::logger;
-use util::ser::{BigSize, LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};
+use crate::util::events::{MessageSendEventsProvider, OnionMessageProvider};
+use crate::util::logger;
+use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};
 
-use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
+use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
 
 /// 21 million * 10^8 * 1000
 pub(crate) const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;
 
 /// An error in decoding a message or struct.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub enum DecodeError {
        /// A version byte specified something we don't know how to handle.
        /// Includes unknown realm byte in an OnionHopData packet
@@ -66,14 +66,13 @@ pub enum DecodeError {
        /// A length descriptor in the packet didn't describe the later data correctly
        BadLengthDescriptor,
        /// Error from std::io
-       Io(/// (C-not exported) as ErrorKind doesn't have a reasonable mapping
-        io::ErrorKind),
+       Io(io::ErrorKind),
        /// The message included zlib-compressed values, which we don't support.
        UnsupportedCompression,
 }
 
 /// An init message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Init {
        /// The relevant features which the sender supports
        pub features: InitFeatures,
@@ -85,7 +84,7 @@ pub struct Init {
 }
 
 /// An error message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ErrorMessage {
        /// The channel ID involved in the error.
        ///
@@ -100,7 +99,7 @@ pub struct ErrorMessage {
 }
 
 /// A warning message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct WarningMessage {
        /// The channel ID involved in the warning.
        ///
@@ -114,7 +113,7 @@ pub struct WarningMessage {
 }
 
 /// A ping message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Ping {
        /// The desired response length
        pub ponglen: u16,
@@ -124,7 +123,7 @@ pub struct Ping {
 }
 
 /// A pong message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Pong {
        /// The pong packet size.
        /// This field is not sent on the wire. byteslen zeros are sent.
@@ -132,7 +131,7 @@ pub struct Pong {
 }
 
 /// An open_channel message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct OpenChannel {
        /// The genesis hash of the blockchain where the channel is to be opened
        pub chain_hash: BlockHash,
@@ -179,7 +178,7 @@ pub struct OpenChannel {
 }
 
 /// An accept_channel message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct AcceptChannel {
        /// A temporary channel ID, until the funding outpoint is announced
        pub temporary_channel_id: [u8; 32],
@@ -220,7 +219,7 @@ pub struct AcceptChannel {
 }
 
 /// A funding_created message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct FundingCreated {
        /// A temporary channel ID, until the funding is established
        pub temporary_channel_id: [u8; 32],
@@ -233,7 +232,7 @@ pub struct FundingCreated {
 }
 
 /// A funding_signed message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct FundingSigned {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -242,7 +241,7 @@ pub struct FundingSigned {
 }
 
 /// A channel_ready message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ChannelReady {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -254,7 +253,7 @@ pub struct ChannelReady {
 }
 
 /// A shutdown message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Shutdown {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -266,7 +265,7 @@ pub struct Shutdown {
 /// The minimum and maximum fees which the sender is willing to place on the closing transaction.
 /// This is provided in [`ClosingSigned`] by both sides to indicate the fee range they are willing
 /// to use.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ClosingSignedFeeRange {
        /// The minimum absolute fee, in satoshis, which the sender is willing to place on the closing
        /// transaction.
@@ -277,7 +276,7 @@ pub struct ClosingSignedFeeRange {
 }
 
 /// A closing_signed message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ClosingSigned {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -291,7 +290,7 @@ pub struct ClosingSigned {
 }
 
 /// An update_add_htlc message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct UpdateAddHTLC {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -307,7 +306,7 @@ pub struct UpdateAddHTLC {
 }
 
  /// An onion message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct OnionMessage {
        /// Used in decrypting the onion packet's payload.
        pub blinding_point: PublicKey,
@@ -315,7 +314,7 @@ pub struct OnionMessage {
 }
 
 /// An update_fulfill_htlc message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct UpdateFulfillHTLC {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -326,7 +325,7 @@ pub struct UpdateFulfillHTLC {
 }
 
 /// An update_fail_htlc message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct UpdateFailHTLC {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -336,7 +335,7 @@ pub struct UpdateFailHTLC {
 }
 
 /// An update_fail_malformed_htlc message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct UpdateFailMalformedHTLC {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -348,7 +347,7 @@ pub struct UpdateFailMalformedHTLC {
 }
 
 /// A commitment_signed message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct CommitmentSigned {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -359,7 +358,7 @@ pub struct CommitmentSigned {
 }
 
 /// A revoke_and_ack message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct RevokeAndACK {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -370,7 +369,7 @@ pub struct RevokeAndACK {
 }
 
 /// An update_fee message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct UpdateFee {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -378,7 +377,7 @@ pub struct UpdateFee {
        pub feerate_per_kw: u32,
 }
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 /// Proof that the sender knows the per-commitment secret of the previous commitment transaction.
 /// This is used to convince the recipient that the channel is at a certain commitment
 /// number even if they lost that data due to a local failure.  Of course, the peer may lie
@@ -392,7 +391,7 @@ pub struct DataLossProtect {
 }
 
 /// A channel_reestablish message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ChannelReestablish {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -405,7 +404,7 @@ pub struct ChannelReestablish {
 }
 
 /// An announcement_signatures message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct AnnouncementSignatures {
        /// The channel ID
        pub channel_id: [u8; 32],
@@ -418,7 +417,7 @@ pub struct AnnouncementSignatures {
 }
 
 /// An address which can be used to connect to a remote peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub enum NetAddress {
        /// An IPv4 address/port on which the peer is listening.
        IPv4 {
@@ -573,7 +572,7 @@ impl Readable for NetAddress {
 
 
 /// The unsigned part of a node_announcement
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct UnsignedNodeAnnouncement {
        /// The advertised features
        pub features: NodeFeatures,
@@ -592,7 +591,7 @@ pub struct UnsignedNodeAnnouncement {
        pub(crate) excess_address_data: Vec<u8>,
        pub(crate) excess_data: Vec<u8>,
 }
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 /// A node_announcement message to be sent or received from a peer
 pub struct NodeAnnouncement {
        /// The signature by the node key
@@ -602,7 +601,7 @@ pub struct NodeAnnouncement {
 }
 
 /// The unsigned part of a channel_announcement
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct UnsignedChannelAnnouncement {
        /// The advertised channel features
        pub features: ChannelFeatures,
@@ -621,7 +620,7 @@ pub struct UnsignedChannelAnnouncement {
        pub(crate) excess_data: Vec<u8>,
 }
 /// A channel_announcement message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ChannelAnnouncement {
        /// Authentication of the announcement by the first public node
        pub node_signature_1: Signature,
@@ -636,7 +635,7 @@ pub struct ChannelAnnouncement {
 }
 
 /// The unsigned part of a channel_update
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct UnsignedChannelUpdate {
        /// The genesis hash of the blockchain where the channel is to be opened
        pub chain_hash: BlockHash,
@@ -669,7 +668,7 @@ pub struct UnsignedChannelUpdate {
        pub excess_data: Vec<u8>,
 }
 /// A channel_update message to be sent or received from a peer
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ChannelUpdate {
        /// A signature of the channel update
        pub signature: Signature,
@@ -681,7 +680,7 @@ pub struct ChannelUpdate {
 /// UTXOs in a range of blocks. The recipient of a query makes a best
 /// effort to reply to the query using one or more reply_channel_range
 /// messages.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct QueryChannelRange {
        /// The genesis hash of the blockchain being queried
        pub chain_hash: BlockHash,
@@ -698,7 +697,7 @@ pub struct QueryChannelRange {
 /// not be a perfect view of the network. The short_channel_ids in the
 /// reply are encoded. We only support encoding_type=0 uncompressed
 /// serialization and do not support encoding_type=1 zlib serialization.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ReplyChannelRange {
        /// The genesis hash of the blockchain being queried
        pub chain_hash: BlockHash,
@@ -720,7 +719,7 @@ pub struct ReplyChannelRange {
 /// reply_short_channel_ids_end message. The short_channel_ids sent in
 /// this query are encoded. We only support encoding_type=0 uncompressed
 /// serialization and do not support encoding_type=1 zlib serialization.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct QueryShortChannelIds {
        /// The genesis hash of the blockchain being queried
        pub chain_hash: BlockHash,
@@ -732,7 +731,7 @@ pub struct QueryShortChannelIds {
 /// query_short_channel_ids message. The query recipient makes a best
 /// effort to respond based on their local network view which may not be
 /// a perfect view of the network.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ReplyShortChannelIdsEnd {
        /// The genesis hash of the blockchain that was queried
        pub chain_hash: BlockHash,
@@ -744,7 +743,7 @@ pub struct ReplyShortChannelIdsEnd {
 /// A gossip_timestamp_filter message is used by a node to request
 /// gossip relay for messages in the requested time range when the
 /// gossip_queries feature has been negotiated.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct GossipTimestampFilter {
        /// The genesis hash of the blockchain for channel and node information
        pub chain_hash: BlockHash,
@@ -805,7 +804,7 @@ pub struct LightningError {
 
 /// Struct used to return values from revoke_and_ack messages, containing a bunch of commitment
 /// transaction updates if they were pending.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct CommitmentUpdate {
        /// update_add_htlc messages which should be sent
        pub update_add_htlcs: Vec<UpdateAddHTLC>,
@@ -826,7 +825,7 @@ pub struct CommitmentUpdate {
 /// OptionalFeild simply gets Present if there are enough bytes to read into it), we have a
 /// separate enum type for them.
 /// (C-not exported) due to a free generic in T
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub enum OptionalField<T> {
        /// Optional field is included in message
        Present(T),
@@ -883,10 +882,17 @@ pub trait ChannelMessageHandler : MessageSendEventsProvider {
        /// is believed to be possible in the future (eg they're sending us messages we don't
        /// understand or indicate they require unknown feature bits), no_connection_possible is set
        /// and any outstanding channels should be failed.
+       ///
+       /// Note that in some rare cases this may be called without a corresponding
+       /// [`Self::peer_connected`].
        fn peer_disconnected(&self, their_node_id: &PublicKey, no_connection_possible: bool);
 
        /// Handle a peer reconnecting, possibly generating channel_reestablish message(s).
-       fn peer_connected(&self, their_node_id: &PublicKey, msg: &Init);
+       ///
+       /// May return an `Err(())` if the features the peer supports are not sufficient to communicate
+       /// with us. Implementors should be somewhat conservative about doing so, however, as other
+       /// message handlers may still wish to communicate with this peer.
+       fn peer_connected(&self, their_node_id: &PublicKey, msg: &Init) -> Result<(), ()>;
        /// Handle an incoming channel_reestablish message from the given peer.
        fn handle_channel_reestablish(&self, their_node_id: &PublicKey, msg: &ChannelReestablish);
 
@@ -900,8 +906,15 @@ pub trait ChannelMessageHandler : MessageSendEventsProvider {
        // Handler information:
        /// Gets the node feature flags which this handler itself supports. All available handlers are
        /// queried similarly and their feature flags are OR'd together to form the [`NodeFeatures`]
-       /// which are broadcasted in our node_announcement message.
+       /// which are broadcasted in our [`NodeAnnouncement`] message.
        fn provided_node_features(&self) -> NodeFeatures;
+
+       /// Gets the init feature flags which should be sent to the given peer. All available handlers
+       /// are queried similarly and their feature flags are OR'd together to form the [`InitFeatures`]
+       /// which are sent in our [`Init`] message.
+       ///
+       /// Note that this method is called before [`Self::peer_connected`].
+       fn provided_init_features(&self, their_node_id: &PublicKey) -> InitFeatures;
 }
 
 /// A trait to describe an object which can receive routing messages.
@@ -933,7 +946,11 @@ pub trait RoutingMessageHandler : MessageSendEventsProvider {
        /// Called when a connection is established with a peer. This can be used to
        /// perform routing table synchronization using a strategy defined by the
        /// implementor.
-       fn peer_connected(&self, their_node_id: &PublicKey, init: &Init);
+       ///
+       /// May return an `Err(())` if the features the peer supports are not sufficient to communicate
+       /// with us. Implementors should be somewhat conservative about doing so, however, as other
+       /// message handlers may still wish to communicate with this peer.
+       fn peer_connected(&self, their_node_id: &PublicKey, init: &Init) -> Result<(), ()>;
        /// Handles the reply of a query we initiated to learn about channels
        /// for a given range of blocks. We can expect to receive one or more
        /// replies to a single query.
@@ -949,6 +966,18 @@ pub trait RoutingMessageHandler : MessageSendEventsProvider {
        /// Handles when a peer asks us to send routing gossip messages for a
        /// list of short_channel_ids.
        fn handle_query_short_channel_ids(&self, their_node_id: &PublicKey, msg: QueryShortChannelIds) -> Result<(), LightningError>;
+
+       // Handler information:
+       /// Gets the node feature flags which this handler itself supports. All available handlers are
+       /// queried similarly and their feature flags are OR'd together to form the [`NodeFeatures`]
+       /// which are broadcasted in our [`NodeAnnouncement`] message.
+       fn provided_node_features(&self) -> NodeFeatures;
+       /// Gets the init feature flags which should be sent to the given peer. All available handlers
+       /// are queried similarly and their feature flags are OR'd together to form the [`InitFeatures`]
+       /// which are sent in our [`Init`] message.
+       ///
+       /// Note that this method is called before [`Self::peer_connected`].
+       fn provided_init_features(&self, their_node_id: &PublicKey) -> InitFeatures;
 }
 
 /// A trait to describe an object that can receive onion messages.
@@ -957,15 +986,35 @@ pub trait OnionMessageHandler : OnionMessageProvider {
        fn handle_onion_message(&self, peer_node_id: &PublicKey, msg: &OnionMessage);
        /// Called when a connection is established with a peer. Can be used to track which peers
        /// advertise onion message support and are online.
-       fn peer_connected(&self, their_node_id: &PublicKey, init: &Init);
+       ///
+       /// May return an `Err(())` if the features the peer supports are not sufficient to communicate
+       /// with us. Implementors should be somewhat conservative about doing so, however, as other
+       /// message handlers may still wish to communicate with this peer.
+       fn peer_connected(&self, their_node_id: &PublicKey, init: &Init) -> Result<(), ()>;
        /// Indicates a connection to the peer failed/an existing connection was lost. Allows handlers to
        /// drop and refuse to forward onion messages to this peer.
+       ///
+       /// Note that in some rare cases this may be called without a corresponding
+       /// [`Self::peer_connected`].
        fn peer_disconnected(&self, their_node_id: &PublicKey, no_connection_possible: bool);
+
+       // Handler information:
+       /// Gets the node feature flags which this handler itself supports. All available handlers are
+       /// queried similarly and their feature flags are OR'd together to form the [`NodeFeatures`]
+       /// which are broadcasted in our [`NodeAnnouncement`] message.
+       fn provided_node_features(&self) -> NodeFeatures;
+
+       /// Gets the init feature flags which should be sent to the given peer. All available handlers
+       /// are queried similarly and their feature flags are OR'd together to form the [`InitFeatures`]
+       /// which are sent in our [`Init`] message.
+       ///
+       /// Note that this method is called before [`Self::peer_connected`].
+       fn provided_init_features(&self, their_node_id: &PublicKey) -> InitFeatures;
 }
 
 mod fuzzy_internal_msgs {
-       use prelude::*;
-       use ln::{PaymentPreimage, PaymentSecret};
+       use crate::prelude::*;
+       use crate::ln::{PaymentPreimage, PaymentSecret};
 
        // These types aren't intended to be pub, but are exposed for direct fuzzing (as we deserialize
        // them from untrusted input):
@@ -978,9 +1027,6 @@ mod fuzzy_internal_msgs {
        }
 
        pub(crate) enum OnionHopDataFormat {
-               Legacy { // aka Realm-0
-                       short_channel_id: u64,
-               },
                NonFinalNode {
                        short_channel_id: u64,
                },
@@ -996,7 +1042,6 @@ mod fuzzy_internal_msgs {
                /// Message serialization may panic if this value is more than 21 million Bitcoin.
                pub(crate) amt_to_forward: u64,
                pub(crate) outgoing_cltv_value: u32,
-               // 12 bytes of 0-padding for Legacy format
        }
 
        pub struct DecodedOnionErrorPacket {
@@ -1033,6 +1078,7 @@ impl onion_utils::Packet for OnionPacket {
        }
 }
 
+impl Eq for OnionPacket { }
 impl PartialEq for OnionPacket {
        fn eq(&self, other: &OnionPacket) -> bool {
                for (i, j) in self.hop_data.iter().zip(other.hop_data.iter()) {
@@ -1050,7 +1096,7 @@ impl fmt::Debug for OnionPacket {
        }
 }
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub(crate) struct OnionErrorPacket {
        // This really should be a constant size slice, but the spec lets these things be up to 128KB?
        // (TODO) We limit it in decode to much lower...
@@ -1408,13 +1454,6 @@ impl Readable for FinalOnionHopData {
 impl Writeable for OnionHopData {
        fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
                match self.format {
-                       OnionHopDataFormat::Legacy { short_channel_id } => {
-                               0u8.write(w)?;
-                               short_channel_id.write(w)?;
-                               self.amt_to_forward.write(w)?;
-                               self.outgoing_cltv_value.write(w)?;
-                               w.write_all(&[0;12])?;
-                       },
                        OnionHopDataFormat::NonFinalNode { short_channel_id } => {
                                encode_varint_length_prefixed_tlv!(w, {
                                        (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
@@ -1437,58 +1476,44 @@ impl Writeable for OnionHopData {
 
 impl Readable for OnionHopData {
        fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
-               let b: BigSize = Readable::read(r)?;
-               const LEGACY_ONION_HOP_FLAG: u64 = 0;
-               let (format, amt, cltv_value) = if b.0 != LEGACY_ONION_HOP_FLAG {
-                       let mut rd = FixedLengthReader::new(r, b.0);
-                       let mut amt = HighZeroBytesDroppedBigSize(0u64);
-                       let mut cltv_value = HighZeroBytesDroppedBigSize(0u32);
-                       let mut short_id: Option<u64> = None;
-                       let mut payment_data: Option<FinalOnionHopData> = None;
-                       let mut keysend_preimage: Option<PaymentPreimage> = None;
-                       decode_tlv_stream!(&mut rd, {
-                               (2, amt, required),
-                               (4, cltv_value, required),
-                               (6, short_id, option),
-                               (8, payment_data, option),
-                               // See https://github.com/lightning/blips/blob/master/blip-0003.md
-                               (5482373484, keysend_preimage, option)
-                       });
-                       rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?;
-                       let format = if let Some(short_channel_id) = short_id {
-                               if payment_data.is_some() { return Err(DecodeError::InvalidValue); }
-                               OnionHopDataFormat::NonFinalNode {
-                                       short_channel_id,
-                               }
-                       } else {
-                               if let &Some(ref data) = &payment_data {
-                                       if data.total_msat > MAX_VALUE_MSAT {
-                                               return Err(DecodeError::InvalidValue);
-                                       }
-                               }
-                               OnionHopDataFormat::FinalNode {
-                                       payment_data,
-                                       keysend_preimage,
-                               }
-                       };
-                       (format, amt.0, cltv_value.0)
+               let mut amt = HighZeroBytesDroppedBigSize(0u64);
+               let mut cltv_value = HighZeroBytesDroppedBigSize(0u32);
+               let mut short_id: Option<u64> = None;
+               let mut payment_data: Option<FinalOnionHopData> = None;
+               let mut keysend_preimage: Option<PaymentPreimage> = None;
+               read_tlv_fields!(r, {
+                       (2, amt, required),
+                       (4, cltv_value, required),
+                       (6, short_id, option),
+                       (8, payment_data, option),
+                       // See https://github.com/lightning/blips/blob/master/blip-0003.md
+                       (5482373484, keysend_preimage, option)
+               });
+
+               let format = if let Some(short_channel_id) = short_id {
+                       if payment_data.is_some() { return Err(DecodeError::InvalidValue); }
+                       OnionHopDataFormat::NonFinalNode {
+                               short_channel_id,
+                       }
                } else {
-                       let format = OnionHopDataFormat::Legacy {
-                               short_channel_id: Readable::read(r)?,
-                       };
-                       let amt: u64 = Readable::read(r)?;
-                       let cltv_value: u32 = Readable::read(r)?;
-                       r.read_exact(&mut [0; 12])?;
-                       (format, amt, cltv_value)
+                       if let &Some(ref data) = &payment_data {
+                               if data.total_msat > MAX_VALUE_MSAT {
+                                       return Err(DecodeError::InvalidValue);
+                               }
+                       }
+                       OnionHopDataFormat::FinalNode {
+                               payment_data,
+                               keysend_preimage,
+                       }
                };
 
-               if amt > MAX_VALUE_MSAT {
+               if amt.0 > MAX_VALUE_MSAT {
                        return Err(DecodeError::InvalidValue);
                }
                Ok(OnionHopData {
                        format,
-                       amt_to_forward: amt,
-                       outgoing_cltv_value: cltv_value,
+                       amt_to_forward: amt.0,
+                       outgoing_cltv_value: cltv_value.0,
                })
        }
 }
@@ -1910,11 +1935,11 @@ impl_writeable_msg!(GossipTimestampFilter, {
 #[cfg(test)]
 mod tests {
        use hex;
-       use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
-       use ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
-       use ln::msgs;
-       use ln::msgs::{FinalOnionHopData, OptionalField, OnionErrorPacket, OnionHopDataFormat};
-       use util::ser::{Writeable, Readable, Hostname};
+       use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
+       use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
+       use crate::ln::msgs;
+       use crate::ln::msgs::{FinalOnionHopData, OptionalField, OnionErrorPacket, OnionHopDataFormat};
+       use crate::util::ser::{Writeable, Readable, Hostname};
 
        use bitcoin::hashes::hex::FromHex;
        use bitcoin::util::address::Address;
@@ -1926,8 +1951,8 @@ mod tests {
        use bitcoin::secp256k1::{PublicKey,SecretKey};
        use bitcoin::secp256k1::{Secp256k1, Message};
 
-       use io::{self, Cursor};
-       use prelude::*;
+       use crate::io::{self, Cursor};
+       use crate::prelude::*;
        use core::convert::TryFrom;
 
        #[test]
@@ -2013,7 +2038,7 @@ mod tests {
                let sig_2 = get_sig_on!(privkey_2, secp_ctx, String::from("01010101010101010101010101010101"));
                let sig_3 = get_sig_on!(privkey_3, secp_ctx, String::from("01010101010101010101010101010101"));
                let sig_4 = get_sig_on!(privkey_4, secp_ctx, String::from("01010101010101010101010101010101"));
-               let mut features = ChannelFeatures::known();
+               let mut features = ChannelFeatures::empty();
                if unknown_features_bits {
                        features = ChannelFeatures::from_le_bytes(vec![0xFF, 0xFF]);
                }
@@ -2618,20 +2643,6 @@ mod tests {
                assert_eq!(encoded_value, target_value);
        }
 
-       #[test]
-       fn encoding_legacy_onion_hop_data() {
-               let msg = msgs::OnionHopData {
-                       format: OnionHopDataFormat::Legacy {
-                               short_channel_id: 0xdeadbeef1bad1dea,
-                       },
-                       amt_to_forward: 0x0badf00d01020304,
-                       outgoing_cltv_value: 0xffffffff,
-               };
-               let encoded_value = msg.encode();
-               let target_value = hex::decode("00deadbeef1bad1dea0badf00d01020304ffffffff000000000000000000000000").unwrap();
-               assert_eq!(encoded_value, target_value);
-       }
-
        #[test]
        fn encoding_nonfinal_onion_hop_data() {
                let mut msg = msgs::OnionHopData {
@@ -2853,7 +2864,7 @@ mod tests {
        }
        // see above test, needs to be a separate method for use of the serialization macros.
        fn encode_big_payload() -> Result<Vec<u8>, io::Error> {
-               use util::ser::HighZeroBytesDroppedBigSize;
+               use crate::util::ser::HighZeroBytesDroppedBigSize;
                let payload = msgs::OnionHopData {
                        format: OnionHopDataFormat::NonFinalNode {
                                short_channel_id: 0xdeadbeef1bad1dea,