Track last_block_hash in ChannelMonitor and expose it on deser

Also make block_connected take a &mut self to ensure serialized
state will always be self-consistent.

Author: TheBlueMatt

fuzz/fuzz_targets/chanmon_deser_target.rs

@@ -1,8 +1,11 @@
 // This file is auto-generated by gen_target.sh based on msg_target_template.txt
 // To modify it, modify msg_target_template.txt and run gen_target.sh instead.
 
+extern crate bitcoin;
 extern crate lightning;
 
+use bitcoin::util::hash::Sha256dHash;
+
 use lightning::ln::channelmonitor;
 use lightning::util::reset_rng_state;
 use lightning::util::ser::{ReadableArgs, Writer};
@@ -28,10 +31,12 @@ impl Writer for VecWriter {
 pub fn do_test(data: &[u8]) {
 	reset_rng_state();
 	let logger = Arc::new(test_logger::TestLogger{});
-	if let Ok(monitor) = channelmonitor::ChannelMonitor::read(&mut Cursor::new(data), logger.clone()) {
+	if let Ok((latest_block_hash, monitor)) = <(Sha256dHash, channelmonitor::ChannelMonitor)>::read(&mut Cursor::new(data), logger.clone()) {
 		let mut w = VecWriter(Vec::new());
 		monitor.write_for_disk(&mut w).unwrap();
-		assert!(channelmonitor::ChannelMonitor::read(&mut Cursor::new(&w.0), logger.clone()).unwrap() == monitor);
+		let deserialized_copy = <(Sha256dHash, channelmonitor::ChannelMonitor)>::read(&mut Cursor::new(&w.0), logger.clone()).unwrap();
+		assert!(latest_block_hash == deserialized_copy.0);
+		assert!(monitor == deserialized_copy.1);
 		w.0.clear();
 		monitor.write_for_watchtower(&mut w).unwrap();
 	}

src/ln/channel.rs

@@ -2674,6 +2674,7 @@ impl Channel {
 		if self.funding_tx_confirmations > 0 {
 			if header.bitcoin_hash() != self.last_block_connected {
 				self.last_block_connected = header.bitcoin_hash();
+				self.channel_monitor.last_block_hash = self.last_block_connected;
 				self.funding_tx_confirmations += 1;
 				if self.funding_tx_confirmations == Channel::derive_minimum_depth(self.channel_value_satoshis*1000, self.value_to_self_msat) as u64 {
 					let need_commitment_update = if non_shutdown_state == ChannelState::FundingSent as u32 {

src/ln/channelmonitor.rs

@@ -16,6 +16,7 @@ use bitcoin::blockdata::transaction::{TxIn,TxOut,SigHashType,Transaction};
 use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
 use bitcoin::blockdata::script::Script;
 use bitcoin::network::serialize;
+use bitcoin::network::serialize::BitcoinHash;
 use bitcoin::network::encodable::{ConsensusDecodable, ConsensusEncodable};
 use bitcoin::util::hash::Sha256dHash;
 use bitcoin::util::bip143;
@@ -114,12 +115,13 @@ pub struct SimpleManyChannelMonitor<Key> {
 }
 
 impl<Key : Send + cmp::Eq + hash::Hash> ChainListener for SimpleManyChannelMonitor<Key> {
-	fn block_connected(&self, _header: &BlockHeader, height: u32, txn_matched: &[&Transaction], _indexes_of_txn_matched: &[u32]) {
+	fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], _indexes_of_txn_matched: &[u32]) {
+		let block_hash = header.bitcoin_hash();
 		let mut new_events: Vec<events::Event> = Vec::with_capacity(0);
 		{
-			let monitors = self.monitors.lock().unwrap();
-			for monitor in monitors.values() {
-				let (txn_outputs, spendable_outputs) = monitor.block_connected(txn_matched, height, &*self.broadcaster);
+			let mut monitors = self.monitors.lock().unwrap();
+			for monitor in monitors.values_mut() {
+				let (txn_outputs, spendable_outputs) = monitor.block_connected(txn_matched, height, &block_hash, &*self.broadcaster);
 				if spendable_outputs.len() > 0 {
 					new_events.push(events::Event::SpendableOutputs {
 						outputs: spendable_outputs,
@@ -279,6 +281,12 @@ pub struct ChannelMonitor {
 
 	destination_script: Script,
 
+	// We simply modify last_block_hash in Channel's block_connected so that serialization is
+	// consistent but hopefully the users' copy handles block_connected in a consistent way.
+	// (we do *not*, however, update them in insert_combine to ensure any local user copies keep
+	// their last_block_hash from its state and not based on updated copies that didn't run through
+	// the full block_connected).
+	pub(crate) last_block_hash: Sha256dHash,
 	secp_ctx: Secp256k1<secp256k1::All>, //TODO: dedup this a bit...
 	logger: Arc<Logger>,
 }
@@ -307,6 +315,7 @@ impl Clone for ChannelMonitor {
 			payment_preimages: self.payment_preimages.clone(),
 
 			destination_script: self.destination_script.clone(),
+			last_block_hash: self.last_block_hash.clone(),
 			secp_ctx: self.secp_ctx.clone(),
 			logger: self.logger.clone(),
 		}
@@ -378,6 +387,7 @@ impl ChannelMonitor {
 			payment_preimages: HashMap::new(),
 			destination_script: destination_script,
 
+			last_block_hash: Default::default(),
 			secp_ctx: Secp256k1::new(),
 			logger,
 		}
@@ -759,17 +769,30 @@ impl ChannelMonitor {
 			writer.write_all(payment_preimage)?;
 		}
 
+		self.last_block_hash.write(writer)?;
 		self.destination_script.write(writer)?;
 
 		Ok(())
 	}
 
 	/// Writes this monitor into the given writer, suitable for writing to disk.
+	///
+	/// Note that the deserializer is only implemented for (Sha256dHash, ChannelMonitor), which
+	/// tells you the last block hash which was block_connect()ed. You MUST rescan any blocks along
+	/// the "reorg path" (ie not just starting at the same height but starting at the highest
+	/// common block that appears on your best chain as well as on the chain which contains the
+	/// last block hash returned) upon deserializing the object!
 	pub fn write_for_disk<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
 		self.write(writer, true)
 	}
 
 	/// Encodes this monitor into the given writer, suitable for sending to a remote watchtower
+	///
+	/// Note that the deserializer is only implemented for (Sha256dHash, ChannelMonitor), which
+	/// tells you the last block hash which was block_connect()ed. You MUST rescan any blocks along
+	/// the "reorg path" (ie not just starting at the same height but starting at the highest
+	/// common block that appears on your best chain as well as on the chain which contains the
+	/// last block hash returned) upon deserializing the object!
 	pub fn write_for_watchtower<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
 		self.write(writer, false)
 	}
@@ -1283,7 +1306,7 @@ impl ChannelMonitor {
 		(Vec::new(), Vec::new())
 	}
 
-	fn block_connected(&self, txn_matched: &[&Transaction], height: u32, broadcaster: &BroadcasterInterface)-> (Vec<(Sha256dHash, Vec<TxOut>)>, Vec<SpendableOutputDescriptor>) {
+	fn block_connected(&mut self, txn_matched: &[&Transaction], height: u32, block_hash: &Sha256dHash, broadcaster: &BroadcasterInterface)-> (Vec<(Sha256dHash, Vec<TxOut>)>, Vec<SpendableOutputDescriptor>) {
 		let mut watch_outputs = Vec::new();
 		let mut spendable_outputs = Vec::new();
 		for tx in txn_matched {
@@ -1344,6 +1367,7 @@ impl ChannelMonitor {
 				}
 			}
 		}
+		self.last_block_hash = block_hash.clone();
 		(watch_outputs, spendable_outputs)
 	}
 
@@ -1382,7 +1406,7 @@ impl ChannelMonitor {
 
 const MAX_ALLOC_SIZE: usize = 64*1024;
 
-impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for ChannelMonitor {
+impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelMonitor) {
 	fn read(reader: &mut R, logger: Arc<Logger>) -> Result<Self, DecodeError> {
 		let secp_ctx = Secp256k1::new();
 		macro_rules! unwrap_obj {
@@ -1578,9 +1602,10 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for ChannelMonitor {
 			}
 		}
 
+		let last_block_hash: Sha256dHash = Readable::read(reader)?;
 		let destination_script = Readable::read(reader)?;
 
-		Ok(ChannelMonitor {
+		Ok((last_block_hash.clone(), ChannelMonitor {
 			funding_txo,
 			commitment_transaction_number_obscure_factor,
 
@@ -1603,9 +1628,10 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for ChannelMonitor {
 			payment_preimages,
 
 			destination_script,
+			last_block_hash,
 			secp_ctx,
 			logger,
-		})
+		}))
 	}
 
 }

src/util/test_utils.rs

@@ -9,6 +9,7 @@ use util::logger::{Logger, Level, Record};
 use util::ser::{ReadableArgs, Writer};
 
 use bitcoin::blockdata::transaction::Transaction;
+use bitcoin::util::hash::Sha256dHash;
 
 use secp256k1::PublicKey;
 
@@ -55,7 +56,8 @@ impl channelmonitor::ManyChannelMonitor for TestChannelMonitor {
 		// to a watchtower and disk...
 		let mut w = VecWriter(Vec::new());
 		monitor.write_for_disk(&mut w).unwrap();
-		assert!(channelmonitor::ChannelMonitor::read(&mut ::std::io::Cursor::new(&w.0), Arc::new(TestLogger::new())).unwrap() == monitor);
+		assert!(<(Sha256dHash, channelmonitor::ChannelMonitor)>::read(
+				&mut ::std::io::Cursor::new(&w.0), Arc::new(TestLogger::new())).unwrap().1 == monitor);
 		w.0.clear();
 		monitor.write_for_watchtower(&mut w).unwrap(); // This at least shouldn't crash...
 		self.added_monitors.lock().unwrap().push((funding_txo, monitor.clone()));