Support parsing persisted v2 channel monitor names

ChannelMonitors are persisted using the corresponding channel's funding
outpoint for the key. This is fine for v1 channels since the funding
output will never change. However, this is not the case for v2 channels
since a splice will result in a new funding outpoint.

Support parsing a MonitorName for v2 channels as a ChannelId, which
still looks like a outpoint only without an index. This allows
differentiating monitors for v1 channels from those of v2 channels.

Author: jkczyz

lightning/src/util/persist.rs

@@ -11,6 +11,7 @@
 //! [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
 
 use bitcoin::{BlockHash, Txid};
+use bitcoin::hashes::hex::FromHex;
 use core::cmp;
 use core::ops::Deref;
 use core::str::FromStr;
@@ -24,6 +25,7 @@ use crate::chain::chainmonitor::Persist;
 use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate};
 use crate::chain::transaction::OutPoint;
 use crate::ln::channelmanager::AChannelManager;
+use crate::ln::types::ChannelId;
 use crate::routing::gossip::NetworkGraph;
 use crate::routing::scoring::WriteableScore;
 use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, SignerProvider};
@@ -896,9 +898,10 @@ where
 /// A struct representing a name for a channel monitor.
 ///
 /// `MonitorName` is primarily used within the [`MonitorUpdatingPersister`]
-/// in functions that store or retrieve channel monitor snapshots.
+/// in functions that store or retrieve [`ChannelMonitor`] snapshots.
 /// It provides a consistent way to generate a unique key for channel
-/// monitors based on their funding outpoints.
+/// monitors based on the channel's funding [`OutPoint`] for v1 channels or
+/// [`ChannelId`] for v2 channels.
 ///
 /// While users of the Lightning Dev Kit library generally won't need
 /// to interact with [`MonitorName`] directly, it can be useful for:
@@ -912,30 +915,49 @@ where
 /// use std::str::FromStr;
 ///
 /// use bitcoin::Txid;
+/// use bitcoin::hashes::hex::FromHex;
 ///
 /// use lightning::util::persist::MonitorName;
 /// use lightning::chain::transaction::OutPoint;
+/// use lightning::ln::types::ChannelId;
 ///
+/// // v1 channel
 /// let outpoint = OutPoint {
 ///	 txid: Txid::from_str("deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef").unwrap(),
 ///	 index: 1,
 /// };
 /// let monitor_name = MonitorName::from(outpoint);
 /// assert_eq!(monitor_name.as_str(), "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1");
 ///
+/// // v2 channel
+/// let channel_id = ChannelId(<[u8; 32]>::from_hex("deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef").unwrap());
+/// let monitor_name = MonitorName::from(channel_id);
+/// assert_eq!(monitor_name.as_str(), "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
+///
 /// // Using MonitorName to generate a storage key
 /// let storage_key = format!("channel_monitors/{}", monitor_name.as_str());
 /// ```
 #[derive(Debug)]
 pub struct MonitorName(String);
 
+/// The source of the [`MonitorName`], either an [`OutPoint`] for V1 channels or a [`ChannelId`] for
+/// V2 channels. The former uses the channel's funding outpoint for historical reasons. The latter
+/// uses the channel's id because the funding outpoint may change during splicing.
+pub enum MonitorNameSource {
+	/// The outpoint of the channel's funding transaction.
+	V1Channel(OutPoint),
+
+	/// The id of the channel produced by [`ChannelId::v2_from_revocation_basepoints`].
+	V2Channel(ChannelId),
+}
+
 impl MonitorName {
 	/// Constructs a [`MonitorName`], after verifying that an [`OutPoint`] can
 	/// be formed from the given `name`.
 	/// This method is useful if you have a String and you want to verify that
 	/// it's a valid storage key for a channel monitor.
 	pub fn new(name: String) -> Result<Self, io::Error> {
-		MonitorName::do_try_into_outpoint(&name)?;
+		MonitorNameSource::try_from(name.as_str())?;
 		Ok(Self(name))
 	}
 
@@ -945,31 +967,48 @@ impl MonitorName {
 	pub fn as_str(&self) -> &str {
 		&self.0
 	}
+}
+
+impl TryFrom<&str> for MonitorNameSource {
+	type Error = io::Error;
 
-	/// Attempt to form a valid [`OutPoint`] from a given name string.
-	fn do_try_into_outpoint(name: &str) -> Result<OutPoint, io::Error> {
+	/// Attempts to convert a `MonitorName` back into an `OutPoint` or `ChannelId`.
+	///
+	/// This is useful when you have a `MonitorName` (perhaps retrieved from storage)
+	/// and need to reconstruct the original data it represents.
+	fn try_from(name: &str) -> Result<Self, io::Error> {
 		let mut parts = name.splitn(2, '_');
-		let txid = if let Some(part) = parts.next() {
-			Txid::from_str(part).map_err(|_| {
+		let id = parts
+			.next()
+			.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "Empty stored key"))?;
+
+		if let Some(part) = parts.next() {
+			let txid = Txid::from_str(id).map_err(|_| {
 				io::Error::new(io::ErrorKind::InvalidData, "Invalid tx ID in stored key")
-			})?
-		} else {
-			return Err(io::Error::new(
-				io::ErrorKind::InvalidData,
-				"Stored monitor key is not a splittable string",
-			));
-		};
-		let index = if let Some(part) = parts.next() {
-			part.parse().map_err(|_| {
+			})?;
+			let index: u16 = part.parse().map_err(|_| {
 				io::Error::new(io::ErrorKind::InvalidData, "Invalid tx index in stored key")
-			})?
+			})?;
+			let outpoint = OutPoint { txid, index };
+			Ok(MonitorNameSource::V1Channel(outpoint))
 		} else {
-			return Err(io::Error::new(
-				io::ErrorKind::InvalidData,
-				"No tx index value found after underscore in stored key",
-			));
-		};
-		Ok(OutPoint { txid, index })
+			let bytes = <[u8; 32]>::from_hex(id).map_err(|_| {
+				io::Error::new(io::ErrorKind::InvalidData, "Invalid channel ID in stored key")
+			})?;
+			Ok(MonitorNameSource::V2Channel(ChannelId(bytes)))
+		}
+	}
+}
+
+impl TryFrom<&MonitorName> for MonitorNameSource {
+	type Error = io::Error;
+
+	/// Attempts to convert a `MonitorName` back into an `OutPoint` or `ChannelId`.
+	///
+	/// This is useful when you have a `MonitorName` (perhaps retrieved from storage)
+	/// and need to reconstruct the original data it represents.
+	fn try_from(value: &MonitorName) -> Result<Self, io::Error> {
+		MonitorNameSource::try_from(value.0.as_str())
 	}
 }
 
