From 70762f90caaad3ceb1c1e81bebbca90876c6be66 Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Fri, 16 Aug 2024 11:34:55 +0200 Subject: [PATCH 1/2] Deprecate AvailableBalances::balance_msat The ChannelMonitor::get_claimable_balances method provides a more straightforward approach to the balance of a channel, which satisfies most use cases. The computation of AvailableBalances::balance_msat is complex and originally had a different purpose that is not applicable anymore. We deprecate AvailableBalances::balance_msat now and will remove it in a following release. Co-authored-by: Willem Van Lint --- lightning/src/chain/chainmonitor.rs | 3 +- lightning/src/ln/channel.rs | 2 + lightning/src/ln/channel_state.rs | 76 +++++++++++-------- lightning/src/routing/router.rs | 2 + .../3247-deprecate-balance_msat.txt | 1 + 5 files changed, 50 insertions(+), 34 deletions(-) create mode 100644 pending_changelog/3247-deprecate-balance_msat.txt diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 7bfa85de613..9523a6ea9be 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -394,8 +394,7 @@ where C::Target: chain::Filter, /// claims which are awaiting confirmation. /// /// Includes the balances from each [`ChannelMonitor`] *except* those included in - /// `ignored_channels`, allowing you to filter out balances from channels which are still open - /// (and whose balance should likely be pulled from the [`ChannelDetails`]). + /// `ignored_channels`. /// /// 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 226fb50826d..f4d55f91693 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -80,6 +80,7 @@ pub struct ChannelValueStat { pub struct AvailableBalances { /// The amount that would go to us if we close the channel, ignoring any on-chain fees. + #[deprecated(since = "0.0.124", note = "use [`ChainMonitor::get_claimable_balances`] instead")] pub balance_msat: u64, /// Total amount available for our counterparty to send to us. pub inbound_capacity_msat: u64, @@ -3215,6 +3216,7 @@ impl ChannelContext where SP::Target: SignerProvider { available_capacity_msat = 0; } + #[allow(deprecated)] // TODO: Remove once balance_msat is removed. AvailableBalances { inbound_capacity_msat: cmp::max(context.channel_value_satoshis as i64 * 1000 - context.value_to_self_msat as i64 diff --git a/lightning/src/ln/channel_state.rs b/lightning/src/ln/channel_state.rs index 991ba077225..7a9bbf9bac4 100644 --- a/lightning/src/ln/channel_state.rs +++ b/lightning/src/ln/channel_state.rs @@ -273,8 +273,14 @@ impl_writeable_tlv_based!(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. +/// /// [`ChannelManager::list_channels`]: crate::ln::channelmanager::ChannelManager::list_channels /// [`ChannelManager::list_usable_channels`]: crate::ln::channelmanager::ChannelManager::list_usable_channels +/// [`ChainMonitor::get_claimable_balances`]: crate::chain::chainmonitor::ChainMonitor::get_claimable_balances +/// [`ChannelMonitor::get_claimable_balances`]: crate::chain::channelmonitor::ChannelMonitor::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, @@ -363,6 +369,7 @@ pub struct ChannelDetails { /// This does not consider any on-chain fees. /// /// See also [`ChannelDetails::outbound_capacity_msat`] + #[deprecated(since = "0.0.124", note = "use [`ChainMonitor::get_claimable_balances`] instead")] 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 @@ -495,6 +502,7 @@ impl ChannelDetails { let balance = context.get_available_balances(fee_estimator); let (to_remote_reserve_satoshis, to_self_reserve_satoshis) = context.get_holder_counterparty_selected_channel_reserve_satoshis(); + #[allow(deprecated)] // TODO: Remove once balance_msat is removed. ChannelDetails { channel_id: context.channel_id(), counterparty: ChannelCounterparty { @@ -561,38 +569,41 @@ impl Writeable for ChannelDetails { // versions prior to 0.0.113, the u128 is serialized as two separate u64 values. let user_channel_id_low = self.user_channel_id as u64; let user_channel_id_high_opt = Some((self.user_channel_id >> 64) as u64); - write_tlv_fields!(writer, { - (1, self.inbound_scid_alias, option), - (2, self.channel_id, required), - (3, self.channel_type, option), - (4, self.counterparty, required), - (5, self.outbound_scid_alias, option), - (6, self.funding_txo, option), - (7, self.config, option), - (8, self.short_channel_id, option), - (9, self.confirmations, option), - (10, self.channel_value_satoshis, required), - (12, self.unspendable_punishment_reserve, option), - (14, user_channel_id_low, required), - (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), - (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), - (28, self.is_channel_ready, required), - (30, self.is_usable, required), - (32, self.is_public, required), - (33, self.inbound_htlc_minimum_msat, option), - (35, self.inbound_htlc_maximum_msat, option), - (37, user_channel_id_high_opt, option), - (39, self.feerate_sat_per_1000_weight, option), - (41, self.channel_shutdown_state, option), - (43, self.pending_inbound_htlcs, optional_vec), - (45, self.pending_outbound_htlcs, optional_vec), - }); + #[allow(deprecated)] // TODO: Remove once balance_msat is removed. + { + write_tlv_fields!(writer, { + (1, self.inbound_scid_alias, option), + (2, self.channel_id, required), + (3, self.channel_type, option), + (4, self.counterparty, required), + (5, self.outbound_scid_alias, option), + (6, self.funding_txo, option), + (7, self.config, option), + (8, self.short_channel_id, option), + (9, self.confirmations, option), + (10, self.channel_value_satoshis, required), + (12, self.unspendable_punishment_reserve, option), + (14, user_channel_id_low, required), + (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), + (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), + (28, self.is_channel_ready, required), + (30, self.is_usable, required), + (32, self.is_public, required), + (33, self.inbound_htlc_minimum_msat, option), + (35, self.inbound_htlc_maximum_msat, option), + (37, user_channel_id_high_opt, option), + (39, self.feerate_sat_per_1000_weight, option), + (41, self.channel_shutdown_state, option), + (43, self.pending_inbound_htlcs, optional_vec), + (45, self.pending_outbound_htlcs, optional_vec), + }); + } Ok(()) } } @@ -640,6 +651,7 @@ 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); + #[allow(deprecated)] // TODO: Remove once balance_msat is removed. Ok(Self { inbound_scid_alias, channel_id: channel_id.0.unwrap(), diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 4a2bdb31bbb..fa2bbac1d14 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -3591,6 +3591,7 @@ mod tests { fn get_channel_details(short_channel_id: Option, node_id: PublicKey, features: InitFeatures, outbound_capacity_msat: u64) -> ChannelDetails { + #[allow(deprecated)] // TODO: Remove once balance_msat is removed. ChannelDetails { channel_id: ChannelId::new_zero(), counterparty: ChannelCounterparty { @@ -8776,6 +8777,7 @@ pub(crate) mod bench_utils { #[inline] pub(crate) fn first_hop(node_id: PublicKey) -> ChannelDetails { + #[allow(deprecated)] // TODO: Remove once balance_msat is removed. ChannelDetails { channel_id: ChannelId::new_zero(), counterparty: ChannelCounterparty { diff --git a/pending_changelog/3247-deprecate-balance_msat.txt b/pending_changelog/3247-deprecate-balance_msat.txt new file mode 100644 index 00000000000..662eb3de9a1 --- /dev/null +++ b/pending_changelog/3247-deprecate-balance_msat.txt @@ -0,0 +1 @@ +* The `AvailableBalances::balance_msat` field has been deprecated in favor of `ChainMonitor::get_claimable_balances`. `AvailableBalances::balance_msat` will be removed in an upcoming release (#3247). From 0a1adeb3762ecde6adad96c2f41aef9698c1787b Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Tue, 20 Aug 2024 09:38:35 +0200 Subject: [PATCH 2/2] Test ChannelDetails serialization to catch mutants --- lightning/src/ln/channel_state.rs | 90 +++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/lightning/src/ln/channel_state.rs b/lightning/src/ln/channel_state.rs index 7a9bbf9bac4..f0a5c37e86f 100644 --- a/lightning/src/ln/channel_state.rs +++ b/lightning/src/ln/channel_state.rs @@ -715,3 +715,93 @@ impl_writeable_tlv_based_enum!(ChannelShutdownState, (6, NegotiatingClosingFee) => {}, (8, ShutdownComplete) => {}, ); + +#[cfg(test)] +mod tests { + use bitcoin::{hashes::Hash as _, secp256k1::PublicKey}; + use lightning_types::features::Features; + use types::payment::PaymentHash; + + use crate::{ + chain::transaction::OutPoint, + ln::{ + channel_state::{ + InboundHTLCDetails, InboundHTLCStateDetails, OutboundHTLCDetails, + OutboundHTLCStateDetails, + }, + types::ChannelId, + }, + util::{ + config::ChannelConfig, + ser::{Readable, Writeable}, + }, + }; + + use super::{ChannelCounterparty, ChannelDetails, ChannelShutdownState}; + + #[test] + fn test_channel_details_serialization() { + #[allow(deprecated)] + let channel_details = ChannelDetails { + channel_id: ChannelId::new_zero(), + counterparty: ChannelCounterparty { + features: Features::empty(), + node_id: PublicKey::from_slice(&[2; 33]).unwrap(), + unspendable_punishment_reserve: 1983, + forwarding_info: None, + outbound_htlc_minimum_msat: None, + outbound_htlc_maximum_msat: None, + }, + funding_txo: Some(OutPoint { + txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), + index: 1, + }), + channel_type: None, + short_channel_id: None, + outbound_scid_alias: None, + inbound_scid_alias: None, + channel_value_satoshis: 50_100, + user_channel_id: (u64::MAX as u128) + 1, // Gets us into the high bytes + balance_msat: 23_100, + outbound_capacity_msat: 24_300, + next_outbound_htlc_limit_msat: 20_000, + next_outbound_htlc_minimum_msat: 132, + inbound_capacity_msat: 42, + unspendable_punishment_reserve: Some(8273), + confirmations_required: Some(5), + confirmations: Some(73), + force_close_spend_delay: Some(10), + is_outbound: true, + is_channel_ready: false, + is_usable: true, + is_public: false, + inbound_htlc_minimum_msat: Some(98), + inbound_htlc_maximum_msat: Some(983274), + config: Some(ChannelConfig::default()), + feerate_sat_per_1000_weight: Some(212), + channel_shutdown_state: Some(ChannelShutdownState::NotShuttingDown), + pending_inbound_htlcs: vec![InboundHTLCDetails { + htlc_id: 12, + amount_msat: 333, + cltv_expiry: 127, + payment_hash: PaymentHash([3; 32]), + state: Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToAdd), + is_dust: true, + }], + pending_outbound_htlcs: vec![OutboundHTLCDetails { + htlc_id: Some(81), + amount_msat: 5000, + cltv_expiry: 129, + payment_hash: PaymentHash([4; 32]), + state: Some(OutboundHTLCStateDetails::AwaitingRemoteRevokeToAdd), + skimmed_fee_msat: Some(42), + is_dust: false, + }], + }; + let mut buffer = Vec::new(); + channel_details.write(&mut buffer).unwrap(); + let deser_channel_details = ChannelDetails::read(&mut buffer.as_slice()).unwrap(); + + assert_eq!(deser_channel_details, channel_details); + } +}