Merge pull request #819 from TheBlueMatt/2021-03-810-rebased

Change ChannelManager deserialization to return an optional blockhash

Author: TheBlueMatt

fuzz/src/chanmon_consistency.rs

@@ -126,7 +126,7 @@ impl chain::Watch<EnforcingSigner> for TestChainMonitor {
 			hash_map::Entry::Occupied(entry) => entry,
 			hash_map::Entry::Vacant(_) => panic!("Didn't have monitor on update call"),
 		};
-		let deserialized_monitor = <(BlockHash, channelmonitor::ChannelMonitor<EnforcingSigner>)>::
+		let deserialized_monitor = <(Option<BlockHash>, channelmonitor::ChannelMonitor<EnforcingSigner>)>::
 			read(&mut Cursor::new(&map_entry.get().1), &OnlyReadsKeysInterface {}).unwrap().1;
 		deserialized_monitor.update_monitor(&update, &&TestBroadcaster{}, &&FuzzEstimator{}, &self.logger).unwrap();
 		let mut ser = VecWriter(Vec::new());
@@ -337,7 +337,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 			let mut monitors = HashMap::new();
 			let mut old_monitors = $old_monitors.latest_monitors.lock().unwrap();
 			for (outpoint, (update_id, monitor_ser)) in old_monitors.drain() {
-				monitors.insert(outpoint, <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(&monitor_ser), &OnlyReadsKeysInterface {}).expect("Failed to read monitor").1);
+				monitors.insert(outpoint, <(Option<BlockHash>, ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(&monitor_ser), &OnlyReadsKeysInterface {}).expect("Failed to read monitor").1);
 				chain_monitor.latest_monitors.lock().unwrap().insert(outpoint, (update_id, monitor_ser));
 			}
 			let mut monitor_refs = HashMap::new();
@@ -355,7 +355,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 				channel_monitors: monitor_refs,
 			};
 
-			(<(BlockHash, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor)
+			(<(Option<BlockHash>, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor)
 		} }
 	}
 

fuzz/src/chanmon_deser.rs

