Migrate ChannelMonitor serialization to new ser framework(ish)

Sadly we can't straight up use the new serialization framework as
we have a few different serialization variants, but that's OK, it
looks identical and is just missing the Writeable impl

diff --git a/fuzz/fuzz_targets/chanmon_deser_target.rs b/fuzz/fuzz_targets/chanmon_deser_target.rs
index cce12d73f02d16d480919ed6318226e3a6b5224b..ba525081a712ef3e5b27fcdbad48efe8a53d4809 100644 (file)
--- a/fuzz/fuzz_targets/chanmon_deser_target.rs
+++ b/fuzz/fuzz_targets/chanmon_deser_target.rs
@@ -5,16 +5,30 @@ extern crate lightning;
 
 use lightning::ln::channelmonitor;
 use lightning::util::reset_rng_state;
-use lightning::util::ser::Readable;
+use lightning::util::ser::{Readable, Writer};
 
 use std::io::Cursor;
 
+struct VecWriter(Vec<u8>);
+impl Writer for VecWriter {
+       fn write_all(&mut self, buf: &[u8]) -> Result<(), ::std::io::Error> {
+               self.0.extend_from_slice(buf);
+               Ok(())
+       }
+       fn size_hint(&mut self, size: usize) {
+               self.0.reserve_exact(size);
+       }
+}
+
 #[inline]
 pub fn do_test(data: &[u8]) {
        reset_rng_state();
        if let Ok(monitor) = channelmonitor::ChannelMonitor::read(&mut Cursor::new(data)) {
-               assert!(channelmonitor::ChannelMonitor::read(&mut Cursor::new(&monitor.serialize_for_disk()[..])).unwrap() == monitor);
-               monitor.serialize_for_watchtower();
+               let mut w = VecWriter(Vec::new());
+               monitor.write_for_disk(&mut w).unwrap();
+               assert!(channelmonitor::ChannelMonitor::read(&mut Cursor::new(&w.0)).unwrap() == monitor);
+               w.0.clear();
+               monitor.write_for_watchtower(&mut w).unwrap();
        }
 }
 

diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs
index fdd2fe8e9c58289cf4d4ced9dcf8a9e368711e11..acd63da79e1bd63b511bfc7ec49ed29bb3dbcebb 100644 (file)
--- a/src/ln/channelmonitor.rs
+++ b/src/ln/channelmonitor.rs
@@ -28,7 +28,7 @@ use ln::chan_utils;
 use ln::chan_utils::HTLCOutputInCommitment;
 use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface};
 use chain::transaction::OutPoint;
-use util::ser::Readable;
+use util::ser::{Readable, Writer};
 use util::sha2::Sha256;
 use util::byte_utils;
 
@@ -531,81 +531,82 @@ impl ChannelMonitor {
        }
 
        /// Serializes into a vec, with various modes for the exposed pub fns
-       fn serialize(&self, for_local_storage: bool) -> Vec<u8> {
-               let mut res = Vec::new();
-               res.push(SERIALIZATION_VERSION);
-               res.push(MIN_SERIALIZATION_VERSION);
+       fn write<W: Writer>(&self, writer: &mut W, for_local_storage: bool) -> Result<(), ::std::io::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])?;
+               writer.write_all(&[MIN_SERIALIZATION_VERSION; 1])?;
 
                match &self.funding_txo {
                        &Some((ref outpoint, ref script)) => {
-                               res.extend_from_slice(&outpoint.txid[..]);
-                               res.extend_from_slice(&byte_utils::be16_to_array(outpoint.index));
-                               res.extend_from_slice(&byte_utils::be64_to_array(script.len() as u64));
-                               res.extend_from_slice(&script[..]);
+                               writer.write_all(&outpoint.txid[..])?;
+                               writer.write_all(&byte_utils::be16_to_array(outpoint.index))?;
+                               writer.write_all(&byte_utils::be64_to_array(script.len() as u64))?;
+                               writer.write_all(&script[..])?;
                        },
                        &None => {
                                // We haven't even been initialized...not sure why anyone is serializing us, but
                                // not much to give them.
-                               return res;
+                               return Ok(());
                        },
                }
 
                // Set in initial Channel-object creation, so should always be set by now:
