Add max dust exposure multiplier config knob

alecchendev · alecchendev · commit cdaffb76e58f · 2023-07-06T18:12:20.000-05:00
With fee rates rising dramatically in mid-April 2023, thresholds for
what is considered dust have risen, often exceeding our previous dust
exposure threshold of 5k sats. This causes all payments and HTLC
forwards between 5k sats and new dust thresholds to fail.

This commit changes our max dust exposure config knob from a fixed
upper limit to a `MaxDustHTLCExposure` enum with an additional variant
to allow setting our max dust exposure to a multiplier on the current
high priority feerate.

To remain backwards compatible we'll always write the fixed limit if
it's set, or its default value in its currently reserved TLV.

We also now write an odd TLV for the new enum, so that previous
versions can safely ignore it upon downgrading, while allowing us to
make use of the new type when it's written.
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -41,7 +41,7 @@ use crate::routing::gossip::NodeId;
 use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
 use crate::util::logger::Logger;
 use crate::util::errors::APIError;
-use crate::util::config::{UserConfig, ChannelConfig, LegacyChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits};
+use crate::util::config::{UserConfig, ChannelConfig, LegacyChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits, MaxDustHTLCExposure};
 use crate::util::scid_utils::scid_from_parts;
 
 use crate::io;
@@ -1060,7 +1060,10 @@ impl<Signer: ChannelSigner> ChannelContext<Signer> {
 	}
 
 	pub fn get_max_dust_htlc_exposure_msat(&self) -> u64 {
-		self.config.options.max_dust_htlc_exposure_msat
+		match self.config.options.max_dust_htlc_exposure {
+			MaxDustHTLCExposure::FixedLimitMsat(limit) => limit,
+			MaxDustHTLCExposure::FeeRateMultiplier(_) => 5_000_000,
+		}
 	}
 
 	/// Returns the previous [`ChannelConfig`] applied to this channel, if any.
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.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};
@@ -2573,7 +2573,7 @@ pub fn test_default_channel_config() -> UserConfig {
 	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;
+	default_config.channel_config.max_dust_htlc_exposure = MaxDustHTLCExposure::FixedLimitMsat(50_000_000);
 	default_config
 }
 
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.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};
@@ -9530,7 +9530,7 @@ 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 = MaxDustHTLCExposure::FixedLimitMsat(5_000_000); // 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 +9574,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())
 	};
 	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 {
@@ -9652,13 +9653,13 @@ 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);
+					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;
diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.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;
@@ -1374,7 +1374,8 @@ fn test_phantom_dust_exposure_failure() {
 	// 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;
+	receiver_config.channel_config.max_dust_htlc_exposure =
+		MaxDustHTLCExposure::FixedLimitMsat(max_dust_exposure);
 	receiver_config.channel_handshake_config.announced_channel = true;
 
 	let chanmon_cfgs = create_chanmon_cfgs(2);
diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs
@@ -315,6 +315,51 @@ impl Default for ChannelHandshakeLimits {
 	}
 }
 
