Merge pull request #2354 from alecchendev/2023-06-bump-default-dust-exp

Bump dust exposure threshold

Author: TheBlueMatt

fuzz/src/full_stack.rs

@@ -1522,10 +1522,13 @@ impl ChannelDetails {
 		self.short_channel_id.or(self.outbound_scid_alias)
 	}
 
-	fn from_channel_context<Signer: WriteableEcdsaChannelSigner>(context: &ChannelContext<Signer>,
-		best_block_height: u32, latest_features: InitFeatures) -> Self {
-
-		let balance = context.get_available_balances();
+	fn from_channel_context<Signer: WriteableEcdsaChannelSigner, F: Deref>(
+		context: &ChannelContext<Signer>, best_block_height: u32, latest_features: InitFeatures,
+		fee_estimator: &LowerBoundedFeeEstimator<F>
+	) -> Self
+	where F::Target: FeeEstimator
+	{
+		let balance = context.get_available_balances(fee_estimator);
 		let (to_remote_reserve_satoshis, to_self_reserve_satoshis) =
 			context.get_holder_counterparty_selected_channel_reserve_satoshis();
 		ChannelDetails {
@@ -2240,7 +2243,7 @@ where
 				let peer_state = &mut *peer_state_lock;
 				for (_channel_id, channel) in peer_state.channel_by_id.iter().filter(f) {
 					let details = ChannelDetails::from_channel_context(&channel.context, best_block_height,
-						peer_state.latest_features.clone());
+						peer_state.latest_features.clone(), &self.fee_estimator);
 					res.push(details);
 				}
 			}
@@ -2266,17 +2269,17 @@ where
 				let peer_state = &mut *peer_state_lock;
 				for (_channel_id, channel) in peer_state.channel_by_id.iter() {
 					let details = ChannelDetails::from_channel_context(&channel.context, best_block_height,
-						peer_state.latest_features.clone());
+						peer_state.latest_features.clone(), &self.fee_estimator);
 					res.push(details);
 				}
 				for (_channel_id, channel) in peer_state.inbound_v1_channel_by_id.iter() {
 					let details = ChannelDetails::from_channel_context(&channel.context, best_block_height,
-						peer_state.latest_features.clone());
+						peer_state.latest_features.clone(), &self.fee_estimator);
 					res.push(details);
 				}
 				for (_channel_id, channel) in peer_state.outbound_v1_channel_by_id.iter() {
 					let details = ChannelDetails::from_channel_context(&channel.context, best_block_height,
-						peer_state.latest_features.clone());
+						peer_state.latest_features.clone(), &self.fee_estimator);
 					res.push(details);
 				}
 			}
@@ -2309,7 +2312,8 @@ where
 			return peer_state.channel_by_id
 				.iter()
 				.map(|(_, channel)|
-					ChannelDetails::from_channel_context(&channel.context, best_block_height, features.clone()))
+					ChannelDetails::from_channel_context(&channel.context, best_block_height,
+					features.clone(), &self.fee_estimator))
 				.collect();
 		}
 		vec![]
@@ -3113,7 +3117,7 @@ where
 						session_priv: session_priv.clone(),
 						first_hop_htlc_msat: htlc_msat,
 						payment_id,
-					}, onion_packet, None, &self.logger);
+					}, onion_packet, None, &self.fee_estimator, &self.logger);
 				match break_chan_entry!(self, send_res, chan) {
 					Some(monitor_update) => {
 						match handle_new_monitor_update!(self, funding_txo, monitor_update, peer_state_lock, peer_state, per_peer_state, chan) {
@@ -3830,7 +3834,8 @@ where
 										});
 										if let Err(e) = chan.get_mut().queue_add_htlc(outgoing_amt_msat,
 											payment_hash, outgoing_cltv_value, htlc_source.clone(),
-											onion_packet, skimmed_fee_msat, &self.logger)
+											onion_packet, skimmed_fee_msat, &self.fee_estimator,
+											&self.logger)
 										{
 											if let ChannelError::Ignore(msg) = e {
 												log_trace!(self.logger, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(payment_hash.0), msg);
@@ -4219,7 +4224,7 @@ where
 		log_trace!(self.logger, "Channel {} qualifies for a feerate change from {} to {}.",
 			log_bytes!(chan_id[..]), chan.context.get_feerate_sat_per_1000_weight(), new_feerate);
 
-		chan.queue_update_fee(new_feerate, &self.logger);
+		chan.queue_update_fee(new_feerate, &self.fee_estimator, &self.logger);
 		NotifyOption::DoPersist
 	}
 
@@ -5555,7 +5560,7 @@ where
 						_ => pending_forward_info
 					}
 				};
