Skip to content

Commit edcd376

Browse files
tankyleoTheBlueMatt
authored andcommitted
Do not dip into the funder's reserve to cover the two anchors
At all times, the funder's balance should cover the commitment transaction fee, any non-zero-value anchors, and the fundee-selected channel reserve. Prior to this commit, we would allow the funder to dip into its reserve to pay for the two 330sat anchors. LDK sets reserves to at least 1000sat, so two 330 sat anchors would never overdraw this reserve. We now prevent any such dips, and ensure that the funder can pay for the complete sum of the transaction fee, the anchors, and the reserve. Substantial conflicts resulted in the `channel.rs` parts of this patch being rewriten. The `functional_tests.rs` changes also conflicted but were re-applied to the proper file.
1 parent bc9ffe4 commit edcd376

File tree

2 files changed

+52
-13
lines changed

2 files changed

+52
-13
lines changed

lightning/src/ln/channel.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5039,7 +5039,12 @@ impl<SP: Deref> Channel<SP> where
50395039
if update_fee {
50405040
debug_assert!(!self.context.is_outbound());
50415041
let counterparty_reserve_we_require_msat = self.context.holder_selected_channel_reserve_satoshis * 1000;
5042-
if commitment_stats.remote_balance_msat < commitment_stats.total_fee_sat * 1000 + counterparty_reserve_we_require_msat {
5042+
let total_anchor_sats = if self.context.channel_type.supports_anchors_zero_fee_htlc_tx() {
5043+
ANCHOR_OUTPUT_VALUE_SATOSHI * 2
5044+
} else {
5045+
0
5046+
};
5047+
if commitment_stats.remote_balance_msat < commitment_stats.total_fee_sat * 1000 + total_anchor_sats * 1000 + counterparty_reserve_we_require_msat {
50435048
return Err(ChannelError::close("Funding remote cannot afford proposed new fee".to_owned()));
50445049
}
50455050
}
@@ -5772,7 +5777,12 @@ impl<SP: Deref> Channel<SP> where
57725777
let commitment_stats = self.context.build_commitment_transaction(self.holder_commitment_point.transaction_number(), &keys, true, true, logger);
57735778
let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_stats.num_nondust_htlcs + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, self.context.get_channel_type()) * 1000;
57745779
let holder_balance_msat = commitment_stats.local_balance_msat - htlc_stats.outbound_holding_cell_msat;
5775-
if holder_balance_msat < buffer_fee_msat + self.context.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 {
5780+
let total_anchor_sats = if self.context.channel_type.supports_anchors_zero_fee_htlc_tx() {
5781+
ANCHOR_OUTPUT_VALUE_SATOSHI * 2
5782+
} else {
5783+
0
5784+
};
5785+
if holder_balance_msat < buffer_fee_msat + total_anchor_sats * 1000 + self.context.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 {
57765786
//TODO: auto-close after a number of failures?
57775787
log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw);
57785788
return None;

lightning/src/ln/functional_tests.rs

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use crate::ln::types::ChannelId;
2424
use crate::types::payment::{PaymentPreimage, PaymentSecret, PaymentHash};
2525
use crate::ln::channel::{CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel, COINBASE_MATURITY, ChannelPhase};
2626
use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, RecipientOnionFields, BREAKDOWN_TIMEOUT, ENABLE_GOSSIP_TICKS, DISABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA};
27-
use crate::ln::channel::{DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, ChannelError};
27+
use crate::ln::channel::{ANCHOR_OUTPUT_VALUE_SATOSHI, DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, ChannelError};
2828
use crate::ln::{chan_utils, onion_utils};
2929
use crate::ln::chan_utils::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, OFFERED_HTLC_SCRIPT_WEIGHT, htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment};
3030
use crate::routing::gossip::{NetworkGraph, NetworkUpdate};
@@ -673,28 +673,49 @@ fn test_update_fee_vanilla() {
673673
check_added_monitors!(nodes[1], 1);
674674
}
675675

