Skip to content

Commit 954b7be

Browse files
committed
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.
1 parent b97d742 commit 954b7be

File tree

4 files changed

+10
-25
lines changed

4 files changed

+10
-25
lines changed

lightning/src/chain/package.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ pub(crate) fn verify_channel_type_features(channel_type_features: &Option<Channe
9393
supported_feature_set |= additional_permitted_features;
9494
}
9595

96-
if !features.is_subset(&supported_feature_set) {
96+
if features.requires_unknown_bits_from(&supported_feature_set) {
9797
return Err(DecodeError::UnknownRequiredFeature);
9898
}
9999
}

lightning/src/ln/chan_utils.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -772,10 +772,12 @@ pub(crate) fn legacy_deserialization_prevention_marker_for_channel_type_features
772772
legacy_version_bit_set.set_scid_privacy_required();
773773
legacy_version_bit_set.set_zero_conf_required();
774774

775-
if features.is_subset(&legacy_version_bit_set) {
776-
None
777-
} else {
775+
debug_assert!(!legacy_version_bit_set.supports_any_optional_bits());
776+
debug_assert!(!features.supports_any_optional_bits());
777+
if features.requires_unknown_bits_from(&legacy_version_bit_set) {
778778
Some(())
779+
} else {
780+
None
779781
}
780782
}
781783

lightning/src/ln/channel.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1877,7 +1877,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
18771877
}
18781878

18791879
let channel_type = get_initial_channel_type(&config, their_features);
1880-
debug_assert!(channel_type.is_subset(&channelmanager::provided_channel_type_features(&config)));
1880+
debug_assert!(!channel_type.supports_any_optional_bits());
1881+
debug_assert!(!channel_type.requires_unknown_bits_from(&channelmanager::provided_channel_type_features(&config)));
18811882

18821883
let (commitment_conf_target, anchor_outputs_value_msat) = if channel_type.supports_anchors_zero_fee_htlc_tx() {
18831884
(ConfirmationTarget::AnchorChannelFee, ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000)
@@ -7967,7 +7968,7 @@ pub(super) fn channel_type_from_open_channel(
79677968
return Err(ChannelError::close("Channel Type was not understood - we require static remote key".to_owned()));
79687969
}
79697970
// Make sure we support all of the features behind the channel type.
7970-
if !channel_type.is_subset(our_supported_features) {
7971+
if channel_type.requires_unknown_bits_from(&our_supported_features) {
79717972
return Err(ChannelError::close("Channel Type contains unsupported features".to_owned()));
79727973
}
79737974
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
93559356
}
93569357

93579358
let chan_features = channel_type.as_ref().unwrap();
9358-
if !chan_features.is_subset(our_supported_features) {
9359+
if chan_features.supports_any_optional_bits() || chan_features.requires_unknown_bits_from(&our_supported_features) {
93599360
// If the channel was written by a new version and negotiated with features we don't
93609361
// understand yet, refuse to read it.
93619362
return Err(DecodeError::UnknownRequiredFeature);

lightning/src/ln/features.rs

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -834,24 +834,6 @@ impl<T: sealed::Context> Features<T> {
834834
})
835835
}
836836

837-
// Returns true if the features within `self` are a subset of the features within `other`.
838-
pub(crate) fn is_subset(&self, other: &Self) -> bool {
839-
for (idx, byte) in self.flags.iter().enumerate() {
840-
if let Some(other_byte) = other.flags.get(idx) {
841-
if byte & other_byte != *byte {
842-
// `self` has bits set that `other` doesn't.
843-
return false;
844-
}
845-
} else {
846-
if *byte > 0 {
847-
// `self` has a non-zero byte that `other` doesn't.
848-
return false;
849-
}
850-
}
851-
}
852-
true
853-
}
854-
855837
/// Sets a required feature bit. Errors if `bit` is outside the feature range as defined
856838
/// by [BOLT 9].
857839
///

0 commit comments

Comments
 (0)