@@ -25,11 +25,13 @@ impl Writer for VecWriter {
 
 #[inline]
 pub fn do_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
-	if let Ok((latest_block_hash, monitor)) = <(BlockHash, channelmonitor::ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(data), &OnlyReadsKeysInterface {}) {
+	if let Ok((Some(latest_block_hash), monitor)) = <(Option<BlockHash>, channelmonitor::ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(data), &OnlyReadsKeysInterface {}) {
 		let mut w = VecWriter(Vec::new());
 		monitor.write(&mut w).unwrap();
-		let deserialized_copy = <(BlockHash, channelmonitor::ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(&w.0), &OnlyReadsKeysInterface {}).unwrap();
-		assert!(latest_block_hash == deserialized_copy.0);
+		let deserialized_copy = <(Option<BlockHash>, channelmonitor::ChannelMonitor<EnforcingSigner>)>::read(&mut Cursor::new(&w.0), &OnlyReadsKeysInterface {}).unwrap();
+		if let Some(deserialized) = deserialized_copy.0 {
+			assert!(latest_block_hash == deserialized);
+		}
 		assert!(monitor == deserialized_copy.1);
 	}
 }

lightning-block-sync/src/init.rs

@@ -73,12 +73,12 @@ BlockSourceResult<ValidatedBlockHeader> {
 /// ) {
 /// 	// Read a serialized channel monitor paired with the block hash when it was persisted.
 /// 	let serialized_monitor = "...";
-/// 	let (monitor_block_hash, mut monitor) = <(BlockHash, ChannelMonitor<S>)>::read(
+/// 	let (monitor_block_hash_option, mut monitor) = <(Option<BlockHash>, ChannelMonitor<S>)>::read(
 /// 		&mut Cursor::new(&serialized_monitor), keys_manager).unwrap();
 ///
 /// 	// Read the channel manager paired with the block hash when it was persisted.
 /// 	let serialized_manager = "...";
-/// 	let (manager_block_hash, mut manager) = {
+/// 	let (manager_block_hash_option, mut manager) = {
 /// 		let read_args = ChannelManagerReadArgs::new(
 /// 			keys_manager,
 /// 			fee_estimator,
@@ -88,17 +88,20 @@ BlockSourceResult<ValidatedBlockHeader> {
 /// 			config,
 /// 			vec![&mut monitor],
 /// 		);
-/// 		<(BlockHash, ChannelManager<S, &ChainMonitor<S, &C, &T, &F, &L, &P>, &T, &K, &F, &L>)>::read(
+/// 		<(Option<BlockHash>, ChannelManager<S, &ChainMonitor<S, &C, &T, &F, &L, &P>, &T, &K, &F, &L>)>::read(
 /// 			&mut Cursor::new(&serialized_manager), read_args).unwrap()
 /// 	};
 ///
 /// 	// Synchronize any channel monitors and the channel manager to be on the best block.
 /// 	let mut cache = UnboundedCache::new();
 /// 	let mut monitor_listener = (monitor, &*tx_broadcaster, &*fee_estimator, &*logger);
-/// 	let listeners = vec![
-/// 		(monitor_block_hash, &mut monitor_listener as &mut dyn chain::Listen),
-/// 		(manager_block_hash, &mut manager as &mut dyn chain::Listen),
-/// 	];
+/// 	let mut listeners = vec![];
+/// 	if let Some(monitor_block_hash) = monitor_block_hash_option {
+/// 		listeners.push((monitor_block_hash, &mut monitor_listener as &mut dyn chain::Listen))
+/// 	}
+/// 	if let Some(manager_block_hash) = manager_block_hash_option {
+/// 		listeners.push((manager_block_hash, &mut manager as &mut dyn chain::Listen))
+/// 	}
 /// 	let chain_tip = init::synchronize_listeners(
 /// 		block_source, Network::Bitcoin, &mut cache, listeners).await.unwrap();
 ///

lightning-persister/src/lib.rs

@@ -124,7 +124,7 @@ impl FilesystemPersister {
 				if contents.is_err() { return Err(ChannelMonitorUpdateErr::PermanentFailure); }
 
 				if let Ok((_, loaded_monitor)) =
-					<(BlockHash, ChannelMonitor<Keys::Signer>)>::read(&mut Cursor::new(&contents.unwrap()), keys) {
+					<(Option<BlockHash>, ChannelMonitor<Keys::Signer>)>::read(&mut Cursor::new(&contents.unwrap()), keys) {
 						res.insert(OutPoint { txid: txid.unwrap(), index: index.unwrap() }, loaded_monitor);
 					} else {
 						return Err(ChannelMonitorUpdateErr::PermanentFailure);

lightning/src/chain/channelmonitor.rs

@@ -620,7 +620,7 @@ impl Readable for ChannelMonitorUpdateStep {
 /// reloaded at deserialize-time. Thus, you must ensure that, when handling events, all events
 /// gotten are fully handled before re-serializing the new state.
 ///
-/// Note that the deserializer is only implemented for (Sha256dHash, ChannelMonitor), which
+/// Note that the deserializer is only implemented for (Option<BlockHash>, ChannelMonitor), which
 /// tells you the last block hash which was block_connect()ed. You MUST rescan any blocks along
 /// the "reorg path" (ie disconnecting blocks until you find a common ancestor from both the
 /// returned block hash and the the current chain and then reconnecting blocks to get to the
@@ -2492,7 +2492,7 @@ where
 const MAX_ALLOC_SIZE: usize = 64*1024;
 
 impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
-		for (BlockHash, ChannelMonitor<Signer>) {
+		for (Option<BlockHash>, ChannelMonitor<Signer>) {
 	fn read<R: ::std::io::Read>(reader: &mut R, keys_manager: &'a K) -> Result<Self, DecodeError> {
 		macro_rules! unwrap_obj {
 			($key: expr) => {
@@ -2735,7 +2735,13 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
 
-		Ok((last_block_hash.clone(), ChannelMonitor {
+		let last_seen_block_hash = if last_block_hash == Default::default() {
+			None
+		} else {
+			Some(last_block_hash)
+		};
+
+		Ok((last_seen_block_hash, ChannelMonitor {
 			inner: Mutex::new(ChannelMonitorImpl {
 				latest_update_id,
 				commitment_transaction_number_obscure_factor,

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -106,7 +106,7 @@ fn test_monitor_and_persister_update_fail() {
 		let monitor = monitors.get(&outpoint).unwrap();
 		let mut w = test_utils::TestVecWriter(Vec::new());
 		monitor.write(&mut w).unwrap();
-		let new_monitor = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
+		let new_monitor = <(Option<BlockHash>, ChannelMonitor<EnforcingSigner>)>::read(
 			&mut ::std::io::Cursor::new(&w.0), &test_utils::OnlyReadsKeysInterface {}).unwrap().1;
 		assert!(new_monitor == *monitor);
 		let chain_mon = test_utils::TestChainMonitor::new(Some(&chain_source), &chanmon_cfgs[0].tx_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager);

lightning/src/ln/channelmanager.rs

@@ -380,7 +380,7 @@ pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, M, T, F, L> = ChannelManage
 /// ChannelMonitors passed by reference to read(), those channels will be force-closed based on the
 /// ChannelMonitor state and no funds will be lost (mod on-chain transaction fees).
 ///
-/// Note that the deserializer is only implemented for (Sha256dHash, ChannelManager), which
+/// Note that the deserializer is only implemented for (Option<BlockHash>, ChannelManager), which
 /// tells you the last block hash which was block_connect()ed. You MUST rescan any blocks along
 /// the "reorg path" (ie call block_disconnected() until you get to a common block and then call
 /// block_connected() to step towards your best block) upon deserialization before using the
@@ -3925,7 +3925,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
 /// At a high-level, the process for deserializing a ChannelManager and resuming normal operation
 /// is:
 /// 1) Deserialize all stored ChannelMonitors.
-/// 2) Deserialize the ChannelManager by filling in this struct and calling <(Sha256dHash,
+/// 2) Deserialize the ChannelManager by filling in this struct and calling <(Option<BlockHash>,
 ///    ChannelManager)>::read(reader, args).
 ///    This may result in closing some Channels if the ChannelMonitor is newer than the stored
 ///    ChannelManager state to ensure no loss of funds. Thus, transactions may be broadcasted.
@@ -4006,21 +4006,21 @@ impl<'a, Signer: 'a + Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 // Implement ReadableArgs for an Arc'd ChannelManager to make it a bit easier to work with the
 // SipmleArcChannelManager type:
 impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
-	ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (BlockHash, Arc<ChannelManager<Signer, M, T, K, F, L>>)
+	ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (Option<BlockHash>, Arc<ChannelManager<Signer, M, T, K, F, L>>)
 	where M::Target: chain::Watch<Signer>,
         T::Target: BroadcasterInterface,
         K::Target: KeysInterface<Signer = Signer>,
         F::Target: FeeEstimator,
         L::Target: Logger,
 {
 	fn read<R: ::std::io::Read>(reader: &mut R, args: ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>) -> Result<Self, DecodeError> {
-		let (blockhash, chan_manager) = <(BlockHash, ChannelManager<Signer, M, T, K, F, L>)>::read(reader, args)?;
+		let (blockhash, chan_manager) = <(Option<BlockHash>, ChannelManager<Signer, M, T, K, F, L>)>::read(reader, args)?;
 		Ok((blockhash, Arc::new(chan_manager)))
 	}
 }
 
 impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
-	ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (BlockHash, ChannelManager<Signer, M, T, K, F, L>)
+	ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (Option<BlockHash>, ChannelManager<Signer, M, T, K, F, L>)
 	where M::Target: chain::Watch<Signer>,
         T::Target: BroadcasterInterface,
         K::Target: KeysInterface<Signer = Signer>,
@@ -4172,7 +4172,12 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 		//TODO: Broadcast channel update for closed channels, but only after we've made a
 		//connection or two.
 
-		Ok((last_block_hash.clone(), channel_manager))
+		let last_seen_block_hash = if last_block_hash == Default::default() {
+			None
+		} else {
+			Some(last_block_hash)
+		};
+		Ok((last_seen_block_hash, channel_manager))
 	}
 }
 

lightning/src/ln/functional_test_utils.rs

@@ -172,7 +172,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 				for (_, old_monitor) in old_monitors.iter() {
 					let mut w = test_utils::TestVecWriter(Vec::new());
 					old_monitor.write(&mut w).unwrap();
-					let (_, deserialized_monitor) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
+					let (_, deserialized_monitor) = <(Option<BlockHash>, ChannelMonitor<EnforcingSigner>)>::read(
 						&mut ::std::io::Cursor::new(&w.0), self.keys_manager).unwrap();
 					deserialized_monitors.push(deserialized_monitor);
 				}
@@ -188,7 +188,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 
 				let mut w = test_utils::TestVecWriter(Vec::new());
 				self.node.write(&mut w).unwrap();
-				<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut ::std::io::Cursor::new(w.0), ChannelManagerReadArgs {
+				<(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut ::std::io::Cursor::new(w.0), ChannelManagerReadArgs {
 					default_config: UserConfig::default(),
 					keys_manager: self.keys_manager,
 					fee_estimator: &test_utils::TestFeeEstimator { sat_per_kw: 253 },