+/// Options for how to set the max dust HTLC exposure allowed on a channel. See
+/// [`ChannelConfig::max_dust_htlc_exposure_msat`] for details.
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum MaxDustHTLCExposure {
+	/// This sets a fixed limit on the total dust exposure in millisatoshis. Setting this too low
+	/// may prevent the sending or receipt of low-value HTLCs on high-traffic nodes, however this
+	/// limit is very important to prevent stealing of large amounts of dust HTLCs by miners
+	/// through [fee griefing
+	/// attacks](https://lists.linuxfoundation.org/pipermail/lightning-dev/2020-May/002714.html).
+	///
+	/// Note that if the feerate increases significantly, without a manual increase
+	/// to this maximum the channel may be unable to send/receive HTLCs between the maximum dust
+	/// exposure and the new minimum value for HTLCs to be economically viable to claim.
+	FixedLimitMsat(u64),
+	/// This sets a multiplier on the estimated high priority feerate (sats/KW, as obtained from
+	/// [`FeeEstimator`]) to determine the maximum allowed dust exposure. If this variant is used
+	/// then the maximum dust exposure in millisatoshis is calculated as:
+	/// `high_priority_feerate_per_kw * value`. For example, with our default value
+	/// `FeeRateMultiplier(5000)`:
+	///
+	/// - For the minimum fee rate of 1 sat/vByte (250 sat/KW, although the minimum
+	/// defaults to 253 sats/KW for rounding, see [`FeeEstimator`]), the max dust exposure would
+	/// be 253 * 5000 = 1,265,000 msats.
+	/// - For a fee rate of 30 sat/vByte (7500 sat/KW), the max dust exposure would be
+	/// 7500 * 5000 = 37,500,000 msats.
+	///
+	/// This allows the maximum dust exposure to automatically scale with fee rate changes.
+	///
+	/// Note, if you're using a third-party fee estimator, this may leave you more exposed to a
+	/// fee griefing attack, where your fee estimator may purposely overestimate the fee rate,
+	/// causing you to accept more dust HTLCs than you would otherwise.
+	///
+	/// # Backwards Compatibility
+	/// This variant only became available in LDK 0.0.116, so if you downgrade to a prior version
+	/// by default this will be set to a [`Self::FixedLimitMsat`] of 5,000,000 msat.
+	///
+	/// [`FeeEstimator`]: crate::chain::chaininterface::FeeEstimator
+	FeeRateMultiplier(u64),
+}
+
+impl_writeable_tlv_based_enum!(MaxDustHTLCExposure, ;
+	(1, FixedLimitMsat),
+	(3, FeeRateMultiplier),
+);
+
 /// Options which apply on a per-channel basis and may change at runtime or based on negotiation
 /// with our counterparty.
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
@@ -374,13 +419,11 @@ pub struct ChannelConfig {
 	/// (specifically the zero fee HTLC transaction variant), this threshold no longer takes into
 	/// account the HTLC transaction fee as it is zero.
 	///
-	/// This limit is applied for sent, forwarded, and received HTLCs and limits the total
-	/// exposure across all three types per-channel. Setting this too low may prevent the
-	/// sending or receipt of low-value HTLCs on high-traffic nodes, and this limit is very
-	/// important to prevent stealing of dust HTLCs by miners.
+	/// The selected limit is applied for sent, forwarded, and received HTLCs and limits the total
+	/// exposure across all three types per-channel.
 	///
-	/// Default value: 5_000_000 msat.
-	pub max_dust_htlc_exposure_msat: u64,
+	/// Default value: [`MaxDustHTLCExposure::FeeRateMultiplier`] with a multiplier of 5000.
+	pub max_dust_htlc_exposure: MaxDustHTLCExposure,
 	/// The additional fee we're willing to pay to avoid waiting for the counterparty's
 	/// `to_self_delay` to reclaim funds.
 	///
@@ -451,7 +494,7 @@ impl ChannelConfig {
 			self.cltv_expiry_delta = cltv_expiry_delta;
 		}
 		if let Some(max_dust_htlc_exposure_msat) = update.max_dust_htlc_exposure_msat {
-			self.max_dust_htlc_exposure_msat = max_dust_htlc_exposure_msat;
+			self.max_dust_htlc_exposure = max_dust_htlc_exposure_msat;
 		}
 		if let Some(force_close_avoidance_max_fee_satoshis) = update.force_close_avoidance_max_fee_satoshis {
 			self.force_close_avoidance_max_fee_satoshis = force_close_avoidance_max_fee_satoshis;
@@ -466,32 +509,75 @@ impl Default for ChannelConfig {
 			forwarding_fee_proportional_millionths: 0,
 			forwarding_fee_base_msat: 1000,
 			cltv_expiry_delta: 6 * 12, // 6 blocks/hour * 12 hours
-			max_dust_htlc_exposure_msat: 5_000_000,
+			max_dust_htlc_exposure: MaxDustHTLCExposure::FeeRateMultiplier(5000),
 			force_close_avoidance_max_fee_satoshis: 1000,
 			accept_underpaying_htlcs: false,
 		}
 	}
 }
 