-               res.extend_from_slice(&byte_utils::be48_to_array(self.commitment_transaction_number_obscure_factor));
+               writer.write_all(&byte_utils::be48_to_array(self.commitment_transaction_number_obscure_factor))?;
 
                match self.key_storage {
                        KeyStorage::PrivMode { ref revocation_base_key, ref htlc_base_key } => {
-                               res.push(0);
-                               res.extend_from_slice(&revocation_base_key[..]);
-                               res.extend_from_slice(&htlc_base_key[..]);
+                               writer.write_all(&[0; 1])?;
+                               writer.write_all(&revocation_base_key[..])?;
+                               writer.write_all(&htlc_base_key[..])?;
                        },
                        KeyStorage::SigsMode { .. } => unimplemented!(),
                }
 
-               res.extend_from_slice(&self.delayed_payment_base_key.serialize());
-               res.extend_from_slice(&self.their_htlc_base_key.as_ref().unwrap().serialize());
-               res.extend_from_slice(&self.their_delayed_payment_base_key.as_ref().unwrap().serialize());
+               writer.write_all(&self.delayed_payment_base_key.serialize())?;
+               writer.write_all(&self.their_htlc_base_key.as_ref().unwrap().serialize())?;
+               writer.write_all(&self.their_delayed_payment_base_key.as_ref().unwrap().serialize())?;
 
                match self.their_cur_revocation_points {
                        Some((idx, pubkey, second_option)) => {
-                               res.extend_from_slice(&byte_utils::be48_to_array(idx));
-                               res.extend_from_slice(&pubkey.serialize());
+                               writer.write_all(&byte_utils::be48_to_array(idx))?;
+                               writer.write_all(&pubkey.serialize())?;
                                match second_option {
                                        Some(second_pubkey) => {
-                                               res.extend_from_slice(&second_pubkey.serialize());
+                                               writer.write_all(&second_pubkey.serialize())?;
                                        },
                                        None => {
-                                               res.extend_from_slice(&[0; 33]);
+                                               writer.write_all(&[0; 33])?;
                                        },
                                }
                        },
                        None => {
-                               res.extend_from_slice(&byte_utils::be48_to_array(0));
+                               writer.write_all(&byte_utils::be48_to_array(0))?;
                        },
                }
 
-               res.extend_from_slice(&byte_utils::be16_to_array(self.our_to_self_delay));
-               res.extend_from_slice(&byte_utils::be16_to_array(self.their_to_self_delay.unwrap()));
+               writer.write_all(&byte_utils::be16_to_array(self.our_to_self_delay))?;
+               writer.write_all(&byte_utils::be16_to_array(self.their_to_self_delay.unwrap()))?;
 
                for &(ref secret, ref idx) in self.old_secrets.iter() {
-                       res.extend_from_slice(secret);
-                       res.extend_from_slice(&byte_utils::be64_to_array(*idx));
+                       writer.write_all(secret)?;
+                       writer.write_all(&byte_utils::be64_to_array(*idx))?;
                }
 
                macro_rules! serialize_htlc_in_commitment {
                        ($htlc_output: expr) => {
-                               res.push($htlc_output.offered as u8);
-                               res.extend_from_slice(&byte_utils::be64_to_array($htlc_output.amount_msat));
-                               res.extend_from_slice(&byte_utils::be32_to_array($htlc_output.cltv_expiry));
-                               res.extend_from_slice(&$htlc_output.payment_hash);
-                               res.extend_from_slice(&byte_utils::be32_to_array($htlc_output.transaction_output_index));
+                               writer.write_all(&[$htlc_output.offered as u8; 1])?;
+                               writer.write_all(&byte_utils::be64_to_array($htlc_output.amount_msat))?;
+                               writer.write_all(&byte_utils::be32_to_array($htlc_output.cltv_expiry))?;
+                               writer.write_all(&$htlc_output.payment_hash)?;
+                               writer.write_all(&byte_utils::be32_to_array($htlc_output.transaction_output_index))?;
                        }
                }
 
