Return BestBlock when deserializing chain-synced structs

The deserialization of `ChannelMonitor`, `ChannelManager`, and
`OutputSweeper` is implemented for a `(BlockHash, ...)` pair rather
than on the object itself. This ensures developers are pushed to
think about initial chain sync after deserialization and provides
the latest chain sync state conviniently at deserialization-time.

In the previous commit we started storing additional recent block
hashes in `BestBlock` for use during initial sync to ensure we can
handle reorgs while offline if the chain source loses the
reorged-out blocks. Here, we move the deserialization routines to
be on a `(BestBlock, ...)` pair instead of `(BlockHash, ...)`,
providing access to those recent block hashes at
deserialization-time.

Author: TheBlueMatt

lightning-block-sync/src/init.rs

@@ -40,11 +40,10 @@ where
 /// switching to [`SpvClient`]. For example:
 ///
 /// ```
-/// use bitcoin::hash_types::BlockHash;
 /// use bitcoin::network::Network;
 ///
 /// use lightning::chain;
-/// use lightning::chain::Watch;
+/// use lightning::chain::{BestBlock, Watch};
 /// use lightning::chain::chainmonitor;
 /// use lightning::chain::chainmonitor::ChainMonitor;
 /// use lightning::chain::channelmonitor::ChannelMonitor;
@@ -89,14 +88,14 @@ where
 /// 	logger: &L,
 /// 	persister: &P,
 /// ) {
-/// 	// Read a serialized channel monitor paired with the block hash when it was persisted.
+/// 	// Read a serialized channel monitor paired with the best block when it was persisted.
 /// 	let serialized_monitor = "...";
-/// 	let (monitor_block_hash, mut monitor) = <(BlockHash, ChannelMonitor<SP::EcdsaSigner>)>::read(
+/// 	let (monitor_best_block, mut monitor) = <(BestBlock, ChannelMonitor<SP::EcdsaSigner>)>::read(
 /// 		&mut Cursor::new(&serialized_monitor), (entropy_source, signer_provider)).unwrap();
 ///
-/// 	// Read the channel manager paired with the block hash when it was persisted.
+/// 	// Read the channel manager paired with the best block when it was persisted.
 /// 	let serialized_manager = "...";
-/// 	let (manager_block_hash, mut manager) = {
+/// 	let (manager_best_block, mut manager) = {
 /// 		let read_args = ChannelManagerReadArgs::new(
 /// 			entropy_source,
 /// 			node_signer,
@@ -110,16 +109,16 @@ where
 /// 			config,
 /// 			vec![&mut monitor],
 /// 		);
-/// 		<(BlockHash, ChannelManager<&ChainMonitor<SP::EcdsaSigner, &C, &T, &F, &L, &P, &ES>, &T, &ES, &NS, &SP, &F, &R, &MR, &L>)>::read(
+/// 		<(BestBlock, ChannelManager<&ChainMonitor<SP::EcdsaSigner, &C, &T, &F, &L, &P, &ES>, &T, &ES, &NS, &SP, &F, &R, &MR, &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, &monitor_listener as &dyn chain::Listen),
-/// 		(manager_block_hash, &manager as &dyn chain::Listen),
+/// 		(monitor_best_block.block_hash, &monitor_listener as &dyn chain::Listen),
+/// 		(manager_best_block.block_hash, &manager as &dyn chain::Listen),
 /// 	];
 /// 	let chain_tip = init::synchronize_listeners(
 /// 		block_source, Network::Bitcoin, &mut cache, listeners).await.unwrap();

lightning/src/chain/channelmonitor.rs

