Remove Channel::counterparty_htlc_minimum_msat

Author: ViktorTigerstrom

lightning-invoice/src/utils.rs

@@ -476,7 +476,6 @@ mod test {
 	use lightning::util::enforcing_trait_impls::EnforcingSigner;
 	use lightning::util::events::{MessageSendEvent, MessageSendEventsProvider, Event};
 	use lightning::util::test_utils;
-	use lightning::util::config::UserConfig;
 	use lightning::chain::keysinterface::KeysInterface;
 	use utils::create_invoice_from_channelmanager_and_duration_since_epoch;
 	use std::collections::HashSet;
@@ -594,46 +593,6 @@ mod test {
 		match_invoice_routes(Some(5000), &nodes[0], scid_aliases);
 	}
 
-	#[test]
-	fn test_forwarding_info_not_assigned_channel_excluded_from_hints() {
-		let chanmon_cfgs = create_chanmon_cfgs(3);
-		let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
-		let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
-		let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
-		let chan_1_0 = create_unannounced_chan_between_nodes_with_value(&nodes, 1, 0, 100000, 10001, InitFeatures::known(), InitFeatures::known());
-
-		// Create an unannonced channel between `nodes[2]` and `nodes[0]`, for which the
-		// `msgs::ChannelUpdate` is never handled for the node(s). As the `msgs::ChannelUpdate`
-		// is never handled, the `channel.counterparty.forwarding_info` is never assigned.
-		let mut private_chan_cfg = UserConfig::default();
-		private_chan_cfg.channel_options.announced_channel = false;
-		let temporary_channel_id = nodes[2].node.create_channel(nodes[0].node.get_our_node_id(), 1_000_000, 500_000_000, 42, Some(private_chan_cfg)).unwrap();
-		let open_channel = get_event_msg!(nodes[2], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id());
-		nodes[0].node.handle_open_channel(&nodes[2].node.get_our_node_id(), InitFeatures::known(), &open_channel);
-		let accept_channel = get_event_msg!(nodes[0], MessageSendEvent::SendAcceptChannel, nodes[2].node.get_our_node_id());
-		nodes[2].node.handle_accept_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &accept_channel);
-
-		let tx = sign_funding_transaction(&nodes[2], &nodes[0], 1_000_000, temporary_channel_id);
-
-		let conf_height = core::cmp::max(nodes[2].best_block_info().1 + 1, nodes[0].best_block_info().1 + 1);
-		confirm_transaction_at(&nodes[2], &tx, conf_height);
-		connect_blocks(&nodes[2], CHAN_CONFIRM_DEPTH - 1);
-		confirm_transaction_at(&nodes[0], &tx, conf_height);
-		connect_blocks(&nodes[0], CHAN_CONFIRM_DEPTH - 1);
-		let as_funding_locked = get_event_msg!(nodes[2], MessageSendEvent::SendFundingLocked, nodes[0].node.get_our_node_id());
-		nodes[2].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendFundingLocked, nodes[2].node.get_our_node_id()));
-		get_event_msg!(nodes[2], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id());
-		nodes[0].node.handle_funding_locked(&nodes[2].node.get_our_node_id(), &as_funding_locked);
-		get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, nodes[2].node.get_our_node_id());
-
-		// As `msgs::ChannelUpdate` was never handled for the participating node(s) of the second
-		// channel, the channel will never be assigned any `counterparty.forwarding_info`.
-		// Therefore only `chan_1_0` should be included in the hints.
-		let mut scid_aliases = HashSet::new();
-		scid_aliases.insert(chan_1_0.0.short_channel_id_alias.unwrap());
-		match_invoice_routes(Some(5000), &nodes[0], scid_aliases);
-	}
-
 	#[test]
 	fn test_no_hints_if_a_mix_between_public_and_private_channel_exists() {
 		let chanmon_cfgs = create_chanmon_cfgs(3);
@@ -959,62 +918,6 @@ mod test {
 		);
 	}
 
