]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Merge pull request #815 from TheBlueMatt/2021-03-router-fuzzzzzzzz
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Thu, 8 Apr 2021 02:06:13 +0000 (02:06 +0000)
committerGitHub <noreply@github.com>
Thu, 8 Apr 2021 02:06:13 +0000 (02:06 +0000)
Track full-path htlc-minimum-msat while routing

19 files changed:
background-processor/src/lib.rs
fuzz/src/chanmon_consistency.rs
fuzz/src/full_stack.rs
fuzz/src/router.rs
lightning-block-sync/src/lib.rs
lightning-persister/Cargo.toml
lightning-persister/src/lib.rs
lightning/src/chain/chainmonitor.rs
lightning/src/chain/channelmonitor.rs
lightning/src/chain/mod.rs
lightning/src/lib.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/mod.rs
lightning/src/ln/reorg_tests.rs
lightning/src/util/test_utils.rs

index c3db4d55ade989e13fb8ac228ac77d6460d0502f..6d9db076fa44a6cd0fd2c749f76aa6c4448e18b7 100644 (file)
@@ -12,6 +12,8 @@ use lightning::chain;
 use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use lightning::chain::keysinterface::{Sign, KeysInterface};
 use lightning::ln::channelmanager::ChannelManager;
+use lightning::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler};
+use lightning::ln::peer_handler::{PeerManager, SocketDescriptor};
 use lightning::util::logger::Logger;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, Ordering};
@@ -63,40 +65,50 @@ impl BackgroundProcessor {
        /// [`ChannelManager`]: lightning::ln::channelmanager::ChannelManager
        /// [`ChannelManager::write`]: lightning::ln::channelmanager::ChannelManager#impl-Writeable
        /// [`FilesystemPersister::persist_manager`]: lightning_persister::FilesystemPersister::persist_manager
-       pub fn start<PM, Signer, M, T, K, F, L>(persist_manager: PM, manager: Arc<ChannelManager<Signer, Arc<M>, Arc<T>, Arc<K>, Arc<F>, Arc<L>>>, logger: Arc<L>) -> Self
-       where Signer: 'static + Sign,
-             M: 'static + chain::Watch<Signer>,
-             T: 'static + BroadcasterInterface,
-             K: 'static + KeysInterface<Signer=Signer>,
-             F: 'static + FeeEstimator,
-             L: 'static + Logger,
-             PM: 'static + Send + Fn(&ChannelManager<Signer, Arc<M>, Arc<T>, Arc<K>, Arc<F>, Arc<L>>) -> Result<(), std::io::Error>,
+       pub fn start<PM, Signer, M, T, K, F, L, Descriptor: 'static + SocketDescriptor + Send, CM, RM>(
+               persist_channel_manager: PM,
+               channel_manager: Arc<ChannelManager<Signer, Arc<M>, Arc<T>, Arc<K>, Arc<F>, Arc<L>>>,
+               peer_manager: Arc<PeerManager<Descriptor, Arc<CM>, Arc<RM>, Arc<L>>>, logger: Arc<L>,
+       ) -> Self
+       where
+               Signer: 'static + Sign,
+               M: 'static + chain::Watch<Signer>,
+               T: 'static + BroadcasterInterface,
+               K: 'static + KeysInterface<Signer = Signer>,
+               F: 'static + FeeEstimator,
+               L: 'static + Logger,
+               CM: 'static + ChannelMessageHandler,
+               RM: 'static + RoutingMessageHandler,
+               PM: 'static
+                       + Send
+                       + Fn(
+                               &ChannelManager<Signer, Arc<M>, Arc<T>, Arc<K>, Arc<F>, Arc<L>>,
+                       ) -> Result<(), std::io::Error>,
        {
                let stop_thread = Arc::new(AtomicBool::new(false));
                let stop_thread_clone = stop_thread.clone();
                let handle = thread::spawn(move || -> Result<(), std::io::Error> {
                        let mut current_time = Instant::now();
                        loop {
-                               let updates_available = manager.await_persistable_update_timeout(Duration::from_millis(100));
+                               peer_manager.process_events();
+                               let updates_available =
+                                       channel_manager.await_persistable_update_timeout(Duration::from_millis(100));
                                if updates_available {
-                                       persist_manager(&*manager)?;
+                                       persist_channel_manager(&*channel_manager)?;
                                }
                                // Exit the loop if the background processor was requested to stop.
                                if stop_thread.load(Ordering::Acquire) == true {
                                        log_trace!(logger, "Terminating background processor.");
-                                       return Ok(())
+                                       return Ok(());
                                }
                                if current_time.elapsed().as_secs() > CHAN_FRESHNESS_TIMER {
                                        log_trace!(logger, "Calling manager's timer_chan_freshness_every_min");
-                                       manager.timer_chan_freshness_every_min();
+                                       channel_manager.timer_chan_freshness_every_min();
                                        current_time = Instant::now();
                                }
                        }
                });
-               Self {
-                       stop_thread: stop_thread_clone,
-                       thread_handle: handle,
-               }
+               Self { stop_thread: stop_thread_clone, thread_handle: handle }
        }
 
        /// Stop `BackgroundProcessor`'s thread.
@@ -120,6 +132,7 @@ mod tests {
        use lightning::ln::channelmanager::{ChainParameters, ChannelManager, SimpleArcChannelManager};
        use lightning::ln::features::InitFeatures;
        use lightning::ln::msgs::ChannelMessageHandler;
+       use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor};
        use lightning::util::config::UserConfig;
        use lightning::util::events::{Event, EventsProvider, MessageSendEventsProvider, MessageSendEvent};
        use lightning::util::logger::Logger;
@@ -132,10 +145,21 @@ mod tests {
        use std::time::Duration;
        use super::BackgroundProcessor;
 
+       #[derive(Clone, Eq, Hash, PartialEq)]
+       struct TestDescriptor{}
+       impl SocketDescriptor for TestDescriptor {
+               fn send_data(&mut self, _data: &[u8], _resume_read: bool) -> usize {
+                       0
+               }
+
+               fn disconnect_socket(&mut self) {}
+       }
+
        type ChainMonitor = chainmonitor::ChainMonitor<InMemorySigner, Arc<test_utils::TestChainSource>, Arc<test_utils::TestBroadcaster>, Arc<test_utils::TestFeeEstimator>, Arc<test_utils::TestLogger>, Arc<FilesystemPersister>>;
 
        struct Node {
                node: Arc<SimpleArcChannelManager<ChainMonitor, test_utils::TestBroadcaster, test_utils::TestFeeEstimator, test_utils::TestLogger>>,
+               peer_manager: Arc<PeerManager<TestDescriptor, Arc<test_utils::TestChannelMessageHandler>, Arc<test_utils::TestRoutingMessageHandler>, Arc<test_utils::TestLogger>>>,
                persister: Arc<FilesystemPersister>,
                logger: Arc<test_utils::TestLogger>,
        }
@@ -176,7 +200,9 @@ mod tests {
                                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 };
+                       let msg_handler = MessageHandler { chan_handler: Arc::new(test_utils::TestChannelMessageHandler::new()), route_handler: Arc::new(test_utils::TestRoutingMessageHandler::new() )};
+                       let peer_manager = Arc::new(PeerManager::new(msg_handler, keys_manager.get_node_secret(), &seed, logger.clone()));
+                       let node = Node { node: manager, peer_manager, persister, logger };
                        nodes.push(node);
                }
                nodes
@@ -220,7 +246,7 @@ mod tests {
                // Initiate the background processors to watch each node.
                let data_dir = nodes[0].persister.get_data_dir();
                let callback = move |node: &ChannelManager<InMemorySigner, Arc<ChainMonitor>, Arc<test_utils::TestBroadcaster>, Arc<KeysManager>, Arc<test_utils::TestFeeEstimator>, Arc<test_utils::TestLogger>>| FilesystemPersister::persist_manager(data_dir.clone(), node);
-               let bg_processor = BackgroundProcessor::start(callback, nodes[0].node.clone(), nodes[0].logger.clone());
+               let bg_processor = BackgroundProcessor::start(callback, nodes[0].node.clone(), nodes[0].peer_manager.clone(), nodes[0].logger.clone());
 
                // Go through the channel creation process until each node should have something persisted.
                let tx = open_channel!(nodes[0], nodes[1], 100000);
@@ -275,7 +301,7 @@ mod tests {
                let nodes = create_nodes(1, "test_chan_freshness_called".to_string());
                let data_dir = nodes[0].persister.get_data_dir();
                let callback = move |node: &ChannelManager<InMemorySigner, Arc<ChainMonitor>, Arc<test_utils::TestBroadcaster>, Arc<KeysManager>, Arc<test_utils::TestFeeEstimator>, Arc<test_utils::TestLogger>>| FilesystemPersister::persist_manager(data_dir.clone(), node);
-               let bg_processor = BackgroundProcessor::start(callback, nodes[0].node.clone(), nodes[0].logger.clone());
+               let bg_processor = BackgroundProcessor::start(callback, nodes[0].node.clone(), nodes[0].peer_manager.clone(), nodes[0].logger.clone());
                loop {
                        let log_entries = nodes[0].logger.lines.lock().unwrap();
                        let desired_log = "Calling manager's timer_chan_freshness_every_min".to_string();
@@ -302,7 +328,7 @@ mod tests {
                }
 
                let nodes = create_nodes(2, "test_persist_error".to_string());
-               let bg_processor = BackgroundProcessor::start(persist_manager, nodes[0].node.clone(), nodes[0].logger.clone());
+               let bg_processor = BackgroundProcessor::start(persist_manager, nodes[0].node.clone(), nodes[0].peer_manager.clone(), nodes[0].logger.clone());
                open_channel!(nodes[0], nodes[1], 100000);
 
                let _ = bg_processor.thread_handle.join().unwrap().expect_err("Errored persisting manager: test");
index 7c5875ac1ef62d31590241228027fd75b954aba2..87b95cf2a538a972e882a2d2ad0ca6fb2bd99c47 100644 (file)
@@ -234,7 +234,7 @@ fn check_api_err(api_err: APIError) {
                                _ if err.starts_with("Cannot send value that would put our balance under counterparty-announced channel reserve value") => {},
                                _ if err.starts_with("Cannot send value that would overdraw remaining funds.") => {},
                                _ if err.starts_with("Cannot send value that would not leave enough to pay for fees.") => {},
-                               _ => panic!(err),
+                               _ => panic!("{}", err),
                        }
                },
                APIError::MonitorUpdateFailed => {
@@ -435,11 +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);
-                       for i in 2..100 {
+                       $node.transactions_confirmed(&header, 1, &txdata);
+                       for _ 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, 99);
                } }
        }
 
index bffb3e8e214b3a1d6d4d4d75c3e46e4dc6dabfa5..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;
@@ -202,7 +203,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);
@@ -216,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 e93618ca7137905ab7677a775e3697e390b6cd02..fb720c9916c2d7b38ee653df9fea675f3f8ea23f 100644 (file)
@@ -130,7 +130,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                                        msgs::DecodeError::InvalidValue => return,
                                        msgs::DecodeError::BadLengthDescriptor => return,
                                        msgs::DecodeError::ShortRead => panic!("We picked the length..."),
-                                       msgs::DecodeError::Io(e) => panic!(format!("{:?}", e)),
+                                       msgs::DecodeError::Io(e) => panic!("{:?}", e),
                                }
                        }
                }}
index bc937b590716cfdf30843ca46c59deba0717d2fd..ac031132a71946f8706954d49138ff3f7b1e574e 100644 (file)
@@ -75,12 +75,12 @@ pub trait BlockSource : Sync + Send {
 }
 
 /// Result type for `BlockSource` requests.
-type BlockSourceResult<T> = Result<T, BlockSourceError>;
+pub type BlockSourceResult<T> = Result<T, BlockSourceError>;
 
 // TODO: Replace with BlockSourceResult once `async` trait functions are supported. For details,
 // see: https://areweasyncyet.rs.
 /// Result type for asynchronous `BlockSource` requests.
-type AsyncBlockSourceResult<'a, T> = Pin<Box<dyn Future<Output = BlockSourceResult<T>> + 'a + Send>>;
+pub type AsyncBlockSourceResult<'a, T> = Pin<Box<dyn Future<Output = BlockSourceResult<T>> + 'a + Send>>;
 
 /// Error type for `BlockSource` requests.
 ///
