diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 7f33937f2dd..cfd8b39f814 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6275,14 +6275,11 @@ impl FundedChannel where } } } - if update_add_count == 0 && update_fulfill_count == 0 && update_fail_count == 0 && self.context.holding_cell_update_fee.is_none() { + let update_fee = self.context.holding_cell_update_fee.take().and_then(|feerate| self.send_update_fee(feerate, false, fee_estimator, logger)); + + if update_add_count == 0 && update_fulfill_count == 0 && update_fail_count == 0 && update_fee.is_none() { return (None, htlcs_to_fail); } - let update_fee = if let Some(feerate) = self.context.holding_cell_update_fee.take() { - self.send_update_fee(feerate, false, fee_estimator, logger) - } else { - None - }; let mut additional_update = self.build_commitment_no_status_check(logger); // build_commitment_no_status_check and get_update_fulfill_htlc may bump latest_monitor_id diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8a0061f8c6b..4b501a5d798 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6586,7 +6586,7 @@ where NotifyOption::DoPersist } - #[cfg(fuzzing)] + #[cfg(any(test, fuzzing, feature = "_externalize_tests"))] /// In chanmon_consistency we want to sometimes do the channel fee updates done in /// timer_tick_occurred, but we can't generate the disabled channel updates as it considers /// these a fuzz failure (as they usually indicate a channel force-close, which is exactly what diff --git a/lightning/src/ln/update_fee_tests.rs b/lightning/src/ln/update_fee_tests.rs index 1512444c830..464b1b543f4 100644 --- a/lightning/src/ln/update_fee_tests.rs +++ b/lightning/src/ln/update_fee_tests.rs @@ -1009,3 +1009,187 @@ pub fn accept_busted_but_better_fee() { _ => panic!("Unexpected event"), }; } + +#[xtest(feature = "_externalize_tests")] +pub fn cannot_afford_on_holding_cell_release() { + do_cannot_afford_on_holding_cell_release(ChannelTypeFeatures::only_static_remote_key(), true); + do_cannot_afford_on_holding_cell_release(ChannelTypeFeatures::only_static_remote_key(), false); + do_cannot_afford_on_holding_cell_release( + ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), + true, + ); + do_cannot_afford_on_holding_cell_release( + ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), + false, + ); +} + +pub fn do_cannot_afford_on_holding_cell_release( + channel_type_features: ChannelTypeFeatures, can_afford: bool, +) { + // Test that if we can't afford a feerate update when releasing an + // update_fee from its holding cell, we do not generate any msg events + let chanmon_cfgs = create_chanmon_cfgs(2); + + let mut default_config = test_default_channel_config(); + default_config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = + 100; + if channel_type_features.supports_anchors_zero_fee_htlc_tx() { + default_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + default_config.manually_accept_inbound_channels = true; + } + + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = + create_node_chanmgrs(2, &node_cfgs, &[Some(default_config.clone()), Some(default_config)]); + + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_a_id = nodes[0].node.get_our_node_id(); + let node_b_id = nodes[1].node.get_our_node_id(); + + let target_feerate = 1000; + let expected_tx_fee_sat = + chan_utils::commit_tx_fee_sat(target_feerate, 1, &channel_type_features); + // This is the number of htlcs that `send_update_fee` will account for when checking whether + // it can afford the new feerate upon releasing an update_fee from its holding cell, + // ie the buffer + the inbound HTLC we will add while the update_fee is in the holding cell + let buffer_htlcs = crate::ln::channel::CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize + 1; + let buffer_tx_fee_sat = + chan_utils::commit_tx_fee_sat(target_feerate, buffer_htlcs, &channel_type_features); + let anchor_value_satoshis = if channel_type_features.supports_anchors_zero_fee_htlc_tx() { + 2 * crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI + } else { + 0 + }; + let channel_reserve_satoshis = 1000; + + let channel_value_sat = 100_000; + let node_0_balance_sat = buffer_tx_fee_sat + anchor_value_satoshis + channel_reserve_satoshis + - if can_afford { 0 } else { 1 }; + let node_1_balance_sat = channel_value_sat - node_0_balance_sat; + + let chan_id = + create_chan_between_nodes_with_value(&nodes[0], &nodes[1], channel_value_sat, 0).3; + + // Set node 0's balance to the can/can't afford threshold + send_payment(&nodes[0], &[&nodes[1]], node_1_balance_sat * 1000); + + { + // Sanity check the reserve + let per_peer_state_lock; + let mut peer_state_lock; + let chan = + get_channel_ref!(nodes[1], nodes[0], per_peer_state_lock, peer_state_lock, chan_id); + assert_eq!( + chan.funding().holder_selected_channel_reserve_satoshis, + channel_reserve_satoshis + ); + } + + { + // Bump the feerate + let mut feerate_lock = chanmon_cfgs[0].fee_estimator.sat_per_kw.lock().unwrap(); + *feerate_lock = target_feerate; + } + + // Put the update fee into the holding cell of node 0 + + nodes[0].node.maybe_update_chan_fees(); + + // While the update_fee is in the holding cell, add an inbound HTLC + + let (route, payment_hash, _, payment_secret) = + get_route_and_payment_hash!(nodes[1], nodes[0], 5000 * 1000); + let onion = RecipientOnionFields::secret_only(payment_secret); + let id = PaymentId(payment_hash.0); + nodes[1].node.send_payment_with_route(route, payment_hash, onion, id).unwrap(); + check_added_monitors(&nodes[1], 1); + + let payment_event = { + let mut events_1 = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events_1.len(), 1); + SendEvent::from_event(events_1.pop().unwrap()) + }; + assert_eq!(payment_event.node_id, node_a_id); + assert_eq!(payment_event.msgs.len(), 1); + + nodes[0].node.handle_update_add_htlc(node_b_id, &payment_event.msgs[0]); + nodes[0].node.handle_commitment_signed(node_b_id, &payment_event.commitment_msg[0]); + check_added_monitors(&nodes[0], 1); + + let (revoke_ack, commitment_signed) = get_revoke_commit_msgs!(nodes[0], node_b_id); + + nodes[1].node.handle_revoke_and_ack(node_a_id, &revoke_ack); + check_added_monitors(&nodes[1], 1); + nodes[1].node.handle_commitment_signed(node_a_id, &commitment_signed[0]); + check_added_monitors(&nodes[1], 1); + + let mut events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + + if let MessageSendEvent::SendRevokeAndACK { node_id, msg } = events.pop().unwrap() { + assert_eq!(node_id, node_a_id); + nodes[0].node.handle_revoke_and_ack(node_b_id, &msg); + check_added_monitors!(nodes[0], 1); + } else { + panic!(); + } + + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + assert!(matches!(events[0], Event::PendingHTLCsForwardable { .. })); + + // Release the update_fee from its holding cell + + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + if can_afford { + // We could afford the update_fee, sanity check everything + assert_eq!(events.len(), 1); + if let MessageSendEvent::UpdateHTLCs { node_id, channel_id, updates } = + events.pop().unwrap() + { + assert_eq!(node_id, node_b_id); + assert_eq!(channel_id, chan_id); + assert_eq!(updates.commitment_signed.len(), 1); + assert_eq!(updates.commitment_signed[0].htlc_signatures.len(), 1); + assert_eq!(updates.update_add_htlcs.len(), 0); + assert_eq!(updates.update_fulfill_htlcs.len(), 0); + assert_eq!(updates.update_fail_htlcs.len(), 0); + assert_eq!(updates.update_fail_malformed_htlcs.len(), 0); + let update_fee = updates.update_fee.unwrap(); + assert_eq!(update_fee.channel_id, chan_id); + assert_eq!(update_fee.feerate_per_kw, target_feerate); + + nodes[1].node.handle_update_fee(node_a_id, &update_fee); + commitment_signed_dance!(nodes[1], nodes[0], updates.commitment_signed, false); + + // Confirm the feerate on node 0's commitment transaction + { + let commitment_tx = get_local_commitment_txn!(nodes[0], channel_id)[0].clone(); + + let mut actual_fee = + commitment_tx.output.iter().fold(0, |acc, output| acc + output.value.to_sat()); + actual_fee = channel_value_sat - actual_fee; + assert_eq!(expected_tx_fee_sat, actual_fee); + } + + // Confirm the feerate on node 1's commitment transaction + { + let commitment_tx = get_local_commitment_txn!(nodes[1], channel_id)[0].clone(); + + let mut actual_fee = + commitment_tx.output.iter().fold(0, |acc, output| acc + output.value.to_sat()); + actual_fee = channel_value_sat - actual_fee; + assert_eq!(expected_tx_fee_sat, actual_fee); + } + } else { + panic!(); + } + } else { + // We could not afford the update_fee, no events should be generated + assert_eq!(events.len(), 0); + let err = format!("Cannot afford to send new feerate at {}", target_feerate); + nodes[0].logger.assert_log("lightning::ln::channel", err, 1); + } +}