Skip to content

Do not dip into the funder's reserve to cover the two anchors #3796

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3808,7 +3808,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
if update_fee {
debug_assert!(!funding.is_outbound());
let counterparty_reserve_we_require_msat = funding.holder_selected_channel_reserve_satoshis * 1000;
if commitment_data.stats.remote_balance_before_fee_anchors_msat < commitment_data.stats.total_fee_sat * 1000 + counterparty_reserve_we_require_msat {
if commitment_data.stats.remote_balance_before_fee_anchors_msat < commitment_data.stats.total_fee_sat * 1000 + commitment_data.stats.total_anchors_sat * 1000 + counterparty_reserve_we_require_msat {
return Err(ChannelError::close("Funding remote cannot afford proposed new fee".to_owned()));
}
}
Expand Down Expand Up @@ -6724,7 +6724,7 @@ impl<SP: Deref> FundedChannel<SP> where
);
let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.tx.nondust_htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, self.funding.get_channel_type()) * 1000;
let holder_balance_msat = commitment_data.stats.local_balance_before_fee_anchors_msat - htlc_stats.outbound_holding_cell_msat;
if holder_balance_msat < buffer_fee_msat + self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 {
if holder_balance_msat < buffer_fee_msat + commitment_data.stats.total_anchors_sat * 1000 + self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 {
//TODO: auto-close after a number of failures?
log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw);
return None;
Expand Down
59 changes: 41 additions & 18 deletions lightning/src/ln/update_fee_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::ln::chan_utils::{
COMMITMENT_TX_WEIGHT_PER_HTLC,
};
use crate::ln::channel::{
get_holder_selected_channel_reserve_satoshis, CONCURRENT_INBOUND_HTLC_FEE_BUFFER,
MIN_AFFORDABLE_HTLC_COUNT,
get_holder_selected_channel_reserve_satoshis, ANCHOR_OUTPUT_VALUE_SATOSHI,
CONCURRENT_INBOUND_HTLC_FEE_BUFFER, MIN_AFFORDABLE_HTLC_COUNT,
};
use crate::ln::channelmanager::PaymentId;
use crate::ln::functional_test_utils::*;
Expand Down Expand Up @@ -388,11 +388,22 @@ pub fn test_update_fee_vanilla() {
check_added_monitors(&nodes[1], 1);
}

#[xtest(feature = "_externalize_tests")]
pub fn test_update_fee_that_funder_cannot_afford() {
pub fn do_test_update_fee_that_funder_cannot_afford(channel_type_features: ChannelTypeFeatures) {
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, &[None, None]);

let mut default_config = test_default_channel_config();
if channel_type_features == ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() {
default_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
// this setting is also needed to create an anchor channel
default_config.manually_accept_inbound_channels = true;
}

let node_chanmgrs = create_node_chanmgrs(
2,
&node_cfgs,
&[Some(default_config.clone()), Some(default_config.clone())],
);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

let node_a_id = nodes[0].node.get_our_node_id();
Expand All @@ -409,22 +420,26 @@ pub fn test_update_fee_that_funder_cannot_afford() {
);
let channel_id = chan.2;
let secp_ctx = Secp256k1::new();
let default_config = UserConfig::default();
let bs_channel_reserve_sats =
get_holder_selected_channel_reserve_satoshis(channel_value, &default_config);

let channel_type_features = ChannelTypeFeatures::only_static_remote_key();
let (anchor_outputs_value_sats, outputs_num_no_htlcs) =
if channel_type_features.supports_anchors_zero_fee_htlc_tx() {
(ANCHOR_OUTPUT_VALUE_SATOSHI * 2, 4)
} else {
(0, 2)
};

// Calculate the maximum feerate that A can afford. Note that we don't send an update_fee
// CONCURRENT_INBOUND_HTLC_FEE_BUFFER HTLCs before actually running out of local balance, so we
// calculate two different feerates here - the expected local limit as well as the expected
// remote limit.
let feerate = ((channel_value - bs_channel_reserve_sats - push_sats) * 1000
/ (commitment_tx_base_weight(&channel_type_features)
+ CONCURRENT_INBOUND_HTLC_FEE_BUFFER as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC))
as u32;
let non_buffer_feerate = ((channel_value - bs_channel_reserve_sats - push_sats) * 1000
/ commitment_tx_base_weight(&channel_type_features)) as u32;
let feerate =
((channel_value - bs_channel_reserve_sats - push_sats - anchor_outputs_value_sats) * 1000
/ (commitment_tx_base_weight(&channel_type_features)
+ CONCURRENT_INBOUND_HTLC_FEE_BUFFER as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC)) as u32;
let non_buffer_feerate =
((channel_value - bs_channel_reserve_sats - push_sats - anchor_outputs_value_sats) * 1000
/ commitment_tx_base_weight(&channel_type_features)) as u32;
{
let mut feerate_lock = chanmon_cfgs[0].fee_estimator.sat_per_kw.lock().unwrap();
*feerate_lock = feerate;
Expand All @@ -441,8 +456,8 @@ pub fn test_update_fee_that_funder_cannot_afford() {
{
let commitment_tx = get_local_commitment_txn!(nodes[1], channel_id)[0].clone();

//We made sure neither party's funds are below the dust limit and there are no HTLCs here
assert_eq!(commitment_tx.output.len(), 2);
// We made sure neither party's funds are below the dust limit and there are no HTLCs here
assert_eq!(commitment_tx.output.len(), outputs_num_no_htlcs);
let total_fee: u64 = commit_tx_fee_msat(feerate, 0, &channel_type_features) / 1000;
let mut actual_fee =
commitment_tx.output.iter().fold(0, |acc, output| acc + output.value.to_sat());
Expand Down Expand Up @@ -486,8 +501,8 @@ pub fn test_update_fee_that_funder_cannot_afford() {
&remote_point,
push_sats,
channel_value
- push_sats - commit_tx_fee_msat(non_buffer_feerate + 4, 0, &channel_type_features)
/ 1000,
- push_sats - anchor_outputs_value_sats
- commit_tx_fee_msat(non_buffer_feerate + 4, 0, &channel_type_features) / 1000,
non_buffer_feerate + 4,
nondust_htlcs,
&local_chan.funding().channel_transaction_parameters.as_counterparty_broadcastable(),
Expand Down Expand Up @@ -526,6 +541,14 @@ pub fn test_update_fee_that_funder_cannot_afford() {
check_closed_event!(nodes[1], 1, reason, [node_a_id], channel_value);
}

#[xtest(feature = "_externalize_tests")]
pub fn test_update_fee_that_funder_cannot_afford() {
do_test_update_fee_that_funder_cannot_afford(ChannelTypeFeatures::only_static_remote_key());
do_test_update_fee_that_funder_cannot_afford(
ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(),
);
}

#[xtest(feature = "_externalize_tests")]
pub fn test_update_fee_that_saturates_subs() {
// Check that when a remote party sends us an `update_fee` message that results in a total fee
Expand Down
Loading