From: Matt Corallo <git@bluematt.me>
Date: Fri, 9 Aug 2024 14:18:09 +0000 (+0000)
Subject: Replace usages of `Features::is_subset` and remove it
X-Git-Tag: v0.0.124-beta~14^2~8
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=954b7be85a63c9a942056a33443bd7979b14dcc9;p=rust-lightning

Replace usages of `Features::is_subset` and remove it

It turns out all the places we use `Features::is_subset` we could
as well be using `Features::requires_unknown_bits_from`. Further,
in the next commit `Features` will move to a different crate so any
methods which the `lightning` crate uses will need to be public. As
the `is_subset` API is prety confusing (it doesn't consider
optional/required bits, only whether the bits themselves are
strictly a subset) it'd be nice to not have to expose it, which is
enabled here.
---

diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs
index b0dfca4c2..361e12fa5 100644
--- a/lightning/src/chain/package.rs
+++ b/lightning/src/chain/package.rs
@@ -93,7 +93,7 @@ pub(crate) fn verify_channel_type_features(channel_type_features: &Option<Channe
 			supported_feature_set |= additional_permitted_features;
 		}
 
-		if !features.is_subset(&supported_feature_set) {
+		if features.requires_unknown_bits_from(&supported_feature_set) {
 			return Err(DecodeError::UnknownRequiredFeature);
 		}
 	}
diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs
index 4d2572e89..8c811c920 100644
--- a/lightning/src/ln/chan_utils.rs
+++ b/lightning/src/ln/chan_utils.rs
@@ -772,10 +772,12 @@ pub(crate) fn legacy_deserialization_prevention_marker_for_channel_type_features
 	legacy_version_bit_set.set_scid_privacy_required();
 	legacy_version_bit_set.set_zero_conf_required();
 
-	if features.is_subset(&legacy_version_bit_set) {
-		None
-	} else {
+	debug_assert!(!legacy_version_bit_set.supports_any_optional_bits());
+	debug_assert!(!features.supports_any_optional_bits());
+	if features.requires_unknown_bits_from(&legacy_version_bit_set) {
 		Some(())
+	} else {
+		None
 	}
 }
 
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index afd4e7357..7ba94998a 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -1877,7 +1877,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 		}
 
 		let channel_type = get_initial_channel_type(&config, their_features);
-		debug_assert!(channel_type.is_subset(&channelmanager::provided_channel_type_features(&config)));
+		debug_assert!(!channel_type.supports_any_optional_bits());
+		debug_assert!(!channel_type.requires_unknown_bits_from(&channelmanager::provided_channel_type_features(&config)));
 
 		let (commitment_conf_target, anchor_outputs_value_msat)  = if channel_type.supports_anchors_zero_fee_htlc_tx() {
 			(ConfirmationTarget::AnchorChannelFee, ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000)
@@ -7967,7 +7968,7 @@ pub(super) fn channel_type_from_open_channel(
 			return Err(ChannelError::close("Channel Type was not understood - we require static remote key".to_owned()));
 		}
 		// Make sure we support all of the features behind the channel type.
-		if !channel_type.is_subset(our_supported_features) {
+		if channel_type.requires_unknown_bits_from(&our_supported_features) {
 			return Err(ChannelError::close("Channel Type contains unsupported features".to_owned()));
 		}
 		let announced_channel = if (common_fields.channel_flags & 1) == 1 { true } else { false };
@@ -9355,7 +9356,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 		}
 
 		let chan_features = channel_type.as_ref().unwrap();
-		if !chan_features.is_subset(our_supported_features) {
+		if chan_features.supports_any_optional_bits() || chan_features.requires_unknown_bits_from(&our_supported_features) {
 			// If the channel was written by a new version and negotiated with features we don't
 			// understand yet, refuse to read it.
 			return Err(DecodeError::UnknownRequiredFeature);
diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs
index 79bf871de..3b041cbc5 100644
--- a/lightning/src/ln/features.rs
+++ b/lightning/src/ln/features.rs
@@ -834,24 +834,6 @@ impl<T: sealed::Context> Features<T> {
 		})
 	}
 
-	// Returns true if the features within `self` are a subset of the features within `other`.
-	pub(crate) fn is_subset(&self, other: &Self) -> bool {
-		for (idx, byte) in self.flags.iter().enumerate() {
-			if let Some(other_byte) = other.flags.get(idx) {
-				if byte & other_byte != *byte {
-					// `self` has bits set that `other` doesn't.
-					return false;
-				}
-			} else {
-				if *byte > 0 {
-					// `self` has a non-zero byte that `other` doesn't.
-					return false;
-				}
-			}
-		}
-		true
-	}
-
 	/// Sets a required feature bit. Errors if `bit` is outside the feature range as defined
 	/// by [BOLT 9].
 	///