@@ -1058,15 +1058,15 @@ impl Readable for IrrevocablyResolvedHTLC {
 /// You MUST ensure that no ChannelMonitors for a given channel anywhere contain out-of-date
 /// information and are actively monitoring the chain.
 ///
-/// Like the [`ChannelManager`], deserialization is implemented for `(BlockHash, ChannelMonitor)`,
+/// Like the [`ChannelManager`], deserialization is implemented for `(BestBlock, ChannelMonitor)`,
 /// providing you with the last block hash which was connected before shutting down. You must begin
 /// syncing the chain from that point, disconnecting and connecting blocks as required to get to
 /// the best chain on startup. Note that all [`ChannelMonitor`]s passed to a [`ChainMonitor`] must
 /// by synced as of the same block, so syncing must happen prior to [`ChainMonitor`]
 /// initialization.
 ///
 /// For those loading potentially-ancient [`ChannelMonitor`]s, deserialization is also implemented
-/// for `Option<(BlockHash, ChannelMonitor)>`. LDK can no longer deserialize a [`ChannelMonitor`]
+/// for `Option<(BestBlock, ChannelMonitor)>`. LDK can no longer deserialize a [`ChannelMonitor`]
 /// that was first created in LDK prior to 0.0.110 and last updated prior to LDK 0.0.119. In such
 /// cases, the `Option<(..)>` deserialization option may return `Ok(None)` rather than failing to
 /// deserialize, allowing you to differentiate between the two cases.
@@ -6295,7 +6295,7 @@ where
 const MAX_ALLOC_SIZE: usize = 64 * 1024;
 
 impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP)>
