From ee995a3a5542dee16b7fbdc5e55c6311b3c78442 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 26 Feb 2021 16:09:28 -0500 Subject: [PATCH] Change ChannelManager deserialization to return an optional blockhash If the ChannelManager never receives any blocks, it'll return a default blockhash on deserialization. It's preferable for this to be an Option instead. --- fuzz/src/chanmon_consistency.rs | 2 +- lightning-block-sync/src/init.rs | 10 ++++++---- lightning/src/ln/channelmanager.rs | 13 +++++++++---- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/functional_tests.rs | 12 ++++++------ 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index d6a106bb7..3f0f2401d 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -355,7 +355,7 @@ pub fn do_test(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, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor) } } } diff --git a/lightning-block-sync/src/init.rs b/lightning-block-sync/src/init.rs index 24080b15a..287f75904 100644 --- a/lightning-block-sync/src/init.rs +++ b/lightning-block-sync/src/init.rs @@ -66,7 +66,7 @@ use lightning::chain; /// /// // 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, @@ -76,17 +76,19 @@ use lightning::chain; /// config, /// vec![&mut monitor], /// ); -/// <(BlockHash, ChannelManager, &T, &K, &F, &L>)>::read( +/// <(Option, ChannelManager, &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![ +/// let mut listeners = vec![ /// (monitor_block_hash, &mut monitor_listener as &mut dyn chain::Listen), -/// (manager_block_hash, &mut manager 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(); /// diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 4b10340f4..2a0a9d760 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4006,7 +4006,7 @@ 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> for (BlockHash, Arc>) + ReadableArgs> for (Option, Arc>) where M::Target: chain::Watch, T::Target: BroadcasterInterface, K::Target: KeysInterface, @@ -4014,13 +4014,13 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> L::Target: Logger, { fn read(reader: &mut R, args: ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>) -> Result { - let (blockhash, chan_manager) = <(BlockHash, ChannelManager)>::read(reader, args)?; + let (blockhash, chan_manager) = <(Option, ChannelManager)>::read(reader, args)?; Ok((blockhash, Arc::new(chan_manager))) } } impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> - ReadableArgs> for (BlockHash, ChannelManager) + ReadableArgs> for (Option, ChannelManager) where M::Target: chain::Watch, T::Target: BroadcasterInterface, K::Target: KeysInterface, @@ -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)) } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index f0937817b..2fc938ab4 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -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)>::read(&mut ::std::io::Cursor::new(w.0), ChannelManagerReadArgs { + <(Option, ChannelManager)>::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 }, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index fdf5d2006..4ca7a87c2 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4332,7 +4332,7 @@ fn test_no_txn_manager_serialize_deserialize() { let (_, nodes_0_deserialized_tmp) = { let mut channel_monitors = HashMap::new(); channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor); - <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + <(Option, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: config, keys_manager, fee_estimator: &fee_estimator, @@ -4441,7 +4441,7 @@ fn test_manager_serialize_deserialize_events() { let (_, nodes_0_deserialized_tmp) = { let mut channel_monitors = HashMap::new(); channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor); - <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + <(Option, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: config, keys_manager, fee_estimator: &fee_estimator, @@ -4532,7 +4532,7 @@ fn test_simple_manager_serialize_deserialize() { let (_, nodes_0_deserialized_tmp) = { let mut channel_monitors = HashMap::new(); channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor); - <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + <(Option, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: UserConfig::default(), keys_manager, fee_estimator: &fee_estimator, @@ -4623,7 +4623,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { let mut nodes_0_read = &nodes_0_serialized[..]; if let Err(msgs::DecodeError::InvalidValue) = - <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + <(Option, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: UserConfig::default(), keys_manager, fee_estimator: &fee_estimator, @@ -4637,7 +4637,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { let mut nodes_0_read = &nodes_0_serialized[..]; let (_, nodes_0_deserialized_tmp) = - <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + <(Option, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: UserConfig::default(), keys_manager, fee_estimator: &fee_estimator, @@ -7498,7 +7498,7 @@ fn test_data_loss_protect() { node_state_0 = { let mut channel_monitors = HashMap::new(); channel_monitors.insert(OutPoint { txid: chan.3.txid(), index: 0 }, &mut chain_monitor); - <(BlockHash, ChannelManager)>::read(&mut ::std::io::Cursor::new(previous_node_state), ChannelManagerReadArgs { + <(Option, ChannelManager)>::read(&mut ::std::io::Cursor::new(previous_node_state), ChannelManagerReadArgs { keys_manager: keys_manager, fee_estimator: &fee_estimator, chain_monitor: &monitor, -- 2.39.5