index 0a9821f14cfc86768a57f48b1e31af2e80c07868..a6e7242b0cb9c13e0eae26ea6bb43063870eb8c1 100644 (file)
@@ -8,6 +8,9 @@ description = """
 Utilities to manage Rust-Lightning channel data persistence and retrieval.
 """
 
+[features]
+unstable = ["lightning/unstable"]
+
 [dependencies]
 bitcoin = "0.26"
 lightning = { version = "0.0.13", path = "../lightning" }
index afcda803397597f12d0d3a597dcf7bff44b3c248..af11dfd167b4d51f1c6a1bde4ea7ab5eb42308a9 100644 (file)
@@ -3,6 +3,9 @@
 #![deny(broken_intra_doc_links)]
 #![deny(missing_docs)]
 
+#![cfg_attr(all(test, feature = "unstable"), feature(test))]
+#[cfg(all(test, feature = "unstable"))] extern crate test;
+
 mod util;
 
 extern crate lightning;
@@ -241,7 +244,7 @@ mod tests {
                // Force close because cooperative close doesn't result in any persisted
                // updates.
                nodes[0].node.force_close_channel(&nodes[0].node.list_channels()[0].channel_id).unwrap();
-               check_closed_broadcast!(nodes[0], false);
+               check_closed_broadcast!(nodes[0], true);
                check_added_monitors!(nodes[0], 1);
 
                let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -249,7 +252,7 @@ mod tests {
 
                let header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
                connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[0].clone()]});
-               check_closed_broadcast!(nodes[1], false);
+               check_closed_broadcast!(nodes[1], true);
                check_added_monitors!(nodes[1], 1);
 
                // Make sure everything is persisted as expected after close.
@@ -330,3 +333,15 @@ mod tests {
                added_monitors.clear();
        }
 }
+
+#[cfg(all(test, feature = "unstable"))]
+pub mod bench {
+       use test::Bencher;
+
+       #[bench]
+       fn bench_sends(bench: &mut Bencher) {
+               let persister_a = super::FilesystemPersister::new("bench_filesystem_persister_a".to_string());
+               let persister_b = super::FilesystemPersister::new("bench_filesystem_persister_b".to_string());
+               lightning::ln::channelmanager::bench::bench_two_sends(bench, persister_a, persister_b);
+       }
+}
index eb17b469a0365df2de4464ef313cc243441b6767..0fd088019247e4e8e9adbea39adb99798e274dd4 100644 (file)
@@ -26,7 +26,7 @@
 use bitcoin::blockdata::block::{Block, BlockHeader};
 
 use chain;
-use chain::Filter;
+use chain::{Filter, WatchedOutput};
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use chain::channelmonitor;
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr, MonitorEvent, Persist};
@@ -82,18 +82,40 @@ where C::Target: chain::Filter,
        /// descendants of such transactions. It is not necessary to re-fetch the block to obtain
        /// updated `txdata`.
        pub fn block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
+               let mut dependent_txdata = Vec::new();
                let monitors = self.monitors.read().unwrap();
                for monitor in monitors.values() {
                        let mut txn_outputs = monitor.block_connected(header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
 
+                       // Register any new outputs with the chain source for filtering, storing any dependent
+                       // transactions from within the block that previously had not been included in txdata.
                        if let Some(ref chain_source) = self.chain_source {
+                               let block_hash = header.block_hash();
                                for (txid, outputs) in txn_outputs.drain(..) {
                                        for (idx, output) in outputs.iter() {
-                                               chain_source.register_output(&OutPoint { txid, index: *idx as u16 }, &output.script_pubkey);
+                                               // Register any new outputs with the chain source for filtering and recurse
+                                               // if it indicates that there are dependent transactions within the block
+                                               // that had not been previously included in txdata.
+                                               let output = WatchedOutput {
+                                                       block_hash: Some(block_hash),
+                                                       outpoint: OutPoint { txid, index: *idx as u16 },
+                                                       script_pubkey: output.script_pubkey.clone(),
+                                               };
+                                               if let Some(tx) = chain_source.register_output(output) {
+                                                       dependent_txdata.push(tx);
+                                               }
                                        }
                                }
                        }
                }
+
+               // Recursively call for any dependent transactions that were identified by the chain source.
+               if !dependent_txdata.is_empty() {
+                       dependent_txdata.sort_unstable_by_key(|(index, _tx)| *index);
+                       dependent_txdata.dedup_by_key(|(index, _tx)| *index);
+                       let txdata: Vec<_> = dependent_txdata.iter().map(|(index, tx)| (*index, tx)).collect();
+                       self.block_connected(header, &txdata, height);
+               }
        }
 
        /// Dispatches to per-channel monitors, which are responsible for updating their on-chain view
@@ -245,3 +267,56 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
                pending_events
        }
 }
+
+#[cfg(test)]
+mod tests {
+       use ::{check_added_monitors, get_local_commitment_txn};
+       use ln::features::InitFeatures;
+       use ln::functional_test_utils::*;
+       use util::events::EventsProvider;
+       use util::events::MessageSendEventsProvider;
+       use util::test_utils::{OnRegisterOutput, TxOutReference};
+
+       /// Tests that in-block dependent transactions are processed by `block_connected` when not
+       /// included in `txdata` but returned by [`chain::Filter::register_output`]. For instance,
+       /// a (non-anchor) commitment transaction's HTLC output may be spent in the same block as the
+       /// commitment transaction itself. An Electrum client may filter the commitment transaction but
+       /// needs to return the HTLC transaction so it can be processed.
+       #[test]
+       fn connect_block_checks_dependent_transactions() {
+               let chanmon_cfgs = create_chanmon_cfgs(2);
+               let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+               let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+               let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+               let channel = create_announced_chan_between_nodes(
+                       &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+
+               // Send a payment, saving nodes[0]'s revoked commitment and HTLC-Timeout transactions.
+               let (commitment_tx, htlc_tx) = {
+                       let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 5_000_000).0;
+                       let mut txn = get_local_commitment_txn!(nodes[0], channel.2);
+                       claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 5_000_000);
+
+                       assert_eq!(txn.len(), 2);
+                       (txn.remove(0), txn.remove(0))
+               };
+
+               // Set expectations on nodes[1]'s chain source to return dependent transactions.
+               let htlc_output = TxOutReference(commitment_tx.clone(), 0);
+               let to_local_output = TxOutReference(commitment_tx.clone(), 1);
+               let htlc_timeout_output = TxOutReference(htlc_tx.clone(), 0);
+               nodes[1].chain_source
+                       .expect(OnRegisterOutput { with: htlc_output, returns: Some((1, htlc_tx)) })
+                       .expect(OnRegisterOutput { with: to_local_output, returns: None })
+                       .expect(OnRegisterOutput { with: htlc_timeout_output, returns: None });
+
+               // Notify nodes[1] that nodes[0]'s revoked commitment transaction was mined. The chain
+               // source should return the dependent HTLC transaction when the HTLC output is registered.
+               mine_transaction(&nodes[1], &commitment_tx);
+
+               // Clean up so uninteresting assertions don't fail.
+               check_added_monitors!(nodes[1], 1);
+               nodes[1].node.get_and_clear_pending_msg_events();
+               nodes[1].node.get_and_clear_pending_events();
+       }
+}
index 28b6da4722a7b4e3820541b30c89047d1e8e5cfe..939337d7b42d572d64c7ab5b1a6224479f6c1ce9 100644 (file)
@@ -40,6 +40,7 @@ use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLC
 use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
 use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
 use chain;
+use chain::WatchedOutput;
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use chain::transaction::{OutPoint, TransactionData};
 use chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, KeysInterface};
@@ -1174,7 +1175,11 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                for (txid, outputs) in lock.get_outputs_to_watch().iter() {
                        for (index, script_pubkey) in outputs.iter() {
                                assert!(*index <= u16::max_value() as u32);
-                               filter.register_output(&OutPoint { txid: *txid, index: *index as u16 }, script_pubkey);
+                               filter.register_output(WatchedOutput {
+                                       block_hash: None,
+                                       outpoint: OutPoint { txid: *txid, index: *index as u16 },
+                                       script_pubkey: script_pubkey.clone(),
+                               });
                        }
                }
        }
index 7d410b9b71dd116cbe22d2858e9664ec6bc14381..18c7fd55d7b6643bbbc140a5208df42e39e42ea3 100644 (file)
@@ -11,7 +11,7 @@
 
 use bitcoin::blockdata::block::{Block, BlockHeader};
 use bitcoin::blockdata::script::Script;
-use bitcoin::blockdata::transaction::TxOut;
+use bitcoin::blockdata::transaction::{Transaction, TxOut};
 use bitcoin::hash_types::{BlockHash, Txid};
 
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr, MonitorEvent};
@@ -129,9 +129,38 @@ pub trait Filter: Send + Sync {
        /// a spending condition.
        fn register_tx(&self, txid: &Txid, script_pubkey: &Script);
 
-       /// Registers interest in spends of a transaction output identified by `outpoint` having
-       /// `script_pubkey` as the spending condition.
-       fn register_output(&self, outpoint: &OutPoint, script_pubkey: &Script);
+       /// Registers interest in spends of a transaction output.
+       ///
+       /// Optionally, when `output.block_hash` is set, should return any transaction spending the
+       /// output that is found in the corresponding block along with its index.
+       ///
+       /// This return value is useful for Electrum clients in order to supply in-block descendant
+       /// transactions which otherwise were not included. This is not necessary for other clients if
+       /// such descendant transactions were already included (e.g., when a BIP 157 client provides the
+       /// full block).
+       fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)>;
+}
+
+/// A transaction output watched by a [`ChannelMonitor`] for spends on-chain.
+///
+/// Used to convey to a [`Filter`] such an output with a given spending condition. Any transaction
+/// spending the output must be given to [`ChannelMonitor::block_connected`] either directly or via
+/// the return value of [`Filter::register_output`].
+///
+/// If `block_hash` is `Some`, this indicates the output was created in the corresponding block and
+/// may have been spent there. See [`Filter::register_output`] for details.
+///
+/// [`ChannelMonitor`]: channelmonitor::ChannelMonitor
+/// [`ChannelMonitor::block_connected`]: channelmonitor::ChannelMonitor::block_connected
+pub struct WatchedOutput {
+       /// First block where the transaction output may have been spent.
+       pub block_hash: Option<BlockHash>,
+
+       /// Outpoint identifying the transaction output.
+       pub outpoint: OutPoint,
+
+       /// Spending condition of the transaction output.
+       pub script_pubkey: Script,
 }
 
 impl<T: Listen> Listen for std::ops::Deref<Target = T> {
index 2d764c8b71b1355200ca062087fcb22044b5c24b..9bf9470458f8310f005788769d781b74055fd835 100644 (file)
@@ -28,8 +28,8 @@
 #![allow(bare_trait_objects)]
 #![allow(ellipsis_inclusive_range_patterns)]
 
-#![cfg_attr(all(test, feature = "unstable"), feature(test))]
-#[cfg(all(test, feature = "unstable"))] extern crate test;
+#![cfg_attr(all(any(test, feature = "_test_utils"), feature = "unstable"), feature(test))]
+#[cfg(all(any(test, feature = "_test_utils"), feature = "unstable"))] extern crate test;
 
 extern crate bitcoin;
 #[cfg(any(test, feature = "_test_utils"))] extern crate hex;
index a4cc5a02ae2e9da9406ba3bd59ea4c2d667e3dec..13eb4ed8b465ae0de4473d82877ed8f13a0b9a63 100644 (file)
@@ -241,7 +241,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool, persister_fail
        // ...and make sure we can force-close a frozen channel
        nodes[0].node.force_close_channel(&channel_id).unwrap();
        check_added_monitors!(nodes[0], 1);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
 
        // TODO: Once we hit the chain with the failure transaction we should check that we get a
        // PaymentFailed event
index ca1887d8b2595c1485f7e120841b9368019237c5..174eed9b9185669dcc704f81d5b849d62837d641 100644 (file)
@@ -7,7 +7,6 @@
 // You may not use this file except in accordance with one or both of these
 // licenses.
 
-use bitcoin::blockdata::block::BlockHeader;
 use bitcoin::blockdata::script::{Script,Builder};
 use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType};
 use bitcoin::blockdata::opcodes;