-	for (BlockHash, ChannelMonitor<SP::EcdsaSigner>)
+	for (BestBlock, ChannelMonitor<SP::EcdsaSigner>)
 {
 	fn read<R: io::Read>(reader: &mut R, args: (&'a ES, &'b SP)) -> Result<Self, DecodeError> {
 		match <Option<Self>>::read(reader, args) {
@@ -6307,7 +6307,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 }
 
 impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP)>
-	for Option<(BlockHash, ChannelMonitor<SP::EcdsaSigner>)>
+	for Option<(BestBlock, ChannelMonitor<SP::EcdsaSigner>)>
 {
 	#[rustfmt::skip]
 	fn read<R: io::Read>(reader: &mut R, args: (&'a ES, &'b SP)) -> Result<Self, DecodeError> {
@@ -6735,14 +6735,14 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 					To continue, run a v0.1 release, send/route a payment over the channel or close it.");
 			}
 		}
-		Ok(Some((best_block.block_hash, monitor)))
+		Ok(Some((best_block, monitor)))
 	}
 }
 
 #[cfg(test)]
 mod tests {
 	use bitcoin::amount::Amount;
-	use bitcoin::hash_types::{BlockHash, Txid};
+	use bitcoin::hash_types::Txid;
 	use bitcoin::hashes::sha256::Hash as Sha256;
 	use bitcoin::hashes::Hash;
 	use bitcoin::hex::FromHex;
@@ -6841,7 +6841,7 @@ mod tests {
 		nodes[1].chain_monitor.chain_monitor.transactions_confirmed(&new_header,
 			&[(0, broadcast_tx)], conf_height);
 
-		let (_, pre_update_monitor) = <(BlockHash, ChannelMonitor<_>)>::read(
+		let (_, pre_update_monitor) = <(BestBlock, ChannelMonitor<_>)>::read(
 						&mut io::Cursor::new(&get_monitor!(nodes[1], channel.2).encode()),
 						(&nodes[1].keys_manager.backing, &nodes[1].keys_manager.backing)).unwrap();
 

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -16,7 +16,7 @@ use crate::chain::chaininterface::LowerBoundedFeeEstimator;
 use crate::chain::chainmonitor::ChainMonitor;
 use crate::chain::channelmonitor::{ChannelMonitor, MonitorEvent, ANTI_REORG_DELAY};
 use crate::chain::transaction::OutPoint;
-use crate::chain::{ChannelMonitorUpdateStatus, Listen, Watch};
+use crate::chain::{BestBlock, ChannelMonitorUpdateStatus, Listen, Watch};
 use crate::events::{ClosureReason, Event, HTLCHandlingFailureType, PaymentPurpose};
 use crate::ln::channel::AnnouncementSigsState;
 use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder};
@@ -89,7 +89,7 @@ fn test_monitor_and_persister_update_fail() {
 	let chain_mon = {
 		let new_monitor = {
 			let monitor = nodes[0].chain_monitor.chain_monitor.get_monitor(chan.2).unwrap();
-			let (_, new_monitor) = <(BlockHash, ChannelMonitor<TestChannelSigner>)>::read(
+			let (_, new_monitor) = <(BestBlock, ChannelMonitor<TestChannelSigner>)>::read(
 				&mut &monitor.encode()[..],
 				(nodes[0].keys_manager, nodes[0].keys_manager),
 			)

lightning/src/ln/channelmanager.rs

@@ -1922,7 +1922,6 @@ impl<
 /// detailed in the [`ChannelManagerReadArgs`] documentation.
 ///
 /// ```
-/// use bitcoin::BlockHash;
 /// use bitcoin::network::Network;
 /// use lightning::chain::BestBlock;
 /// # use lightning::chain::channelmonitor::ChannelMonitor;
@@ -1971,8 +1970,8 @@ impl<
 ///     entropy_source, node_signer, signer_provider, fee_estimator, chain_monitor, tx_broadcaster,
 ///     router, message_router, logger, config, channel_monitors.iter().collect(),
 /// );
-/// let (block_hash, channel_manager) =
-///     <(BlockHash, ChannelManager<_, _, _, _, _, _, _, _, _>)>::read(&mut reader, args)?;
+/// let (best_block, channel_manager) =
+///     <(BestBlock, ChannelManager<_, _, _, _, _, _, _, _, _>)>::read(&mut reader, args)?;
 ///
 /// // Update the ChannelManager and ChannelMonitors with the latest chain data
 /// // ...
@@ -2539,7 +2538,7 @@ impl<
 /// [`read`], those channels will be force-closed based on the `ChannelMonitor` state and no funds
 /// will be lost (modulo on-chain transaction fees).
 ///
-/// Note that the deserializer is only implemented for `(`[`BlockHash`]`, `[`ChannelManager`]`)`, which
+/// Note that the deserializer is only implemented for `(`[`BestBlock`]`, `[`ChannelManager`]`)`, which
 /// tells you the last block hash which was connected. You should get the best block tip before using the manager.
 /// See [`chain::Listen`] and [`chain::Confirm`] for more details.
 ///
@@ -2606,7 +2605,6 @@ impl<
 /// [`peer_disconnected`]: msgs::BaseMessageHandler::peer_disconnected
 /// [`funding_created`]: msgs::FundingCreated
 /// [`funding_transaction_generated`]: Self::funding_transaction_generated
-/// [`BlockHash`]: bitcoin::hash_types::BlockHash
 /// [`update_channel`]: chain::Watch::update_channel
 /// [`ChannelUpdate`]: msgs::ChannelUpdate
 /// [`read`]: ReadableArgs::read
@@ -17617,7 +17615,7 @@ impl<'a, ES: EntropySource, NS: NodeSigner, SP: SignerProvider, L: Logger>
 /// is:
 /// 1) Deserialize all stored [`ChannelMonitor`]s.
 /// 2) Deserialize the [`ChannelManager`] by filling in this struct and calling:
-///    `<(BlockHash, ChannelManager)>::read(reader, args)`
+///    `<(BestBlock, 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.
 /// 3) If you are not fetching full blocks, register all relevant [`ChannelMonitor`] outpoints the
@@ -17807,14 +17805,14 @@ impl<
 		MR: MessageRouter,
 		L: Logger + Clone,
 	> ReadableArgs<ChannelManagerReadArgs<'a, M, T, ES, NS, SP, F, R, MR, L>>
-	for (BlockHash, Arc<ChannelManager<M, T, ES, NS, SP, F, R, MR, L>>)
+	for (BestBlock, Arc<ChannelManager<M, T, ES, NS, SP, F, R, MR, L>>)
 {
 	fn read<Reader: io::Read>(
 		reader: &mut Reader, args: ChannelManagerReadArgs<'a, M, T, ES, NS, SP, F, R, MR, L>,
 	) -> Result<Self, DecodeError> {
-		let (blockhash, chan_manager) =
-			<(BlockHash, ChannelManager<M, T, ES, NS, SP, F, R, MR, L>)>::read(reader, args)?;
-		Ok((blockhash, Arc::new(chan_manager)))
+		let (best_block, chan_manager) =
+			<(BestBlock, ChannelManager<M, T, ES, NS, SP, F, R, MR, L>)>::read(reader, args)?;
+		Ok((best_block, Arc::new(chan_manager)))
 	}
 }
 
@@ -17830,7 +17828,7 @@ impl<
 		MR: MessageRouter,
 		L: Logger + Clone,
 	> ReadableArgs<ChannelManagerReadArgs<'a, M, T, ES, NS, SP, F, R, MR, L>>
-	for (BlockHash, ChannelManager<M, T, ES, NS, SP, F, R, MR, L>)
+	for (BestBlock, ChannelManager<M, T, ES, NS, SP, F, R, MR, L>)
 {
 	fn read<Reader: io::Read>(
 		reader: &mut Reader, args: ChannelManagerReadArgs<'a, M, T, ES, NS, SP, F, R, MR, L>,
@@ -17875,7 +17873,7 @@ impl<
 	pub(super) fn from_channel_manager_data(
 		data: ChannelManagerData<SP>,
 		mut args: ChannelManagerReadArgs<'_, M, T, ES, NS, SP, F, R, MR, L>,
-	) -> Result<(BlockHash, Self), DecodeError> {
+	) -> Result<(BestBlock, Self), DecodeError> {
 		let ChannelManagerData {
 			chain_hash,
 			best_block_height,
@@ -19459,7 +19457,7 @@ impl<
 		//TODO: Broadcast channel update for closed channels, but only after we've made a
 		//connection or two.
 
-		Ok((best_block_hash, channel_manager))
+		Ok((best_block, channel_manager))
 	}
 }
 

lightning/src/ln/functional_test_utils.rs

@@ -849,7 +849,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 					let mon = self.chain_monitor.chain_monitor.get_monitor(channel_id).unwrap();
 					mon.write(&mut w).unwrap();
 					let (_, deserialized_monitor) =
-						<(BlockHash, ChannelMonitor<TestChannelSigner>)>::read(
+						<(BestBlock, ChannelMonitor<TestChannelSigner>)>::read(
 							&mut io::Cursor::new(&w.0),
 							(self.keys_manager, self.keys_manager),
 						)
@@ -878,7 +878,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,
+					BestBlock,
 					ChannelManager<
 						&test_utils::TestChainMonitor,
 						&test_utils::TestBroadcaster,
@@ -1307,7 +1307,7 @@ pub fn _reload_node<'a, 'b, 'c>(
 	let mut monitors_read = Vec::with_capacity(monitors_encoded.len());
 	for encoded in monitors_encoded {
 		let mut monitor_read = &encoded[..];
-		let (_, monitor) = <(BlockHash, ChannelMonitor<TestChannelSigner>)>::read(
+		let (_, monitor) = <(BestBlock, ChannelMonitor<TestChannelSigner>)>::read(
 			&mut monitor_read,
 			(node.keys_manager, node.keys_manager),
 		)
@@ -1322,7 +1322,7 @@ pub fn _reload_node<'a, 'b, 'c>(
 		for monitor in monitors_read.iter() {
 			assert!(channel_monitors.insert(monitor.channel_id(), monitor).is_none());
 		}
-		<(BlockHash, TestChannelManager<'b, 'c>)>::read(
+		<(BestBlock, TestChannelManager<'b, 'c>)>::read(
 			&mut node_read,
 			ChannelManagerReadArgs {
 				config,

lightning/src/ln/functional_tests.rs

@@ -19,6 +19,7 @@ use crate::chain::channelmonitor::{
 	LATENCY_GRACE_PERIOD_BLOCKS,
 };
 use crate::chain::transaction::OutPoint;
+use crate::chain::BestBlock;
 use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
 use crate::events::{
 	ClosureReason, Event, HTLCHandlingFailureType, PathFailure, PaymentFailureReason,
@@ -7382,7 +7383,7 @@ pub fn test_update_err_monitor_lockdown() {
 		let new_monitor = {
 			let monitor = nodes[0].chain_monitor.chain_monitor.get_monitor(chan_1.2).unwrap();
 			let new_monitor =
-				<(BlockHash, channelmonitor::ChannelMonitor<TestChannelSigner>)>::read(
+				<(BestBlock, channelmonitor::ChannelMonitor<TestChannelSigner>)>::read(
 					&mut io::Cursor::new(&monitor.encode()),
 					(nodes[0].keys_manager, nodes[0].keys_manager),
 				)
@@ -7490,7 +7491,7 @@ pub fn test_concurrent_monitor_claim() {
 		let new_monitor = {
 			let monitor = nodes[0].chain_monitor.chain_monitor.get_monitor(chan_1.2).unwrap();
 			let new_monitor =
-				<(BlockHash, channelmonitor::ChannelMonitor<TestChannelSigner>)>::read(
+				<(BestBlock, channelmonitor::ChannelMonitor<TestChannelSigner>)>::read(
 					&mut io::Cursor::new(&monitor.encode()),
 					(nodes[0].keys_manager, nodes[0].keys_manager),
 				)
@@ -7540,7 +7541,7 @@ pub fn test_concurrent_monitor_claim() {
 		let new_monitor = {
 			let monitor = nodes[0].chain_monitor.chain_monitor.get_monitor(chan_1.2).unwrap();
 			let new_monitor =
-				<(BlockHash, channelmonitor::ChannelMonitor<TestChannelSigner>)>::read(
+				<(BestBlock, channelmonitor::ChannelMonitor<TestChannelSigner>)>::read(
 					&mut io::Cursor::new(&monitor.encode()),
 					(nodes[0].keys_manager, nodes[0].keys_manager),
 				)

lightning/src/ln/reload_tests.rs

@@ -11,7 +11,7 @@
 
 //! Functional tests which test for correct behavior across node restarts.
 
-use crate::chain::{ChannelMonitorUpdateStatus, Watch};
+use crate::chain::{BestBlock, ChannelMonitorUpdateStatus, Watch};
 use crate::chain::chaininterface::LowerBoundedFeeEstimator;
 use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateStep};
 use crate::routing::router::{PaymentParameters, RouteParameters};
@@ -30,7 +30,6 @@ use crate::util::ser::{Writeable, ReadableArgs};
 use crate::util::config::{HTLCInterceptionFlags, UserConfig};
 
 use bitcoin::hashes::Hash;
-use bitcoin::hash_types::BlockHash;
 use types::payment::{PaymentHash, PaymentPreimage};
 
 use crate::prelude::*;
@@ -412,22 +411,22 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 	let mut node_0_stale_monitors = Vec::new();
 	for serialized in node_0_stale_monitors_serialized.iter() {
 		let mut read = &serialized[..];
-		let (_, monitor) = <(BlockHash, ChannelMonitor<TestChannelSigner>)>::read(&mut read, (keys_manager, keys_manager)).unwrap();
+		let (_, monitor) = <(BestBlock, ChannelMonitor<TestChannelSigner>)>::read(&mut read, (keys_manager, keys_manager)).unwrap();
 		assert!(read.is_empty());
 		node_0_stale_monitors.push(monitor);
 	}
 
 	let mut node_0_monitors = Vec::new();
 	for serialized in node_0_monitors_serialized.iter() {
 		let mut read = &serialized[..];
-		let (_, monitor) = <(BlockHash, ChannelMonitor<TestChannelSigner>)>::read(&mut read, (keys_manager, keys_manager)).unwrap();
+		let (_, monitor) = <(BestBlock, ChannelMonitor<TestChannelSigner>)>::read(&mut read, (keys_manager, keys_manager)).unwrap();
 		assert!(read.is_empty());
 		node_0_monitors.push(monitor);
 	}
 
 	let mut nodes_0_read = &nodes_0_serialized[..];
 	if let Err(msgs::DecodeError::DangerousValue) =
-		<(BlockHash, ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestMessageRouter, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
+		<(BestBlock, ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestMessageRouter, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
 		config: UserConfig::default(),
 		entropy_source: keys_manager,
 		node_signer: keys_manager,
@@ -445,7 +444,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 
 	let mut nodes_0_read = &nodes_0_serialized[..];
 	let (_, nodes_0_deserialized_tmp) =
-		<(BlockHash, ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestMessageRouter, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
+		<(BestBlock, ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestMessageRouter, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
 		config: UserConfig::default(),
 		entropy_source: keys_manager,
 		node_signer: keys_manager,