From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Thu, 28 Apr 2022 02:43:04 +0000 (+0000) Subject: Merge pull request #1435 from TheBlueMatt/2022-04-1126-first-step X-Git-Tag: v0.0.107~51 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=62edee568985e3362bd1609c6089d05428023925;hp=df1c4ee1506061adf285c09638d401ec6e8397c3;p=rust-lightning Merge pull request #1435 from TheBlueMatt/2022-04-1126-first-step --- diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index b0f052db..27c5ee2b 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -29,6 +29,7 @@ use bitcoin::blockdata::constants::genesis_block; use utils::test_logger; +use std::convert::TryInto; use std::collections::HashSet; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -205,7 +206,8 @@ pub fn do_test(data: &[u8], out: Out) { count => { for _ in 0..count { scid += 1; - let rnid = node_pks.iter().skip(slice_to_be16(get_slice!(2))as usize % node_pks.len()).next().unwrap(); + let rnid = node_pks.iter().skip(u16::from_be_bytes(get_slice!(2).try_into().unwrap()) as usize % node_pks.len()).next().unwrap(); + let capacity = u64::from_be_bytes(get_slice!(8).try_into().unwrap()); first_hops_vec.push(ChannelDetails { channel_id: [0; 32], counterparty: ChannelCounterparty { @@ -220,7 +222,7 @@ pub fn do_test(data: &[u8], out: Out) { channel_type: None, short_channel_id: Some(scid), inbound_scid_alias: None, - channel_value_satoshis: slice_to_be64(get_slice!(8)), + channel_value_satoshis: capacity, user_channel_id: 0, inbound_capacity_msat: 0, unspendable_punishment_reserve: None, confirmations_required: None, @@ -228,7 +230,8 @@ pub fn do_test(data: &[u8], out: Out) { is_outbound: true, is_funding_locked: true, is_usable: true, is_public: true, balance_msat: 0, - outbound_capacity_msat: 0, + outbound_capacity_msat: capacity.saturating_mul(1000), + next_outbound_htlc_limit_msat: capacity.saturating_mul(1000), inbound_htlc_minimum_msat: None, inbound_htlc_maximum_msat: None, }); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index b0551bf8..3a12a2c0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -62,6 +62,17 @@ pub struct ChannelValueStat { pub counterparty_dust_limit_msat: u64, } +pub struct AvailableBalances { + /// The amount that would go to us if we close the channel, ignoring any on-chain fees. + pub balance_msat: u64, + /// Total amount available for our counterparty to send to us. + pub inbound_capacity_msat: u64, + /// Total amount available for us to send to our counterparty. + pub outbound_capacity_msat: u64, + /// The maximum value we can assign to the next outbound HTLC + pub next_outbound_htlc_limit_msat: u64, +} + #[derive(Debug, Clone, Copy, PartialEq)] enum FeeUpdateState { // Inbound states mirroring InboundHTLCState @@ -2330,40 +2341,39 @@ impl Channel { stats } - /// Get the available (ie not including pending HTLCs) inbound and outbound balance in msat. + /// Get the available balances, see [`AvailableBalances`]'s fields for more info. /// Doesn't bother handling the /// if-we-removed-it-already-but-haven't-fully-resolved-they-can-still-send-an-inbound-HTLC /// corner case properly. - /// The channel reserve is subtracted from each balance. - /// See also [`Channel::get_balance_msat`] - pub fn get_inbound_outbound_available_balance_msat(&self) -> (u64, u64) { + pub fn get_available_balances(&self) -> AvailableBalances { // Note that we have to handle overflow due to the above case. - ( - cmp::max(self.channel_value_satoshis as i64 * 1000 - - self.value_to_self_msat as i64 - - self.get_inbound_pending_htlc_stats(None).pending_htlcs_value_msat as i64 - - self.holder_selected_channel_reserve_satoshis as i64 * 1000, - 0) as u64, - cmp::max(self.value_to_self_msat as i64 - - self.get_outbound_pending_htlc_stats(None).pending_htlcs_value_msat as i64 - - self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000, - 0) as u64 - ) - } + let outbound_stats = self.get_outbound_pending_htlc_stats(None); - /// Get our total balance in msat. - /// This is the amount that would go to us if we close the channel, ignoring any on-chain fees. - /// See also [`Channel::get_inbound_outbound_available_balance_msat`] - pub fn get_balance_msat(&self) -> u64 { - // Include our local balance, plus any inbound HTLCs we know the preimage for, minus any - // HTLCs sent or which will be sent after commitment signed's are exchanged. let mut balance_msat = self.value_to_self_msat; for ref htlc in self.pending_inbound_htlcs.iter() { if let InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_)) = htlc.state { balance_msat += htlc.amount_msat; } } - balance_msat - self.get_outbound_pending_htlc_stats(None).pending_htlcs_value_msat + 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; + AvailableBalances { + inbound_capacity_msat: cmp::max(self.channel_value_satoshis as i64 * 1000 + - self.value_to_self_msat as i64 + - self.get_inbound_pending_htlc_stats(None).pending_htlcs_value_msat as i64 + - 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, + balance_msat, + } } pub fn get_holder_counterparty_selected_channel_reserve_satoshis(&self) -> (u64, Option) { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index df1ba36c..d7f2aeb6 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1005,6 +1005,13 @@ pub struct ChannelDetails { /// conflict-avoidance policy, exactly this amount is not likely to be spendable. However, we /// should be able to spend nearly this amount. pub outbound_capacity_msat: u64, + /// The available outbound capacity for sending a single HTLC to the remote peer. This is + /// similar to [`ChannelDetails::outbound_capacity_msat`] but it may be further restricted by + /// 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`]. + pub next_outbound_htlc_limit_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). @@ -1670,8 +1677,7 @@ impl ChannelMana let channel_state = self.channel_state.lock().unwrap(); res.reserve(channel_state.by_id.len()); for (channel_id, channel) in channel_state.by_id.iter().filter(f) { - let (inbound_capacity_msat, outbound_capacity_msat) = channel.get_inbound_outbound_available_balance_msat(); - let balance_msat = channel.get_balance_msat(); + let balance = channel.get_available_balances(); let (to_remote_reserve_satoshis, to_self_reserve_satoshis) = channel.get_holder_counterparty_selected_channel_reserve_satoshis(); res.push(ChannelDetails { @@ -1698,9 +1704,10 @@ impl ChannelMana inbound_scid_alias: channel.latest_inbound_scid_alias(), channel_value_satoshis: channel.get_value_satoshis(), unspendable_punishment_reserve: to_self_reserve_satoshis, - balance_msat, - inbound_capacity_msat, - outbound_capacity_msat, + balance_msat: balance.balance_msat, + 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, user_channel_id: channel.get_user_id(), confirmations_required: channel.minimum_depth(), force_close_spend_delay: channel.get_counterparty_selected_contest_delay(), @@ -5936,6 +5943,9 @@ impl_writeable_tlv_based!(ChannelDetails, { (14, user_channel_id, required), (16, balance_msat, required), (18, 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, next_outbound_htlc_limit_msat, (default_value, outbound_capacity_msat.0.unwrap())), (20, inbound_capacity_msat, required), (22, confirmations_required, option), (24, force_close_spend_delay, option), diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 913c1a20..816dfaad 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -412,7 +412,7 @@ impl<'a> CandidateRouteHop<'a> { fn effective_capacity(&self) -> EffectiveCapacity { match self { CandidateRouteHop::FirstHop { details } => EffectiveCapacity::ExactLiquidity { - liquidity_msat: details.outbound_capacity_msat, + liquidity_msat: details.next_outbound_htlc_limit_msat, }, CandidateRouteHop::PublicHop { info, .. } => info.effective_capacity(), CandidateRouteHop::PrivateHop { .. } => EffectiveCapacity::Infinite, @@ -818,7 +818,8 @@ where L::Target: Logger { // We don't want multiple paths (as per MPP) share liquidity of the same channels. // This map allows paths to be aware of the channel use by other paths in the same call. // This would help to make a better path finding decisions and not "overbook" channels. - // It is unaware of the directions (except for `outbound_capacity_msat` in `first_hops`). + // It is unaware of the directions (except for `next_outbound_htlc_limit_msat` in + // `first_hops`). let mut bookkept_channels_liquidity_available_msat = HashMap::with_capacity(network_nodes.len()); // Keeping track of how much value we already collected across other paths. Helps to decide: @@ -841,12 +842,12 @@ where L::Target: Logger { // sort channels above `recommended_value_msat` in ascending order, preferring channels // which have enough, but not too much, capacity for the payment. channels.sort_unstable_by(|chan_a, chan_b| { - if chan_b.outbound_capacity_msat < recommended_value_msat || chan_a.outbound_capacity_msat < recommended_value_msat { + if chan_b.next_outbound_htlc_limit_msat < recommended_value_msat || chan_a.next_outbound_htlc_limit_msat < recommended_value_msat { // Sort in descending order - chan_b.outbound_capacity_msat.cmp(&chan_a.outbound_capacity_msat) + chan_b.next_outbound_htlc_limit_msat.cmp(&chan_a.next_outbound_htlc_limit_msat) } else { // Sort in ascending order - chan_a.outbound_capacity_msat.cmp(&chan_b.outbound_capacity_msat) + chan_a.next_outbound_htlc_limit_msat.cmp(&chan_b.next_outbound_htlc_limit_msat) } }); } @@ -1746,6 +1747,7 @@ mod tests { user_channel_id: 0, balance_msat: 0, outbound_capacity_msat, + next_outbound_htlc_limit_msat: outbound_capacity_msat, inbound_capacity_msat: 42, unspendable_punishment_reserve: None, confirmations_required: None, @@ -3407,7 +3409,7 @@ mod tests { assert_eq!(path.last().unwrap().fee_msat, 250_000_000); } - // Check that setting outbound_capacity_msat in first_hops limits the channels. + // Check that setting next_outbound_htlc_limit_msat in first_hops limits the channels. // Disable channel #1 and use another first hop. update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate { chain_hash: genesis_block(Network::Testnet).header.block_hash(), @@ -3422,7 +3424,7 @@ mod tests { excess_data: Vec::new() }); - // Now, limit the first_hop by the outbound_capacity_msat of 200_000 sats. + // Now, limit the first_hop by the next_outbound_htlc_limit_msat of 200_000 sats. let our_chans = vec![get_channel_details(Some(42), nodes[0].clone(), InitFeatures::from_le_bytes(vec![0b11]), 200_000_000)]; { @@ -5476,6 +5478,7 @@ mod benches { user_channel_id: 0, balance_msat: 10_000_000, outbound_capacity_msat: 10_000_000, + next_outbound_htlc_limit_msat: 10_000_000, inbound_capacity_msat: 0, unspendable_punishment_reserve: None, confirmations_required: None, diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index d7eb16ee..f35dc14e 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -230,6 +230,47 @@ pub enum Event { /// [`Route::get_total_fees`]: crate::routing::router::Route::get_total_fees fee_paid_msat: Option, }, + /// Indicates an outbound payment failed. Individual [`Event::PaymentPathFailed`] events + /// provide failure information for each MPP part in the payment. + /// + /// This event is provided once there are no further pending HTLCs for the payment and the + /// payment is no longer retryable, either due to a several-block timeout or because + /// [`ChannelManager::abandon_payment`] was previously called for the corresponding payment. + /// + /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment + PaymentFailed { + /// The id returned by [`ChannelManager::send_payment`] and used with + /// [`ChannelManager::retry_payment`] and [`ChannelManager::abandon_payment`]. + /// + /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment + /// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment + /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment + payment_id: PaymentId, + /// The hash that was given to [`ChannelManager::send_payment`]. + /// + /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment + payment_hash: PaymentHash, + }, + /// Indicates that a path for an outbound payment was successful. + /// + /// Always generated after [`Event::PaymentSent`] and thus useful for scoring channels. See + /// [`Event::PaymentSent`] for obtaining the payment preimage. + PaymentPathSuccessful { + /// The id returned by [`ChannelManager::send_payment`] and used with + /// [`ChannelManager::retry_payment`]. + /// + /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment + /// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment + payment_id: PaymentId, + /// The hash that was given to [`ChannelManager::send_payment`]. + /// + /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment + payment_hash: Option, + /// The payment path that was successful. + /// + /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain. + path: Vec, + }, /// Indicates an outbound HTLC we sent failed. Probably some intermediary node dropped /// something. You may wish to retry with a different route. /// @@ -299,27 +340,6 @@ pub enum Event { #[cfg(test)] error_data: Option>, }, - /// Indicates an outbound payment failed. Individual [`Event::PaymentPathFailed`] events - /// provide failure information for each MPP part in the payment. - /// - /// This event is provided once there are no further pending HTLCs for the payment and the - /// payment is no longer retryable, either due to a several-block timeout or because - /// [`ChannelManager::abandon_payment`] was previously called for the corresponding payment. - /// - /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment - PaymentFailed { - /// The id returned by [`ChannelManager::send_payment`] and used with - /// [`ChannelManager::retry_payment`] and [`ChannelManager::abandon_payment`]. - /// - /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment - /// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment - /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment - payment_id: PaymentId, - /// The hash that was given to [`ChannelManager::send_payment`]. - /// - /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment - payment_hash: PaymentHash, - }, /// Used to indicate that [`ChannelManager::process_pending_htlc_forwards`] should be called at /// a time in the future. /// @@ -390,26 +410,6 @@ pub enum Event { /// The full transaction received from the user transaction: Transaction }, - /// Indicates that a path for an outbound payment was successful. - /// - /// Always generated after [`Event::PaymentSent`] and thus useful for scoring channels. See - /// [`Event::PaymentSent`] for obtaining the payment preimage. - PaymentPathSuccessful { - /// The id returned by [`ChannelManager::send_payment`] and used with - /// [`ChannelManager::retry_payment`]. - /// - /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment - /// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment - payment_id: PaymentId, - /// The hash that was given to [`ChannelManager::send_payment`]. - /// - /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment - payment_hash: Option, - /// The payment path that was successful. - /// - /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain. - path: Vec, - }, /// Indicates a request to open a new channel by a peer. /// /// To accept the request, call [`ChannelManager::accept_inbound_channel`]. To reject the