@@ -376,13 +375,10 @@ pub(super) struct Channel<Signer: Sign> {
 
        last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, holder_sig)
 
-       /// The hash of the block in which the funding transaction reached our CONF_TARGET. We use this
-       /// to detect unconfirmation after a serialize-unserialize roundtrip where we may not see a full
-       /// series of block_connected/block_disconnected calls. Obviously this is not a guarantee as we
-       /// could miss the funding_tx_confirmed_in block as well, but it serves as a useful fallback.
+       /// The hash of the block in which the funding transaction was included.
        funding_tx_confirmed_in: Option<BlockHash>,
+       funding_tx_confirmation_height: u64,
        short_channel_id: Option<u64>,
-       funding_tx_confirmations: u64,
 
        counterparty_dust_limit_satoshis: u64,
        #[cfg(test)]
@@ -441,10 +437,6 @@ struct CommitmentTxInfoCached {
 }
 
 pub const OUR_MAX_HTLCS: u16 = 50; //TODO
-/// Confirmation count threshold at which we close a channel. Ideally we'd keep the channel around
-/// on ice until the funding transaction gets more confirmations, but the LN protocol doesn't
-/// really allow for this, so instead we're stuck closing it out at that point.
-const UNCONF_THRESHOLD: u32 = 6;
 const SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT: u64 = 79; // prevout: 36, nSequence: 4, script len: 1, witness lengths: (3+1)/4, sig: 73/4, if-selector: 1, redeemScript: (6 ops + 2*33 pubkeys + 1*2 delay)/4
 const B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT: u64 = 104; // prevout: 40, nSequence: 4, script len: 1, witness lengths: 3/4, sig: 73/4, pubkey: 33/4, output: 31 (TODO: Wrong? Useless?)
 
@@ -581,8 +573,8 @@ impl<Signer: Sign> Channel<Signer> {
                        last_sent_closing_fee: None,
 
                        funding_tx_confirmed_in: None,
+                       funding_tx_confirmation_height: 0,
                        short_channel_id: None,
-                       funding_tx_confirmations: 0,
 
                        feerate_per_kw: feerate,
                        counterparty_dust_limit_satoshis: 0,
@@ -818,8 +810,8 @@ impl<Signer: Sign> Channel<Signer> {
                        last_sent_closing_fee: None,
 
                        funding_tx_confirmed_in: None,
+                       funding_tx_confirmation_height: 0,
                        short_channel_id: None,
-                       funding_tx_confirmations: 0,
 
                        feerate_per_kw: msg.feerate_per_kw,
                        channel_value_satoshis: msg.funding_satoshis,
@@ -3509,26 +3501,140 @@ impl<Signer: Sign> Channel<Signer> {
                self.network_sync == UpdateStatus::DisabledMarked
        }
 
-       /// When we receive a new block, we (a) check whether the block contains the funding
-       /// transaction (which would start us counting blocks until we send the funding_signed), and
-       /// (b) check the height of the block against outbound holding cell HTLCs in case we need to
-       /// give up on them prematurely and time them out. Everything else (e.g. commitment
-       /// transaction broadcasts, channel closure detection, HTLC transaction broadcasting, etc) is
+       fn check_get_funding_locked(&mut self, height: u32) -> Option<msgs::FundingLocked> {
+               if self.funding_tx_confirmation_height == 0 {
+                       return None;
+               }
+
+               let funding_tx_confirmations = height as i64 - self.funding_tx_confirmation_height as i64 + 1;
+               if funding_tx_confirmations <= 0 {
+                       self.funding_tx_confirmation_height = 0;
+               }
+
+               if funding_tx_confirmations < self.minimum_depth as i64 {
+                       return None;
+               }
+
+               let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
+               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 ignore.
+                       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 ignore.
+                       false
+               };
+
+               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 Some(msgs::FundingLocked {
+                                       channel_id: self.channel_id,
+                                       next_per_commitment_point,
+                               });
+                       } else {
+                               self.monitor_pending_funding_locked = true;
+                       }
+               }
+               None
+       }
+
+       /// When a transaction is confirmed, we check whether it is or spends the funding transaction
+       /// In the first case, we store the confirmation height and calculating the short channel id.
+       /// In the second, we simply return an Err indicating we need to be force-closed now.
+       pub fn transactions_confirmed<L: Deref>(&mut self, block_hash: &BlockHash, height: u32, txdata: &TransactionData, logger: &L)
+                       -> Result<Option<msgs::FundingLocked>, msgs::ErrorMessage> where L::Target: Logger {
+               let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
+               for &(index_in_block, tx) in txdata.iter() {
+                       if let Some(funding_txo) = self.get_funding_txo() {
+                               // If we haven't yet sent a funding_locked, but are in FundingSent (ignoring
+                               // whether they've sent a funding_locked or not), check if we should send one.
+                               if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 {
+                                       if tx.txid() == funding_txo.txid {
+                                               let txo_idx = funding_txo.index as usize;
+                                               if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() ||
+                                                               tx.output[txo_idx].value != self.channel_value_satoshis {
+                                                       if self.is_outbound() {
+                                                               // If we generated the funding transaction and it doesn't match what it
+                                                               // should, the client is really broken and we should just panic and
+                                                               // tell them off. That said, because hash collisions happen with high
+                                                               // probability in fuzztarget mode, if we're fuzzing we just close the
+                                                               // channel and move on.
+                                                               #[cfg(not(feature = "fuzztarget"))]
+                                                               panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
+                                                       }
+                                                       self.channel_state = ChannelState::ShutdownComplete as u32;
+                                                       self.update_time_counter += 1;
+                                                       return Err(msgs::ErrorMessage {
+                                                               channel_id: self.channel_id(),
+                                                               data: "funding tx had wrong script/value or output index".to_owned()
+                                                       });
+                                               } else {
+                                                       if self.is_outbound() {
+                                                               for input in tx.input.iter() {
+                                                                       if input.witness.is_empty() {
+                                                                               // We generated a malleable funding transaction, implying we've
+                                                                               // just exposed ourselves to funds loss to our counterparty.
+                                                                               #[cfg(not(feature = "fuzztarget"))]
+                                                                               panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
+                                                                       }
+                                                               }
+                                                       }
+                                                       self.funding_tx_confirmation_height = height as u64;
+                                                       self.funding_tx_confirmed_in = Some(*block_hash);
+                                                       self.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) {
+                                                               Ok(scid) => Some(scid),
+                                                               Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"),
+                                                       }
+                                               }
+                                       }
+                                       // If we allow 1-conf funding, we may need to check for funding_locked here and
+                                       // send it immediately instead of waiting for an update_best_block call (which
+                                       // may have already happened for this block).
+                                       if let Some(funding_locked) = self.check_get_funding_locked(height) {
+                                               return Ok(Some(funding_locked));
+                                       }
+                               }
+                               for inp in tx.input.iter() {
+                                       if inp.previous_output == funding_txo.into_bitcoin_outpoint() {
+                                               log_trace!(logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.txid(), inp.previous_output.txid, inp.previous_output.vout, log_bytes!(self.channel_id()));
+                                               return Err(msgs::ErrorMessage {
+                                                       channel_id: self.channel_id(),
+                                                       data: "Commitment or closing transaction was confirmed on chain.".to_owned()
+                                               });
+                                       }
+                               }
+                       }
+               }
+               Ok(None)
+       }
+
+       /// When a new block is connected, we check the height of the block against outbound holding
+       /// cell HTLCs in case we need to give up on them prematurely and time them out. Everything
+       /// else (e.g. commitment transaction broadcasts, HTLC transaction broadcasting, etc) is
        /// handled by the ChannelMonitor.
        ///
        /// If we return Err, the channel may have been closed, at which point the standard
        /// requirements apply - no calls may be made except those explicitly stated to be allowed
        /// post-shutdown.
-       /// Only returns an ErrorAction of DisconnectPeer, if Err.
        ///
        /// May return some HTLCs (and their payment_hash) which have timed out and should be failed
        /// back.
-       pub fn block_connected(&mut self, header: &BlockHeader, txdata: &TransactionData, height: u32) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> {
+       pub fn update_best_block(&mut self, height: u32, highest_header_time: u32) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> {
                let mut timed_out_htlcs = Vec::new();
+               let unforwarded_htlc_cltv_limit = height + HTLC_FAIL_BACK_BUFFER;
                self.holding_cell_htlc_updates.retain(|htlc_update| {
                        match htlc_update {
                                &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, ref source, ref cltv_expiry, .. } => {
-                                       if *cltv_expiry <= height + HTLC_FAIL_BACK_BUFFER {
+                                       if *cltv_expiry <= unforwarded_htlc_cltv_limit {
                                                timed_out_htlcs.push((source.clone(), payment_hash.clone()));
                                                false
                                        } else { true }
@@ -3537,112 +3643,36 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                });
 
-               if self.funding_tx_confirmations > 0 {
-                       self.funding_tx_confirmations += 1;
+               self.update_time_counter = cmp::max(self.update_time_counter, highest_header_time);
+
+               if let Some(funding_locked) = self.check_get_funding_locked(height) {
+                       return Ok((Some(funding_locked), timed_out_htlcs));
                }
 
                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();
-                               if tx.txid() == funding_txo.txid {
-                                       let txo_idx = funding_txo.index as usize;
-                                       if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() ||
-                                                       tx.output[txo_idx].value != self.channel_value_satoshis {
-                                               if self.is_outbound() {
-                                                       // If we generated the funding transaction and it doesn't match what it
-                                                       // should, the client is really broken and we should just panic and
-                                                       // tell them off. That said, because hash collisions happen with high
-                                                       // probability in fuzztarget mode, if we're fuzzing we just close the
-                                                       // channel and move on.
-                                                       #[cfg(not(feature = "fuzztarget"))]
-                                                       panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
-                                               }
-                                               self.channel_state = ChannelState::ShutdownComplete as u32;
-                                               self.update_time_counter += 1;
-                                               return Err(msgs::ErrorMessage {
-                                                       channel_id: self.channel_id(),
-                                                       data: "funding tx had wrong script/value".to_owned()
-                                               });
-                                       } else {
-                                               if self.is_outbound() {
-                                                       for input in tx.input.iter() {
-                                                               if input.witness.is_empty() {
-                                                                       // We generated a malleable funding transaction, implying we've
-                                                                       // just exposed ourselves to funds loss to our counterparty.
-                                                                       #[cfg(not(feature = "fuzztarget"))]
-                                                                       panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
-                                                               }
-                                                       }
-                                               }
-                                               self.funding_tx_confirmations = 1;
-                                               self.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) {
-                                                       Ok(scid) => Some(scid),
-                                                       Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"),
-                                               }
-                                       }
-                               }
+               if non_shutdown_state >= ChannelState::ChannelFunded as u32 ||
+                  (non_shutdown_state & ChannelState::OurFundingLocked as u32) == ChannelState::OurFundingLocked as u32 {
+                       let mut funding_tx_confirmations = height as i64 - self.funding_tx_confirmation_height as i64 + 1;
+                       if self.funding_tx_confirmation_height == 0 {
+                               // Note that check_get_funding_locked may reset funding_tx_confirmation_height to
+                               // zero if it has been reorged out, however in either case, our state flags
+                               // indicate we've already sent a funding_locked
+                               funding_tx_confirmations = 0;
                        }
-               }
 
-               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(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,
-                               //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));
-                                       }
-                               }
+                       // If we've sent funding_locked (or have both sent and received funding_locked), and
+                       // the funding transaction's confirmation count has dipped below minimum_depth / 2,
+                       // close the channel and hope we can get the latest state on chain (because presumably
+                       // the funding transaction is at least still in the mempool of most nodes).
+                       if funding_tx_confirmations < self.minimum_depth as i64 / 2 {
+                               return Err(msgs::ErrorMessage {
+                                       channel_id: self.channel_id(),
+                                       data: format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth, funding_tx_confirmations),
+                               });
                        }
                }
-               Ok((None, timed_out_htlcs))
-       }
 
