ln: do not send and reject receive of update_fee for zero fee commits

Co-authored-by: Matt Corallo <git@bluematt.me>

Author: carlaKC

lightning/src/chain/package.rs

@@ -99,6 +99,9 @@ pub(crate) fn verify_channel_type_features(channel_type_features: &Option<Channe
 		supported_feature_set.set_scid_privacy_required();
 		supported_feature_set.set_zero_conf_required();
 
+		#[cfg(test)]
+		supported_feature_set.set_anchor_zero_fee_commitments_required();
+
 		// allow the passing of an additional necessary permitted flag
 		if let Some(additional_permitted_features) = additional_permitted_features {
 			supported_feature_set |= additional_permitted_features;

lightning/src/ln/channel.rs

@@ -7709,6 +7709,10 @@ where
 			panic!("Cannot update fee while peer is disconnected/we're awaiting a monitor update (ChannelManager should have caught this)");
 		}
 
+		// Sending a fee update for zero fee commitments will trigger a warning and disconnect
+		// from our peer, but does not result in a loss of funds so we do not panic here.
+		debug_assert!(!self.funding.get_channel_type().supports_anchor_zero_fee_commitments());
+
 		let can_send_update_fee = core::iter::once(&self.funding)
 			.chain(self.pending_funding.iter())
 			.all(|funding| self.context.can_send_update_fee(funding, feerate_per_kw, fee_estimator, logger));
@@ -8004,6 +8008,9 @@ where
 		if self.context.channel_state.is_remote_stfu_sent() || self.context.channel_state.is_quiescent() {
 			return Err(ChannelError::WarnAndDisconnect("Got fee update message while quiescent".to_owned()));
 		}
+		if self.funding.get_channel_type().supports_anchor_zero_fee_commitments() {
+			return Err(ChannelError::WarnAndDisconnect("Update fee message received for zero fee commitment channel".to_owned()));
+		}
 
 		core::iter::once(&self.funding)
 			.chain(self.pending_funding.iter())

lightning/src/ln/channelmanager.rs

@@ -7191,10 +7191,17 @@ where
 
 		let logger = WithChannelContext::from(&self.logger, &chan.context, None);
 
-		// If the feerate has decreased by less than half, don't bother
-		if new_feerate <= chan.context.get_feerate_sat_per_1000_weight() && new_feerate * 2 > chan.context.get_feerate_sat_per_1000_weight() {
-			return NotifyOption::SkipPersistNoEvents;
+		let current_feerate = chan.context.get_feerate_sat_per_1000_weight();
+		let update_fee_required = match new_feerate.cmp(&current_feerate) {
+			cmp::Ordering::Greater => true,
+			cmp::Ordering::Equal => false,
+			// Only bother with a fee update if feerate has decreased at least half.
+			cmp::Ordering::Less => new_feerate * 2 <= current_feerate,
+		};
+		if !update_fee_required {
+			return NotifyOption::SkipPersistNoEvents
 		}
+
 		if !chan.context.is_live() {
 			log_trace!(logger, "Channel {} does not qualify for a feerate change from {} to {} as it cannot currently be updated (probably the peer is disconnected).",
 				chan_id, chan.context.get_feerate_sat_per_1000_weight(), new_feerate);

lightning/src/ln/update_fee_tests.rs

@@ -1375,3 +1375,59 @@ pub fn do_can_afford_given_trimmed_htlcs(inequality_regions: core::cmp::Ordering
 		nodes[0].logger.assert_log("lightning::ln::channel", err, 1);
 	}
 }
+
+#[test]
+pub fn test_zero_fee_commitments_no_update_fee() {
+	// Tests that option_zero_fee_commitment channels do not sent update_fee messages, and that
+	// they'll disconnect and warn if they receive them.
+	let mut cfg = test_default_channel_config();
+	cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true;
+	cfg.manually_accept_inbound_channels = true;
+
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(cfg.clone()), Some(cfg)]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let channel = create_chan_between_nodes(&nodes[0], &nodes[1]);
+
+	let assert_zero_fee = || {
+		for node in nodes.iter() {
+			let channels = node.node.list_channels();
+			assert_eq!(channels.len(), 1);
+			assert!(channels[0]
+				.channel_type
+				.as_ref()
+				.unwrap()
+				.supports_anchor_zero_fee_commitments());
+			assert_eq!(channels[0].feerate_sat_per_1000_weight.unwrap(), 0);
+		}
+	};
+	assert_zero_fee();
+
+	// Sender should not queue an update_fee message.
+	nodes[0].node.timer_tick_occurred();
+	let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(events_0.len(), 0);
+
+	// Receiver should ignore and warn if sent update_fee.
+	let channel_id = channel.3;
+	let update_fee_msg = msgs::UpdateFee { channel_id, feerate_per_kw: 5000 };
+	nodes[1].node.handle_update_fee(nodes[0].node.get_our_node_id(), &update_fee_msg);
+
+	let events_1 = nodes[1].node.get_and_clear_pending_msg_events();
+	assert_eq!(events_1.len(), 1);
+	match events_1[0] {
+		MessageSendEvent::HandleError { ref action, .. } => match action {
+			ErrorAction::DisconnectPeerWithWarning { ref msg, .. } => {
+				assert_eq!(msg.channel_id, channel_id);
+				assert!(msg
+					.data
+					.contains("Update fee message received for zero fee commitment channel"));
+			},
+			_ => panic!("Expected DisconnectPeerWithWarning, got {:?}", action),
+		},
+		_ => panic!("Expected HandleError event, got {:?}", events_1[0]),
+	}
+	assert_zero_fee();
+}