@@ -981,20 +1020,34 @@ impl TryFrom<&MonitorName> for OutPoint {
 	/// This is useful when you have a `MonitorName` (perhaps retrieved from storage)
 	/// and need to reconstruct the original `OutPoint` it represents.
 	fn try_from(value: &MonitorName) -> Result<Self, io::Error> {
-		MonitorName::do_try_into_outpoint(&value.0)
+		if let MonitorNameSource::V1Channel(outpoint) = MonitorNameSource::try_from(value)? {
+			Ok(outpoint)
+		} else {
+			Err(io::Error::new(io::ErrorKind::InvalidData, "Invalid outpoint in stored key"))
+		}
 	}
 }
 
 impl From<OutPoint> for MonitorName {
 	/// Creates a `MonitorName` from an `OutPoint`.
 	///
 	/// This is typically used when you need to generate a storage key or identifier
-	/// for a new or existing channel monitor.
+	/// for a new or existing channel monitor for a v1 channel.
 	fn from(value: OutPoint) -> Self {
 		MonitorName(format!("{}_{}", value.txid.to_string(), value.index))
 	}
 }
 
+impl From<ChannelId> for MonitorName {
+	/// Creates a `MonitorName` from a `ChannelId`.
+	///
+	/// This is typically used when you need to generate a storage key or identifier
+	/// for a new or existing channel monitor for a v2 channel.
+	fn from(id: ChannelId) -> Self {
+		MonitorName(id.to_string())
+	}
+}
+
 /// A struct representing a name for a channel monitor update.
 ///
 /// [`UpdateName`] is primarily used within the [`MonitorUpdatingPersister`] in
@@ -1085,6 +1138,7 @@ impl From<u64> for UpdateName {
 #[cfg(test)]
 mod tests {
 	use super::*;
+	use bitcoin::hashes::hex::FromHex;
 	use crate::chain::ChannelMonitorUpdateStatus;
 	use crate::events::{ClosureReason, MessageSendEventsProvider};
 	use crate::ln::functional_test_utils::*;
@@ -1109,36 +1163,52 @@ mod tests {
 	}
 
 	#[test]
-	fn monitor_from_outpoint_works() {
-		let monitor_name1 = MonitorName::from(OutPoint {
+	fn creates_monitor_from_outpoint() {
+		let monitor_name = MonitorName::from(OutPoint {
 			txid: Txid::from_str(
 				"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
 			)
 			.unwrap(),
 			index: 1,
 		});
 		assert_eq!(
-			monitor_name1.as_str(),
+			monitor_name.as_str(),
 			"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_1"
 		);
+		assert!(matches!(MonitorNameSource::try_from(&monitor_name), Ok(MonitorNameSource::V1Channel(_))));
 
-		let monitor_name2 = MonitorName::from(OutPoint {
+		let monitor_name = MonitorName::from(OutPoint {
 			txid: Txid::from_str(
 				"f33dbeeff33dbeeff33dbeeff33dbeeff33dbeeff33dbeeff33dbeeff33dbeef",
 			)
 			.unwrap(),
 			index: u16::MAX,
 		});
 		assert_eq!(
-			monitor_name2.as_str(),
+			monitor_name.as_str(),
 			"f33dbeeff33dbeeff33dbeeff33dbeeff33dbeeff33dbeeff33dbeeff33dbeef_65535"
 		);
+		assert!(matches!(MonitorNameSource::try_from(&monitor_name), Ok(MonitorNameSource::V1Channel(_))));
+	}
+
+	#[test]
+	fn creates_monitor_from_channel_id() {
+		let monitor_name = MonitorName::from(
+			ChannelId(<[u8; 32]>::from_hex(
+				"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
+			).unwrap())
+		);
+		assert_eq!(
+			monitor_name.as_str(),
+			"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
+		);
+		assert!(matches!(MonitorNameSource::try_from(&monitor_name), Ok(MonitorNameSource::V2Channel(_))));
 	}
 
 	#[test]
-	fn bad_monitor_string_fails() {
+	fn fails_parsing_monitor_name() {
 		assert!(MonitorName::new(
-			"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef".to_string()
+			"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_".to_string()
 		)
 		.is_err());
 		assert!(MonitorName::new(