From 4bd2c974a2c96a2ce98a4055a72b2b3c687e78bc Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 22 Apr 2020 16:52:11 -0700 Subject: [PATCH] Generalize with_known_relevant_init_flags 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 | 64 ++++++++++++++++------------ lightning/src/ln/functional_tests.rs | 3 +- lightning/src/ln/router.rs | 10 ++--- 3 files changed, 43 insertions(+), 34 deletions(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 0dd5fa30..38129770 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -282,6 +282,12 @@ impl InitFeatures { } self } + + /// Converts `InitFeatures` to `Features`. Only known `InitFeatures` relevant to context `C` + /// are included in the result. + pub(crate) fn to_context(&self) -> Features { + self.to_context_internal() + } } impl Features { @@ -303,18 +309,19 @@ impl Features { } } - /// 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` to `Features`. Only known `T` features relevant to context `C` are + /// included in the result. + fn to_context_internal(&self) -> Features { + let byte_count = C::KNOWN_FEATURE_MASK.len(); 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 { - 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:: { flags, mark: PhantomData, } } #[cfg(test)] @@ -399,8 +406,9 @@ impl Features { ::supports_feature(&self.flags) } #[cfg(test)] - pub(crate) fn unset_upfront_shutdown_script(&mut self) { - ::clear_bits(&mut self.flags) + pub(crate) fn clear_upfront_shutdown_script(mut self) -> Self { + ::clear_bits(&mut self.flags); + self } } @@ -461,7 +469,7 @@ impl Readable for Features { #[cfg(test)] mod tests { - use super::{ChannelFeatures, InitFeatures, NodeFeatures, Features}; + use super::{ChannelFeatures, InitFeatures, NodeFeatures}; #[test] fn sanity_test_our_features() { @@ -503,26 +511,28 @@ mod tests { } #[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.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()); } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 0a2d1b59..d7bdcc2d 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -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 - 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()); diff --git a/lightning/src/ln/router.rs b/lightning/src/ln/router.rs index 9143d09b..fb2364f7 100644 --- a/lightning/src/ln/router.rs +++ b/lightning/src/ln/router.rs @@ -893,9 +893,9 @@ impl Router { 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, - 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, }]], @@ -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) { - 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. - 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 @@ -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) { - 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 { -- 2.30.2