-impl_writeable_tlv_based!(ChannelConfig, {
-	(0, forwarding_fee_proportional_millionths, required),
-	(1, accept_underpaying_htlcs, (default_value, false)),
-	(2, forwarding_fee_base_msat, required),
-	(4, cltv_expiry_delta, required),
-	(6, max_dust_htlc_exposure_msat, required),
-	// ChannelConfig serialized this field with a required type of 8 prior to the introduction of
-	// LegacyChannelConfig. To make sure that serialization is not compatible with this one, we use
-	// the next required type of 10, which if seen by the old serialization will always fail.
-	(10, force_close_avoidance_max_fee_satoshis, required),
-});
+impl crate::util::ser::Writeable for ChannelConfig {
+	fn write<W: crate::util::ser::Writer>(&self, writer: &mut W) -> Result<(), crate::io::Error> {
+		let max_dust_htlc_exposure_msat_fixed_limit = match self.max_dust_htlc_exposure {
+			MaxDustHTLCExposure::FixedLimitMsat(limit) => limit,
+			MaxDustHTLCExposure::FeeRateMultiplier(_) => 5_000_000,
+		};
+		write_tlv_fields!(writer, {
+			(0, self.forwarding_fee_proportional_millionths, required),
+			(1, self.accept_underpaying_htlcs, (default_value, false)),
+			(2, self.forwarding_fee_base_msat, required),
+			(3, self.max_dust_htlc_exposure, required),
+			(4, self.cltv_expiry_delta, required),
+			(6, max_dust_htlc_exposure_msat_fixed_limit, required),
+			// ChannelConfig serialized this field with a required type of 8 prior to the introduction of
+			// LegacyChannelConfig. To make sure that serialization is not compatible with this one, we use
+			// the next required type of 10, which if seen by the old serialization will always fail.
+			(10, self.force_close_avoidance_max_fee_satoshis, required),
+		});
+		Ok(())
+	}
+}
+
+impl crate::util::ser::Readable for ChannelConfig {
+	fn read<R: crate::io::Read>(reader: &mut R) -> Result<Self, crate::ln::msgs::DecodeError> {
+		let mut forwarding_fee_proportional_millionths = 0;
+		let mut accept_underpaying_htlcs = false;
+		let mut forwarding_fee_base_msat = 1000;
+		let mut cltv_expiry_delta = 6 * 12;
+		let mut max_dust_htlc_exposure_msat = None;
+		let mut max_dust_htlc_exposure_enum = None;
+		let mut force_close_avoidance_max_fee_satoshis = 1000;
+		read_tlv_fields!(reader, {
+			(0, forwarding_fee_proportional_millionths, required),
+			(1, accept_underpaying_htlcs, (default_value, false)),
+			(2, forwarding_fee_base_msat, required),
+			(3, max_dust_htlc_exposure_enum, option),
+			(4, cltv_expiry_delta, required),
+			// Has always been written, but became optionally read in 0.0.116
+			(6, max_dust_htlc_exposure_msat, option),
+			(10, force_close_avoidance_max_fee_satoshis, required),
+		});
+		let max_dust_htlc_fixed_limit = max_dust_htlc_exposure_msat.unwrap_or(5_000_000);
+		let max_dust_htlc_exposure_msat = max_dust_htlc_exposure_enum
+			.unwrap_or(MaxDustHTLCExposure::FixedLimitMsat(max_dust_htlc_fixed_limit));
+		Ok(Self {
+			forwarding_fee_proportional_millionths,
+			accept_underpaying_htlcs,
+			forwarding_fee_base_msat,
+			cltv_expiry_delta,
+			max_dust_htlc_exposure: max_dust_htlc_exposure_msat,
+			force_close_avoidance_max_fee_satoshis,
+		})
+	}
+}
 
 /// A parallel struct to [`ChannelConfig`] to define partial updates.
 #[allow(missing_docs)]
 pub struct ChannelConfigUpdate {
 	pub forwarding_fee_proportional_millionths: Option<u32>,
 	pub forwarding_fee_base_msat: Option<u32>,
 	pub cltv_expiry_delta: Option<u16>,
-	pub max_dust_htlc_exposure_msat: Option<u64>,
+	pub max_dust_htlc_exposure_msat: Option<MaxDustHTLCExposure>,
 	pub force_close_avoidance_max_fee_satoshis: Option<u64>,
 }
 
@@ -513,7 +599,7 @@ impl From<ChannelConfig> for ChannelConfigUpdate {
 			forwarding_fee_proportional_millionths: Some(config.forwarding_fee_proportional_millionths),
 			forwarding_fee_base_msat: Some(config.forwarding_fee_base_msat),
 			cltv_expiry_delta: Some(config.cltv_expiry_delta),
-			max_dust_htlc_exposure_msat: Some(config.max_dust_htlc_exposure_msat),
+			max_dust_htlc_exposure_msat: Some(config.max_dust_htlc_exposure),
 			force_close_avoidance_max_fee_satoshis: Some(config.force_close_avoidance_max_fee_satoshis),
 		}
 	}