676-
#[test]
677-
fn test_update_fee_that_funder_cannot_afford() {
676+
pub fn do_test_update_fee_that_funder_cannot_afford(channel_type_features: ChannelTypeFeatures) {
678677
let chanmon_cfgs = create_chanmon_cfgs(2);
679678
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
680-
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
679+
680+
let mut default_config = test_default_channel_config();
681+
if channel_type_features == ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() {
682+
default_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
683+
// this setting is also needed to create an anchor channel
684+
default_config.manually_accept_inbound_channels = true;
685+
}
686+
687+
let node_chanmgrs = create_node_chanmgrs(
688+
2,
689+
&node_cfgs,
690+
&[Some(default_config.clone()), Some(default_config.clone())],
691+
);
692+
681693
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
682694
let channel_value = 5000;
683695
let push_sats = 700;
684696
let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, channel_value, push_sats * 1000);
685697
let channel_id = chan.2;
686698
let secp_ctx = Secp256k1::new();
687-
let default_config = UserConfig::default();
688699
let bs_channel_reserve_sats = get_holder_selected_channel_reserve_satoshis(channel_value, &default_config);
689700

690-
let channel_type_features = ChannelTypeFeatures::only_static_remote_key();
701+
let (anchor_outputs_value_sats, outputs_num_no_htlcs) =
702+
if channel_type_features.supports_anchors_zero_fee_htlc_tx() {
703+
(ANCHOR_OUTPUT_VALUE_SATOSHI * 2, 4)
704+
} else {
705+
(0, 2)
706+
};
691707

692708
// Calculate the maximum feerate that A can afford. Note that we don't send an update_fee
693709
// CONCURRENT_INBOUND_HTLC_FEE_BUFFER HTLCs before actually running out of local balance, so we
694710
// calculate two different feerates here - the expected local limit as well as the expected
695711
// remote limit.
696-
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;
697-
let non_buffer_feerate = ((channel_value - bs_channel_reserve_sats - push_sats) * 1000 / commitment_tx_base_weight(&channel_type_features)) as u32;
712+
let feerate =
713+
((channel_value - bs_channel_reserve_sats - push_sats - anchor_outputs_value_sats) * 1000
714+
/ (commitment_tx_base_weight(&channel_type_features)
715+
+ CONCURRENT_INBOUND_HTLC_FEE_BUFFER as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC)) as u32;
716+
let non_buffer_feerate =
717+
((channel_value - bs_channel_reserve_sats - push_sats - anchor_outputs_value_sats) * 1000
718+
/ commitment_tx_base_weight(&channel_type_features)) as u32;
698719
{
699720
let mut feerate_lock = chanmon_cfgs[0].fee_estimator.sat_per_kw.lock().unwrap();
700721
*feerate_lock = feerate;
@@ -711,8 +732,8 @@ fn test_update_fee_that_funder_cannot_afford() {
711732
{
712733
let commitment_tx = get_local_commitment_txn!(nodes[1], channel_id)[0].clone();
713734

714-
//We made sure neither party's funds are below the dust limit and there are no HTLCs here
715-
assert_eq!(commitment_tx.output.len(), 2);
735+
// We made sure neither party's funds are below the dust limit and there are no HTLCs here
736+
assert_eq!(commitment_tx.output.len(), outputs_num_no_htlcs);
716737
let total_fee: u64 = commit_tx_fee_msat(feerate, 0, &channel_type_features) / 1000;
717738
let mut actual_fee = commitment_tx.output.iter().fold(0, |acc, output| acc + output.value.to_sat());
718739
actual_fee = channel_value - actual_fee;
@@ -771,7 +792,7 @@ fn test_update_fee_that_funder_cannot_afford() {
771792
let commitment_tx = CommitmentTransaction::new_with_auxiliary_htlc_data(
772793
INITIAL_COMMITMENT_NUMBER - 1,
773794
push_sats,
774-
channel_value - push_sats - commit_tx_fee_msat(non_buffer_feerate + 4, 0, &channel_type_features) / 1000,
795+
channel_value - push_sats - anchor_outputs_value_sats - commit_tx_fee_msat(non_buffer_feerate + 4, 0, &channel_type_features) / 1000,
775796
local_funding, remote_funding,
776797
commit_tx_keys.clone(),
777798
non_buffer_feerate + 4,
@@ -808,6 +829,14 @@ fn test_update_fee_that_funder_cannot_afford() {
808829
[nodes[0].node.get_our_node_id()], channel_value);
809830
}
810831

832+
#[test]
833+
pub fn test_update_fee_that_funder_cannot_afford() {
834+
do_test_update_fee_that_funder_cannot_afford(ChannelTypeFeatures::only_static_remote_key());
835+
do_test_update_fee_that_funder_cannot_afford(
836+
ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(),
837+
);
838+
}
839+
811840
#[test]
812841
fn test_update_fee_with_fundee_update_add_htlc() {
813842
let chanmon_cfgs = create_chanmon_cfgs(2);

0 commit comments

Comments
 (0)