Skip to content

Commit b1c6499

Browse files
committed
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.
1 parent 4bd2c97 commit b1c6499

File tree

2 files changed

+71
-28
lines changed

2 files changed

+71
-28
lines changed

lightning/src/ln/features.rs

Lines changed: 61 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,16 @@ mod sealed {
173173
(flags[Self::BYTE_OFFSET] & (Self::REQUIRED_MASK | Self::OPTIONAL_MASK)) != 0
174174
}
175175

176+
/// Sets the feature's required (even) bit in the given flags.
177+
#[inline]
178+
fn set_required_bit(flags: &mut Vec<u8>) {
179+
if flags.len() <= Self::BYTE_OFFSET {
180+
flags.resize(Self::BYTE_OFFSET + 1, 0u8);
181+
}
182+
183+
flags[Self::BYTE_OFFSET] |= Self::REQUIRED_MASK;
184+
}
185+
176186
/// Sets the feature's optional (odd) bit in the given flags.
177187
#[inline]
178188
fn set_optional_bit(flags: &mut Vec<u8>) {
@@ -191,6 +201,10 @@ mod sealed {
191201
flags[Self::BYTE_OFFSET] &= !Self::REQUIRED_MASK;
192202
flags[Self::BYTE_OFFSET] &= !Self::OPTIONAL_MASK;
193203
}
204+
205+
let last_non_zero_byte = flags.iter().rposition(|&byte| byte != 0);
206+
let size = if let Some(offset) = last_non_zero_byte { offset + 1 } else { 0 };
207+
flags.resize(size, 0u8);
194208
}
195209
}
196210

@@ -219,6 +233,30 @@ mod sealed {
219233
"Feature flags for `payment_secret`.");
220234
define_feature!(17, BasicMPP, [InitContext, NodeContext],
221235
"Feature flags for `basic_mpp`.");
236+
237+
#[cfg(test)]
238+
define_context!(TestingContext {
239+
required_features: [
240+
// Byte 0
241+
,
242+
// Byte 1
243+
,
244+
// Byte 2
245+
UnknownFeature,
246+
],
247+
optional_features: [
248+
// Byte 0
249+
,
250+
// Byte 1
251+
,
252+
// Byte 2
253+
,
254+
],
255+
});
256+
257+
#[cfg(test)]
258+
define_feature!(23, UnknownFeature, [TestingContext],
259+
"Feature flags for an unknown feature used in testing.");
222260
}
223261

224262
/// Tracks the set of features which a node implements, templated by the context in which it
@@ -375,23 +413,18 @@ impl<T: sealed::Context> Features<T> {
375413
}
376414

