Generalize with_known_relevant_init_flags
authorJeffrey Czyz <jkczyz@gmail.com>
Wed, 22 Apr 2020 23:52:11 +0000 (16:52 -0700)
committerJeffrey Czyz <jkczyz@gmail.com>
Wed, 29 Apr 2020 18:11:51 +0000 (11:11 -0700)
Converting from InitFeatures to other Features is accomplished using
Features::with_known_relevant_init_flags. Define a more general
to_context method which converts from Features of one Context to
another.

Additionally, ensure the source context only has known flags before
selecting flags for the target context.

lightning/src/ln/features.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/router.rs

index 0dd5fa3049364a853c99ce34fe69d226a8dab689..381297703e882a08cce4577e01d5bd3f7a9a87af 100644 (file)
@@ -282,6 +282,12 @@ impl InitFeatures {
                }
                self
        }
                }
                self
        }
+
+       /// Converts `InitFeatures` to `Features<C>`. Only known `InitFeatures` relevant to context `C`
+       /// are included in the result.
+       pub(crate) fn to_context<C: sealed::Context>(&self) -> Features<C> {
+               self.to_context_internal()
+       }
 }
 
 impl<T: sealed::Context> Features<T> {
 }
 
 impl<T: sealed::Context> Features<T> {
@@ -303,18 +309,19 @@ impl<T: sealed::Context> Features<T> {
                }
        }
 
                }
        }
 
-       /// Takes the flags that we know how to interpret in an init-context features that are also
-       /// relevant in a node-context features and creates a node-context features from them.
-       /// Be sure to blank out features that are unknown to us.
-       pub(crate) fn with_known_relevant_init_flags(init_ctx: &InitFeatures) -> Self {
-               let byte_count = T::KNOWN_FEATURE_MASK.len();
+       /// Converts `Features<T>` to `Features<C>`. Only known `T` features relevant to context `C` are
+       /// included in the result.
+       fn to_context_internal<C: sealed::Context>(&self) -> Features<C> {
+               let byte_count = C::KNOWN_FEATURE_MASK.len();
                let mut flags = Vec::new();
                let mut flags = Vec::new();
-               for (i, feature_byte) in init_ctx.flags.iter().enumerate() {
+               for (i, byte) in self.flags.iter().enumerate() {
                        if i < byte_count {
                        if i < byte_count {
-                               flags.push(feature_byte & T::KNOWN_FEATURE_MASK[i]);
+                               let known_source_features = T::KNOWN_FEATURE_MASK[i];
+                               let known_target_features = C::KNOWN_FEATURE_MASK[i];
+                               flags.push(byte & known_source_features & known_target_features);
                        }
                }
                        }
                }
-               Self { flags, mark: PhantomData, }
+               Features::<C> { flags, mark: PhantomData, }
        }
 
        #[cfg(test)]
        }
 
        #[cfg(test)]
@@ -399,8 +406,9 @@ impl<T: sealed::UpfrontShutdownScript> Features<T> {
                <T as sealed::UpfrontShutdownScript>::supports_feature(&self.flags)
        }
        #[cfg(test)]
                <T as sealed::UpfrontShutdownScript>::supports_feature(&self.flags)
        }
        #[cfg(test)]
-       pub(crate) fn unset_upfront_shutdown_script(&mut self) {
-               <T as sealed::UpfrontShutdownScript>::clear_bits(&mut self.flags)
+       pub(crate) fn clear_upfront_shutdown_script(mut self) -> Self {
+               <T as sealed::UpfrontShutdownScript>::clear_bits(&mut self.flags);
+               self
        }
 }
 
        }
 }
 
@@ -461,7 +469,7 @@ impl<T: sealed::Context> Readable for Features<T> {
 
 #[cfg(test)]
 mod tests {
 
 #[cfg(test)]
 mod tests {
-       use super::{ChannelFeatures, InitFeatures, NodeFeatures, Features};
+       use super::{ChannelFeatures, InitFeatures, NodeFeatures};
 
        #[test]
        fn sanity_test_our_features() {
 
        #[test]
        fn sanity_test_our_features() {
@@ -503,26 +511,28 @@ mod tests {
        }
 
        #[test]
        }
 
        #[test]
-       fn test_node_with_known_relevant_init_flags() {
-               // Create an InitFeatures with initial_routing_sync supported.
-               let init_features = InitFeatures::known();
+       fn convert_to_context_with_relevant_flags() {
+               let init_features = InitFeatures::known().clear_upfront_shutdown_script();
                assert!(init_features.initial_routing_sync());
                assert!(init_features.initial_routing_sync());
+               assert!(!init_features.supports_upfront_shutdown_script());
 
 
-               // Attempt to pull out non-node-context feature flags from these InitFeatures.
-               let res = NodeFeatures::with_known_relevant_init_flags(&init_features);
-
+               let node_features: NodeFeatures = init_features.to_context();
                {
                {
-                       // Check that the flags are as expected: optional_data_loss_protect,
-                       // option_upfront_shutdown_script, var_onion_optin, payment_secret, and
-                       // basic_mpp.
-                       assert_eq!(res.flags.len(), 3);
-                       assert_eq!(res.flags[0], 0b00100010);
-                       assert_eq!(res.flags[1], 0b10000010);
-                       assert_eq!(res.flags[2], 0b00000010);
+                       // Check that the flags are as expected:
+                       // - option_data_loss_protect
+                       // - var_onion_optin | payment_secret
+                       // - basic_mpp
+                       assert_eq!(node_features.flags.len(), 3);
+                       assert_eq!(node_features.flags[0], 0b00000010);
+                       assert_eq!(node_features.flags[1], 0b10000010);
+                       assert_eq!(node_features.flags[2], 0b00000010);
                }
 
                }
 
-               // Check that the initial_routing_sync feature was correctly blanked out.
-               let new_features: InitFeatures = Features::from_le_bytes(res.flags);
-               assert!(!new_features.initial_routing_sync());
+               // Check that cleared flags are kept blank when converting back:
+               // - initial_routing_sync was not applicable to NodeContext
+               // - upfront_shutdown_script was cleared before converting
+               let features: InitFeatures = node_features.to_context_internal();
+               assert!(!features.initial_routing_sync());
+               assert!(!features.supports_upfront_shutdown_script());
        }
 }
        }
 }
index 0a2d1b590026a63e8334cd1ac4d1f0bb59c3d886..d7bdcc2d2ac50299bd84dbdf93a3d156aaf7e24e 100644 (file)
@@ -6694,8 +6694,7 @@ fn test_upfront_shutdown_script() {
        }
 
        // We test that if case of peer non-signaling we don't enforce committed script at channel opening
        }
 
        // We test that if case of peer non-signaling we don't enforce committed script at channel opening
-       let mut flags_no = InitFeatures::known();
-       flags_no.unset_upfront_shutdown_script();
+       let flags_no = InitFeatures::known().clear_upfront_shutdown_script();
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, flags_no, flags.clone());
        nodes[0].node.close_channel(&OutPoint::new(chan.3.txid(), 0).to_channel_id()).unwrap();
        let mut node_1_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id());
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, flags_no, flags.clone());
        nodes[0].node.close_channel(&OutPoint::new(chan.3.txid(), 0).to_channel_id()).unwrap();
        let mut node_1_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id());
