diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index f8c6c08a2ba..02c8450fa62 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -285,7 +285,7 @@ impl KeyProvider { } #[inline] -fn check_api_err(api_err: APIError) { +fn check_api_err(api_err: APIError, sendable_bounds_violated: bool) { match api_err { APIError::APIMisuseError { .. } => panic!("We can't misuse the API"), APIError::FeeRateTooHigh { .. } => panic!("We can't send too much fee?"), @@ -296,15 +296,11 @@ fn check_api_err(api_err: APIError) { // is probably just stale and you should add new messages here. match err.as_str() { "Peer for first hop currently disconnected" => {}, - _ if err.starts_with("Cannot push more than their max accepted HTLCs ") => {}, - _ if err.starts_with("Cannot send value that would put us over the max HTLC value in flight our peer will accept ") => {}, - _ if err.starts_with("Cannot send value that would put our balance under counterparty-announced channel reserve value") => {}, - _ if err.starts_with("Cannot send value that would put counterparty balance under holder-announced channel reserve value") => {}, - _ if err.starts_with("Cannot send value that would overdraw remaining funds.") => {}, - _ if err.starts_with("Cannot send value that would not leave enough to pay for fees.") => {}, - _ if err.starts_with("Cannot send value that would put our exposure to dust HTLCs at") => {}, + _ if err.starts_with("Cannot send less than our next-HTLC minimum - ") => {}, + _ if err.starts_with("Cannot send more than our next-HTLC maximum - ") => {}, _ => panic!("{}", err), } + assert!(sendable_bounds_violated); }, APIError::MonitorUpdateInProgress => { // We can (obviously) temp-fail a monitor update @@ -313,17 +309,17 @@ fn check_api_err(api_err: APIError) { } } #[inline] -fn check_payment_err(send_err: PaymentSendFailure) { +fn check_payment_err(send_err: PaymentSendFailure, sendable_bounds_violated: bool) { match send_err { - PaymentSendFailure::ParameterError(api_err) => check_api_err(api_err), + PaymentSendFailure::ParameterError(api_err) => check_api_err(api_err, sendable_bounds_violated), PaymentSendFailure::PathParameterError(per_path_results) => { - for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } } + for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err, sendable_bounds_violated); } } }, PaymentSendFailure::AllFailedResendSafe(per_path_results) => { - for api_err in per_path_results { check_api_err(api_err); } + for api_err in per_path_results { check_api_err(api_err, sendable_bounds_violated); } }, PaymentSendFailure::PartialFailure { results, .. } => { - for res in results { if let Err(api_err) = res { check_api_err(api_err); } } + for res in results { if let Err(api_err) = res { check_api_err(api_err, sendable_bounds_violated); } } }, PaymentSendFailure::DuplicatePayment => panic!(), } @@ -351,6 +347,11 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p let mut payment_id = [0; 32]; payment_id[0..8].copy_from_slice(&payment_idx.to_ne_bytes()); *payment_idx += 1; + let (min_value_sendable, max_value_sendable) = source.list_usable_channels() + .iter().find(|chan| chan.short_channel_id == Some(dest_chan_id)) + .map(|chan| + (chan.next_outbound_htlc_minimum_msat, chan.next_outbound_htlc_limit_msat)) + .unwrap_or((0, 0)); if let Err(err) = source.send_payment_with_route(&Route { paths: vec![Path { hops: vec![RouteHop { pubkey: dest.get_our_node_id(), @@ -362,9 +363,15 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p }], blinded_tail: None }], payment_params: None, }, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_id)) { - check_payment_err(err); + check_payment_err(err, amt > max_value_sendable || amt < min_value_sendable); false - } else { true } + } else { + // Note that while the max is a strict upper-bound, we can occasionally send substantially + // below the minimum, with some gap which is unusable immediately below the minimum. Thus, + // we don't check against min_value_sendable here. + assert!(amt <= max_value_sendable); + true + } } #[inline] fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, dest: &ChanMan, dest_chan_id: u64, amt: u64, payment_id: &mut u8, payment_idx: &mut u64) -> bool { @@ -373,13 +380,19 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des let mut payment_id = [0; 32]; payment_id[0..8].copy_from_slice(&payment_idx.to_ne_bytes()); *payment_idx += 1; + let (min_value_sendable, max_value_sendable) = source.list_usable_channels() + .iter().find(|chan| chan.short_channel_id == Some(middle_chan_id)) + .map(|chan| + (chan.next_outbound_htlc_minimum_msat, chan.next_outbound_htlc_limit_msat)) + .unwrap_or((0, 0)); + let first_hop_fee = 50_000; if let Err(err) = source.send_payment_with_route(&Route { paths: vec![Path { hops: vec![RouteHop { pubkey: middle.get_our_node_id(), node_features: middle.node_features(), short_channel_id: middle_chan_id, channel_features: middle.channel_features(), - fee_msat: 50000, + fee_msat: first_hop_fee, cltv_expiry_delta: 100, },RouteHop { pubkey: dest.get_our_node_id(), @@ -391,9 +404,16 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des }], blinded_tail: None }], payment_params: None, }, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_id)) { - check_payment_err(err); + let sent_amt = amt + first_hop_fee; + check_payment_err(err, sent_amt < min_value_sendable || sent_amt > max_value_sendable); false - } else { true } + } else { + // Note that while the max is a strict upper-bound, we can occasionally send substantially + // below the minimum, with some gap which is unusable immediately below the minimum. Thus, + // we don't check against min_value_sendable here. + assert!(amt + first_hop_fee <= max_value_sendable); + true + } } #[inline] diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 00c53dfe58e..72935f153ea 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -265,6 +265,7 @@ pub fn do_test(data: &[u8], out: Out) { balance_msat: 0, outbound_capacity_msat: capacity.saturating_mul(1000), next_outbound_htlc_limit_msat: capacity.saturating_mul(1000), + next_outbound_htlc_minimum_msat: 0, inbound_htlc_minimum_msat: None, inbound_htlc_maximum_msat: None, config: None, diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 11f0261d677..32750d048ca 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -73,6 +73,8 @@ pub struct AvailableBalances { pub outbound_capacity_msat: u64, /// The maximum value we can assign to the next outbound HTLC pub next_outbound_htlc_limit_msat: u64, + /// The minimum value we can assign to the next outbound HTLC + pub next_outbound_htlc_minimum_msat: u64, } #[derive(Debug, Clone, Copy, PartialEq)] @@ -2676,6 +2678,7 @@ impl Channel { /// corner case properly. pub fn get_available_balances(&self) -> AvailableBalances { // Note that we have to handle overflow due to the above case. + let inbound_stats = self.get_inbound_pending_htlc_stats(None); let outbound_stats = self.get_outbound_pending_htlc_stats(None); let mut balance_msat = self.value_to_self_msat; @@ -2686,10 +2689,112 @@ impl Channel { } balance_msat -= outbound_stats.pending_htlcs_value_msat; - let outbound_capacity_msat = cmp::max(self.value_to_self_msat as i64 - - outbound_stats.pending_htlcs_value_msat as i64 - - self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000, - 0) as u64; + let outbound_capacity_msat = self.value_to_self_msat + .saturating_sub(outbound_stats.pending_htlcs_value_msat) + .saturating_sub( + self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) * 1000); + + let mut available_capacity_msat = outbound_capacity_msat; + + if self.is_outbound() { + // We should mind channel commit tx fee when computing how much of the available capacity + // can be used in the next htlc. Mirrors the logic in send_htlc. + // + // The fee depends on whether the amount we will be sending is above dust or not, + // and the answer will in turn change the amount itself — making it a circular + // dependency. + // This complicates the computation around dust-values, up to the one-htlc-value. + let mut real_dust_limit_timeout_sat = self.holder_dust_limit_satoshis; + if !self.opt_anchors() { + real_dust_limit_timeout_sat += self.feerate_per_kw as u64 * htlc_timeout_tx_weight(false) / 1000; + } + + let htlc_above_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000, HTLCInitiator::LocalOffered); + let max_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * self.next_local_commit_tx_fee_msat(htlc_above_dust, Some(())); + let htlc_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000 - 1, HTLCInitiator::LocalOffered); + let min_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * self.next_local_commit_tx_fee_msat(htlc_dust, Some(())); + + // We will first subtract the fee as if we were above-dust. Then, if the resulting + // value ends up being below dust, we have this fee available again. In that case, + // match the value to right-below-dust. + let mut capacity_minus_commitment_fee_msat: i64 = (available_capacity_msat as i64) - (max_reserved_commit_tx_fee_msat as i64); + if capacity_minus_commitment_fee_msat < (real_dust_limit_timeout_sat as i64) * 1000 { + let one_htlc_difference_msat = max_reserved_commit_tx_fee_msat - min_reserved_commit_tx_fee_msat; + debug_assert!(one_htlc_difference_msat != 0); + capacity_minus_commitment_fee_msat += one_htlc_difference_msat as i64; + capacity_minus_commitment_fee_msat = cmp::min(real_dust_limit_timeout_sat as i64 * 1000 - 1, capacity_minus_commitment_fee_msat); + available_capacity_msat = cmp::max(0, cmp::min(capacity_minus_commitment_fee_msat, available_capacity_msat as i64)) as u64; + } else { + available_capacity_msat = capacity_minus_commitment_fee_msat as u64; + } + } else { + // If the channel is inbound (i.e. counterparty pays the fee), we need to make sure + // sending a new HTLC won't reduce their balance below our reserve threshold. + let mut real_dust_limit_success_sat = self.counterparty_dust_limit_satoshis; + if !self.opt_anchors() { + real_dust_limit_success_sat += self.feerate_per_kw as u64 * htlc_success_tx_weight(false) / 1000; + } + + let htlc_above_dust = HTLCCandidate::new(real_dust_limit_success_sat * 1000, HTLCInitiator::LocalOffered); + let max_reserved_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(htlc_above_dust, None); + + let holder_selected_chan_reserve_msat = self.holder_selected_channel_reserve_satoshis * 1000; + let remote_balance_msat = (self.channel_value_satoshis * 1000 - self.value_to_self_msat) + .saturating_sub(inbound_stats.pending_htlcs_value_msat); + + if remote_balance_msat < max_reserved_commit_tx_fee_msat + holder_selected_chan_reserve_msat { + // If another HTLC's fee would reduce the remote's balance below the reserve limit + // we've selected for them, we can only send dust HTLCs. + available_capacity_msat = cmp::min(available_capacity_msat, real_dust_limit_success_sat * 1000 - 1); + } + } + + let mut next_outbound_htlc_minimum_msat = self.counterparty_htlc_minimum_msat; + + // If we get close to our maximum dust exposure, we end up in a situation where we can send + // between zero and the remaining dust exposure limit remaining OR above the dust limit. + // Because we cannot express this as a simple min/max, we prefer to tell the user they can + // send above the dust limit (as the router can always overpay to meet the dust limit). + let mut remaining_msat_below_dust_exposure_limit = None; + let mut dust_exposure_dust_limit_msat = 0; + + let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if self.opt_anchors() { + (self.counterparty_dust_limit_satoshis, self.holder_dust_limit_satoshis) + } else { + let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64; + (self.counterparty_dust_limit_satoshis + dust_buffer_feerate * htlc_success_tx_weight(false) / 1000, + self.holder_dust_limit_satoshis + dust_buffer_feerate * htlc_timeout_tx_weight(false) / 1000) + }; + let on_counterparty_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat; + if on_counterparty_dust_htlc_exposure_msat as i64 + htlc_success_dust_limit as i64 * 1000 - 1 > self.get_max_dust_htlc_exposure_msat() as i64 { + remaining_msat_below_dust_exposure_limit = + Some(self.get_max_dust_htlc_exposure_msat().saturating_sub(on_counterparty_dust_htlc_exposure_msat)); + dust_exposure_dust_limit_msat = cmp::max(dust_exposure_dust_limit_msat, htlc_success_dust_limit * 1000); + } + + let on_holder_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat; + if on_holder_dust_htlc_exposure_msat as i64 + htlc_timeout_dust_limit as i64 * 1000 - 1 > self.get_max_dust_htlc_exposure_msat() as i64 { + remaining_msat_below_dust_exposure_limit = Some(cmp::min( + remaining_msat_below_dust_exposure_limit.unwrap_or(u64::max_value()), + self.get_max_dust_htlc_exposure_msat().saturating_sub(on_holder_dust_htlc_exposure_msat))); + dust_exposure_dust_limit_msat = cmp::max(dust_exposure_dust_limit_msat, htlc_timeout_dust_limit * 1000); + } + + if let Some(remaining_limit_msat) = remaining_msat_below_dust_exposure_limit { + if available_capacity_msat < dust_exposure_dust_limit_msat { + available_capacity_msat = cmp::min(available_capacity_msat, remaining_limit_msat); + } else { + next_outbound_htlc_minimum_msat = cmp::max(next_outbound_htlc_minimum_msat, dust_exposure_dust_limit_msat); + } + } + + available_capacity_msat = cmp::min(available_capacity_msat, + self.counterparty_max_htlc_value_in_flight_msat - outbound_stats.pending_htlcs_value_msat); + + if outbound_stats.pending_htlcs + 1 > self.counterparty_max_accepted_htlcs as u32 { + available_capacity_msat = 0; + } + AvailableBalances { inbound_capacity_msat: cmp::max(self.channel_value_satoshis as i64 * 1000 - self.value_to_self_msat as i64 @@ -2697,10 +2802,8 @@ impl Channel { - self.holder_selected_channel_reserve_satoshis as i64 * 1000, 0) as u64, outbound_capacity_msat, - next_outbound_htlc_limit_msat: cmp::max(cmp::min(outbound_capacity_msat as i64, - self.counterparty_max_htlc_value_in_flight_msat as i64 - - outbound_stats.pending_htlcs_value_msat as i64), - 0) as u64, + next_outbound_htlc_limit_msat: available_capacity_msat, + next_outbound_htlc_minimum_msat, balance_msat, } } @@ -2724,10 +2827,16 @@ impl Channel { feerate_per_kw as u64 * (commitment_tx_base_weight(opt_anchors) + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000 } - // Get the commitment tx fee for the local's (i.e. our) next commitment transaction based on the - // number of pending HTLCs that are on track to be in our next commitment tx, plus an additional - // HTLC if `fee_spike_buffer_htlc` is Some, plus a new HTLC given by `new_htlc_amount`. Dust HTLCs - // are excluded. + /// Get the commitment tx fee for the local's (i.e. our) next commitment transaction based on the + /// number of pending HTLCs that are on track to be in our next commitment tx. + /// + /// Optionally includes the `HTLCCandidate` given by `htlc` and an additional non-dust HTLC if + /// `fee_spike_buffer_htlc` is `Some`. + /// + /// The first extra HTLC is useful for determining whether we can accept a further HTLC, the + /// second allows for creating a buffer to ensure a further HTLC can always be accepted/added. + /// + /// Dust HTLCs are excluded. fn next_local_commit_tx_fee_msat(&self, htlc: HTLCCandidate, fee_spike_buffer_htlc: Option<()>) -> u64 { assert!(self.is_outbound()); @@ -2821,10 +2930,16 @@ impl Channel { res } - // Get the commitment tx fee for the remote's next commitment transaction based on the number of - // pending HTLCs that are on track to be in their next commitment tx, plus an additional HTLC if - // `fee_spike_buffer_htlc` is Some, plus a new HTLC given by `new_htlc_amount`. Dust HTLCs are - // excluded. + /// Get the commitment tx fee for the remote's next commitment transaction based on the number of + /// pending HTLCs that are on track to be in their next commitment tx + /// + /// Optionally includes the `HTLCCandidate` given by `htlc` and an additional non-dust HTLC if + /// `fee_spike_buffer_htlc` is `Some`. + /// + /// The first extra HTLC is useful for determining whether we can accept a further HTLC, the + /// second allows for creating a buffer to ensure a further HTLC can always be accepted/added. + /// + /// Dust HTLCs are excluded. fn next_remote_commit_tx_fee_msat(&self, htlc: HTLCCandidate, fee_spike_buffer_htlc: Option<()>) -> u64 { assert!(!self.is_outbound()); @@ -5833,8 +5948,15 @@ impl Channel { return Err(ChannelError::Ignore("Cannot send 0-msat HTLC".to_owned())); } - if amount_msat < self.counterparty_htlc_minimum_msat { - return Err(ChannelError::Ignore(format!("Cannot send less than their minimum HTLC value ({})", self.counterparty_htlc_minimum_msat))); + let available_balances = self.get_available_balances(); + if amount_msat < available_balances.next_outbound_htlc_minimum_msat { + return Err(ChannelError::Ignore(format!("Cannot send less than our next-HTLC minimum - {} msat", + available_balances.next_outbound_htlc_minimum_msat))); + } + + if amount_msat > available_balances.next_outbound_htlc_limit_msat { + return Err(ChannelError::Ignore(format!("Cannot send more than our next-HTLC maximum - {} msat", + available_balances.next_outbound_htlc_limit_msat))); } if (self.channel_state & (ChannelState::PeerDisconnected as u32)) != 0 { @@ -5847,75 +5969,13 @@ impl Channel { return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected from channel counterparty".to_owned())); } - let inbound_stats = self.get_inbound_pending_htlc_stats(None); - let outbound_stats = self.get_outbound_pending_htlc_stats(None); - if outbound_stats.pending_htlcs + 1 > self.counterparty_max_accepted_htlcs as u32 { - return Err(ChannelError::Ignore(format!("Cannot push more than their max accepted HTLCs ({})", self.counterparty_max_accepted_htlcs))); - } - // Check their_max_htlc_value_in_flight_msat - if outbound_stats.pending_htlcs_value_msat + amount_msat > self.counterparty_max_htlc_value_in_flight_msat { - return Err(ChannelError::Ignore(format!("Cannot send value that would put us over the max HTLC value in flight our peer will accept ({})", self.counterparty_max_htlc_value_in_flight_msat))); - } + let need_holding_cell = (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateInProgress as u32)) != 0; + log_debug!(logger, "Pushing new outbound HTLC for {} msat {}", amount_msat, + if force_holding_cell { "into holding cell" } + else if need_holding_cell { "into holding cell as we're awaiting an RAA or monitor" } + else { "to peer" }); - let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number); - let commitment_stats = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, true, logger); - if !self.is_outbound() { - // Check that we won't violate the remote channel reserve by adding this HTLC. - let htlc_candidate = HTLCCandidate::new(amount_msat, HTLCInitiator::LocalOffered); - let counterparty_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(htlc_candidate, None); - let holder_selected_chan_reserve_msat = self.holder_selected_channel_reserve_satoshis * 1000; - if commitment_stats.remote_balance_msat < counterparty_commit_tx_fee_msat + holder_selected_chan_reserve_msat { - return Err(ChannelError::Ignore("Cannot send value that would put counterparty balance under holder-announced channel reserve value".to_owned())); - } - } - - let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if self.opt_anchors() { - (0, 0) - } else { - let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64; - (dust_buffer_feerate * htlc_success_tx_weight(false) / 1000, - dust_buffer_feerate * htlc_timeout_tx_weight(false) / 1000) - }; - let exposure_dust_limit_success_sats = htlc_success_dust_limit + self.counterparty_dust_limit_satoshis; - if amount_msat / 1000 < exposure_dust_limit_success_sats { - let on_counterparty_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat + amount_msat; - if on_counterparty_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { - return Err(ChannelError::Ignore(format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", - on_counterparty_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()))); - } - } - - let exposure_dust_limit_timeout_sats = htlc_timeout_dust_limit + self.holder_dust_limit_satoshis; - if amount_msat / 1000 < exposure_dust_limit_timeout_sats { - let on_holder_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat + amount_msat; - if on_holder_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { - return Err(ChannelError::Ignore(format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", - on_holder_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()))); - } - } - - let holder_balance_msat = commitment_stats.local_balance_msat - outbound_stats.holding_cell_msat; - if holder_balance_msat < amount_msat { - return Err(ChannelError::Ignore(format!("Cannot send value that would overdraw remaining funds. Amount: {}, pending value to self {}", amount_msat, holder_balance_msat))); - } - - // `2 *` and extra HTLC are for the fee spike buffer. - let commit_tx_fee_msat = if self.is_outbound() { - let htlc_candidate = HTLCCandidate::new(amount_msat, HTLCInitiator::LocalOffered); - FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * self.next_local_commit_tx_fee_msat(htlc_candidate, Some(())) - } else { 0 }; - if holder_balance_msat - amount_msat < commit_tx_fee_msat { - return Err(ChannelError::Ignore(format!("Cannot send value that would not leave enough to pay for fees. Pending value to self: {}. local_commit_tx_fee {}", holder_balance_msat, commit_tx_fee_msat))); - } - - // Check self.counterparty_selected_channel_reserve_satoshis (the amount we must keep as - // reserve for the remote to have something to claim if we misbehave) - let chan_reserve_msat = self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000; - if holder_balance_msat - amount_msat - commit_tx_fee_msat < chan_reserve_msat { - return Err(ChannelError::Ignore(format!("Cannot send value that would put our balance under counterparty-announced channel reserve value ({})", chan_reserve_msat))); - } - - if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateInProgress as u32)) != 0 { + if need_holding_cell { force_holding_cell = true; } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 30eab9066fd..b5e7d603411 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1274,8 +1274,14 @@ pub struct ChannelDetails { /// the current state and per-HTLC limit(s). This is intended for use when routing, allowing us /// to use a limit as close as possible to the HTLC limit we can currently send. /// - /// See also [`ChannelDetails::balance_msat`] and [`ChannelDetails::outbound_capacity_msat`]. + /// See also [`ChannelDetails::next_outbound_htlc_minimum_msat`], + /// [`ChannelDetails::balance_msat`], and [`ChannelDetails::outbound_capacity_msat`]. pub next_outbound_htlc_limit_msat: u64, + /// The minimum value for sending a single HTLC to the remote peer. This is the equivalent of + /// [`ChannelDetails::next_outbound_htlc_limit_msat`] but represents a lower-bound, rather than + /// an upper-bound. This is intended for use when routing, allowing us to ensure we pick a + /// route which is valid. + pub next_outbound_htlc_minimum_msat: u64, /// The available inbound capacity for the remote peer to send HTLCs to us. This does not /// include any pending HTLCs which are not yet fully resolved (and, thus, whose balance is not /// available for inclusion in new inbound HTLCs). @@ -1395,6 +1401,7 @@ impl ChannelDetails { inbound_capacity_msat: balance.inbound_capacity_msat, outbound_capacity_msat: balance.outbound_capacity_msat, next_outbound_htlc_limit_msat: balance.next_outbound_htlc_limit_msat, + next_outbound_htlc_minimum_msat: balance.next_outbound_htlc_minimum_msat, user_channel_id: channel.get_user_id(), confirmations_required: channel.minimum_depth(), confirmations: Some(channel.get_funding_tx_confirmations(best_block_height)), @@ -6951,10 +6958,9 @@ impl Writeable for ChannelDetails { (14, user_channel_id_low, required), (16, self.balance_msat, required), (18, self.outbound_capacity_msat, required), - // Note that by the time we get past the required read above, outbound_capacity_msat will be - // filled in, so we can safely unwrap it here. - (19, self.next_outbound_htlc_limit_msat, (default_value, outbound_capacity_msat.0.unwrap() as u64)), + (19, self.next_outbound_htlc_limit_msat, required), (20, self.inbound_capacity_msat, required), + (21, self.next_outbound_htlc_minimum_msat, required), (22, self.confirmations_required, option), (24, self.force_close_spend_delay, option), (26, self.is_outbound, required), @@ -6991,6 +6997,7 @@ impl Readable for ChannelDetails { // filled in, so we can safely unwrap it here. (19, next_outbound_htlc_limit_msat, (default_value, outbound_capacity_msat.0.unwrap() as u64)), (20, inbound_capacity_msat, required), + (21, next_outbound_htlc_minimum_msat, (default_value, 0)), (22, confirmations_required, option), (24, force_close_spend_delay, option), (26, is_outbound, required), @@ -7024,6 +7031,7 @@ impl Readable for ChannelDetails { balance_msat: balance_msat.0.unwrap(), outbound_capacity_msat: outbound_capacity_msat.0.unwrap(), next_outbound_htlc_limit_msat: next_outbound_htlc_limit_msat.0.unwrap(), + next_outbound_htlc_minimum_msat: next_outbound_htlc_minimum_msat.0.unwrap(), inbound_capacity_msat: inbound_capacity_msat.0.unwrap(), confirmations_required, confirmations, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 98fc4bd95b6..ec0068f3194 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -1102,6 +1102,9 @@ fn holding_cell_htlc_counting() { create_announced_chan_between_nodes(&nodes, 0, 1); let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2); + // Fetch a route in advance as we will be unable to once we're unable to send. + let (route, payment_hash_1, _, payment_secret_1) = get_route_and_payment_hash!(nodes[1], nodes[2], 100000); + let mut payments = Vec::new(); for _ in 0..50 { let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[2], 100000); @@ -1119,14 +1122,11 @@ fn holding_cell_htlc_counting() { // There is now one HTLC in an outbound commitment transaction and (OUR_MAX_HTLCS - 1) HTLCs in // the holding cell waiting on B's RAA to send. At this point we should not be able to add // another HTLC. - let (route, payment_hash_1, _, payment_secret_1) = get_route_and_payment_hash!(nodes[1], nodes[2], 100000); { unwrap_send_err!(nodes[1].node.send_payment_with_route(&route, payment_hash_1, RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert!(regex::Regex::new(r"Cannot push more than their max accepted HTLCs \(\d+\)").unwrap().is_match(err))); + ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); - nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot push more than their max accepted HTLCs", 1); } // This should also be true if we try to forward a payment. @@ -1342,21 +1342,19 @@ fn test_basic_channel_reserve() { // The 2* and +1 are for the fee spike reserve. let commit_tx_fee = 2 * commit_tx_fee_msat(get_feerate!(nodes[0], nodes[1], chan.2), 1 + 1, get_opt_anchors!(nodes[0], nodes[1], chan.2)); let max_can_send = 5000000 - channel_reserve - commit_tx_fee; - let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send + 1); + let (mut route, our_payment_hash, _, our_payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[1], max_can_send); + route.paths[0].hops.last_mut().unwrap().fee_msat += 1; let err = nodes[0].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0)).err().unwrap(); match err { PaymentSendFailure::AllFailedResendSafe(ref fails) => { - match &fails[0] { - &APIError::ChannelUnavailable{ref err} => - assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err)), - _ => panic!("Unexpected error variant"), - } + if let &APIError::ChannelUnavailable { .. } = &fails[0] {} + else { panic!("Unexpected error variant"); } }, _ => panic!("Unexpected error variant"), } assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send value that would put our balance under counterparty-announced channel reserve value", 1); send_payment(&nodes[0], &vec![&nodes[1]], max_can_send); } @@ -1369,7 +1367,9 @@ fn test_fee_spike_violation_fails_htlc() { let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000); - let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 3460001); + let (mut route, payment_hash, _, payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[1], 3460000); + route.paths[0].hops[0].fee_msat += 1; // Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc() let secp_ctx = Secp256k1::new(); let session_priv = SecretKey::from_slice(&[42; 32]).expect("RNG is bad!"); @@ -1521,19 +1521,18 @@ fn test_chan_reserve_violation_outbound_htlc_inbound_chan() { let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, push_amt); + // Fetch a route in advance as we will be unable to once we're unable to send. + let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 1_000_000); // Sending exactly enough to hit the reserve amount should be accepted for _ in 0..MIN_AFFORDABLE_HTLC_COUNT { let (_, _, _) = route_payment(&nodes[1], &[&nodes[0]], 1_000_000); } // However one more HTLC should be significantly over the reserve amount and fail. - let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 1_000_000); unwrap_send_err!(nodes[1].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert_eq!(err, "Cannot send value that would put counterparty balance under holder-announced channel reserve value")); + ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); - nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put counterparty balance under holder-announced channel reserve value".to_string(), 1); } #[test] @@ -1559,7 +1558,9 @@ fn test_chan_reserve_violation_inbound_htlc_outbound_channel() { let (_, _, _) = route_payment(&nodes[1], &[&nodes[0]], 1_000_000); } - let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 700_000); + let (mut route, payment_hash, _, payment_secret) = + get_route_and_payment_hash!(nodes[1], nodes[0], 1000); + route.paths[0].hops[0].fee_msat = 700_000; // Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc() let secp_ctx = Secp256k1::new(); let session_priv = SecretKey::from_slice(&[42; 32]).unwrap(); @@ -1621,11 +1622,12 @@ fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() { } // One more than the dust amt should fail, however. - let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], dust_amt + 1); + let (mut route, our_payment_hash, _, our_payment_secret) = + get_route_and_payment_hash!(nodes[1], nodes[0], dust_amt); + route.paths[0].hops[0].fee_msat += 1; unwrap_send_err!(nodes[1].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert_eq!(err, "Cannot send value that would put counterparty balance under holder-announced channel reserve value")); + ), true, APIError::ChannelUnavailable { .. }, {}); } #[test] @@ -1732,7 +1734,8 @@ fn test_chan_reserve_violation_inbound_htlc_inbound_chan() { let commit_tx_fee_2_htlcs = commit_tx_fee_msat(feerate, 2, opt_anchors); let recv_value_2 = chan_stat.value_to_self_msat - amt_msat_1 - chan_stat.channel_reserve_msat - total_routing_fee_msat - commit_tx_fee_2_htlcs + 1; let amt_msat_2 = recv_value_2 + total_routing_fee_msat; - let (route_2, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[2], amt_msat_2); + let mut route_2 = route_1.clone(); + route_2.paths[0].hops.last_mut().unwrap().fee_msat = amt_msat_2; // Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc() let secp_ctx = Secp256k1::new(); @@ -1832,10 +1835,8 @@ fn test_channel_reserve_holding_cell_htlcs() { unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert!(regex::Regex::new(r"Cannot send value that would put us over the max HTLC value in flight our peer will accept \(\d+\)").unwrap().is_match(err))); + ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send value that would put us over the max HTLC value in flight our peer will accept", 1); } // channel reserve is bigger than their_max_htlc_value_in_flight_msat so loop to deplete @@ -1901,11 +1902,12 @@ fn test_channel_reserve_holding_cell_htlcs() { // channel reserve test with htlc pending output > 0 let recv_value_2 = stat01.value_to_self_msat - amt_msat_1 - stat01.channel_reserve_msat - total_fee_msat - commit_tx_fee_2_htlcs; { - let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], recv_value_2 + 1); + let mut route = route_1.clone(); + route.paths[0].hops.last_mut().unwrap().fee_msat = recv_value_2 + 1; + let (_, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(nodes[2]); unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err))); + ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); } @@ -1930,13 +1932,13 @@ fn test_channel_reserve_holding_cell_htlcs() { // test with outbound holding cell amount > 0 { - let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], recv_value_22+1); + let (mut route, our_payment_hash, _, our_payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[2], recv_value_22); + route.paths[0].hops.last_mut().unwrap().fee_msat += 1; unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err))); + ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send value that would put our balance under counterparty-announced channel reserve value", 2); } let (route_22, our_payment_hash_22, our_payment_preimage_22, our_payment_secret_22) = get_route_and_payment_hash!(nodes[0], nodes[2], recv_value_22); @@ -5719,9 +5721,6 @@ fn test_fail_holding_cell_htlc_upon_free() { chan_stat = get_channel_value_stat!(nodes[0], nodes[1], chan.2); assert_eq!(chan_stat.holding_cell_outbound_amount_msat, 0); nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Freeing holding cell with 1 HTLC updates in channel {}", hex::encode(chan.2)), 1); - let failure_log = format!("Failed to send HTLC with payment_hash {} due to Cannot send value that would put our balance under counterparty-announced channel reserve value ({}) in channel {}", - hex::encode(our_payment_hash.0), chan_stat.channel_reserve_msat, hex::encode(chan.2)); - nodes[0].logger.assert_log("lightning::ln::channel".to_string(), failure_log.to_string(), 1); // Check that the payment failed to be sent out. let events = nodes[0].node.get_and_clear_pending_events(); @@ -5810,9 +5809,6 @@ fn test_free_and_fail_holding_cell_htlcs() { chan_stat = get_channel_value_stat!(nodes[0], nodes[1], chan.2); assert_eq!(chan_stat.holding_cell_outbound_amount_msat, 0); nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Freeing holding cell with 2 HTLC updates in channel {}", hex::encode(chan.2)), 1); - let failure_log = format!("Failed to send HTLC with payment_hash {} due to Cannot send value that would put our balance under counterparty-announced channel reserve value ({}) in channel {}", - hex::encode(payment_hash_2.0), chan_stat.channel_reserve_msat, hex::encode(chan.2)); - nodes[0].logger.assert_log("lightning::ln::channel".to_string(), failure_log.to_string(), 1); // Check that the second payment failed to be sent out. let events = nodes[0].node.get_and_clear_pending_events(); @@ -5878,10 +5874,10 @@ fn test_free_and_fail_holding_cell_htlcs() { fn test_fail_holding_cell_htlc_upon_free_multihop() { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); - // When this test was written, the default base fee floated based on the HTLC count. - // It is now fixed, so we simply set the fee to the expected value here. + // Avoid having to include routing fees in calculations let mut config = test_default_channel_config(); - config.channel_config.forwarding_fee_base_msat = 196; + config.channel_config.forwarding_fee_base_msat = 0; + config.channel_config.forwarding_fee_proportional_millionths = 0; let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config.clone()), Some(config.clone()), Some(config.clone())]); let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); let chan_0_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000); @@ -5913,9 +5909,7 @@ fn test_fail_holding_cell_htlc_upon_free_multihop() { let opt_anchors = get_opt_anchors!(nodes[0], nodes[1], chan_0_1.2); // Send a payment which passes reserve checks but gets stuck in the holding cell. - let feemsat = 239; - let total_routing_fee_msat = (nodes.len() - 2) as u64 * feemsat; - let max_can_send = 5000000 - channel_reserve - 2*commit_tx_fee_msat(feerate, 1 + 1, opt_anchors) - total_routing_fee_msat; + let max_can_send = 5000000 - channel_reserve - 2*commit_tx_fee_msat(feerate, 1 + 1, opt_anchors); let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], max_can_send); let payment_event = { nodes[0].node.send_payment_with_route(&route, our_payment_hash, @@ -6023,10 +6017,8 @@ fn test_update_add_htlc_bolt2_sender_value_below_minimum_msat() { unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert!(regex::Regex::new(r"Cannot send less than their minimum HTLC value \(\d+\)").unwrap().is_match(err))); + ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send less than their minimum HTLC value", 1); } #[test] @@ -6105,6 +6097,8 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_num_and_htlc_id_increment() let max_accepted_htlcs = nodes[1].node.per_peer_state.read().unwrap().get(&nodes[0].node.get_our_node_id()) .unwrap().lock().unwrap().channel_by_id.get(&chan.2).unwrap().counterparty_max_accepted_htlcs as u64; + // Fetch a route in advance as we will be unable to once we're unable to send. + let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100000); for i in 0..max_accepted_htlcs { let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100000); let payment_event = { @@ -6128,14 +6122,11 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_num_and_htlc_id_increment() expect_pending_htlcs_forwardable!(nodes[1]); expect_payment_claimable!(nodes[1], our_payment_hash, our_payment_secret, 100000); } - let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100000); unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert!(regex::Regex::new(r"Cannot push more than their max accepted HTLCs \(\d+\)").unwrap().is_match(err))); + ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot push more than their max accepted HTLCs", 1); } #[test] @@ -6157,11 +6148,8 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_value_in_flight() { route.paths[0].hops[0].fee_msat = max_in_flight + 1; unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, our_payment_hash, RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert!(regex::Regex::new(r"Cannot send value that would put us over the max HTLC value in flight our peer will accept \(\d+\)").unwrap().is_match(err))); - + ), true, APIError::ChannelUnavailable { .. }, {}); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send value that would put us over the max HTLC value in flight our peer will accept", 1); send_payment(&nodes[0], &[&nodes[1]], max_in_flight); } @@ -6243,12 +6231,15 @@ fn test_update_add_htlc_bolt2_receiver_check_max_htlc_limit() { let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000); - let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 3999999); + let send_amt = 3999999; + let (mut route, our_payment_hash, _, our_payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[1], 1000); + route.paths[0].hops[0].fee_msat = send_amt; let session_priv = SecretKey::from_slice(&[42; 32]).unwrap(); let cur_height = nodes[0].node.best_block.read().unwrap().height() + 1; let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::signing_only(), &route.paths[0], &session_priv).unwrap(); let (onion_payloads, _htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads( - &route.paths[0], 3999999, RecipientOnionFields::secret_only(our_payment_secret), cur_height, &None).unwrap(); + &route.paths[0], send_amt, RecipientOnionFields::secret_only(our_payment_secret), cur_height, &None).unwrap(); let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash).unwrap(); let mut msg = msgs::UpdateAddHTLC { @@ -7618,45 +7609,6 @@ fn test_bump_txn_sanitize_tracking_maps() { } } -#[test] -fn test_pending_claimed_htlc_no_balance_underflow() { - // Tests that if we have a pending outbound HTLC as well as a claimed-but-not-fully-removed - // HTLC we will not underflow when we call `Channel::get_balance_msat()`. - 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 nodes = create_network(2, &node_cfgs, &node_chanmgrs); - create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0); - - let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1_010_000); - nodes[1].node.claim_funds(payment_preimage); - expect_payment_claimed!(nodes[1], payment_hash, 1_010_000); - check_added_monitors!(nodes[1], 1); - let fulfill_ev = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); - - nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &fulfill_ev.update_fulfill_htlcs[0]); - expect_payment_sent_without_paths!(nodes[0], payment_preimage); - nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &fulfill_ev.commitment_signed); - check_added_monitors!(nodes[0], 1); - let (_raa, _cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id()); - - // At this point nodes[1] has received 1,010k msat (10k msat more than their reserve) and can - // send an HTLC back (though it will go in the holding cell). Send an HTLC back and check we - // can get our balance. - - // Get a route from nodes[1] to nodes[0] by getting a route going the other way and then flip - // the public key of the only hop. This works around ChannelDetails not showing the - // almost-claimed HTLC as available balance. - let (mut route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 10_000); - route.payment_params = None; // This is all wrong, but unnecessary - route.paths[0].hops[0].pubkey = nodes[0].node.get_our_node_id(); - let (_, payment_hash_2, payment_secret_2) = get_payment_preimage_hash!(nodes[0]); - nodes[1].node.send_payment_with_route(&route, payment_hash_2, - RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap(); - - assert_eq!(nodes[1].node.list_channels()[0].balance_msat, 1_000_000); -} - #[test] fn test_channel_conf_timeout() { // Tests that, for inbound channels, we give up on them if the funding transaction does not @@ -9656,6 +9608,10 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &channel_ready); update_nodes_with_chan_announce(&nodes, 0, 1, &announcement, &as_update, &bs_update); + // Fetch a route in advance as we will be unable to once we're unable to send. + let (mut route, payment_hash, _, payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[1], 1000); + let dust_buffer_feerate = { let per_peer_state = nodes[0].node.per_peer_state.read().unwrap(); let chan_lock = per_peer_state.get(&nodes[1].node.get_our_node_id()).unwrap().lock().unwrap(); @@ -9668,7 +9624,7 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e let dust_inbound_htlc_on_holder_tx_msat: u64 = (dust_buffer_feerate * htlc_success_tx_weight(opt_anchors) / 1000 + open_channel.dust_limit_satoshis - 1) * 1000; let dust_inbound_htlc_on_holder_tx: u64 = config.channel_config.max_dust_htlc_exposure_msat / dust_inbound_htlc_on_holder_tx_msat; - let dust_htlc_on_counterparty_tx: u64 = 25; + let dust_htlc_on_counterparty_tx: u64 = 4; let dust_htlc_on_counterparty_tx_msat: u64 = config.channel_config.max_dust_htlc_exposure_msat / dust_htlc_on_counterparty_tx; if on_holder_tx { @@ -9693,7 +9649,7 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e if dust_outbound_balance { // Outbound dust threshold: 2132 sats (`dust_buffer_feerate` * HTLC_TIMEOUT_TX_WEIGHT / 1000 + counteparty's `dust_limit_satoshis`) // Outbound dust balance: 5000 sats - for _ in 0..dust_htlc_on_counterparty_tx { + for _ in 0..dust_htlc_on_counterparty_tx - 1 { let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], dust_htlc_on_counterparty_tx_msat); nodes[0].node.send_payment_with_route(&route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap(); @@ -9701,32 +9657,27 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e } else { // Inbound dust threshold: 2031 sats (`dust_buffer_feerate` * HTLC_TIMEOUT_TX_WEIGHT / 1000 + counteparty's `dust_limit_satoshis`) // Inbound dust balance: 5000 sats - for _ in 0..dust_htlc_on_counterparty_tx { + for _ in 0..dust_htlc_on_counterparty_tx - 1 { route_payment(&nodes[1], &[&nodes[0]], dust_htlc_on_counterparty_tx_msat); } } } - let dust_overflow = dust_htlc_on_counterparty_tx_msat * (dust_htlc_on_counterparty_tx + 1); if exposure_breach_event == ExposureEvent::AtHTLCForward { - let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], if on_holder_tx { dust_outbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat }); - let mut config = UserConfig::default(); + route.paths[0].hops.last_mut().unwrap().fee_msat = + if on_holder_tx { dust_outbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat + 1 }; // With default dust exposure: 5000 sats if on_holder_tx { - let dust_outbound_overflow = dust_outbound_htlc_on_holder_tx_msat * (dust_outbound_htlc_on_holder_tx + 1); - let dust_inbound_overflow = dust_inbound_htlc_on_holder_tx_msat * dust_inbound_htlc_on_holder_tx + dust_outbound_htlc_on_holder_tx_msat; unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert_eq!(err, &format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", if dust_outbound_balance { dust_outbound_overflow } else { dust_inbound_overflow }, config.channel_config.max_dust_htlc_exposure_msat))); + ), true, APIError::ChannelUnavailable { .. }, {}); } else { unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0) - ), true, APIError::ChannelUnavailable { ref err }, - assert_eq!(err, &format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", dust_overflow, config.channel_config.max_dust_htlc_exposure_msat))); + ), true, APIError::ChannelUnavailable { .. }, {}); } } else if exposure_breach_event == ExposureEvent::AtHTLCReception { - let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], if on_holder_tx { dust_inbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat }); + let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], if on_holder_tx { dust_inbound_htlc_on_holder_tx_msat } else { dust_htlc_on_counterparty_tx_msat + 1 }); nodes[1].node.send_payment_with_route(&route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap(); check_added_monitors!(nodes[1], 1); @@ -9742,10 +9693,13 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", if dust_outbound_balance { dust_outbound_overflow } else { dust_inbound_overflow }, config.channel_config.max_dust_htlc_exposure_msat), 1); } else { // Outbound dust balance: 5200 sats - nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", dust_overflow, config.channel_config.max_dust_htlc_exposure_msat), 1); + nodes[0].logger.assert_log("lightning::ln::channel".to_string(), + format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", + dust_htlc_on_counterparty_tx_msat * (dust_htlc_on_counterparty_tx - 1) + dust_htlc_on_counterparty_tx_msat + 1, + config.channel_config.max_dust_htlc_exposure_msat), 1); } } else if exposure_breach_event == ExposureEvent::AtUpdateFeeOutbound { - let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 2_500_000); + route.paths[0].hops.last_mut().unwrap().fee_msat = 2_500_000; nodes[0].node.send_payment_with_route(&route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap(); { diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 9e044d1c92d..5efd1cb9148 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -2167,12 +2167,11 @@ fn retry_multi_path_single_failed_payment() { assert_eq!(events.len(), 1); match events[0] { Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently: false, - failure: PathFailure::InitialSend { err: APIError::ChannelUnavailable { err: ref err_msg }}, + failure: PathFailure::InitialSend { err: APIError::ChannelUnavailable { .. }}, short_channel_id: Some(expected_scid), .. } => { assert_eq!(payment_hash, ev_payment_hash); assert_eq!(expected_scid, route.paths[1].hops[0].short_channel_id); - assert!(err_msg.contains("max HTLC")); }, _ => panic!("Unexpected event"), } @@ -2242,12 +2241,11 @@ fn immediate_retry_on_failure() { assert_eq!(events.len(), 1); match events[0] { Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently: false, - failure: PathFailure::InitialSend { err: APIError::ChannelUnavailable { err: ref err_msg }}, + failure: PathFailure::InitialSend { err: APIError::ChannelUnavailable { .. }}, short_channel_id: Some(expected_scid), .. } => { assert_eq!(payment_hash, ev_payment_hash); assert_eq!(expected_scid, route.paths[1].hops[0].short_channel_id); - assert!(err_msg.contains("max HTLC")); }, _ => panic!("Unexpected event"), } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index ef6fef0a7eb..bf854aed637 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -929,7 +929,7 @@ impl<'a> CandidateRouteHop<'a> { fn htlc_minimum_msat(&self) -> u64 { match self { - CandidateRouteHop::FirstHop { .. } => 0, + CandidateRouteHop::FirstHop { details } => details.next_outbound_htlc_minimum_msat, CandidateRouteHop::PublicHop { info, .. } => info.direction().htlc_minimum_msat, CandidateRouteHop::PrivateHop { hint } => hint.htlc_minimum_msat.unwrap_or(0), } @@ -2460,6 +2460,7 @@ mod tests { balance_msat: 0, outbound_capacity_msat, next_outbound_htlc_limit_msat: outbound_capacity_msat, + next_outbound_htlc_minimum_msat: 0, inbound_capacity_msat: 42, unspendable_punishment_reserve: None, confirmations_required: None, @@ -6121,6 +6122,7 @@ pub(crate) mod bench_utils { user_channel_id: 0, balance_msat: 10_000_000_000, outbound_capacity_msat: 10_000_000_000, + next_outbound_htlc_minimum_msat: 0, next_outbound_htlc_limit_msat: 10_000_000_000, inbound_capacity_msat: 0, unspendable_punishment_reserve: None,