From c7c4226e99234ded7ade4f67ab40f4729cf9a20c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 29 Sep 2023 18:32:25 +0000 Subject: [PATCH] Revert "Remove AvailableBalances::balance_msat" While removing the `balance_msat` field absolutely makes sense - it is, at best, confusing - we really need a solid replacement for it before we can do so. While one such replacement is in progress, it is not complete and we'd like to not block our current release on its completion. This reverts commit ef5be580f51d6756612fea710516c0297203f4dc. --- fuzz/src/router.rs | 1 + lightning/src/chain/chainmonitor.rs | 3 ++- lightning/src/ln/channel.rs | 11 ++++++++ lightning/src/ln/channelmanager.rs | 31 ++++++++++++++--------- lightning/src/routing/router.rs | 2 ++ pending_changelog/remove_balance_msat.txt | 1 - 6 files changed, 35 insertions(+), 14 deletions(-) delete mode 100644 pending_changelog/remove_balance_msat.txt diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index b7d45bf72..7f4e7ad40 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -233,6 +233,7 @@ pub fn do_test(data: &[u8], out: Out) { force_close_spend_delay: None, is_outbound: true, is_channel_ready: true, is_usable: true, is_public: true, + 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, diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 6d003df72..cf51dbab7 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -423,7 +423,8 @@ where C::Target: chain::Filter, /// claims which are awaiting confirmation. /// /// Includes the balances from each [`ChannelMonitor`] *except* those included in - /// `ignored_channels`. + /// `ignored_channels`, allowing you to filter out balances from channels which are still open + /// (and whose balance should likely be pulled from the [`ChannelDetails`]). /// /// See [`ChannelMonitor::get_claimable_balances`] for more details on the exact criteria for /// inclusion in the return value. diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f618a4618..77a04e872 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -66,6 +66,8 @@ pub struct ChannelValueStat { } 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. @@ -1655,6 +1657,14 @@ impl ChannelContext where SP::Target: SignerProvider { let inbound_stats = context.get_inbound_pending_htlc_stats(None); let outbound_stats = context.get_outbound_pending_htlc_stats(None); + let mut balance_msat = context.value_to_self_msat; + for ref htlc in context.pending_inbound_htlcs.iter() { + if let InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_)) = htlc.state { + balance_msat += htlc.amount_msat; + } + } + balance_msat -= outbound_stats.pending_htlcs_value_msat; + let outbound_capacity_msat = context.value_to_self_msat .saturating_sub(outbound_stats.pending_htlcs_value_msat) .saturating_sub( @@ -1771,6 +1781,7 @@ impl ChannelContext where SP::Target: SignerProvider { outbound_capacity_msat, next_outbound_htlc_limit_msat: available_capacity_msat, next_outbound_htlc_minimum_msat, + balance_msat, } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index cfa17295a..775a51afc 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1448,12 +1448,6 @@ pub struct ChannelCounterparty { } /// Details of a channel, as returned by [`ChannelManager::list_channels`] and [`ChannelManager::list_usable_channels`] -/// -/// Balances of a channel are available through [`ChainMonitor::get_claimable_balances`] and -/// [`ChannelMonitor::get_claimable_balances`], calculated with respect to the corresponding on-chain -/// transactions. -/// -/// [`ChainMonitor::get_claimable_balances`]: crate::chain::chainmonitor::ChainMonitor::get_claimable_balances #[derive(Clone, Debug, PartialEq)] pub struct ChannelDetails { /// The channel's ID (prior to funding transaction generation, this is a random 32 bytes, @@ -1535,11 +1529,24 @@ pub struct ChannelDetails { /// /// This value will be `None` for objects serialized with LDK versions prior to 0.0.115. pub feerate_sat_per_1000_weight: Option, + /// Our total balance. This is the amount we would get if we close the channel. + /// This value is not exact. Due to various in-flight changes and feerate changes, exactly this + /// amount is not likely to be recoverable on close. + /// + /// This does not include any pending HTLCs which are not yet fully resolved (and, thus, whose + /// balance is not available for inclusion in new outbound HTLCs). This further does not include + /// any pending outgoing HTLCs which are awaiting some other resolution to be sent. + /// This does not consider any on-chain fees. + /// + /// See also [`ChannelDetails::outbound_capacity_msat`] + pub balance_msat: u64, /// The available outbound capacity for sending HTLCs to the remote peer. This does not include /// any pending HTLCs which are not yet fully resolved (and, thus, whose balance is not /// available for inclusion in new outbound HTLCs). This further does not include any pending /// outgoing HTLCs which are awaiting some other resolution to be sent. /// + /// See also [`ChannelDetails::balance_msat`] + /// /// This value is not exact. Due to various in-flight changes, feerate changes, and our /// conflict-avoidance policy, exactly this amount is not likely to be spendable. However, we /// should be able to spend nearly this amount. @@ -1549,8 +1556,8 @@ 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::next_outbound_htlc_minimum_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 @@ -1680,6 +1687,7 @@ impl ChannelDetails { channel_value_satoshis: context.get_value_satoshis(), feerate_sat_per_1000_weight: Some(context.get_feerate_sat_per_1000_weight()), unspendable_punishment_reserve: to_self_reserve_satoshis, + 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, @@ -8419,7 +8427,7 @@ impl Writeable for ChannelDetails { (10, self.channel_value_satoshis, required), (12, self.unspendable_punishment_reserve, option), (14, user_channel_id_low, required), - (16, self.next_outbound_htlc_limit_msat, required), // Forwards compatibility for removed balance_msat field. + (16, self.balance_msat, required), (18, self.outbound_capacity_msat, required), (19, self.next_outbound_htlc_limit_msat, required), (20, self.inbound_capacity_msat, required), @@ -8455,7 +8463,7 @@ impl Readable for ChannelDetails { (10, channel_value_satoshis, required), (12, unspendable_punishment_reserve, option), (14, user_channel_id_low, required), - (16, _balance_msat, option), // Backwards compatibility for removed balance_msat field. + (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. @@ -8481,8 +8489,6 @@ impl Readable for ChannelDetails { let user_channel_id = user_channel_id_low as u128 + ((user_channel_id_high_opt.unwrap_or(0 as u64) as u128) << 64); - let _balance_msat: Option = _balance_msat; - Ok(Self { inbound_scid_alias, channel_id: channel_id.0.unwrap(), @@ -8495,6 +8501,7 @@ impl Readable for ChannelDetails { channel_value_satoshis: channel_value_satoshis.0.unwrap(), unspendable_punishment_reserve, user_channel_id, + 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(), diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 33182f7cb..1297322fb 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -2870,6 +2870,7 @@ mod tests { inbound_scid_alias: None, channel_value_satoshis: 0, user_channel_id: 0, + balance_msat: 0, outbound_capacity_msat, next_outbound_htlc_limit_msat: outbound_capacity_msat, next_outbound_htlc_minimum_msat: 0, @@ -7780,6 +7781,7 @@ pub(crate) mod bench_utils { outbound_scid_alias: None, channel_value_satoshis: 10_000_000_000, 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, diff --git a/pending_changelog/remove_balance_msat.txt b/pending_changelog/remove_balance_msat.txt deleted file mode 100644 index 2ba5df1fd..000000000 --- a/pending_changelog/remove_balance_msat.txt +++ /dev/null @@ -1 +0,0 @@ -* The `AvailableBalances::balance_msat` field has been removed in favor of `ChannelMonitor::get_claimable_balances`. `ChannelDetails` serialized with versions of LDK >= 0.0.117 will have their `balance_msat` field set to `next_outbound_htlc_limit_msat` when read by versions of LDK prior to 0.0.117 (#2476). -- 2.39.5