Merge pull request #3247 from dunxen/2024-08-deprecate-balancemsat

Deprecate AvailableBalances::balance_msat
lightningdevkit · Aug 20, 2024 · bbfa15e · bbfa15e
2 parents 3605777 + 0a1adeb
commit bbfa15e
Show file tree

Hide file tree

Showing 5 changed files with 140 additions and 34 deletions.
diff --git 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
@@ -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<SP: Deref> ChannelContext<SP> 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
@@ -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(),
@@ -703,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);
+	}
+}
diff --git 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<u64>, 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
@@ -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).
Original file line number	Diff line number	Diff line change
		@@ -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).