-               res.extend_from_slice(&byte_utils::be64_to_array(self.remote_claimable_outpoints.len() as u64));
+               writer.write_all(&byte_utils::be64_to_array(self.remote_claimable_outpoints.len() as u64))?;
                for (txid, htlc_outputs) in self.remote_claimable_outpoints.iter() {
-                       res.extend_from_slice(&txid[..]);
-                       res.extend_from_slice(&byte_utils::be64_to_array(htlc_outputs.len() as u64));
+                       writer.write_all(&txid[..])?;
+                       writer.write_all(&byte_utils::be64_to_array(htlc_outputs.len() as u64))?;
                        for htlc_output in htlc_outputs.iter() {
                                serialize_htlc_in_commitment!(htlc_output);
                        }
@@ -613,77 +614,77 @@ impl ChannelMonitor {
 
                {
                        let remote_commitment_txn_on_chain = self.remote_commitment_txn_on_chain.lock().unwrap();
-                       res.extend_from_slice(&byte_utils::be64_to_array(remote_commitment_txn_on_chain.len() as u64));
+                       writer.write_all(&byte_utils::be64_to_array(remote_commitment_txn_on_chain.len() as u64))?;
                        for (txid, commitment_number) in remote_commitment_txn_on_chain.iter() {
-                               res.extend_from_slice(&txid[..]);
-                               res.extend_from_slice(&byte_utils::be48_to_array(*commitment_number));
+                               writer.write_all(&txid[..])?;
+                               writer.write_all(&byte_utils::be48_to_array(*commitment_number))?;
                        }
                }
 
                if for_local_storage {
-                       res.extend_from_slice(&byte_utils::be64_to_array(self.remote_hash_commitment_number.len() as u64));
+                       writer.write_all(&byte_utils::be64_to_array(self.remote_hash_commitment_number.len() as u64))?;
                        for (payment_hash, commitment_number) in self.remote_hash_commitment_number.iter() {
-                               res.extend_from_slice(payment_hash);
-                               res.extend_from_slice(&byte_utils::be48_to_array(*commitment_number));
+                               writer.write_all(payment_hash)?;
+                               writer.write_all(&byte_utils::be48_to_array(*commitment_number))?;
                        }
                } else {
-                       res.extend_from_slice(&byte_utils::be64_to_array(0));
+                       writer.write_all(&byte_utils::be64_to_array(0))?;
                }
 
                macro_rules! serialize_local_tx {
                        ($local_tx: expr) => {
                                let tx_ser = serialize::serialize(&$local_tx.tx).unwrap();
-                               res.extend_from_slice(&byte_utils::be64_to_array(tx_ser.len() as u64));
-                               res.extend_from_slice(&tx_ser);
+                               writer.write_all(&byte_utils::be64_to_array(tx_ser.len() as u64))?;
+                               writer.write_all(&tx_ser)?;
 
-                               res.extend_from_slice(&$local_tx.revocation_key.serialize());
-                               res.extend_from_slice(&$local_tx.a_htlc_key.serialize());
-                               res.extend_from_slice(&$local_tx.b_htlc_key.serialize());
-                               res.extend_from_slice(&$local_tx.delayed_payment_key.serialize());
+                               writer.write_all(&$local_tx.revocation_key.serialize())?;
+                               writer.write_all(&$local_tx.a_htlc_key.serialize())?;
+                               writer.write_all(&$local_tx.b_htlc_key.serialize())?;
+                               writer.write_all(&$local_tx.delayed_payment_key.serialize())?;
 
-                               res.extend_from_slice(&byte_utils::be64_to_array($local_tx.feerate_per_kw));
-                               res.extend_from_slice(&byte_utils::be64_to_array($local_tx.htlc_outputs.len() as u64));
+                               writer.write_all(&byte_utils::be64_to_array($local_tx.feerate_per_kw))?;
+                               writer.write_all(&byte_utils::be64_to_array($local_tx.htlc_outputs.len() as u64))?;
                                for &(ref htlc_output, ref their_sig, ref our_sig) in $local_tx.htlc_outputs.iter() {
                                        serialize_htlc_in_commitment!(htlc_output);
-                                       res.extend_from_slice(&their_sig.serialize_compact(&self.secp_ctx));
-                                       res.extend_from_slice(&our_sig.serialize_compact(&self.secp_ctx));
+                                       writer.write_all(&their_sig.serialize_compact(&self.secp_ctx))?;
+                                       writer.write_all(&our_sig.serialize_compact(&self.secp_ctx))?;
                                }
                        }
                }
 
                if let Some(ref prev_local_tx) = self.prev_local_signed_commitment_tx {
-                       res.push(1);
+                       writer.write_all(&[1; 1])?;
                        serialize_local_tx!(prev_local_tx);
                } else {
-                       res.push(0);
+                       writer.write_all(&[0; 1])?;
                }
 
                if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
-                       res.push(1);
+                       writer.write_all(&[1; 1])?;
                        serialize_local_tx!(cur_local_tx);
                } else {
-                       res.push(0);
+                       writer.write_all(&[0; 1])?;
                }
 
-               res.extend_from_slice(&byte_utils::be64_to_array(self.payment_preimages.len() as u64));
+               writer.write_all(&byte_utils::be64_to_array(self.payment_preimages.len() as u64))?;
                for payment_preimage in self.payment_preimages.values() {
-                       res.extend_from_slice(payment_preimage);
+                       writer.write_all(payment_preimage)?;
                }
 
-               res.extend_from_slice(&byte_utils::be64_to_array(self.destination_script.len() as u64));
-               res.extend_from_slice(&self.destination_script[..]);
+               writer.write_all(&byte_utils::be64_to_array(self.destination_script.len() as u64))?;
+               writer.write_all(&self.destination_script[..])?;
 
-               res
+               Ok(())
        }
 