-				try_chan_entry!(self, chan.get_mut().update_add_htlc(&msg, pending_forward_info, create_pending_htlc_status, &self.logger), chan);
+				try_chan_entry!(self, chan.get_mut().update_add_htlc(&msg, pending_forward_info, create_pending_htlc_status, &self.fee_estimator, &self.logger), chan);
 			},
 			hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id))
 		}
@@ -5772,7 +5777,7 @@ where
 			match peer_state.channel_by_id.entry(msg.channel_id) {
 				hash_map::Entry::Occupied(mut chan) => {
 					let funding_txo = chan.get().context.get_funding_txo();
-					let (htlcs_to_fail, monitor_update_opt) = try_chan_entry!(self, chan.get_mut().revoke_and_ack(&msg, &self.logger), chan);
+					let (htlcs_to_fail, monitor_update_opt) = try_chan_entry!(self, chan.get_mut().revoke_and_ack(&msg, &self.fee_estimator, &self.logger), chan);
 					let res = if let Some(monitor_update) = monitor_update_opt {
 						handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update,
 							peer_state_lock, peer_state, per_peer_state, chan).map(|_| ())
@@ -6043,7 +6048,7 @@ where
 						let counterparty_node_id = chan.context.get_counterparty_node_id();
 						let funding_txo = chan.context.get_funding_txo();
 						let (monitor_opt, holding_cell_failed_htlcs) =
-							chan.maybe_free_holding_cell_htlcs(&self.logger);
+							chan.maybe_free_holding_cell_htlcs(&self.fee_estimator, &self.logger);
 						if !holding_cell_failed_htlcs.is_empty() {
 							failed_htlcs.push((holding_cell_failed_htlcs, *channel_id, counterparty_node_id));
 						}
@@ -10060,7 +10065,7 @@ pub mod bench {
 	use crate::routing::gossip::NetworkGraph;
 	use crate::routing::router::{PaymentParameters, RouteParameters};
 	use crate::util::test_utils;
-	use crate::util::config::UserConfig;
+	use crate::util::config::{UserConfig, MaxDustHTLCExposure};
 
 	use bitcoin::hashes::Hash;
 	use bitcoin::hashes::sha256::Hash as Sha256;
@@ -10107,6 +10112,7 @@ pub mod bench {
 		let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(network, &logger_a)), &scorer);
 
 		let mut config: UserConfig = Default::default();
+		config.channel_config.max_dust_htlc_exposure = MaxDustHTLCExposure::FeeRateMultiplier(5_000_000 / 253);
 		config.channel_handshake_config.minimum_depth = 1;
 
 		let chain_monitor_a = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_a);

lightning/src/ln/channel.rs

@@ -27,7 +27,7 @@ use crate::util::scid_utils;
 use crate::util::test_utils;
 use crate::util::test_utils::{panicking, TestChainMonitor, TestScorer, TestKeysInterface};
 use crate::util::errors::APIError;
-use crate::util::config::UserConfig;
+use crate::util::config::{UserConfig, MaxDustHTLCExposure};
 use crate::util::ser::{ReadableArgs, Writeable};
 
 use bitcoin::blockdata::block::{Block, BlockHeader};
@@ -2582,8 +2582,10 @@ pub fn test_default_channel_config() -> UserConfig {
 	// It now defaults to 1, so we simply set it to the expected value here.
 	default_config.channel_handshake_config.our_htlc_minimum_msat = 1000;
 	// When most of our tests were written, we didn't have the notion of a `max_dust_htlc_exposure_msat`,
-	// It now defaults to 5_000_000 msat; to avoid interfering with tests we bump it to 50_000_000 msat.
-	default_config.channel_config.max_dust_htlc_exposure_msat = 50_000_000;
+	// to avoid interfering with tests we bump it to 50_000_000 msat (assuming the default test
+	// feerate of 253).
+	default_config.channel_config.max_dust_htlc_exposure =
+		MaxDustHTLCExposure::FeeRateMultiplier(50_000_000 / 253);
 	default_config
 }
 

lightning/src/ln/channelmanager.rs

@@ -35,7 +35,7 @@ use crate::util::test_utils;
 use crate::util::errors::APIError;
 use crate::util::ser::{Writeable, ReadableArgs};
 use crate::util::string::UntrustedString;
-use crate::util::config::UserConfig;
+use crate::util::config::{UserConfig, MaxDustHTLCExposure};
 
 use bitcoin::hash_types::BlockHash;
 use bitcoin::blockdata::script::{Builder, Script};
@@ -9515,7 +9515,7 @@ enum ExposureEvent {
 	AtUpdateFeeOutbound,
 }
 
-fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_event: ExposureEvent, on_holder_tx: bool) {
+fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_event: ExposureEvent, on_holder_tx: bool, multiplier_dust_limit: bool) {
 	// Test that we properly reject dust HTLC violating our `max_dust_htlc_exposure_msat`
 	// policy.
 	//
@@ -9530,7 +9530,12 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 
 	let chanmon_cfgs = create_chanmon_cfgs(2);
 	let mut config = test_default_channel_config();
-	config.channel_config.max_dust_htlc_exposure_msat = 5_000_000; // default setting value
+	config.channel_config.max_dust_htlc_exposure = if multiplier_dust_limit {
+		// Default test fee estimator rate is 253 sat/kw, so we set the multiplier to 5_000_000 / 253
+		// to get roughly the same initial value as the default setting when this test was
+		// originally written.
+		MaxDustHTLCExposure::FeeRateMultiplier(5_000_000 / 253)
+	} else { MaxDustHTLCExposure::FixedLimitMsat(5_000_000) }; // initial default setting value
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), None]);
 	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