-	#[test]
-	#[cfg(feature = "std")]
-	fn test_multi_node_forwarding_info_not_assigned_channel_excluded_from_hints() {
-		let mut chanmon_cfgs = create_chanmon_cfgs(4);
-		let seed_1 = [42 as u8; 32];
-		let seed_2 = [43 as u8; 32];
-		let cross_node_seed = [44 as u8; 32];
-		chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed);
-		chanmon_cfgs[3].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed);
-		let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
-		let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
-		let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
-
-		let chan_0_2 = create_unannounced_chan_between_nodes_with_value(&nodes, 0, 2, 100000, 10001, InitFeatures::known(), InitFeatures::known());
-		let chan_0_3 = create_unannounced_chan_between_nodes_with_value(&nodes, 0, 3, 1000000, 10001, InitFeatures::known(), InitFeatures::known());
-
-		// Create an unannonced channel between `nodes[1]` and `nodes[3]`, for which the
-		// `msgs::ChannelUpdate` is never handled for the node(s). As the `msgs::ChannelUpdate`
-		// is never handled, the `channel.counterparty.forwarding_info` is never assigned.
-		let mut private_chan_cfg = UserConfig::default();
-		private_chan_cfg.channel_options.announced_channel = false;
-		let temporary_channel_id = nodes[1].node.create_channel(nodes[3].node.get_our_node_id(), 1_000_000, 500_000_000, 42, Some(private_chan_cfg)).unwrap();
-		let open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[3].node.get_our_node_id());
-		nodes[3].node.handle_open_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &open_channel);
-		let accept_channel = get_event_msg!(nodes[3], MessageSendEvent::SendAcceptChannel, nodes[1].node.get_our_node_id());
-		nodes[1].node.handle_accept_channel(&nodes[3].node.get_our_node_id(), InitFeatures::known(), &accept_channel);
-
-		let tx = sign_funding_transaction(&nodes[1], &nodes[3], 1_000_000, temporary_channel_id);
-
-		let conf_height = core::cmp::max(nodes[1].best_block_info().1 + 1, nodes[3].best_block_info().1 + 1);
-		confirm_transaction_at(&nodes[1], &tx, conf_height);
-		connect_blocks(&nodes[1], CHAN_CONFIRM_DEPTH - 1);
-		confirm_transaction_at(&nodes[3], &tx, conf_height);
-		connect_blocks(&nodes[3], CHAN_CONFIRM_DEPTH - 1);
-		let as_funding_locked = get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[3].node.get_our_node_id());
-		nodes[1].node.handle_funding_locked(&nodes[3].node.get_our_node_id(), &get_event_msg!(nodes[3], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id()));
-		get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[3].node.get_our_node_id());
-		nodes[3].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &as_funding_locked);
-		get_event_msg!(nodes[3], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
-
-		// As `msgs::ChannelUpdate` was never handled for the participating node(s) of the third
-		// channel, the channel will never be assigned any `counterparty.forwarding_info`.
-		// Therefore only `chan_0_3` should be included in the hints for `nodes[3]`.
-		let mut scid_aliases = HashSet::new();
-		scid_aliases.insert(chan_0_2.0.short_channel_id_alias.unwrap());
-		scid_aliases.insert(chan_0_3.0.short_channel_id_alias.unwrap());
-
-		match_multi_node_invoice_routes(
-			Some(10_000),
-			&nodes[2],
-			vec![&nodes[2], &nodes[3],],
-			scid_aliases,
-			false
-		);
-	}
-
 	#[test]
 	#[cfg(feature = "std")]
 	fn test_multi_node_with_only_public_channels_hints_includes_only_phantom_route() {

lightning/src/ln/channel.rs

@@ -631,7 +631,6 @@ pub(super) struct Channel<Signer: Sign> {
 	#[cfg(not(test))]
 	holder_selected_channel_reserve_satoshis: u64,
 
-	counterparty_htlc_minimum_msat: u64,
 	holder_htlc_minimum_msat: u64,
 	#[cfg(test)]
 	pub counterparty_max_accepted_htlcs: u16,
@@ -924,7 +923,6 @@ impl<Signer: Sign> Channel<Signer> {
 			holder_max_htlc_value_in_flight_msat: Self::get_holder_max_htlc_value_in_flight_msat(channel_value_satoshis),
 			counterparty_selected_channel_reserve_satoshis: None, // Filled in in accept_channel
 			holder_selected_channel_reserve_satoshis,
-			counterparty_htlc_minimum_msat: 0,
 			holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
 			counterparty_max_accepted_htlcs: 0,
 			minimum_depth: None, // Filled in in accept_channel
@@ -1230,12 +1228,17 @@ impl<Signer: Sign> Channel<Signer> {
 			holder_max_htlc_value_in_flight_msat: Self::get_holder_max_htlc_value_in_flight_msat(msg.funding_satoshis),
 			counterparty_selected_channel_reserve_satoshis: Some(msg.channel_reserve_satoshis),
 			holder_selected_channel_reserve_satoshis,
-			counterparty_htlc_minimum_msat: msg.htlc_minimum_msat,
 			holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
 			counterparty_max_accepted_htlcs: msg.max_accepted_htlcs,
 			minimum_depth: Some(config.own_channel_config.minimum_depth),
 
-			counterparty_forwarding_info: None,
+			counterparty_forwarding_info: Some(CounterpartyForwardingInfo {
+				fee_base_msat: 0,
+				fee_proportional_millionths: 0,
+				cltv_expiry_delta: 0,
+				outbound_htlc_minimum_msat: Some(msg.htlc_minimum_msat),
+				outbound_htlc_maximum_msat: None,
+			}),
 
 			channel_transaction_parameters: ChannelTransactionParameters {
 				holder_pubkeys: pubkeys,
@@ -1949,8 +1952,15 @@ impl<Signer: Sign> Channel<Signer> {
 		self.counterparty_dust_limit_satoshis = msg.dust_limit_satoshis;
 		self.counterparty_max_htlc_value_in_flight_msat = cmp::min(msg.max_htlc_value_in_flight_msat, self.channel_value_satoshis * 1000);
 		self.counterparty_selected_channel_reserve_satoshis = Some(msg.channel_reserve_satoshis);
-		self.counterparty_htlc_minimum_msat = msg.htlc_minimum_msat;
 		self.counterparty_max_accepted_htlcs = msg.max_accepted_htlcs;
+		self.counterparty_forwarding_info = Some(CounterpartyForwardingInfo {
+			fee_base_msat: 0,
+			fee_proportional_millionths: 0,
+			cltv_expiry_delta: 0,
+			outbound_htlc_minimum_msat: Some(msg.htlc_minimum_msat),
+			outbound_htlc_maximum_msat: self.get_counterparty_htlc_maximum_msat(),
+		});
+
 		self.minimum_depth = Some(msg.minimum_depth);
 
 		let counterparty_pubkeys = ChannelPublicKeys {
@@ -4307,7 +4317,7 @@ impl<Signer: Sign> Channel<Signer> {
 
 	/// Allowed in any state (including after shutdown)
 	pub fn get_counterparty_htlc_minimum_msat(&self) -> u64 {
-		self.counterparty_htlc_minimum_msat
+		self.counterparty_forwarding_info.as_ref().map(|info| info.outbound_htlc_minimum_msat.unwrap_or(0)).unwrap_or(0)
 	}
 
 	/// Allowed in any state (including after shutdown), but will return none before TheirInitSent
@@ -5103,8 +5113,9 @@ impl<Signer: Sign> Channel<Signer> {
 			return Err(ChannelError::Ignore("Cannot send 0-msat HTLC".to_owned()));
 		}
 
-		if amount_msat < self.counterparty_htlc_minimum_msat {
-			return Err(ChannelError::Ignore(format!("Cannot send less than their minimum HTLC value ({})", self.counterparty_htlc_minimum_msat)));
+		let counterparty_htlc_minimum_msat = self.get_counterparty_htlc_minimum_msat();
+		if amount_msat < counterparty_htlc_minimum_msat {
+			return Err(ChannelError::Ignore(format!("Cannot send less than their minimum HTLC value ({})", counterparty_htlc_minimum_msat)));
 		}
 
 		if (self.channel_state & (ChannelState::PeerDisconnected as u32)) != 0 {
@@ -5775,7 +5786,11 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 		// Note that this field is ignored by 0.0.99+ as the TLV Optional variant is used instead.
 		self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0).write(writer)?;
 
-		self.counterparty_htlc_minimum_msat.write(writer)?;
+		// Versions prior to 0.0.107, stored the `outbound_htlc_minimum_msat` field as a
+		// non-optional field of the `Channel` struct, which was written here. It has since been
+		// refactored to be part of the `CounterpartyForwardingInfo` struct.
+		self.counterparty_forwarding_info.as_ref()
+			.map(|info| info.outbound_htlc_minimum_msat.unwrap_or(0)).unwrap_or(0).write(writer)?;
 		self.holder_htlc_minimum_msat.write(writer)?;
 		self.counterparty_max_accepted_htlcs.write(writer)?;
 
@@ -6036,7 +6051,10 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			// Read the 8 bytes of backwards-compatibility data.
 			let _dummy: u64 = Readable::read(reader)?;
 		}
-		let counterparty_htlc_minimum_msat = Readable::read(reader)?;
+		// Versions prior to 0.0.107, stored the `outbound_htlc_minimum_msat` field as a
+		// non-optional field of the `Channel` struct, which has since been refactored to be part
+		// of the `CounterpartyForwardingInfo` struct. It's therefore read here.
+		let outbound_htlc_minimum_msat = Readable::read(reader)?;
 		let holder_htlc_minimum_msat = Readable::read(reader)?;
 		let counterparty_max_accepted_htlcs = Readable::read(reader)?;
 
@@ -6055,7 +6073,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 				fee_base_msat: Readable::read(reader)?,
 				fee_proportional_millionths: Readable::read(reader)?,
 				cltv_expiry_delta: Readable::read(reader)?,
-				outbound_htlc_minimum_msat: Some(counterparty_htlc_minimum_msat),
+				outbound_htlc_minimum_msat: Some(outbound_htlc_minimum_msat),
 				outbound_htlc_maximum_msat: None,
 			}),
 			_ => return Err(DecodeError::InvalidValue),
@@ -6238,7 +6256,6 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			holder_max_htlc_value_in_flight_msat: holder_max_htlc_value_in_flight_msat.unwrap(),
 			counterparty_selected_channel_reserve_satoshis,
 			holder_selected_channel_reserve_satoshis: holder_selected_channel_reserve_satoshis.unwrap(),
-			counterparty_htlc_minimum_msat,
 			holder_htlc_minimum_msat,
 			counterparty_max_accepted_htlcs,
 			minimum_depth,