Merge pull request #191 from TheBlueMatt/2018-09-chanmon-ser-framework
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Thu, 20 Sep 2018 16:14:59 +0000 (12:14 -0400)
committerGitHub <noreply@github.com>
Thu, 20 Sep 2018 16:14:59 +0000 (12:14 -0400)
Migrate ChannelMonitor writing/reading to new serialization framework

Cargo.toml
fuzz/fuzz_targets/chanmon_deser_target.rs
fuzz/fuzz_targets/router_target.rs
src/ln/channelmanager.rs
src/ln/channelmonitor.rs
src/ln/msgs.rs
src/ln/peer_handler.rs
src/util/ser.rs
src/util/test_utils.rs

index c0f13bde7cd83bea5a2de9df285ff302e7f62873..c8248518f215678bdde5c28784c4946912ea4c6a 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.0.4"
+version = "0.0.5"
 authors = ["Matt Corallo"]
 license = "Apache-2.0"
 repository = "https://github.com/rust-bitcoin/rust-lightning/"
@@ -37,3 +37,6 @@ features = ["bitcoinconsensus"]
 
 [dev-dependencies]
 hex = "0.3"
+
+[profile.dev]
+opt-level = 1
index a7e9079a73883580a2cada40fb19b95cb3b2566e..ba525081a712ef3e5b27fcdbad48efe8a53d4809 100644 (file)
@@ -5,13 +5,30 @@ extern crate lightning;
 
 use lightning::ln::channelmonitor;
 use lightning::util::reset_rng_state;
+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 Some(monitor) = channelmonitor::ChannelMonitor::deserialize(data) {
-               assert!(channelmonitor::ChannelMonitor::deserialize(&monitor.serialize_for_disk()[..]).unwrap() == monitor);
-               monitor.serialize_for_watchtower();
+       if let Ok(monitor) = channelmonitor::ChannelMonitor::read(&mut Cursor::new(data)) {
+               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();
        }
 }
 