@@ -9574,20 +9579,21 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 	let (mut route, payment_hash, _, payment_secret) =
 		get_route_and_payment_hash!(nodes[0], nodes[1], 1000);
 
-	let dust_buffer_feerate = {
+	let (dust_buffer_feerate, max_dust_htlc_exposure_msat) = {
 		let per_peer_state = nodes[0].node.per_peer_state.read().unwrap();
 		let chan_lock = per_peer_state.get(&nodes[1].node.get_our_node_id()).unwrap().lock().unwrap();
 		let chan = chan_lock.channel_by_id.get(&channel_id).unwrap();
-		chan.context.get_dust_buffer_feerate(None) as u64
+		(chan.context.get_dust_buffer_feerate(None) as u64,
+		chan.context.get_max_dust_htlc_exposure_msat(&LowerBoundedFeeEstimator(nodes[0].fee_estimator)))
 	};
 	let dust_outbound_htlc_on_holder_tx_msat: u64 = (dust_buffer_feerate * htlc_timeout_tx_weight(&channel_type_features) / 1000 + open_channel.dust_limit_satoshis - 1) * 1000;
-	let dust_outbound_htlc_on_holder_tx: u64 = config.channel_config.max_dust_htlc_exposure_msat / dust_outbound_htlc_on_holder_tx_msat;
+	let dust_outbound_htlc_on_holder_tx: u64 = max_dust_htlc_exposure_msat / dust_outbound_htlc_on_holder_tx_msat;
 
 	let dust_inbound_htlc_on_holder_tx_msat: u64 = (dust_buffer_feerate * htlc_success_tx_weight(&channel_type_features) / 1000 + open_channel.dust_limit_satoshis - 1) * 1000;
-	let dust_inbound_htlc_on_holder_tx: u64 = config.channel_config.max_dust_htlc_exposure_msat / dust_inbound_htlc_on_holder_tx_msat;
+	let dust_inbound_htlc_on_holder_tx: u64 = max_dust_htlc_exposure_msat / dust_inbound_htlc_on_holder_tx_msat;
 
 	let dust_htlc_on_counterparty_tx: u64 = 4;
-	let dust_htlc_on_counterparty_tx_msat: u64 = config.channel_config.max_dust_htlc_exposure_msat / dust_htlc_on_counterparty_tx;
+	let dust_htlc_on_counterparty_tx_msat: u64 = max_dust_htlc_exposure_msat / dust_htlc_on_counterparty_tx;
 
 	if on_holder_tx {
 		if dust_outbound_balance {
@@ -9639,7 +9645,7 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 				), true, APIError::ChannelUnavailable { .. }, {});
 		}
 	} else if exposure_breach_event == ExposureEvent::AtHTLCReception {
-		let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], if on_holder_tx { dust_inbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat + 1 });
+		let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], if on_holder_tx { dust_inbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat + 4 });
 		nodes[1].node.send_payment_with_route(&route, payment_hash,
 			RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
 		check_added_monitors!(nodes[1], 1);
@@ -9652,18 +9658,24 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 			// Outbound dust balance: 6399 sats
 			let dust_inbound_overflow = dust_inbound_htlc_on_holder_tx_msat * (dust_inbound_htlc_on_holder_tx + 1);
 			let dust_outbound_overflow = dust_outbound_htlc_on_holder_tx_msat * dust_outbound_htlc_on_holder_tx + dust_inbound_htlc_on_holder_tx_msat;
-			nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", if dust_outbound_balance { dust_outbound_overflow } else { dust_inbound_overflow }, config.channel_config.max_dust_htlc_exposure_msat), 1);
+			nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", if dust_outbound_balance { dust_outbound_overflow } else { dust_inbound_overflow }, max_dust_htlc_exposure_msat), 1);
 		} else {
 			// Outbound dust balance: 5200 sats
 			nodes[0].logger.assert_log("lightning::ln::channel".to_string(),
 				format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx",
-					dust_htlc_on_counterparty_tx_msat * (dust_htlc_on_counterparty_tx - 1) + dust_htlc_on_counterparty_tx_msat + 1,
-					config.channel_config.max_dust_htlc_exposure_msat), 1);
+					dust_htlc_on_counterparty_tx_msat * (dust_htlc_on_counterparty_tx - 1) + dust_htlc_on_counterparty_tx_msat + 4,
+					max_dust_htlc_exposure_msat), 1);
 		}
 	} else if exposure_breach_event == ExposureEvent::AtUpdateFeeOutbound {
 		route.paths[0].hops.last_mut().unwrap().fee_msat = 2_500_000;
-		nodes[0].node.send_payment_with_route(&route, payment_hash,
-			RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
+		// For the multiplier dust exposure limit, since it scales with feerate,
+		// we need to add a lot of HTLCs that will become dust at the new feerate
+		// to cross the threshold.
+		for _ in 0..20 {
+			let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[1], Some(1_000), None);
+			nodes[0].node.send_payment_with_route(&route, payment_hash,
+				RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
+		}
 		{
 			let mut feerate_lock = chanmon_cfgs[0].fee_estimator.sat_per_kw.lock().unwrap();
 			*feerate_lock = *feerate_lock * 10;
@@ -9678,20 +9690,25 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 	added_monitors.clear();
 }
 
+fn do_test_max_dust_htlc_exposure_by_threshold_type(multiplier_dust_limit: bool) {
+	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCForward, true, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCForward, true, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCReception, true, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCReception, false, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCForward, false, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCReception, false, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCReception, true, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCForward, false, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtUpdateFeeOutbound, true, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtUpdateFeeOutbound, false, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtUpdateFeeOutbound, false, multiplier_dust_limit);
+	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtUpdateFeeOutbound, true, multiplier_dust_limit);
+}
+
 #[test]
 fn test_max_dust_htlc_exposure() {
-	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCForward, true);
-	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCForward, true);
-	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCReception, true);
-	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCReception, false);
-	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCForward, false);
-	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCReception, false);
-	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCReception, true);
-	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCForward, false);
-	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtUpdateFeeOutbound, true);
-	do_test_max_dust_htlc_exposure(true, ExposureEvent::AtUpdateFeeOutbound, false);
-	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtUpdateFeeOutbound, false);
-	do_test_max_dust_htlc_exposure(false, ExposureEvent::AtUpdateFeeOutbound, true);
+	do_test_max_dust_htlc_exposure_by_threshold_type(false);
+	do_test_max_dust_htlc_exposure_by_threshold_type(true);
 }
 
 #[test]

