Drop ChannelManager::block_disconnected() entirely
authorMatt Corallo <git@bluematt.me>
Sat, 20 Mar 2021 04:22:09 +0000 (00:22 -0400)
committerMatt Corallo <git@bluematt.me>
Mon, 5 Apr 2021 21:33:04 +0000 (17:33 -0400)
It is now entirely redundant with ChannelManager::update_best_block
and is still accessible via `Listen::block_disconnected`.

fuzz/src/full_stack.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs

index aae5484d7ee68cfa1bc60911cce2a0d3b1e5e0bc..132d4f8ebd20123ccde8c120a63ab0e1b1a264cc 100644 (file)
@@ -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;
index 14f78de4d48660d12dee944eb5b909ab4784b853..9f943979d8459ad4fa3e7b4634a04380be82bf51 100644 (file)
@@ -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<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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
index 1be56f7d0305e963c8fb9ee61ce11f377c58f4f5..dddd2b8e61f7068f5006dff3231a3ef79e70d88f 100644 (file)
@@ -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);
        }
 }