-       /// Called by channelmanager based on chain blocks being disconnected.
-       /// Returns true if we need to close the channel now due to funding transaction
-       /// unconfirmation/reorg.
-       pub fn block_disconnected(&mut self, header: &BlockHeader) -> bool {
-               if self.funding_tx_confirmations > 0 {
-                       self.funding_tx_confirmations -= 1;
-                       if self.funding_tx_confirmations == UNCONF_THRESHOLD as u64 {
-                               return true;
-                       }
-               }
-               if Some(header.block_hash()) == self.funding_tx_confirmed_in {
-                       self.funding_tx_confirmations = self.minimum_depth as u64 - 1;
-               }
-               false
+               Ok((None, timed_out_htlcs))
        }
 
        // Methods to get unprompted messages to send to the remote end (or where we already returned
@@ -4466,8 +4496,8 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                }
 
                self.funding_tx_confirmed_in.write(writer)?;
+               self.funding_tx_confirmation_height.write(writer)?;
                self.short_channel_id.write(writer)?;
-               self.funding_tx_confirmations.write(writer)?;
 
                self.counterparty_dust_limit_satoshis.write(writer)?;
                self.holder_dust_limit_satoshis.write(writer)?;
@@ -4636,8 +4666,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                };
 
                let funding_tx_confirmed_in = Readable::read(reader)?;
+               let funding_tx_confirmation_height = Readable::read(reader)?;
                let short_channel_id = Readable::read(reader)?;
-               let funding_tx_confirmations = Readable::read(reader)?;
 
                let counterparty_dust_limit_satoshis = Readable::read(reader)?;
                let holder_dust_limit_satoshis = Readable::read(reader)?;
@@ -4716,8 +4746,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                        last_sent_closing_fee,
 
                        funding_tx_confirmed_in,
+                       funding_tx_confirmation_height,
                        short_channel_id,
-                       funding_tx_confirmations,
 
                        counterparty_dust_limit_satoshis,
                        holder_dust_limit_satoshis,
index 50f8ccbb74f4833175b00060fcd7ee882873c71b..efb2fe9c37bd9a8ec2c9a36d9a521072a6f8c6f3 100644 (file)
@@ -434,6 +434,7 @@ pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref,
        #[cfg(not(any(test, feature = "_test_utils")))]
        channel_state: Mutex<ChannelHolder<Signer>>,
        our_network_key: SecretKey,
+       our_network_pubkey: PublicKey,
 
        /// Used to track the last value sent in a node_announcement "timestamp" field. We ensure this
        /// value increases strictly since we don't assume access to a time source.
@@ -822,7 +823,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
                        latest_block_height: AtomicUsize::new(params.latest_height),
                        last_block_hash: RwLock::new(params.latest_hash),
-                       secp_ctx,
 
                        channel_state: Mutex::new(ChannelHolder{
                                by_id: HashMap::new(),
@@ -832,6 +832,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                pending_msg_events: Vec::new(),
                        }),
                        our_network_key: keys_manager.get_node_secret(),
+                       our_network_pubkey: PublicKey::from_secret_key(&secp_ctx, &keys_manager.get_node_secret()),
+                       secp_ctx,
 
                        last_node_announcement_serial: AtomicUsize::new(0),
 
@@ -1002,16 +1004,14 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
        }
 
-       fn force_close_channel_with_peer(&self, channel_id: &[u8; 32], peer_node_id: Option<&PublicKey>) -> Result<(), APIError> {
+       fn force_close_channel_with_peer(&self, channel_id: &[u8; 32], peer_node_id: Option<&PublicKey>) -> Result<PublicKey, APIError> {
                let mut chan = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_state_lock;
                        if let hash_map::Entry::Occupied(chan) = channel_state.by_id.entry(channel_id.clone()) {
                                if let Some(node_id) = peer_node_id {
                                        if chan.get().get_counterparty_node_id() != *node_id {
-                                               // Error or Ok here doesn't matter - the result is only exposed publicly
-                                               // when peer_node_id is None anyway.
-                                               return Ok(());
+                                               return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()});
                                        }
                                }
                                if let Some(short_id) = chan.get().get_short_channel_id() {
@@ -1031,14 +1031,27 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        });
                }
 
-               Ok(())
+               Ok(chan.get_counterparty_node_id())
        }
 
        /// Force closes a channel, immediately broadcasting the latest local commitment transaction to
        /// the chain and rejecting new HTLCs on the given channel. Fails if channel_id is unknown to the manager.
        pub fn force_close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
-               self.force_close_channel_with_peer(channel_id, None)
+               match self.force_close_channel_with_peer(channel_id, None) {
+                       Ok(counterparty_node_id) => {
+                               self.channel_state.lock().unwrap().pending_msg_events.push(
+                                       events::MessageSendEvent::HandleError {
+                                               node_id: counterparty_node_id,
+                                               action: msgs::ErrorAction::SendErrorMessage {
+                                                       msg: msgs::ErrorMessage { channel_id: *channel_id, data: "Channel force-closed".to_owned() }
+                                               },
+                                       }
+                               );
+                               Ok(())
+                       },
+                       Err(e) => Err(e)
+               }
        }
 
        /// Force close all channels, immediately broadcasting the latest local commitment transaction
@@ -2315,7 +2328,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
        /// Gets the node_id held by this ChannelManager
        pub fn get_our_node_id(&self) -> PublicKey {
-               PublicKey::from_secret_key(&self.secp_ctx, &self.our_network_key)
+               self.our_network_pubkey.clone()
        }
 
        /// Restores a single, given channel to normal operation after a
@@ -3194,6 +3207,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                        msg: update
                                                                });
                                                        }
+                                                       pending_msg_events.push(events::MessageSendEvent::HandleError {
+                                                               node_id: chan.get_counterparty_node_id(),
+                                                               action: msgs::ErrorAction::SendErrorMessage {
+                                                                       msg: msgs::ErrorMessage { channel_id: chan.channel_id(), data: "Channel force-closed".to_owned() }
+                                                               },
+                                                       });
                                                }
                                        },
                                }
@@ -3276,12 +3295,26 @@ 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) {
-               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));
        }
 }
 
@@ -3292,22 +3325,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();
@@ -3317,7 +3339,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.block_connected(header, txdata, height);
+                               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
@@ -3343,32 +3365,23 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id());
                                        }
                                } else if let Err(e) = res {
+                                       if let Some(short_id) = channel.get_short_channel_id() {
+                                               short_to_id.remove(&short_id);
+                                       }
+                                       // It looks like our counterparty went on-chain or funding transaction was
+                                       // reorged out of the main chain. Close the channel.
+                                       failed_channels.push(channel.force_shutdown(true));
+                                       if let Ok(update) = self.get_channel_update(&channel) {
+                                               pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
+                                                       msg: update
+                                               });
+                                       }
                                        pending_msg_events.push(events::MessageSendEvent::HandleError {
                                                node_id: channel.get_counterparty_node_id(),
                                                action: msgs::ErrorAction::SendErrorMessage { msg: e },
                                        });
                                        return false;
                                }
-                               if let Some(funding_txo) = channel.get_funding_txo() {
-                                       for &(_, tx) in txdata.iter() {
-                                               for inp in tx.input.iter() {
-                                                       if inp.previous_output == funding_txo.into_bitcoin_outpoint() {
-                                                               log_trace!(self.logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.txid(), inp.previous_output.txid, inp.previous_output.vout, log_bytes!(channel.channel_id()));
-                                                               if let Some(short_id) = channel.get_short_channel_id() {
-                                                                       short_to_id.remove(&short_id);
-                                                               }
-                                                               // It looks like our counterparty went on-chain. Close the channel.
-                                                               failed_channels.push(channel.force_shutdown(true));
-                                                               if let Ok(update) = self.get_channel_update(&channel) {
-                                                                       pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
-                                                                               msg: update
-                                                                       });
-                                                               }
-                                                               return false;
-                                                       }
-                                               }
-                                       }
-                               }
                                true
                        });
 
@@ -3397,6 +3410,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(|a| (a, 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
@@ -3412,48 +3483,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");
-               self.latest_block_height.fetch_sub(1, Ordering::AcqRel);
-               *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 v.block_disconnected(header) {
-                                       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
-                                               });
-                                       }
-                                       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
@@ -4318,7 +4347,6 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 
                        latest_block_height: AtomicUsize::new(latest_block_height as usize),
                        last_block_hash: RwLock::new(last_block_hash),
-                       secp_ctx,
 
                        channel_state: Mutex::new(ChannelHolder {
                                by_id,
@@ -4328,6 +4356,8 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                pending_msg_events: Vec::new(),
                        }),
                        our_network_key: args.keys_manager.get_node_secret(),
+                       our_network_pubkey: PublicKey::from_secret_key(&secp_ctx, &args.keys_manager.get_node_secret()),
+                       secp_ctx,
 
                        last_node_announcement_serial: AtomicUsize::new(last_node_announcement_serial as usize),
 
@@ -4404,3 +4434,154 @@ mod tests {
                }
        }
 }
