Skip to content

Commit fba99b7

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #2674 from wpaulino/consider-anchor-outputs-value-balances
Consider anchor outputs value throughout balance checks and computations
2 parents be8797e + 27fba2d commit fba99b7

File tree

4 files changed

+209
-56
lines changed

4 files changed

+209
-56
lines changed

‎fuzz/src/chanmon_consistency.rs‎

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
432432
}
433433

434434
#[inline]
435-
pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
435+
pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
436436
let out = SearchingOutput::new(underlying_out);
437437
let broadcast = Arc::new(TestBroadcaster{});
438438
let router = FuzzRouter {};
@@ -450,6 +450,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
450450
let mut config = UserConfig::default();
451451
config.channel_config.forwarding_fee_proportional_millionths = 0;
452452
config.channel_handshake_config.announced_channel = true;
453+
if anchors {
454+
config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
455+
config.manually_accept_inbound_channels = true;
456+
}
453457
let network = Network::Bitcoin;
454458
let best_block_timestamp = genesis_block(network).header.time;
455459
let params = ChainParameters {
@@ -473,6 +477,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
473477
let mut config = UserConfig::default();
474478
config.channel_config.forwarding_fee_proportional_millionths = 0;
475479
config.channel_handshake_config.announced_channel = true;
480+
if anchors {
481+
config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
482+
config.manually_accept_inbound_channels = true;
483+
}
476484

477485
let mut monitors = HashMap::new();
478486
let mut old_monitors = $old_monitors.latest_monitors.lock().unwrap();
@@ -509,7 +517,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
509517

510518
let mut channel_txn = Vec::new();
511519
macro_rules! make_channel {
512-
($source: expr, $dest: expr, $chan_id: expr) => { {
520+
($source: expr, $dest: expr, $dest_keys_manager: expr, $chan_id: expr) => { {
513521
$source.peer_connected(&$dest.get_our_node_id(), &Init {
514522
features: $dest.init_features(), networks: None, remote_network_address: None
515523
}, true).unwrap();
@@ -528,6 +536,22 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
528536

529537
$dest.handle_open_channel(&$source.get_our_node_id(), &open_channel);
530538
let accept_channel = {
539+
if anchors {
540+
let events = $dest.get_and_clear_pending_events();
541+
assert_eq!(events.len(), 1);
542+
if let events::Event::OpenChannelRequest {
543+
ref temporary_channel_id, ref counterparty_node_id, ..
544+
} = events[0] {
545+
let mut random_bytes = [0u8; 16];
546+
random_bytes.copy_from_slice(&$dest_keys_manager.get_secure_random_bytes()[..16]);
547+
let user_channel_id = u128::from_be_bytes(random_bytes);
548+
$dest.accept_inbound_channel(
549+
temporary_channel_id,
550+
counterparty_node_id,
551+
user_channel_id,
552+
).unwrap();
553+
} else { panic!("Wrong event type"); }
554+
}
531555
let events = $dest.get_and_clear_pending_msg_events();
532556
assert_eq!(events.len(), 1);
533557
if let events::MessageSendEvent::SendAcceptChannel { ref msg, .. } = events[0] {
@@ -639,8 +663,8 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
639663

640664
let mut nodes = [node_a, node_b, node_c];
641665

642-
let chan_1_funding = make_channel!(nodes[0], nodes[1], 0);
643-
let chan_2_funding = make_channel!(nodes[1], nodes[2], 1);
666+
let chan_1_funding = make_channel!(nodes[0], nodes[1], keys_manager_b, 0);
667+
let chan_2_funding = make_channel!(nodes[1], nodes[2], keys_manager_c, 1);
644668

645669
for node in nodes.iter() {
646670
confirm_txn!(node);
@@ -1199,7 +1223,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
11991223
0x6d => { send_hop_payment(&nodes[2], &nodes[1], chan_b, &nodes[0], chan_a, 1, &mut payment_id, &mut payment_idx); },
12001224

12011225
0x80 => {
1202-
let max_feerate = last_htlc_clear_fee_a * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
1226+
let mut max_feerate = last_htlc_clear_fee_a;
1227+
if !anchors {
1228+
max_feerate *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
1229+
}
12031230
if fee_est_a.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
12041231
fee_est_a.ret_val.store(max_feerate, atomic::Ordering::Release);
12051232
}
@@ -1208,7 +1235,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
12081235
0x81 => { fee_est_a.ret_val.store(253, atomic::Ordering::Release); nodes[0].maybe_update_chan_fees(); },
12091236

12101237
0x84 => {
1211-
let max_feerate = last_htlc_clear_fee_b * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
1238+
let mut max_feerate = last_htlc_clear_fee_b;
1239+
if !anchors {
1240+
max_feerate *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
1241+
}
12121242
if fee_est_b.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
12131243
fee_est_b.ret_val.store(max_feerate, atomic::Ordering::Release);
12141244
}
@@ -1217,7 +1247,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
12171247
0x85 => { fee_est_b.ret_val.store(253, atomic::Ordering::Release); nodes[1].maybe_update_chan_fees(); },
12181248

12191249
0x88 => {
1220-
let max_feerate = last_htlc_clear_fee_c * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
1250+
let mut max_feerate = last_htlc_clear_fee_c;
1251+
if !anchors {
1252+
max_feerate *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
1253+
}
12211254
if fee_est_c.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
12221255
fee_est_c.ret_val.store(max_feerate, atomic::Ordering::Release);
12231256
}
@@ -1338,10 +1371,12 @@ impl<O: Output> SearchingOutput<O> {
13381371
}
13391372

13401373
pub fn chanmon_consistency_test<Out: Output>(data: &[u8], out: Out) {
1341-
do_test(data, out);
1374+
do_test(data, out.clone(), false);
1375+
do_test(data, out, true);
13421376
}
13431377

13441378
#[no_mangle]
13451379
pub extern "C" fn chanmon_consistency_run(data: *const u8, datalen: usize) {
1346-
do_test(unsafe { std::slice::from_raw_parts(data, datalen) }, test_logger::DevNull{});
1380+
do_test(unsafe { std::slice::from_raw_parts(data, datalen) }, test_logger::DevNull{}, false);
1381+
do_test(unsafe { std::slice::from_raw_parts(data, datalen) }, test_logger::DevNull{}, true);
13471382
}

‎lightning/src/ln/channel.rs‎

Lines changed: 60 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,7 @@ pub(super) struct ChannelContext<SP: Deref> where SP::Target: SignerProvider {
724724

725725
cur_holder_commitment_transaction_number: u64,
726726
cur_counterparty_commitment_transaction_number: u64,
727-
value_to_self_msat: u64, // Excluding all pending_htlcs, excluding fees
727+
value_to_self_msat: u64, // Excluding all pending_htlcs, fees, and anchor outputs
728728
pending_inbound_htlcs: Vec<InboundHTLCOutput>,
729729
pending_outbound_htlcs: Vec<OutboundHTLCOutput>,
730730
holding_cell_htlc_updates: Vec<HTLCUpdateAwaitingACK>,
@@ -1673,6 +1673,11 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
16731673

16741674
let mut available_capacity_msat = outbound_capacity_msat;
16751675

1676+
let anchor_outputs_value_msat = if context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
1677+
ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000
1678+
} else {
1679+
0
1680+
};
16761681
if context.is_outbound() {
16771682
// We should mind channel commit tx fee when computing how much of the available capacity
16781683
// can be used in the next htlc. Mirrors the logic in send_htlc.
@@ -1687,14 +1692,19 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
16871692
}
16881693

16891694
let htlc_above_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000, HTLCInitiator::LocalOffered);
1690-
let max_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * context.next_local_commit_tx_fee_msat(htlc_above_dust, Some(()));
1695+
let mut max_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(htlc_above_dust, Some(()));
16911696
let htlc_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000 - 1, HTLCInitiator::LocalOffered);
1692-
let min_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * context.next_local_commit_tx_fee_msat(htlc_dust, Some(()));
1697+
let mut min_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(htlc_dust, Some(()));
1698+
if !context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
1699+
max_reserved_commit_tx_fee_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
1700+
min_reserved_commit_tx_fee_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
1701+
}
16931702

16941703
// We will first subtract the fee as if we were above-dust. Then, if the resulting
16951704
// value ends up being below dust, we have this fee available again. In that case,
16961705
// match the value to right-below-dust.
1697-
let mut capacity_minus_commitment_fee_msat: i64 = (available_capacity_msat as i64) - (max_reserved_commit_tx_fee_msat as i64);
1706+
let mut capacity_minus_commitment_fee_msat: i64 = available_capacity_msat as i64 -
1707+
max_reserved_commit_tx_fee_msat as i64 - anchor_outputs_value_msat as i64;
16981708
if capacity_minus_commitment_fee_msat < (real_dust_limit_timeout_sat as i64) * 1000 {
16991709
let one_htlc_difference_msat = max_reserved_commit_tx_fee_msat - min_reserved_commit_tx_fee_msat;
17001710
debug_assert!(one_htlc_difference_msat != 0);
@@ -1719,7 +1729,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
17191729
let remote_balance_msat = (context.channel_value_satoshis * 1000 - context.value_to_self_msat)
17201730
.saturating_sub(inbound_stats.pending_htlcs_value_msat);
17211731

1722-
if remote_balance_msat < max_reserved_commit_tx_fee_msat + holder_selected_chan_reserve_msat {
1732+
if remote_balance_msat < max_reserved_commit_tx_fee_msat + holder_selected_chan_reserve_msat + anchor_outputs_value_msat {
17231733
// If another HTLC's fee would reduce the remote's balance below the reserve limit
17241734
// we've selected for them, we can only send dust HTLCs.
17251735
available_capacity_msat = cmp::min(available_capacity_msat, real_dust_limit_success_sat * 1000 - 1);
@@ -2119,7 +2129,7 @@ fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, channel_type_feature
21192129

21202130
// Get the fee cost in MSATS of a commitment tx with a given number of HTLC outputs.
21212131
// Note that num_htlcs should not include dust HTLCs.
2122-
fn commit_tx_fee_msat(feerate_per_kw: u32, num_htlcs: usize, channel_type_features: &ChannelTypeFeatures) -> u64 {
2132+
pub(crate) fn commit_tx_fee_msat(feerate_per_kw: u32, num_htlcs: usize, channel_type_features: &ChannelTypeFeatures) -> u64 {
21232133
// Note that we need to divide before multiplying to round properly,
21242134
// since the lowest denomination of bitcoin on-chain is the satoshi.
21252135
(commitment_tx_base_weight(channel_type_features) + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * feerate_per_kw as u64 / 1000 * 1000
@@ -2766,6 +2776,7 @@ impl<SP: Deref> Channel<SP> where
27662776
if inbound_stats.pending_htlcs_value_msat + msg.amount_msat > self.context.holder_max_htlc_value_in_flight_msat {
27672777
return Err(ChannelError::Close(format!("Remote HTLC add would put them over our max HTLC value ({})", self.context.holder_max_htlc_value_in_flight_msat)));
27682778
}
2779+
27692780
// Check holder_selected_channel_reserve_satoshis (we're getting paid, so they have to at least meet
27702781
// the reserve_satoshis we told them to always have as direct payment so that they lose
27712782
// something if we punish them for broadcasting an old state).
@@ -2825,30 +2836,40 @@ impl<SP: Deref> Channel<SP> where
28252836

28262837
// Check that the remote can afford to pay for this HTLC on-chain at the current
28272838
// feerate_per_kw, while maintaining their channel reserve (as required by the spec).
2828-
let remote_commit_tx_fee_msat = if self.context.is_outbound() { 0 } else {
2829-
let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
2830-
self.context.next_remote_commit_tx_fee_msat(htlc_candidate, None) // Don't include the extra fee spike buffer HTLC in calculations
2831-
};
2832-
if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
2833-
return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees".to_owned()));
2834-
};
2835-
2836-
if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < self.context.holder_selected_channel_reserve_satoshis * 1000 {
2837-
return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value".to_owned()));
2839+
{
2840+
let remote_commit_tx_fee_msat = if self.context.is_outbound() { 0 } else {
2841+
let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
2842+
self.context.next_remote_commit_tx_fee_msat(htlc_candidate, None) // Don't include the extra fee spike buffer HTLC in calculations
2843+
};
2844+
let anchor_outputs_value_msat = if !self.context.is_outbound() && self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
2845+
ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000
2846+
} else {
2847+
0
2848+
};
2849+
if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(anchor_outputs_value_msat) < remote_commit_tx_fee_msat {
2850+
return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees".to_owned()));
2851+
};
2852+
if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat).saturating_sub(anchor_outputs_value_msat) < self.context.holder_selected_channel_reserve_satoshis * 1000 {
2853+
return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value".to_owned()));
2854+
}
28382855
}
28392856

2857+
let anchor_outputs_value_msat = if self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
2858+
ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000
2859+
} else {
2860+
0
2861+
};
28402862
if !self.context.is_outbound() {
2841-
// `2 *` and `Some(())` is for the fee spike buffer we keep for the remote. This deviates from
2842-
// the spec because in the spec, the fee spike buffer requirement doesn't exist on the
2843-
// receiver's side, only on the sender's.
2844-
// Note that when we eventually remove support for fee updates and switch to anchor output
2845-
// fees, we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep
2846-
// the extra htlc when calculating the next remote commitment transaction fee as we should
2847-
// still be able to afford adding this HTLC plus one more future HTLC, regardless of being
2848-
// sensitive to fee spikes.
2863+
// `Some(())` is for the fee spike buffer we keep for the remote. This deviates from
2864+
// the spec because the fee spike buffer requirement doesn't exist on the receiver's
2865+
// side, only on the sender's. Note that with anchor outputs we are no longer as
2866+
// sensitive to fee spikes, so we need to account for them.
28492867
let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
2850-
let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.context.next_remote_commit_tx_fee_msat(htlc_candidate, Some(()));
2851-
if pending_remote_value_msat - msg.amount_msat - self.context.holder_selected_channel_reserve_satoshis * 1000 < remote_fee_cost_incl_stuck_buffer_msat {
2868+
let mut remote_fee_cost_incl_stuck_buffer_msat = self.context.next_remote_commit_tx_fee_msat(htlc_candidate, Some(()));
2869+
if !self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
2870+
remote_fee_cost_incl_stuck_buffer_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
2871+
}
2872+
if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(self.context.holder_selected_channel_reserve_satoshis * 1000).saturating_sub(anchor_outputs_value_msat) < remote_fee_cost_incl_stuck_buffer_msat {
28522873
// Note that if the pending_forward_status is not updated here, then it's because we're already failing
28532874
// the HTLC, i.e. its status is already set to failing.
28542875
log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", &self.context.channel_id());
@@ -2858,7 +2879,7 @@ impl<SP: Deref> Channel<SP> where
28582879
// Check that they won't violate our local required channel reserve by adding this HTLC.
28592880
let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
28602881
let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(htlc_candidate, None);
2861-
if self.context.value_to_self_msat < self.context.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat {
2882+
if self.context.value_to_self_msat < self.context.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat {
28622883
return Err(ChannelError::Close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned()));
28632884
}
28642885
}
@@ -5721,16 +5742,16 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
57215742
let channel_type = Self::get_initial_channel_type(&config, their_features);
57225743
debug_assert!(channel_type.is_subset(&channelmanager::provided_channel_type_features(&config)));
57235744

5724-
let commitment_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() {
5725-
ConfirmationTarget::AnchorChannelFee
5745+
let (commitment_conf_target, anchor_outputs_value_msat) = if channel_type.supports_anchors_zero_fee_htlc_tx() {
5746+
(ConfirmationTarget::AnchorChannelFee, ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000)
57265747
} else {
5727-
ConfirmationTarget::NonAnchorChannelFee
5748+
(ConfirmationTarget::NonAnchorChannelFee, 0)
57285749
};
57295750
let commitment_feerate = fee_estimator.bounded_sat_per_1000_weight(commitment_conf_target);
57305751

57315752
let value_to_self_msat = channel_value_satoshis * 1000 - push_msat;
57325753
let commitment_tx_fee = commit_tx_fee_msat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type);
5733-
if value_to_self_msat < commitment_tx_fee {
5754+
if value_to_self_msat.saturating_sub(anchor_outputs_value_msat) < commitment_tx_fee {
57345755
return Err(APIError::APIMisuseError{ err: format!("Funding amount ({}) can't even pay fee for initial commitment transaction fee of {}.", value_to_self_msat / 1000, commitment_tx_fee / 1000) });
57355756
}
57365757

@@ -6347,13 +6368,18 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
63476368

63486369
// check if the funder's amount for the initial commitment tx is sufficient
63496370
// for full fee payment plus a few HTLCs to ensure the channel will be useful.
6371+
let anchor_outputs_value = if channel_type.supports_anchors_zero_fee_htlc_tx() {
6372+
ANCHOR_OUTPUT_VALUE_SATOSHI * 2
6373+
} else {
6374+
0
6375+
};
63506376
let funders_amount_msat = msg.funding_satoshis * 1000 - msg.push_msat;
63516377
let commitment_tx_fee = commit_tx_fee_msat(msg.feerate_per_kw, MIN_AFFORDABLE_HTLC_COUNT, &channel_type) / 1000;
6352-
if funders_amount_msat / 1000 < commitment_tx_fee {
6353-
return Err(ChannelError::Close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", funders_amount_msat / 1000, commitment_tx_fee)));
6378+
if (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value) < commitment_tx_fee {
6379+
return Err(ChannelError::Close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value), commitment_tx_fee)));
63546380
}
63556381

6356-
let to_remote_satoshis = funders_amount_msat / 1000 - commitment_tx_fee;
6382+
let to_remote_satoshis = funders_amount_msat / 1000 - commitment_tx_fee - anchor_outputs_value;
63576383
// While it's reasonable for us to not meet the channel reserve initially (if they don't
63586384
// want to push much to us), our counterparty should always have more than our reserve.
63596385
if to_remote_satoshis < holder_selected_channel_reserve_satoshis {

0 commit comments

Comments
 (0)