index 4ccd32746e39a2b87a247a9581808ade4eb74459..52f9a235fe731aa93bfa94b4ccaa913a53df41bf 100644 (file)
@@ -125,15 +125,12 @@ pub fn do_test(data: &[u8]) {
                        match <($MsgType)>::read(&mut reader) {
                                Ok(msg) => msg,
                                Err(e) => match e {
-                                       msgs::DecodeError::UnknownRealmByte => return,
+                                       msgs::DecodeError::UnknownVersion => return,
                                        msgs::DecodeError::UnknownRequiredFeature => return,
-                                       msgs::DecodeError::BadPublicKey => return,
-                                       msgs::DecodeError::BadSignature => return,
-                                       msgs::DecodeError::BadText => return,
+                                       msgs::DecodeError::InvalidValue => return,
                                        msgs::DecodeError::ExtraAddressesPerType => return,
                                        msgs::DecodeError::BadLengthDescriptor => return,
                                        msgs::DecodeError::ShortRead => panic!("We picked the length..."),
-                                       msgs::DecodeError::InvalidValue => panic!("Should not happen with p2p message decoding"),
                                        msgs::DecodeError::Io(e) => panic!(format!("{}", e)),
                                }
                        }
index 9b3f7713ca403dbef3a180c05bb53f18681c46ee..01faec671c309c5a870ce732bb2bbf2d2316a23d 100644 (file)
@@ -806,7 +806,7 @@ impl ChannelManager {
                        match msgs::OnionHopData::read(&mut Cursor::new(&decoded[..])) {
                                Err(err) => {
                                        let error_code = match err {
-                                               msgs::DecodeError::UnknownRealmByte => 0x4000 | 1,
+                                               msgs::DecodeError::UnknownVersion => 0x4000 | 1, // unknown realm byte
                                                _ => 0x2000 | 2, // Should never happen
                                        };
                                        return_err!("Unable to decode our hop data", error_code, &[0;0]);
index f3eed0fe16ac0ed9d7e4a59d67f8e82bf5330e8f..acd63da79e1bd63b511bfc7ec49ed29bb3dbcebb 100644 (file)
@@ -23,11 +23,12 @@ use secp256k1::{Secp256k1,Message,Signature};
 use secp256k1::key::{SecretKey,PublicKey};
 use secp256k1;
 
-use ln::msgs::HandleError;
+use ln::msgs::{DecodeError, HandleError};
 use ln::chan_utils;
 use ln::chan_utils::HTLCOutputInCommitment;
 use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface};
 use chain::transaction::OutPoint;
+use util::ser::{Readable, Writer};
 use util::sha2::Sha256;
 use util::byte_utils;
 
@@ -530,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);
                        }
@@ -612,314 +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)
-       }
-
-       /// Attempts to decode a serialized monitor
-       pub fn deserialize(data: &[u8]) -> Option<Self> {
-               let mut read_pos = 0;
-               macro_rules! read_bytes {
-                       ($byte_count: expr) => {
-                               {
-                                       if ($byte_count as usize) > data.len() - read_pos {
-                                               return None;
-                                       }
-                                       read_pos += $byte_count as usize;
-                                       &data[read_pos - $byte_count as usize..read_pos]
-                               }
-                       }
-               }
-
-               let secp_ctx = Secp256k1::new();
-               macro_rules! unwrap_obj {
-                       ($key: expr) => {
-                               match $key {
-                                       Ok(res) => res,
-                                       Err(_) => return None,
-                               }
-                       }
-               }
-
-               let _ver = read_bytes!(1)[0];
-               let min_ver = read_bytes!(1)[0];
-               if min_ver > SERIALIZATION_VERSION {
-                       return None;
-               }
-
-               // Technically this can fail and serialize fail a round-trip, but only for serialization of
-               // barely-init'd ChannelMonitors that we can't do anything with.
-               let outpoint = OutPoint {
-                       txid: Sha256dHash::from(read_bytes!(32)),
-                       index: byte_utils::slice_to_be16(read_bytes!(2)),
-               };
-               let script_len = byte_utils::slice_to_be64(read_bytes!(8));
-               let funding_txo = Some((outpoint, Script::from(read_bytes!(script_len).to_vec())));
-               let commitment_transaction_number_obscure_factor = byte_utils::slice_to_be48(read_bytes!(6));
-
-               let key_storage = match read_bytes!(1)[0] {
-                       0 => {
-                               KeyStorage::PrivMode {
-                                       revocation_base_key: unwrap_obj!(SecretKey::from_slice(&secp_ctx, read_bytes!(32))),
-                                       htlc_base_key: unwrap_obj!(SecretKey::from_slice(&secp_ctx, read_bytes!(32))),
-                               }
-                       },
-                       _ => return None,
-               };
-
-               let delayed_payment_base_key = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
-               let their_htlc_base_key = Some(unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33))));
-               let their_delayed_payment_base_key = Some(unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33))));
-
-               let their_cur_revocation_points = {
-                       let first_idx = byte_utils::slice_to_be48(read_bytes!(6));
-                       if first_idx == 0 {
-                               None
-                       } else {
-                               let first_point = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
-                               let second_point_slice = read_bytes!(33);
-                               if second_point_slice[0..32] == [0; 32] && second_point_slice[32] == 0 {
-                                       Some((first_idx, first_point, None))
-                               } else {
-                                       Some((first_idx, first_point, Some(unwrap_obj!(PublicKey::from_slice(&secp_ctx, second_point_slice)))))
-                               }
-                       }
-               };
-
-               let our_to_self_delay = byte_utils::slice_to_be16(read_bytes!(2));
-               let their_to_self_delay = Some(byte_utils::slice_to_be16(read_bytes!(2)));
-
-               let mut old_secrets = [([0; 32], 1 << 48); 49];
-               for &mut (ref mut secret, ref mut idx) in old_secrets.iter_mut() {
-                       secret.copy_from_slice(read_bytes!(32));
-                       *idx = byte_utils::slice_to_be64(read_bytes!(8));
-               }
-
-               macro_rules! read_htlc_in_commitment {
-                       () => {
-                               {
-                                       let offered = match read_bytes!(1)[0] {
-                                               0 => false, 1 => true,
-                                               _ => return None,
-                                       };
-                                       let amount_msat = byte_utils::slice_to_be64(read_bytes!(8));
-                                       let cltv_expiry = byte_utils::slice_to_be32(read_bytes!(4));
-                                       let mut payment_hash = [0; 32];
-                                       payment_hash[..].copy_from_slice(read_bytes!(32));
-                                       let transaction_output_index = byte_utils::slice_to_be32(read_bytes!(4));
-
-                                       HTLCOutputInCommitment {
-                                               offered, amount_msat, cltv_expiry, payment_hash, transaction_output_index
-                                       }
-                               }
-                       }
-               }
-
-               let remote_claimable_outpoints_len = byte_utils::slice_to_be64(read_bytes!(8));
-               if remote_claimable_outpoints_len > data.len() as u64 / 64 { return None; }
-               let mut remote_claimable_outpoints = HashMap::with_capacity(remote_claimable_outpoints_len as usize);
-               for _ in 0..remote_claimable_outpoints_len {
-                       let txid = Sha256dHash::from(read_bytes!(32));
-                       let outputs_count = byte_utils::slice_to_be64(read_bytes!(8));
-                       if outputs_count > data.len() as u64 / 32 { return None; }
-                       let mut outputs = Vec::with_capacity(outputs_count as usize);
-                       for _ in 0..outputs_count {
-                               outputs.push(read_htlc_in_commitment!());
-                       }
-                       if let Some(_) = remote_claimable_outpoints.insert(txid, outputs) {
-                               return None;
-                       }
-               }
-
-               let remote_commitment_txn_on_chain_len = byte_utils::slice_to_be64(read_bytes!(8));
-               if remote_commitment_txn_on_chain_len > data.len() as u64 / 32 { return None; }
-               let mut remote_commitment_txn_on_chain = HashMap::with_capacity(remote_commitment_txn_on_chain_len as usize);
-               for _ in 0..remote_commitment_txn_on_chain_len {
-                       let txid = Sha256dHash::from(read_bytes!(32));
-                       let commitment_number = byte_utils::slice_to_be48(read_bytes!(6));
-                       if let Some(_) = remote_commitment_txn_on_chain.insert(txid, commitment_number) {
-                               return None;
-                       }
-               }
-
-               let remote_hash_commitment_number_len = byte_utils::slice_to_be64(read_bytes!(8));
-               if remote_hash_commitment_number_len > data.len() as u64 / 32 { return None; }
-               let mut remote_hash_commitment_number = HashMap::with_capacity(remote_hash_commitment_number_len as usize);
-               for _ in 0..remote_hash_commitment_number_len {
-                       let mut txid = [0; 32];
-                       txid[..].copy_from_slice(read_bytes!(32));
-                       let commitment_number = byte_utils::slice_to_be48(read_bytes!(6));
-                       if let Some(_) = remote_hash_commitment_number.insert(txid, commitment_number) {
-                               return None;
-                       }
-               }
-
-               macro_rules! read_local_tx {
-                       () => {
-                               {
-                                       let tx_len = byte_utils::slice_to_be64(read_bytes!(8));
-                                       let tx_ser = read_bytes!(tx_len);
-                                       let tx: Transaction = unwrap_obj!(serialize::deserialize(tx_ser));
-                                       if serialize::serialize(&tx).unwrap() != tx_ser {
-                                               // We check that the tx re-serializes to the same form to ensure there is
-                                               // no extra data, and as rust-bitcoin doesn't handle the 0-input ambiguity
-                                               // all that well.
-                                               return None;
-                                       }
-
-                                       let revocation_key = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
-                                       let a_htlc_key = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
-                                       let b_htlc_key = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
-                                       let delayed_payment_key = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
-                                       let feerate_per_kw = byte_utils::slice_to_be64(read_bytes!(8));
-
-                                       let htlc_outputs_len = byte_utils::slice_to_be64(read_bytes!(8));
-                                       if htlc_outputs_len > data.len() as u64 / 128 { return None; }
-                                       let mut htlc_outputs = Vec::with_capacity(htlc_outputs_len as usize);
-                                       for _ in 0..htlc_outputs_len {
-                                               htlc_outputs.push((read_htlc_in_commitment!(),
-                                                               unwrap_obj!(Signature::from_compact(&secp_ctx, read_bytes!(64))),
-                                                               unwrap_obj!(Signature::from_compact(&secp_ctx, read_bytes!(64)))));
-                                       }
-
-                                       LocalSignedTx {
-                                               txid: tx.txid(),
-                                               tx, revocation_key, a_htlc_key, b_htlc_key, delayed_payment_key, feerate_per_kw, htlc_outputs
-                                       }
-                               }
-                       }
-               }
-
-               let prev_local_signed_commitment_tx = match read_bytes!(1)[0] {
-                       0 => None,
-                       1 => {
-                               Some(read_local_tx!())
-                       },
-                       _ => return None,
-               };
-
-               let current_local_signed_commitment_tx = match read_bytes!(1)[0] {
-                       0 => None,
-                       1 => {
-                               Some(read_local_tx!())
-                       },
-                       _ => return None,
-               };
-
-               let payment_preimages_len = byte_utils::slice_to_be64(read_bytes!(8));
-               if payment_preimages_len > data.len() as u64 / 32 { return None; }
-               let mut payment_preimages = HashMap::with_capacity(payment_preimages_len as usize);
-               let mut sha = Sha256::new();
-               for _ in 0..payment_preimages_len {
-                       let mut preimage = [0; 32];
-                       preimage[..].copy_from_slice(read_bytes!(32));
-                       sha.reset();
-                       sha.input(&preimage);
-                       let mut hash = [0; 32];
-                       sha.result(&mut hash);
-                       if let Some(_) = payment_preimages.insert(hash, preimage) {
-                               return None;
-                       }
-               }
-
-               let destination_script_len = byte_utils::slice_to_be64(read_bytes!(8));
-               let destination_script = Script::from(read_bytes!(destination_script_len).to_vec());
-
-               Some(ChannelMonitor {
-                       funding_txo,
-                       commitment_transaction_number_obscure_factor,
-
-                       key_storage,
-                       delayed_payment_base_key,
-                       their_htlc_base_key,
-                       their_delayed_payment_base_key,
-                       their_cur_revocation_points,
-
-                       our_to_self_delay,
-                       their_to_self_delay,
-
-                       old_secrets,
-                       remote_claimable_outpoints,
-                       remote_commitment_txn_on_chain: Mutex::new(remote_commitment_txn_on_chain),
-                       remote_hash_commitment_number,
-
-                       prev_local_signed_commitment_tx,
-                       current_local_signed_commitment_tx,
-
-                       payment_preimages,
-
-                       destination_script,
-                       secp_ctx,
-               })
+       /// 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
@@ -1440,6 +1205,251 @@ impl ChannelMonitor {
        }
 }
 
