From b1c6499e1c50fc41c443883dde8e8248106089c3 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 27 Apr 2020 22:12:53 -0700 Subject: [PATCH] Expand testing of unknown feature bits Include tests for requires_unknown_bits and supports_unknown_bits when an unknown even bit, odd bit, or neither is set. Refactor bit clearing such that tests and production code share the same code path. Fix a potential spec incompatibility (currently only exposed in testing code) where trailing zero bytes are not removed after a bit is cleared. --- lightning/src/ln/features.rs | 79 ++++++++++++++++++++++++++++-------- lightning/src/ln/router.rs | 20 ++++----- 2 files changed, 71 insertions(+), 28 deletions(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 381297703..613d2db14 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -173,6 +173,16 @@ mod sealed { (flags[Self::BYTE_OFFSET] & (Self::REQUIRED_MASK | Self::OPTIONAL_MASK)) != 0 } + /// Sets the feature's required (even) bit in the given flags. + #[inline] + fn set_required_bit(flags: &mut Vec) { + if flags.len() <= Self::BYTE_OFFSET { + flags.resize(Self::BYTE_OFFSET + 1, 0u8); + } + + flags[Self::BYTE_OFFSET] |= Self::REQUIRED_MASK; + } + /// Sets the feature's optional (odd) bit in the given flags. #[inline] fn set_optional_bit(flags: &mut Vec) { @@ -191,6 +201,10 @@ mod sealed { flags[Self::BYTE_OFFSET] &= !Self::REQUIRED_MASK; flags[Self::BYTE_OFFSET] &= !Self::OPTIONAL_MASK; } + + let last_non_zero_byte = flags.iter().rposition(|&byte| byte != 0); + let size = if let Some(offset) = last_non_zero_byte { offset + 1 } else { 0 }; + flags.resize(size, 0u8); } } @@ -219,6 +233,30 @@ mod sealed { "Feature flags for `payment_secret`."); define_feature!(17, BasicMPP, [InitContext, NodeContext], "Feature flags for `basic_mpp`."); + + #[cfg(test)] + define_context!(TestingContext { + required_features: [ + // Byte 0 + , + // Byte 1 + , + // Byte 2 + UnknownFeature, + ], + optional_features: [ + // Byte 0 + , + // Byte 1 + , + // Byte 2 + , + ], + }); + + #[cfg(test)] + define_feature!(23, UnknownFeature, [TestingContext], + "Feature flags for an unknown feature used in testing."); } /// Tracks the set of features which a node implements, templated by the context in which it @@ -375,23 +413,18 @@ impl Features { } #[cfg(test)] - pub(crate) fn set_require_unknown_bits(&mut self) { - let newlen = cmp::max(3, self.flags.len()); - self.flags.resize(newlen, 0u8); - self.flags[2] |= 0x40; + pub(crate) fn set_required_unknown_bits(&mut self) { + ::set_required_bit(&mut self.flags); } #[cfg(test)] - pub(crate) fn clear_require_unknown_bits(&mut self) { - let newlen = cmp::max(3, self.flags.len()); - self.flags.resize(newlen, 0u8); - self.flags[2] &= !0x40; - if self.flags.len() == 3 && self.flags[2] == 0 { - self.flags.resize(2, 0u8); - } - if self.flags.len() == 2 && self.flags[1] == 0 { - self.flags.resize(1, 0u8); - } + pub(crate) fn set_optional_unknown_bits(&mut self) { + ::set_optional_bit(&mut self.flags); + } + + #[cfg(test)] + pub(crate) fn clear_unknown_bits(&mut self) { + ::clear_bits(&mut self.flags); } } @@ -502,12 +535,22 @@ mod tests { } #[test] - fn sanity_test_unkown_bits_testing() { - let mut features = ChannelFeatures::known(); - features.set_require_unknown_bits(); + fn sanity_test_unknown_bits() { + let mut features = ChannelFeatures::empty(); + assert!(!features.requires_unknown_bits()); + assert!(!features.supports_unknown_bits()); + + features.set_required_unknown_bits(); assert!(features.requires_unknown_bits()); - features.clear_require_unknown_bits(); + assert!(features.supports_unknown_bits()); + + features.clear_unknown_bits(); + assert!(!features.requires_unknown_bits()); + assert!(!features.supports_unknown_bits()); + + features.set_optional_unknown_bits(); assert!(!features.requires_unknown_bits()); + assert!(features.supports_unknown_bits()); } #[test] diff --git a/lightning/src/ln/router.rs b/lightning/src/ln/router.rs index fb2364f7d..1eec7533b 100644 --- a/lightning/src/ln/router.rs +++ b/lightning/src/ln/router.rs @@ -1539,8 +1539,8 @@ mod tests { { // Disable channels 4 and 12 by requiring unknown feature bits let mut network = router.network_map.write().unwrap(); - network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.set_require_unknown_bits(); - network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.set_require_unknown_bits(); + network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.set_required_unknown_bits(); + network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.set_required_unknown_bits(); } { // If all the channels require some features we don't understand, route should fail @@ -1581,15 +1581,15 @@ mod tests { { // Re-enable channels 4 and 12 by wiping the unknown feature bits let mut network = router.network_map.write().unwrap(); - network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.clear_require_unknown_bits(); - network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.clear_require_unknown_bits(); + network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.clear_unknown_bits(); + network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.clear_unknown_bits(); } { // Disable nodes 1, 2, and 8 by requiring unknown feature bits let mut network = router.network_map.write().unwrap(); - network.nodes.get_mut(&node1).unwrap().features.set_require_unknown_bits(); - network.nodes.get_mut(&node2).unwrap().features.set_require_unknown_bits(); - network.nodes.get_mut(&node8).unwrap().features.set_require_unknown_bits(); + network.nodes.get_mut(&node1).unwrap().features.set_required_unknown_bits(); + network.nodes.get_mut(&node2).unwrap().features.set_required_unknown_bits(); + network.nodes.get_mut(&node8).unwrap().features.set_required_unknown_bits(); } { // If all nodes require some features we don't understand, route should fail @@ -1630,9 +1630,9 @@ mod tests { { // Re-enable nodes 1, 2, and 8 let mut network = router.network_map.write().unwrap(); - network.nodes.get_mut(&node1).unwrap().features.clear_require_unknown_bits(); - network.nodes.get_mut(&node2).unwrap().features.clear_require_unknown_bits(); - network.nodes.get_mut(&node8).unwrap().features.clear_require_unknown_bits(); + network.nodes.get_mut(&node1).unwrap().features.clear_unknown_bits(); + network.nodes.get_mut(&node2).unwrap().features.clear_unknown_bits(); + network.nodes.get_mut(&node8).unwrap().features.clear_unknown_bits(); } // Note that we don't test disabling node 3 and failing to route to it, as we (somewhat -- 2.39.5