Change ChannelManager deserialization to return an optional blockhash
authorValentine Wallace <vwallace@protonmail.com>
Fri, 26 Feb 2021 21:09:28 +0000 (16:09 -0500)
committerMatt Corallo <git@bluematt.me>
Tue, 2 Mar 2021 19:30:56 +0000 (14:30 -0500)
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
lightning-block-sync/src/init.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs

index d6a106bb7d01a80d632193b32c74b89bc8d21a04..3f0f2401d755931030e331e4f82c3158839029d1 100644 (file)
@@ -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)
                } }
        }
 
index 24080b15acb1aa208eca0982e83f01a60117ab29..287f75904f667fc492734c6a8ff5e429b0585536 100644 (file)
@@ -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<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![
+///    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();
 ///
index 4b10340f4c37ba05683679169e9d674262485774..2a0a9d7602476716b255d1909c1c53a4b5b7b119 100644 (file)
@@ -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<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>,
@@ -4014,13 +4014,13 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
         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))
        }
 }
 
index f0937817bdccd1fba55753efd95202a89ed67ec1..2fc938ab4f13800ae3563713487a9b3720c928dd 100644 (file)
@@ -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 },
index fdf5d2006754930276c70ad0c802df4a8c6a6348..4ca7a87c2ad22513e61853a208a603a46f61e6b8 100644 (file)
@@ -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<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
+               <(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::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<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
+               <(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::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<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
+               <(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::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<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
+               <(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::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<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
+               <(Option<BlockHash>, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::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<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut ::std::io::Cursor::new(previous_node_state), 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(previous_node_state), ChannelManagerReadArgs {
                        keys_manager: keys_manager,
                        fee_estimator: &fee_estimator,
                        chain_monitor: &monitor,