chaininterface+multi: add filter_block and reentered to ChainWatchInterface

Because filter_block takes a `&'a Block` and returns a list of `&'a Transaction`s , we must add a lifetime to the ChainWatchInterface, which bubbles up in a lot of places. These places include adding a lifetime  to the Node struct, which causes a lot of rearranging tests so that variables don't go out of scope before the Node that owns them does.

Author: valentinewallace

fuzz/fuzz_targets/chanmon_fail_consistency.rs

@@ -81,16 +81,16 @@ impl Writer for VecWriter {
 }
 
 static mut IN_RESTORE: bool = false;
-pub struct TestChannelMonitor {
-	pub simple_monitor: Arc<channelmonitor::SimpleManyChannelMonitor<OutPoint>>,
+pub struct TestChannelMonitor<'a> {
+	pub simple_monitor: Arc<channelmonitor::SimpleManyChannelMonitor<'a, OutPoint>>,
 	pub update_ret: Mutex<Result<(), channelmonitor::ChannelMonitorUpdateErr>>,
 	pub latest_good_update: Mutex<HashMap<OutPoint, Vec<u8>>>,
 	pub latest_update_good: Mutex<HashMap<OutPoint, bool>>,
 	pub latest_updates_good_at_last_ser: Mutex<HashMap<OutPoint, bool>>,
 	pub should_update_manager: atomic::AtomicBool,
 }
