From 0f5580afd42c6604e8035738b20c28a9628eae72 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 25 Nov 2020 15:03:19 -0500 Subject: [PATCH] Use Writeable for ChannelMonitor instead of a specific function. There's no reason to have ChannelMonitor::write_for_disk instead of just using the Writeable trait anymore. Previously, it was used to differentiate with `write_for_watchtower`, but support for watchtower-mode ChannelMonitors was never completed and the partial bits were removed long ago. This has the nice benefit of hitting the custom Writeable codepaths in C bindings instead of trying to hit trait-generics paths. --- fuzz/src/chanmon_consistency.rs | 4 ++-- fuzz/src/chanmon_deser.rs | 4 ++-- lightning-persister/src/lib.rs | 2 +- lightning/src/chain/channelmonitor.rs | 17 ++++++++--------- lightning/src/ln/chanmon_update_fail_tests.rs | 4 ++-- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/functional_tests.rs | 18 +++++++++--------- lightning/src/util/test_utils.rs | 4 ++-- 8 files changed, 27 insertions(+), 28 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 377ac8d7..732f0f5b 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -112,7 +112,7 @@ impl chain::Watch for TestChainMonitor { fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> { let mut ser = VecWriter(Vec::new()); - monitor.serialize_for_disk(&mut ser).unwrap(); + monitor.write(&mut ser).unwrap(); if let Some(_) = self.latest_monitors.lock().unwrap().insert(funding_txo, (monitor.get_latest_update_id(), ser.0)) { panic!("Already had monitor pre-watch_channel"); } @@ -131,7 +131,7 @@ impl chain::Watch for TestChainMonitor { read(&mut Cursor::new(&map_entry.get().1)).unwrap().1; deserialized_monitor.update_monitor(&update, &&TestBroadcaster{}, &&FuzzEstimator{}, &self.logger).unwrap(); let mut ser = VecWriter(Vec::new()); - deserialized_monitor.serialize_for_disk(&mut ser).unwrap(); + deserialized_monitor.write(&mut ser).unwrap(); map_entry.insert((update.update_id, ser.0)); self.should_update_manager.store(true, atomic::Ordering::Relaxed); self.update_ret.lock().unwrap().clone() diff --git a/fuzz/src/chanmon_deser.rs b/fuzz/src/chanmon_deser.rs index fd326cc2..75a4044b 100644 --- a/fuzz/src/chanmon_deser.rs +++ b/fuzz/src/chanmon_deser.rs @@ -5,7 +5,7 @@ use bitcoin::hash_types::BlockHash; use lightning::chain::channelmonitor; use lightning::util::enforcing_trait_impls::EnforcingChannelKeys; -use lightning::util::ser::{Readable, Writer}; +use lightning::util::ser::{Readable, Writer, Writeable}; use utils::test_logger; @@ -26,7 +26,7 @@ impl Writer for VecWriter { pub fn do_test(data: &[u8], _out: Out) { if let Ok((latest_block_hash, monitor)) = <(BlockHash, channelmonitor::ChannelMonitor)>::read(&mut Cursor::new(data)) { let mut w = VecWriter(Vec::new()); - monitor.serialize_for_disk(&mut w).unwrap(); + monitor.write(&mut w).unwrap(); let deserialized_copy = <(BlockHash, channelmonitor::ChannelMonitor)>::read(&mut Cursor::new(&w.0)).unwrap(); assert!(latest_block_hash == deserialized_copy.0); assert!(monitor == deserialized_copy.1); diff --git a/lightning-persister/src/lib.rs b/lightning-persister/src/lib.rs index ec08e948..ff8eeeb1 100644 --- a/lightning-persister/src/lib.rs +++ b/lightning-persister/src/lib.rs @@ -45,7 +45,7 @@ trait DiskWriteable { impl DiskWriteable for ChannelMonitor { fn write(&self, writer: &mut fs::File) -> Result<(), Error> { - self.serialize_for_disk(writer) + Writeable::write(self, writer) } } diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 548c8f06..52783761 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -617,6 +617,12 @@ impl Readable for ChannelMonitorUpdateStep { /// get_and_clear_pending_monitor_events or get_and_clear_pending_events are serialized to disk and /// reloaded at deserialize-time. Thus, you must ensure that, when handling events, all events /// gotten are fully handled before re-serializing the new state. +/// +/// 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 disconnecting blocks until you find a common ancestor from both the +/// returned block hash and the the current chain and then reconnecting blocks to get to the +/// best chain) upon deserializing the object! pub struct ChannelMonitor { latest_update_id: u64, commitment_transaction_number_obscure_factor: u64, @@ -755,15 +761,8 @@ impl PartialEq for ChannelMonitor { } } -impl ChannelMonitor { - /// 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 disconnecting blocks until you find a common ancestor from both the - /// returned block hash and the the current chain and then reconnecting blocks to get to the - /// best chain) upon deserializing the object! - pub fn serialize_for_disk(&self, writer: &mut W) -> Result<(), Error> { +impl Writeable for ChannelMonitor { + fn write(&self, writer: &mut W) -> Result<(), Error> { //TODO: We still write out all the serialization here manually instead of using the fancy //serialization framework we have, we should migrate things over to it. writer.write_all(&[SERIALIZATION_VERSION; 1])?; diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 689c3496..1d4655bb 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -26,7 +26,7 @@ use routing::router::get_route; use util::enforcing_trait_impls::EnforcingChannelKeys; use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider}; use util::errors::APIError; -use util::ser::Readable; +use util::ser::{Readable, Writeable}; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; @@ -105,7 +105,7 @@ fn test_monitor_and_persister_update_fail() { let monitors = nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap(); let monitor = monitors.get(&outpoint).unwrap(); let mut w = test_utils::TestVecWriter(Vec::new()); - monitor.serialize_for_disk(&mut w).unwrap(); + monitor.write(&mut w).unwrap(); let new_monitor = <(BlockHash, ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0)).unwrap().1; assert!(new_monitor == *monitor); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index d54045f8..94c4474f 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -170,7 +170,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> { let old_monitors = self.chain_monitor.chain_monitor.monitors.lock().unwrap(); for (_, old_monitor) in old_monitors.iter() { let mut w = test_utils::TestVecWriter(Vec::new()); - old_monitor.serialize_for_disk(&mut w).unwrap(); + old_monitor.write(&mut w).unwrap(); let (_, deserialized_monitor) = <(BlockHash, ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0)).unwrap(); deserialized_monitors.push(deserialized_monitor); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 36709062..98b16628 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4279,7 +4279,7 @@ fn test_no_txn_manager_serialize_deserialize() { let nodes_0_serialized = nodes[0].node.encode(); let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new()); - nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.serialize_for_disk(&mut chan_0_monitor_serialized).unwrap(); + nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap(); logger = test_utils::TestLogger::new(); fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 }; @@ -4388,7 +4388,7 @@ fn test_manager_serialize_deserialize_events() { // Start the de/seriailization process mid-channel creation to check that the channel manager will hold onto events that are serialized let nodes_0_serialized = nodes[0].node.encode(); let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new()); - nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.serialize_for_disk(&mut chan_0_monitor_serialized).unwrap(); + nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap(); fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 }; logger = test_utils::TestLogger::new(); @@ -4480,7 +4480,7 @@ fn test_simple_manager_serialize_deserialize() { let nodes_0_serialized = nodes[0].node.encode(); let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new()); - nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.serialize_for_disk(&mut chan_0_monitor_serialized).unwrap(); + nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap(); logger = test_utils::TestLogger::new(); fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 }; @@ -4539,7 +4539,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { let mut node_0_stale_monitors_serialized = Vec::new(); for monitor in nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter() { let mut writer = test_utils::TestVecWriter(Vec::new()); - monitor.1.serialize_for_disk(&mut writer).unwrap(); + monitor.1.write(&mut writer).unwrap(); node_0_stale_monitors_serialized.push(writer.0); } @@ -4558,7 +4558,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { let mut node_0_monitors_serialized = Vec::new(); for monitor in nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter() { let mut writer = test_utils::TestVecWriter(Vec::new()); - monitor.1.serialize_for_disk(&mut writer).unwrap(); + monitor.1.write(&mut writer).unwrap(); node_0_monitors_serialized.push(writer.0); } @@ -7392,7 +7392,7 @@ fn test_data_loss_protect() { // Cache node A state before any channel update let previous_node_state = nodes[0].node.encode(); let mut previous_chain_monitor_state = test_utils::TestVecWriter(Vec::new()); - nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.serialize_for_disk(&mut previous_chain_monitor_state).unwrap(); + nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap().iter().next().unwrap().1.write(&mut previous_chain_monitor_state).unwrap(); send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000, 8_000_000); send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000, 8_000_000); @@ -8274,7 +8274,7 @@ fn test_update_err_monitor_lockdown() { let monitors = nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap(); let monitor = monitors.get(&outpoint).unwrap(); let mut w = test_utils::TestVecWriter(Vec::new()); - monitor.serialize_for_disk(&mut w).unwrap(); + monitor.write(&mut w).unwrap(); let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0)).unwrap().1; assert!(new_monitor == *monitor); @@ -8333,7 +8333,7 @@ fn test_concurrent_monitor_claim() { let monitors = nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap(); let monitor = monitors.get(&outpoint).unwrap(); let mut w = test_utils::TestVecWriter(Vec::new()); - monitor.serialize_for_disk(&mut w).unwrap(); + monitor.write(&mut w).unwrap(); let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0)).unwrap().1; assert!(new_monitor == *monitor); @@ -8359,7 +8359,7 @@ fn test_concurrent_monitor_claim() { let monitors = nodes[0].chain_monitor.chain_monitor.monitors.lock().unwrap(); let monitor = monitors.get(&outpoint).unwrap(); let mut w = test_utils::TestVecWriter(Vec::new()); - monitor.serialize_for_disk(&mut w).unwrap(); + monitor.write(&mut w).unwrap(); let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0)).unwrap().1; assert!(new_monitor == *monitor); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index c944e572..6e27e043 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -87,7 +87,7 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { // At every point where we get a monitor update, we should be able to send a useful monitor // to a watchtower and disk... let mut w = TestVecWriter(Vec::new()); - monitor.serialize_for_disk(&mut w).unwrap(); + monitor.write(&mut w).unwrap(); let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0)).unwrap().1; assert!(new_monitor == monitor); @@ -120,7 +120,7 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { let monitors = self.chain_monitor.monitors.lock().unwrap(); let monitor = monitors.get(&funding_txo).unwrap(); w.0.clear(); - monitor.serialize_for_disk(&mut w).unwrap(); + monitor.write(&mut w).unwrap(); let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>::read( &mut ::std::io::Cursor::new(&w.0)).unwrap().1; assert!(new_monitor == *monitor); -- 2.30.2