Separate `ChannelDetails`' outbound capacity from the next HTLC max
authorMatt Corallo <git@bluematt.me>
Sat, 16 Apr 2022 20:07:34 +0000 (20:07 +0000)
committerMatt Corallo <git@bluematt.me>
Mon, 25 Apr 2022 15:04:21 +0000 (15:04 +0000)
`ChannelDetails::outbound_capacity_msat` describes the total amount
available for sending across several HTLCs, basically just our
balance minus the reserve value maintained by our counterparty.
However, when routing we use it to guess the maximum amount we can
send in a single additional HTLC, which it is not.

There are numerous reasons why our balance may not match the amount
we can send in a single HTLC, whether the HTLC in-flight limit, the
channe's HTLC maximum, or our feerate buffer.

This commit splits the `outbound_capacity_msat` field into two -
`outbound_capacity_msat` and `outbound_htlc_limit_msat`, setting us
up for correctly handling our next-HTLC-limit in the future.

This also addresses the first of the reasons why the values may
not match - the max-in-flight limit. The inaccuracy is ultimately
tracked as #1126.

fuzz/src/router.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/routing/router.rs

index b0f052dbc646f613a1d8c038c88c96c3604ef88e..27c5ee2b7ca108a19bbe9f7ca3c5a7a5f91d01ae 100644 (file)
@@ -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<Out: test_logger::Output>(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<Out: test_logger::Output>(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<Out: test_logger::Output>(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,
                                                        });
index b0551bf8323f1617f7cba4c991c783221c778c46..afb1de43f1c296f771296d7ea7e728b4138e3fca 100644 (file)
@@ -2330,23 +2330,30 @@ impl<Signer: Sign> Channel<Signer> {
                stats
        }
 
-       /// Get the available (ie not including pending HTLCs) inbound and outbound balance in msat.
+       /// Get the available (ie not including pending HTLCs) inbound and outbound balance, plus the
+       /// amount available for a single HTLC send, all in msat.
        /// 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_inbound_outbound_available_balance_msat(&self) -> (u64, u64, u64) {
                // Note that we have to handle overflow due to the above case.
+               let outbound_stats = self.get_outbound_pending_htlc_stats(None);
+               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;
                (
                        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,
+                       outbound_capacity_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
                )
        }
index 93746f0fde05cbc6b3ae3272a6851f36ef7d993c..4aad59ff2f4f56986abdc2816a7a27a2220c86fd 100644 (file)
@@ -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,7 +1677,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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 (inbound_capacity_msat, outbound_capacity_msat, next_outbound_htlc_limit_msat) =
+                                       channel.get_inbound_outbound_available_balance_msat();
                                let balance_msat = channel.get_balance_msat();
                                let (to_remote_reserve_satoshis, to_self_reserve_satoshis) =
                                        channel.get_holder_counterparty_selected_channel_reserve_satoshis();
@@ -1701,6 +1709,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        balance_msat,
                                        inbound_capacity_msat,
                                        outbound_capacity_msat,
+                                       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(),
@@ -5937,6 +5946,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),
index 212ac2f10cae7d62695f032c6fd8ab0a4e523ffa..2d7e1d202810c51596a5dbe7273d98e77f3d2e08 100644 (file)
@@ -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)];
 
                {
@@ -5473,6 +5475,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,