Make the ChannelManager::block_connected API more electrum-friendly
authorMatt Corallo <git@bluematt.me>
Sat, 20 Mar 2021 05:00:54 +0000 (01:00 -0400)
committerMatt Corallo <git@bluematt.me>
Mon, 5 Apr 2021 21:33:04 +0000 (17:33 -0400)
See the similar commit that operates on `Channel`'s internal API
for more details on the reasoning.

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

index 7c5875ac1ef62d31590241228027fd75b954aba2..64933d32545f85beeb46f0f92f1f1b3af158e159 100644 (file)
@@ -435,10 +435,11 @@ pub fn do_test<Out: test_logger::Output>(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);
                        }
                } }
        }
index bffb3e8e214b3a1d6d4d4d75c3e46e4dc6dabfa5..aae5484d7ee68cfa1bc60911cce2a0d3b1e5e0bc 100644 (file)
@@ -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);
index acdc064884994e9936e307d5617c62c5e64fe29b..14f78de4d48660d12dee944eb5b909ab4784b853 100644 (file)
@@ -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<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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<FN: Fn(&mut Channel<Signer>) -> Result<(Option<msgs::FundingLocked>, 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<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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
index d868479eba60d963c56034c0a0acef1f53dba0d7..1be56f7d0305e963c8fb9ee61ce11f377c58f4f5 100644 (file)
@@ -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));
 }