index 9143d09bb95e54a9a812c16f4fab883db2da8ed3..fb2364f7da3b062a48b941d9e49ba81ec07c7c6b 100644 (file)
@@ -893,9 +893,9 @@ impl Router {
                                        return Ok(Route {
                                                paths: vec![vec![RouteHop {
                                                        pubkey: chan.remote_network_id,
                                        return Ok(Route {
                                                paths: vec![vec![RouteHop {
                                                        pubkey: chan.remote_network_id,
-                                                       node_features: NodeFeatures::with_known_relevant_init_flags(&chan.counterparty_features),
+                                                       node_features: chan.counterparty_features.to_context(),
                                                        short_channel_id,
                                                        short_channel_id,
-                                                       channel_features: ChannelFeatures::with_known_relevant_init_flags(&chan.counterparty_features),
+                                                       channel_features: chan.counterparty_features.to_context(),
                                                        fee_msat: final_value_msat,
                                                        cltv_expiry_delta: final_cltv,
                                                }]],
                                                        fee_msat: final_value_msat,
                                                        cltv_expiry_delta: final_cltv,
                                                }]],
@@ -972,7 +972,7 @@ impl Router {
                        ( $node: expr, $node_id: expr, $fee_to_target_msat: expr ) => {
                                if first_hops.is_some() {
                                        if let Some(&(ref first_hop, ref features)) = first_hop_targets.get(&$node_id) {
                        ( $node: expr, $node_id: expr, $fee_to_target_msat: expr ) => {
                                if first_hops.is_some() {
                                        if let Some(&(ref first_hop, ref features)) = first_hop_targets.get(&$node_id) {
-                                               add_entry!(first_hop, $node_id, dummy_directional_info, ChannelFeatures::with_known_relevant_init_flags(&features), $fee_to_target_msat);
+                                               add_entry!(first_hop, $node_id, dummy_directional_info, features.to_context(), $fee_to_target_msat);
                                        }
                                }
 
                                        }
                                }
 
@@ -1016,7 +1016,7 @@ impl Router {
                                                        // bit lazy here. In the future, we should pull them out via our
                                                        // ChannelManager, but there's no reason to waste the space until we
                                                        // need them.
                                                        // bit lazy here. In the future, we should pull them out via our
                                                        // ChannelManager, but there's no reason to waste the space until we
                                                        // need them.
-                                                       add_entry!(first_hop, hop.src_node_id, dummy_directional_info, ChannelFeatures::with_known_relevant_init_flags(&features), 0);
+                                                       add_entry!(first_hop, hop.src_node_id, dummy_directional_info, features.to_context(), 0);
                                                }
                                        }
                                        // BOLT 11 doesn't allow inclusion of features for the last hop hints, which
                                                }
                                        }
                                        // BOLT 11 doesn't allow inclusion of features for the last hop hints, which
@@ -1031,7 +1031,7 @@ impl Router {
                                let mut res = vec!(dist.remove(&network.our_node_id).unwrap().3);
                                loop {
                                        if let Some(&(_, ref features)) = first_hop_targets.get(&res.last().unwrap().pubkey) {
                                let mut res = vec!(dist.remove(&network.our_node_id).unwrap().3);
                                loop {
                                        if let Some(&(_, ref features)) = first_hop_targets.get(&res.last().unwrap().pubkey) {
-                                               res.last_mut().unwrap().node_features = NodeFeatures::with_known_relevant_init_flags(&features);
+                                               res.last_mut().unwrap().node_features = features.to_context();
                                        } else if let Some(node) = network.nodes.get(&res.last().unwrap().pubkey) {
                                                res.last_mut().unwrap().node_features = node.features.clone();
                                        } else {
                                        } else if let Some(node) = network.nodes.get(&res.last().unwrap().pubkey) {
                                                res.last_mut().unwrap().node_features = node.features.clone();
                                        } else {