Skip to content

Bump dust exposure threshold #2354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 8, 2023
Merged

Bump dust exposure threshold #2354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cfc7ec6
Add max dust exposure multiplier config knob
alecchendev Jun 19, 2023
c2992fd
Send fee estimator through to `get_max_htlc_dust_exposure_threshold`
alecchendev Jun 20, 2023
b040335
Use multiplier in dust exposure threshold calculation
alecchendev Jul 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions fuzz/src/full_stack.rs
View file
Open in desktop

Large diffs are not rendered by default.

132 changes: 90 additions & 42 deletions lightning/src/ln/channel.rs
View file
Open in desktop

Large diffs are not rendered by default.

38 changes: 22 additions & 16 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1503,10 +1503,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 {
Expand Down Expand Up @@ -2192,7 +2195,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);
}
}
Expand All @@ -2218,17 +2221,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);
}
}
Expand Down Expand Up @@ -2261,7 +2264,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![]
Expand Down Expand Up @@ -3065,7 +3069,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) {
Expand Down Expand Up @@ -3782,7 +3786,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);
Expand Down Expand Up @@ -4171,7 +4176,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
}

Expand Down Expand Up @@ -5507,7 +5512,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))
}
Expand Down Expand Up @@ -5724,7 +5729,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(|_| ())
Expand Down Expand Up @@ -5995,7 +6000,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));
}
Expand Down Expand Up @@ -10001,7 +10006,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;
Expand Down Expand Up @@ -10047,6 +10052,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);
Expand Down
8 changes: 5 additions & 3 deletions lightning/src/ln/functional_test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -2572,8 +2572,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
}

Expand Down
69 changes: 43 additions & 26 deletions lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
//
Expand All @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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]
Expand Down
14 changes: 12 additions & 2 deletions lightning/src/ln/onion_route_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Loading