377415
#[cfg(test)]
378-
pub(crate) fn set_require_unknown_bits(&mut self) {
379-
let newlen = cmp::max(3, self.flags.len());
380-
self.flags.resize(newlen, 0u8);
381-
self.flags[2] |= 0x40;
416+
pub(crate) fn set_required_unknown_bits(&mut self) {
417+
<sealed::TestingContext as sealed::UnknownFeature>::set_required_bit(&mut self.flags);
382418
}
383419

384420
#[cfg(test)]
385-
pub(crate) fn clear_require_unknown_bits(&mut self) {
386-
let newlen = cmp::max(3, self.flags.len());
387-
self.flags.resize(newlen, 0u8);
388-
self.flags[2] &= !0x40;
389-
if self.flags.len() == 3 && self.flags[2] == 0 {
390-
self.flags.resize(2, 0u8);
391-
}
392-
if self.flags.len() == 2 && self.flags[1] == 0 {
393-
self.flags.resize(1, 0u8);
394-
}
421+
pub(crate) fn set_optional_unknown_bits(&mut self) {
422+
<sealed::TestingContext as sealed::UnknownFeature>::set_optional_bit(&mut self.flags);
423+
}
424+
425+
#[cfg(test)]
426+
pub(crate) fn clear_unknown_bits(&mut self) {
427+
<sealed::TestingContext as sealed::UnknownFeature>::clear_bits(&mut self.flags);
395428
}
396429
}
397430

@@ -502,12 +535,22 @@ mod tests {
502535
}
503536

504537
#[test]
505-
fn sanity_test_unkown_bits_testing() {
506-
let mut features = ChannelFeatures::known();
507-
features.set_require_unknown_bits();
538+
fn sanity_test_unknown_bits() {
539+
let mut features = ChannelFeatures::empty();
540+
assert!(!features.requires_unknown_bits());
541+
assert!(!features.supports_unknown_bits());
542+
543+
features.set_required_unknown_bits();
508544
assert!(features.requires_unknown_bits());
509-
features.clear_require_unknown_bits();
545+
assert!(features.supports_unknown_bits());
546+
547+
features.clear_unknown_bits();
548+
assert!(!features.requires_unknown_bits());
549+
assert!(!features.supports_unknown_bits());
550+
551+
features.set_optional_unknown_bits();
510552
assert!(!features.requires_unknown_bits());
553+
assert!(features.supports_unknown_bits());
511554
}
512555

513556
#[test]

lightning/src/ln/router.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,8 +1539,8 @@ mod tests {
15391539

15401540
{ // Disable channels 4 and 12 by requiring unknown feature bits
15411541
let mut network = router.network_map.write().unwrap();
1542-
network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.set_require_unknown_bits();
1543-
network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.set_require_unknown_bits();
1542+
network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.set_required_unknown_bits();
1543+
network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.set_required_unknown_bits();
15441544
}
15451545

15461546
{ // If all the channels require some features we don't understand, route should fail
@@ -1581,15 +1581,15 @@ mod tests {
15811581

15821582
{ // Re-enable channels 4 and 12 by wiping the unknown feature bits
15831583
let mut network = router.network_map.write().unwrap();
1584-
network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.clear_require_unknown_bits();
1585-
network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.clear_require_unknown_bits();
1584+
network.channels.get_mut(&NetworkMap::get_key(4, zero_hash.clone())).unwrap().features.clear_unknown_bits();
1585+
network.channels.get_mut(&NetworkMap::get_key(12, zero_hash.clone())).unwrap().features.clear_unknown_bits();
15861586
}
15871587

15881588
{ // Disable nodes 1, 2, and 8 by requiring unknown feature bits
15891589
let mut network = router.network_map.write().unwrap();
1590-
network.nodes.get_mut(&node1).unwrap().features.set_require_unknown_bits();
1591-
network.nodes.get_mut(&node2).unwrap().features.set_require_unknown_bits();
1592-
network.nodes.get_mut(&node8).unwrap().features.set_require_unknown_bits();
1590+
network.nodes.get_mut(&node1).unwrap().features.set_required_unknown_bits();
1591+
network.nodes.get_mut(&node2).unwrap().features.set_required_unknown_bits();
1592+
network.nodes.get_mut(&node8).unwrap().features.set_required_unknown_bits();
15931593
}
15941594

15951595
{ // If all nodes require some features we don't understand, route should fail
@@ -1630,9 +1630,9 @@ mod tests {
16301630

16311631
{ // Re-enable nodes 1, 2, and 8
16321632
let mut network = router.network_map.write().unwrap();
1633-
network.nodes.get_mut(&node1).unwrap().features.clear_require_unknown_bits();
1634-
network.nodes.get_mut(&node2).unwrap().features.clear_require_unknown_bits();
1635-
network.nodes.get_mut(&node8).unwrap().features.clear_require_unknown_bits();
1633+
network.nodes.get_mut(&node1).unwrap().features.clear_unknown_bits();
1634+
network.nodes.get_mut(&node2).unwrap().features.clear_unknown_bits();
1635+
network.nodes.get_mut(&node8).unwrap().features.clear_unknown_bits();
16361636
}
16371637

16381638
// Note that we don't test disabling node 3 and failing to route to it, as we (somewhat

0 commit comments

Comments
 (0)