lightning/src/ln/functional_test_utils.rs

@@ -26,7 +26,7 @@ use crate::ln::msgs::{ChannelMessageHandler, ChannelUpdate};
 use crate::ln::wire::Encode;
 use crate::util::ser::{Writeable, Writer};
 use crate::util::test_utils;
-use crate::util::config::{UserConfig, ChannelConfig};
+use crate::util::config::{UserConfig, ChannelConfig, MaxDustHTLCExposure};
 use crate::util::errors::APIError;
 
 use bitcoin::hash_types::BlockHash;
@@ -671,6 +671,7 @@ fn do_test_onion_failure_stale_channel_update(announced_channel: bool) {
 	config.channel_handshake_config.announced_channel = announced_channel;
 	config.channel_handshake_limits.force_announced_channel_preference = false;
 	config.accept_forwards_to_priv_channels = !announced_channel;
+	config.channel_config.max_dust_htlc_exposure = MaxDustHTLCExposure::FeeRateMultiplier(5_000_000 / 253);
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let persister;
 	let chain_monitor;
@@ -1371,10 +1372,19 @@ fn test_phantom_failure_too_low_recv_amt() {
 
 #[test]
 fn test_phantom_dust_exposure_failure() {
+	do_test_phantom_dust_exposure_failure(false);
+	do_test_phantom_dust_exposure_failure(true);
+}
+
+fn do_test_phantom_dust_exposure_failure(multiplier_dust_limit: bool) {
 	// Set the max dust exposure to the dust limit.
 	let max_dust_exposure = 546;
 	let mut receiver_config = UserConfig::default();
-	receiver_config.channel_config.max_dust_htlc_exposure_msat = max_dust_exposure;
+	// Default test fee estimator rate is 253, so to set the max dust exposure to the dust limit,
+	// we need to set the multiplier to 2.
+	receiver_config.channel_config.max_dust_htlc_exposure =
+		if multiplier_dust_limit { MaxDustHTLCExposure::FeeRateMultiplier(2) }
+		else { MaxDustHTLCExposure::FixedLimitMsat(max_dust_exposure) };
 	receiver_config.channel_handshake_config.announced_channel = true;
 
 	let chanmon_cfgs = create_chanmon_cfgs(2);