+impl<R: ::std::io::Read> Readable<R> for ChannelMonitor {
+       fn read(reader: &mut R) -> Result<Self, DecodeError> {
+               // TODO: read_to_end and then deserializing from that vector is really dumb, we should
+               // actually use the fancy serialization framework we have instead of hacking around it.
+               let mut datavec = Vec::new();
+               reader.read_to_end(&mut datavec)?;
+               let data = &datavec;
+
+               let mut read_pos = 0;
+               macro_rules! read_bytes {
+                       ($byte_count: expr) => {
+                               {
+                                       if ($byte_count as usize) > data.len() - read_pos {
+                                               return Err(DecodeError::ShortRead);
+                                       }
+                                       read_pos += $byte_count as usize;
+                                       &data[read_pos - $byte_count as usize..read_pos]
+                               }
+                       }
+               }
+
+               let secp_ctx = Secp256k1::new();
+               macro_rules! unwrap_obj {
+                       ($key: expr) => {
+                               match $key {
+                                       Ok(res) => res,
+                                       Err(_) => return Err(DecodeError::InvalidValue),
+                               }
+                       }
+               }
+
+               let _ver = read_bytes!(1)[0];
+               let min_ver = read_bytes!(1)[0];
+               if min_ver > SERIALIZATION_VERSION {
+                       return Err(DecodeError::UnknownVersion);
+               }
+
+               // Technically this can fail and serialize fail a round-trip, but only for serialization of
+               // barely-init'd ChannelMonitors that we can't do anything with.
+               let outpoint = OutPoint {
+                       txid: Sha256dHash::from(read_bytes!(32)),
+                       index: byte_utils::slice_to_be16(read_bytes!(2)),
+               };
+               let script_len = byte_utils::slice_to_be64(read_bytes!(8));
+               let funding_txo = Some((outpoint, Script::from(read_bytes!(script_len).to_vec())));
+               let commitment_transaction_number_obscure_factor = byte_utils::slice_to_be48(read_bytes!(6));
+
+               let key_storage = match read_bytes!(1)[0] {
+                       0 => {
+                               KeyStorage::PrivMode {
+                                       revocation_base_key: unwrap_obj!(SecretKey::from_slice(&secp_ctx, read_bytes!(32))),
+                                       htlc_base_key: unwrap_obj!(SecretKey::from_slice(&secp_ctx, read_bytes!(32))),
+                               }
+                       },
+                       _ => return Err(DecodeError::InvalidValue),
+               };
+
+               let delayed_payment_base_key = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
+               let their_htlc_base_key = Some(unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33))));
+               let their_delayed_payment_base_key = Some(unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33))));
+
+               let their_cur_revocation_points = {
+                       let first_idx = byte_utils::slice_to_be48(read_bytes!(6));
+                       if first_idx == 0 {
+                               None
+                       } else {
+                               let first_point = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
+                               let second_point_slice = read_bytes!(33);
+                               if second_point_slice[0..32] == [0; 32] && second_point_slice[32] == 0 {
+                                       Some((first_idx, first_point, None))
+                               } else {
+                                       Some((first_idx, first_point, Some(unwrap_obj!(PublicKey::from_slice(&secp_ctx, second_point_slice)))))
+                               }
+                       }
+               };
+
+               let our_to_self_delay = byte_utils::slice_to_be16(read_bytes!(2));
+               let their_to_self_delay = Some(byte_utils::slice_to_be16(read_bytes!(2)));
+
+               let mut old_secrets = [([0; 32], 1 << 48); 49];
+               for &mut (ref mut secret, ref mut idx) in old_secrets.iter_mut() {
+                       secret.copy_from_slice(read_bytes!(32));
+                       *idx = byte_utils::slice_to_be64(read_bytes!(8));
+               }
+
+               macro_rules! read_htlc_in_commitment {
+                       () => {
+                               {
+                                       let offered = match read_bytes!(1)[0] {
+                                               0 => false, 1 => true,
+                                               _ => return Err(DecodeError::InvalidValue),
+                                       };
+                                       let amount_msat = byte_utils::slice_to_be64(read_bytes!(8));
+                                       let cltv_expiry = byte_utils::slice_to_be32(read_bytes!(4));
+                                       let mut payment_hash = [0; 32];
+                                       payment_hash[..].copy_from_slice(read_bytes!(32));
+                                       let transaction_output_index = byte_utils::slice_to_be32(read_bytes!(4));
+
+                                       HTLCOutputInCommitment {
+                                               offered, amount_msat, cltv_expiry, payment_hash, transaction_output_index
+                                       }
+                               }
+                       }
+               }
+
+               let remote_claimable_outpoints_len = byte_utils::slice_to_be64(read_bytes!(8));
+               if remote_claimable_outpoints_len > data.len() as u64 / 64 { return Err(DecodeError::BadLengthDescriptor); }
+               let mut remote_claimable_outpoints = HashMap::with_capacity(remote_claimable_outpoints_len as usize);
+               for _ in 0..remote_claimable_outpoints_len {
+                       let txid = Sha256dHash::from(read_bytes!(32));
+                       let outputs_count = byte_utils::slice_to_be64(read_bytes!(8));
+                       if outputs_count > data.len() as u64 / 32 { return Err(DecodeError::BadLengthDescriptor); }
+                       let mut outputs = Vec::with_capacity(outputs_count as usize);
+                       for _ in 0..outputs_count {
+                               outputs.push(read_htlc_in_commitment!());
+                       }
+                       if let Some(_) = remote_claimable_outpoints.insert(txid, outputs) {
+                               return Err(DecodeError::InvalidValue);
+                       }
+               }
+
+               let remote_commitment_txn_on_chain_len = byte_utils::slice_to_be64(read_bytes!(8));
+               if remote_commitment_txn_on_chain_len > data.len() as u64 / 32 { return Err(DecodeError::BadLengthDescriptor); }
+               let mut remote_commitment_txn_on_chain = HashMap::with_capacity(remote_commitment_txn_on_chain_len as usize);
+               for _ in 0..remote_commitment_txn_on_chain_len {
+                       let txid = Sha256dHash::from(read_bytes!(32));
+                       let commitment_number = byte_utils::slice_to_be48(read_bytes!(6));
+                       if let Some(_) = remote_commitment_txn_on_chain.insert(txid, commitment_number) {
+                               return Err(DecodeError::InvalidValue);
+                       }
+               }
+
+               let remote_hash_commitment_number_len = byte_utils::slice_to_be64(read_bytes!(8));
+               if remote_hash_commitment_number_len > data.len() as u64 / 32 { return Err(DecodeError::BadLengthDescriptor); }
+               let mut remote_hash_commitment_number = HashMap::with_capacity(remote_hash_commitment_number_len as usize);
+               for _ in 0..remote_hash_commitment_number_len {
+                       let mut txid = [0; 32];
+                       txid[..].copy_from_slice(read_bytes!(32));
+                       let commitment_number = byte_utils::slice_to_be48(read_bytes!(6));
+                       if let Some(_) = remote_hash_commitment_number.insert(txid, commitment_number) {
+                               return Err(DecodeError::InvalidValue);
+                       }
+               }
+
+               macro_rules! read_local_tx {
+                       () => {
+                               {
+                                       let tx_len = byte_utils::slice_to_be64(read_bytes!(8));
+                                       let tx_ser = read_bytes!(tx_len);
+                                       let tx: Transaction = unwrap_obj!(serialize::deserialize(tx_ser));
+                                       if serialize::serialize(&tx).unwrap() != tx_ser {
+                                               // We check that the tx re-serializes to the same form to ensure there is
+                                               // no extra data, and as rust-bitcoin doesn't handle the 0-input ambiguity
+                                               // all that well.
+                                               return Err(DecodeError::InvalidValue);
+                                       }
+
+                                       let revocation_key = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
+                                       let a_htlc_key = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
+                                       let b_htlc_key = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
+                                       let delayed_payment_key = unwrap_obj!(PublicKey::from_slice(&secp_ctx, read_bytes!(33)));
+                                       let feerate_per_kw = byte_utils::slice_to_be64(read_bytes!(8));
+
+                                       let htlc_outputs_len = byte_utils::slice_to_be64(read_bytes!(8));
+                                       if htlc_outputs_len > data.len() as u64 / 128 { return Err(DecodeError::BadLengthDescriptor); }
+                                       let mut htlc_outputs = Vec::with_capacity(htlc_outputs_len as usize);
+                                       for _ in 0..htlc_outputs_len {
+                                               htlc_outputs.push((read_htlc_in_commitment!(),
+                                                               unwrap_obj!(Signature::from_compact(&secp_ctx, read_bytes!(64))),
+                                                               unwrap_obj!(Signature::from_compact(&secp_ctx, read_bytes!(64)))));
+                                       }
+
+                                       LocalSignedTx {
+                                               txid: tx.txid(),
+                                               tx, revocation_key, a_htlc_key, b_htlc_key, delayed_payment_key, feerate_per_kw, htlc_outputs
+                                       }
+                               }
+                       }
+               }
+
+               let prev_local_signed_commitment_tx = match read_bytes!(1)[0] {
+                       0 => None,
+                       1 => {
+                               Some(read_local_tx!())
+                       },
+                       _ => return Err(DecodeError::InvalidValue),
+               };
+
+               let current_local_signed_commitment_tx = match read_bytes!(1)[0] {
+                       0 => None,
+                       1 => {
+                               Some(read_local_tx!())
+                       },
+                       _ => return Err(DecodeError::InvalidValue),
+               };
+
+               let payment_preimages_len = byte_utils::slice_to_be64(read_bytes!(8));
+               if payment_preimages_len > data.len() as u64 / 32 { return Err(DecodeError::InvalidValue); }
+               let mut payment_preimages = HashMap::with_capacity(payment_preimages_len as usize);
+               let mut sha = Sha256::new();
+               for _ in 0..payment_preimages_len {
+                       let mut preimage = [0; 32];
+                       preimage[..].copy_from_slice(read_bytes!(32));
+                       sha.reset();
+                       sha.input(&preimage);
+                       let mut hash = [0; 32];
+                       sha.result(&mut hash);
+                       if let Some(_) = payment_preimages.insert(hash, preimage) {
+                               return Err(DecodeError::InvalidValue);
+                       }
+               }
+
+               let destination_script_len = byte_utils::slice_to_be64(read_bytes!(8));
+               let destination_script = Script::from(read_bytes!(destination_script_len).to_vec());
+
+               Ok(ChannelMonitor {
+                       funding_txo,
+                       commitment_transaction_number_obscure_factor,
+
+                       key_storage,
+                       delayed_payment_base_key,
+                       their_htlc_base_key,
+                       their_delayed_payment_base_key,
+                       their_cur_revocation_points,
+
+                       our_to_self_delay,
+                       their_to_self_delay,
+
+                       old_secrets,
+                       remote_claimable_outpoints,
+                       remote_commitment_txn_on_chain: Mutex::new(remote_commitment_txn_on_chain),
+                       remote_hash_commitment_number,
+
+                       prev_local_signed_commitment_tx,
+                       current_local_signed_commitment_tx,
+
+                       payment_preimages,
+
+                       destination_script,
+                       secp_ctx,
+               })
+       }
+
+}
+
 #[cfg(test)]
 mod tests {
        use bitcoin::blockdata::script::Script;
index 5dd3165bfc0977728f8c80b03ba9cfeff3ecdb9f..f87e3722f999d239934241e8aa5ee046872d8fd8 100644 (file)
@@ -30,16 +30,14 @@ use util::ser::{Readable, Writeable, Writer};
 /// An error in decoding a message or struct.
 #[derive(Debug)]
 pub enum DecodeError {
-       /// Unknown realm byte in an OnionHopData packet
-       UnknownRealmByte,
+       /// A version byte specified something we don't know how to handle.
+       /// Includes unknown realm byte in an OnionHopData packet
+       UnknownVersion,
        /// Unknown feature mandating we fail to parse message
        UnknownRequiredFeature,
-       /// Failed to decode a public key (ie it's invalid)
-       BadPublicKey,
-       /// Failed to decode a signature (ie it's invalid)
-       BadSignature,
-       /// Value expected to be text wasn't decodable as text
-       BadText,
+       /// Value was invalid, eg a byte which was supposed to be a bool was something other than a 0
+       /// or 1, a public key/private key/signature was invalid, text wasn't UTF-8, etc
+       InvalidValue,
        /// Buffer too short
        ShortRead,
        /// node_announcement included more than one address of a given type!
@@ -49,8 +47,6 @@ pub enum DecodeError {
        BadLengthDescriptor,
        /// Error from std::io
        Io(::std::io::Error),
-       /// 1 or 0 is not found for boolean value
-       InvalidValue,
 }
 
 /// Tracks localfeatures which are only in init messages
@@ -614,16 +610,13 @@ pub(crate) struct OnionErrorPacket {
 impl Error for DecodeError {
        fn description(&self) -> &str {
                match *self {
-                       DecodeError::UnknownRealmByte => "Unknown realm byte in Onion packet",
+                       DecodeError::UnknownVersion => "Unknown realm byte in Onion packet",
                        DecodeError::UnknownRequiredFeature => "Unknown required feature preventing decode",
-                       DecodeError::BadPublicKey => "Invalid public key in packet",
-                       DecodeError::BadSignature => "Invalid signature in packet",
-                       DecodeError::BadText => "Invalid text in packet",
+                       DecodeError::InvalidValue => "Nonsense bytes didn't map to the type they were interpreted as",
                        DecodeError::ShortRead => "Packet extended beyond the provided bytes",
                        DecodeError::ExtraAddressesPerType => "More than one address of a single type",
                        DecodeError::BadLengthDescriptor => "A length descriptor in the packet didn't describe the later data correctly",
                        DecodeError::Io(ref e) => e.description(),
-                       DecodeError::InvalidValue => "0 or 1 is not found for boolean",
                }
        }
 }
@@ -919,7 +912,7 @@ impl<R: Read> Readable<R> for OnionHopData {
                        realm: {
                                let r: u8 = Readable::read(r)?;
                                if r != 0 {
-                                       return Err(DecodeError::UnknownRealmByte);
+                                       return Err(DecodeError::UnknownVersion);
                                }
                                r
                        },
@@ -1087,7 +1080,7 @@ impl<R: Read> Readable<R> for ErrorMessage {
                                sz = cmp::min(data_len, sz);
                                match String::from_utf8(data[..sz as usize].to_vec()) {
                                        Ok(s) => s,
-                                       Err(_) => return Err(DecodeError::BadText),
+                                       Err(_) => return Err(DecodeError::InvalidValue),
                                }
                        }
                })
index 6c2b4d798c4a002ac52c359c2fe2d5d067eefafd..797c55191f680a336a71fd58409d1e9c832f93d9 100644 (file)
@@ -374,14 +374,12 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                                                Ok(x) => x,
                                                                                Err(e) => {
                                                                                        match e {
-                                                                                               msgs::DecodeError::UnknownRealmByte => return Err(PeerHandleError{ no_connection_possible: false }),
+                                                                                               msgs::DecodeError::UnknownVersion => return Err(PeerHandleError{ no_connection_possible: false }),
                                                                                                msgs::DecodeError::UnknownRequiredFeature => {
                                                                                                        log_debug!(self, "Got a channel/node announcement with an known required feature flag, you may want to udpate!");
                                                                                                        continue;
                                                                                                },
-                                                                                               msgs::DecodeError::BadPublicKey => return Err(PeerHandleError{ no_connection_possible: false }),
-                                                                                               msgs::DecodeError::BadSignature => return Err(PeerHandleError{ no_connection_possible: false }),
-                                                                                               msgs::DecodeError::BadText => return Err(PeerHandleError{ no_connection_possible: false }),
+                                                                                               msgs::DecodeError::InvalidValue => return Err(PeerHandleError{ no_connection_possible: false }),
                                                                                                msgs::DecodeError::ShortRead => return Err(PeerHandleError{ no_connection_possible: false }),
                                                                                                msgs::DecodeError::ExtraAddressesPerType => {
                                                                                                        log_debug!(self, "Error decoding message, ignoring due to lnd spec incompatibility. See https://github.com/lightningnetwork/lnd/issues/1407");
@@ -389,7 +387,6 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                                                                },
                                                                                                msgs::DecodeError::BadLengthDescriptor => return Err(PeerHandleError{ no_connection_possible: false }),
                                                                                                msgs::DecodeError::Io(_) => return Err(PeerHandleError{ no_connection_possible: false }),
-                                                                                               msgs::DecodeError::InvalidValue => panic!("should not happen with message decoding"),
                                                                                        }
                                                                                }
                                                                        };
index 8c713dfeebef297f6751d756e2966e83fa75b37f..92cc66b81e30af9599f35bcd8cec1bcb088d2713 100644 (file)
@@ -295,7 +295,7 @@ impl<R: Read> Readable<R> for PublicKey {
                let buf: [u8; 33] = Readable::read(r)?;
                match PublicKey::from_slice(&Secp256k1::without_caps(), &buf) {
                        Ok(key) => Ok(key),
-                       Err(_) => return Err(DecodeError::BadPublicKey),
+                       Err(_) => return Err(DecodeError::InvalidValue),
                }
        }
 }
@@ -324,7 +324,7 @@ impl<R: Read> Readable<R> for Signature {
                let buf: [u8; 64] = Readable::read(r)?;
                match Signature::from_compact(&Secp256k1::without_caps(), &buf) {
                        Ok(sig) => Ok(sig),
-                       Err(_) => return Err(DecodeError::BadSignature),
+                       Err(_) => return Err(DecodeError::InvalidValue),
                }
        }
 }
index 476c9e5741dd19bb296eeb108daa35db6332de8a..49aa269b3b44cbe018401cc0386afe5478ccb2aa 100644 (file)
@@ -6,6 +6,7 @@ use ln::msgs;
 use ln::msgs::{HandleError};
 use util::events;
 use util::logger::{Logger, Level, Record};
+use util::ser::{Readable, Writer};
 
 use bitcoin::blockdata::transaction::Transaction;
 
@@ -14,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,
 }
@@ -39,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::deserialize(&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)
        }