-impl TestChannelMonitor {
-	pub fn new(chain_monitor: Arc<chaininterface::ChainWatchInterface>, broadcaster: Arc<chaininterface::BroadcasterInterface>, logger: Arc<Logger>, feeest: Arc<chaininterface::FeeEstimator>) -> Self {
+impl<'a> TestChannelMonitor<'a> {
+	pub fn new(chain_monitor: Arc<chaininterface::ChainWatchInterface<'a>>, broadcaster: Arc<chaininterface::BroadcasterInterface>, logger: Arc<Logger>, feeest: Arc<chaininterface::FeeEstimator>) -> Self {
 		Self {
 			simple_monitor: channelmonitor::SimpleManyChannelMonitor::new(chain_monitor, broadcaster, logger, feeest),
 			update_ret: Mutex::new(Ok(())),
@@ -101,7 +101,7 @@ impl TestChannelMonitor {
 		}
 	}
 }
-impl channelmonitor::ManyChannelMonitor for TestChannelMonitor {
+impl<'a> channelmonitor::ManyChannelMonitor for TestChannelMonitor<'a> {
 	fn add_update_monitor(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
 		let ret = self.update_ret.lock().unwrap().clone();
 		if let Ok(()) = ret {
@@ -198,55 +198,6 @@ pub fn do_test(data: &[u8]) {
 		} }
 	}
 
-	macro_rules! reload_node {
-		($ser: expr, $node_id: expr, $old_monitors: expr) => { {
-			let logger: Arc<Logger> = Arc::new(test_logger::TestLogger::new($node_id.to_string()));
-			let watch = Arc::new(ChainWatchInterfaceUtil::new(Network::Bitcoin, Arc::clone(&logger)));
-			let monitor = Arc::new(TestChannelMonitor::new(watch.clone(), broadcast.clone(), logger.clone(), fee_est.clone()));
-
-			let keys_manager = Arc::new(KeyProvider { node_id: $node_id, session_id: atomic::AtomicU8::new(0), channel_id: atomic::AtomicU8::new(0) });
-			let mut config = UserConfig::new();
-			config.channel_options.fee_proportional_millionths = 0;
-			config.channel_options.announced_channel = true;
-			config.peer_channel_config_limits.min_dust_limit_satoshis = 0;
-
-			let mut monitors = HashMap::new();
-			let mut old_monitors = $old_monitors.latest_good_update.lock().unwrap();
-			for (outpoint, monitor_ser) in old_monitors.drain() {
-				monitors.insert(outpoint, <(Sha256d, ChannelMonitor)>::read(&mut Cursor::new(&monitor_ser), Arc::clone(&logger)).expect("Failed to read monitor").1);
-				monitor.latest_good_update.lock().unwrap().insert(outpoint, monitor_ser);
-			}
-			let mut monitor_refs = HashMap::new();
-			for (outpoint, monitor) in monitors.iter() {
-				monitor_refs.insert(*outpoint, monitor);
-			}
-
-			let read_args = ChannelManagerReadArgs {
-				keys_manager,
-				fee_estimator: fee_est.clone(),
-				monitor: monitor.clone(),
-				chain_monitor: watch,
-				tx_broadcaster: broadcast.clone(),
-				logger,
-				default_config: config,
-				channel_monitors: &monitor_refs,
-			};
-
-			let res = (<(Sha256d, ChannelManager)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, monitor);
-			for (_, was_good) in $old_monitors.latest_updates_good_at_last_ser.lock().unwrap().iter() {
-				if !was_good {
-					// If the last time we updated a monitor we didn't successfully update (and we
-					// have sense updated our serialized copy of the ChannelManager) we may
-					// force-close the channel on our counterparty cause we know we're missing
-					// something. Thus, we just return here since we can't continue to test.
-					return;
-				}
-			}
-			res
-		} }
-	}
-
-
 	let mut channel_txn = Vec::new();
 	macro_rules! make_channel {
 		($source: expr, $dest: expr, $chan_id: expr) => { {
@@ -356,11 +307,11 @@ pub fn do_test(data: &[u8]) {
 
 	// 3 nodes is enough to hit all the possible cases, notably unknown-source-unknown-dest
 	// forwarding.
-	let (mut node_a, mut monitor_a) = make_node!(0);
-	let (mut node_b, mut monitor_b) = make_node!(1);
-	let (mut node_c, mut monitor_c) = make_node!(2);
+	let (node_a, monitor_a) = make_node!(0);
+	let (node_b, monitor_b) = make_node!(1);
+	let (node_c, monitor_c) = make_node!(2);
 
-	let mut nodes = [node_a, node_b, node_c];
+	let nodes = [node_a, node_b, node_c];
 
 	make_channel!(nodes[0], nodes[1], 0);
 	make_channel!(nodes[1], nodes[2], 1);
@@ -688,10 +639,6 @@ pub fn do_test(data: &[u8]) {
 					chan_a_disconnected = true;
 					drain_msg_events_on_disconnect!(0);
 				}
-				let (new_node_a, new_monitor_a) = reload_node!(node_a_ser, 0, monitor_a);
-				node_a = Arc::new(new_node_a);
-				nodes[0] = node_a.clone();
-				monitor_a = new_monitor_a;
 			},
 			0x20 => {
 				if !chan_a_disconnected {
@@ -706,21 +653,13 @@ pub fn do_test(data: &[u8]) {
 					nodes[2].get_and_clear_pending_msg_events();
 					bc_events.clear();
 				}
-				let (new_node_b, new_monitor_b) = reload_node!(node_b_ser, 1, monitor_b);
-				node_b = Arc::new(new_node_b);
-				nodes[1] = node_b.clone();
-				monitor_b = new_monitor_b;
 			},
 			0x21 => {
 				if !chan_b_disconnected {
 					nodes[1].peer_disconnected(&nodes[2].get_our_node_id(), false);
 					chan_b_disconnected = true;
 					drain_msg_events_on_disconnect!(2);
 				}
-				let (new_node_c, new_monitor_c) = reload_node!(node_c_ser, 2, monitor_c);
-				node_c = Arc::new(new_node_c);
-				nodes[2] = node_c.clone();
-				monitor_c = new_monitor_c;
 			},
 			_ => test_return!(),
 		}

fuzz/fuzz_targets/full_stack_target.rs

@@ -144,9 +144,9 @@ impl<'a> Hash for Peer<'a> {
 	}
 }
 
-struct MoneyLossDetector<'a> {
-	manager: Arc<ChannelManager>,
-	monitor: Arc<channelmonitor::SimpleManyChannelMonitor<OutPoint>>,
+struct MoneyLossDetector<'a, 'b> {
+	manager: Arc<ChannelManager<'b>>,
+	monitor: Arc<channelmonitor::SimpleManyChannelMonitor<'b, OutPoint>>,
 	handler: PeerManager<Peer<'a>>,
 
 	peers: &'a RefCell<[bool; 256]>,
@@ -157,8 +157,8 @@ struct MoneyLossDetector<'a> {
 	max_height: usize,
 	blocks_connected: u32,
 }
-impl<'a> MoneyLossDetector<'a> {
-	pub fn new(peers: &'a RefCell<[bool; 256]>, manager: Arc<ChannelManager>, monitor: Arc<channelmonitor::SimpleManyChannelMonitor<OutPoint>>, handler: PeerManager<Peer<'a>>) -> Self {
+impl<'a, 'b> MoneyLossDetector<'a, 'b> {
+	pub fn new(peers: &'a RefCell<[bool; 256]>, manager: Arc<ChannelManager<'b>>, monitor: Arc<channelmonitor::SimpleManyChannelMonitor<'b, OutPoint>>, handler: PeerManager<Peer<'a>>) -> Self {
 		MoneyLossDetector {
 			manager,
 			monitor,
@@ -217,7 +217,7 @@ impl<'a> MoneyLossDetector<'a> {
 	}
 }
 
-impl<'a> Drop for MoneyLossDetector<'a> {
+impl<'a, 'b> Drop for MoneyLossDetector<'a, 'b> {
 	fn drop(&mut self) {
 		if !::std::thread::panicking() {
 			// Disconnect all peers

fuzz/fuzz_targets/router_target.rs

@@ -5,6 +5,8 @@ extern crate secp256k1;
 
 use bitcoin_hashes::sha256d::Hash as Sha256dHash;
 use bitcoin::blockdata::script::{Script, Builder};
+use bitcoin::blockdata::block::Block;
+use bitcoin::blockdata::transaction::Transaction;
 
 use lightning::chain::chaininterface::{ChainError,ChainWatchInterface, ChainListener};
 use lightning::ln::channelmanager::ChannelDetails;
@@ -75,11 +77,14 @@ struct DummyChainWatcher {
 	input: Arc<InputData>,
 }
 
-impl ChainWatchInterface for DummyChainWatcher {
+impl<'a> ChainWatchInterface<'a> for DummyChainWatcher {
 	fn install_watch_tx(&self, _txid: &Sha256dHash, _script_pub_key: &Script) { }
 	fn install_watch_outpoint(&self, _outpoint: (Sha256dHash, u32), _out_script: &Script) { }
 	fn watch_all_txn(&self) { }
-	fn register_listener(&self, _listener: Weak<ChainListener>) { }
+	fn filter_block(&self, _block: &'a Block) -> (Vec<&'a Transaction>, Vec<u32>) {
+		(Vec::new(), Vec::new())
+	}
+	fn reentered(&self) -> usize { 0 }
 
 	fn get_chain_utxo(&self, _genesis_hash: Sha256dHash, _unspent_tx_output_identifier: u64) -> Result<(Script, u64), ChainError> {
 		match self.input.get_slice(2) {

src/chain/chaininterface.rs

@@ -34,7 +34,7 @@ pub enum ChainError {
 /// Note that all of the functions implemented here *must* be reentrant-safe (obviously - they're
 /// called from inside the library in response to ChainListener events, P2P events, or timer
 /// events).
-pub trait ChainWatchInterface: Sync + Send {
+pub trait ChainWatchInterface<'a>: Sync + Send {
 	/// Provides a txid/random-scriptPubKey-in-the-tx which much be watched for.
 	fn install_watch_tx(&self, txid: &Sha256dHash, script_pub_key: &Script);
 
@@ -50,6 +50,15 @@ pub trait ChainWatchInterface: Sync + Send {
 	/// bytes are the block height, the next 3 the transaction index within the block, and the
 	/// final two the output within the transaction.
 	fn get_chain_utxo(&self, genesis_hash: Sha256dHash, unspent_tx_output_identifier: u64) -> Result<(Script, u64), ChainError>;
+
+	/// Gets the list of transactions and transaction indices that the ChainWatchInterface is
+	/// watching for.
+	fn filter_block(&self, block: &'a Block) -> (Vec<&'a Transaction>, Vec<u32>);
+
+	/// Returns a usize that changes when the ChainWatchInterface's watched data is modified.
+	/// Users of `filter_block` should pre-save a copy of `reentered`'s return value and use it to
+	/// determine whether they need to re-filter a given block.
+	fn reentered(&self) -> usize;
 }
 
 /// An interface to send a transaction to the Bitcoin network.
@@ -273,7 +282,7 @@ pub struct ChainWatchInterfaceUtil {
 }
 
 /// Register listener
-impl ChainWatchInterface for ChainWatchInterfaceUtil {
+impl<'a> ChainWatchInterface<'a> for ChainWatchInterfaceUtil {
 	fn install_watch_tx(&self, txid: &Sha256dHash, script_pub_key: &Script) {
 		let mut watched = self.watched.lock().unwrap();
 		if watched.register_tx(txid, script_pub_key) {
@@ -301,6 +310,25 @@ impl ChainWatchInterface for ChainWatchInterfaceUtil {
 		}
 		Err(ChainError::NotSupported)
 	}
+
+	fn filter_block(&self, block: &'a Block) -> (Vec<&'a Transaction>, Vec<u32>) {
+		let mut matched = Vec::new();
+		let mut matched_index = Vec::new();
+		{
+			let watched = self.watched.lock().unwrap();
+			for (index, transaction) in block.txdata.iter().enumerate() {
+				if self.does_match_tx_unguarded(transaction, &watched) {
+					matched.push(transaction);
+					matched_index.push(index as u32);
+				}
+			}
+		}
+		(matched, matched_index)
+	}
+
+	fn reentered(&self) -> usize {
+		self.reentered.load(Ordering::Relaxed)
+	}
 }
 
 impl ChainWatchInterfaceUtil {

src/ln/channelmanager.rs

@@ -318,11 +318,11 @@ const ERR: () = "You need at least 32 bit pointers (well, usize, but we'll assum
 /// the "reorg path" (ie call block_disconnected() until you get to a common block and then call
 /// block_connected() to step towards your best block) upon deserialization before using the
 /// object!
-pub struct ChannelManager {
+pub struct ChannelManager<'a> {
 	default_configuration: UserConfig,
 	genesis_hash: Sha256dHash,
 	fee_estimator: Arc<FeeEstimator>,
-	monitor: Arc<ManyChannelMonitor>,
+	monitor: Arc<ManyChannelMonitor + 'a>,
 	tx_broadcaster: Arc<BroadcasterInterface>,
 
 	#[cfg(test)]
@@ -575,7 +575,7 @@ macro_rules! maybe_break_monitor_err {
 	}
 }
 
-impl ChannelManager {
+impl<'a> ChannelManager<'a> {
 	/// Constructs a new ChannelManager to hold several channels and route between them.
 	///
 	/// This is the main "logic hub" for all channel-related actions, and implements
@@ -584,7 +584,7 @@ impl ChannelManager {
 	/// Non-proportional fees are fixed according to our risk using the provided fee estimator.
 	///
 	/// panics if channel_value_satoshis is >= `MAX_FUNDING_SATOSHIS`!
-	pub fn new(network: Network, feeest: Arc<FeeEstimator>, monitor: Arc<ManyChannelMonitor>, chain_monitor: Arc<ChainWatchInterface>, tx_broadcaster: Arc<BroadcasterInterface>, logger: Arc<Logger>,keys_manager: Arc<KeysInterface>, config: UserConfig) -> Result<Arc<ChannelManager>, secp256k1::Error> {
+	pub fn new(network: Network, feeest: Arc<FeeEstimator>, monitor: Arc<ManyChannelMonitor + 'a>, tx_broadcaster: Arc<BroadcasterInterface>, logger: Arc<Logger>,keys_manager: Arc<KeysInterface>, config: UserConfig) -> Result<Arc<ChannelManager<'a>>, secp256k1::Error> {
 		let secp_ctx = Secp256k1::new();
 
 		let res = Arc::new(ChannelManager {
@@ -2457,7 +2457,7 @@ impl ChannelManager {
 	}
 }
 
-impl events::MessageSendEventsProvider for ChannelManager {
+impl<'a> events::MessageSendEventsProvider for ChannelManager<'a> {
 	fn get_and_clear_pending_msg_events(&self) -> Vec<events::MessageSendEvent> {
 		// TODO: Event release to users and serialization is currently race-y: it's very easy for a
 		// user to serialize a ChannelManager with pending events in it and lose those events on
@@ -2482,7 +2482,7 @@ impl events::MessageSendEventsProvider for ChannelManager {
 	}
 }
 
-impl events::EventsProvider for ChannelManager {
+impl<'a> events::EventsProvider for ChannelManager<'a> {
 	fn get_and_clear_pending_events(&self) -> Vec<events::Event> {
 		// TODO: Event release to users and serialization is currently race-y: it's very easy for a
 		// user to serialize a ChannelManager with pending events in it and lose those events on
@@ -2507,7 +2507,7 @@ impl events::EventsProvider for ChannelManager {
 	}
 }
 
-impl ChainListener for ChannelManager {
+impl<'a> ChainListener for ChannelManager<'a> {
 	fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) {
 		let header_hash = header.bitcoin_hash();
 		log_trace!(self, "Block {} at height {} connected with {} txn matched", header_hash, height, txn_matched.len());
@@ -2621,7 +2621,7 @@ impl ChainListener for ChannelManager {
 	}
 }
 
-impl ChannelMessageHandler for ChannelManager {
+impl<'a> ChannelMessageHandler for ChannelManager<'a> {
 	//TODO: Handle errors and close channel (or so)
 	fn handle_open_channel(&self, their_node_id: &PublicKey, their_local_features: LocalFeatures, msg: &msgs::OpenChannel) -> Result<(), HandleError> {
 		let _ = self.total_consistency_lock.read().unwrap();
@@ -3006,7 +3006,7 @@ impl<R: ::std::io::Read> Readable<R> for HTLCForwardInfo {
 	}
 }
 
-impl Writeable for ChannelManager {
+impl<'a> Writeable for ChannelManager<'a> {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
 		let _ = self.total_consistency_lock.write().unwrap();
 
@@ -3084,7 +3084,7 @@ pub struct ChannelManagerReadArgs<'a> {
 	/// No calls to the ManyChannelMonitor will be made during deserialization. It is assumed that
 	/// you have deserialized ChannelMonitors separately and will add them to your
 	/// ManyChannelMonitor after deserializing this ChannelManager.
-	pub monitor: Arc<ManyChannelMonitor>,
+	pub monitor: Arc<ManyChannelMonitor + 'a>,
 
 	/// The BroadcasterInterface which will be used in the ChannelManager in the future and may be
 	/// used to broadcast the latest local commitment transactions of channels which must be
@@ -3110,7 +3110,7 @@ pub struct ChannelManagerReadArgs<'a> {
 	pub channel_monitors: &'a HashMap<OutPoint, &'a ChannelMonitor>,
 }
 
-impl<'a, R : ::std::io::Read> ReadableArgs<R, ChannelManagerReadArgs<'a>> for (Sha256dHash, ChannelManager) {
+impl<'a, R : ::std::io::Read> ReadableArgs<R, ChannelManagerReadArgs<'a>> for (Sha256dHash, ChannelManager<'a>) {
 	fn read(reader: &mut R, args: ChannelManagerReadArgs<'a>) -> Result<Self, DecodeError> {
 		let _ver: u8 = Readable::read(reader)?;
 		let min_ver: u8 = Readable::read(reader)?;