Remove Outpoint::to_channel_id method

To avoid confusion and for accuracy going forward, we remove this method
as it is inconsistent with channel IDs generated during V2 channel
establishment. If one wants to create a V1, funding outpoint-based
channel ID, then `ChannelId::v1_from_funding_outpoint` should be used
instead.

A large portion of the library has always made the assumption that having
the funding outpoint will always allow us to generate the channel ID.
This will not be the case anymore and we need to pass the channel ID along
where appropriate. All channels that could have been persisted up to this
point could only have used V1 establishment, so if some structures don't
store a channel ID for them they can safely fall back to the funding
outpoint-based version.

Author: dunxen

fuzz/src/chanmon_consistency.rs

@@ -38,7 +38,7 @@ use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget,
 use lightning::sign::{KeyMaterial, InMemorySigner, Recipient, EntropySource, NodeSigner, SignerProvider};
 use lightning::events;
 use lightning::events::MessageSendEventsProvider;
-use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
+use lightning::ln::{ChannelId, PaymentHash, PaymentPreimage, PaymentSecret};
 use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentSendFailure, ChannelManagerReadArgs, PaymentId, RecipientOnionFields};
 use lightning::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
 use lightning::ln::msgs::{self, CommitmentUpdate, ChannelMessageHandler, DecodeError, UpdateAddHTLC, Init};
