From 4cd2e4e94b980b1830337dc7aa2ebb6de881ccf8 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 5 Mar 2021 13:28:20 -0800 Subject: [PATCH 1/9] Revert "Merge pull request #819 from TheBlueMatt/2021-03-810-rebased" This reverts commit 793de5fe6944bb6ab414934e53a7ae80bb5a9a31, reversing changes made to 03a518965100b6852f36e4f95ead4c1d93f5c4b0. --- fuzz/src/chanmon_consistency.rs | 6 ++-- fuzz/src/chanmon_deser.rs | 8 ++--- lightning-block-sync/src/init.rs | 17 +++++------ lightning-persister/src/lib.rs | 2 +- lightning/src/chain/channelmonitor.rs | 12 ++------ lightning/src/ln/chanmon_update_fail_tests.rs | 2 +- lightning/src/ln/channelmanager.rs | 17 ++++------- lightning/src/ln/functional_test_utils.rs | 4 +-- lightning/src/ln/functional_tests.rs | 30 +++++++++---------- lightning/src/ln/reorg_tests.rs | 4 +-- lightning/src/util/test_utils.rs | 4 +-- 11 files changed, 45 insertions(+), 61 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index dcf8c31ffb7..d6a106bb7d0 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -126,7 +126,7 @@ impl chain::Watch for TestChainMonitor { hash_map::Entry::Occupied(entry) => entry, hash_map::Entry::Vacant(_) => panic!("Didn't have monitor on update call"), }; - let deserialized_monitor = <(Option, channelmonitor::ChannelMonitor)>:: + let deserialized_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>:: 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(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, <(Option, ChannelMonitor)>::read(&mut Cursor::new(&monitor_ser), &OnlyReadsKeysInterface {}).expect("Failed to read monitor").1); + monitors.insert(outpoint, <(BlockHash, ChannelMonitor)>::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(data: &[u8], out: Out) { channel_monitors: monitor_refs, }; - (<(Option, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor) + (<(BlockHash, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor) } } } diff --git a/fuzz/src/chanmon_deser.rs b/fuzz/src/chanmon_deser.rs index e7dbf3ed8d6..933930cf6d1 100644 --- a/fuzz/src/chanmon_deser.rs +++ b/fuzz/src/chanmon_deser.rs @@ -25,13 +25,11 @@ impl Writer for VecWriter { #[inline] pub fn do_test(data: &[u8], _out: Out) { - if let Ok((Some(latest_block_hash), monitor)) = <(Option, channelmonitor::ChannelMonitor)>::read(&mut Cursor::new(data), &OnlyReadsKeysInterface {}) { + if let Ok((latest_block_hash, monitor)) = <(BlockHash, channelmonitor::ChannelMonitor)>::read(&mut Cursor::new(data), &OnlyReadsKeysInterface {}) { let mut w = VecWriter(Vec::new()); monitor.write(&mut w).unwrap(); - let deserialized_copy = <(Option, channelmonitor::ChannelMonitor)>::read(&mut Cursor::new(&w.0), &OnlyReadsKeysInterface {}).unwrap(); - if let Some(deserialized) = deserialized_copy.0 { - assert!(latest_block_hash == deserialized); - } + let deserialized_copy = <(BlockHash, channelmonitor::ChannelMonitor)>::read(&mut Cursor::new(&w.0), &OnlyReadsKeysInterface {}).unwrap(); + assert!(latest_block_hash == deserialized_copy.0); assert!(monitor == deserialized_copy.1); } } diff --git a/lightning-block-sync/src/init.rs b/lightning-block-sync/src/init.rs index 59c057157ef..83e356919b8 100644 --- a/lightning-block-sync/src/init.rs +++ b/lightning-block-sync/src/init.rs @@ -73,12 +73,12 @@ BlockSourceResult { /// ) { /// // Read a serialized channel monitor paired with the block hash when it was persisted. /// let serialized_monitor = "..."; -/// let (monitor_block_hash_option, mut monitor) = <(Option, ChannelMonitor)>::read( +/// let (monitor_block_hash, mut monitor) = <(BlockHash, ChannelMonitor)>::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_option, mut manager) = { +/// let (manager_block_hash, mut manager) = { /// let read_args = ChannelManagerReadArgs::new( /// keys_manager, /// fee_estimator, @@ -88,20 +88,17 @@ BlockSourceResult { /// config, /// vec![&mut monitor], /// ); -/// <(Option, ChannelManager, &T, &K, &F, &L>)>::read( +/// <(BlockHash, 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 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 listeners = vec![ +/// (monitor_block_hash, &mut monitor_listener as &mut dyn chain::Listen), +/// (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-persister/src/lib.rs b/lightning-persister/src/lib.rs index 0151b1e464e..2226bcba609 100644 --- a/lightning-persister/src/lib.rs +++ b/lightning-persister/src/lib.rs @@ -124,7 +124,7 @@ impl FilesystemPersister { if contents.is_err() { return Err(ChannelMonitorUpdateErr::PermanentFailure); } if let Ok((_, loaded_monitor)) = - <(Option, ChannelMonitor)>::read(&mut Cursor::new(&contents.unwrap()), keys) { + <(BlockHash, ChannelMonitor)>::read(&mut Cursor::new(&contents.unwrap()), keys) { res.insert(OutPoint { txid: txid.unwrap(), index: index.unwrap() }, loaded_monitor); } else { return Err(ChannelMonitorUpdateErr::PermanentFailure); diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 4d2d1b848ae..034854810c6 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/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 (Option, ChannelMonitor), which +/// Note that the deserializer is only implemented for (Sha256dHash, 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> ReadableArgs<&'a K> - for (Option, ChannelMonitor) { + for (BlockHash, ChannelMonitor) { fn read(reader: &mut R, keys_manager: &'a K) -> Result { macro_rules! unwrap_obj { ($key: expr) => { @@ -2735,13 +2735,7 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes()); - let last_seen_block_hash = if last_block_hash == Default::default() { - None - } else { - Some(last_block_hash) - }; - - Ok((last_seen_block_hash, ChannelMonitor { + Ok((last_block_hash.clone(), ChannelMonitor { inner: Mutex::new(ChannelMonitorImpl { latest_update_id, commitment_transaction_number_obscure_factor, diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 3ed84de95ae..d764bc78215 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/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 = <(Option, ChannelMonitor)>::read( + let new_monitor = <(BlockHash, ChannelMonitor)>::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); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 86ccc03bab6..7fceddeb5aa 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -389,7 +389,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 (Option, ChannelManager), which +/// Note that the deserializer is only implemented for (Sha256dHash, 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 @@ -4005,7 +4005,7 @@ impl 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 <(Option, +/// 2) Deserialize the ChannelManager by filling in this struct and calling <(Sha256dHash, /// 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. @@ -4097,7 +4097,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 (Option, Arc>) + ReadableArgs> for (BlockHash, Arc>) where M::Target: chain::Watch, T::Target: BroadcasterInterface, K::Target: KeysInterface, @@ -4105,13 +4105,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) = <(Option, ChannelManager)>::read(reader, args)?; + let (blockhash, chan_manager) = <(BlockHash, 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 (Option, ChannelManager) + ReadableArgs> for (BlockHash, ChannelManager) where M::Target: chain::Watch, T::Target: BroadcasterInterface, K::Target: KeysInterface, @@ -4273,12 +4273,7 @@ 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. - let last_seen_block_hash = if last_block_hash == Default::default() { - None - } else { - Some(last_block_hash) - }; - Ok((last_seen_block_hash, channel_manager)) + Ok((last_block_hash.clone(), channel_manager)) } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index b0ffef2a57f..cac714c4ce2 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -174,7 +174,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) = <(Option, ChannelMonitor)>::read( + let (_, deserialized_monitor) = <(BlockHash, ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0), self.keys_manager).unwrap(); deserialized_monitors.push(deserialized_monitor); } @@ -190,7 +190,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(); - <(Option, ChannelManager)>::read(&mut ::std::io::Cursor::new(w.0), ChannelManagerReadArgs { + <(BlockHash, 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 6fbea2fee98..4604d741cef 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4292,7 +4292,7 @@ fn test_no_txn_manager_serialize_deserialize() { new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster.clone(), &logger, &fee_estimator, &persister, keys_manager); nodes[0].chain_monitor = &new_chain_monitor; let mut chan_0_monitor_read = &chan_0_monitor_serialized.0[..]; - let (_, mut chan_0_monitor) = <(Option, ChannelMonitor)>::read( + let (_, mut chan_0_monitor) = <(BlockHash, ChannelMonitor)>::read( &mut chan_0_monitor_read, keys_manager).unwrap(); assert!(chan_0_monitor_read.is_empty()); @@ -4301,7 +4301,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); - <(Option, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: config, keys_manager, fee_estimator: &fee_estimator, @@ -4401,7 +4401,7 @@ fn test_manager_serialize_deserialize_events() { new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster.clone(), &logger, &fee_estimator, &persister, keys_manager); nodes[0].chain_monitor = &new_chain_monitor; let mut chan_0_monitor_read = &chan_0_monitor_serialized.0[..]; - let (_, mut chan_0_monitor) = <(Option, ChannelMonitor)>::read( + let (_, mut chan_0_monitor) = <(BlockHash, ChannelMonitor)>::read( &mut chan_0_monitor_read, keys_manager).unwrap(); assert!(chan_0_monitor_read.is_empty()); @@ -4410,7 +4410,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); - <(Option, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: config, keys_manager, fee_estimator: &fee_estimator, @@ -4493,7 +4493,7 @@ fn test_simple_manager_serialize_deserialize() { new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster.clone(), &logger, &fee_estimator, &persister, keys_manager); nodes[0].chain_monitor = &new_chain_monitor; let mut chan_0_monitor_read = &chan_0_monitor_serialized.0[..]; - let (_, mut chan_0_monitor) = <(Option, ChannelMonitor)>::read( + let (_, mut chan_0_monitor) = <(BlockHash, ChannelMonitor)>::read( &mut chan_0_monitor_read, keys_manager).unwrap(); assert!(chan_0_monitor_read.is_empty()); @@ -4501,7 +4501,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); - <(Option, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: UserConfig::default(), keys_manager, fee_estimator: &fee_estimator, @@ -4577,7 +4577,7 @@ 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) = <(Option, ChannelMonitor)>::read(&mut read, keys_manager).unwrap(); + let (_, monitor) = <(BlockHash, ChannelMonitor)>::read(&mut read, keys_manager).unwrap(); assert!(read.is_empty()); node_0_stale_monitors.push(monitor); } @@ -4585,14 +4585,14 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { let mut node_0_monitors = Vec::new(); for serialized in node_0_monitors_serialized.iter() { let mut read = &serialized[..]; - let (_, monitor) = <(Option, ChannelMonitor)>::read(&mut read, keys_manager).unwrap(); + let (_, monitor) = <(BlockHash, ChannelMonitor)>::read(&mut read, 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::InvalidValue) = - <(Option, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: UserConfig::default(), keys_manager, fee_estimator: &fee_estimator, @@ -4606,7 +4606,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { let mut nodes_0_read = &nodes_0_serialized[..]; let (_, nodes_0_deserialized_tmp) = - <(Option, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { + <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: UserConfig::default(), keys_manager, fee_estimator: &fee_estimator, @@ -7462,7 +7462,7 @@ fn test_data_loss_protect() { // Restore node A from previous state logger = test_utils::TestLogger::with_id(format!("node {}", 0)); - let mut chain_monitor = <(Option, ChannelMonitor)>::read(&mut ::std::io::Cursor::new(previous_chain_monitor_state.0), keys_manager).unwrap().1; + let mut chain_monitor = <(BlockHash, ChannelMonitor)>::read(&mut ::std::io::Cursor::new(previous_chain_monitor_state.0), keys_manager).unwrap().1; chain_source = test_utils::TestChainSource::new(Network::Testnet); tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())}; fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 }; @@ -7471,7 +7471,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); - <(Option, ChannelManager)>::read(&mut ::std::io::Cursor::new(previous_node_state), ChannelManagerReadArgs { + <(BlockHash, ChannelManager)>::read(&mut ::std::io::Cursor::new(previous_node_state), ChannelManagerReadArgs { keys_manager: keys_manager, fee_estimator: &fee_estimator, chain_monitor: &monitor, @@ -8240,7 +8240,7 @@ fn test_update_err_monitor_lockdown() { let monitor = monitors.get(&outpoint).unwrap(); let mut w = test_utils::TestVecWriter(Vec::new()); monitor.write(&mut w).unwrap(); - let new_monitor = <(Option, channelmonitor::ChannelMonitor)>::read( + let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0), &test_utils::OnlyReadsKeysInterface {}).unwrap().1; assert!(new_monitor == *monitor); let watchtower = test_utils::TestChainMonitor::new(Some(&chain_source), &chanmon_cfgs[0].tx_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager); @@ -8299,7 +8299,7 @@ fn test_concurrent_monitor_claim() { let monitor = monitors.get(&outpoint).unwrap(); let mut w = test_utils::TestVecWriter(Vec::new()); monitor.write(&mut w).unwrap(); - let new_monitor = <(Option, channelmonitor::ChannelMonitor)>::read( + let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0), &test_utils::OnlyReadsKeysInterface {}).unwrap().1; assert!(new_monitor == *monitor); let watchtower = test_utils::TestChainMonitor::new(Some(&chain_source), &chanmon_cfgs[0].tx_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager); @@ -8325,7 +8325,7 @@ fn test_concurrent_monitor_claim() { let monitor = monitors.get(&outpoint).unwrap(); let mut w = test_utils::TestVecWriter(Vec::new()); monitor.write(&mut w).unwrap(); - let new_monitor = <(Option, channelmonitor::ChannelMonitor)>::read( + let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0), &test_utils::OnlyReadsKeysInterface {}).unwrap().1; assert!(new_monitor == *monitor); let watchtower = test_utils::TestChainMonitor::new(Some(&chain_source), &chanmon_cfgs[0].tx_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager); diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index 165f86fc266..b19a355d8f1 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -240,7 +240,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool) { new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster.clone(), nodes[0].logger, node_cfgs[0].fee_estimator, &persister, keys_manager); nodes[0].chain_monitor = &new_chain_monitor; let mut chan_0_monitor_read = &chan_0_monitor_serialized.0[..]; - let (_, mut chan_0_monitor) = <(Option, ChannelMonitor)>::read( + let (_, mut chan_0_monitor) = <(BlockHash, ChannelMonitor)>::read( &mut chan_0_monitor_read, keys_manager).unwrap(); assert!(chan_0_monitor_read.is_empty()); @@ -249,7 +249,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool) { nodes_0_deserialized = { let mut channel_monitors = HashMap::new(); channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor); - <(Option, ChannelManager)>::read( &mut nodes_0_read, ChannelManagerReadArgs { default_config: config, diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index b96169aadb8..ea110023823 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -109,7 +109,7 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { // to a watchtower and disk... let mut w = TestVecWriter(Vec::new()); monitor.write(&mut w).unwrap(); - let new_monitor = <(Option, channelmonitor::ChannelMonitor)>::read( + let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0), self.keys_manager).unwrap().1; assert!(new_monitor == monitor); self.latest_monitor_update_id.lock().unwrap().insert(funding_txo.to_channel_id(), (funding_txo, monitor.get_latest_update_id())); @@ -150,7 +150,7 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { let monitor = monitors.get(&funding_txo).unwrap(); w.0.clear(); monitor.write(&mut w).unwrap(); - let new_monitor = <(Option, channelmonitor::ChannelMonitor)>::read( + let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0), self.keys_manager).unwrap().1; assert!(new_monitor == *monitor); self.added_monitors.lock().unwrap().push((funding_txo, new_monitor)); From af49a60e2d53945b7c6d9d6e425b9b6de6a1843e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 2 Mar 2021 13:07:43 -0500 Subject: [PATCH 2/9] Update docs with correct hash type Co-authored-by: Matt Corallo Co-authored-by: Jeffrey Czyz --- lightning/src/chain/channelmonitor.rs | 2 +- lightning/src/ln/channelmanager.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 034854810c6..0c23a1efe75 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/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 (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 diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7fceddeb5aa..43bee89dc28 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -389,7 +389,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 (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 @@ -4005,8 +4005,8 @@ impl 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, -/// ChannelManager)>::read(reader, args). +/// 2) Deserialize the ChannelManager by filling in this struct and calling: +/// <(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. /// 3) If you are not fetching full blocks, register all relevant ChannelMonitor outpoints the same From d28fa54edbfde53171aa1e61b03502b02dc422d2 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 3 Mar 2021 11:24:55 -0800 Subject: [PATCH 3/9] Parameterize ChannelManager::new with a block hash When ChannelMonitors are persisted, they need to store the most recent block hash seen. However, for newly created channels the default block hash is used. If persisted before a block is connected, the funding output may be missed when syncing after a restart. Instead, initialize ChannelManager with a "birthday" hash so it can be used later when creating channels. --- background-processor/src/lib.rs | 12 ++++++--- fuzz/src/chanmon_consistency.rs | 11 ++++++-- fuzz/src/full_stack.rs | 13 ++++++--- lightning/src/ln/channelmanager.rs | 32 +++++++++++++++++------ lightning/src/ln/functional_test_utils.rs | 11 ++++++-- 5 files changed, 61 insertions(+), 18 deletions(-) diff --git a/background-processor/src/lib.rs b/background-processor/src/lib.rs index 0c7191fbad2..00f984580f2 100644 --- a/background-processor/src/lib.rs +++ b/background-processor/src/lib.rs @@ -106,7 +106,7 @@ mod tests { use lightning::chain::keysinterface::{Sign, InMemorySigner, KeysInterface, KeysManager}; use lightning::chain::transaction::OutPoint; use lightning::get_event_msg; - use lightning::ln::channelmanager::{ChannelManager, SimpleArcChannelManager}; + use lightning::ln::channelmanager::{ChainParameters, ChannelManager, SimpleArcChannelManager}; use lightning::ln::features::InitFeatures; use lightning::ln::msgs::ChannelMessageHandler; use lightning::util::config::UserConfig; @@ -155,10 +155,16 @@ mod tests { let persister = Arc::new(FilesystemPersister::new(format!("{}_persister_{}", persist_dir, i))); let seed = [i as u8; 32]; let network = Network::Testnet; - let now = Duration::from_secs(genesis_block(network).header.time as u64); + let genesis_block = genesis_block(network); + let now = Duration::from_secs(genesis_block.header.time as u64); let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos())); let chain_monitor = Arc::new(chainmonitor::ChainMonitor::new(Some(chain_source.clone()), tx_broadcaster.clone(), logger.clone(), fee_estimator.clone(), persister.clone())); - let manager = Arc::new(ChannelManager::new(Network::Testnet, fee_estimator.clone(), chain_monitor.clone(), tx_broadcaster, logger.clone(), keys_manager.clone(), UserConfig::default(), i)); + let params = ChainParameters { + network, + latest_hash: genesis_block.block_hash(), + latest_height: 0, + }; + let manager = Arc::new(ChannelManager::new(fee_estimator.clone(), chain_monitor.clone(), tx_broadcaster, logger.clone(), keys_manager.clone(), UserConfig::default(), params)); let node = Node { node: manager, persister, logger }; nodes.push(node); } diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index d6a106bb7d0..3e205c07183 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -19,6 +19,7 @@ //! channel being force-closed. use bitcoin::blockdata::block::BlockHeader; +use bitcoin::blockdata::constants::genesis_block; use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::blockdata::script::{Builder, Script}; use bitcoin::blockdata::opcodes; @@ -35,7 +36,7 @@ use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, use lightning::chain::transaction::OutPoint; use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator}; use lightning::chain::keysinterface::{KeysInterface, InMemorySigner}; -use lightning::ln::channelmanager::{ChannelManager, PaymentHash, PaymentPreimage, PaymentSecret, PaymentSendFailure, ChannelManagerReadArgs}; +use lightning::ln::channelmanager::{ChainParameters, ChannelManager, PaymentHash, PaymentPreimage, PaymentSecret, PaymentSendFailure, ChannelManagerReadArgs}; use lightning::ln::features::{ChannelFeatures, InitFeatures, NodeFeatures}; use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, DecodeError, ErrorAction, UpdateAddHTLC, Init}; use lightning::util::enforcing_trait_impls::{EnforcingSigner, INITIAL_REVOKED_COMMITMENT_NUMBER}; @@ -318,7 +319,13 @@ pub fn do_test(data: &[u8], out: Out) { config.channel_options.fee_proportional_millionths = 0; config.channel_options.announced_channel = true; config.peer_channel_config_limits.min_dust_limit_satoshis = 0; - (ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0), + let network = Network::Bitcoin; + let params = ChainParameters { + network, + latest_hash: genesis_block(network).block_hash(), + latest_height: 0, + }; + (ChannelManager::new(fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, params), monitor, keys_manager) } } } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 3774d0dce83..132ff95f6bb 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -31,7 +31,7 @@ use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, use lightning::chain::chainmonitor; use lightning::chain::transaction::OutPoint; use lightning::chain::keysinterface::{InMemorySigner, KeysInterface}; -use lightning::ln::channelmanager::{ChannelManager, PaymentHash, PaymentPreimage, PaymentSecret}; +use lightning::ln::channelmanager::{ChainParameters, ChannelManager, PaymentHash, PaymentPreimage, PaymentSecret}; use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor}; use lightning::ln::msgs::DecodeError; use lightning::routing::router::get_route; @@ -348,9 +348,16 @@ pub fn do_test(data: &[u8], logger: &Arc) { config.channel_options.fee_proportional_millionths = slice_to_be32(get_slice!(4)); config.channel_options.announced_channel = get_slice!(1)[0] != 0; config.peer_channel_config_limits.min_dust_limit_satoshis = 0; - let channelmanager = Arc::new(ChannelManager::new(Network::Bitcoin, fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, 0)); + let network = Network::Bitcoin; + let genesis_hash = genesis_block(network).block_hash(); + let params = ChainParameters { + network, + latest_hash: genesis_hash, + latest_height: 0, + }; + let channelmanager = Arc::new(ChannelManager::new(fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, params)); let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret()); - let net_graph_msg_handler = Arc::new(NetGraphMsgHandler::new(genesis_block(Network::Bitcoin).header.block_hash(), None, Arc::clone(&logger))); + let net_graph_msg_handler = Arc::new(NetGraphMsgHandler::new(genesis_hash, None, Arc::clone(&logger))); let peers = RefCell::new([false; 256]); let mut loss_detector = MoneyLossDetector::new(&peers, channelmanager.clone(), monitor.clone(), PeerManager::new(MessageHandler { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 43bee89dc28..0940cda2e2d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -461,6 +461,24 @@ pub struct ChannelManager ChannelMana /// /// panics if channel_value_satoshis is >= `MAX_FUNDING_SATOSHIS`! /// - /// Users must provide the current blockchain height from which to track onchain channel - /// funding outpoints and send payments with reliable timelocks. - /// /// Users need to notify the new ChannelManager when a new block is connected or - /// disconnected using its `block_connected` and `block_disconnected` methods. - pub fn new(network: Network, fee_est: F, chain_monitor: M, tx_broadcaster: T, logger: L, keys_manager: K, config: UserConfig, current_blockchain_height: usize) -> Self { + /// disconnected using its `block_connected` and `block_disconnected` methods, starting + /// from after `params.latest_hash`. + pub fn new(fee_est: F, chain_monitor: M, tx_broadcaster: T, logger: L, keys_manager: K, config: UserConfig, params: ChainParameters) -> Self { let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes()); ChannelManager { default_configuration: config.clone(), - genesis_hash: genesis_block(network).header.block_hash(), + genesis_hash: genesis_block(params.network).header.block_hash(), fee_estimator: fee_est, chain_monitor, tx_broadcaster, - latest_block_height: AtomicUsize::new(current_blockchain_height), - last_block_hash: Mutex::new(Default::default()), + latest_block_height: AtomicUsize::new(params.latest_height), + last_block_hash: Mutex::new(params.latest_hash), secp_ctx, channel_state: Mutex::new(ChannelHolder{ diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index cac714c4ce2..e7204a045ab 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -13,7 +13,7 @@ use chain::Watch; use chain::channelmonitor::ChannelMonitor; use chain::transaction::OutPoint; -use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSecret, PaymentSendFailure}; +use ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSecret, PaymentSendFailure}; use routing::router::{Route, get_route}; use routing::network_graph::{NetGraphMsgHandler, NetworkGraph}; use ln::features::InitFeatures; @@ -28,6 +28,7 @@ use util::config::UserConfig; use util::ser::{ReadableArgs, Writeable, Readable}; use bitcoin::blockdata::block::{Block, BlockHeader}; +use bitcoin::blockdata::constants::genesis_block; use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::network::constants::Network; @@ -1163,7 +1164,13 @@ pub fn create_node_chanmgrs<'a, 'b>(node_count: usize, cfgs: &'a Vec default_config.channel_options.announced_channel = true; default_config.peer_channel_config_limits.force_announced_channel_preference = false; default_config.own_channel_config.our_htlc_minimum_msat = 1000; // sanitization being done by the sender, to exerce receiver logic we need to lift of limit - let node = ChannelManager::new(Network::Testnet, cfgs[i].fee_estimator, &cfgs[i].chain_monitor, cfgs[i].tx_broadcaster, cfgs[i].logger, cfgs[i].keys_manager, if node_config[i].is_some() { node_config[i].clone().unwrap() } else { default_config }, 0); + let network = Network::Testnet; + let params = ChainParameters { + network, + latest_hash: genesis_block(network).header.block_hash(), + latest_height: 0, + }; + let node = ChannelManager::new(cfgs[i].fee_estimator, &cfgs[i].chain_monitor, cfgs[i].tx_broadcaster, cfgs[i].logger, cfgs[i].keys_manager, if node_config[i].is_some() { node_config[i].clone().unwrap() } else { default_config }, params); chanmgrs.push(node); } From ff4b4c4d36e2403fe1682a352f54db4ab57f3987 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 4 Mar 2021 16:58:26 -0800 Subject: [PATCH 4/9] Remove block_connected de-duplication logic Calling block_connected for the same block was necessary when clients were expected to re-fetch filtered blocks with relevant transactions. At the time, all listeners were notified of the connected block again for re-scanning. Thus, de-duplication logic was put in place. However, this requirement is no longer applicable for ChannelManager as of commit bd39b20f642e042981e4fdd5f3600a357be51931, so remove this logic. --- lightning/src/ln/channel.rs | 90 ++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a5f4b16e6a1..73f79f3882f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -368,8 +368,7 @@ pub(super) struct Channel { /// could miss the funding_tx_confirmed_in block as well, but it serves as a useful fallback. funding_tx_confirmed_in: Option, short_channel_id: Option, - /// Used to deduplicate block_connected callbacks, also used to verify consistency during - /// ChannelManager deserialization (hence pub(super)) + /// Used to verify consistency during ChannelManager deserialization (hence pub(super)). pub(super) last_block_connected: BlockHash, funding_tx_confirmations: u64, @@ -3517,12 +3516,12 @@ impl Channel { _ => true } }); - let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS); - if header.block_hash() != self.last_block_connected { - if self.funding_tx_confirmations > 0 { - self.funding_tx_confirmations += 1; - } + + if self.funding_tx_confirmations > 0 { + self.funding_tx_confirmations += 1; } + + let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS); if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 { for &(index_in_block, tx) in txdata.iter() { let funding_txo = self.get_funding_txo().unwrap(); @@ -3568,46 +3567,45 @@ impl Channel { } } } - if header.block_hash() != self.last_block_connected { - self.last_block_connected = header.block_hash(); - self.update_time_counter = cmp::max(self.update_time_counter, header.time); - if self.funding_tx_confirmations > 0 { - if self.funding_tx_confirmations == self.minimum_depth as u64 { - let need_commitment_update = if non_shutdown_state == ChannelState::FundingSent as u32 { - self.channel_state |= ChannelState::OurFundingLocked as u32; - true - } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) { - self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS); - self.update_time_counter += 1; - true - } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) { - // We got a reorg but not enough to trigger a force close, just update - // funding_tx_confirmed_in and return. - false - } else if self.channel_state < ChannelState::ChannelFunded as u32 { - panic!("Started confirming a channel in a state pre-FundingSent?: {}", self.channel_state); + + self.last_block_connected = header.block_hash(); + self.update_time_counter = cmp::max(self.update_time_counter, header.time); + if self.funding_tx_confirmations > 0 { + if self.funding_tx_confirmations == self.minimum_depth as u64 { + let need_commitment_update = if non_shutdown_state == ChannelState::FundingSent as u32 { + self.channel_state |= ChannelState::OurFundingLocked as u32; + true + } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) { + self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS); + self.update_time_counter += 1; + true + } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) { + // We got a reorg but not enough to trigger a force close, just update + // funding_tx_confirmed_in and return. + false + } else if self.channel_state < ChannelState::ChannelFunded as u32 { + panic!("Started confirming a channel in a state pre-FundingSent?: {}", self.channel_state); + } else { + // We got a reorg but not enough to trigger a force close, just update + // funding_tx_confirmed_in and return. + false + }; + self.funding_tx_confirmed_in = Some(self.last_block_connected); + + //TODO: Note that this must be a duplicate of the previous commitment point they sent us, + //as otherwise we will have a commitment transaction that they can't revoke (well, kinda, + //they can by sending two revoke_and_acks back-to-back, but not really). This appears to be + //a protocol oversight, but I assume I'm just missing something. + if need_commitment_update { + if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 { + let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx); + return Ok((Some(msgs::FundingLocked { + channel_id: self.channel_id, + next_per_commitment_point, + }), timed_out_htlcs)); } else { - // We got a reorg but not enough to trigger a force close, just update - // funding_tx_confirmed_in and return. - false - }; - self.funding_tx_confirmed_in = Some(self.last_block_connected); - - //TODO: Note that this must be a duplicate of the previous commitment point they sent us, - //as otherwise we will have a commitment transaction that they can't revoke (well, kinda, - //they can by sending two revoke_and_acks back-to-back, but not really). This appears to be - //a protocol oversight, but I assume I'm just missing something. - if need_commitment_update { - if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 { - let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx); - return Ok((Some(msgs::FundingLocked { - channel_id: self.channel_id, - next_per_commitment_point, - }), timed_out_htlcs)); - } else { - self.monitor_pending_funding_locked = true; - return Ok((None, timed_out_htlcs)); - } + self.monitor_pending_funding_locked = true; + return Ok((None, timed_out_htlcs)); } } } From caabc4ef39233a78a9b88787719c653f76c31d1d Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 4 Mar 2021 17:39:21 -0800 Subject: [PATCH 5/9] Remove last_block_connected from Channel Tracking the last block was only used to de-duplicate block_connected calls, but this is no longer required as of the previous commit. Further, the ChannelManager can pass the latest block hash when needing to create a ChannelMonitor rather than have each Channel maintain an up-to-date copy. This is implemented in the next commit. --- lightning/src/ln/channel.rs | 16 ++-------------- lightning/src/ln/channelmanager.rs | 4 ---- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 73f79f3882f..0158448dd6d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -39,7 +39,6 @@ use util::errors::APIError; use util::config::{UserConfig,ChannelConfig}; use std; -use std::default::Default; use std::{cmp,mem,fmt}; use std::ops::Deref; #[cfg(any(test, feature = "fuzztarget"))] @@ -368,8 +367,6 @@ pub(super) struct Channel { /// could miss the funding_tx_confirmed_in block as well, but it serves as a useful fallback. funding_tx_confirmed_in: Option, short_channel_id: Option, - /// Used to verify consistency during ChannelManager deserialization (hence pub(super)). - pub(super) last_block_connected: BlockHash, funding_tx_confirmations: u64, counterparty_dust_limit_satoshis: u64, @@ -568,7 +565,6 @@ impl Channel { funding_tx_confirmed_in: None, short_channel_id: None, - last_block_connected: Default::default(), funding_tx_confirmations: 0, feerate_per_kw: feerate, @@ -804,7 +800,6 @@ impl Channel { funding_tx_confirmed_in: None, short_channel_id: None, - last_block_connected: Default::default(), funding_tx_confirmations: 0, feerate_per_kw: msg.feerate_per_kw, @@ -3568,7 +3563,6 @@ impl Channel { } } - self.last_block_connected = header.block_hash(); self.update_time_counter = cmp::max(self.update_time_counter, header.time); if self.funding_tx_confirmations > 0 { if self.funding_tx_confirmations == self.minimum_depth as u64 { @@ -3590,7 +3584,7 @@ impl Channel { // funding_tx_confirmed_in and return. false }; - self.funding_tx_confirmed_in = Some(self.last_block_connected); + self.funding_tx_confirmed_in = Some(header.block_hash()); //TODO: Note that this must be a duplicate of the previous commitment point they sent us, //as otherwise we will have a commitment transaction that they can't revoke (well, kinda, @@ -3623,8 +3617,7 @@ impl Channel { return true; } } - self.last_block_connected = header.block_hash(); - if Some(self.last_block_connected) == self.funding_tx_confirmed_in { + if Some(header.block_hash()) == self.funding_tx_confirmed_in { self.funding_tx_confirmations = self.minimum_depth as u64 - 1; } false @@ -4433,8 +4426,6 @@ impl Writeable for Channel { self.funding_tx_confirmed_in.write(writer)?; self.short_channel_id.write(writer)?; - - self.last_block_connected.write(writer)?; self.funding_tx_confirmations.write(writer)?; self.counterparty_dust_limit_satoshis.write(writer)?; @@ -4595,8 +4586,6 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel let funding_tx_confirmed_in = Readable::read(reader)?; let short_channel_id = Readable::read(reader)?; - - let last_block_connected = Readable::read(reader)?; let funding_tx_confirmations = Readable::read(reader)?; let counterparty_dust_limit_satoshis = Readable::read(reader)?; @@ -4667,7 +4656,6 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel funding_tx_confirmed_in, short_channel_id, - last_block_connected, funding_tx_confirmations, counterparty_dust_limit_satoshis, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0940cda2e2d..24d7c453784 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4153,10 +4153,6 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> let mut short_to_id = HashMap::with_capacity(cmp::min(channel_count as usize, 128)); for _ in 0..channel_count { let mut channel: Channel = Channel::read(reader, &args.keys_manager)?; - if channel.last_block_connected != Default::default() && channel.last_block_connected != last_block_hash { - return Err(DecodeError::InvalidValue); - } - let funding_txo = channel.get_funding_txo().ok_or(DecodeError::InvalidValue)?; funding_txo_set.insert(funding_txo.clone()); if let Some(ref mut monitor) = args.channel_monitors.get_mut(&funding_txo) { From 31093adef83223494f4fc93c26d91d4bc365bdba Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 4 Mar 2021 18:22:37 -0800 Subject: [PATCH 6/9] Pass along ChannelManager's last_block_hash ChannelMonitor keeps track of the last block connected. However, it is initialized with the default block hash, which is a problem if the ChannelMonitor is serialized before a block is connected. Instead, pass ChannelManager's last_block_hash, which is initialized with a "birthday" hash, when creating a new ChannelMonitor. --- lightning/src/chain/channelmonitor.rs | 10 +++++++--- lightning/src/ln/channel.rs | 16 +++++++++------- lightning/src/ln/channelmanager.rs | 6 ++++-- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 0c23a1efe75..8a8a5b96d66 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -980,7 +980,8 @@ impl ChannelMonitor { channel_parameters: &ChannelTransactionParameters, funding_redeemscript: Script, channel_value_satoshis: u64, commitment_transaction_number_obscure_factor: u64, - initial_holder_commitment_tx: HolderCommitmentTransaction) -> ChannelMonitor { + initial_holder_commitment_tx: HolderCommitmentTransaction, + last_block_hash: BlockHash) -> ChannelMonitor { assert!(commitment_transaction_number_obscure_factor <= (1 << 48)); let our_channel_close_key_hash = WPubkeyHash::hash(&shutdown_pubkey.serialize()); @@ -1067,7 +1068,7 @@ impl ChannelMonitor { lockdown_from_offchain: false, holder_tx_signed: false, - last_block_hash: Default::default(), + last_block_hash, secp_ctx, }), } @@ -2789,6 +2790,7 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> #[cfg(test)] mod tests { + use bitcoin::blockdata::constants::genesis_block; use bitcoin::blockdata::script::{Script, Builder}; use bitcoin::blockdata::opcodes; use bitcoin::blockdata::transaction::{Transaction, TxIn, TxOut, SigHashType}; @@ -2798,6 +2800,7 @@ mod tests { use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::hex::FromHex; use bitcoin::hash_types::Txid; + use bitcoin::network::constants::Network; use hex; use chain::channelmonitor::ChannelMonitor; use chain::transaction::OutPoint; @@ -2897,12 +2900,13 @@ mod tests { }; // Prune with one old state and a holder commitment tx holding a few overlaps with the // old state. + let last_block_hash = genesis_block(Network::Testnet).block_hash(); let monitor = ChannelMonitor::new(Secp256k1::new(), keys, &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(), (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()), &channel_parameters, Script::new(), 46, 0, - HolderCommitmentTransaction::dummy()); + HolderCommitmentTransaction::dummy(), last_block_hash); monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..10])).unwrap(); let dummy_txid = dummy_tx.txid(); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 0158448dd6d..9e4bb081c1a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1516,7 +1516,7 @@ impl Channel { &self.get_counterparty_pubkeys().funding_pubkey } - pub fn funding_created(&mut self, msg: &msgs::FundingCreated, logger: &L) -> Result<(msgs::FundingSigned, ChannelMonitor), ChannelError> where L::Target: Logger { + pub fn funding_created(&mut self, msg: &msgs::FundingCreated, last_block_hash: BlockHash, logger: &L) -> Result<(msgs::FundingSigned, ChannelMonitor), ChannelError> where L::Target: Logger { if self.is_outbound() { return Err(ChannelError::Close("Received funding_created for an outbound channel?".to_owned())); } @@ -1570,7 +1570,7 @@ impl Channel { &self.channel_transaction_parameters, funding_redeemscript.clone(), self.channel_value_satoshis, obscure_factor, - holder_commitment_tx); + holder_commitment_tx, last_block_hash); channel_monitor.provide_latest_counterparty_commitment_tx(counterparty_initial_commitment_txid, Vec::new(), self.cur_counterparty_commitment_transaction_number, self.counterparty_cur_commitment_point.unwrap(), logger); @@ -1587,7 +1587,7 @@ impl Channel { /// Handles a funding_signed message from the remote end. /// If this call is successful, broadcast the funding transaction (and not before!) - pub fn funding_signed(&mut self, msg: &msgs::FundingSigned, logger: &L) -> Result, ChannelError> where L::Target: Logger { + pub fn funding_signed(&mut self, msg: &msgs::FundingSigned, last_block_hash: BlockHash, logger: &L) -> Result, ChannelError> where L::Target: Logger { if !self.is_outbound() { return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned())); } @@ -1640,7 +1640,7 @@ impl Channel { &self.channel_transaction_parameters, funding_redeemscript.clone(), self.channel_value_satoshis, obscure_factor, - holder_commitment_tx); + holder_commitment_tx, last_block_hash); channel_monitor.provide_latest_counterparty_commitment_tx(counterparty_initial_bitcoin_tx.txid, Vec::new(), self.cur_counterparty_commitment_transaction_number, self.counterparty_cur_commitment_point.unwrap(), logger); @@ -4910,6 +4910,8 @@ mod tests { let secp_ctx = Secp256k1::new(); let seed = [42; 32]; let network = Network::Testnet; + let chain_hash = genesis_block(network).header.block_hash(); + let last_block_hash = chain_hash; let keys_provider = test_utils::TestKeysInterface::new(&seed, network); // Go through the flow of opening a channel between two nodes. @@ -4920,7 +4922,7 @@ mod tests { let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); // Create Node B's channel by receiving Node A's open_channel message - let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash()); + let open_channel_msg = node_a_chan.get_open_channel(chain_hash); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); let mut node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), &open_channel_msg, 7, &config).unwrap(); @@ -4935,10 +4937,10 @@ mod tests { }]}; let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 }; let funding_created_msg = node_a_chan.get_outbound_funding_created(funding_outpoint, &&logger).unwrap(); - let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, &&logger).unwrap(); + let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, last_block_hash, &&logger).unwrap(); // Node B --> Node A: funding signed - let _ = node_a_chan.funding_signed(&funding_signed_msg, &&logger); + let _ = node_a_chan.funding_signed(&funding_signed_msg, last_block_hash, &&logger); // Now disconnect the two nodes and check that the commitment point in // Node B's channel_reestablish message is sane. diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 24d7c453784..b356641a0f2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2461,7 +2461,8 @@ impl ChannelMana if chan.get().get_counterparty_node_id() != *counterparty_node_id { return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.temporary_channel_id)); } - (try_chan_entry!(self, chan.get_mut().funding_created(msg, &self.logger), channel_state, chan), chan.remove()) + let last_block_hash = *self.last_block_hash.lock().unwrap(); + (try_chan_entry!(self, chan.get_mut().funding_created(msg, last_block_hash, &self.logger), channel_state, chan), chan.remove()) }, hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.temporary_channel_id)) } @@ -2517,7 +2518,8 @@ impl ChannelMana if chan.get().get_counterparty_node_id() != *counterparty_node_id { return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id)); } - let monitor = match chan.get_mut().funding_signed(&msg, &self.logger) { + let last_block_hash = *self.last_block_hash.lock().unwrap(); + let monitor = match chan.get_mut().funding_signed(&msg, last_block_hash, &self.logger) { Ok(update) => update, Err(e) => try_chan_entry!(self, Err(e), channel_state, chan), }; From d21d8b3463505f559d6b7cd38159ec9ef8f59ae5 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 4 Mar 2021 11:47:09 -0800 Subject: [PATCH 7/9] Rename header_hash to block_hash --- lightning/src/ln/channelmanager.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b356641a0f2..1c8390faa46 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3255,8 +3255,8 @@ impl ChannelMana // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called // during initialization prior to the chain_monitor being fully configured in some cases. // See the docs for `ChannelManagerReadArgs` for more. - let header_hash = header.block_hash(); - log_trace!(self.logger, "Block {} at height {} connected", header_hash, height); + let block_hash = header.block_hash(); + log_trace!(self.logger, "Block {} at height {} connected", block_hash, height); let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier); let mut failed_channels = Vec::new(); let mut timed_out_htlcs = Vec::new(); @@ -3347,7 +3347,7 @@ impl ChannelMana self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), source, &payment_hash, reason); } self.latest_block_height.store(height as usize, Ordering::Release); - *self.last_block_hash.try_lock().expect("block_(dis)connected must not be called in parallel") = header_hash; + *self.last_block_hash.try_lock().expect("block_(dis)connected must not be called in parallel") = block_hash; loop { // Update last_node_announcement_serial to be the max of its current value and the // block timestamp. This should keep us close to the current time without relying on From 035dda67080431f460c18bd1087e511c0ab778ec Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 5 Mar 2021 12:37:50 -0800 Subject: [PATCH 8/9] Hold ChannelManager locks independently ChannelManager reads channel_state and last_block_hash while processing funding_created and funding_signed messages. It writes these while processing block_connected and block_disconnected events. To avoid any potential deadlocks, have each site hold these locks independent of one another and in a consistent order. Additionally, use a RwLock instead of Mutex for last_block_hash since exclusive access is not needed in funding_created / funding_signed and cannot be guaranteed in block_connected / block_disconnected because of the reads in the former. --- lightning/src/ln/channelmanager.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 1c8390faa46..450d014033a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -423,7 +423,7 @@ pub struct ChannelManager, + last_block_hash: RwLock, secp_ctx: Secp256k1, #[cfg(any(test, feature = "_test_utils"))] @@ -803,7 +803,7 @@ impl ChannelMana tx_broadcaster, latest_block_height: AtomicUsize::new(params.latest_height), - last_block_hash: Mutex::new(params.latest_hash), + last_block_hash: RwLock::new(params.latest_hash), secp_ctx, channel_state: Mutex::new(ChannelHolder{ @@ -2454,6 +2454,7 @@ impl ChannelMana fn internal_funding_created(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), MsgHandleErrInternal> { let ((funding_msg, monitor), mut chan) = { + let last_block_hash = *self.last_block_hash.read().unwrap(); let mut channel_lock = self.channel_state.lock().unwrap(); let channel_state = &mut *channel_lock; match channel_state.by_id.entry(msg.temporary_channel_id.clone()) { @@ -2461,7 +2462,6 @@ impl ChannelMana if chan.get().get_counterparty_node_id() != *counterparty_node_id { return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.temporary_channel_id)); } - let last_block_hash = *self.last_block_hash.lock().unwrap(); (try_chan_entry!(self, chan.get_mut().funding_created(msg, last_block_hash, &self.logger), channel_state, chan), chan.remove()) }, hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.temporary_channel_id)) @@ -2511,6 +2511,7 @@ impl ChannelMana fn internal_funding_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingSigned) -> Result<(), MsgHandleErrInternal> { let (funding_txo, user_id) = { + let last_block_hash = *self.last_block_hash.read().unwrap(); let mut channel_lock = self.channel_state.lock().unwrap(); let channel_state = &mut *channel_lock; match channel_state.by_id.entry(msg.channel_id) { @@ -2518,7 +2519,6 @@ impl ChannelMana if chan.get().get_counterparty_node_id() != *counterparty_node_id { return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id)); } - let last_block_hash = *self.last_block_hash.lock().unwrap(); let monitor = match chan.get_mut().funding_signed(&msg, last_block_hash, &self.logger) { Ok(update) => update, Err(e) => try_chan_entry!(self, Err(e), channel_state, chan), @@ -3257,7 +3257,12 @@ impl ChannelMana // See the docs for `ChannelManagerReadArgs` for more. let block_hash = header.block_hash(); log_trace!(self.logger, "Block {} at height {} connected", block_hash, height); + let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier); + + self.latest_block_height.store(height as usize, Ordering::Release); + *self.last_block_hash.write().unwrap() = block_hash; + let mut failed_channels = Vec::new(); let mut timed_out_htlcs = Vec::new(); { @@ -3346,8 +3351,7 @@ impl ChannelMana for (source, payment_hash, reason) in timed_out_htlcs.drain(..) { self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), source, &payment_hash, reason); } - self.latest_block_height.store(height as usize, Ordering::Release); - *self.last_block_hash.try_lock().expect("block_(dis)connected must not be called in parallel") = block_hash; + loop { // Update last_node_announcement_serial to be the max of its current value and the // block timestamp. This should keep us close to the current time without relying on @@ -3371,6 +3375,10 @@ impl ChannelMana // during initialization prior to the chain_monitor being fully configured in some cases. // See the docs for `ChannelManagerReadArgs` for more. let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier); + + self.latest_block_height.fetch_sub(1, Ordering::AcqRel); + *self.last_block_hash.write().unwrap() = header.block_hash(); + let mut failed_channels = Vec::new(); { let mut channel_lock = self.channel_state.lock().unwrap(); @@ -3394,9 +3402,8 @@ impl ChannelMana } }); } + self.handle_init_event_channel_failures(failed_channels); - self.latest_block_height.fetch_sub(1, Ordering::AcqRel); - *self.last_block_hash.try_lock().expect("block_(dis)connected must not be called in parallel") = header.block_hash(); } /// Blocks until ChannelManager needs to be persisted or a timeout is reached. It returns a bool @@ -3952,7 +3959,7 @@ impl Writeable f self.genesis_hash.write(writer)?; (self.latest_block_height.load(Ordering::Acquire) as u32).write(writer)?; - self.last_block_hash.lock().unwrap().write(writer)?; + self.last_block_hash.read().unwrap().write(writer)?; let channel_state = self.channel_state.lock().unwrap(); let mut unfunded_channels = 0; @@ -4254,7 +4261,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> tx_broadcaster: args.tx_broadcaster, latest_block_height: AtomicUsize::new(latest_block_height as usize), - last_block_hash: Mutex::new(last_block_hash), + last_block_hash: RwLock::new(last_block_hash), secp_ctx, channel_state: Mutex::new(ChannelHolder { From 873014875c9a9797bd1e062e6b1cfacd9f958b97 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 3 Mar 2021 18:33:54 -0800 Subject: [PATCH 9/9] Correctly update the last block hash on disconnect When a block is disconnected, the hash of the disconnected block was used to update the last connected block. However, this amounts to a no-op because these hashes should be equal. Successive disconnections would update the hash but leave it one block off. Normally, this not a problem because the last block_disconnected should be followed by block_connected since the former is triggered by a chain re-org. However, this assumes the user calls the API correctly and that no failure occurs that would prevent block_connected from being called (e.g., if fetching the connected block fails). Instead, update the last block hash with the disconnected block's previous block hash. --- lightning/src/chain/channelmonitor.rs | 5 ++--- lightning/src/ln/channelmanager.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 8a8a5b96d66..2352661271f 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2090,8 +2090,7 @@ impl ChannelMonitorImpl { F::Target: FeeEstimator, L::Target: Logger, { - let block_hash = header.block_hash(); - log_trace!(logger, "Block {} at height {} disconnected", block_hash, height); + log_trace!(logger, "Block {} at height {} disconnected", header.block_hash(), height); if let Some(_) = self.onchain_events_waiting_threshold_conf.remove(&(height + ANTI_REORG_DELAY - 1)) { //We may discard: @@ -2101,7 +2100,7 @@ impl ChannelMonitorImpl { self.onchain_tx_handler.block_disconnected(height, broadcaster, fee_estimator, logger); - self.last_block_hash = block_hash; + self.last_block_hash = header.prev_blockhash; } /// Filters a block's `txdata` for transactions spending watched outputs or for any child diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 450d014033a..6e46d79fb08 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3377,7 +3377,7 @@ impl ChannelMana let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier); self.latest_block_height.fetch_sub(1, Ordering::AcqRel); - *self.last_block_hash.write().unwrap() = header.block_hash(); + *self.last_block_hash.write().unwrap() = header.prev_blockhash; let mut failed_channels = Vec::new(); {