Reset inbound fee knobs to zero if our counterparty doesn't support

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 5e46667724c707ba5328a4917ea301af81c9ef45..b0a6def47a1f1167ec4d1670d2a510e7466016e9 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -918,6 +918,17 @@ impl<Signer: Sign> Channel<Signer> {
                let holder_signer = keys_provider.derive_channel_signer(channel_value_satoshis, channel_keys_id);
                let pubkeys = holder_signer.pubkeys().clone();
 
+               let mut config = LegacyChannelConfig {
+                       options: user_config.channel_config.clone(),
+                       announced_channel: user_config.channel_handshake_config.announced_channel,
+                       commit_upfront_shutdown_pubkey: user_config.channel_handshake_config.commit_upfront_shutdown_pubkey,
+               };
+
+               if !their_features.supports_inbound_fees() {
+                       config.options.inbound_forwarding_fee_proportional_millionths = 0;
+                       config.options.inbound_forwarding_fee_base_msat = 0;
+               }
+
                if !their_features.supports_wumbo() && channel_value_satoshis > MAX_FUNDING_SATOSHIS_NO_WUMBO {
                        return Err(APIError::APIMisuseError{err: format!("funding_value must not exceed {}, it was {}", MAX_FUNDING_SATOSHIS_NO_WUMBO, channel_value_satoshis)});
                }
@@ -962,12 +973,7 @@ impl<Signer: Sign> Channel<Signer> {
                Ok(Channel {
                        user_id,
 
-                       config: LegacyChannelConfig {
-                               options: user_config.channel_config.clone(),
-                               announced_channel: user_config.channel_handshake_config.announced_channel,
-                               commit_upfront_shutdown_pubkey: user_config.channel_handshake_config.commit_upfront_shutdown_pubkey,
-                       },
-
+                       config,
                        prev_config: None,
 
                        inbound_handshake_limits_override: Some(user_config.channel_handshake_limits.clone()),
@@ -1129,6 +1135,17 @@ impl<Signer: Sign> Channel<Signer> {
                let opt_anchors = false; // TODO - should be based on features
                let announced_channel = if (msg.channel_flags & 1) == 1 { true } else { false };
 
+               let mut config = LegacyChannelConfig {
+                       options: user_config.channel_config.clone(),
+                       announced_channel,
+                       commit_upfront_shutdown_pubkey: user_config.channel_handshake_config.commit_upfront_shutdown_pubkey,
+               };
+
+               if !their_features.supports_inbound_fees() {
+                       config.options.inbound_forwarding_fee_proportional_millionths = 0;
+                       config.options.inbound_forwarding_fee_base_msat = 0;
+               }
+
                // First check the channel type is known, failing before we do anything else if we don't
                // support this channel type.
                let channel_type = if let Some(channel_type) = &msg.channel_type {
@@ -1323,12 +1340,7 @@ impl<Signer: Sign> Channel<Signer> {
                let chan = Channel {
                        user_id,
 
-                       config: LegacyChannelConfig {
-                               options: user_config.channel_config.clone(),
-                               announced_channel,
-                               commit_upfront_shutdown_pubkey: user_config.channel_handshake_config.commit_upfront_shutdown_pubkey,
-                       },
-
+                       config,
                        prev_config: None,
 
                        inbound_handshake_limits_override: None,
@@ -4690,7 +4702,12 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Updates the channel's config. A bool is returned indicating whether the config update
        /// applied resulted in a new ChannelUpdate message.
-       pub fn update_config(&mut self, config: &ChannelConfig) -> bool {
+       pub fn update_config(&mut self, their_features: &InitFeatures, mut config: ChannelConfig) -> bool {
+               if !their_features.supports_inbound_fees() {
+                       config.inbound_forwarding_fee_proportional_millionths = 0;
+                       config.inbound_forwarding_fee_base_msat = 0;
+               }
+
                let did_channel_update =
                        self.config.options.forwarding_fee_proportional_millionths != config.forwarding_fee_proportional_millionths ||
                        self.config.options.forwarding_fee_base_msat != config.forwarding_fee_base_msat ||
@@ -4701,7 +4718,7 @@ impl<Signer: Sign> Channel<Signer> {
                        // policy change to propagate throughout the network.
                        self.update_time_counter += 1;
                }
-               self.config.options = *config;
+               self.config.options = config;
                did_channel_update
        }
 

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 03a150e4fe208493dea2480abeba968b5b79a877..1d1d3283f98f2902a2816018ac768f3912947e65 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -2649,7 +2649,7 @@ where
        /// [`ChannelUnavailable`]: APIError::ChannelUnavailable
        /// [`APIMisuseError`]: APIError::APIMisuseError
        pub fn update_channel_config(
-               &self, counterparty_node_id: &PublicKey, channel_ids: &[[u8; 32]], config: &ChannelConfig,
+               &self, counterparty_node_id: &PublicKey, channel_ids: &[[u8; 32]], config: ChannelConfig,
        ) -> Result<(), APIError> {
                if config.cltv_expiry_delta < MIN_CLTV_EXPIRY_DELTA {
                        return Err(APIError::APIMisuseError {
@@ -2677,9 +2677,14 @@ where
                        }
                        for channel_id in channel_ids {
                                let channel = channel_state.by_id.get_mut(channel_id).unwrap();
-                               if !channel.update_config(config) {
-                                       continue;
-                               }
+
+                               //XXX : do this in the same per_peer_state lock post-#1507
+                               let per_peer_state = self.per_peer_state.read().unwrap();
+                               if let Some(peer_state) = per_peer_state.get(counterparty_node_id) {
+                                       if !channel.update_config(&peer_state.lock().unwrap().latest_features, config) {
+                                               continue;
+                                       }
+                               } else { return Err(APIError::APIMisuseError { err: format!("XXX: DROP THIS CODE") }); }
                                if let Ok(msg) = self.get_channel_update_for_broadcast(channel) {
                                        channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { msg });
                                } else if let Ok(msg) = self.get_channel_update_for_unicast(channel) {

diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs
index 4f90202fd2f5cac45ed91e395c2cefd0a8373ed5..b7a9a6956d32282954f2bf5684bec90eb9c06ecf 100644 (file)
--- a/lightning/src/ln/onion_route_tests.rs
+++ b/lightning/src/ln/onion_route_tests.rs
@@ -671,7 +671,7 @@ fn do_test_onion_failure_stale_channel_update(announced_channel: bool) {
        };
 
        // Closure to update and retrieve the latest ChannelUpdate.
-       let update_and_get_channel_update = |config: &ChannelConfig, expect_new_update: bool,
+       let update_and_get_channel_update = |config: ChannelConfig, expect_new_update: bool,
                prev_update: Option<&msgs::ChannelUpdate>, should_expire_prev_config: bool| -> Option<msgs::ChannelUpdate> {
                nodes[1].node.update_channel_config(
                        channel_to_update_counterparty, &[channel_to_update.0], config,
@@ -718,7 +718,7 @@ fn do_test_onion_failure_stale_channel_update(announced_channel: bool) {
        let mut invalid_config = default_config.clone();
        invalid_config.cltv_expiry_delta = 0;
        match nodes[1].node.update_channel_config(
-               channel_to_update_counterparty, &[channel_to_update.0], &invalid_config,
+               channel_to_update_counterparty, &[channel_to_update.0], invalid_config,
        ) {
                Err(APIError::APIMisuseError{ .. }) => {},
                _ => panic!("unexpected result applying invalid cltv_expiry_delta"),
@@ -729,7 +729,7 @@ fn do_test_onion_failure_stale_channel_update(announced_channel: bool) {
                .find(|channel| channel.channel_id == channel_to_update.0).unwrap()
                .config.unwrap();
        config.forwarding_fee_base_msat = u32::max_value();
-       let msg = update_and_get_channel_update(&config, true, None, false).unwrap();
+       let msg = update_and_get_channel_update(config, true, None, false).unwrap();
 
        // The old policy should still be in effect until a new block is connected.
        send_along_route_with_secret(&nodes[0], route.clone(), &[&[&nodes[1], &nodes[2]]], PAYMENT_AMT,
@@ -742,24 +742,24 @@ fn do_test_onion_failure_stale_channel_update(announced_channel: bool) {
        expect_onion_failure("fee_insufficient", UPDATE|12, &msg);
 
        // Redundant updates should not trigger a new ChannelUpdate.
-       assert!(update_and_get_channel_update(&config, false, None, false).is_none());
+       assert!(update_and_get_channel_update(config, false, None, false).is_none());
 
        // Similarly, updates that do not have an affect on ChannelUpdate should not trigger a new one.
        config.force_close_avoidance_max_fee_satoshis *= 2;
-       assert!(update_and_get_channel_update(&config, false, None, false).is_none());
+       assert!(update_and_get_channel_update(config, false, None, false).is_none());
 
        // Reset the base fee to the default and increase the proportional fee which should trigger a
        // new ChannelUpdate.
        config.forwarding_fee_base_msat = default_config.forwarding_fee_base_msat;
        config.cltv_expiry_delta = u16::max_value();
-       let msg = update_and_get_channel_update(&config, true, Some(&msg), true).unwrap();
+       let msg = update_and_get_channel_update(config, true, Some(&msg), true).unwrap();
        expect_onion_failure("incorrect_cltv_expiry", UPDATE|13, &msg);
 
        // Reset the proportional fee and increase the CLTV expiry delta which should trigger a new
        // ChannelUpdate.
        config.cltv_expiry_delta = default_config.cltv_expiry_delta;
        config.forwarding_fee_proportional_millionths = u32::max_value();
-       let msg = update_and_get_channel_update(&config, true, Some(&msg), true).unwrap();
+       let msg = update_and_get_channel_update(config, true, Some(&msg), true).unwrap();
        expect_onion_failure("fee_insufficient", UPDATE|12, &msg);
 
        // To test persistence of the updated config, we'll re-initialize the ChannelManager.

diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 6c3f98df406d99ec331053d0289c91f0dbd24952..d5339fbb824429c891c647006f5ceed0809bfaf3 100644 (file)
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -504,7 +504,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
                let mut channel = channel_state.by_id.get_mut(&chan_id_2).unwrap();
                let mut new_config = channel.config();
                new_config.forwarding_fee_base_msat += 100_000;
-               channel.update_config(&new_config);
+               channel.update_config(&channelmanager::provided_init_features(), new_config);
                new_route.paths[0][0].fee_msat += 100_000;
        }