Move ChannelContext::short_channel_id to FundingScope

When processing confirmed transactions, if the funding transaction is
found then information about it in the ChannelContext is updated. In
preparation for splicing, move this data to FundingScope.

Author: jkczyz

lightning/src/ln/channel.rs

@@ -1688,6 +1688,7 @@ pub(super) struct FundingScope {
 	/// The hash of the block in which the funding transaction was included.
 	funding_tx_confirmed_in: Option<BlockHash>,
 	funding_tx_confirmation_height: u32,
+	short_channel_id: Option<u64>,
 }
 
 impl Writeable for FundingScope {
@@ -1700,6 +1701,7 @@ impl Writeable for FundingScope {
 			(9, self.funding_transaction, option),
 			(11, self.funding_tx_confirmed_in, option),
 			(13, self.funding_tx_confirmation_height, required),
+			(15, self.short_channel_id, option),
 		});
 		Ok(())
 	}
@@ -1714,6 +1716,7 @@ impl Readable for FundingScope {
 		let mut funding_transaction = None;
 		let mut funding_tx_confirmed_in = None;
 		let mut funding_tx_confirmation_height = RequiredWrapper(None);
+		let mut short_channel_id = None;
 
 		read_tlv_fields!(reader, {
 			(1, value_to_self_msat, required),
@@ -1723,6 +1726,7 @@ impl Readable for FundingScope {
 			(9, funding_transaction, option),
 			(11, funding_tx_confirmed_in, option),
 			(13, funding_tx_confirmation_height, required),
+			(15, short_channel_id, option),
 		});
 
 		Ok(Self {
@@ -1737,6 +1741,7 @@ impl Readable for FundingScope {
 			funding_transaction,
 			funding_tx_confirmed_in,
 			funding_tx_confirmation_height: funding_tx_confirmation_height.0.unwrap(),
+			short_channel_id,
 			#[cfg(any(test, fuzzing))]
 			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
 			#[cfg(any(test, fuzzing))]
@@ -1835,6 +1840,13 @@ impl FundingScope {
 
 		height.checked_sub(self.funding_tx_confirmation_height).map_or(0, |c| c + 1)
 	}
+
+	/// Gets the channel's `short_channel_id`.
+	///
+	/// Will return `None` if the funding hasn't been confirmed yet.
+	pub fn get_short_channel_id(&self) -> Option<u64> {
+		self.short_channel_id
+	}
 }
 
 /// Info about a pending splice, used in the pre-splice channel
@@ -1994,7 +2006,6 @@ pub(super) struct ChannelContext<SP: Deref> where SP::Target: SignerProvider {
 	/// milliseconds, so any accidental force-closes here should be exceedingly rare.
 	expecting_peer_commitment_signed: bool,
 
-	short_channel_id: Option<u64>,
 	/// Either the height at which this channel was created or the height at which it was last
 	/// serialized if it was serialized by versions prior to 0.0.103.
 	/// We use this to close if funding is never broadcasted.
@@ -2805,6 +2816,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			funding_transaction: None,
 			funding_tx_confirmed_in: None,
 			funding_tx_confirmation_height: 0,
+			short_channel_id: None,
 		};
 		let channel_context = ChannelContext {
 			user_id,
@@ -2868,7 +2880,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			closing_fee_limits: None,
 			target_closing_feerate_sats_per_kw: None,
 
-			short_channel_id: None,
 			channel_creation_height: current_chain_height,
 
 			feerate_per_kw: open_channel_fields.commitment_feerate_sat_per_1000_weight,
@@ -3041,6 +3052,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			funding_transaction: None,
 			funding_tx_confirmed_in: None,
 			funding_tx_confirmation_height: 0,
+			short_channel_id: None,
 		};
 		let channel_context = Self {
 			user_id,
@@ -3102,7 +3114,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			closing_fee_limits: None,
 			target_closing_feerate_sats_per_kw: None,
 
-			short_channel_id: None,
 			channel_creation_height: current_chain_height,
 
 			feerate_per_kw: commitment_feerate,
@@ -3312,13 +3323,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 		self.user_id
 	}
 
-	/// Gets the channel's `short_channel_id`.
-	///
-	/// Will return `None` if the channel hasn't been confirmed yet.
-	pub fn get_short_channel_id(&self) -> Option<u64> {
-		self.short_channel_id
-	}
-
 	/// Allowed in any state (including after shutdown)
 	pub fn latest_inbound_scid_alias(&self) -> Option<u64> {
 		self.latest_inbound_scid_alias
@@ -4862,7 +4866,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel is not currently usable".to_owned()));
 		}
 
-		let short_channel_id = self.get_short_channel_id()
+		let short_channel_id = funding.get_short_channel_id()
 			.ok_or(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel has not been confirmed yet".to_owned()))?;
 		let node_id = NodeId::from_pubkey(&node_signer.get_node_id(Recipient::Node)
 			.map_err(|_| ChannelError::Ignore("Failed to retrieve own public key".to_owned()))?);
@@ -4934,7 +4938,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 					},
 					Ok(v) => v
 				};
-				let short_channel_id = match self.get_short_channel_id() {
+				let short_channel_id = match funding.get_short_channel_id() {
 					Some(scid) => scid,
 					None => return None,
 				};
@@ -5634,7 +5638,7 @@ impl<SP: Deref> FundedChannel<SP> where
 		}
 
 		if let Some(scid_alias) = msg.short_channel_id_alias {
-			if Some(scid_alias) != self.context.short_channel_id {
+			if Some(scid_alias) != self.funding.short_channel_id {
 				// The scid alias provided can be used to route payments *from* our counterparty,
 				// i.e. can be used for inbound payments and provided in invoices, but is not used
 				// when routing outbound payments.
@@ -8314,7 +8318,7 @@ impl<SP: Deref> FundedChannel<SP> where
 
 							self.funding.funding_tx_confirmation_height = height;
 							self.funding.funding_tx_confirmed_in = Some(*block_hash);
-							self.context.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) {
+							self.funding.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) {
 								Ok(scid) => Some(scid),
 								Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"),
 							};
@@ -10730,7 +10734,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 
 		self.funding.funding_tx_confirmed_in.write(writer)?;
 		self.funding.funding_tx_confirmation_height.write(writer)?;
-		self.context.short_channel_id.write(writer)?;
+		self.funding.short_channel_id.write(writer)?;
 
 		self.context.counterparty_dust_limit_satoshis.write(writer)?;
 		self.context.holder_dust_limit_satoshis.write(writer)?;
@@ -11366,6 +11370,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 				funding_transaction,
 				funding_tx_confirmed_in,
 				funding_tx_confirmation_height,
+				short_channel_id,
 			},
 			pending_funding: pending_funding.unwrap(),
 			context: ChannelContext {
@@ -11429,7 +11434,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 				closing_fee_limits: None,
 				target_closing_feerate_sats_per_kw,
 
-				short_channel_id,
 				channel_creation_height,
 
 				counterparty_dust_limit_satoshis,

lightning/src/ln/channel_state.rs

@@ -516,7 +516,7 @@ impl ChannelDetails {
 			} else {
 				None
 			},
-			short_channel_id: context.get_short_channel_id(),
+			short_channel_id: funding.get_short_channel_id(),
 			outbound_scid_alias: if context.is_usable() {
 				Some(context.outbound_scid_alias())
 			} else {

lightning/src/ln/channelmanager.rs

@@ -3031,7 +3031,7 @@ macro_rules! locked_close_channel {
 			$peer_state.closed_channel_monitor_update_ids.insert(chan_id, update_id);
 		}
 		let mut short_to_chan_info = $self.short_to_chan_info.write().unwrap();
-		if let Some(short_id) = $channel_context.get_short_channel_id() {
+		if let Some(short_id) = $channel_funding.get_short_channel_id() {
 			short_to_chan_info.remove(&short_id);
 		} else {
 			// If the channel was never confirmed on-chain prior to its closure, remove the
@@ -3147,7 +3147,7 @@ macro_rules! send_channel_ready {
 		let outbound_alias_insert = short_to_chan_info.insert($channel.context.outbound_scid_alias(), ($channel.context.get_counterparty_node_id(), $channel.context.channel_id()));
 		assert!(outbound_alias_insert.is_none() || outbound_alias_insert.unwrap() == ($channel.context.get_counterparty_node_id(), $channel.context.channel_id()),
 			"SCIDs should never collide - ensure you weren't behind the chain tip by a full month when creating channels");
-		if let Some(real_scid) = $channel.context.get_short_channel_id() {
+		if let Some(real_scid) = $channel.funding.get_short_channel_id() {
 			let scid_insert = short_to_chan_info.insert(real_scid, ($channel.context.get_counterparty_node_id(), $channel.context.channel_id()));
 			assert!(scid_insert.is_none() || scid_insert.unwrap() == ($channel.context.get_counterparty_node_id(), $channel.context.channel_id()),
 				"SCIDs should never collide - ensure you weren't behind the chain tip by a full month when creating channels");
@@ -4572,7 +4572,7 @@ where
 				action: msgs::ErrorAction::IgnoreError
 			});
 		}
-		if chan.context.get_short_channel_id().is_none() {
+		if chan.funding.get_short_channel_id().is_none() {
 			return Err(LightningError{err: "Channel not yet established".to_owned(), action: msgs::ErrorAction::IgnoreError});
 		}
 		let logger = WithChannelContext::from(&self.logger, &chan.context, None);
@@ -4594,7 +4594,7 @@ where
 	fn get_channel_update_for_unicast(&self, chan: &FundedChannel<SP>) -> Result<msgs::ChannelUpdate, LightningError> {
 		let logger = WithChannelContext::from(&self.logger, &chan.context, None);
 		log_trace!(logger, "Attempting to generate channel update for channel {}", chan.context.channel_id());
-		let short_channel_id = match chan.context.get_short_channel_id().or(chan.context.latest_inbound_scid_alias()) {
+		let short_channel_id = match chan.funding.get_short_channel_id().or(chan.context.latest_inbound_scid_alias()) {
 			None => return Err(LightningError{err: "Channel not yet established".to_owned(), action: msgs::ErrorAction::IgnoreError}),
 			Some(id) => id,
 		};
@@ -5657,7 +5657,7 @@ where
 							err: format!("Channel with id {} not fully established", next_hop_channel_id)
 						})
 					}
-					funded_chan.context.get_short_channel_id().unwrap_or(funded_chan.context.outbound_scid_alias())
+					funded_chan.funding.get_short_channel_id().unwrap_or(funded_chan.context.outbound_scid_alias())
 				} else {
 					return Err(APIError::ChannelUnavailable {
 						err: format!("Channel with id {} for the passed counterparty node_id {} is still opening.",
@@ -6099,7 +6099,7 @@ where
 								};
 
 								let logger = WithChannelContext::from(&self.logger, &optimal_channel.context, Some(payment_hash));
-								let channel_description = if optimal_channel.context.get_short_channel_id() == Some(short_chan_id) {
+								let channel_description = if optimal_channel.funding.get_short_channel_id() == Some(short_chan_id) {
 									"specified"
 								} else {
 									"alternate"
@@ -7663,7 +7663,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 		);
 
 		let counterparty_node_id = channel.context.get_counterparty_node_id();
-		let short_channel_id = channel.context.get_short_channel_id().unwrap_or(channel.context.outbound_scid_alias());
+		let short_channel_id = channel.funding.get_short_channel_id().unwrap_or(channel.context.outbound_scid_alias());
 
 		let mut htlc_forwards = None;
 		if !pending_forwards.is_empty() {
@@ -10913,7 +10913,7 @@ where
 					.iter()
 					.filter(|(_, channel)| channel.context().is_usable())
 					.min_by_key(|(_, channel)| channel.context().channel_creation_height)
-					.and_then(|(_, channel)| channel.context().get_short_channel_id()),
+					.and_then(|(_, channel)| channel.funding().get_short_channel_id()),
 			})
 			.collect::<Vec<_>>();
 
@@ -11776,7 +11776,7 @@ where
 									});
 								}
 								if funded_channel.is_our_channel_ready() {
-									if let Some(real_scid) = funded_channel.context.get_short_channel_id() {
+									if let Some(real_scid) = funded_channel.funding.get_short_channel_id() {
 										// If we sent a 0conf channel_ready, and now have an SCID, we add it
 										// to the short_to_chan_info map here. Note that we check whether we
 										// can relay using the real SCID at relay-time (i.e.
@@ -13870,7 +13870,7 @@ where
 					log_info!(logger, "Successfully loaded channel {} at update_id {} against monitor at update id {} with {} blocked updates",
 						&channel.context.channel_id(), channel.context.get_latest_monitor_update_id(),
 						monitor.get_latest_update_id(), channel.blocked_monitor_updates_pending());
-					if let Some(short_channel_id) = channel.context.get_short_channel_id() {
+					if let Some(short_channel_id) = channel.funding.get_short_channel_id() {
 						short_to_chan_info.insert(short_channel_id, (channel.context.get_counterparty_node_id(), channel.context.channel_id()));
 					}
 					per_peer_state.entry(channel.context.get_counterparty_node_id())

lightning/src/ln/payment_tests.rs

@@ -1709,7 +1709,7 @@ fn test_trivial_inflight_htlc_tracking(){
 		let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat(
 			&NodeId::from_pubkey(&nodes[0].node.get_our_node_id()) ,
 			&NodeId::from_pubkey(&nodes[1].node.get_our_node_id()),
-			channel_1.context().get_short_channel_id().unwrap()
+			channel_1.funding().get_short_channel_id().unwrap()
 		);
 		assert_eq!(chan_1_used_liquidity, None);
 	}
@@ -1721,7 +1721,7 @@ fn test_trivial_inflight_htlc_tracking(){
 		let chan_2_used_liquidity = inflight_htlcs.used_liquidity_msat(
 			&NodeId::from_pubkey(&nodes[1].node.get_our_node_id()) ,
 			&NodeId::from_pubkey(&nodes[2].node.get_our_node_id()),
-			channel_2.context().get_short_channel_id().unwrap()
+			channel_2.funding().get_short_channel_id().unwrap()
 		);
 
 		assert_eq!(chan_2_used_liquidity, None);
@@ -1746,7 +1746,7 @@ fn test_trivial_inflight_htlc_tracking(){
 		let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat(
 			&NodeId::from_pubkey(&nodes[0].node.get_our_node_id()) ,
 			&NodeId::from_pubkey(&nodes[1].node.get_our_node_id()),
-			channel_1.context().get_short_channel_id().unwrap()
+			channel_1.funding().get_short_channel_id().unwrap()
 		);
 		// First hop accounts for expected 1000 msat fee
 		assert_eq!(chan_1_used_liquidity, Some(501000));
@@ -1759,7 +1759,7 @@ fn test_trivial_inflight_htlc_tracking(){
 		let chan_2_used_liquidity = inflight_htlcs.used_liquidity_msat(
 			&NodeId::from_pubkey(&nodes[1].node.get_our_node_id()) ,
 			&NodeId::from_pubkey(&nodes[2].node.get_our_node_id()),
-			channel_2.context().get_short_channel_id().unwrap()
+			channel_2.funding().get_short_channel_id().unwrap()
 		);
 
 		assert_eq!(chan_2_used_liquidity, Some(500000));
@@ -1785,7 +1785,7 @@ fn test_trivial_inflight_htlc_tracking(){
 		let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat(
 			&NodeId::from_pubkey(&nodes[0].node.get_our_node_id()) ,
 			&NodeId::from_pubkey(&nodes[1].node.get_our_node_id()),
-			channel_1.context().get_short_channel_id().unwrap()
+			channel_1.funding().get_short_channel_id().unwrap()
 		);
 		assert_eq!(chan_1_used_liquidity, None);
 	}
@@ -1797,7 +1797,7 @@ fn test_trivial_inflight_htlc_tracking(){
 		let chan_2_used_liquidity = inflight_htlcs.used_liquidity_msat(
 			&NodeId::from_pubkey(&nodes[1].node.get_our_node_id()) ,
 			&NodeId::from_pubkey(&nodes[2].node.get_our_node_id()),
-			channel_2.context().get_short_channel_id().unwrap()
+			channel_2.funding().get_short_channel_id().unwrap()
 		);
 		assert_eq!(chan_2_used_liquidity, None);
 	}
@@ -1838,7 +1838,7 @@ fn test_holding_cell_inflight_htlcs() {
 		let used_liquidity = inflight_htlcs.used_liquidity_msat(
 			&NodeId::from_pubkey(&nodes[0].node.get_our_node_id()) ,
 			&NodeId::from_pubkey(&nodes[1].node.get_our_node_id()),
-			channel.context().get_short_channel_id().unwrap()
+			channel.funding().get_short_channel_id().unwrap()
 		);
 
 		assert_eq!(used_liquidity, Some(2000000));