-       /// Encodes this monitor into a byte array, suitable for writing to disk.
-       pub fn serialize_for_disk(&self) -> Vec<u8> {
-               self.serialize(true)
+       /// Writes this monitor into the given writer, suitable for writing to disk.
+       pub fn write_for_disk<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
+               self.write(writer, true)
        }
 
-       /// Encodes this monitor into a byte array, suitable for sending to a remote watchtower
-       pub fn serialize_for_watchtower(&self) -> Vec<u8> {
-               self.serialize(false)
+       /// Encodes this monitor into the given writer, suitable for sending to a remote watchtower
+       pub fn write_for_watchtower<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
+               self.write(writer, false)
        }
 
        //TODO: Functions to serialize/deserialize (with different forms depending on which information

diff --git a/src/util/test_utils.rs b/src/util/test_utils.rs
index 6812f72a93ed6df10ef6cd25cf2c1b56bc490208..49aa269b3b44cbe018401cc0386afe5478ccb2aa 100644 (file)
--- a/src/util/test_utils.rs
+++ b/src/util/test_utils.rs
@@ -6,7 +6,7 @@ use ln::msgs;
 use ln::msgs::{HandleError};
 use util::events;
 use util::logger::{Logger, Level, Record};
-use util::ser::Readable;
+use util::ser::{Readable, Writer};
 
 use bitcoin::blockdata::transaction::Transaction;
 
@@ -15,6 +15,17 @@ use secp256k1::PublicKey;
 use std::sync::{Arc,Mutex};
 use std::{mem};
 
+struct VecWriter(Vec<u8>);
+impl Writer for VecWriter {
+       fn write_all(&mut self, buf: &[u8]) -> Result<(), ::std::io::Error> {
+               self.0.extend_from_slice(buf);
+               Ok(())
+       }
+       fn size_hint(&mut self, size: usize) {
+               self.0.reserve_exact(size);
+       }
+}
+
 pub struct TestFeeEstimator {
        pub sat_per_kw: u64,
 }
@@ -40,8 +51,11 @@ impl channelmonitor::ManyChannelMonitor for TestChannelMonitor {
        fn add_update_monitor(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
                // At every point where we get a monitor update, we should be able to send a useful monitor
                // to a watchtower and disk...
-               assert!(channelmonitor::ChannelMonitor::read(&mut ::std::io::Cursor::new(&monitor.serialize_for_disk()[..])).unwrap() == monitor);
-               monitor.serialize_for_watchtower(); // This at least shouldn't crash...
+               let mut w = VecWriter(Vec::new());
+               monitor.write_for_disk(&mut w).unwrap();
+               assert!(channelmonitor::ChannelMonitor::read(&mut ::std::io::Cursor::new(&w.0)).unwrap() == 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()));
                self.simple_monitor.add_update_monitor(funding_txo, monitor)
        }