Merge pull request #1331 from TheBlueMatt/2022-02-no-copy-invoice-fields

Use &mut self in invoice updaters, not take-self-return-Self

diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs
index 841e3f9c69549b4a5a61e4a6e98183261b27e1db..1dd7123a486d1077f1295b7655b1fac497cfb57f 100644 (file)
--- a/lightning-invoice/src/lib.rs
+++ b/lightning-invoice/src/lib.rs
@@ -612,9 +612,9 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, T,
 impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T, C, tb::False> {
        /// Sets the payment secret and relevant features.
        pub fn payment_secret(mut self, payment_secret: PaymentSecret) -> InvoiceBuilder<D, H, T, C, tb::True> {
-               let features = InvoiceFeatures::empty()
-                       .set_variable_length_onion_required()
-                       .set_payment_secret_required();
+               let mut features = InvoiceFeatures::empty();
+               features.set_variable_length_onion_required();
+               features.set_payment_secret_required();
                self.tagged_fields.push(TaggedField::PaymentSecret(payment_secret));
                self.tagged_fields.push(TaggedField::Features(features));
                self.set_flags()
@@ -624,13 +624,11 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T,
 impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T, C, tb::True> {
        /// Sets the `basic_mpp` feature as optional.
        pub fn basic_mpp(mut self) -> Self {
-               self.tagged_fields = self.tagged_fields
-                       .drain(..)
-                       .map(|field| match field {
-                               TaggedField::Features(f) => TaggedField::Features(f.set_basic_mpp_optional()),
-                               _ => field,
-                       })
-                       .collect();
+               for field in self.tagged_fields.iter_mut() {
+                       if let TaggedField::Features(f) = field {
+                               f.set_basic_mpp_optional();
+                       }
+               }
                self
        }
 }

diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs
index 0ccce88e9f5d71743530a441504ac7c18b1da6a2..7d7553dfa0222acc76c66e239bb64613c658004d 100644 (file)
--- a/lightning/src/ln/features.rs
+++ b/lightning/src/ln/features.rs
@@ -318,15 +318,13 @@ mod sealed {
 
                        impl <T: $feature> Features<T> {
                                /// Set this feature as optional.
-                               pub fn $optional_setter(mut self) -> Self {
+                               pub fn $optional_setter(&mut self) {
                                        <T as $feature>::set_optional_bit(&mut self.flags);
-                                       self
                                }
 
                                /// Set this feature as required.
-                               pub fn $required_setter(mut self) -> Self {
+                               pub fn $required_setter(&mut self) {
                                        <T as $feature>::set_required_bit(&mut self.flags);
-                                       self
                                }
 
                                /// Checks if this feature is supported.
@@ -506,7 +504,9 @@ impl InvoiceFeatures {
        /// [`PaymentParameters::for_keysend`]: crate::routing::router::PaymentParameters::for_keysend
        /// [`find_route`]: crate::routing::router::find_route
        pub(crate) fn for_keysend() -> InvoiceFeatures {
-               InvoiceFeatures::empty().set_variable_length_onion_optional()
+               let mut res = InvoiceFeatures::empty();
+               res.set_variable_length_onion_optional();
+               res
        }
 }
 
@@ -838,11 +838,13 @@ mod tests {
                assert!(!features.requires_unknown_bits());
                assert!(!features.supports_unknown_bits());
 
-               let features = ChannelFeatures::empty().set_unknown_feature_required();
+               let mut features = ChannelFeatures::empty();
+               features.set_unknown_feature_required();
                assert!(features.requires_unknown_bits());
                assert!(features.supports_unknown_bits());
 
-               let features = ChannelFeatures::empty().set_unknown_feature_optional();
+               let mut features = ChannelFeatures::empty();
+               features.set_unknown_feature_optional();
                assert!(!features.requires_unknown_bits());
                assert!(features.supports_unknown_bits());
        }
@@ -886,7 +888,8 @@ mod tests {
        fn convert_to_context_with_unknown_flags() {
                // Ensure the `from` context has fewer known feature bytes than the `to` context.
                assert!(InvoiceFeatures::known().flags.len() < NodeFeatures::known().flags.len());
-               let invoice_features = InvoiceFeatures::known().set_unknown_feature_optional();
+               let mut invoice_features = InvoiceFeatures::known();
+               invoice_features.set_unknown_feature_optional();
                assert!(invoice_features.supports_unknown_bits());
                let node_features: NodeFeatures = invoice_features.to_context();
                assert!(!node_features.supports_unknown_bits());
@@ -894,9 +897,9 @@ mod tests {
 
        #[test]
        fn set_feature_bits() {
-               let features = InvoiceFeatures::empty()
-                       .set_basic_mpp_optional()
-                       .set_payment_secret_required();
+               let mut features = InvoiceFeatures::empty();
+               features.set_basic_mpp_optional();
+               features.set_payment_secret_required();
                assert!(features.supports_basic_mpp());
                assert!(!features.requires_basic_mpp());
                assert!(features.requires_payment_secret());
@@ -938,7 +941,8 @@ mod tests {
        fn test_channel_type_mapping() {
                // If we map an InvoiceFeatures with StaticRemoteKey optional, it should map into a
                // required-StaticRemoteKey ChannelTypeFeatures.
-               let init_features = InitFeatures::empty().set_static_remote_key_optional();
+               let mut init_features = InitFeatures::empty();
+               init_features.set_static_remote_key_optional();
                let converted_features = ChannelTypeFeatures::from_counterparty_init(&init_features);
                assert_eq!(converted_features, ChannelTypeFeatures::only_static_remote_key());
                assert!(!converted_features.supports_any_optional_bits());

diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 96eabf89bbf570d5d0bae7d9299c064648487880..62cbda9a76ddd097710d897537e11f104c1ec408 100644 (file)
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -2488,7 +2488,8 @@ mod tests {
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
                // Disable nodes 1, 2, and 8 by requiring unknown feature bits
-               let unknown_features = NodeFeatures::known().set_unknown_feature_required();
+               let mut unknown_features = NodeFeatures::known();
+               unknown_features.set_unknown_feature_required();
                add_or_update_node(&net_graph_msg_handler, &secp_ctx, &privkeys[0], unknown_features.clone(), 1);
                add_or_update_node(&net_graph_msg_handler, &secp_ctx, &privkeys[1], unknown_features.clone(), 1);
                add_or_update_node(&net_graph_msg_handler, &secp_ctx, &privkeys[7], unknown_features.clone(), 1);