@@ -138,16 +138,16 @@ impl TestChainMonitor {
 	}
 }
 impl chain::Watch<TestChannelSigner> for TestChainMonitor {
-	fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<TestChannelSigner>) -> Result<chain::ChannelMonitorUpdateStatus, ()> {
+	fn watch_channel(&self, funding_txo: OutPoint, channel_id: ChannelId, monitor: channelmonitor::ChannelMonitor<TestChannelSigner>) -> Result<chain::ChannelMonitorUpdateStatus, ()> {
 		let mut ser = VecWriter(Vec::new());
 		monitor.write(&mut ser).unwrap();
 		if let Some(_) = self.latest_monitors.lock().unwrap().insert(funding_txo, (monitor.get_latest_update_id(), ser.0)) {
 			panic!("Already had monitor pre-watch_channel");
 		}
-		self.chain_monitor.watch_channel(funding_txo, monitor)
+		self.chain_monitor.watch_channel(funding_txo, channel_id, monitor)
 	}
 
-	fn update_channel(&self, funding_txo: OutPoint, update: &channelmonitor::ChannelMonitorUpdate) -> chain::ChannelMonitorUpdateStatus {
+	fn update_channel(&self, funding_txo: OutPoint, channel_id: ChannelId, update: &channelmonitor::ChannelMonitorUpdate) -> chain::ChannelMonitorUpdateStatus {
 		let mut map_lock = self.latest_monitors.lock().unwrap();
 		let mut map_entry = match map_lock.entry(funding_txo) {
 			hash_map::Entry::Occupied(entry) => entry,
@@ -159,10 +159,10 @@ impl chain::Watch<TestChannelSigner> for TestChainMonitor {
 		let mut ser = VecWriter(Vec::new());
 		deserialized_monitor.write(&mut ser).unwrap();
 		map_entry.insert((update.update_id, ser.0));
-		self.chain_monitor.update_channel(funding_txo, update)
+		self.chain_monitor.update_channel(funding_txo, channel_id, update)
 	}
 
-	fn release_pending_monitor_events(&self) -> Vec<(OutPoint, Vec<MonitorEvent>, Option<PublicKey>)> {
+	fn release_pending_monitor_events(&self) -> Vec<(OutPoint, Option<ChannelId>, Vec<MonitorEvent>, Option<PublicKey>)> {
 		return self.chain_monitor.release_pending_monitor_events();
 	}
 }
@@ -510,7 +510,8 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
 
 			let res = (<(BlockHash, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor.clone());
 			for (funding_txo, mon) in monitors.drain() {
-				assert_eq!(chain_monitor.chain_monitor.watch_channel(funding_txo, mon),
+				let channel_id = mon.get_channel_id().unwrap_or(ChannelId::v1_from_funding_outpoint(funding_txo));
+				assert_eq!(chain_monitor.chain_monitor.watch_channel(funding_txo, channel_id, mon),
 					Ok(ChannelMonitorUpdateStatus::Completed));
 			}
 			res
@@ -675,7 +676,9 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
 	lock_fundings!(nodes);
 
 	let chan_a = nodes[0].list_usable_channels()[0].short_channel_id.unwrap();
+	let chan_1_id = Some(nodes[0].list_usable_channels()[0].channel_id);
 	let chan_b = nodes[2].list_usable_channels()[0].short_channel_id.unwrap();
+	let chan_2_id = Some(nodes[2].list_usable_channels()[0].channel_id);
 
 	let mut payment_id: u8 = 0;
 	let mut payment_idx: u64 = 0;
@@ -1031,25 +1034,25 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
 
 			0x08 => {
 				if let Some((id, _)) = monitor_a.latest_monitors.lock().unwrap().get(&chan_1_funding) {
-					monitor_a.chain_monitor.force_channel_monitor_updated(chan_1_funding, *id);
+					monitor_a.chain_monitor.force_channel_monitor_updated(chan_1_funding, chan_1_id, *id);
 					nodes[0].process_monitor_events();
 				}
 			},
 			0x09 => {
 				if let Some((id, _)) = monitor_b.latest_monitors.lock().unwrap().get(&chan_1_funding) {
-					monitor_b.chain_monitor.force_channel_monitor_updated(chan_1_funding, *id);
+					monitor_b.chain_monitor.force_channel_monitor_updated(chan_1_funding, chan_1_id, *id);
 					nodes[1].process_monitor_events();
 				}
 			},
 			0x0a => {
 				if let Some((id, _)) = monitor_b.latest_monitors.lock().unwrap().get(&chan_2_funding) {
-					monitor_b.chain_monitor.force_channel_monitor_updated(chan_2_funding, *id);
+					monitor_b.chain_monitor.force_channel_monitor_updated(chan_2_funding, chan_2_id, *id);
 					nodes[1].process_monitor_events();
 				}
 			},
 			0x0b => {
 				if let Some((id, _)) = monitor_c.latest_monitors.lock().unwrap().get(&chan_2_funding) {
-					monitor_c.chain_monitor.force_channel_monitor_updated(chan_2_funding, *id);
+					monitor_c.chain_monitor.force_channel_monitor_updated(chan_2_funding, chan_2_id, *id);
 					nodes[2].process_monitor_events();
 				}
 			},
@@ -1270,19 +1273,19 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
 				*monitor_c.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
 
 				if let Some((id, _)) = monitor_a.latest_monitors.lock().unwrap().get(&chan_1_funding) {
-					monitor_a.chain_monitor.force_channel_monitor_updated(chan_1_funding, *id);
+					monitor_a.chain_monitor.force_channel_monitor_updated(chan_1_funding, chan_1_id, *id);
 					nodes[0].process_monitor_events();
 				}
 				if let Some((id, _)) = monitor_b.latest_monitors.lock().unwrap().get(&chan_1_funding) {
-					monitor_b.chain_monitor.force_channel_monitor_updated(chan_1_funding, *id);
+					monitor_b.chain_monitor.force_channel_monitor_updated(chan_1_funding, chan_1_id, *id);
 					nodes[1].process_monitor_events();
 				}
 				if let Some((id, _)) = monitor_b.latest_monitors.lock().unwrap().get(&chan_2_funding) {
-					monitor_b.chain_monitor.force_channel_monitor_updated(chan_2_funding, *id);
+					monitor_b.chain_monitor.force_channel_monitor_updated(chan_2_funding, chan_1_id, *id);
 					nodes[1].process_monitor_events();
 				}
 				if let Some((id, _)) = monitor_c.latest_monitors.lock().unwrap().get(&chan_2_funding) {
-					monitor_c.chain_monitor.force_channel_monitor_updated(chan_2_funding, *id);
+					monitor_c.chain_monitor.force_channel_monitor_updated(chan_2_funding, chan_1_id, *id);
 					nodes[2].process_monitor_events();
 				}
 

fuzz/src/full_stack.rs

@@ -622,7 +622,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 						if let None = loss_detector.txids_confirmed.get(&funding_txid) {
 							let outpoint = OutPoint { txid: funding_txid, index: 0 };
 							for chan in channelmanager.list_channels() {
-								if chan.channel_id == outpoint.to_channel_id() {
+								if chan.channel_id == funding_generation.0 {
 									tx.version += 1;
 									continue 'search_loop;
 								}

fuzz/src/utils/test_persister.rs

@@ -2,6 +2,7 @@ use lightning::chain;
 use lightning::chain::{chainmonitor, channelmonitor};
 use lightning::chain::chainmonitor::MonitorUpdateId;
 use lightning::chain::transaction::OutPoint;
+use lightning::ln::ChannelId;
 use lightning::util::test_channel_signer::TestChannelSigner;
 
 use std::sync::Mutex;
@@ -10,11 +11,11 @@ pub struct TestPersister {
 	pub update_ret: Mutex<chain::ChannelMonitorUpdateStatus>,
 }
 impl chainmonitor::Persist<TestChannelSigner> for TestPersister {
-	fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
+	fn persist_new_channel(&self, _funding_txo: OutPoint, _channel_id: ChannelId, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
 		self.update_ret.lock().unwrap().clone()
 	}
 
-	fn update_persisted_channel(&self, _funding_txo: OutPoint, _update: Option<&channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
+	fn update_persisted_channel(&self, _funding_txo: OutPoint, _channel_id: ChannelId, _update: Option<&channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<TestChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
 		self.update_ret.lock().unwrap().clone()
 	}
 }

lightning-background-processor/src/lib.rs

@@ -896,7 +896,7 @@ mod tests {
 	use lightning::chain::transaction::OutPoint;
 	use lightning::events::{Event, PathFailure, MessageSendEventsProvider, MessageSendEvent};
 	use lightning::{get_event_msg, get_event};
-	use lightning::ln::PaymentHash;
+	use lightning::ln::{PaymentHash, ChannelId};
 	use lightning::ln::channelmanager;
 	use lightning::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChainParameters, MIN_CLTV_EXPIRY_DELTA, PaymentId};
 	use lightning::ln::features::{ChannelFeatures, NodeFeatures};
@@ -1380,7 +1380,7 @@ mod tests {
 		}
 
 		// Force-close the channel.
-		nodes[0].node.force_close_broadcasting_latest_txn(&OutPoint { txid: tx.txid(), index: 0 }.to_channel_id(), &nodes[1].node.get_our_node_id()).unwrap();
+		nodes[0].node.force_close_broadcasting_latest_txn(&ChannelId::v1_from_funding_outpoint(OutPoint { txid: tx.txid(), index: 0 }), &nodes[1].node.get_our_node_id()).unwrap();
 
 		// Check that the force-close updates are persisted.
 		check_persisted_data!(nodes[0].node, filepath.clone());

lightning-block-sync/src/init.rs

@@ -49,6 +49,7 @@ BlockSourceResult<ValidatedBlockHeader> where B::Target: BlockSource {
 /// use lightning::chain::chaininterface::FeeEstimator;
 /// use lightning::sign;
 /// use lightning::sign::{EntropySource, NodeSigner, SignerProvider};
+/// use lightning::ln::{ChannelId};
 /// use lightning::ln::channelmanager::{ChannelManager, ChannelManagerReadArgs};
 /// use lightning::routing::router::Router;
 /// use lightning::util::config::UserConfig;
@@ -119,7 +120,9 @@ BlockSourceResult<ValidatedBlockHeader> where B::Target: BlockSource {
 ///
 /// 	// Allow the chain monitor to watch any channels.
 /// 	let monitor = monitor_listener.0;
-/// 	chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);
+/// 	let funding_outpoint = monitor.get_funding_txo().0;
+/// 	let channel_id = monitor.get_channel_id().unwrap_or(ChannelId::v1_from_funding_outpoint(funding_outpoint));
+/// 	chain_monitor.watch_channel(funding_outpoint, channel_id, monitor);
 ///
 /// 	// Create an SPV client to notify the chain monitor and channel manager of block events.
 /// 	let chain_poller = poll::ChainPoller::new(block_source, Network::Bitcoin);

lightning-persister/src/fs_store.rs

@@ -441,6 +441,8 @@ mod tests {
 	#[cfg(not(target_os = "windows"))]
 	#[test]
 	fn test_readonly_dir_perm_failure() {
+    use lightning::ln::ChannelId;
+
 		let store = FilesystemStore::new("test_readonly_dir_perm_failure".into());
 		fs::create_dir_all(&store.get_data_dir()).unwrap();
 
@@ -455,7 +457,9 @@ mod tests {
 		check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000);
 		let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap();
 		let update_map = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap();
-		let update_id = update_map.get(&added_monitors[0].0.to_channel_id()).unwrap();
+		let funding_outpoint = &added_monitors[0].0;
+		let channel_id = &added_monitors[0].1.get_channel_id().unwrap_or(ChannelId::v1_from_funding_outpoint(*funding_outpoint));
+		let update_id = update_map.get(&channel_id).unwrap();
 
 		// Set the store's directory to read-only, which should result in
 		// returning an unrecoverable failure when we then attempt to persist a
@@ -469,7 +473,7 @@ mod tests {
 			txid: Txid::from_str("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(),
 			index: 0
 		};
-		match store.persist_new_channel(test_txo, &added_monitors[0].1, update_id.2) {
+		match store.persist_new_channel(test_txo, *channel_id, &added_monitors[0].1, update_id.2) {
 			ChannelMonitorUpdateStatus::UnrecoverableError => {},
 			_ => panic!("unexpected result from persisting new channel")
 		}
@@ -494,7 +498,9 @@ mod tests {
 		check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000);
 		let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap();
 		let update_map = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap();
-		let update_id = update_map.get(&added_monitors[0].0.to_channel_id()).unwrap();
+		let funding_outpoint = &added_monitors[0].0;
+		let channel_id = &added_monitors[0].1.get_channel_id().unwrap_or(ChannelId::v1_from_funding_outpoint(*funding_outpoint));
+		let update_id = update_map.get(*channel_id).unwrap();
 
 		// Create the store with an invalid directory name and test that the
 		// channel fails to open because the directories fail to be created. There