From a3ddb9fb19f68584c1724328931afbef37e9f172 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 6 Jan 2020 21:50:55 -0500 Subject: [PATCH 1/2] Avoid treating option_upfront_shutdown (req) as unknown_required. This fixes the bitmask in requires_unknown_bits. --- lightning/src/ln/features.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index dd0b7fa1a82..4f45b9338c6 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -187,8 +187,14 @@ impl Features { pub(crate) fn requires_unknown_bits(&self) -> bool { self.flags.iter().enumerate().any(|(idx, &byte)| { (match idx { - 0 => (byte & 0b00010100), + // Unknown bits are even bits which we don't understand, we list ones which we do + // here: + // unknown, upfront_shutdown_script, unknown (actually initial_routing_sync, but it + // is only valid as an optional feature), and data_loss_protect: + 0 => (byte & 0b01000100), + // unknown, unknown, unknown, var_onion_optin: 1 => (byte & 0b01010100), + // fallback, all even bits set: _ => (byte & 0b01010101), }) != 0 }) @@ -197,7 +203,10 @@ impl Features { pub(crate) fn supports_unknown_bits(&self) -> bool { self.flags.iter().enumerate().any(|(idx, &byte)| { (match idx { + // unknown, upfront_shutdown_script, initial_routing_sync (is only valid as an + // optional feature), and data_loss_protect: 0 => (byte & 0b11000100), + // unknown, unknown, unknown, var_onion_optin: 1 => (byte & 0b11111100), _ => byte, }) != 0 From 4ac9ed2f00a099cdc97898b59f8ac45ff300f65e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 7 Jan 2020 13:58:50 -0500 Subject: [PATCH 2/2] Add some basic sanity tests for feature flags --- lightning/src/ln/features.rs | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 4f45b9338c6..61a862c0c55 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -293,3 +293,41 @@ impl Readable for Features { }) } } + +#[cfg(test)] +mod tests { + use super::{ChannelFeatures, InitFeatures, NodeFeatures}; + + #[test] + fn sanity_test_our_features() { + assert!(!ChannelFeatures::supported().requires_unknown_bits()); + assert!(!ChannelFeatures::supported().supports_unknown_bits()); + assert!(!InitFeatures::supported().requires_unknown_bits()); + assert!(!InitFeatures::supported().supports_unknown_bits()); + assert!(!NodeFeatures::supported().requires_unknown_bits()); + assert!(!NodeFeatures::supported().supports_unknown_bits()); + + assert!(InitFeatures::supported().supports_upfront_shutdown_script()); + assert!(NodeFeatures::supported().supports_upfront_shutdown_script()); + + assert!(InitFeatures::supported().supports_data_loss_protect()); + assert!(NodeFeatures::supported().supports_data_loss_protect()); + + assert!(InitFeatures::supported().supports_variable_length_onion()); + assert!(NodeFeatures::supported().supports_variable_length_onion()); + + let mut init_features = InitFeatures::supported(); + init_features.set_initial_routing_sync(); + assert!(!init_features.requires_unknown_bits()); + assert!(!init_features.supports_unknown_bits()); + } + + #[test] + fn sanity_test_unkown_bits_testing() { + let mut features = ChannelFeatures::supported(); + features.set_require_unknown_bits(); + assert!(features.requires_unknown_bits()); + features.clear_require_unknown_bits(); + assert!(!features.requires_unknown_bits()); + } +}