From af5ed440c554ce4ebe130ea739afad7b97af59b8 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 16 Jun 2020 15:10:17 -0700 Subject: [PATCH] Remove ChainWatchInterface from BlockNotifier ChainListeners should be independent of each other, but in practice this is not the case because ChainWatchInterface introduces a dependency between them. Push ChainWatchInterface down into the ChainListener implementations where needed. Update ChainListener's block_connected method to take the entire block. --- fuzz/src/chanmon_consistency.rs | 22 +-- fuzz/src/full_stack.rs | 21 +- lightning/src/chain/chaininterface.rs | 69 ++----- lightning/src/ln/chanmon_update_fail_tests.rs | 10 +- lightning/src/ln/channel.rs | 24 +-- lightning/src/ln/channelmanager.rs | 17 +- lightning/src/ln/channelmonitor.rs | 29 +-- lightning/src/ln/functional_test_utils.rs | 48 +++-- lightning/src/ln/functional_tests.rs | 179 +++++++++++------- lightning/src/ln/reorg_tests.rs | 34 ++-- 10 files changed, 246 insertions(+), 207 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index eba0cb9e4..6d337a083 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -10,7 +10,7 @@ //! channel being force-closed. use bitcoin::BitcoinHash; -use bitcoin::blockdata::block::BlockHeader; +use bitcoin::blockdata::block::{Block, BlockHeader}; use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::blockdata::script::{Builder, Script}; use bitcoin::blockdata::opcodes; @@ -306,17 +306,17 @@ pub fn do_test(data: &[u8], out: Out) { macro_rules! confirm_txn { ($node: expr) => { { - let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - let mut txn = Vec::with_capacity(channel_txn.len()); - let mut posn = Vec::with_capacity(channel_txn.len()); - for i in 0..channel_txn.len() { - txn.push(&channel_txn[i]); - posn.push(i + 1); - } - $node.block_connected(&header, 1, &txn, &posn); + let mut block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: channel_txn.clone(), + }; + $node.block_connected(&block, 1); for i in 2..100 { - header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - $node.block_connected(&header, i, &Vec::new(), &[0; 0]); + block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: block.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; + $node.block_connected(&block, i); } } } } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 18e31fc94..614c6290c 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -4,7 +4,7 @@ //! or payments to send/ways to handle events generated. //! This test has been very useful, though due to its complexity good starting inputs are critical. -use bitcoin::blockdata::block::BlockHeader; +use bitcoin::blockdata::block::{Block, BlockHeader}; use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::blockdata::script::{Builder, Script}; use bitcoin::blockdata::opcodes; @@ -175,30 +175,29 @@ impl<'a> MoneyLossDetector<'a> { } fn connect_block(&mut self, all_txn: &[Transaction]) { - let mut txn = Vec::with_capacity(all_txn.len()); - let mut txn_idxs = Vec::with_capacity(all_txn.len()); - for (idx, tx) in all_txn.iter().enumerate() { + for tx in all_txn.iter() { let txid = tx.txid(); match self.txids_confirmed.entry(txid) { hash_map::Entry::Vacant(e) => { e.insert(self.height); - txn.push(tx); - txn_idxs.push(idx + 1); }, _ => {}, } } - let header = BlockHeader { version: 0x20000000, prev_blockhash: self.header_hashes[self.height], merkle_root: Default::default(), time: self.blocks_connected, bits: 42, nonce: 42 }; + let block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: self.header_hashes[self.height], merkle_root: Default::default(), time: self.blocks_connected, bits: 42, nonce: 42 }, + txdata: all_txn.to_vec(), + }; self.height += 1; self.blocks_connected += 1; - self.manager.block_connected(&header, self.height as u32, &txn[..], &txn_idxs[..]); - (*self.monitor).block_connected(&header, self.height as u32, &txn[..], &txn_idxs[..]); + self.manager.block_connected(&block, self.height as u32); + (*self.monitor).block_connected(&block, self.height as u32); if self.header_hashes.len() > self.height { - self.header_hashes[self.height] = header.bitcoin_hash(); + self.header_hashes[self.height] = block.bitcoin_hash(); } else { assert_eq!(self.header_hashes.len(), self.height); - self.header_hashes.push(header.bitcoin_hash()); + self.header_hashes.push(block.bitcoin_hash()); } self.max_height = cmp::max(self.height, self.max_height); } diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs index 2e874296f..4cf4c5fb7 100644 --- a/lightning/src/chain/chaininterface.rs +++ b/lightning/src/chain/chaininterface.rs @@ -72,21 +72,7 @@ pub trait BroadcasterInterface: Sync + Send { /// A trait indicating a desire to listen for events from the chain pub trait ChainListener: Sync + Send { /// Notifies a listener that a block was connected. - /// - /// The txn_matched array should be set to references to transactions which matched the - /// relevant installed watch outpoints/txn, or the full set of transactions in the block. - /// - /// Note that if txn_matched includes only matched transactions, and a new - /// transaction/outpoint is watched during a block_connected call, the block *must* be - /// re-scanned with the new transaction/outpoints and block_connected should be called - /// again with the same header and (at least) the new transactions. - /// - /// Note that if non-new transaction/outpoints are be registered during a call, a second call - /// *must not* happen. - /// - /// This also means those counting confirmations using block_connected callbacks should watch - /// for duplicate headers and not count them towards confirmations! - fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]); + fn block_connected(&self, block: &Block, height: u32); /// Notifies a listener that a block was disconnected. /// Unlike block_connected, this *must* never be called twice for the same disconnect event. /// Height must be the one of the block which was disconnected (not new height of the best chain) @@ -218,7 +204,7 @@ impl ChainWatchedUtil { /// parameters with static lifetimes). Other times you can afford a reference, which is more /// efficient, in which case BlockNotifierRef is a more appropriate type. Defining these type /// aliases prevents issues such as overly long function definitions. -pub type BlockNotifierArc = Arc, C>>; +pub type BlockNotifierArc = Arc>>; /// BlockNotifierRef is useful when you want a BlockNotifier that points to ChainListeners /// with nonstatic lifetimes. This is useful for when static lifetimes are not needed. Nonstatic @@ -227,27 +213,24 @@ pub type BlockNotifierArc = Arc, C> /// requires parameters with static lifetimes), in which case BlockNotifierArc is a more /// appropriate type. Defining these type aliases for common usages prevents issues such as /// overly long function definitions. -pub type BlockNotifierRef<'a, C> = BlockNotifier<'a, &'a ChainListener, C>; +pub type BlockNotifierRef<'a> = BlockNotifier<'a, &'a ChainListener>; -/// Utility for notifying listeners about new blocks, and handling block rescans if new watch -/// data is registered. +/// Utility for notifying listeners when blocks are connected or disconnected. /// /// Rather than using a plain BlockNotifier, it is preferable to use either a BlockNotifierArc /// or a BlockNotifierRef for conciseness. See their documentation for more details, but essentially /// you should default to using a BlockNotifierRef, and use a BlockNotifierArc instead when you /// require ChainListeners with static lifetimes, such as when you're using lightning-net-tokio. -pub struct BlockNotifier<'a, CL: Deref + 'a, C: Deref> where C::Target: ChainWatchInterface { +pub struct BlockNotifier<'a, CL: Deref + 'a> { listeners: Mutex>, - chain_monitor: C, phantom: PhantomData<&'a ()>, } -impl<'a, CL: Deref + 'a, C: Deref> BlockNotifier<'a, CL, C> where C::Target: ChainWatchInterface { +impl<'a, CL: Deref + 'a> BlockNotifier<'a, CL> { /// Constructs a new BlockNotifier without any listeners. - pub fn new(chain_monitor: C) -> BlockNotifier<'a, CL, C> { + pub fn new() -> BlockNotifier<'a, CL> { BlockNotifier { listeners: Mutex::new(Vec::new()), - chain_monitor, phantom: PhantomData, } } @@ -270,36 +253,12 @@ impl<'a, CL: Deref + 'a, C: Deref> BlockNotifier<'a vec.retain(|item | !ptr::eq(&(**item), &(*listener))); } - /// Notify listeners that a block was connected given a full, unfiltered block. - /// - /// Handles re-scanning the block and calling block_connected again if listeners register new - /// watch data during the callbacks for you (see ChainListener::block_connected for more info). - pub fn block_connected(&self, block: &Block, height: u32) { - let mut reentered = true; - while reentered { - let matched_indexes = self.chain_monitor.filter_block(block); - let mut matched_txn = Vec::new(); - for index in matched_indexes.iter() { - matched_txn.push(&block.txdata[*index]); - } - reentered = self.block_connected_checked(&block.header, height, matched_txn.as_slice(), matched_indexes.as_slice()); - } - } - - /// Notify listeners that a block was connected, given pre-filtered list of transactions in the - /// block which matched the filter (probably using does_match_tx). - /// - /// Returns true if notified listeners registered additional watch data (implying that the - /// block must be re-scanned and this function called again prior to further block_connected - /// calls, see ChainListener::block_connected for more info). - pub fn block_connected_checked(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]) -> bool { - let last_seen = self.chain_monitor.reentered(); - + /// Notify listeners that a block was connected. + pub fn block_connected<'b>(&self, block: &'b Block, height: u32) { let listeners = self.listeners.lock().unwrap(); for listener in listeners.iter() { - listener.block_connected(header, height, txn_matched, indexes_of_txn_matched); + listener.block_connected(block, height); } - return last_seen != self.chain_monitor.reentered(); } /// Notify listeners that a block was disconnected. @@ -410,7 +369,7 @@ mod tests { fn register_listener_test() { let chanmon_cfgs = create_chanmon_cfgs(1); let node_cfgs = create_node_cfgs(1, &chanmon_cfgs); - let block_notifier = BlockNotifier::new(node_cfgs[0].chain_monitor); + let block_notifier = BlockNotifier::new(); assert_eq!(block_notifier.listeners.lock().unwrap().len(), 0); let listener = &node_cfgs[0].chan_monitor.simple_monitor as &ChainListener; block_notifier.register_listener(listener); @@ -424,7 +383,7 @@ mod tests { fn unregister_single_listener_test() { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let block_notifier = BlockNotifier::new(node_cfgs[0].chain_monitor); + let block_notifier = BlockNotifier::new(); let listener1 = &node_cfgs[0].chan_monitor.simple_monitor as &ChainListener; let listener2 = &node_cfgs[1].chan_monitor.simple_monitor as &ChainListener; block_notifier.register_listener(listener1); @@ -443,7 +402,7 @@ mod tests { fn unregister_single_listener_ref_test() { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let block_notifier = BlockNotifier::new(node_cfgs[0].chain_monitor); + let block_notifier = BlockNotifier::new(); block_notifier.register_listener(&node_cfgs[0].chan_monitor.simple_monitor as &ChainListener); block_notifier.register_listener(&node_cfgs[1].chan_monitor.simple_monitor as &ChainListener); let vec = block_notifier.listeners.lock().unwrap(); @@ -460,7 +419,7 @@ mod tests { fn unregister_multiple_of_the_same_listeners_test() { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let block_notifier = BlockNotifier::new(node_cfgs[0].chain_monitor); + let block_notifier = BlockNotifier::new(); let listener1 = &node_cfgs[0].chan_monitor.simple_monitor as &ChainListener; let listener2 = &node_cfgs[1].chan_monitor.simple_monitor as &ChainListener; block_notifier.register_listener(listener1); diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 50e63834a..4df9179fc 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -1747,11 +1747,11 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: }; if confirm_a_first { - confirm_transaction(&nodes[0].block_notifier, &nodes[0].chain_monitor, &funding_tx, funding_tx.version); + confirm_transaction(&nodes[0].block_notifier, &funding_tx); nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id())); } else { assert!(!restore_b_before_conf); - confirm_transaction(&nodes[1].block_notifier, &nodes[1].chain_monitor, &funding_tx, funding_tx.version); + confirm_transaction(&nodes[1].block_notifier, &funding_tx); assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); } @@ -1763,7 +1763,7 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); if !restore_b_before_conf { - confirm_transaction(&nodes[1].block_notifier, &nodes[1].chain_monitor, &funding_tx, funding_tx.version); + confirm_transaction(&nodes[1].block_notifier, &funding_tx); assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } @@ -1776,12 +1776,12 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: let (channel_id, (announcement, as_update, bs_update)) = if !confirm_a_first { nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[0].node.get_our_node_id())); - confirm_transaction(&nodes[0].block_notifier, &nodes[0].chain_monitor, &funding_tx, funding_tx.version); + confirm_transaction(&nodes[0].block_notifier, &funding_tx); let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[1], &nodes[0]); (channel_id, create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked)) } else { if restore_b_before_conf { - confirm_transaction(&nodes[1].block_notifier, &nodes[1].chain_monitor, &funding_tx, funding_tx.version); + confirm_transaction(&nodes[1].block_notifier, &funding_tx); } let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[0], &nodes[1]); (channel_id, create_chan_between_nodes_with_value_b(&nodes[1], &nodes[0], &funding_locked)) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 719d03c6e..26fc8dcc0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1,4 +1,4 @@ -use bitcoin::blockdata::block::BlockHeader; +use bitcoin::blockdata::block::{Block, BlockHeader}; use bitcoin::blockdata::script::{Script,Builder}; use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType}; use bitcoin::blockdata::opcodes; @@ -3314,7 +3314,7 @@ impl Channel { /// /// 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, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> { + pub fn block_connected(&mut self, block: &Block, height: u32) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> { let mut timed_out_htlcs = Vec::new(); self.holding_cell_htlc_updates.retain(|htlc_update| { match htlc_update { @@ -3328,13 +3328,13 @@ impl Channel { } }); let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS); - if header.bitcoin_hash() != self.last_block_connected { + if block.bitcoin_hash() != self.last_block_connected { if self.funding_tx_confirmations > 0 { self.funding_tx_confirmations += 1; } } if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 { - for (ref tx, index_in_block) in txn_matched.iter().zip(indexes_of_txn_matched) { + for (index_in_block, ref tx) in block.txdata.iter().enumerate() { if tx.txid() == self.funding_txo.unwrap().txid { let txo_idx = self.funding_txo.unwrap().index as usize; if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() || @@ -3365,21 +3365,21 @@ impl Channel { } } } - if height > 0xff_ff_ff || (*index_in_block) > 0xff_ff_ff { + if height > 0xff_ff_ff || (index_in_block) > 0xff_ff_ff { panic!("Block was bogus - either height 16 million or had > 16 million transactions"); } assert!(txo_idx <= 0xffff); // txo_idx is a (u16 as usize), so this is just listed here for completeness self.funding_tx_confirmations = 1; - self.short_channel_id = Some(((height as u64) << (5*8)) | - ((*index_in_block as u64) << (2*8)) | - ((txo_idx as u64) << (0*8))); + self.short_channel_id = Some(((height as u64) << (5*8)) | + ((index_in_block as u64) << (2*8)) | + ((txo_idx as u64) << (0*8))); } } } } - if header.bitcoin_hash() != self.last_block_connected { - self.last_block_connected = header.bitcoin_hash(); - self.update_time_counter = cmp::max(self.update_time_counter, header.time); + if block.bitcoin_hash() != self.last_block_connected { + self.last_block_connected = block.bitcoin_hash(); + self.update_time_counter = cmp::max(self.update_time_counter, block.header.time); if let Some(channel_monitor) = self.channel_monitor.as_mut() { channel_monitor.last_block_hash = self.last_block_connected; } @@ -3403,7 +3403,7 @@ impl Channel { // funding_tx_confirmed_in and return. false }; - self.funding_tx_confirmed_in = Some(header.bitcoin_hash()); + self.funding_tx_confirmed_in = Some(block.bitcoin_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, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5466b3d23..c595e1023 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8,8 +8,7 @@ //! on-chain transactions (it only monitors the chain to watch for any force-closes that might //! imply it needs to fail HTLCs/payments/channels it manages). -use bitcoin::blockdata::block::BlockHeader; -use bitcoin::blockdata::transaction::Transaction; +use bitcoin::blockdata::block::{Block, BlockHeader}; use bitcoin::blockdata::constants::genesis_block; use bitcoin::network::constants::Network; use bitcoin::util::hash::BitcoinHash; @@ -2978,9 +2977,9 @@ impl= header.time as usize { break; } - if self.last_node_announcement_serial.compare_exchange(old_serial, header.time as usize, Ordering::AcqRel, Ordering::Relaxed).is_ok() { + if old_serial >= block.header.time as usize { break; } + if self.last_node_announcement_serial.compare_exchange(old_serial, block.header.time as usize, Ordering::AcqRel, Ordering::Relaxed).is_ok() { break; } } diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index ce37944ca..e2ac2b8eb 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -11,7 +11,7 @@ //! security-domain-separated system design, you should consider having multiple paths for //! ChannelMonitors to get out of the HSM and onto monitoring devices. -use bitcoin::blockdata::block::BlockHeader; +use bitcoin::blockdata::block::{Block, BlockHeader}; use bitcoin::blockdata::transaction::{TxOut,Transaction}; use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint; use bitcoin::blockdata::script::{Script, Builder}; @@ -183,19 +183,26 @@ impl = matched_indexes.iter().map(|index| &block.txdata[*index]).collect(); + let last_seen = self.chain_monitor.reentered(); + let block_hash = block.bitcoin_hash(); + { + let mut monitors = self.monitors.lock().unwrap(); + for monitor in monitors.values_mut() { + let txn_outputs = monitor.block_connected(&matched_txn, height, &block_hash, &*self.broadcaster, &*self.fee_estimator, &*self.logger); + + for (ref txid, ref outputs) in txn_outputs { + for (idx, output) in outputs.iter().enumerate() { + self.chain_monitor.install_watch_outpoint((txid.clone(), idx as u32), &output.script_pubkey); + } } } } + reentered = last_seen != self.chain_monitor.reentered(); } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 2c994c12c..07dfad00e 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -19,7 +19,7 @@ use util::config::UserConfig; use util::ser::{ReadableArgs, Writeable, Readable}; use bitcoin::util::hash::BitcoinHash; -use bitcoin::blockdata::block::BlockHeader; +use bitcoin::blockdata::block::{Block, BlockHeader}; use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::network::constants::Network; @@ -36,24 +36,38 @@ use std::mem; use std::collections::HashMap; pub const CHAN_CONFIRM_DEPTH: u32 = 100; -pub fn confirm_transaction<'a, 'b: 'a>(notifier: &'a chaininterface::BlockNotifierRef<'b, &chaininterface::ChainWatchInterfaceUtil>, chain: &chaininterface::ChainWatchInterfaceUtil, tx: &Transaction, chan_id: u32) { - assert!(chain.does_match_tx(tx)); - let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - notifier.block_connected_checked(&header, 1, &[tx; 1], &[chan_id as usize; 1]); +pub fn confirm_transaction<'a, 'b: 'a>(notifier: &'a chaininterface::BlockNotifierRef<'b>, tx: &Transaction) { + let dummy_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() }; + let dummy_tx_count = tx.version as usize; + let mut block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![dummy_tx; dummy_tx_count], + }; + block.txdata.push(tx.clone()); + notifier.block_connected(&block, 1); for i in 2..CHAN_CONFIRM_DEPTH { - header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - notifier.block_connected_checked(&header, i, &vec![], &[0; 0]); + block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: block.header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; + notifier.block_connected(&block, i); } } -pub fn connect_blocks<'a, 'b>(notifier: &'a chaininterface::BlockNotifierRef<'b, &chaininterface::ChainWatchInterfaceUtil>, depth: u32, height: u32, parent: bool, prev_blockhash: BlockHash) -> BlockHash { - let mut header = BlockHeader { version: 0x2000000, prev_blockhash: if parent { prev_blockhash } else { Default::default() }, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - notifier.block_connected_checked(&header, height + 1, &Vec::new(), &Vec::new()); +pub fn connect_blocks<'a, 'b>(notifier: &'a chaininterface::BlockNotifierRef<'b>, depth: u32, height: u32, parent: bool, prev_blockhash: BlockHash) -> BlockHash { + let mut block = Block { + header: BlockHeader { version: 0x2000000, prev_blockhash: if parent { prev_blockhash } else { Default::default() }, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; + notifier.block_connected(&block, height + 1); for i in 2..depth + 1 { - header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - notifier.block_connected_checked(&header, height + i, &Vec::new(), &Vec::new()); + block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: block.header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; + notifier.block_connected(&block, height + i); } - header.bitcoin_hash() + block.header.bitcoin_hash() } pub struct TestChanMonCfg { @@ -74,7 +88,7 @@ pub struct NodeCfg<'a> { } pub struct Node<'a, 'b: 'a, 'c: 'b> { - pub block_notifier: chaininterface::BlockNotifierRef<'a, &'c chaininterface::ChainWatchInterfaceUtil>, + pub block_notifier: chaininterface::BlockNotifierRef<'a>, pub chain_monitor: &'c chaininterface::ChainWatchInterfaceUtil, pub tx_broadcaster: &'c test_utils::TestBroadcaster, pub chan_monitor: &'b test_utils::TestChannelMonitor<'c>, @@ -365,7 +379,7 @@ pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, ' } pub fn create_chan_between_nodes_with_value_confirm_first<'a, 'b, 'c, 'd>(node_recv: &'a Node<'b, 'c, 'c>, node_conf: &'a Node<'b, 'c, 'd>, tx: &Transaction) { - confirm_transaction(&node_conf.block_notifier, &node_conf.chain_monitor, &tx, tx.version); + confirm_transaction(&node_conf.block_notifier, &tx); node_recv.node.handle_funding_locked(&node_conf.node.get_our_node_id(), &get_event_msg!(node_conf, MessageSendEvent::SendFundingLocked, node_recv.node.get_our_node_id())); } @@ -391,7 +405,7 @@ pub fn create_chan_between_nodes_with_value_confirm_second<'a, 'b, 'c>(node_recv pub fn create_chan_between_nodes_with_value_confirm<'a, 'b, 'c, 'd>(node_a: &'a Node<'b, 'c, 'd>, node_b: &'a Node<'b, 'c, 'd>, tx: &Transaction) -> ((msgs::FundingLocked, msgs::AnnouncementSignatures), [u8; 32]) { create_chan_between_nodes_with_value_confirm_first(node_a, node_b, tx); - confirm_transaction(&node_a.block_notifier, &node_a.chain_monitor, &tx, tx.version); + confirm_transaction(&node_a.block_notifier, &tx); create_chan_between_nodes_with_value_confirm_second(node_b, node_a) } @@ -1100,7 +1114,7 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec(_name: &str, test_case: { // reset block height - let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + let block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; for ix in 0..nodes.len() { - nodes[ix].block_notifier.block_connected_checked(&header, 1, &[], &[]); + nodes[ix].block_notifier.block_connected(&block, 1); } macro_rules! expect_event { @@ -6258,9 +6297,12 @@ fn test_onion_failure() { run_onion_failure_test("expiry_too_soon", 0, &nodes, &route, &payment_hash, |msg| { let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - LATENCY_GRACE_PERIOD_BLOCKS + 1; - let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + let block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; - nodes[1].block_notifier.block_connected_checked(&header, height, &[], &[]); + nodes[1].block_notifier.block_connected(&block, height); }, ||{}, true, Some(UPDATE|14), Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage{msg: ChannelUpdate::dummy()})); run_onion_failure_test("unknown_payment_hash", 2, &nodes, &route, &payment_hash, |_| {}, || { @@ -6269,9 +6311,12 @@ fn test_onion_failure() { run_onion_failure_test("final_expiry_too_soon", 1, &nodes, &route, &payment_hash, |msg| { let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - LATENCY_GRACE_PERIOD_BLOCKS + 1; - let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + let block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; - nodes[2].block_notifier.block_connected_checked(&header, height, &[], &[]); + nodes[2].block_notifier.block_connected(&block, height); }, || {}, true, Some(17), None); run_onion_failure_test("final_incorrect_cltv_expiry", 1, &nodes, &route, &payment_hash, |_| {}, || { @@ -7199,12 +7244,15 @@ fn test_no_failure_dust_htlc_local_commitment() { output: vec![outp] }; - let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - nodes[0].chan_monitor.simple_monitor.block_connected(&header, 1, &[&dummy_tx], &[1;1]); + let block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![dummy_tx], + }; + nodes[0].chan_monitor.simple_monitor.block_connected(&block, 1); assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); assert_eq!(nodes[0].node.get_and_clear_pending_msg_events().len(), 0); // We broadcast a few more block to check everything is all right - connect_blocks(&nodes[0].block_notifier, 20, 1, true, header.bitcoin_hash()); + connect_blocks(&nodes[0].block_notifier, 20, 1, true, block.bitcoin_hash()); assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); assert_eq!(nodes[0].node.get_and_clear_pending_msg_events().len(), 0); @@ -7512,7 +7560,7 @@ fn test_data_loss_protect() { nodes[0].chan_monitor = &monitor; nodes[0].chain_monitor = &chain_monitor; - nodes[0].block_notifier = BlockNotifier::new(&nodes[0].chain_monitor); + nodes[0].block_notifier = BlockNotifier::new(); nodes[0].block_notifier.register_listener(&nodes[0].chan_monitor.simple_monitor); nodes[0].block_notifier.register_listener(nodes[0].node); @@ -7841,13 +7889,13 @@ fn test_bump_penalty_txn_on_revoked_htlcs() { let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap(); assert_eq!(node_txn.len(), 5); // 3 penalty txn on revoked commitment tx + A commitment tx + 1 penalty tnx on revoked HTLC txn // Verify claim tx are spending revoked HTLC txn - assert_eq!(node_txn[4].input.len(), 2); - assert_eq!(node_txn[4].output.len(), 1); - check_spends!(node_txn[4], revoked_htlc_txn[0], revoked_htlc_txn[1]); - first = node_txn[4].txid(); + assert_eq!(node_txn[3].input.len(), 2); + assert_eq!(node_txn[3].output.len(), 1); + check_spends!(node_txn[3], revoked_htlc_txn[0], revoked_htlc_txn[1]); + first = node_txn[3].txid(); // Store both feerates for later comparison - let fee_1 = revoked_htlc_txn[0].output[0].value + revoked_htlc_txn[1].output[0].value - node_txn[4].output[0].value; - feerate_1 = fee_1 * 1000 / node_txn[4].get_weight() as u64; + let fee_1 = revoked_htlc_txn[0].output[0].value + revoked_htlc_txn[1].output[0].value - node_txn[3].output[0].value; + feerate_1 = fee_1 * 1000 / node_txn[3].get_weight() as u64; penalty_txn = vec![node_txn[0].clone(), node_txn[1].clone(), node_txn[2].clone()]; node_txn.clear(); } @@ -8329,8 +8377,11 @@ fn test_update_err_monitor_lockdown() { assert!(watchtower.add_monitor(outpoint, new_monitor).is_ok()); watchtower }; - let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - watchtower.simple_monitor.block_connected(&header, 200, &vec![], &vec![]); + let block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; + watchtower.simple_monitor.block_connected(&block, 200); // Try to update ChannelMonitor assert!(nodes[1].node.claim_funds(preimage, &None, 9_000_000)); diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index 49a55ade0..d54e170d2 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -43,8 +43,7 @@ fn do_test_onchain_htlc_reorg(local_commitment: bool, claim: bool) { check_added_monitors!(nodes[2], 1); get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id()); - let mut headers = Vec::new(); - let mut header = BlockHeader { version: 0x2000_0000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + let header = BlockHeader { version: 0x2000_0000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; let claim_txn = if local_commitment { // Broadcast node 1 commitment txn to broadcast the HTLC-Timeout let node_1_commitment_txn = get_local_commitment_txn!(nodes[1], chan_2.2); @@ -93,33 +92,44 @@ fn do_test_onchain_htlc_reorg(local_commitment: bool, claim: bool) { }; check_added_monitors!(nodes[1], 1); check_closed_broadcast!(nodes[1], false); // We should get a BroadcastChannelUpdate (and *only* a BroadcstChannelUpdate) - headers.push(header.clone()); + let mut block = Block { header, txdata: vec![] }; + let mut blocks = Vec::new(); + blocks.push(block.clone()); // At CHAN_CONFIRM_DEPTH + 1 we have a confirmation count of 1, so CHAN_CONFIRM_DEPTH + // ANTI_REORG_DELAY - 1 will give us a confirmation count of ANTI_REORG_DELAY - 1. for i in CHAN_CONFIRM_DEPTH + 2..CHAN_CONFIRM_DEPTH + ANTI_REORG_DELAY - 1 { - header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - nodes[1].block_notifier.block_connected_checked(&header, i, &vec![], &[0; 0]); - headers.push(header.clone()); + block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: block.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; + nodes[1].block_notifier.block_connected(&block, i); + blocks.push(block.clone()); } check_added_monitors!(nodes[1], 0); assert_eq!(nodes[1].node.get_and_clear_pending_events().len(), 0); if claim { // Now reorg back to CHAN_CONFIRM_DEPTH and confirm node 2's broadcasted transactions: - for (height, header) in (CHAN_CONFIRM_DEPTH + 1..CHAN_CONFIRM_DEPTH + ANTI_REORG_DELAY - 1).zip(headers.iter()).rev() { - nodes[1].block_notifier.block_disconnected(&header, height); + for (height, block) in (CHAN_CONFIRM_DEPTH + 1..CHAN_CONFIRM_DEPTH + ANTI_REORG_DELAY - 1).zip(blocks.iter()).rev() { + nodes[1].block_notifier.block_disconnected(&block.header, height); } - header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - nodes[1].block_notifier.block_connected(&Block { header, txdata: claim_txn }, CHAN_CONFIRM_DEPTH + 1); + block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: claim_txn, + }; + nodes[1].block_notifier.block_connected(&block, CHAN_CONFIRM_DEPTH + 1); // ChannelManager only polls ManyChannelMonitor::get_and_clear_pending_htlcs_updated when we // probe it for events, so we probe non-message events here (which should still end up empty): assert_eq!(nodes[1].node.get_and_clear_pending_events().len(), 0); } else { // Confirm the timeout tx and check that we fail the HTLC backwards - header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - nodes[1].block_notifier.block_connected_checked(&header, CHAN_CONFIRM_DEPTH + ANTI_REORG_DELAY, &vec![], &[0; 0]); + block = Block { + header: BlockHeader { version: 0x20000000, prev_blockhash: block.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }, + txdata: vec![], + }; + nodes[1].block_notifier.block_connected(&block, CHAN_CONFIRM_DEPTH + ANTI_REORG_DELAY); expect_pending_htlcs_forwardable!(nodes[1]); } -- 2.39.5