From c88b707ac2f70616de109a2fc20c8f3d8e2341e9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 20 Mar 2021 00:22:09 -0400 Subject: [PATCH] Drop ChannelManager::block_disconnected() entirely It is now entirely redundant with ChannelManager::update_best_block and is still accessible via `Listen::block_disconnected`. --- fuzz/src/full_stack.rs | 3 +- lightning/src/ln/channelmanager.rs | 59 +++++------------------ lightning/src/ln/functional_test_utils.rs | 2 +- 3 files changed, 14 insertions(+), 50 deletions(-) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index aae5484d7..132d4f8eb 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -27,6 +27,7 @@ use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash}; use lightning::chain; +use lightning::chain::Listen; use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator}; use lightning::chain::chainmonitor; use lightning::chain::transaction::OutPoint; @@ -217,7 +218,7 @@ impl<'a> MoneyLossDetector<'a> { fn disconnect_block(&mut self) { if self.height > 0 && (self.max_height < 6 || self.height >= self.max_height - 6) { let header = BlockHeader { version: 0x20000000, prev_blockhash: self.header_hashes[self.height - 1].0, merkle_root: Default::default(), time: self.header_hashes[self.height].1, bits: 42, nonce: 42 }; - self.manager.block_disconnected(&header); + self.manager.block_disconnected(&header, self.height as u32); self.monitor.block_disconnected(&header, self.height as u32); self.height -= 1; let removal_height = self.height; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 14f78de4d..9f943979d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3302,8 +3302,17 @@ where self.update_best_block(&block.header, height); } - fn block_disconnected(&self, header: &BlockHeader, _height: u32) { - ChannelManager::block_disconnected(self, header); + fn block_disconnected(&self, header: &BlockHeader, height: u32) { + assert_eq!(*self.last_block_hash.read().unwrap(), header.block_hash(), + "Blocks must be disconnected in chain-order - the disconnected header must be the last connected header"); + + let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier); + let new_height = self.latest_block_height.fetch_sub(1, Ordering::AcqRel) as u32 - 1; + assert_eq!(new_height, height - 1, + "Blocks must be disconnected in chain-order - the disconnected block must have the correct height"); + *self.last_block_hash.write().unwrap() = header.prev_blockhash; + + self.do_chain_event(new_height, |channel| channel.update_best_block(new_height, header.time)); } } @@ -3472,52 +3481,6 @@ impl ChannelMana } } - /// Updates channel state based on a disconnected block. - /// - /// If necessary, the channel may be force-closed without letting the counterparty participate - /// in the shutdown. - pub fn block_disconnected(&self, header: &BlockHeader) { - // 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 _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier); - - assert_eq!(*self.last_block_hash.read().unwrap(), header.block_hash(), - "Blocks must be disconnected in chain-order - the disconnected header must be the last connected header"); - let new_height = self.latest_block_height.fetch_sub(1, Ordering::AcqRel) as u32 - 1; - *self.last_block_hash.write().unwrap() = header.prev_blockhash; - - let mut failed_channels = Vec::new(); - { - let mut channel_lock = self.channel_state.lock().unwrap(); - let channel_state = &mut *channel_lock; - let short_to_id = &mut channel_state.short_to_id; - let pending_msg_events = &mut channel_state.pending_msg_events; - channel_state.by_id.retain(|_, v| { - if let Err(err_msg) = v.update_best_block(new_height, header.time) { - if let Some(short_id) = v.get_short_channel_id() { - short_to_id.remove(&short_id); - } - failed_channels.push(v.force_shutdown(true)); - if let Ok(update) = self.get_channel_update(&v) { - pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { - msg: update - }); - } - pending_msg_events.push(events::MessageSendEvent::HandleError { - node_id: v.get_counterparty_node_id(), - action: msgs::ErrorAction::SendErrorMessage { msg: err_msg }, - }); - false - } else { - true - } - }); - } - - self.handle_init_event_channel_failures(failed_channels); - } - /// Blocks until ChannelManager needs to be persisted or a timeout is reached. It returns a bool /// indicating whether persistence is necessary. Only one listener on /// `await_persistable_update` or `await_persistable_update_timeout` is guaranteed to be woken diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 1be56f7d0..dddd2b8e6 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -112,7 +112,7 @@ pub fn disconnect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, count: u32) let orig_header = node.blocks.borrow_mut().pop().unwrap(); assert!(orig_header.1 > 0); // Cannot disconnect genesis node.chain_monitor.chain_monitor.block_disconnected(&orig_header.0, orig_header.1); - node.node.block_disconnected(&orig_header.0); + node.node.block_disconnected(&orig_header.0, orig_header.1); } } -- 2.39.5