+
+#[cfg(all(any(test, feature = "_test_utils"), feature = "unstable"))]
+pub mod bench {
+       use chain::Listen;
+       use chain::chainmonitor::ChainMonitor;
+       use chain::channelmonitor::Persist;
+       use chain::keysinterface::{KeysManager, InMemorySigner};
+       use chain::transaction::OutPoint;
+       use ln::channelmanager::{ChainParameters, ChannelManager, PaymentHash, PaymentPreimage};
+       use ln::features::InitFeatures;
+       use ln::functional_test_utils::*;
+       use ln::msgs::ChannelMessageHandler;
+       use routing::network_graph::NetworkGraph;
+       use routing::router::get_route;
+       use util::test_utils;
+       use util::config::UserConfig;
+       use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
+
+       use bitcoin::hashes::Hash;
+       use bitcoin::hashes::sha256::Hash as Sha256;
+       use bitcoin::{Block, BlockHeader, Transaction, TxOut};
+
+       use std::sync::Mutex;
+
+       use test::Bencher;
+
+       struct NodeHolder<'a, P: Persist<InMemorySigner>> {
+               node: &'a ChannelManager<InMemorySigner,
+                       &'a ChainMonitor<InMemorySigner, &'a test_utils::TestChainSource,
+                               &'a test_utils::TestBroadcaster, &'a test_utils::TestFeeEstimator,
+                               &'a test_utils::TestLogger, &'a P>,
+                       &'a test_utils::TestBroadcaster, &'a KeysManager,
+                       &'a test_utils::TestFeeEstimator, &'a test_utils::TestLogger>
+       }
+
+       #[cfg(test)]
+       #[bench]
+       fn bench_sends(bench: &mut Bencher) {
+               bench_two_sends(bench, test_utils::TestPersister::new(), test_utils::TestPersister::new());
+       }
+
+       pub fn bench_two_sends<P: Persist<InMemorySigner>>(bench: &mut Bencher, persister_a: P, persister_b: P) {
+               // Do a simple benchmark of sending a payment back and forth between two nodes.
+               // Note that this is unrealistic as each payment send will require at least two fsync
+               // calls per node.
+               let network = bitcoin::Network::Testnet;
+               let genesis_hash = bitcoin::blockdata::constants::genesis_block(network).header.block_hash();
+
+               let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())};
+               let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
+
+               let mut config: UserConfig = Default::default();
+               config.own_channel_config.minimum_depth = 1;
+
+               let logger_a = test_utils::TestLogger::with_id("node a".to_owned());
+               let chain_monitor_a = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_a);
+               let seed_a = [1u8; 32];
+               let keys_manager_a = KeysManager::new(&seed_a, 42, 42);
+               let node_a = ChannelManager::new(&fee_estimator, &chain_monitor_a, &tx_broadcaster, &logger_a, &keys_manager_a, config.clone(), ChainParameters {
+                       network,
+                       latest_hash: genesis_hash,
+                       latest_height: 0,
+               });
+               let node_a_holder = NodeHolder { node: &node_a };
+
+               let logger_b = test_utils::TestLogger::with_id("node a".to_owned());
+               let chain_monitor_b = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_b);
+               let seed_b = [2u8; 32];
+               let keys_manager_b = KeysManager::new(&seed_b, 42, 42);
+               let node_b = ChannelManager::new(&fee_estimator, &chain_monitor_b, &tx_broadcaster, &logger_b, &keys_manager_b, config.clone(), ChainParameters {
+                       network,
+                       latest_hash: genesis_hash,
+                       latest_height: 0,
+               });
+               let node_b_holder = NodeHolder { node: &node_b };
+
+               node_a.create_channel(node_b.get_our_node_id(), 8_000_000, 100_000_000, 42, None).unwrap();
+               node_b.handle_open_channel(&node_a.get_our_node_id(), InitFeatures::known(), &get_event_msg!(node_a_holder, MessageSendEvent::SendOpenChannel, node_b.get_our_node_id()));
+               node_a.handle_accept_channel(&node_b.get_our_node_id(), InitFeatures::known(), &get_event_msg!(node_b_holder, MessageSendEvent::SendAcceptChannel, node_a.get_our_node_id()));
+
+               let tx;
+               if let Event::FundingGenerationReady { temporary_channel_id, output_script, .. } = get_event!(node_a_holder, Event::FundingGenerationReady) {
+                       tx = Transaction { version: 2, lock_time: 0, input: Vec::new(), output: vec![TxOut {
+                               value: 8_000_000, script_pubkey: output_script,
+                       }]};
+                       let funding_outpoint = OutPoint { txid: tx.txid(), index: 0 };
+                       node_a.funding_transaction_generated(&temporary_channel_id, funding_outpoint);
+               } else { panic!(); }
+
+               node_b.handle_funding_created(&node_a.get_our_node_id(), &get_event_msg!(node_a_holder, MessageSendEvent::SendFundingCreated, node_b.get_our_node_id()));
+               node_a.handle_funding_signed(&node_b.get_our_node_id(), &get_event_msg!(node_b_holder, MessageSendEvent::SendFundingSigned, node_a.get_our_node_id()));
+
+               get_event!(node_a_holder, Event::FundingBroadcastSafe);
+
+               let block = Block {
+                       header: BlockHeader { version: 0x20000000, prev_blockhash: genesis_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
+                       txdata: vec![tx],
+               };
+               Listen::block_connected(&node_a, &block, 1);
+               Listen::block_connected(&node_b, &block, 1);
+
+               node_a.handle_funding_locked(&node_b.get_our_node_id(), &get_event_msg!(node_b_holder, MessageSendEvent::SendFundingLocked, node_a.get_our_node_id()));
+               node_b.handle_funding_locked(&node_a.get_our_node_id(), &get_event_msg!(node_a_holder, MessageSendEvent::SendFundingLocked, node_b.get_our_node_id()));
+
+               let dummy_graph = NetworkGraph::new(genesis_hash);
+
+               macro_rules! send_payment {
+                       ($node_a: expr, $node_b: expr) => {
+                               let usable_channels = $node_a.list_usable_channels();
+                               let route = get_route(&$node_a.get_our_node_id(), &dummy_graph, &$node_b.get_our_node_id(), None, Some(&usable_channels.iter().map(|r| r).collect::<Vec<_>>()), &[], 10_000, TEST_FINAL_CLTV, &logger_a).unwrap();
+
+                               let payment_preimage = PaymentPreimage([0; 32]);
+                               let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
+
+                               $node_a.send_payment(&route, payment_hash, &None).unwrap();
+                               let payment_event = SendEvent::from_event($node_a.get_and_clear_pending_msg_events().pop().unwrap());
+                               $node_b.handle_update_add_htlc(&$node_a.get_our_node_id(), &payment_event.msgs[0]);
+                               $node_b.handle_commitment_signed(&$node_a.get_our_node_id(), &payment_event.commitment_msg);
+                               let (raa, cs) = get_revoke_commit_msgs!(NodeHolder { node: &$node_b }, $node_a.get_our_node_id());
+                               $node_a.handle_revoke_and_ack(&$node_b.get_our_node_id(), &raa);
+                               $node_a.handle_commitment_signed(&$node_b.get_our_node_id(), &cs);
+                               $node_b.handle_revoke_and_ack(&$node_a.get_our_node_id(), &get_event_msg!(NodeHolder { node: &$node_a }, MessageSendEvent::SendRevokeAndACK, $node_b.get_our_node_id()));
+
+                               expect_pending_htlcs_forwardable!(NodeHolder { node: &$node_b });
+                               expect_payment_received!(NodeHolder { node: &$node_b }, payment_hash, 10_000);
+                               assert!($node_b.claim_funds(payment_preimage, &None, 10_000));
+
+                               match $node_b.get_and_clear_pending_msg_events().pop().unwrap() {
+                                       MessageSendEvent::UpdateHTLCs { node_id, updates } => {
+                                               assert_eq!(node_id, $node_a.get_our_node_id());
+                                               $node_a.handle_update_fulfill_htlc(&$node_b.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
+                                               $node_a.handle_commitment_signed(&$node_b.get_our_node_id(), &updates.commitment_signed);
+                                       },
+                                       _ => panic!("Failed to generate claim event"),
+                               }
+
+                               let (raa, cs) = get_revoke_commit_msgs!(NodeHolder { node: &$node_a }, $node_b.get_our_node_id());
+                               $node_b.handle_revoke_and_ack(&$node_a.get_our_node_id(), &raa);
+                               $node_b.handle_commitment_signed(&$node_a.get_our_node_id(), &cs);
+                               $node_a.handle_revoke_and_ack(&$node_b.get_our_node_id(), &get_event_msg!(NodeHolder { node: &$node_b }, MessageSendEvent::SendRevokeAndACK, $node_a.get_our_node_id()));
+
+                               expect_payment_sent!(NodeHolder { node: &$node_a }, payment_preimage);
+                       }
+               }
+
+               bench.iter(|| {
+                       send_payment!(node_a, node_b);
+                       send_payment!(node_b, node_a);
+               });
+       }
+}
index e7f61c6a3dfeefb23f845b7a47d6ab0efa5abf60..af11ef5d86baf01c98d0d815a753ac23ffc3535b 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};
@@ -60,21 +60,15 @@ pub fn mine_transaction<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transac
 /// Mine the given transaction at the given height, mining blocks as required to build to that
 /// height
 pub fn confirm_transaction_at<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transaction, conf_height: u32) {
-       let starting_block = node.best_block_info();
+       let first_connect_height = node.best_block_info().1 + 1;
+       assert!(first_connect_height <= conf_height);
+       if conf_height - first_connect_height >= 1 {
+               connect_blocks(node, conf_height - first_connect_height);
+       }
        let mut block = Block {
-               header: BlockHeader { version: 0x20000000, prev_blockhash: starting_block.0, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
+               header: BlockHeader { version: 0x20000000, prev_blockhash: node.best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
                txdata: Vec::new(),
        };
-       let height = starting_block.1 + 1;
-       assert!(height <= conf_height);
-       for _ in height..conf_height {
-               connect_block(node, &block);
-               block = Block {
-                       header: BlockHeader { version: 0x20000000, prev_blockhash: block.header.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
-                       txdata: vec![],
-               };
-       }
-
        for _ in 0..*node.network_chan_count.borrow() { // Make sure we don't end up with channels at the same short id by offsetting by chan_count
                block.txdata.push(Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() });
        }
@@ -82,37 +76,94 @@ pub fn confirm_transaction_at<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &T
        connect_block(node, &block);
 }
 
+/// The possible ways we may notify a ChannelManager of a new block
+pub enum ConnectStyle {
+       /// Calls update_best_block first, detecting transactions in the block only after receiving the
+       /// header and height information.
+       BestBlockFirst,
+       /// The same as BestBlockFirst, however when we have multiple blocks to connect, we only
+       /// make a single update_best_block call.
+       BestBlockFirstSkippingBlocks,
+       /// Calls transactions_confirmed first, detecting transactions in the block before updating the
+       /// header and height information.
+       TransactionsFirst,
+       /// The same as TransactionsFirst, however when we have multiple blocks to connect, we only
+       /// make a single update_best_block call.
+       TransactionsFirstSkippingBlocks,
+       /// Provides the full block via the chain::Listen interface. In the current code this is
+       /// equivalent to TransactionsFirst with some additional assertions.
+       FullBlockViaListen,
+}
+
 pub fn connect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, depth: u32) -> BlockHash {
+       let skip_intermediaries = match *node.connect_style.borrow() {
+               ConnectStyle::BestBlockFirstSkippingBlocks|ConnectStyle::TransactionsFirstSkippingBlocks => true,
+               _ => false,
+       };
+
        let mut block = Block {
                header: BlockHeader { version: 0x2000000, prev_blockhash: node.best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
                txdata: vec![],
        };
-       connect_block(node, &block);
-       for _ in 2..depth + 1 {
+       assert!(depth >= 1);
+       for _ in 0..depth - 1 {
+               do_connect_block(node, &block, skip_intermediaries);
                block = Block {
                        header: BlockHeader { version: 0x20000000, prev_blockhash: block.header.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
                        txdata: vec![],
                };
-               connect_block(node, &block);
        }
+       connect_block(node, &block);
        block.header.block_hash()
 }
 
 pub fn connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block) {
+       do_connect_block(node, block, false);
+}
+
+fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block, skip_manager: bool) {
        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);
+       if !skip_manager {
+               match *node.connect_style.borrow() {
+                       ConnectStyle::BestBlockFirst|ConnectStyle::BestBlockFirstSkippingBlocks => {
+                               node.node.update_best_block(&block.header, height);
+                               node.node.transactions_confirmed(&block.header, height, &block.txdata.iter().enumerate().collect::<Vec<_>>());
+                       },
+                       ConnectStyle::TransactionsFirst|ConnectStyle::TransactionsFirstSkippingBlocks => {
+                               node.node.transactions_confirmed(&block.header, height, &block.txdata.iter().enumerate().collect::<Vec<_>>());
+                               node.node.update_best_block(&block.header, height);
+                       },
+                       ConnectStyle::FullBlockViaListen => {
+                               Listen::block_connected(node.node, &block, height);
+                       }
+               }
+       }
        node.node.test_process_background_events();
        node.blocks.borrow_mut().push((block.header, height));
 }
 
 pub fn disconnect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, count: u32) {
-       for _ in 0..count {
+       for i in 0..count {
                let orig_header = node.blocks.borrow_mut().pop().unwrap();
                assert!(orig_header.1 > 0); // Cannot disconnect genesis
+               let prev_header = node.blocks.borrow().last().unwrap().clone();
+
                node.chain_monitor.chain_monitor.block_disconnected(&orig_header.0, orig_header.1);
-               node.node.block_disconnected(&orig_header.0);
+               match *node.connect_style.borrow() {
+                       ConnectStyle::FullBlockViaListen => {
+                               Listen::block_disconnected(node.node, &orig_header.0, orig_header.1);
+                       },
+                       ConnectStyle::BestBlockFirstSkippingBlocks|ConnectStyle::TransactionsFirstSkippingBlocks => {
+                               if i == count - 1 {
+                                       node.node.update_best_block(&prev_header.0, prev_header.1);
+                               }
+                       },
+                       _ => {
+                               node.node.update_best_block(&prev_header.0, prev_header.1);
+                       },
+               }
        }
 }
 
@@ -152,6 +203,7 @@ pub struct Node<'a, 'b: 'a, 'c: 'b> {
        pub network_chan_count: Rc<RefCell<u32>>,
        pub logger: &'c test_utils::TestLogger,
        pub blocks: RefCell<Vec<(BlockHeader, u32)>>,
+       pub connect_style: Rc<RefCell<ConnectStyle>>,
 }
 impl<'a, 'b, 'c> Node<'a, 'b, 'c> {
        pub fn best_block_hash(&self) -> BlockHash {
@@ -313,6 +365,24 @@ macro_rules! get_event_msg {
        }
 }
 
+/// Get a specific event from the pending events queue.
+#[macro_export]
+macro_rules! get_event {
+       ($node: expr, $event_type: path) => {
+               {
+                       let mut events = $node.node.get_and_clear_pending_events();
+                       assert_eq!(events.len(), 1);
+                       let ev = events.pop().unwrap();
+                       match ev {
+                               $event_type { .. } => {
+                                       ev
+                               },
+                               _ => panic!("Unexpected event"),
+                       }
+               }
+       }
+}
+
 #[cfg(test)]
 macro_rules! get_htlc_update_msgs {
        ($node: expr, $node_id: expr) => {
@@ -341,7 +411,8 @@ macro_rules! get_feerate {
        }
 }
 
-#[cfg(test)]
+/// Returns any local commitment transactions for the channel.
+#[macro_export]
 macro_rules! get_local_commitment_txn {
        ($node: expr, $channel_id: expr) => {
                {
@@ -847,7 +918,7 @@ macro_rules! expect_pending_htlcs_forwardable {
        }}
 }
 
-#[cfg(test)]
+#[cfg(any(test, feature = "unstable"))]
 macro_rules! expect_payment_received {
        ($node: expr, $expected_payment_hash: expr, $expected_recv_value: expr) => {
                let events = $node.node.get_and_clear_pending_events();
@@ -1224,6 +1295,7 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec<NodeC
        let mut nodes = Vec::new();
        let chan_count = Rc::new(RefCell::new(0));
        let payment_count = Rc::new(RefCell::new(0));
+       let connect_style = Rc::new(RefCell::new(ConnectStyle::FullBlockViaListen));
 
        for i in 0..node_count {
                let net_graph_msg_handler = NetGraphMsgHandler::new(cfgs[i].chain_source.genesis_hash, None, cfgs[i].logger);
@@ -1232,7 +1304,8 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec<NodeC
                                 keys_manager: &cfgs[i].keys_manager, node: &chan_mgrs[i], net_graph_msg_handler,
                                 node_seed: cfgs[i].node_seed, network_chan_count: chan_count.clone(),
                                 network_payment_count: payment_count.clone(), logger: cfgs[i].logger,
-                                blocks: RefCell::new(vec![(genesis_block(Network::Testnet).header, 0)])
+                                blocks: RefCell::new(vec![(genesis_block(Network::Testnet).header, 0)]),
+                                connect_style: Rc::clone(&connect_style),
                })
        }
 
@@ -1345,22 +1418,36 @@ pub fn check_preimage_claim<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, prev_txn: &Vec<
 
 pub fn get_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec<Node<'a, 'b, 'c>>, a: usize, b: usize)  {
        let events_1 = nodes[a].node.get_and_clear_pending_msg_events();
-       assert_eq!(events_1.len(), 1);
+       assert_eq!(events_1.len(), 2);
        let as_update = match events_1[0] {
                MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
                        msg.clone()
                },
                _ => panic!("Unexpected event"),
        };
+       match events_1[1] {
+               MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => {
+                       assert_eq!(node_id, nodes[b].node.get_our_node_id());
+                       assert_eq!(msg.data, "Commitment or closing transaction was confirmed on chain.");
+               },
+               _ => panic!("Unexpected event"),
+       }
 
        let events_2 = nodes[b].node.get_and_clear_pending_msg_events();
-       assert_eq!(events_2.len(), 1);
+       assert_eq!(events_2.len(), 2);
        let bs_update = match events_2[0] {
                MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
                        msg.clone()
                },
                _ => panic!("Unexpected event"),
        };
+       match events_2[1] {
+               MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => {
+                       assert_eq!(node_id, nodes[a].node.get_our_node_id());
+                       assert_eq!(msg.data, "Commitment or closing transaction was confirmed on chain.");
+               },
+               _ => panic!("Unexpected event"),
+       }
 
        for node in nodes {
                node.net_graph_msg_handler.handle_channel_update(&as_update).unwrap();
index c7a79e32210639fddcd4c3d01ab16e6f311248a3..293ccf9f3ea949940525c11aaa18375c6a7e6c43 100644 (file)
@@ -394,8 +394,7 @@ fn test_multi_flight_update_fee() {
        check_added_monitors!(nodes[1], 1);
 }
 
-#[test]
-fn test_1_conf_open() {
+fn do_test_1_conf_open(connect_style: ConnectStyle) {
        // Previously, if the minium_depth config was set to 1, we'd never send a funding_locked. This
        // tests that we properly send one in that case.
        let mut alice_config = UserConfig::default();
@@ -409,7 +408,8 @@ fn test_1_conf_open() {
        let chanmon_cfgs = create_chanmon_cfgs(2);
        let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(alice_config), Some(bob_config)]);
-       let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+       let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+       *nodes[0].connect_style.borrow_mut() = connect_style;
 
        let tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 100000, 10001, InitFeatures::known(), InitFeatures::known());
        mine_transaction(&nodes[1], &tx);
@@ -425,6 +425,12 @@ fn test_1_conf_open() {
                node.net_graph_msg_handler.handle_channel_update(&bs_update).unwrap();
        }
 }
+#[test]
+fn test_1_conf_open() {
+       do_test_1_conf_open(ConnectStyle::BestBlockFirst);
+       do_test_1_conf_open(ConnectStyle::TransactionsFirst);
+       do_test_1_conf_open(ConnectStyle::FullBlockViaListen);
+}
 
 fn do_test_sanity_on_in_flight_opens(steps: u8) {
        // Previously, we had issues deserializing channels when we hadn't connected the first block
@@ -1508,10 +1514,14 @@ fn test_duplicate_htlc_different_direction_onchain() {
        check_spends!(htlc_pair.1, remote_txn[0]);
 
        let events = nodes[0].node.get_and_clear_pending_msg_events();
-       assert_eq!(events.len(), 2);
+       assert_eq!(events.len(), 3);
        for e in events {
                match e {
                        MessageSendEvent::BroadcastChannelUpdate { .. } => {},
+                       MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => {
+                               assert_eq!(node_id, nodes[1].node.get_our_node_id());
+                               assert_eq!(msg.data, "Commitment or closing transaction was confirmed on chain.");
+                       },
                        MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, .. } } => {
                                assert!(update_add_htlcs.is_empty());
                                assert!(update_fail_htlcs.is_empty());
@@ -2334,6 +2344,7 @@ fn channel_monitor_network_test() {
        // Simple case with no pending HTLCs:
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), true);
        check_added_monitors!(nodes[1], 1);
+       check_closed_broadcast!(nodes[1], false);
        {
                let mut node_txn = test_txn_broadcast(&nodes[1], &chan_1, None, HTLCType::NONE);
                assert_eq!(node_txn.len(), 1);
@@ -2341,7 +2352,7 @@ fn channel_monitor_network_test() {
                check_added_monitors!(nodes[0], 1);
                test_txn_broadcast(&nodes[0], &chan_1, None, HTLCType::NONE);
        }
-       get_announce_close_broadcast_events(&nodes, 0, 1);
+       check_closed_broadcast!(nodes[0], true);
        assert_eq!(nodes[0].node.list_channels().len(), 0);
        assert_eq!(nodes[1].node.list_channels().len(), 1);
 
@@ -2350,6 +2361,7 @@ fn channel_monitor_network_test() {
 
        // Simple case of one pending HTLC to HTLC-Timeout
        nodes[1].node.peer_disconnected(&nodes[2].node.get_our_node_id(), true);
+       check_closed_broadcast!(nodes[1], false);
        check_added_monitors!(nodes[1], 1);
        {
                let mut node_txn = test_txn_broadcast(&nodes[1], &chan_2, None, HTLCType::TIMEOUT);
@@ -2357,7 +2369,7 @@ fn channel_monitor_network_test() {
                check_added_monitors!(nodes[2], 1);
                test_txn_broadcast(&nodes[2], &chan_2, None, HTLCType::NONE);
        }
-       get_announce_close_broadcast_events(&nodes, 1, 2);
+       check_closed_broadcast!(nodes[2], true);
        assert_eq!(nodes[1].node.list_channels().len(), 0);
        assert_eq!(nodes[2].node.list_channels().len(), 1);
 
@@ -2385,6 +2397,7 @@ fn channel_monitor_network_test() {
        // HTLC-Timeout and a nodes[3] claim against it (+ its own announces)
        nodes[2].node.peer_disconnected(&nodes[3].node.get_our_node_id(), true);
        check_added_monitors!(nodes[2], 1);
+       check_closed_broadcast!(nodes[2], false);
        let node2_commitment_txid;
        {
                let node_txn = test_txn_broadcast(&nodes[2], &chan_3, None, HTLCType::TIMEOUT);
@@ -2396,7 +2409,7 @@ fn channel_monitor_network_test() {
                check_added_monitors!(nodes[3], 1);
                check_preimage_claim(&nodes[3], &node_txn);
        }
-       get_announce_close_broadcast_events(&nodes, 2, 3);
+       check_closed_broadcast!(nodes[3], true);
        assert_eq!(nodes[2].node.list_channels().len(), 0);
        assert_eq!(nodes[3].node.list_channels().len(), 1);
 
@@ -2412,13 +2425,19 @@ fn channel_monitor_network_test() {
        let (close_chan_update_1, close_chan_update_2) = {
                connect_blocks(&nodes[3], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
                let events = nodes[3].node.get_and_clear_pending_msg_events();
-               assert_eq!(events.len(), 1);
+               assert_eq!(events.len(), 2);
                let close_chan_update_1 = match events[0] {
                        MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
                                msg.clone()
                        },
                        _ => panic!("Unexpected event"),
                };
+               match events[1] {
+                       MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id } => {
+                               assert_eq!(node_id, nodes[4].node.get_our_node_id());
+                       },
+                       _ => panic!("Unexpected event"),
+               }
                check_added_monitors!(nodes[3], 1);
 
                // Clear bumped claiming txn spending node 2 commitment tx. Bumped txn are generated after reaching some height timer.
@@ -2438,13 +2457,19 @@ fn channel_monitor_network_test() {
 
                connect_blocks(&nodes[4], TEST_FINAL_CLTV - CLTV_CLAIM_BUFFER + 2);
                let events = nodes[4].node.get_and_clear_pending_msg_events();
-               assert_eq!(events.len(), 1);
+               assert_eq!(events.len(), 2);
                let close_chan_update_2 = match events[0] {
                        MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
                                msg.clone()
                        },
                        _ => panic!("Unexpected event"),
                };
+               match events[1] {
+                       MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id } => {
+                               assert_eq!(node_id, nodes[3].node.get_our_node_id());
+                       },
+                       _ => panic!("Unexpected event"),
+               }
                check_added_monitors!(nodes[4], 1);
                test_txn_broadcast(&nodes[4], &chan_4, None, HTLCType::SUCCESS);
 
@@ -2785,7 +2810,7 @@ fn test_htlc_on_chain_success() {
        assert_eq!(updates.update_fulfill_htlcs.len(), 1);
 
        mine_transaction(&nodes[2], &commitment_tx[0]);
-       check_closed_broadcast!(nodes[2], false);
+       check_closed_broadcast!(nodes[2], true);
        check_added_monitors!(nodes[2], 1);
        let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 3 (commitment tx, 2*htlc-success tx), ChannelMonitor : 2 (2 * HTLC-Success tx)
        assert_eq!(node_txn.len(), 5);
@@ -2818,12 +2843,17 @@ fn test_htlc_on_chain_success() {
                assert_eq!(added_monitors[1].0.txid, chan_1.3.txid());
                added_monitors.clear();
        }
-       assert_eq!(events.len(), 2);
+       assert_eq!(events.len(), 3);
        match events[0] {
                MessageSendEvent::BroadcastChannelUpdate { .. } => {},
                _ => panic!("Unexpected event"),
        }
        match events[1] {
+               MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {},
+               _ => panic!("Unexpected event"),
+       }
+
+       match events[2] {
                MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, ref update_fulfill_htlcs, ref update_fail_malformed_htlcs, .. } } => {
                        assert!(update_add_htlcs.is_empty());
                        assert!(update_fail_htlcs.is_empty());
@@ -2877,7 +2907,7 @@ fn test_htlc_on_chain_success() {
        let commitment_tx = get_local_commitment_txn!(nodes[0], chan_1.2);
        check_spends!(commitment_tx[0], chan_1.3);
        mine_transaction(&nodes[1], &commitment_tx[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 3 (commitment tx + HTLC-Sucess * 2), ChannelMonitor : 1 (HTLC-Success)
        assert_eq!(node_txn.len(), 4);
@@ -2897,7 +2927,7 @@ fn test_htlc_on_chain_success() {
        // Verify that A's ChannelManager is able to extract preimage from preimage tx and generate PaymentSent
        let mut header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42};
        connect_block(&nodes[0], &Block { header, txdata: vec![commitment_tx[0].clone(), node_txn[0].clone()] });
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 2);
@@ -2964,7 +2994,7 @@ fn test_htlc_on_chain_timeout() {
                _ => panic!("Unexpected event"),
        };
        mine_transaction(&nodes[2], &commitment_tx[0]);
-       check_closed_broadcast!(nodes[2], false);
+       check_closed_broadcast!(nodes[2], true);
        check_added_monitors!(nodes[2], 1);
        let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 1 (commitment tx)
        assert_eq!(node_txn.len(), 1);
@@ -2996,7 +3026,7 @@ fn test_htlc_on_chain_timeout() {
 
        mine_transaction(&nodes[1], &timeout_tx);
        check_added_monitors!(nodes[1], 1);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        {
                // B will rebroadcast a fee-bumped timeout transaction here.
                let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
@@ -3033,7 +3063,7 @@ fn test_htlc_on_chain_timeout() {
 
        mine_transaction(&nodes[0], &commitment_tx[0]);
 
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : 1 timeout tx
        assert_eq!(node_txn.len(), 3);
@@ -3070,7 +3100,7 @@ fn test_simple_commitment_revoked_fail_backward() {
        mine_transaction(&nodes[1], &revoked_local_txn[0]);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
        check_added_monitors!(nodes[1], 1);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
 
        expect_pending_htlcs_forwardable!(nodes[1]);
        check_added_monitors!(nodes[1], 1);
@@ -3241,11 +3271,18 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
        check_added_monitors!(nodes[1], 1);
 
        let events = nodes[1].node.get_and_clear_pending_msg_events();
-       assert_eq!(events.len(), if deliver_bs_raa { 3 } else { 2 });
+       assert_eq!(events.len(), if deliver_bs_raa { 4 } else { 3 });
        match events[if deliver_bs_raa { 1 } else { 0 }] {
                MessageSendEvent::BroadcastChannelUpdate { msg: msgs::ChannelUpdate { .. } } => {},
                _ => panic!("Unexpected event"),
        }
+       match events[if deliver_bs_raa { 2 } else { 1 }] {
+               MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id, ref data } }, node_id: _ } => {
+                       assert_eq!(channel_id, chan_2.2);
+                       assert_eq!(data.as_str(), "Commitment or closing transaction was confirmed on chain.");
+               },
+               _ => panic!("Unexpected event"),
+       }
        if deliver_bs_raa {
                match events[0] {
                        MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, ref update_fulfill_htlcs, ref update_fail_malformed_htlcs, .. } } => {
@@ -3258,7 +3295,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
                        _ => panic!("Unexpected event"),
                }
        }
-       match events[if deliver_bs_raa { 2 } else { 1 }] {
+       match events[if deliver_bs_raa { 3 } else { 2 }] {
                MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, ref update_fulfill_htlcs, ref update_fail_malformed_htlcs, ref commitment_signed, .. } } => {
                        assert!(update_add_htlcs.is_empty());
                        assert_eq!(update_fail_htlcs.len(), 3);
@@ -3407,7 +3444,7 @@ fn test_htlc_ignore_latest_remote_commitment() {
 
        route_payment(&nodes[0], &[&nodes[1]], 10000000);
        nodes[0].node.force_close_channel(&nodes[0].node.list_channels()[0].channel_id).unwrap();
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -3415,7 +3452,7 @@ fn test_htlc_ignore_latest_remote_commitment() {
 
        let mut header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[1].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[1].clone()]});
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        // Duplicate the connect_block call since this may happen due to other listeners
@@ -3469,7 +3506,7 @@ fn test_force_close_fail_back() {
        // transaction and ensure nodes[1] doesn't fail-backwards (this was originally a bug!).
 
        nodes[2].node.force_close_channel(&payment_event.commitment_msg.channel_id).unwrap();
-       check_closed_broadcast!(nodes[2], false);
+       check_closed_broadcast!(nodes[2], true);
        check_added_monitors!(nodes[2], 1);
        let tx = {
                let mut node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -3483,7 +3520,7 @@ fn test_force_close_fail_back() {
        mine_transaction(&nodes[1], &tx);
 
        // Note no UpdateHTLCs event here from nodes[1] to nodes[0]!
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        // Now check that if we add the preimage to ChannelMonitor it broadcasts our HTLC-Success..
@@ -4658,7 +4695,7 @@ fn test_claim_sizeable_push_msat() {
 
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 99000000, InitFeatures::known(), InitFeatures::known());
        nodes[1].node.force_close_channel(&chan.2).unwrap();
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
        assert_eq!(node_txn.len(), 1);
@@ -4684,7 +4721,7 @@ fn test_claim_on_remote_sizeable_push_msat() {
 
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 99000000, InitFeatures::known(), InitFeatures::known());
        nodes[0].node.force_close_channel(&chan.2).unwrap();
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -4693,7 +4730,7 @@ fn test_claim_on_remote_sizeable_push_msat() {
        assert_eq!(node_txn[0].output.len(), 2); // We can't force trimming of to_remote output as channel_reserve_satoshis block us to do so at channel opening
 
        mine_transaction(&nodes[1], &node_txn[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 
@@ -4720,7 +4757,7 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() {
 
        claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 3_000_000);
        mine_transaction(&nodes[1], &revoked_local_txn[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -4846,7 +4883,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
        claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 3_000_000);
 
        mine_transaction(&nodes[1], &revoked_local_txn[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -4882,7 +4919,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
 
        // A will generate HTLC-Timeout from revoked commitment tx
        mine_transaction(&nodes[0], &revoked_local_txn[0]);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let revoked_htlc_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -4895,7 +4932,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
        // B will generate justice tx from A's revoked commitment/HTLC tx
        let header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[1].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        connect_block(&nodes[1], &Block { header, txdata: vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()] });
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -4951,7 +4988,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
 
        // B will generate HTLC-Success from revoked commitment tx
        mine_transaction(&nodes[1], &revoked_local_txn[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 
@@ -4967,7 +5004,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
        // A will generate justice tx from B's revoked commitment/HTLC tx
        let header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        connect_block(&nodes[0], &Block { header, txdata: vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()] });
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -5041,7 +5078,7 @@ fn test_onchain_to_onchain_claim() {
        assert!(updates.update_fail_malformed_htlcs.is_empty());
 
        mine_transaction(&nodes[2], &commitment_tx[0]);
-       check_closed_broadcast!(nodes[2], false);
+       check_closed_broadcast!(nodes[2], true);
        check_added_monitors!(nodes[2], 1);
 
        let c_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 2 (commitment tx, HTLC-Success tx), ChannelMonitor : 1 (HTLC-Success tx)
@@ -5075,12 +5112,17 @@ fn test_onchain_to_onchain_claim() {
        }
        check_added_monitors!(nodes[1], 1);
        let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
+       assert_eq!(msg_events.len(), 3);
        check_added_monitors!(nodes[1], 1);
        match msg_events[0] {
-               MessageSendEvent::BroadcastChannelUpdate {  .. } => {},
+               MessageSendEvent::BroadcastChannelUpdate { .. } => {},
                _ => panic!("Unexpected event"),
        }
        match msg_events[1] {
+               MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {},
+               _ => panic!("Unexpected event"),
+       }
+       match msg_events[2] {
                MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, .. } } => {
                        assert!(update_add_htlcs.is_empty());
                        assert!(update_fail_htlcs.is_empty());
@@ -5103,7 +5145,7 @@ fn test_onchain_to_onchain_claim() {
        assert!(b_txn[0].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
        assert_eq!(b_txn[0].lock_time, 0); // Success tx
 
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 }
 
@@ -5128,7 +5170,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
        check_spends!(commitment_txn[0], chan_2.3);
 
        mine_transaction(&nodes[1], &commitment_txn[0]);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        let htlc_timeout_tx;
@@ -5408,7 +5450,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
                mine_transaction(&nodes[2], &ds_prev_commitment_tx[0]);
        }
        connect_blocks(&nodes[2], ANTI_REORG_DELAY - 1);
-       check_closed_broadcast!(nodes[2], false);
+       check_closed_broadcast!(nodes[2], true);
        expect_pending_htlcs_forwardable!(nodes[2]);
        check_added_monitors!(nodes[2], 3);
 
@@ -5545,7 +5587,7 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 
        // Timeout HTLC on A's chain and so it can generate a HTLC-Timeout tx
        mine_transaction(&nodes[0], &local_txn[0]);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let htlc_timeout = {
@@ -5613,7 +5655,7 @@ fn test_key_derivation_params() {
 
        // Timeout HTLC on A's chain and so it can generate a HTLC-Timeout tx
        mine_transaction(&nodes[0], &local_txn_1[0]);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let htlc_timeout = {
@@ -5705,7 +5747,7 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) {
                block.header.prev_blockhash = block.block_hash();
        }
        test_txn_broadcast(&nodes[1], &chan, None, if use_dust { HTLCType::NONE } else { HTLCType::SUCCESS });
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 }
 
@@ -5737,7 +5779,7 @@ fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) {
                header.prev_blockhash = header.block_hash();
        }
        test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 }
 
@@ -5785,7 +5827,7 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no
        }
        if !check_revoke_no_close {
                test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
-               check_closed_broadcast!(nodes[0], false);
+               check_closed_broadcast!(nodes[0], true);
                check_added_monitors!(nodes[0], 1);
        } else {
                expect_payment_failed!(nodes[0], our_payment_hash, true);
@@ -6967,7 +7009,7 @@ fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) {
                mine_transaction(&nodes[0], &as_prev_commitment_tx[0]);
        }
 
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
@@ -7029,7 +7071,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) {
        if local {
                // We fail dust-HTLC 1 by broadcast of local commitment tx
                mine_transaction(&nodes[0], &as_commitment_tx[0]);
-               check_closed_broadcast!(nodes[0], false);
+               check_closed_broadcast!(nodes[0], true);
                check_added_monitors!(nodes[0], 1);
                assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
                timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].clone());
@@ -7044,7 +7086,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) {
        } else {
                // We fail dust-HTLC 1 by broadcast of remote commitment tx. If revoked, fail also non-dust HTLC
                mine_transaction(&nodes[0], &bs_commitment_tx[0]);
-               check_closed_broadcast!(nodes[0], false);
+               check_closed_broadcast!(nodes[0], true);
                check_added_monitors!(nodes[0], 1);
                assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
                timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].clone());
@@ -7719,7 +7761,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
        let header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[1].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        // B will generate both revoked HTLC-timeout/HTLC-preimage txn from revoked commitment tx
        connect_block(&nodes[1], &Block { header, txdata: vec![revoked_local_txn[0].clone()] });
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
 
        let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -7851,7 +7893,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
                assert_eq!(node_txn.len(), 0);
                node_txn.clear();
        }
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 }
 
@@ -8025,7 +8067,7 @@ fn test_bump_txn_sanitize_tracking_maps() {
        assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 0);
 
        mine_transaction(&nodes[0], &revoked_local_txn[0]);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
        let penalty_txn = {
                let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -8388,7 +8430,7 @@ fn test_htlc_no_detection() {
        // We deliberately connect the local tx twice as this should provoke a failure calling
        // this test before #653 fix.
        chain::Listen::block_connected(&nodes[0].chain_monitor.chain_monitor, &Block { header, txdata: vec![local_txn[0].clone()] }, nodes[0].best_block_info().1 + 1);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
 
        let htlc_timeout = {
@@ -8446,7 +8488,7 @@ fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain
        let mut force_closing_node = 0; // Alice force-closes
        if !broadcast_alice { force_closing_node = 1; } // Bob force-closes
        nodes[force_closing_node].node.force_close_channel(&chan_ab.2).unwrap();
-       check_closed_broadcast!(nodes[force_closing_node], false);
+       check_closed_broadcast!(nodes[force_closing_node], true);
        check_added_monitors!(nodes[force_closing_node], 1);
        if go_onchain_before_fulfill {
                let txn_to_broadcast = match broadcast_alice {
@@ -8457,7 +8499,7 @@ fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain
                connect_block(&nodes[1], &Block { header, txdata: vec![txn_to_broadcast[0].clone()]});
                let mut bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
                if broadcast_alice {
-                       check_closed_broadcast!(nodes[1], false);
+                       check_closed_broadcast!(nodes[1], true);
                        check_added_monitors!(nodes[1], 1);
                }
                assert_eq!(bob_txn.len(), 1);
@@ -8536,7 +8578,7 @@ fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain
                connect_block(&nodes[1], &Block { header, txdata: vec![txn_to_broadcast[0].clone()]});
                // If Bob was the one to force-close, he will have already passed these checks earlier.
                if broadcast_alice {
-                       check_closed_broadcast!(nodes[1], false);
+                       check_closed_broadcast!(nodes[1], true);
                        check_added_monitors!(nodes[1], 1);
                }
                let mut bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
index 1afcb3530fe16b995a4671c563da98d8e8e8d115..3827cea84e0a3d983c8946bf9703fa5f9ff6826c 100644 (file)
 //! you want to learn things about the network topology (eg get a route for sending a payment),
 //! call into your NetGraphMsgHandler.
 
+#[cfg(any(test, feature = "_test_utils"))]
+#[macro_use]
+pub mod functional_test_utils;
+
 pub mod channelmanager;
 pub mod msgs;
 pub mod peer_handler;
@@ -38,9 +42,6 @@ mod wire;
 // without the node parameter being mut. This is incorrect, and thus newer rustcs will complain
 // about an unnecessary mut. Thus, we silence the unused_mut warning in two test modules below.
 
-#[cfg(any(test, feature = "_test_utils"))]
-#[macro_use]
-pub mod functional_test_utils;
 #[cfg(test)]
 #[allow(unused_mut)]
 mod functional_tests;
index 46400641bc178ba21d6e41366f62fe0cbaaef4f9..c9573a67c13244cc73e1298ca43a9e1d66537e33 100644 (file)
@@ -76,7 +76,7 @@ fn do_test_onchain_htlc_reorg(local_commitment: bool, claim: bool) {
                // Give node 2 node 1's transactions and get its response (claiming the HTLC instead).
                connect_block(&nodes[2], &Block { header, txdata: node_1_commitment_txn.clone() });
                check_added_monitors!(nodes[2], 1);
-               check_closed_broadcast!(nodes[2], false); // We should get a BroadcastChannelUpdate (and *only* a BroadcstChannelUpdate)
+               check_closed_broadcast!(nodes[2], true); // We should get a BroadcastChannelUpdate (and *only* a BroadcstChannelUpdate)
                let node_2_commitment_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap();
                assert_eq!(node_2_commitment_txn.len(), 3); // ChannelMonitor: 1 offered HTLC-Claim, ChannelManger: 1 local commitment tx, 1 Received HTLC-Claim
                assert_eq!(node_2_commitment_txn[1].output.len(), 2); // to-remote and Received HTLC (to-self is dust)
@@ -116,7 +116,7 @@ fn do_test_onchain_htlc_reorg(local_commitment: bool, claim: bool) {
                node_2_commitment_txn
        };
        check_added_monitors!(nodes[1], 1);
-       check_closed_broadcast!(nodes[1], false); // We should get a BroadcastChannelUpdate (and *only* a BroadcstChannelUpdate)
+       check_closed_broadcast!(nodes[1], true); // We should get a BroadcastChannelUpdate (and *only* a BroadcstChannelUpdate)
        // Connect ANTI_REORG_DELAY - 2 blocks, giving us a confirmation count of ANTI_REORG_DELAY - 1.
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 2);
        check_added_monitors!(nodes[1], 0);
@@ -184,7 +184,7 @@ fn test_onchain_htlc_timeout_delay_remote_commitment() {
        do_test_onchain_htlc_reorg(false, false);
 }
 
-fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool) {
+fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, connect_style: ConnectStyle) {
        // After creating a chan between nodes, we disconnect all blocks previously seen to force a
        // channel close on nodes[0] side. We also use this to provide very basic testing of logic
        // around freeing background events which store monitor updates during block_[dis]connected.
@@ -195,6 +195,8 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool) {
        let new_chain_monitor: test_utils::TestChainMonitor;
        let nodes_0_deserialized: ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+       *nodes[0].connect_style.borrow_mut() = connect_style;
+
        let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;
 
        let channel_state = nodes[0].node.channel_state.lock().unwrap();
@@ -204,7 +206,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool) {
 
        if !reorg_after_reload {
                disconnect_all_blocks(&nodes[0]);
-               check_closed_broadcast!(nodes[0], false);
+               check_closed_broadcast!(nodes[0], true);
                {
                        let channel_state = nodes[0].node.channel_state.lock().unwrap();
                        assert_eq!(channel_state.by_id.len(), 0);
@@ -256,7 +258,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool) {
 
        if reorg_after_reload {
                disconnect_all_blocks(&nodes[0]);
-               check_closed_broadcast!(nodes[0], false);
+               check_closed_broadcast!(nodes[0], true);
                {
                        let channel_state = nodes[0].node.channel_state.lock().unwrap();
                        assert_eq!(channel_state.by_id.len(), 0);
@@ -272,10 +274,18 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool) {
 
 #[test]
 fn test_unconf_chan() {
-       do_test_unconf_chan(true, true);
-       do_test_unconf_chan(false, true);
-       do_test_unconf_chan(true, false);
-       do_test_unconf_chan(false, false);
+       do_test_unconf_chan(true, true, ConnectStyle::BestBlockFirstSkippingBlocks);
+       do_test_unconf_chan(false, true, ConnectStyle::BestBlockFirstSkippingBlocks);
+       do_test_unconf_chan(true, false, ConnectStyle::BestBlockFirstSkippingBlocks);
+       do_test_unconf_chan(false, false, ConnectStyle::BestBlockFirstSkippingBlocks);
+}
+
+#[test]
+fn test_unconf_chan_via_listen() {
+       do_test_unconf_chan(true, true, ConnectStyle::FullBlockViaListen);
+       do_test_unconf_chan(false, true, ConnectStyle::FullBlockViaListen);
+       do_test_unconf_chan(true, false, ConnectStyle::FullBlockViaListen);
+       do_test_unconf_chan(false, false, ConnectStyle::FullBlockViaListen);
 }
 
 #[test]
@@ -311,7 +321,7 @@ fn test_set_outpoints_partial_claiming() {
 
        // Connect blocks on node A commitment transaction
        mine_transaction(&nodes[0], &remote_txn[0]);
-       check_closed_broadcast!(nodes[0], false);
+       check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
        // Verify node A broadcast tx claiming both HTLCs
        {
@@ -328,7 +338,7 @@ fn test_set_outpoints_partial_claiming() {
 
        // Connect blocks on node B
        connect_blocks(&nodes[1], 135);
-       check_closed_broadcast!(nodes[1], false);
+       check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
        // Verify node B broadcast 2 HTLC-timeout txn
        let partial_claim_tx = {
index a0ccf4f816ea991e31a019351018d43a95633324..153f2f28eed11ffe7e55536c98c191714edb6443 100644 (file)
@@ -8,6 +8,7 @@
 // licenses.
 
 use chain;
+use chain::WatchedOutput;
 use chain::chaininterface;
 use chain::chaininterface::ConfirmationTarget;
 use chain::chainmonitor;
@@ -38,7 +39,7 @@ use std::time::Duration;
 use std::sync::{Mutex, Arc};
 use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::{cmp, mem};
-use std::collections::{HashMap, HashSet};
+use std::collections::{HashMap, HashSet, VecDeque};
 use chain::keysinterface::InMemorySigner;
 
 pub struct TestVecWriter(pub Vec<u8>);
@@ -185,12 +186,12 @@ impl TestPersister {
                *self.update_ret.lock().unwrap() = ret;
        }
 }
-impl channelmonitor::Persist<EnforcingSigner> for TestPersister {
-       fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
+impl<Signer: keysinterface::Sign> channelmonitor::Persist<Signer> for TestPersister {
+       fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<Signer>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
                self.update_ret.lock().unwrap().clone()
        }
 
-       fn update_persisted_channel(&self, _funding_txo: OutPoint, _update: &channelmonitor::ChannelMonitorUpdate, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
+       fn update_persisted_channel(&self, _funding_txo: OutPoint, _update: &channelmonitor::ChannelMonitorUpdate, _data: &channelmonitor::ChannelMonitor<Signer>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
                self.update_ret.lock().unwrap().clone()
        }
 }
@@ -517,6 +518,7 @@ pub struct TestChainSource {
        pub utxo_ret: Mutex<Result<TxOut, chain::AccessError>>,
        pub watched_txn: Mutex<HashSet<(Txid, Script)>>,
        pub watched_outputs: Mutex<HashSet<(OutPoint, Script)>>,
+       expectations: Mutex<Option<VecDeque<OnRegisterOutput>>>,
 }
 
 impl TestChainSource {
@@ -527,8 +529,17 @@ impl TestChainSource {
                        utxo_ret: Mutex::new(Ok(TxOut { value: u64::max_value(), script_pubkey })),
                        watched_txn: Mutex::new(HashSet::new()),
                        watched_outputs: Mutex::new(HashSet::new()),
+                       expectations: Mutex::new(None),
                }
        }
+
+       /// Sets an expectation that [`chain::Filter::register_output`] is called.
+       pub fn expect(&self, expectation: OnRegisterOutput) -> &Self {
+               self.expectations.lock().unwrap()
+                       .get_or_insert_with(|| VecDeque::new())
+                       .push_back(expectation);
+               self
+       }
 }
 
 impl chain::Access for TestChainSource {
@@ -546,7 +557,72 @@ impl chain::Filter for TestChainSource {
                self.watched_txn.lock().unwrap().insert((*txid, script_pubkey.clone()));
        }
 
-       fn register_output(&self, outpoint: &OutPoint, script_pubkey: &Script) {
-               self.watched_outputs.lock().unwrap().insert((*outpoint, script_pubkey.clone()));
+       fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)> {
+               let dependent_tx = match &mut *self.expectations.lock().unwrap() {
+                       None => None,
+                       Some(expectations) => match expectations.pop_front() {
+                               None => {
+                                       panic!("Unexpected register_output: {:?}",
+                                               (output.outpoint, output.script_pubkey));
+                               },
+                               Some(expectation) => {
+                                       assert_eq!(output.outpoint, expectation.outpoint());
+                                       assert_eq!(&output.script_pubkey, expectation.script_pubkey());
+                                       expectation.returns
+                               },
+                       },
+               };
+
+               self.watched_outputs.lock().unwrap().insert((output.outpoint, output.script_pubkey));
+               dependent_tx
+       }
+}
+
+impl Drop for TestChainSource {
+       fn drop(&mut self) {
+               if std::thread::panicking() {
+                       return;
+               }
+
+               if let Some(expectations) = &*self.expectations.lock().unwrap() {
+                       if !expectations.is_empty() {
+                               panic!("Unsatisfied expectations: {:?}", expectations);
+                       }
+               }
+       }
+}
+
+/// An expectation that [`chain::Filter::register_output`] was called with a transaction output and
+/// returns an optional dependent transaction that spends the output in the same block.
+pub struct OnRegisterOutput {
+       /// The transaction output to register.
+       pub with: TxOutReference,
+
+       /// A dependent transaction spending the output along with its position in the block.
+       pub returns: Option<(usize, Transaction)>,
+}
+
+/// A transaction output as identified by an index into a transaction's output list.
+pub struct TxOutReference(pub Transaction, pub usize);
+
+impl OnRegisterOutput {
+       fn outpoint(&self) -> OutPoint {
+               let txid = self.with.0.txid();
+               let index = self.with.1 as u16;
+               OutPoint { txid, index }
+       }
+
+       fn script_pubkey(&self) -> &Script {
+               let index = self.with.1;
+               &self.with.0.output[index].script_pubkey
+       }
+}
+
+impl std::fmt::Debug for OnRegisterOutput {
+       fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+               f.debug_struct("OnRegisterOutput")
+                       .field("outpoint", &self.outpoint())
+                       .field("script_pubkey", self.script_pubkey())
+                       .finish()
        }
 }