From 56513f2927b0c8d3b55c059ddcde86eb5132d508 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 24 Oct 2018 11:14:12 -0400 Subject: [PATCH] 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. --- fuzz/fuzz_targets/chanmon_deser_target.rs | 9 +++-- src/ln/channel.rs | 1 + src/ln/channelmonitor.rs | 42 ++++++++++++++++++----- src/util/test_utils.rs | 4 ++- 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/fuzz/fuzz_targets/chanmon_deser_target.rs b/fuzz/fuzz_targets/chanmon_deser_target.rs index 466a15c44..9ddf52c66 100644 --- a/fuzz/fuzz_targets/chanmon_deser_target.rs +++ b/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(); } diff --git a/src/ln/channel.rs b/src/ln/channel.rs index 9935f88cf..28b938ec5 100644 --- a/src/ln/channel.rs +++ b/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 { diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 3ba6ebc10..6eba2ff26 100644 --- a/src/ln/channelmonitor.rs +++ b/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 { } impl ChainListener for SimpleManyChannelMonitor { - 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 = 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, //TODO: dedup this a bit... logger: Arc, } @@ -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(&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(&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)>, Vec) { + fn block_connected(&mut self, txn_matched: &[&Transaction], height: u32, block_hash: &Sha256dHash, broadcaster: &BroadcasterInterface)-> (Vec<(Sha256dHash, Vec)>, Vec) { 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 ReadableArgs> for ChannelMonitor { +impl ReadableArgs> for (Sha256dHash, ChannelMonitor) { fn read(reader: &mut R, logger: Arc) -> Result { let secp_ctx = Secp256k1::new(); macro_rules! unwrap_obj { @@ -1578,9 +1602,10 @@ impl ReadableArgs> 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 ReadableArgs> for ChannelMonitor { payment_preimages, destination_script, + last_block_hash, secp_ctx, logger, - }) + })) } } diff --git a/src/util/test_utils.rs b/src/util/test_utils.rs index 3981750c8..0eb49702a 100644 --- a/src/util/test_utils.rs +++ b/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())); -- 2.39.5