From: Matt Corallo Date: Sat, 20 Mar 2021 05:00:54 +0000 (-0400) Subject: Make the ChannelManager::block_connected API more electrum-friendly X-Git-Tag: v0.0.14~38^2~3 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=a15c8541dc4b55be7c0fc76d252f2501d4cc1735;p=rust-lightning Make the ChannelManager::block_connected API more electrum-friendly See the similar commit that operates on `Channel`'s internal API for more details on the reasoning. --- diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 7c5875ac1..64933d325 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -435,10 +435,11 @@ pub fn do_test(data: &[u8], out: Out) { let chain_hash = genesis_block(Network::Bitcoin).block_hash(); let mut header = BlockHeader { version: 0x20000000, prev_blockhash: chain_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; let txdata: Vec<_> = channel_txn.iter().enumerate().map(|(i, tx)| (i + 1, tx)).collect(); - $node.block_connected(&header, &txdata, 1); + $node.transactions_confirmed(&header, 1, &txdata); + $node.update_best_block(&header, 1); for i in 2..100 { header = BlockHeader { version: 0x20000000, prev_blockhash: header.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - $node.block_connected(&header, &[], i); + $node.update_best_block(&header, i); } } } } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index bffb3e8e2..aae5484d7 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -202,7 +202,8 @@ impl<'a> MoneyLossDetector<'a> { self.blocks_connected += 1; let header = BlockHeader { version: 0x20000000, prev_blockhash: self.header_hashes[self.height].0, merkle_root: Default::default(), time: self.blocks_connected, bits: 42, nonce: 42 }; self.height += 1; - self.manager.block_connected(&header, &txdata, self.height as u32); + self.manager.transactions_confirmed(&header, self.height as u32, &txdata); + self.manager.update_best_block(&header, self.height as u32); (*self.monitor).block_connected(&header, &txdata, self.height as u32); if self.header_hashes.len() > self.height { self.header_hashes[self.height] = (header.block_hash(), self.blocks_connected); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index acdc06488..14f78de4d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3293,8 +3293,13 @@ where L::Target: Logger, { fn block_connected(&self, block: &Block, height: u32) { + assert_eq!(*self.last_block_hash.read().unwrap(), block.header.prev_blockhash, + "Blocks must be connected in chain-order - the connected header must build on the last connected header"); + assert_eq!(self.latest_block_height.load(Ordering::Acquire) as u64, height as u64 - 1, + "Blocks must be connected in chain-order - the connected block height must be one greater than the previous height"); let txdata: Vec<_> = block.txdata.iter().enumerate().collect(); - ChannelManager::block_connected(self, &block.header, &txdata, height); + self.transactions_confirmed(&block.header, height, &txdata); + self.update_best_block(&block.header, height); } fn block_disconnected(&self, header: &BlockHeader, _height: u32) { @@ -3309,22 +3314,11 @@ impl ChannelMana F::Target: FeeEstimator, L::Target: Logger, { - /// Updates channel state based on transactions seen in a connected block. - pub fn block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) { + fn do_chain_event) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage>> + (&self, height: u32, f: FN) { // 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 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); - - assert_eq!(*self.last_block_hash.read().unwrap(), header.prev_blockhash, - "Blocks must be connected in chain-order - the connected header must build on the last connected header"); - assert_eq!(self.latest_block_height.load(Ordering::Acquire) as u64, height as u64 - 1, - "Blocks must be connected in chain-order - the connected header must build on the last connected header"); - 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(); @@ -3334,8 +3328,7 @@ impl ChannelMana 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(|_, channel| { - let res = channel.transactions_confirmed(&block_hash, height, txdata, &self.logger) - .and_then(|_| channel.update_best_block(height, header.time)); + let res = f(channel); if let Ok((chan_res, mut timed_out_pending_htlcs)) = res { for (source, payment_hash) in timed_out_pending_htlcs.drain(..) { let chan_update = self.get_channel_update(&channel).map(|u| u.encode_with_len()).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe @@ -3406,6 +3399,64 @@ 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); } + } + + /// Updates channel state to take note of transactions which were confirmed in the given block + /// at the given height. + /// + /// Note that you must still call (or have called) [`update_best_block`] with the block + /// information which is included here. + /// + /// This method may be called before or after [`update_best_block`] for a given block's + /// transaction data and may be called multiple times with additional transaction data for a + /// given block. + /// + /// This method may be called for a previous block after an [`update_best_block`] call has + /// been made for a later block, however it must *not* be called with transaction data from a + /// block which is no longer in the best chain (ie where [`update_best_block`] has already + /// been informed about a blockchain reorganization which no longer includes the block which + /// corresponds to `header`). + /// + /// [`update_best_block`]: `Self::update_best_block` + pub fn transactions_confirmed(&self, header: &BlockHeader, height: u32, txdata: &TransactionData) { + // 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 block_hash = header.block_hash(); + log_trace!(self.logger, "{} transactions included in block {} at height {} provided", txdata.len(), block_hash, height); + + let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier); + self.do_chain_event(height, |channel| channel.transactions_confirmed(&block_hash, height, txdata, &self.logger).map(|_| (None, Vec::new()))); + } + + /// Updates channel state with the current best blockchain tip. You should attempt to call this + /// quickly after a new block becomes available, however if multiple new blocks become + /// available at the same time, only a single `update_best_block()` call needs to be made. + /// + /// This method should also be called immediately after any block disconnections, once at the + /// reorganization fork point, and once with the new chain tip. Calling this method at the + /// blockchain reorganization fork point ensures we learn when a funding transaction which was + /// previously confirmed is reorganized out of the blockchain, ensuring we do not continue to + /// accept payments which cannot be enforced on-chain. + /// + /// In both the block-connection and block-disconnection case, this method may be called either + /// once per block connected or disconnected, or simply at the fork point and new tip(s), + /// skipping any intermediary blocks. + pub fn update_best_block(&self, header: &BlockHeader, height: u32) { + // 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 block_hash = header.block_hash(); + log_trace!(self.logger, "New best block: {} at height {}", 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; + + self.do_chain_event(height, |channel| channel.update_best_block(height, header.time)); loop { // Update last_node_announcement_serial to be the max of its current value and the diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index d868479eb..1be56f7d0 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -10,7 +10,7 @@ //! A bunch of useful utilities for building networks of nodes and exchanging messages between //! nodes for functional tests. -use chain::Watch; +use chain::{Listen, Watch}; use chain::channelmonitor::ChannelMonitor; use chain::transaction::OutPoint; use ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSecret, PaymentSendFailure}; @@ -102,7 +102,7 @@ pub fn connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block) let txdata: Vec<_> = block.txdata.iter().enumerate().collect(); let height = node.best_block_info().1 + 1; node.chain_monitor.chain_monitor.block_connected(&block.header, &txdata, height); - node.node.block_connected(&block.header, &txdata, height); + node.node.block_connected(&block, height); node.node.test_process_background_events(); node.blocks.borrow_mut().push((block.header, height)); }