@@ -546,12 +632,17 @@ impl Default for LegacyChannelConfig {
 
 impl crate::util::ser::Writeable for LegacyChannelConfig {
 	fn write<W: crate::util::ser::Writer>(&self, writer: &mut W) -> Result<(), crate::io::Error> {
+		let max_dust_htlc_exposure_msat_fixed_limit = match self.options.max_dust_htlc_exposure {
+			MaxDustHTLCExposure::FixedLimitMsat(limit) => limit,
+			MaxDustHTLCExposure::FeeRateMultiplier(_) => 5_000_000,
+		};
 		write_tlv_fields!(writer, {
 			(0, self.options.forwarding_fee_proportional_millionths, required),
-			(1, self.options.max_dust_htlc_exposure_msat, (default_value, 5_000_000)),
+			(1, max_dust_htlc_exposure_msat_fixed_limit, required),
 			(2, self.options.cltv_expiry_delta, required),
 			(3, self.options.force_close_avoidance_max_fee_satoshis, (default_value, 1000)),
 			(4, self.announced_channel, required),
+			(5, self.options.max_dust_htlc_exposure, required),
 			(6, self.commit_upfront_shutdown_pubkey, required),
 			(8, self.options.forwarding_fee_base_msat, required),
 		});
@@ -562,25 +653,32 @@ impl crate::util::ser::Writeable for LegacyChannelConfig {
 impl crate::util::ser::Readable for LegacyChannelConfig {
 	fn read<R: crate::io::Read>(reader: &mut R) -> Result<Self, crate::ln::msgs::DecodeError> {
 		let mut forwarding_fee_proportional_millionths = 0;
-		let mut max_dust_htlc_exposure_msat = 5_000_000;
+		let mut max_dust_htlc_exposure_msat_fixed_limit = None;
 		let mut cltv_expiry_delta = 0;
 		let mut force_close_avoidance_max_fee_satoshis = 1000;
 		let mut announced_channel = false;
 		let mut commit_upfront_shutdown_pubkey = false;
 		let mut forwarding_fee_base_msat = 0;
+		let mut max_dust_htlc_exposure_enum = None;
 		read_tlv_fields!(reader, {
 			(0, forwarding_fee_proportional_millionths, required),
-			(1, max_dust_htlc_exposure_msat, (default_value, 5_000_000u64)),
+			// Has always been written, but became optionally read in 0.0.116
+			(1, max_dust_htlc_exposure_msat_fixed_limit, option),
 			(2, cltv_expiry_delta, required),
 			(3, force_close_avoidance_max_fee_satoshis, (default_value, 1000u64)),
 			(4, announced_channel, required),
+			(5, max_dust_htlc_exposure_enum, option),
 			(6, commit_upfront_shutdown_pubkey, required),
 			(8, forwarding_fee_base_msat, required),
 		});
+		let max_dust_htlc_exposure_msat_fixed_limit =
+			max_dust_htlc_exposure_msat_fixed_limit.unwrap_or(5_000_000);
+		let max_dust_htlc_exposure_msat = max_dust_htlc_exposure_enum
+			.unwrap_or(MaxDustHTLCExposure::FixedLimitMsat(max_dust_htlc_exposure_msat_fixed_limit));
 		Ok(Self {
 			options: ChannelConfig {
 				forwarding_fee_proportional_millionths,
-				max_dust_htlc_exposure_msat,
+				max_dust_htlc_exposure: max_dust_htlc_exposure_msat,
 				cltv_expiry_delta,
 				force_close_avoidance_max_fee_satoshis,
 				forwarding_fee_base_msat,
diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs
@@ -981,7 +981,7 @@ macro_rules! impl_writeable_tlv_based_enum {
 						f()
 					}),*
 					$($tuple_variant_id => {
-						Ok($st::$tuple_variant_name(Readable::read(reader)?))
+						Ok($st::$tuple_variant_name($crate::util::ser::Readable::read(reader)?))
 					}),*
 					_ => {
 						Err($crate::ln::msgs::DecodeError::UnknownRequiredFeature)
