From: Matt Corallo <git@bluematt.me>
Date: Wed, 8 Jan 2020 00:21:17 +0000 (-0500)
Subject: Seal the features contexts
X-Git-Tag: v0.0.12~160^2
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=49f88eca28372f1a3b5b6f95049f9dad5946f718;p=rust-lightning

Seal the features contexts
---

diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs
index ac9af1434..816bf1dff 100644
--- a/lightning/src/ln/features.rs
+++ b/lightning/src/ln/features.rs
@@ -8,63 +8,38 @@ use std::marker::PhantomData;
 use ln::msgs::DecodeError;
 use util::ser::{Readable, Writeable, Writer};
 
-/// The context in which a Feature object appears determines which bits of features the node
-/// supports will be set. We use this when creating our own Feature objects to select which bits to
-/// set and when passing around Feature objects to ensure the bits we're checking for are
-/// available.
-///
-/// This Context represents when the Feature appears in the init message, sent between peers and not
-/// rumored around the P2P network.
-pub struct FeatureContextInit {}
-/// The context in which a Feature object appears determines which bits of features the node
-/// supports will be set. We use this when creating our own Feature objects to select which bits to
-/// set and when passing around Feature objects to ensure the bits we're checking for are
-/// available.
-///
-/// This Context represents when the Feature appears in the node_announcement message, as it is
-/// rumored around the P2P network.
-pub struct FeatureContextNode {}
-/// The context in which a Feature object appears determines which bits of features the node
-/// supports will be set. We use this when creating our own Feature objects to select which bits to
-/// set and when passing around Feature objects to ensure the bits we're checking for are
-/// available.
-///
-/// This Context represents when the Feature appears in the ChannelAnnouncement message, as it is
-/// rumored around the P2P network.
-pub struct FeatureContextChannel {}
-/// The context in which a Feature object appears determines which bits of features the node
-/// supports will be set. We use this when creating our own Feature objects to select which bits to
-/// set and when passing around Feature objects to ensure the bits we're checking for are
-/// available.
-///
-/// This Context represents when the Feature appears in an invoice, used to determine the different
-/// options available for routing a payment.
-///
-/// Note that this is currently unused as invoices come to us via a different crate and are not
-/// native to rust-lightning directly.
-pub struct FeatureContextInvoice {}
+mod sealed { // You should just use the type aliases instead.
+	pub struct InitContext {}
+	pub struct NodeContext {}
+	pub struct ChannelContext {}
 
-/// An internal trait capturing the various future context types
-pub trait FeatureContext {}
-impl FeatureContext for FeatureContextInit {}
-impl FeatureContext for FeatureContextNode {}
-impl FeatureContext for FeatureContextChannel {}
-impl FeatureContext for FeatureContextInvoice {}
+	/// An internal trait capturing the various feature context types
+	pub trait Context {}
+	impl Context for InitContext {}
+	impl Context for NodeContext {}
+	impl Context for ChannelContext {}
 
-/// An internal trait capturing FeatureContextInit and FeatureContextNode
-pub trait FeatureContextInitNode : FeatureContext {}
-impl FeatureContextInitNode for FeatureContextInit {}
-impl FeatureContextInitNode for FeatureContextNode {}
+	pub trait DataLossProtect: Context {}
+	impl DataLossProtect for InitContext {}
+	impl DataLossProtect for NodeContext {}
+
+	pub trait InitialRoutingSync: Context {}
+	impl InitialRoutingSync for InitContext {}
+
+	pub trait UpfrontShutdownScript: Context {}
+	impl UpfrontShutdownScript for InitContext {}
+	impl UpfrontShutdownScript for NodeContext {}
+}
 
 /// Tracks the set of features which a node implements, templated by the context in which it
 /// appears.
