// force-close which we should detect as an error).
assert_eq!(msg.contents.flags & 2, 0);
},
- _ => panic!("Unhandled message event {:?}", event),
+ _ => if out.locked_contains(&"Outbound update_fee HTLC buffer overflow - counterparty should force-close this channel") {
+ } else {
+ panic!("Unhandled message event {:?}", event)
+ },
}
if $limit_events != ProcessMessages::AllMessages {
break;
events::MessageSendEvent::SendChannelUpdate { ref msg, .. } => {
assert_eq!(msg.contents.flags & 2, 0); // The disable bit must never be set!
},
- _ => panic!("Unhandled message event"),
+ _ => if out.locked_contains(&"Outbound update_fee HTLC buffer overflow - counterparty should force-close this channel") {
+ } else {
+ panic!("Unhandled message event")
+ },
}
}
push_excess_b_events!(nodes[1].get_and_clear_pending_msg_events().drain(..), Some(0));
events::MessageSendEvent::SendChannelUpdate { ref msg, .. } => {
assert_eq!(msg.contents.flags & 2, 0); // The disable bit must never be set!
},
- _ => panic!("Unhandled message event"),
+ _ => if out.locked_contains(&"Outbound update_fee HTLC buffer overflow - counterparty should force-close this channel") {
+ } else {
+ panic!("Unhandled message event")
+ },
}
}
push_excess_b_events!(nodes[1].get_and_clear_pending_msg_events().drain(..), Some(2));
events::Event::PendingHTLCsForwardable { .. } => {
nodes[$node].process_pending_htlc_forwards();
},
- _ => panic!("Unhandled event"),
+ _ => if out.locked_contains(&"Outbound update_fee HTLC buffer overflow - counterparty should force-close this channel") {
+ } else {
+ panic!("Unhandled event")
+ },
}
}
had_events
}
// Finally, make sure that at least one end of each channel can make a substantial payment.
- assert!(
- send_payment(&nodes[0], &nodes[1], chan_a, 10_000_000, &mut payment_id) ||
- send_payment(&nodes[1], &nodes[0], chan_a, 10_000_000, &mut payment_id));
- assert!(
- send_payment(&nodes[1], &nodes[2], chan_b, 10_000_000, &mut payment_id) ||
- send_payment(&nodes[2], &nodes[1], chan_b, 10_000_000, &mut payment_id));
+ // Unless we expected to hit a limitation of LN state machine (see comment in `send_update_fee_and_commit`)
+ if out.locked_contains(&"Outbound update_fee HTLC buffer overflow - counterparty should force-close this channel") {
+ } else {
+ assert!(
+ send_payment(&nodes[0], &nodes[1], chan_a, 10_000_000, &mut payment_id) ||
+ send_payment(&nodes[1], &nodes[0], chan_a, 10_000_000, &mut payment_id));
+ assert!(
+ send_payment(&nodes[1], &nodes[2], chan_b, 10_000_000, &mut payment_id) ||
+ send_payment(&nodes[2], &nodes[1], chan_b, 10_000_000, &mut payment_id));
+ }
last_htlc_clear_fee_a = fee_est_a.ret_val.load(atomic::Ordering::Acquire);
last_htlc_clear_fee_b = fee_est_b.ret_val.load(atomic::Ordering::Acquire);
pub trait Output : Clone + 'static {
fn locked_write(&self, data: &[u8]);
+ fn locked_contains(&self, pattern: &str) -> bool;
}
#[derive(Clone)]
pub struct DevNull {}
impl Output for DevNull {
fn locked_write(&self, _data: &[u8]) {}
+ fn locked_contains(&self, _pattern: &str) -> bool {
+ false
+ }
}
#[derive(Clone)]
pub struct StringBuffer(Arc<Mutex<String>>);
fn locked_write(&self, data: &[u8]) {
self.0.lock().unwrap().push_str(&String::from_utf8(data.to_vec()).unwrap());
}
+ fn locked_contains(&self, pattern: &str) -> bool {
+ self.0.lock().unwrap().contains(pattern)
+ }
}
impl StringBuffer {
pub fn new() -> Self {
pub counterparty_dust_limit_msat: u64,
}
-#[derive(Clone, Copy, PartialEq)]
+#[derive(Debug, Clone, Copy, PartialEq)]
enum FeeUpdateState {
// Inbound states mirroring InboundHTLCState
RemoteAnnounced,
/// Adds a pending update to this channel. See the doc for send_htlc for
/// further details on the optionness of the return value.
+ /// If our balance is too low to cover the cost of the next commitment transaction at the
+ /// new feerate, the update is cancelled.
/// You MUST call send_commitment prior to any other calls on this Channel
- fn send_update_fee(&mut self, feerate_per_kw: u32) -> Option<msgs::UpdateFee> {
+ fn send_update_fee<L: Deref>(&mut self, feerate_per_kw: u32, logger: &L) -> Option<msgs::UpdateFee> where L::Target: Logger {
if !self.is_outbound() {
panic!("Cannot send fee from inbound channel");
}
panic!("Cannot update fee while peer is disconnected/we're awaiting a monitor update (ChannelManager should have caught this)");
}
+ // Before proposing a feerate update, check that we can actually afford the new fee.
+ let inbound_stats = self.get_inbound_pending_htlc_stats();
+ let outbound_stats = self.get_outbound_pending_htlc_stats();
+ // In case of a concurrent update_add_htlc proposed by our counterparty, we might
+ // not have enough balance value remaining to cover the onchain cost of this new
+ // HTLC weight. If this happens, our counterparty fails the reception of our
+ // commitment_signed including this new HTLC due to infringement on the channel
+ // reserve.
+ // To prevent this case, we compute our outbound update_fee with an HTLC buffer of
+ // size 2. However, if the number of concurrent update_add_htlc is higher, this still
+ // leads to a channel force-close. Ultimately, this is an issue coming from the
+ // design of LN state machines, allowing asynchronous updates.
+ let total_fee_sat = Channel::<Signer>::commit_tx_fee_sat(feerate_per_kw, (inbound_stats.pending_htlcs + /* HTLC feerate buffer */ 2 + outbound_stats.pending_htlcs) as usize);
+ let holder_balance_after_fee_sub = (self.value_to_self_msat / 1000).checked_sub(total_fee_sat).map(|a| a.checked_sub(outbound_stats.pending_htlcs_value_msat / 1000)).unwrap_or(None).unwrap_or(u64::min_value());
+ if holder_balance_after_fee_sub < self.counterparty_selected_channel_reserve_satoshis.unwrap() {
+ //TODO: auto-close after a number of failures?
+ log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw);
+ return None;
+ }
+
if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateFailed as u32)) != 0 {
self.holding_cell_update_fee = Some(feerate_per_kw);
return None;
}
pub fn send_update_fee_and_commit<L: Deref>(&mut self, feerate_per_kw: u32, logger: &L) -> Result<Option<(msgs::UpdateFee, msgs::CommitmentSigned, ChannelMonitorUpdate)>, ChannelError> where L::Target: Logger {
- match self.send_update_fee(feerate_per_kw) {
+ match self.send_update_fee(feerate_per_kw, logger) {
Some(update_fee) => {
let (commitment_signed, monitor_update) = self.send_commitment_no_status_check(logger)?;
Ok(Some((update_fee, commitment_signed, monitor_update)))
}
}
}
+ if self.is_outbound() {
+ // Log if we can't afford next remote commitment tx fee at pending outbound feerate update.
+ if let Some(pending_feerate) = self.pending_update_fee {
+ assert_eq!(pending_feerate.1, FeeUpdateState::Outbound);
+ let next_total_fee_sat = Channel::<Signer>::commit_tx_fee_sat(pending_feerate.0, self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len());
+ let outbound_stats = self.get_outbound_pending_htlc_stats();
+ let holder_balance_after_fee_sub_sats = (self.value_to_self_msat / 1000).checked_sub(next_total_fee_sat).map(|a| a.checked_sub(outbound_stats.pending_htlcs_value_msat / 1000)).unwrap_or(None).unwrap_or(u64::min_value());
+ if holder_balance_after_fee_sub_sats < self.counterparty_selected_channel_reserve_satoshis.unwrap() {
+ log_debug!(logger, "Outbound update_fee HTLC buffer overflow - counterparty should force-close this channel");
+ }
+ }
+ }
{
let mut htlcs = Vec::with_capacity(counterparty_commitment_tx.3.len());
use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA};
use ln::channel::{Channel, ChannelError};
use ln::{chan_utils, onion_utils};
-use ln::chan_utils::HTLC_SUCCESS_TX_WEIGHT;
+use ln::chan_utils::{HTLC_SUCCESS_TX_WEIGHT, HTLCOutputInCommitment};
use routing::network_graph::{NetworkUpdate, RoutingFees};
use routing::router::{Route, RouteHop, RouteHint, RouteHintHop, get_route, get_keysend_route};
use routing::scorer::Scorer;
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
- let channel_value = 1888;
+ let channel_value = 1977;
let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, channel_value, 700000, InitFeatures::known(), InitFeatures::known());
let channel_id = chan.2;
+ let secp_ctx = Secp256k1::new();
let feerate = 260;
{
*feerate_lock = feerate + 2;
}
nodes[0].node.timer_tick_occurred();
- check_added_monitors!(nodes[0], 1);
+ nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot afford to send new feerate at {}", feerate + 2), 1);
+ check_added_monitors!(nodes[0], 0);
- let update2_msg = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+ const INITIAL_COMMITMENT_NUMBER: u64 = 281474976710654;
- nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), &update2_msg.update_fee.unwrap());
+ // Get the EnforcingSigner for each channel, which will be used to (1) get the keys
+ // needed to sign the new commitment tx and (2) sign the new commitment tx.
+ let (local_revocation_basepoint, local_htlc_basepoint, local_funding) = {
+ let chan_lock = nodes[0].node.channel_state.lock().unwrap();
+ let local_chan = chan_lock.by_id.get(&chan.2).unwrap();
+ let chan_signer = local_chan.get_signer();
+ let pubkeys = chan_signer.pubkeys();
+ (pubkeys.revocation_basepoint, pubkeys.htlc_basepoint,
+ pubkeys.funding_pubkey)
+ };
+ let (remote_delayed_payment_basepoint, remote_htlc_basepoint,remote_point, remote_funding) = {
+ let chan_lock = nodes[1].node.channel_state.lock().unwrap();
+ let remote_chan = chan_lock.by_id.get(&chan.2).unwrap();
+ let chan_signer = remote_chan.get_signer();
+ let pubkeys = chan_signer.pubkeys();
+ (pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint,
+ chan_signer.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx),
+ pubkeys.funding_pubkey)
+ };
+
+ // Assemble the set of keys we can use for signatures for our commitment_signed message.
+ let commit_tx_keys = chan_utils::TxCreationKeys::derive_new(&secp_ctx, &remote_point, &remote_delayed_payment_basepoint,
+ &remote_htlc_basepoint, &local_revocation_basepoint, &local_htlc_basepoint).unwrap();
+
+ let res = {
+ let local_chan_lock = nodes[0].node.channel_state.lock().unwrap();
+ let local_chan = local_chan_lock.by_id.get(&chan.2).unwrap();
+ let local_chan_signer = local_chan.get_signer();
+ let mut htlcs: Vec<(HTLCOutputInCommitment, ())> = vec![];
+ let commitment_tx = CommitmentTransaction::new_with_auxiliary_htlc_data(
+ INITIAL_COMMITMENT_NUMBER - 1,
+ 700,
+ 999,
+ false, local_funding, remote_funding,
+ commit_tx_keys.clone(),
+ feerate + 124,
+ &mut htlcs,
+ &local_chan.channel_transaction_parameters.as_counterparty_broadcastable()
+ );
+ local_chan_signer.sign_counterparty_commitment(&commitment_tx, &secp_ctx).unwrap()
+ };
+
+ let commit_signed_msg = msgs::CommitmentSigned {
+ channel_id: chan.2,
+ signature: res.0,
+ htlc_signatures: res.1
+ };
+
+ let update_fee = msgs::UpdateFee {
+ channel_id: chan.2,
+ feerate_per_kw: feerate + 124,
+ };
+
+ nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), &update_fee);
//While producing the commitment_signed response after handling a received update_fee request the
//check to see if the funder, who sent the update_fee request, can afford the new fee (funder_balance >= fee+channel_reserve)
//Should produce and error.
- nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &update2_msg.commitment_signed);
+ nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &commit_signed_msg);
nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Funding remote cannot afford proposed new fee".to_string(), 1);
check_added_monitors!(nodes[1], 1);
check_closed_broadcast!(nodes[1], true);