-pub struct Features<T: FeatureContext> {
+pub struct Features<T: sealed::Context> {
 	/// Note that, for convinience, flags is LITTLE endian (despite being big-endian on the wire)
 	flags: Vec<u8>,
 	mark: PhantomData<T>,
 }
 
-impl<T: FeatureContext> Clone for Features<T> {
+impl<T: sealed::Context> Clone for Features<T> {
 	fn clone(&self) -> Self {
 		Self {
 			flags: self.flags.clone(),
@@ -72,61 +47,106 @@ impl<T: FeatureContext> Clone for Features<T> {
 		}
 	}
 }
-impl<T: FeatureContext> PartialEq for Features<T> {
+impl<T: sealed::Context> PartialEq for Features<T> {
 	fn eq(&self, o: &Self) -> bool {
 		self.flags.eq(&o.flags)
 	}
 }
-impl<T: FeatureContext> fmt::Debug for Features<T> {
+impl<T: sealed::Context> fmt::Debug for Features<T> {
 	fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> {
 		self.flags.fmt(fmt)
 	}
 }
 
 /// A feature message as it appears in an init message
-pub type InitFeatures = Features<FeatureContextInit>;
+pub type InitFeatures = Features<sealed::InitContext>;
 /// A feature message as it appears in a node_announcement message
-pub type NodeFeatures = Features<FeatureContextNode>;
+pub type NodeFeatures = Features<sealed::NodeContext>;
 /// A feature message as it appears in a channel_announcement message
-pub type ChannelFeatures = Features<FeatureContextChannel>;
+pub type ChannelFeatures = Features<sealed::ChannelContext>;
 
-impl<T: FeatureContextInitNode> Features<T> {
+impl InitFeatures {
 	/// Create a Features with the features we support
 	#[cfg(not(feature = "fuzztarget"))]
-	pub(crate) fn supported() -> Features<T> {
-		Features {
+	pub(crate) fn supported() -> InitFeatures {
+		InitFeatures {
 			flags: vec![2 | 1 << 5],
 			mark: PhantomData,
 		}
 	}
 	#[cfg(feature = "fuzztarget")]
-	pub fn supported() -> Features<T> {
-		Features {
+	pub fn supported() -> InitFeatures {
+		InitFeatures {
 			flags: vec![2 | 1 << 5],
 			mark: PhantomData,
 		}
 	}
+
+	/// Writes all features present up to, and including, 13.
+	pub(crate) fn write_up_to_13<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
+		let len = cmp::min(2, self.flags.len());
+		w.size_hint(len + 2);
+		(len as u16).write(w)?;
+		for i in (0..len).rev() {
+			if i == 0 {
+				self.flags[i].write(w)?;
+			} else {
+				// On byte 1, we want up-to-and-including-bit-13, 0-indexed, which is
+				// up-to-and-including-bit-5, 0-indexed, on this byte:
+				(self.flags[i] & 0b00_11_11_11).write(w)?;
+			}
+		}
+		Ok(())
+	}
+
+	/// or's another InitFeatures into this one.
+	pub(crate) fn or(mut self, o: InitFeatures) -> InitFeatures {
+		let total_feature_len = cmp::max(self.flags.len(), o.flags.len());
+		self.flags.resize(total_feature_len, 0u8);
+		for (byte, o_byte) in self.flags.iter_mut().zip(o.flags.iter()) {
+			*byte |= *o_byte;
+		}
+		self
+	}
 }
 
-impl Features<FeatureContextChannel> {
+impl ChannelFeatures {
 	/// Create a Features with the features we support
 	#[cfg(not(feature = "fuzztarget"))]
-	pub(crate) fn supported() -> Features<FeatureContextChannel> {
-		Features {
+	pub(crate) fn supported() -> ChannelFeatures {
+		ChannelFeatures {
 			flags: Vec::new(),
 			mark: PhantomData,
 		}
 	}
 	#[cfg(feature = "fuzztarget")]
-	pub fn supported() -> Features<FeatureContextChannel> {
-		Features {
+	pub fn supported() -> ChannelFeatures {
+		ChannelFeatures {
 			flags: Vec::new(),
 			mark: PhantomData,
 		}
 	}
 }
 
-impl<T: FeatureContext> Features<T> {
+impl NodeFeatures {
+	/// Create a Features with the features we support
+	#[cfg(not(feature = "fuzztarget"))]
+	pub(crate) fn supported() -> NodeFeatures {
+		NodeFeatures {
+			flags: vec![2 | 1 << 5],
+			mark: PhantomData,
+		}
+	}
+	#[cfg(feature = "fuzztarget")]
+	pub fn supported() -> NodeFeatures {
+		NodeFeatures {
+			flags: vec![2 | 1 << 5],
+			mark: PhantomData,
+		}
+	}
+}
+
+impl<T: sealed::Context> Features<T> {
 	/// Create a blank Features with no features set
 	pub fn empty() -> Features<T> {
 		Features {
@@ -186,11 +206,13 @@ impl<T: FeatureContext> Features<T> {
 	}
 }
 
-impl<T: FeatureContextInitNode> Features<T> {
+impl<T: sealed::DataLossProtect> Features<T> {
 	pub(crate) fn supports_data_loss_protect(&self) -> bool {
 		self.flags.len() > 0 && (self.flags[0] & 3) != 0
 	}
+}
 
+impl<T: sealed::UpfrontShutdownScript> Features<T> {
 	pub(crate) fn supports_upfront_shutdown_script(&self) -> bool {
 		self.flags.len() > 0 && (self.flags[0] & (3 << 4)) != 0
 	}
@@ -200,7 +222,7 @@ impl<T: FeatureContextInitNode> Features<T> {
 	}
 }
 
-impl Features<FeatureContextInit> {
+impl<T: sealed::InitialRoutingSync> Features<T> {
 	pub(crate) fn initial_routing_sync(&self) -> bool {
 		self.flags.len() > 0 && (self.flags[0] & (1 << 3)) != 0
 	}
@@ -211,34 +233,9 @@ impl Features<FeatureContextInit> {
 			self.flags[0] |= 1 << 3;
 		}
 	}
-
-	/// Writes all features present up to, and including, 13.
-	pub(crate) fn write_up_to_13<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
-		let len = cmp::min(2, self.flags.len());
-		w.size_hint(len + 2);
-		(len as u16).write(w)?;
-		for i in (0..len).rev() {
-			if i == 0 {
-				self.flags[i].write(w)?;
-			} else {
-				(self.flags[i] & ((1 << (14 - 8)) - 1)).write(w)?;
-			}
-		}
-		Ok(())
-	}
-
-	/// or's another InitFeatures into this one.
-	pub(crate) fn or(mut self, o: InitFeatures) -> InitFeatures {
-		let total_feature_len = cmp::max(self.flags.len(), o.flags.len());
-		self.flags.resize(total_feature_len, 0u8);
-		for (feature, o_feature) in self.flags.iter_mut().zip(o.flags.iter()) {
-			*feature |= *o_feature;
-		}
-		self
-	}
 }
 
-impl<T: FeatureContext> Writeable for Features<T> {
+impl<T: sealed::Context> Writeable for Features<T> {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
 		w.size_hint(self.flags.len() + 2);
 		(self.flags.len() as u16).write(w)?;
@@ -249,7 +246,7 @@ impl<T: FeatureContext> Writeable for Features<T> {
 	}
 }
 
-impl<R: ::std::io::Read, T: FeatureContext> Readable<R> for Features<T> {
+impl<R: ::std::io::Read, T: sealed::Context> Readable<R> for Features<T> {
 	fn read(r: &mut R) -> Result<Self, DecodeError> {
 		let mut flags: Vec<u8> = Readable::read(r)?;
 		flags.reverse(); // Swap to little-endian